ESO-540/ESO-550: Adds concurrent flag, replicas, and experimentalOverrides to ExternalSecretsConfig - #2068
ESO-540/ESO-550: Adds concurrent flag, replicas, and experimentalOverrides to ExternalSecretsConfig#2068siddhibhor-56 wants to merge 2 commits into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@siddhibhor-56: This pull request references ESO-533 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "5.0.0" version, but no target version was set. 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. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe proposal expands ChangesExternal Secrets controller configuration
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
siddhibhor-56
changed the title
siddhibhor-56
changed the title
siddhibhor-56
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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 `@enhancements/external-secrets-operator/external-secrets-component-config.md`:
- Line 22: Update the Jira metadata for ESO-533 so its target version is 5.0.0,
or document the intentional exception in the external-secrets component
configuration if the target version should remain unset.
- Around line 191-198: Update the workflow and support sections around the
visible forbidden-path references, including the diagram’s “Reject forbidden
nested lists” step, to consistently include containers, initContainers, volumes,
ephemeralContainers, volumeMounts, and volumeDevices. Preserve the existing
forbidden-path handling and ensure every mention uses this complete normative
list.
- Around line 105-115: Update the trustedCABundle mount decision in the External
Secrets controller reconciliation to skip the user CA volume only when the
referenced ConfigMap is confirmed to be the known CNO/proxy-managed bundle
already mounted in the controller pod. Do not skip mounting solely because the
ConfigMap has the injection label; mount arbitrary labeled references normally,
while preserving existing optional and invalid-PEM handling.
- Around line 471-479: Update the required podAntiAffinity selectors in both
examples around the affinity configuration to use the exact labels emitted on
the operator-rendered core-controller pods instead of hard-coded
app.kubernetes.io/name: external-secrets. Verify the selector matches those pods
and add or update the relevant test to enforce that match.
- Around line 140-152: The experimentalOverrides validation described in the
component configuration must enforce an explicit allowlist rather than only
rejecting forbidden paths. Update the override validation or application logic
associated with the external secrets component to permit only the documented
scheduling paths under Deployment.spec, such as template.spec.affinity,
tolerations, nodeSelector, and topologySpreadConstraints, and reject every other
path—including currently unlisted fields—by setting Degraded and skipping the
patch.
- Around line 680-685: Clarify the Downgrade section to document the supported
downgrade procedure: distinguish operator-image rollback from the active CRD
schema transition, specify the required CRD/version change, and describe backup
and reapply expectations when fields are rejected or pruned. State the exact
supported downgrade path before describing the resulting behavior or
availability risks.
- Around line 696-700: The upgrade discussion must not describe OLM bundle
upgrades as atomic or imply there is no version skew. Update the section around
the ExternalSecretsConfig fields to document the supported upgrade order,
acknowledge that operator and operand Deployments may temporarily run different
revisions, and specify the behavior when ExternalSecretsConfig is changed before
the new operator finishes rolling out.
In `@enhancements/installer/gcp-sovereign-cloud.md`:
- Around line 320-362: Update validateGCPServices to probe all six required
services listed in requiredServices, rather than validating only Compute. Add
bounded API-access checks for Cloud Resource Manager, DNS, IAM, IAM Credentials,
Service Usage, and Compute; classify any non-required capabilities separately as
optional, and return a clear service-specific error immediately when a required
probe is unavailable so provisioning does not begin.
- Around line 654-692: The GCD installer documentation does not specify
enforceable defaults or preflight checks for unsupported resources. Update the
installer configuration and provisioning flow to use supported C3/M3/A3 Edge
machine types and Hyperdisk Balanced disks, and validate load balancers, DNS
zones, and SSL certificates before infrastructure creation, failing fast with
actionable errors for unsupported global, public, or managed options.
- Around line 340-352: Make the snippets around validateGCPServices and the
related resource-manager example self-contained: define or pass projectID
explicitly, expose client.projectID if it is intended, and provide the
GetResourceManagerService helper before use. If these examples are intentionally
illustrative, label them clearly as pseudocode instead of leaving undefined
references.
- Around line 373-400: Update validateRegion to follow the nextPageToken
returned by computeSvc.Regions.List(client.projectID).Do(), fetching and
scanning every regions page before concluding the region is unavailable.
Accumulate region names from all pages so both validation and the
available-regions error message include the complete result set.
- Around line 62-69: Unify the GCP configuration contract across
enhancements/installer/gcp-sovereign-cloud.md:62-69 by defining universeDomain
as the canonical source, its precedence over cloudEnvironment and other inputs,
defaults, and mismatch behavior; update
enhancements/installer/gcp-sovereign-cloud.md:591-603 so the GCD example uses
only documented install-config fields; and update
enhancements/installer/gcp-sovereign-cloud.md:791-805 so version-skew handling
derives from the same source of truth used for component discovery.
- Around line 535-547: Update the credential-loading flow before
compute.NewService and the other GCP client constructors to use the typed
authentication loader instead of deprecated option.WithCredentialsJSON. Validate
the parsed credential type and universe/endpoint fields, rejecting unexpected
values before creating any client, while preserving automatic universe-domain
endpoint and authentication selection for supported credentials.
- Around line 218-240: Align the authentication guidance in
enhancements/installer/gcp-sovereign-cloud.md lines 218-240 and 471-508 to one
consistent current Go client-library contract: update the examples to use
option.WithAuthCredentialsJSON with option.ServiceAccount and credentials JSON,
and describe self-signed JWT as being derived by the SDK from the
service-account key rather than embedded in the JSON. Ensure both sections
consistently explain whether WithUniverseDomain is required, without mixing
incompatible authentication paths.
🪄 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: dce47c73-3036-4784-bc38-611c4f38ee6c
📒 Files selected for processing (2)
enhancements/external-secrets-operator/external-secrets-component-config.mdenhancements/installer/gcp-sovereign-cloud.md
| `**ConfigMap` with the CNO injection label:** If the `ConfigMap` referenced by `trustedCABundle` is labeled with `config.openshift.io/inject-trusted-cabundle: "true"`, operator **skips** mounting that reference for `trustedCABundle`. | ||
|
|
||
| **Interaction with `overrideEnv`:** The operator owns `**SSL_CERT_DIR`** (and, when applicable, `**SSL_CERT_FILE**`) on the **External Secrets core controller** for proxy/CNO trust and for `**trustedCABundle`** injection. `**overrideEnv**` therefore **must not** set `**SSL_CERT_DIR`** or `**SSL_CERT_FILE**` on **any** operand component: the `**ExternalSecretsConfig`** CRD extends the existing `**overrideEnv**` CEL rule so the API server **rejects** those names up front (same pattern as reserved prefixes such as `KUBERNETES_`). No runtime “ignore vs reject” choice is required for a valid CR. Concurrency is **not** configurable via `overrideEnv`; use the first-class `**concurrent`** field (the operator builds container `args`, not an env var, for `--concurrent`). | ||
|
|
||
|
|
||
| | Situation | Expected behaviour | | ||
| | ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `optional: false` (default) and missing `ConfigMap` or key | **Degraded**; do not patch the controller `Deployment` until valid. | | ||
| | `optional: true` and missing `ConfigMap` or key | Skip user bundle; no error for the missing reference alone. | | ||
| | Present key with **invalid PEM** | **Degraded** regardless of `optional`. | | ||
| | Referenced `ConfigMap` has `**config.openshift.io/inject-trusted-cabundle: "true"`** | A `ConfigMap` is already created, when proxy is configured, and its contents are mounted at `/etc/pki/tls/certs` path. Mounting it again under `/etc/pki/tls/user-certs` would be a duplicate. The operator **skips** the trustedCABundle volume mount. | |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not skip the user CA mount based only on the injection label.
A config.openshift.io/inject-trusted-cabundle: "true" label does not by itself prove that this referenced ConfigMap is the CNO-managed bundle already mounted in the controller pod. Skipping the mount for an arbitrary labeled ConfigMap can leave the controller without the requested trust roots. Restrict this optimization to the known CNO/proxy-mounted bundle and otherwise mount the referenced ConfigMap.
🤖 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 `@enhancements/external-secrets-operator/external-secrets-component-config.md`
around lines 105 - 115, Update the trustedCABundle mount decision in the
External Secrets controller reconciliation to skip the user CA volume only when
the referenced ConfigMap is confirmed to be the known CNO/proxy-managed bundle
already mounted in the controller pod. Do not skip mounting solely because the
ConfigMap has the injection label; mount arbitrary labeled references normally,
while preserving existing optional and invalid-PEM handling.
| template: | ||
| spec: | ||
| affinity: | ||
| podAntiAffinity: | ||
| requiredDuringSchedulingIgnoredDuringExecution: | ||
| - labelSelector: | ||
| matchLabels: | ||
| app.kubernetes.io/name: external-secrets | ||
| topologyKey: kubernetes.io/hostname |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use labels guaranteed to match the operand pods.
The required anti-affinity examples hard-code app.kubernetes.io/name: external-secrets. If the operator-rendered pod labels differ, the selector matches nothing and replicas can co-locate, defeating the HA example. Use the exact emitted labels and test that the selector matches the core-controller pods.
Also applies to: 537-546
🤖 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 `@enhancements/external-secrets-operator/external-secrets-component-config.md`
around lines 471 - 479, Update the required podAntiAffinity selectors in both
examples around the affinity configuration to use the exact labels emitted on
the operator-rendered core-controller pods instead of hard-coded
app.kubernetes.io/name: external-secrets. Verify the selector matches those pods
and add or update the relevant test to enforce that match.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
openshift-ci
Bot
removed
the
needs-rebase
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
enhancements/external-secrets-operator/external-secrets-component-config.md (1)
291-291: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
=for these Kubebuilder validation markers.
Enum:=,Minimum:=,Maximum:=,MinItems:=,MinLength:=,MaxLength:=, andPattern:=should all be=here, otherwise the generated CRD will miss the intended constraints.🤖 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 `@enhancements/external-secrets-operator/external-secrets-component-config.md` at line 291, Update the Kubebuilder validation marker near the component enum to use the supported equals syntax, changing the Enum marker from `Enum:=` to `Enum=` so the generated CRD preserves the constraint.
🤖 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.
Outside diff comments:
In `@enhancements/external-secrets-operator/external-secrets-component-config.md`:
- Line 291: Update the Kubebuilder validation marker near the component enum to
use the supported equals syntax, changing the Enum marker from `Enum:=` to
`Enum=` so the generated CRD preserves the constraint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 48b40099-9e37-4461-830e-e17b62e43ede
📒 Files selected for processing (1)
enhancements/external-secrets-operator/external-secrets-component-config.md
siddhibhor-56
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@enhancements/external-secrets-operator/external-secrets-component-config.md`:
- Around line 75-82: Update the Non-Goals wording around component-level
scheduling configuration to clarify that affinity, anti-affinity, tolerations,
and node selectors are excluded only as first-class API fields. Explicitly state
that these settings remain available through the documented
experimentalOverrides escape hatch, while preserving the existing restrictions
on operator-owned containers, initContainers, and volumes.
- Around line 635-637: Update the test plan around experimentalOverrides to
separate rejected protected-field patches from successful allowed scheduling
patches. Verify patches targeting image, --enable-leader-election, concurrent,
replicas, containers, initContainers, or volumes produce Degraded and leave the
Deployment unchanged; separately verify allowed scheduling patches succeed while
preserving the operator baseline.
- Around line 135-150: Remove the statement that webhook, cert-controller, and
Bitwarden replica counts can be configured through experimentalOverrides, since
the forbidden-path rules reject replicas patches. Update the surrounding
experimentalOverrides documentation to state that non-core replica counts are
unsupported while preserving the existing replicas prohibition.
🪄 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: 3acfa3ce-91fb-4fab-8f7a-9aa10d486786
📒 Files selected for processing (1)
enhancements/external-secrets-operator/external-secrets-component-config.md
siddhibhor-56
changed the title
|
@siddhibhor-56: This pull request references ESO-550 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. 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. |
siddhibhor-56
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
enhancements/external-secrets-operator/external-secrets-component-config.md (1)
13-22: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSet the target version for ESO-550.
OpenShift CI confirms that ESO-550 is valid but has no target version, while this branch expects
5.0.0. Update the Jira metadata or document the intentional exception before merge.🤖 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 `@enhancements/external-secrets-operator/external-secrets-component-config.md` around lines 13 - 22, Update the Jira metadata associated with ESO-550 in the tracking-link section to set its target version to 5.0.0. If that metadata cannot be changed, document the intentional target-version exception in this component configuration instead.
🤖 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 `@enhancements/external-secrets-operator/external-secrets-component-config.md`:
- Around line 629-635: The test plan for experimentalOverrides must explicitly
verify that forbidden patches both set the component to Degraded and leave the
Deployment unchanged. Add parameterized rejection cases covering image,
args/command, trust-related environment variables, concurrent, replicas,
ephemeralContainers, volumeMounts, and volumeDevices, while keeping successful
scheduling-only patches in a separate test group.
- Line 188: Update the forbidden nested-list guidance in the workflow and
support references, including the diagram at the visible Op->>Op step and the
corresponding sections, to consistently name containers, initContainers,
ephemeralContainers, volumes, volumeMounts, and volumeDevices. Replace
abbreviated “mounts” wording with the exact normative field names while
preserving the existing validation guidance.
- Around line 139-153: Update the override contract documentation to define an
explicit allowlist: only the listed scheduling and placement paths may be
patched, and every other Deployment.spec path must be rejected. Resolve the
non-core replicas inconsistency by either removing its support or documenting it
as an explicit validated exception, ensuring the stated scaling contract and
validation behavior agree.
---
Outside diff comments:
In `@enhancements/external-secrets-operator/external-secrets-component-config.md`:
- Around line 13-22: Update the Jira metadata associated with ESO-550 in the
tracking-link section to set its target version to 5.0.0. If that metadata
cannot be changed, document the intentional target-version exception in this
component configuration instead.
🪄 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: 87fadc52-3755-4962-9a08-8e27f779755c
📒 Files selected for processing (1)
enhancements/external-secrets-operator/external-secrets-component-config.md
| - **Allowed paths** (scheduling / placement fields), for example: | ||
| - `template.spec.affinity` | ||
| - `template.spec.tolerations` | ||
| - `template.spec.nodeSelector` | ||
| - `template.spec.topologySpreadConstraints` | ||
| - **Forbidden paths** — the operator rejects patches targeting these and sets **Degraded**: | ||
| - `template.spec.containers`, `initContainers`, `ephemeralContainers` | ||
| - `template.spec.volumes` | ||
| - `template.spec.containers[*].volumeMounts` / `volumeDevices` (same for init containers) | ||
| - Operator-owned scalars that conflict with dedicated fields on the **core controller**: `replicas` (use the dedicated field instead), container `image`, `args` / `command`, `SSL_CERT_DIR` / `SSL_CERT_FILE`. For non-core components (webhook, cert-controller, Bitwarden), `replicas` is allowed via `experimentalOverrides` since no dedicated field exists. | ||
| - **Apply order** per component Deployment: | ||
| 1. Start from the operator baseline (bindata). | ||
| 2. Apply operator-managed settings (image, args, trusted CA mounts, labels, annotations, `revisionHistoryLimit`, `overrideEnv`, `concurrent`, `replicas`). | ||
| 3. Check `experimentalOverrides` against forbidden paths; if violated, set **Degraded** and skip the patch. | ||
| 4. Strategic-merge-patch `Deployment.spec` with the allowed overrides. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Make the override contract an explicit allowlist.
“Allowed paths … for example” combined with forbidden-path checks leaves every unlisted Deployment.spec field implicitly patchable. That can expose selectors, service accounts, networking, security settings, and rollout behavior. Also resolve whether non-core replicas are truly supported; that exception conflicts with the scheduling-only scope and the core-only scaling contract.
Document that every path outside the explicit allowlist is rejected, and either remove non-core replica support or add it as an explicit, validated, tested exception.
🤖 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 `@enhancements/external-secrets-operator/external-secrets-component-config.md`
around lines 139 - 153, Update the override contract documentation to define an
explicit allowlist: only the listed scheduling and placement paths may be
patched, and every other Deployment.spec path must be rejected. Resolve the
non-core replicas inconsistency by either removing its support or documenting it
as an explicit validated exception, ensuring the stated scaling contract and
validation behavior agree.
|
|
||
| rect rgb(120, 60, 140) | ||
| Note over Op,Deps: 2b — experimentalOverrides (per component) | ||
| Op->>Op: Reject forbidden nested lists<br/>(containers / initContainers / volumes / mounts) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep forbidden-path guidance complete everywhere.
The normative contract includes ephemeralContainers, volumeMounts, and volumeDevices, but these workflow and support references only mention abbreviated subsets such as containers, initContainers, volumes, and “mounts.” Use the complete list consistently so remediation guidance matches validation behavior.
Also applies to: 254-256, 749-749
🤖 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 `@enhancements/external-secrets-operator/external-secrets-component-config.md`
at line 188, Update the forbidden nested-list guidance in the workflow and
support references, including the diagram at the visible Op->>Op step and the
corresponding sections, to consistently name containers, initContainers,
ephemeralContainers, volumes, volumeMounts, and volumeDevices. Replace
abbreviated “mounts” wording with the exact normative field names while
preserving the existing validation guidance.
| 14. Test that `concurrent` renders `--concurrent=<N>` on the core controller; unset defaults to `1`. | ||
| 15. Test that `replicas` sets `Deployment.spec.replicas` on the core controller; unset defaults to `1`. | ||
| 16. Test that valid `experimentalOverrides` are merged into `Deployment.spec` for allowed paths. | ||
| 17. Test that invalid `experimentalOverrides` sets **Degraded**. | ||
| 18. Test that protected fields (`image`, `--enable-leader-election`, `concurrent`, `replicas`) are not overridden by `experimentalOverrides`. | ||
| 19. Test that `experimentalOverrides` targeting `containers`, `initContainers`, or `volumes` is rejected with **Degraded**. | ||
| 20. Test that operator-owned fields are restored after an allowed patch is applied. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test the Degraded/no-change contract explicitly.
The protected-field case only checks that values are not overridden, but forbidden patches must be rejected with Degraded and leave the Deployment unchanged. Add parameterized cases for image, args/command, trust-related environment variables, concurrent, replicas, ephemeralContainers, volumeMounts, and volumeDevices; keep successful allowed scheduling patches as a separate test group.
🧰 Tools
🪛 LanguageTool
[style] ~631-~631: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...troller; unset defaults to 1. 16. Test that valid experimentalOverrides are ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~632-~632: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...oyment.specfor allowed paths. 17. Test that invalidexperimentalOverrides` se...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~633-~633: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...alOverrides sets **Degraded**. 18. Test that protected fields (image, --enab...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~634-~634: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...den by experimentalOverrides. 19. Test that experimentalOverrides targeting ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~635-~635: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... is rejected with Degraded. 20. Test that operator-owned fields are restored...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 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 `@enhancements/external-secrets-operator/external-secrets-component-config.md`
around lines 629 - 635, The test plan for experimentalOverrides must explicitly
verify that forbidden patches both set the component to Degraded and leave the
Deployment unchanged. Add parameterized rejection cases covering image,
args/command, trust-related environment variables, concurrent, replicas,
ephemeralContainers, volumeMounts, and volumeDevices, while keeping successful
scheduling-only patches in a separate test group.
|
@siddhibhor-56: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
2 participants
Summary
Updates the external-secrets-component-config enhancement proposal to add three new capabilities to the
ExternalSecretsConfigAPI:concurrent(spec.controllerConfig.concurrent): First-class field that translates to the upstream--concurrent=<N>CLI argument on the core controller, allowing administrators to tune reconcile parallelism for large-scale deployments (thousands of ExternalSecrets). CRD-validated min1, max100; defaults to1when unset. Not configurable viaoverrideEnvsince it is a CLI arg, not an env var.replicas(spec.controllerConfig.replicas): First-class field for the core controller Deployment replica count, enabling high availability. CRD-validated min1, max10; defaults to1. Leader election (--enable-leader-election=true) is always enforced so only one replica actively reconciles.experimentalOverrides(spec.controllerConfig.componentConfigs[].experimentalOverrides): Per-componentruntime.RawExtensionescape hatch that applies a strategic merge patch to the componentDeployment.spec. Intended for scheduling fields (affinity, tolerations, nodeSelector, topologySpreadConstraints) until first-class fields exist. Patches targeting operator-owned nested lists (containers,initContainers,volumes, mounts) are rejected with a Degraded condition. Protected fields are re-asserted after any allowed patch.Summary by CodeRabbit