Skip to content

refactor(kargo): adopt canonical ring-based directory layout#462

Merged
openshift-merge-bot[bot] merged 1 commit into
redhat-appstudio:mainfrom
flacatus:feat/kargo-canonical-directory-layout
Jun 12, 2026
Merged

refactor(kargo): adopt canonical ring-based directory layout#462
openshift-merge-bot[bot] merged 1 commit into
redhat-appstudio:mainfrom
flacatus:feat/kargo-canonical-directory-layout

Conversation

@flacatus

Copy link
Copy Markdown
Contributor

Extract shared resources (route-and-oauth, RBAC) into base/ (Tier 1). Each ring's deployment/ (Tier 2) now references ../../base and owns its helm generator and OCP patches. Root kustomization.yaml (Tier 3) adds cluster-specific oauth redirect patches.

Move project-level RBAC into its own rbac/ subdirectory.

Extract shared resources (route-and-oauth, RBAC) into base/ (Tier 1).
Each ring's deployment/ (Tier 2) now references ../../base and owns
its helm generator and OCP patches. Root kustomization.yaml (Tier 3)
adds cluster-specific oauth redirect patches.

Move project-level RBAC into its own rbac/ subdirectory.
@openshift-ci

openshift-ci Bot commented Jun 12, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flacatus

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

The pull request process is described 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

@qodo-for-redhat-appstudio

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

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1)

Context used
✅ Compliance rules (platform): 5 rules

Grey Divider


Remediation recommended

1. Tier-2 OAuth redirect missing 🐞 Bug ≡ Correctness
Description
The shared base defines the dex-client ServiceAccount OAuth redirect annotation as the literal
placeholder "tba", and internal-production/deployment no longer patches it. As a result,
building/applying the Tier-2 deployment overlay directly will produce an invalid redirect URI and
break the OpenShift OAuth callback flow for Dex.
Code

components/kargo/internal-production/deployment/kustomization.yaml[R5-6]

resources:
-  - route-and-oauth.yaml
-  - rbac
+  - ../../base
Relevance

⭐⭐⭐ High

Historically patched oauth-redirecturi in deployment overlay because base keeps 'tba' placeholder;
likely fix to keep Tier-2 valid.

PR-#355
PR-#415

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The base resource being pulled into Tier-2 contains a placeholder redirect URI ("tba"). The Tier-2
deployment kustomization includes the base but does not patch the dex-client ServiceAccount; the
patch exists only at Tier-3, so Tier-2 builds will retain "tba".

components/kargo/base/route-and-oauth.yaml[18-23]
components/kargo/internal-production/deployment/kustomization.yaml[5-35]
components/kargo/internal-production/kustomization.yaml[9-19]

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

## Issue description
`components/kargo/base/route-and-oauth.yaml` hardcodes `serviceaccounts.openshift.io/oauth-redirecturi.kargo: tba` for the `dex-client` ServiceAccount. After this refactor, the Tier-2 deployment overlays (e.g., `internal-production/deployment`) no longer override that placeholder, so anyone building/applying the Tier-2 overlay directly will deploy a broken OAuth redirect.

## Issue Context
Tier-3 overlays do patch the ServiceAccount to the correct cluster URL, but Tier-2 is now non-self-contained with respect to OAuth redirect configuration.

## Fix Focus Areas
Choose one:
1) Reintroduce the ServiceAccount redirect patch into each Tier-2 deployment overlay that is intended to be deployable on its own, OR
2) Remove the placeholder `tba` annotation from the base ServiceAccount and require overlays to add the correct annotation (so base is not “silently wrong” when applied alone).

### Suggested edits
- components/kargo/internal-production/deployment/kustomization.yaml[13-35]
- components/kargo/base/route-and-oauth.yaml[18-23]
- (if needed) components/kargo/internal-staging/deployment/kustomization.yaml[13-35]

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



Informational

2. rbac.yaml has multiple Kinds 📘 Rule violation ⚙ Maintainability
Description
components/kargo/base/rbac/rbac.yaml contains multiple top-level resources (Role, RoleBinding,
ClusterRole, ClusterRoleBinding), so the filename cannot match a single primary kind as
required. This breaks the manifest naming convention and makes the file ambiguous for review and
tooling.
Code

