Containerize baton-cloudflare-zero-trust connector#18
Containerize baton-cloudflare-zero-trust connector#18laurenleach wants to merge 3 commits intomainfrom
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 <noreply@anthropic.com>
|
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 |
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.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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
There was a problem hiding this comment.
CI workflow go versions seem to be in a weird state, but this otherwise looks good
- 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>
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.