Skip to content

[WIP] consider both API groups for Component CRD#336

Open
rcerven wants to merge 1 commit into
konflux-ci:mainfrom
rcerven:usenewcomponentgroup
Open

[WIP] consider both API groups for Component CRD#336
rcerven wants to merge 1 commit into
konflux-ci:mainfrom
rcerven:usenewcomponentgroup

Conversation

@rcerven

@rcerven rcerven commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Description

Testing

@rcerven rcerven requested a review from a team as a code owner June 23, 2026 20:06
@rcerven rcerven changed the title consider bot API groups for Component CRD [WIP] consider bot API groups for Component CRD Jun 23, 2026
@qodo-app-for-konflux-ci

Copy link
Copy Markdown

PR Summary by Qodo

Support both Component API groups when reconciling ImageRepository
✨ Enhancement ⚙️ Configuration changes 🧪 Tests 🕐 40+ Minutes

Grey Divider

Description

• Add dual-stack Component lookup/update supporting both old and new API groups.
• Introduce build-service Component dependency and upgrade controller-runtime/Kubernetes libraries.
• Extend controller test scheme registration to include the new Component API.
Diagram

graph TD
  IR["ImageRepository CRD"] --> R["ImageRepositoryReconciler"] --> C["controller-runtime client"] --> D{"IsOldGroup?"} --> N["Component CRD (build-service)"] --> U["Update ContainerImage + OwnerRef"] --> K[("Kubernetes API")]
  D --> O["Component CRD (application-api)"] --> U
  subgraph Legend
    direction LR
    _crd["CRD/Resource"] ~~~ _ctrl["Controller"] ~~~ _dec{"Decision"} ~~~ _api[("API Server")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use dynamic/unstructured client keyed by GVK
  • ➕ Avoids compile-time dependency on both Component Go types
  • ➕ Simplifies branching code by sharing a single update path
  • ➖ Lossof type safety for spec fields like ContainerImage
  • ➖ More verbose and error-prone field access/patching
2. Run two controllers (one per Component API group)
  • ➕ Keeps each controller strongly typed and simpler to read
  • ➕ Reduces runtime branching and mixed-object handling
  • ➖ Operational overhead (deploy/maintain two controllers)
  • ➖ Risk of double-reconciliation if both watch the same resources
3. Version-gated build (compile-time switch via build tags)
  • ➕ No runtime flag; clean code paths per build artifact
  • ➕ Smaller dependency surface per build
  • ➖ Requires separate builds/releases during migration
  • ➖ Harder to test/validate both modes in one CI run

Recommendation: The current approach (runtime toggle + typed objects) is reasonable for a migration window because it keeps typed access to Component.Spec.ContainerImage while allowing a controlled cutover. If the migration is expected to last long or additional fields are added, consider moving to a dynamic GVK-based lookup to reduce duplicated Get/Update blocks; otherwise, keep this as a temporary bridge and remove the old-group branch once all clusters are migrated.

Files changed (4) +329 / -282

Enhancement (1) +77 / -29
imagerepository_controller.goBranch Component fetch/update by API group (old vs new) +77/-29

Branch Component fetch/update by API group (old vs new)

• Adds support for the build-service Component API group and selects between old (application-api) and new (build-service) Component types based on r.IsOldGroup. Updates code paths that fetch Components, update ContainerImage, check existence, and set owner references to work with client.Object.

internal/controller/imagerepository_controller.go

Tests (1) +4 / -0
suite_test.goRegister build-service Component types in controller test scheme +4/-0

Register build-service Component types in controller test scheme

• Extends the controller test suite scheme setup to include the build-service Component API group, enabling tests to run with the new Component type registered.

internal/controller/suite_test.go

Other (2) +248 / -253
go.modUpgrade Kubernetes/controller-runtime stack and add build-service Component API +80/-68

Upgrade Kubernetes/controller-runtime stack and add build-service Component API

• Bumps Go patch version and updates core dependencies (controller-runtime, k8s libraries, Prometheus, Ginkgo). Adds a new direct dependency on github.com/konflux-ci/build-service and introduces replace directives (including a temporary fork for build-service).

go.mod

go.sumRefresh dependency checksums for upgraded modules +168/-185

Refresh dependency checksums for upgraded modules

• Updates go.sum entries to match the new dependency graph after Kubernetes/controller-runtime and related library upgrades, including the added build-service module and its transitive dependencies.

go.sum

@qodo-app-for-konflux-ci

qodo-app-for-konflux-ci Bot commented Jun 23, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 4 rules

Grey Divider


Action required

1. RBAC missing for Components ✓ Resolved 🐞 Bug ⛨ Security
Description
The controller now reads/updates compv1alpha1.Component on the IsOldGroup=false path, but the
shipped ClusterRole only grants permissions for appstudio.redhat.com components. If the
build-service Component CRD is served from a different API group (expected for the new path), these
requests will be rejected as Forbidden and reconciliation will break.
Code

internal/controller/imagerepository_controller.go[R697-702]

+	var component client.Object // remove after fully migrated to new group - replace with: var component *compv1alpha1.Component
+	if !r.IsOldGroup {
+		component = &compv1alpha1.Component{}
+	} else { // remove after fully migrated to new group - start
+		component = &compapiv1alpha1.Component{}
+	} // remove after fully migrated to new group - end
Relevance

⭐⭐⭐ High

RBAC gaps causing Forbidden are routinely fixed (e.g., added update/finalizers permissions in
role.yaml in PRs #229/#224).

PR-#229
PR-#224

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new reconcile path uses compv1alpha1.Component, while the manager ClusterRole only allows
components in the appstudio.redhat.com apiGroup. Therefore, if the build-service Component is in
a different group for the new path, API calls will fail with RBAC Forbidden and block
reconciliation.

internal/controller/imagerepository_controller.go[688-744]
internal/controller/imagerepository_controller.go[534-575]
config/rbac/role.yaml[55-65]

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 controller now performs `get/list/watch/update/patch` operations on Components via the build-service API type (`compv1alpha1.Component`) when reconciling the new ImageRepository group. Current RBAC only grants `components` permissions under the old API group, so the controller service account may not be authorized for the new Component API group.

## Issue Context
This affects multiple paths (component existence checks, owner references, and ContainerImage updates) whenever `IsOldGroup=false`.

## Fix Focus Areas
- config/rbac/role.yaml[55-65]
- internal/controller/imagerepository_controller.go[688-744]
- internal/controller/imagerepository_controller.go[534-575]

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


2. Component scheme not registered ✓ Resolved 🐞 Bug ☼ Reliability
Description
ImageRepositoryReconciler now Get/Updates build-service compv1alpha1.Component when IsOldGroup is
false, but cmd/main.go never adds that API to the manager scheme. This will fail at runtime with
scheme/GVK registration errors as soon as a konflux-ci.dev ImageRepository reconcile touches a
Component.
Code

internal/controller/imagerepository_controller.go[R539-541]

+			if !r.IsOldGroup {
+				component := &compv1alpha1.Component{}
+				if err := r.Client.Get(ctx, componentKey, component); err != nil {
Relevance

⭐⭐⭐ High

Team previously added missing AddToScheme for new API groups in cmd/main.go (PR #326).

PR-#326

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The reconciler now instantiates compv1alpha1.Component for the !IsOldGroup path, but the
production manager scheme setup in cmd/main.go only registers application-api components and
ImageRepository APIs, not build-service components. The test suite explicitly adds compv1alpha1 to
the scheme, indicating the new type must be registered for that path to work.

internal/controller/imagerepository_controller.go[534-575]
cmd/main.go[84-91]
internal/controller/suite_test.go[91-101]

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 controller now uses `&compv1alpha1.Component{}` (build-service) on the `IsOldGroup=false` path, but the production manager scheme does not register `compv1alpha1`. Controller-runtime’s client relies on the scheme for GVK mapping/encoding/decoding, so `Client.Get/Update` against `compv1alpha1.Component` will error.

## Issue Context
Tests already register `compv1alpha1` in envtest, but production `cmd/main.go` does not, so this will only be caught in-cluster.

## Fix Focus Areas
- cmd/main.go[84-91]
- internal/controller/imagerepository_controller.go[534-575]
- internal/controller/suite_test.go[91-101]

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


Grey Divider

Qodo Logo

Comment thread internal/controller/imagerepository_controller.go
Comment thread internal/controller/imagerepository_controller.go Outdated
@rcerven rcerven force-pushed the usenewcomponentgroup branch 2 times, most recently from 2591003 to 3f2cde0 Compare July 2, 2026 17:41
@rcerven rcerven changed the title [WIP] consider bot API groups for Component CRD [WIP] consider both API groups for Component CRD Jul 2, 2026
@rcerven rcerven force-pushed the usenewcomponentgroup branch 2 times, most recently from d75b376 to 5f77ce3 Compare July 3, 2026 20:08
STONEBLD-4824

Signed-off-by: Robert Cerven <rcerven@redhat.com>
@rcerven rcerven force-pushed the usenewcomponentgroup branch from 5f77ce3 to 270ff97 Compare July 3, 2026 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant