-
Notifications
You must be signed in to change notification settings - Fork 0
Containerize baton-temporalcloud connector #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Update baton-sdk to v0.7.10 - Create pkg/config package with generated configuration - Update main.go to use config.RunConnector API - Update connector to use V2 interface - Update Makefile for config generation and lambda support - Update GitHub workflows Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / Caller
participant Config as pkg/config.TemporalCloud
participant Connector as pkg/connector.New
participant Client as Temporal Cloud Client
participant Builder as ConnectorBuilderV2
CLI->>Config: provide configuration (api-key, allow-insecure, default role)
CLI->>Connector: call New(ctx, tc, opts)
Connector->>Client: create client with ApiKey and AllowInsecure
Connector->>Connector: validate/derive DefaultAccountRole
Connector->>Builder: construct ConnectorBuilderV2 and opts
Connector-->>CLI: return Builder and opts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pkg/config/conf.gen.go`:
- Around line 12-25: The generated method findFieldByTag should guard against a
nil receiver to avoid reflect.ValueOf(c).Elem() panics; update the generator so
the generated function checks if c == nil (and optionally if
reflect.ValueOf(c).IsNil()) before calling reflect.ValueOf(c).Elem(), and return
nil,false immediately when nil, keeping the rest of the logic (iterating
t.NumField() and matching tag) unchanged; specifically modify the generated
findFieldByTag function (receiver c) to perform the nil check at the top and
exit safely if the receiver is nil.
| func (c *TemporalCloud) findFieldByTag(tagValue string) (any, bool) { | ||
| v := reflect.ValueOf(c).Elem() // Dereference pointer to struct | ||
| t := v.Type() | ||
|
|
||
| for i := 0; i < t.NumField(); i++ { | ||
| field := t.Field(i) | ||
| tag := field.Tag.Get("mapstructure") | ||
|
|
||
| if tag == tagValue { | ||
| return v.Field(i).Interface(), true | ||
| } | ||
| } | ||
| return nil, false | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against nil receiver before reflection.
reflect.ValueOf(c).Elem() panics if c is nil. Consider adding a nil check in the generator (since this file is generated) to avoid a hard crash if accessors are called before config is initialized.
🔧 Proposed fix (apply in generator)
func (c *TemporalCloud) findFieldByTag(tagValue string) (any, bool) {
+ if c == nil {
+ return nil, false
+ }
v := reflect.ValueOf(c).Elem() // Dereference pointer to struct
t := v.Type()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (c *TemporalCloud) findFieldByTag(tagValue string) (any, bool) { | |
| v := reflect.ValueOf(c).Elem() // Dereference pointer to struct | |
| t := v.Type() | |
| for i := 0; i < t.NumField(); i++ { | |
| field := t.Field(i) | |
| tag := field.Tag.Get("mapstructure") | |
| if tag == tagValue { | |
| return v.Field(i).Interface(), true | |
| } | |
| } | |
| return nil, false | |
| } | |
| func (c *TemporalCloud) findFieldByTag(tagValue string) (any, bool) { | |
| if c == nil { | |
| return nil, false | |
| } | |
| v := reflect.ValueOf(c).Elem() // Dereference pointer to struct | |
| t := v.Type() | |
| for i := 0; i < t.NumField(); i++ { | |
| field := t.Field(i) | |
| tag := field.Tag.Get("mapstructure") | |
| if tag == tagValue { | |
| return v.Field(i).Interface(), true | |
| } | |
| } | |
| return nil, false | |
| } |
🤖 Prompt for AI Agents
In `@pkg/config/conf.gen.go` around lines 12 - 25, The generated method
findFieldByTag should guard against a nil receiver to avoid
reflect.ValueOf(c).Elem() panics; update the generator so the generated function
checks if c == nil (and optionally if reflect.ValueOf(c).IsNil()) before calling
reflect.ValueOf(c).Elem(), and return nil,false immediately when nil, keeping
the rest of the logic (iterating t.NumField() and matching tag) unchanged;
specifically modify the generated findFieldByTag function (receiver c) to
perform the nil check at the top and exit safely if the receiver is nil.
commit_and_push.sh
Outdated
| @@ -0,0 +1,23 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
guessing this file shouldn't be included :)
- Added WithDisplayName() to all fields - Marked API key as secret with WithIsSecret(true) - Added connector display name, help URL, and icon URL to Config - Converted Config to use field.NewConfiguration pattern Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
StringField with In() validation should be SelectField for proper UI rendering
Use correct SelectField signature: SelectField(name, options, ...fieldOption)
No custom validation needed, so function can be removed
Function was removed, so remove the call as well
jugonzalez12
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make generate needs to be run one more time here (lint fails), but otherwise looks good
Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@go.mod`:
- Line 3: The module declares "go 1.25.2" in go.mod; confirm and update CI/build
and container base images to a Go runtime that supports that version (or bump
this line to a newer patch like 1.25.6 if your CI images already include it).
Check CI config files (e.g., Dockerfiles, actions/workflows, buildkite/jenkins
pipelines) for pinned Go versions and either upgrade those images to a Go 1.25.x
image or change the go directive in go.mod accordingly to match the runtime your
CI provides.
| module github.com/conductorone/baton-temporalcloud | ||
|
|
||
| go 1.25 | ||
| go 1.25.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Go 1.25.2 release available known issues
💡 Result:
Go 1.25.2 availability
- Go 1.25.2 was released on 2025-10-07 and is available from the official Go downloads page and via
git checkout go1.25.2. [1] - Note: Go 1.25.2 is not the latest 1.25 patch anymore (e.g., Go 1.25.6 was released 2026-01-15). [1]
What’s in Go 1.25.2 (high level)
- Includes security fixes in
archive/tar,crypto/tls,crypto/x509,encoding/asn1,encoding/pem,net/http,net/mail,net/textproto, andnet/url, plus assorted bug fixes (compiler/runtime and a few stdlib packages). [1]
Security fixes called out publicly
- Examples from the coordinated disclosure include:
archive/tar: unbounded allocation when parsing GNU sparse map (CVE-2025-58183) [2]net/textproto: excessive CPU consumption inReader.ReadResponse(CVE-2025-61724) [2]crypto/x509: panic when validating certificate chains with DSA public keys (CVE-2025-58188) [2]
Known issues
- The Go project’s Go1.25.2 milestone shows “There are no open issues in this milestone” (i.e., no remaining tracked release-blocking/known issues for that patch release). [3]
Sources: [1] [2] [3]
Verify CI/build images support Go 1.25.2.
Go 1.25.2 is released and available (since October 2025) with no known issues. However, ensure your CI images and container base images have been updated to include this version before merging. Note: Go 1.25.6 was released on 2026-01-15 if a newer patch is preferred.
🤖 Prompt for AI Agents
In `@go.mod` at line 3, The module declares "go 1.25.2" in go.mod; confirm and
update CI/build and container base images to a Go runtime that supports that
version (or bump this line to a newer patch like 1.25.6 if your CI images
already include it). Check CI config files (e.g., Dockerfiles,
actions/workflows, buildkite/jenkins pipelines) for pinned Go versions and
either upgrade those images to a Go 1.25.x image or change the go directive in
go.mod accordingly to match the runtime your CI provides.
…nnectorBuilder - Update main.go to use config.RunConnector() pattern - Update connector.New() to match expected signature - Update workflow to capabilities_and_config.yaml - Run go mod tidy and vendor Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/connector/connector.go (1)
28-33:⚠️ Potential issue | 🔴 CriticalUpdate builders to implement ResourceSyncerV2 interface signature.
The
userBuilder,namespaceBuilder, andaccountRoleBuilderimplementations use an older signature forEntitlements()that takes*pagination.Tokenand returns(string, annotations.Annotations, error), butResourceSyncerV2requires the method to acceptresource.SyncOpAttrsand return*resource.SyncOpResults, error. All three builders must be updated to match the V2 interface signature before this code will compile.
🧹 Nitpick comments (1)
pkg/connector/connector.go (1)
85-113: Remove the unusedoptsparameter or document why it's needed. The connector uses API Key authentication from config and doesn't leverageTokenSourceorSelectedAuthMethod. If this parameter is required by the SDK interface but genuinely unused, consider clarifying in a comment. Otherwise, verify with the SDK maintainers whether API Key connectors should accept and use this parameter for consistency.
Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
The V2 SyncOp pattern update (commit aa08fbb) incorrectly removed the pagination bag logic (unmarshal, bag.Next(), bag.Marshal()) when updating method signatures. This commit restores the pagination bag logic while keeping the V2 SyncOp signatures: - Method signatures use rs.SyncOpAttrs and return *rs.SyncOpResults - Access token via opts.PageToken instead of pToken - Pagination bag logic is restored (unmarshal, bag.Next(), bag.Marshal()) Changes: - Added pagination.Bag handling to List() and Grants() methods - Restored paginate() helper function with V2 signature - Updated userBuilder to implement AccountManagerV2 interface - Updated main.go to use WithDefaultCapabilitiesConnectorBuilderV2 Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Fix lint error ST1019 by removing the duplicate import of the resource package and ensuring all references use the aliased 'rs' import. This eliminates the import conflict while maintaining code functionality. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Containerizes the connector following baton-databricks#35 and baton-contentful#48.
Co-Authored-By: Claude Sonnet 4.5 [email protected]
Summary by CodeRabbit
New Features
Chores