components/kargo/base/rbac/rbac.yaml[R47-77]

+---
+kind: ClusterRole
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: konflux-devprod-kargo-projectconfigs
+rules:
+  - apiGroups:
+      - kargo.akuity.io
+    resources:
+      - projectconfigs
+    verbs:
+      - get
+      - list
+      - watch
+      - create
+      - update
+      - patch
+      - delete
+---
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: konflux-devprod-kargo-projectconfigs
+subjects:
+  - apiGroup: rbac.authorization.k8s.io
+    kind: Group
+    name: konflux-devprod
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: konflux-devprod-kargo-projectconfigs
Relevance

⭐ Low

Repo commonly keeps multi-doc/multi-kind manifests (e.g., route-and-oauth.yaml); no history
enforcing single-kind filenames.

PR-#415
PR-#355

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1100 requires each manifest file to contain exactly one primary top-level kind
and for the filename to match that kind. The updated components/kargo/base/rbac/rbac.yaml contains
multiple different kind values (e.g., Role, RoleBinding, ClusterRole, ClusterRoleBinding),
so it cannot be correctly named as a single kind-specific filename.

Rule 1100: Name Kubernetes manifest files after single primary Kind
components/kargo/base/rbac/rbac.yaml[1-77]

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

## Issue description
`components/kargo/base/rbac/rbac.yaml` contains multiple primary Kubernetes resource kinds, but the compliance rule requires exactly one primary `kind` per manifest file and the filename (without extension) must equal the lowercase `kind`.

## Issue Context
Current file includes `Role`, `RoleBinding`, `ClusterRole`, and `ClusterRoleBinding`, which makes the filename `rbac.yaml` non-compliant and ambiguous.

## Fix Focus Areas
- components/kargo/base/rbac/rbac.yaml[47-77]
- components/kargo/base/rbac/kustomization.yaml[1-5]

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


Grey Divider

Qodo Logo

@qodo-for-redhat-appstudio

Copy link
Copy Markdown

PR Summary by Qodo

refactor(kargo): adopt canonical ring-based directory layout
⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

Walkthroughs

Description
• Extract shared Kargo Route/OAuth and RBAC into a reusable Tier-1 base.
• Update ring deployments to consume the base and keep ring-specific generators/OCP patches local.
• Move cluster-specific OAuth redirect URIs to ring root kustomizations and isolate project RBAC.
Diagram
graph TD
  Root["Ring root overlays (Tier 3)"] --> Deploy["Ring deployment overlays (Tier 2)"] --> Base["Shared base (Tier 1)"]
  Base --> Route[("route-and-oauth.yaml")]
  Base --> SharedRBAC[("shared RBAC")]
  ProjBase["kargo-infra-common base"] --> ProjRBAC[("project RBAC")]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use Kustomize Components for shared Route/RBAC
  • ➕ Explicitly models “shared optional building blocks” across rings
  • ➕ Can reduce deep relative paths and improve reuse granularity
  • ➖ Requires enabling/standardizing components usage across the repo
  • ➖ May be unfamiliar to some contributors vs conventional base/overlay layout
2. Keep per-ring Route/RBAC (no shared base)
  • ➕ No cross-ring coupling; each ring fully self-contained
  • ➖ Duplication across rings (Route/RBAC/OAuth scaffolding) increases drift risk
  • ➖ Harder to apply consistent security changes everywhere

Recommendation: The chosen Tier-1 base + Tier-2 ring deployments + Tier-3 ring roots is the best fit for a canonical ring layout: it removes duplication while preserving ring ownership of generators and OCP-specific patches. The main thing to validate in review is that RBAC semantics and the OAuth redirect patch targets remain identical post-move.

Grey Divider

File Changes

Refactor (6)
kustomization.yaml Promote shared Route/OAuth and RBAC into Tier-1 base +2/-3

Promote shared Route/OAuth and RBAC into Tier-1 base

• Updates the base kustomization to include the shared route-and-oauth manifest and the RBAC sub-kustomization directory. This makes ring overlays consume common resources via ../../base.

components/kargo/base/kustomization.yaml


