OCPBUGS-83427: Prevent multiple providers being injected in ClusterSecretStore Form view#16616
Conversation
…ecretStore spec.provider The External Secrets Operator ClusterSecretStore CRD defines spec.provider as an object with maxProperties: 1, meaning exactly one provider should be set. However, the Console Form view rendered all provider sub-objects with their CRD schema defaults, and prune() only strips empty values, so all provider keys survived into the YAML — causing validation failure: "must have at most 1 items for field 'spec.provider'" Fix: add SinglePropertyObjectField to the dynamic-form layer. When a schema object has maxProperties: 1 with more than one defined property, the form now renders a dropdown to select exactly one active key and only emits that key's data via onChange. This is schema-driven and applies to any operator CRD using the maxProperties: 1 discriminated-union pattern. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@sampadasawant-36: This pull request references Jira Issue OCPBUGS-83427, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sampadasawant-36 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR adds a discriminated-union field pattern to the dynamic form system. A new utility detects JSON schemas with ChangesDiscriminated Union Dropdown Field
Sequence DiagramsequenceDiagram
participant CustomSchemaField
participant isSinglePropertyObject
participant SinglePropertyObjectField
participant SchemaField
CustomSchemaField->>isSinglePropertyObject: check resolved schema
isSinglePropertyObject-->>CustomSchemaField: true (maxProperties: 1, multiple props)
CustomSchemaField->>SinglePropertyObjectField: render with schema and formData
SinglePropertyObjectField->>SinglePropertyObjectField: determine selected key from formData or first property
SinglePropertyObjectField->>SinglePropertyObjectField: render dropdown with property keys
Note over SinglePropertyObjectField: User selects provider from dropdown
SinglePropertyObjectField->>SchemaField: render inner field for selected property schema
SchemaField->>SchemaField: render form inputs for selected property
SchemaField-->>SinglePropertyObjectField: onChange with field value
SinglePropertyObjectField-->>CustomSchemaField: onChange with {selectedKey: updatedValue}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @sampadasawant-36. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/jira refresh |
|
@sampadasawant-36: This pull request references Jira Issue OCPBUGS-83427, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
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 `@frontend/packages/console-shared/src/components/dynamic-form/fields.tsx`:
- Around line 350-361: The component currently only enforces single-key provider
selection after user interaction; fix this by normalizing pre-populated formData
on mount: after computing initialKey (using propertyNames and formData) and
initializing selectedKey state, detect if formData contains more than one
provider key and call onChange({ [initialKey]: formData[initialKey] ?? {} }) to
replace the payload with only the chosen key so stale keys are removed; keep
handleKeyChange as-is for user changes. Also add a regression test that renders
the component with formData containing two provider keys and asserts that on
mount onChange is called (or final form state) with only the single normalized
key.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5e6db521-fa8d-40a2-9c40-b43093c970c9
📒 Files selected for processing (4)
frontend/packages/console-shared/src/components/dynamic-form/__tests__/fields.spec.tsxfrontend/packages/console-shared/src/components/dynamic-form/fields.tsxfrontend/packages/console-shared/src/components/dynamic-form/utils.spec.tsfrontend/packages/console-shared/src/components/dynamic-form/utils.ts
Address CodeRabbit review: when formData already contains multiple provider keys on mount (e.g. from alm-examples), immediately call onChange with only the initially selected key so stale providers are removed before the user interacts with the form. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@sampadasawant-36: This pull request references Jira Issue OCPBUGS-83427, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Summary
ClusterSecretStoreCRD definesspec.provideras a JSON Schema object withmaxProperties: 1— meaning exactly one provider (e.g.azurekv,vault,gcpsm) may be set at a time. The Console Form view was rendering all provider sub-objects with their CRD schema defaults pre-populated. Sinceprune()only strips empty values, all provider keys survived into the YAML, causing a validation failure:must have at most 1 items for field 'spec.provider'.isSinglePropertyObject()helper inutils.tsto detect themaxProperties: 1+ multiple-properties pattern.SinglePropertyObjectFieldinfields.tsx: shows a dropdown to pick exactly one active provider, renders only that provider's sub-form, and callsonChangewith only{ [selectedKey]: data }— preventing sibling providers from being injected into the YAML.CustomSchemaFieldso it activates automatically for any operator CRD using this pattern (not just ESO).Test plan
spec.providermust have at most 1 itemserrorcd frontend && yarn test --testPathPatterns="dynamic-form/(utils|fields)"Fixes: https://redhat.atlassian.net/browse/OCPBUGS-83427
Made with Cursor
Summary by CodeRabbit
New Features
Tests