Conversation
WalkthroughAdds three config-driven integer probabilities and Client helpers that use crypto/rand + math/big to make probabilistic decisions. Connector code now calls these helpers to conditionally omit resource types, resources, or grants at runtime; connector falls back to a default builder if all types are dropped. Changes
Sequence Diagram(s)sequenceDiagram
participant Connector
participant Client
participant Config
rect rgba(200,230,255,0.3)
Note over Connector: Resource-type selection
Connector->>Client: ShouldDropRT()?
Client->>Config: read RT probability
Client-->>Connector: bool (crypto-rand)
alt keep type
Connector->>Connector: include builder
else drop type
Connector->>Connector: skip builder
end
end
rect rgba(230,255,200,0.3)
Note over Connector: Resource / Grant emission
loop per resource
Connector->>Client: ShouldDropResource()?
Client->>Config: read resource probability
Client-->>Connector: bool (crypto-rand)
alt keep resource
Connector->>Connector: append resource
loop per grant
Connector->>Client: ShouldDropGrant()?
Client->>Config: read grant probability
Client-->>Connector: bool (crypto-rand)
alt keep grant
Connector->>Connector: append grant
else drop grant
Connector->>Connector: skip grant
end
end
else drop resource
Connector->>Connector: skip resource and its grants
end
end
Note right of Connector: If no types included -> append fallback builder
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 0
🧹 Nitpick comments (1)
pkg/config/config.go (1)
28-32: Consider adding range validation for probability fields.The three probability fields accept any integer value but are documented as "int from 0 - 100". Consider adding validation similar to the count fields to ensure values stay within the expected range.
Apply this diff to add range constraints:
- ResourceDropProbability = field.IntField("resource-drop-probability", field.WithDescription("The probability we should drop any given resource from a sync, int from 0 - 100"), field.WithDefaultValue(0)) - RTDropProbability = field.IntField("resource-type-drop-probability", field.WithDescription("The probability we should drop any whole resource type from the sync, int from 0 - 100"), field.WithDefaultValue(0)) - GrantDropProbability = field.IntField("grant-drop-probability", field.WithDescription("The probability we should drop any grant from the sync, int from 0 - 100"), field.WithDefaultValue(0)) + ResourceDropProbability = field.IntField("resource-drop-probability", field.WithDescription("The probability we should drop any given resource from a sync, int from 0 - 100"), field.WithDefaultValue(0), field.WithInt(func(r *field.IntRuler) { r.Gte(0); r.Lte(100) })) + RTDropProbability = field.IntField("resource-type-drop-probability", field.WithDescription("The probability we should drop any whole resource type from the sync, int from 0 - 100"), field.WithDefaultValue(0), field.WithInt(func(r *field.IntRuler) { r.Gte(0); r.Lte(100) })) + GrantDropProbability = field.IntField("grant-drop-probability", field.WithDescription("The probability we should drop any grant from the sync, int from 0 - 100"), field.WithDefaultValue(0), field.WithInt(func(r *field.IntRuler) { r.Gte(0); r.Lte(100) }))This would catch configuration errors early (e.g.,
--resource-drop-probability=150) and provide clear error messages to users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
pkg/client/client.go(2 hunks)pkg/config/conf.gen.go(1 hunks)pkg/config/config.go(2 hunks)pkg/connector/connector.go(1 hunks)pkg/connector/groups.go(3 hunks)pkg/connector/projects.go(3 hunks)pkg/connector/roles.go(3 hunks)pkg/connector/users.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
pkg/connector/roles.go (1)
vendor/github.com/conductorone/baton-sdk/pkg/types/grant/grant.go (1)
NewGrant(59-94)
pkg/config/conf.gen.go (1)
pkg/config/config.go (2)
ResourceDropProbability(30-30)GrantDropProbability(32-32)
pkg/config/config.go (1)
vendor/github.com/conductorone/baton-sdk/pkg/field/field_options.go (1)
WithDefaultValue(62-68)
pkg/client/client.go (1)
pkg/config/config.go (2)
ResourceDropProbability(30-30)GrantDropProbability(32-32)
pkg/connector/connector.go (1)
vendor/github.com/conductorone/baton-sdk/pkg/connectorbuilder/connectorbuilder.go (1)
ResourceSyncer(49-54)
pkg/connector/groups.go (1)
vendor/github.com/conductorone/baton-sdk/pkg/types/grant/grant.go (1)
NewGrant(59-94)
🪛 GitHub Actions: ci
pkg/client/client.go
[error] 129-129: GolangCI-Lint (gosec) G404: Use of weak random number generator (math/rand) instead of crypto/rand.
🪛 GitHub Check: go-lint
pkg/client/client.go
[failure] 137-137:
G404: Use of weak random number generator (math/rand or math/rand/v2 instead of crypto/rand) (gosec)
[failure] 133-133:
G404: Use of weak random number generator (math/rand or math/rand/v2 instead of crypto/rand) (gosec)
[failure] 129-129:
G404: Use of weak random number generator (math/rand or math/rand/v2 instead of crypto/rand) (gosec)
🔇 Additional comments (14)
pkg/connector/users.go (1)
85-87: LGTM: Conditional resource dropping logic.The implementation correctly gates resource inclusion based on the chaos configuration. This aligns with the probabilistic dropping behavior introduced across the connector.
pkg/config/conf.gen.go (1)
13-15: LGTM: Generated configuration fields.The three new probability fields are correctly added to the configuration struct with appropriate mapstructure tags.
pkg/connector/roles.go (3)
57-59: LGTM: Resource dropping logic.The conditional append correctly implements probabilistic resource dropping for roles.
134-136: LGTM: Grant dropping logic for direct assignments.The implementation correctly gates direct role assignment grants based on the chaos configuration.
164-166: LGTM: Grant dropping logic for group assignments.The conditional append correctly applies probabilistic dropping to group-based role assignments.
pkg/connector/projects.go (3)
48-50: LGTM: Resource dropping logic.The conditional append correctly implements probabilistic resource dropping for projects.
110-114: LGTM: Owner grant with probabilistic dropping.The owner grant logic correctly combines pagination (offset == 0) with chaos testing (ShouldDropGrant). The owner grants are appropriately returned only on the first page.
132-134: LGTM: Grant dropping logic for group assignments.The implementation correctly gates group access grants based on the chaos configuration.
pkg/connector/connector.go (1)
20-35: LGTM: Dynamic resource syncer construction with fallback.The implementation correctly applies probabilistic filtering to resource types while ensuring at least one type remains (user builder fallback). This defensive approach prevents sync failures when all types are dropped.
pkg/connector/groups.go (3)
62-64: LGTM: Resource dropping logic.The conditional append correctly implements probabilistic resource dropping for groups.
142-145: LGTM: Atomic grant dropping for admin entitlements.The implementation correctly drops both admin and member grants atomically when
ShouldDropGrant()returns true. This preserves the semantic that admins receive both entitlements as a unit.
166-168: LGTM: Grant dropping logic for members.The conditional append correctly applies probabilistic dropping to member grants.
pkg/config/config.go (1)
45-47: LGTM: Configuration fields registered.The three new probability fields are correctly added to the configuration slice for runtime initialization.
pkg/client/client.go (1)
128-138: The review comment is based on outdated assumptions about Go's random seeding behavior.In Go 1.20 and later, the
math/randpackage automatically seeds its global generator at program startup. Since the codebase targets Go 1.20 (confirmed in go.mod), explicit seeding withrand.Seed()is unnecessary. Additionally,rand.Seed()was deprecated in Go 1.20.The current code is correct:
rand.Intn()will produce different sequences across runs without explicit seeding in Go 1.20+. Adding the proposedrand.Seed(time.Now().UnixNano())would inject deprecated code that contradicts Go's evolution and provides no functional benefit for the target version.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/config/config.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/config/config.go (1)
vendor/github.com/conductorone/baton-sdk/pkg/field/field_options.go (2)
WithDefaultValue(62-68)WithInt(155-165)
🔇 Additional comments (1)
pkg/config/config.go (1)
63-65: LGTM!The new probability fields are correctly added to the configuration.
1 participant
Adds cli options to baton-demo to probabilistically drop resources, grants, or entire resource types.
Maybe this isn't the right place for this, maybe this should be a different type of connector builder, or live in a different baton-chaos connector, but it's helpful for my testing.
Summary by CodeRabbit