kustomization.yaml Simplify base RBAC kustomization to a single RBAC manifest +1/-3

Simplify base RBAC kustomization to a single RBAC manifest

• Replaces the prior per-file resource list with a single rbac.yaml entry under the base/rbac kustomization. This consolidates shared RBAC into one managed unit.

components/kargo/base/rbac/kustomization.yaml


kustomization.yaml Internal production deployment consumes shared base and drops local Route/RBAC +62/-82

Internal production deployment consumes shared base and drops local Route/RBAC

• Replaces direct inclusion of route-and-oauth.yaml and rbac/ with ../../base. Retains the helm generator and OCP-specific patches (serving cert annotations, CA volume projection, initContainer securityContext) as ring-owned concerns.

components/kargo/internal-production/deployment/kustomization.yaml


kustomization.yaml Switch project base to use RBAC subdirectory +1/-1

Switch project base to use RBAC subdirectory

• Updates the project base resources to reference rbac/ rather than an inline rbac.yaml. This aligns project overlays with the new directory-based RBAC layout.

components/kargo/internal-production/projects/kargo-infra-common/base/kustomization.yaml


kustomization.yaml Add project RBAC kustomization wrapper +5/-0

Add project RBAC kustomization wrapper

• Adds a minimal kustomization.yaml to manage project-level RBAC as a directory resource. This supports clearer ownership and composition from the project base.

components/kargo/internal-production/projects/kargo-infra-common/base/rbac/kustomization.yaml


kustomization.yaml Internal staging deployment consumes shared base and drops local Route/RBAC +62/-82

Internal staging deployment consumes shared base and drops local Route/RBAC

• Replaces direct inclusion of route-and-oauth.yaml and rbac/ with ../../base. Keeps helm generation and OCP-specific patches local to the staging ring deployment overlay.

components/kargo/internal-staging/deployment/kustomization.yaml


Other (5)
rbac.yaml Consolidate shared RBAC and add Kargo ProjectConfig ClusterRole/Binding +32/-4

Consolidate shared RBAC and add Kargo ProjectConfig ClusterRole/Binding

• Keeps the existing admin Role/RoleBinding and adds a ClusterRole + ClusterRoleBinding granting konflux-devprod access to kargo.akuity.io ProjectConfigs. This also matches content previously defined in ring-local RBAC files, now centralized.

components/kargo/base/rbac/rbac.yaml


route-and-oauth.yaml Centralize OpenShift Route/OAuth scaffolding into base +0/-0

Centralize OpenShift Route/OAuth scaffolding into base

• Introduces a shared Route/OAuth resource manifest under the Tier-1 base so rings no longer carry their own copies. Ring roots now patch cluster-specific redirect URIs instead of embedding them in deployment overlays.

components/kargo/base/route-and-oauth.yaml


kustomization.yaml Internal production root adds OAuth redirect URI patch for dex-client +14/-0

Internal production root adds OAuth redirect URI patch for dex-client

• Adds a ring-root patch targeting the dex-client ServiceAccount to set the production cluster OAuth redirect URL. This moves cluster-specific data out of the deployment overlay.

components/kargo/internal-production/kustomization.yaml


rbac.yaml Project-level RBAC moved under dedicated directory +0/-0

Project-level RBAC moved under dedicated directory

• Adds/relocates the project-scoped RBAC manifest into the new rbac/ subdirectory so it can be included via the RBAC kustomization wrapper.

components/kargo/internal-production/projects/kargo-infra-common/base/rbac/rbac.yaml


kustomization.yaml Internal staging root adds OAuth redirect URI patch for dex-client +14/-0

Internal staging root adds OAuth redirect URI patch for dex-client

• Adds a ring-root patch targeting the dex-client ServiceAccount to set the staging cluster OAuth redirect URL. This ensures only the cluster-specific redirect differs per ring root.

components/kargo/internal-staging/kustomization.yaml


Grey Divider

Qodo Logo

@p8r-the-gr8

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Jun 12, 2026
@openshift-merge-bot openshift-merge-bot Bot merged commit 1096c39 into redhat-appstudio:main Jun 12, 2026
6 checks passed
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.

2 participants