Skip to content

Conversation

@laurenleach
Copy link

@laurenleach laurenleach commented Jan 29, 2026

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

    • Added Temporal Cloud connector settings: API key authentication, insecure-connection toggle, and default account role selection.
    • Added a configuration schema with validation and UI metadata for those settings.
  • Chores

    • Improved configuration access and value retrieval for the Temporal Cloud connector.
    • Updated Go toolchain and dependency versions for compatibility and maintenance.

- 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]>
@laurenleach laurenleach requested a review from a team January 29, 2026 23:16
@coderabbitai
Copy link

coderabbitai bot commented Jan 29, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

  • 🔍 Trigger a full review

Walkthrough

Adds a new TemporalCloud config struct with reflection-based typed accessors, introduces an external configuration schema for the Temporal Cloud connector, updates the connector constructor and ResourceSyncers to a V2 builder + opts return, and bumps module dependencies and toolchain.

Changes

Cohort / File(s) Summary
Config Type & Accessors
pkg/config/conf.gen.go
Adds TemporalCloud struct (ApiKey string, AllowInsecure bool, DefaultAccountRole string) with mapstructure tags; adds unexported reflection helper findFieldByTag and typed accessors: GetStringSlice, GetString, GetInt, GetBool, GetStringMap. Review: reflection lookup and panic-on-type-mismatch behavior.
Configuration Schema
pkg/config/config.go
Adds external config schema and UI metadata: APIKeyField (secret, required), AllowInsecureField (bool, default false), DefaultAccountRoleField (select: read,developer,admin, default read); defines ConfigurationFields, FieldRelationships, and Config.
Connector refactor
pkg/connector/connector.go
Constructor changed to New(ctx, tc *cfg.TemporalCloud, opts *cli.ConnectorOpts) (connectorbuilder.ConnectorBuilderV2, []connectorbuilder.Opt, error) and now derives client options and DefaultAccountRole from tc; ResourceSyncers now returns []connectorbuilder.ResourceSyncerV2. Review: call sites and builder/opts handling.
Module deps / Tooling
go.mod
Bumps Go toolchain to go1.25.2, upgrades/changes multiple direct and indirect dependencies and replace directives. Review: dependency updates and replacements.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I nibbled tags and chased a key,
Hopped through structs where reflections be,
I set a role, and wired a client true,
Bumped a toolchain, then waved adieu,
A tiny rabbit's hop — config anew!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main objective of the PR: containerizing the baton-temporalcloud connector, which is reflected in the code changes that add configuration support and refactor the connector to work with container-friendly patterns.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch containerize-temporalcloud

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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.

Comment on lines +12 to +25
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
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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.

@@ -0,0 +1,23 @@
#!/bin/bash

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 :)

laurenleach and others added 7 commits January 30, 2026 12:22
- 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
Copy link

@jugonzalez12 jugonzalez12 left a 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

Copy link

@coderabbitai coderabbitai bot left a 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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 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, and net/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 in Reader.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]>
Copy link

@coderabbitai coderabbitai bot left a 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 | 🔴 Critical

Update builders to implement ResourceSyncerV2 interface signature.

The userBuilder, namespaceBuilder, and accountRoleBuilder implementations use an older signature for Entitlements() that takes *pagination.Token and returns (string, annotations.Annotations, error), but ResourceSyncerV2 requires the method to accept resource.SyncOpAttrs and 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 unused opts parameter or document why it's needed. The connector uses API Key authentication from config and doesn't leverage TokenSource or SelectedAuthMethod. 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.

@laurenleach laurenleach marked this pull request as draft February 3, 2026 20:07
laurenleach and others added 4 commits February 3, 2026 12:32
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants