Containerize baton-cloudflare-zero-trust connector - #18
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughUpgrades Go toolchain and dependencies (notably Baton SDK), adds a Cloudflare Zero Trust configuration module with typed accessors and field relationships, and refactors credential-selection logic to an if/else flow preventing client override. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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)
47-60: Guard against missing/partial credentials to avoid a nil client.Line 52-56 only initializes the client for valid credential paths; otherwise
clientstays nil anderrnil, soNewreturns a connector that will crash later. Consider returning a clear error when credentials are missing or incomplete.🔧 Proposed fix
import ( "context" + "errors" "github.com/cloudflare/cloudflare-go" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" "github.com/conductorone/baton-sdk/pkg/annotations" "github.com/conductorone/baton-sdk/pkg/connectorbuilder" ) @@ - if apiKey != "" && email != "" { - client, err = cloudflare.New(apiKey, email) - } else if apiToken != "" { - client, err = cloudflare.NewWithAPIToken(apiToken) - } + if apiKey != "" || email != "" { + if apiKey == "" || email == "" { + return nil, errors.New("api-key and email must be provided together") + } + client, err = cloudflare.New(apiKey, email) + } else if apiToken != "" { + client, err = cloudflare.NewWithAPIToken(apiToken) + } else { + return nil, errors.New("missing credentials: provide api-token or api-key+email") + }
🤖 Fix all issues with AI agents
In `@pkg/config/conf.gen.go`:
- Around line 13-15: In findFieldByTag, add a nil check for the receiver c
before calling reflect.ValueOf(c).Elem(): if c == nil return nil,false to avoid
a panic when the generated accessor is called with a nil receiver; update the
generated method (CloudflareZeroTrust.findFieldByTag) to guard early and return
a safe zero value and false when c is nil.
| func (c *CloudflareZeroTrust) findFieldByTag(tagValue string) (any, bool) { | ||
| v := reflect.ValueOf(c).Elem() // Dereference pointer to struct | ||
| t := v.Type() |
There was a problem hiding this comment.
Add a nil receiver guard to avoid panics in reflection.
Line 14 will panic if c is nil. Even if callers are expected to pass non-nil, a defensive guard makes generated accessors safer.
🛡️ Proposed fix (apply in generator)
func (c *CloudflareZeroTrust) 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 *CloudflareZeroTrust) findFieldByTag(tagValue string) (any, bool) { | |
| v := reflect.ValueOf(c).Elem() // Dereference pointer to struct | |
| t := v.Type() | |
| func (c *CloudflareZeroTrust) findFieldByTag(tagValue string) (any, bool) { | |
| if c == nil { | |
| return nil, false | |
| } | |
| v := reflect.ValueOf(c).Elem() // Dereference pointer to struct | |
| t := v.Type() |
🤖 Prompt for AI Agents
In `@pkg/config/conf.gen.go` around lines 13 - 15, In findFieldByTag, add a nil
check for the receiver c before calling reflect.ValueOf(c).Elem(): if c == nil
return nil,false to avoid a panic when the generated accessor is called with a
nil receiver; update the generated method (CloudflareZeroTrust.findFieldByTag)
to guard early and return a safe zero value and false when c is nil.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@go.mod`:
- Line 3: The CI test job is hardcoded to Go 1.23.x causing a mismatch with
go.mod (go 1.25.2); edit the test job's Install Go step (the "test" job and its
"Install Go" step in ci.yaml) and replace the go-version: 1.23.x setting with
go-version-file: 'go.mod' so the action reads the Go version from go.mod and
aligns the pipeline versions.
| module github.com/conductorone/baton-cloudflare-zero-trust | ||
|
|
||
| go 1.23.4 | ||
| go 1.25.2 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Go version references in configs
rg -n -C 2 'go 1\.25\.2|GOLANG_VERSION|golang:1\.25|1\.25\.2|go 1\.|Go version' --type=toml --type=yaml --type=dockerfile --type=sh .Repository: ConductorOne/baton-cloudflare-zero-trust
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Also look for CI workflow files
fd -e yaml -e yml . .github/workflows 2>/dev/null | head -20Repository: ConductorOne/baton-cloudflare-zero-trust
Length of output: 187
🏁 Script executed:
#!/bin/bash
# Check for Dockerfiles
fd -i dockerfile . | head -20Repository: ConductorOne/baton-cloudflare-zero-trust
Length of output: 193
🏁 Script executed:
#!/bin/bash
# Check for any build/CI related files
find . -maxdepth 3 -type f \( -name ".gitlab-ci.yml" -o -name "*.mk" -o -name "Makefile" -o -name "tox.ini" -o -name ".travis.yml" \) 2>/dev/nullRepository: ConductorOne/baton-cloudflare-zero-trust
Length of output: 94
🏁 Script executed:
cat .github/workflows/ci.yamlRepository: ConductorOne/baton-cloudflare-zero-trust
Length of output: 2312
🏁 Script executed:
cat .github/workflows/release.yamlRepository: ConductorOne/baton-cloudflare-zero-trust
Length of output: 720
🏁 Script executed:
cat .github/workflows/capabilities_and_config.yamlRepository: ConductorOne/baton-cloudflare-zero-trust
Length of output: 1129
🏁 Script executed:
cat MakefileRepository: ConductorOne/baton-cloudflare-zero-trust
Length of output: 963
🏁 Script executed:
# Check if there's any docker-compose or other container config
fd -i docker-compose . --maxdepth 2Repository: ConductorOne/baton-cloudflare-zero-trust
Length of output: 66
Update test job in ci.yaml to use Go 1.25.2 from go.mod.
The test job hardcodes go-version: 1.23.x while other CI jobs and go.mod specify 1.25.2. This version mismatch can cause build inconsistencies. Update it to go-version-file: 'go.mod' to align with the rest of the pipeline.
ci.yaml test job (line ~39)
test:
runs-on: ubuntu-latest
steps:
- name: Install Go
uses: actions/setup-go@v5
with:
go-version: 1.23.x # Change to: go-version-file: 'go.mod'🤖 Prompt for AI Agents
In `@go.mod` at line 3, The CI test job is hardcoded to Go 1.23.x causing a
mismatch with go.mod (go 1.25.2); edit the test job's Install Go step (the
"test" job and its "Install Go" step in ci.yaml) and replace the go-version:
1.23.x setting with go-version-file: 'go.mod' so the action reads the Go version
from go.mod and aligns the pipeline versions.
jugonzalez12
left a comment
jugonzalez12
left a comment
There was a problem hiding this comment.
CI workflow go versions seem to be in a weird state, but this otherwise looks good
|
Claude finished @laurenleach's task in 2m 40s —— View job PR Review: Containerize baton-cloudflare-zero-trust connector
OverviewThis PR migrates the connector to the newer baton-sdk patterns: Issues Found1. Critical: Field validation constraints were dropped
field.FieldsAtLeastOneUsed(apiTokenField, apiKeyField),
field.FieldsMutuallyExclusive(apiTokenField, apiKeyField),
field.FieldsDependentOn([]field.SchemaField{apiKeyField}, []field.SchemaField{emailField}),The new config uses This means the connector could be started with no credentials (no api-token, no api-key), or with api-key but no email, and the validation wouldn't catch it — the Additionally, all four fields are now marked 2. Critical:
|
- 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 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Updated main.go to use config.RunConnector instead of DefineConfiguration - Updated connector.New signature to accept config and opts parameters - Updated all resource syncers (users, groups, roles, members) to use V2 interface with SyncOpAttrs and SyncOpResults - Ran go mod tidy and go mod vendor to update dependencies Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
f18826e to
a2af02f
Compare
|
Claude finished @laurenleach's task in 2m 1s —— View job
OverviewThis PR migrates the connector to newer baton-sdk patterns ( Issues Found1. Critical: Field validation constraints dropped
field.FieldsAtLeastOneUsed(apiTokenField, apiKeyField),
field.FieldsMutuallyExclusive(apiTokenField, apiKeyField),
field.FieldsDependentOn([]field.SchemaField{apiKeyField}, []field.SchemaField{emailField}),The new config uses Additionally, all four fields are marked Recommendation: Remove var Config = field.NewConfiguration(
configurationFields,
field.WithConnectorDisplayName("Cloudflare Zero Trust"),
// ...existing options...
field.WithFieldGroups([]field.SchemaFieldGroup{...}),
field.WithConstraints(
field.FieldsAtLeastOneUsed(apiTokenField, apiKeyField),
field.FieldsMutuallyExclusive(apiTokenField, apiKeyField),
field.FieldsDependentOn([]field.SchemaField{apiKeyField}, []field.SchemaField{emailField}),
),
)2. Critical:
|
|
Claude finished @laurenleach's task in 1m 57s —— View job
OverviewThis PR migrates the connector to the newer baton-sdk patterns: Issues Found1. Critical: Field validation constraints dropped — users can start the connector with invalid credentials
field.FieldsAtLeastOneUsed(apiTokenField, apiKeyField),
field.FieldsMutuallyExclusive(apiTokenField, apiKeyField),
field.FieldsDependentOn([]field.SchemaField{apiKeyField}, []field.SchemaField{emailField}),These were replaced with The old config correctly had Fix: Remove 2. Critical:
|
3 participants
Containerizes the connector following baton-databricks#35 and baton-contentful#48.
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.