Skip to content

check-cluster-profiles-config: Workround to handle Cluster Profile Sets #5282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
danilo-gemoli wants to merge 1 commit into
base: main
Choose a base branch
from
+17 −4
Open

check-cluster-profiles-config: Workround to handle Cluster Profile Sets #5282

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions cmd/check-cluster-profiles-config/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/sirupsen/logrus"

coreapi "k8s.io/api/core/v1"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/sets"
ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
fakectrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
Expand Down Expand Up @@ -174,18 +175,30 @@ func (validator *profileValidator) Validate(profiles api.ClusterProfilesList) er

// checkCISecrets verifies that the secret for each cluster profile exists in the ci namespace
func (validator *profileValidator) checkCISecrets() error {
// FIXME: temporary workaround, we should read this information from a config file.
clusterProfileSets := sets.New("openshift-org-aws", "openshift-org-azure", "openshift-org-gcp")
errs := make([]error, 0)

for p := range validator.profiles {
profileDetails, err := server.NewResolverClient(configResolverAddress).ClusterProfile(p.Name())
profileName := p.Name()
if clusterProfileSets.Has(profileName) {
continue
Comment on lines +178 to +185

@coderabbitai coderabbitai Bot Jun 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Don't hardcode the profile-set bypass.

This validator gates openshift/release, and cluster profile sets are already a first-class concept in adjacent config/jobs. A fixed name list here will drift and start rejecting valid new sets until this binary is patched again. Please drive the skip from the source-of-truth profile-set metadata (or the resolved profile details) instead of profile names.

🤖 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 `@cmd/check-cluster-profiles-config/main.go` around lines 178 - 185, The
profile-set bypass in the validator is hardcoded to specific names, which will
drift from the real source of truth. Update the logic in main.go around the
validator.profiles loop to derive skip behavior from the resolved cluster
profile-set metadata (or profile details) instead of matching profileName
against a fixed set, so new valid sets are handled without code changes.

Source: Linked repositories

}

profileDetails, err := server.NewResolverClient(configResolverAddress).ClusterProfile(profileName)

if err != nil {
return fmt.Errorf("failed to retrieve details from config resolver for '%s' cluster profile", p.Name())
errs = append(errs, fmt.Errorf("failed to retrieve details from config resolver for '%s' cluster profile", profileName))
continue
Comment on lines 190 to +192

@coderabbitai coderabbitai Bot Jun 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Preserve the resolver failure cause.

This branch drops err, so the aggregate loses the actual config-resolver failure. Wrap it the same way as the secret lookup branch.

Suggested fix 
-			errs = append(errs, fmt.Errorf("failed to retrieve details from config resolver for '%s' cluster profile", profileName))
+			errs = append(errs, fmt.Errorf("failed to retrieve details from config resolver for %q cluster profile: %w", profileName, err))

As per path instructions, **/*.go: Wrap errors with fmt.Errorf("context: %w", err), lowercase messages, no trailing punctuation; and coding guidelines, Errors should be handled correctly - determine whether to ignore, log, wrap and raise up.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err != nil {
return fmt.Errorf("failed to retrieve details from config resolver for '%s' cluster profile", p.Name())
errs = append(errs, fmt.Errorf("failed to retrieve details from config resolver for '%s' cluster profile", profileName))
continue
if err != nil {
errs = append(errs, fmt.Errorf("failed to retrieve details from config resolver for %q cluster profile: %w", profileName, err))
continue
🤖 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 `@cmd/check-cluster-profiles-config/main.go` around lines 190 - 192, The error
handling in the config-resolver branch drops the original failure, so the
aggregate error loses the real cause. Update the `check-cluster-profiles-config`
flow where `profileName` details are appended to `errs` to wrap `err` with
`fmt.Errorf("...: %w", err)` just like the secret lookup branch, keeping the
message lowercase and without trailing punctuation.

Sources: Coding guidelines, Path instructions

}
ciSecret := &coreapi.Secret{}
err = validator.kubeClient.Get(context.Background(), ctrlruntimeclient.ObjectKey{Namespace: "ci", Name: profileDetails.Secret}, ciSecret)
if err != nil {
return fmt.Errorf("failed to get secret '%s' for cluster profile '%s': %w", profileDetails.Secret, p.Name(), err)
errs = append(errs, fmt.Errorf("failed to get secret '%s' for cluster profile '%s': %w", profileDetails.Secret, profileName, err))
}
}
return nil

return utilerrors.NewAggregate(errs)
}

func normalize(profiles api.ClusterProfilesList) {
Expand Down