Skip to content

feat(KONFLUX-14375): Allow deploying ExternalSecrets in managed NS for Konflux perf&scale probe runs#12652

Open
jhutar wants to merge 1 commit into
redhat-appstudio:mainfrom
jhutar:82-managed-allow-ext-sec
Open

feat(KONFLUX-14375): Allow deploying ExternalSecrets in managed NS for Konflux perf&scale probe runs#12652
jhutar wants to merge 1 commit into
redhat-appstudio:mainfrom
jhutar:82-managed-allow-ext-sec

Conversation

@jhutar

@jhutar jhutar commented Jun 22, 2026

Copy link
Copy Markdown
Member

We are migrating to run release pipeline runs in separate managed NS and we want to use ExternalSecrets to manage Quay push secret.

Details: https://redhat.atlassian.net/browse/KONFLUX-14375

@openshift-ci openshift-ci Bot requested review from enkeefe00 and meyrevived June 22, 2026 14:05
@openshift-ci

openshift-ci Bot commented Jun 22, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jhutar
Once this PR has been reviewed and has the lgtm label, please assign filariow for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions

Copy link
Copy Markdown
Contributor

Kustomize Render Diff

Comparing 09f413fd57c2d7dcdb

Component Environment Changes
components/cluster-secret-store/staging staging +1 -0

Total: 1 components, +1 -0 lines

📋 Full diff available in the workflow summary and as a downloadable artifact.

@qodo-for-redhat-appstudio

Copy link
Copy Markdown

PR Summary by Qodo

Allow ClusterSecretStore access from managed perfscale namespace
✨ Enhancement ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

Description

• Add the managed perfscale tenant namespace to the ClusterSecretStore allowlist.
• Enable ExternalSecrets usage in managed namespaces for perf/scale probe runs.
• Unblock Quay push secret provisioning during managed-namespace release pipeline runs.
Diagram

graph TD
  A["Perf/scale probe runs"] --> B["Managed perfscale NS"] --> C["ExternalSecret"] --> D["ExternalSecrets controller"] --> E["ClusterSecretStore"] --> F["Quay push Secret"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Label/selector-based namespace authorization (if supported)
  • ➕ Avoids manual allowlist drift as managed namespaces change
  • ➕ Reduces repeated PRs for new perfscale namespaces
  • ➖ May not be supported by the ClusterSecretStore CRD/implementation
  • ➖ Potentially broader blast radius if selectors are too permissive
2. Pre-provision Secret into managed namespace (no ExternalSecrets)
  • ➕ Simpler runtime dependency (no controller/store access needed)
  • ➕ Avoids expanding ClusterSecretStore namespace permissions
  • ➖ Harder secret rotation and auditing
  • ➖ Higher operational overhead and risk of stale credentials

Recommendation: Current approach (explicitly allowlisting the managed perfscale namespace) is the safest incremental change to unblock managed-namespace probe runs while preserving tight scoping. If additional managed perfscale namespaces are expected to proliferate, consider a follow-up to move to selector-based authorization (if available) to prevent ongoing allowlist churn.

Files changed (1) +3 / -0

Other (1) +3 / -0
perfscale-namespaces-patch.yamlAllow ClusterSecretStore usage from managed perfscale tenant namespace +3/-0

Allow ClusterSecretStore usage from managed perfscale tenant namespace

• Adds the managed perfscale tenant namespace to the list of namespaces permitted by the ClusterSecretStore conditions in staging. This enables ExternalSecrets to be deployed in the managed namespace and still resolve secrets for Quay push operations.

components/cluster-secret-store/staging/perfscale-namespaces-patch.yaml

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.32%. Comparing base (09f413f) to head (21a6329).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #12652   +/-   ##
=======================================
  Coverage   53.32%   53.32%           
=======================================
  Files          20       20           
  Lines        1309     1309           
=======================================
  Hits          698      698           
  Misses        539      539           
  Partials       72       72           
Flag Coverage Δ
go 53.32% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@qodo-for-redhat-appstudio

qodo-for-redhat-appstudio Bot commented Jun 22, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 3 rules

Grey Divider


Remediation recommended

1. Managed namespace missing prod 🐞 Bug ☼ Reliability
Description
managed-konflux-perfscale-tenant is added to the appsre-stonesoup-vault ClusterSecretStore
allowlist only in the staging overlay, while production continues to apply a separate perfscale
allowlist patch that omits it. As a result, ExternalSecrets created in
managed-konflux-perfscale-tenant will still be rejected on clusters that deploy
components/cluster-secret-store/production.
Code

components/cluster-secret-store/staging/perfscale-namespaces-patch.yaml[R14-16]

+- op: add
+  path: /spec/conditions/0/namespaces/-
+  value: managed-konflux-perfscale-tenant
Relevance

⭐⭐⭐ High

Team previously fixed staging-only allowlist gaps by adding namespaces to prod/base (PR #11883,
#11565).

PR-#11883
PR-#11565
PR-#12199

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Staging explicitly adds the managed namespace to the allowlist patch. Production applies a different
allowlist patch to the same ClusterSecretStore, but that production patch does not include the
managed namespace, so production behavior remains unchanged and will still block ExternalSecrets
from that namespace.

components/cluster-secret-store/staging/perfscale-namespaces-patch.yaml[11-16]
components/cluster-secret-store/staging/kustomization.yaml[18-23]
components/cluster-secret-store/production/kustomization.yaml[12-17]
components/cluster-secret-store/production/perfscale-namespaces-patch.yaml[1-10]
components/cluster-secret-store/base/appsre-stonesoup-vault-secret-store.yaml[28-33]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The PR adds `managed-konflux-perfscale-tenant` to the ClusterSecretStore namespace allowlist only for the staging overlay. Production uses a different `perfscale-namespaces-patch.yaml`, so the managed namespace remains disallowed there.

### Issue Context
- Staging: `components/cluster-secret-store/staging/kustomization.yaml` applies `perfscale-namespaces-patch.yaml` to `appsre-stonesoup-vault`.
- Production: `components/cluster-secret-store/production/kustomization.yaml` also applies its own `perfscale-namespaces-patch.yaml` to `appsre-stonesoup-vault`.
- Production’s patch currently only appends `konflux-perfscale-{1..3}-tenant` and does not include `managed-konflux-perfscale-tenant`.

### Fix Focus Areas
- components/cluster-secret-store/production/perfscale-namespaces-patch.yaml[1-10]
- components/cluster-secret-store/production/kustomization.yaml[1-17]
- components/cluster-secret-store/staging/perfscale-namespaces-patch.yaml[11-16]

### Proposed fix
Append `managed-konflux-perfscale-tenant` to `components/cluster-secret-store/production/perfscale-namespaces-patch.yaml` (same JSON6902 patch style used in staging) so production clusters allow ExternalSecrets in that namespace as well.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant