feat: add EnableAccountVerifier configuration flag#505
Conversation
WalkthroughThis PR introduces a new optional ChangesEnableAccountVerifier Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/v1alpha1/toolchainconfig_types.go`:
- Around line 265-269: The new EnableAccountVerifier (*bool) field on
ToolchainConfig is added to the API but not propagated to host-operator; update
host-operator to read ToolchainConfig.Spec.EnableAccountVerifier (or the
ToolchainConfig status path you use) and propagate it into the
registration-service configuration (e.g., add an env var or ConfigMap key named
enableAccountVerifier consumed by registration-service) by updating the relevant
reconciler (controller) that manages registration-service deployment/template,
ensure the JSON field name enableAccountVerifier is used consistently, add any
necessary RBAC and CR watch for ToolchainConfig in the reconciler, and
add/adjust unit/integration tests to assert the field flows through from
ToolchainConfig -> host-operator -> registration-service.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: c445be49-6af6-4f1e-bbd2-2b6c2e4bb2ef
📒 Files selected for processing (4)
api/v1alpha1/docs/apiref.adocapi/v1alpha1/toolchainconfig_types.goapi/v1alpha1/zz_generated.deepcopy.goapi/v1alpha1/zz_generated.openapi.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
api/v1alpha1/docs/apiref.adocapi/v1alpha1/toolchainconfig_types.goapi/v1alpha1/zz_generated.openapi.goapi/v1alpha1/zz_generated.deepcopy.go
🔀 Multi-repo context codeready-toolchain/host-operator, codeready-toolchain/toolchain-common, codeready-toolchain/toolchain-e2e
[::codeready-toolchain/host-operator::controllers/toolchainconfig/configuration.go:116,259,276]
- host-operator consumes the API type toolchainv1alpha1.RegistrationServiceConfig and exposes accessors (RegistrationService(), RegistrationServiceConfig, VerificationConfig). The file contains methods that read registration config (Environment, Replicas, Analytics, Verification, RegistrationServiceURL, etc.).
[::codeready-toolchain/host-operator::search]
- No references to the new field name EnableAccountVerifier / enableAccountVerifier were found in host-operator.
[::codeready-toolchain/toolchain-common::search]
- No occurrences of RegistrationServiceConfig or the new field were found.
[::codeready-toolchain/toolchain-e2e::search]
- No occurrences of RegistrationServiceConfig or the new field were found.
Implication: The API addition (EnableAccountVerifier) is not yet consumed by host-operator or other linked repos; host-operator may require updates if it should act on this flag (e.g., provide accessor, defaulting, or logic changes). No other repos currently reference the new field.
🔇 Additional comments (3)
api/v1alpha1/docs/apiref.adoc (1)
2142-2145: LGTM!api/v1alpha1/zz_generated.deepcopy.go (1)
1940-1944: LGTM!api/v1alpha1/zz_generated.openapi.go (1)
2661-2667: LGTM!
| // EnableAccountVerifier controls whether the registration service acts on responses from the account-verifier service, | ||
| // which verifies disposable email domains. When set to true, the registration service will enforce the account-verifier's | ||
| // response. When set to false or not set, the account-verifier is still called but the response is only logged. | ||
| // +optional | ||
| EnableAccountVerifier *bool `json:"enableAccountVerifier,omitempty"` |
There was a problem hiding this comment.
Wire this field through host-operator before relying on it.
EnableAccountVerifier is added to the API, but linked-repo findings show no EnableAccountVerifier/enableAccountVerifier usage in codeready-toolchain/host-operator. Until host-operator consumes and propagates this field to registration-service behavior, this flag is effectively inert and does not meet the PR objective.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/v1alpha1/toolchainconfig_types.go` around lines 265 - 269, The new
EnableAccountVerifier (*bool) field on ToolchainConfig is added to the API but
not propagated to host-operator; update host-operator to read
ToolchainConfig.Spec.EnableAccountVerifier (or the ToolchainConfig status path
you use) and propagate it into the registration-service configuration (e.g., add
an env var or ConfigMap key named enableAccountVerifier consumed by
registration-service) by updating the relevant reconciler (controller) that
manages registration-service deployment/template, ensure the JSON field name
enableAccountVerifier is used consistently, add any necessary RBAC and CR watch
for ToolchainConfig in the reconciler, and add/adjust unit/integration tests to
assert the field flows through from ToolchainConfig -> host-operator ->
registration-service.
2 participants
Description
Add a configuration flag to control whether the registration service should act on responses from the account-verifier service.
Jira: https://redhat.atlassian.net/browse/SANDBOX-1837
Checks
Did you run
make generatetarget? yesDid
make generatechange anything in other projects (host-operator)? yesIn case of new CRD, did you the following? N/A
resources/setup/roles/host.yamlin the sandbox-sre repositoryPROJECTfile: https://github.com/codeready-toolchain/host-operator/blob/master/PROJECTCSVfile: https://github.com/codeready-toolchain/host-operator/blob/master/config/manifests/bases/host-operator.clusterserviceversion.yamlIn case other projects are changed, please provides PR links.
Summary by CodeRabbit