ci-operator-checkconfig: Add cluster profile sets allowlist#5171
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
📝 WalkthroughWalkthroughRefactors cluster-profile-set handling to a new exported ChangesCluster Profile Allowlisting with Schema Migration
Sequence DiagramsequenceDiagram
participant CLI as "ci-operator-checkconfig CLI"
participant Loader as "pkg/load"
participant API as "pkg/api (types)"
participant Validator as "pkg/validation"
CLI->>Loader: ClusterProfileSetDetails(path)
Loader->>API: json.Unmarshal -> ClusterProfileSetDetails (handles old/new schema)
API-->>Loader: ClusterProfileSetDetails struct
CLI->>Validator: pass ClusterProfileSetDetails into options
Validator->>API: IsTestAllowlisted(test, metadata) / FindSetByProfile(profile)
API-->>Validator: allowlist decision / suggestion
Validator-->>CLI: validation result / errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@pkg/validation/config.go`:
- Line 45: Fix the typo in the comment for the TestsWhitelist field: change
"enfoce" to "enforce" in the comment that reads "TestsWhitelist holds a list of
tests for which we do not enfoce policy" (search for the TestsWhitelist comment
in pkg/validation/config.go and update the text accordingly).
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 4b24535d-a229-4ecf-8af4-8093fb722155
📒 Files selected for processing (5)
cmd/ci-operator-checkconfig/main.gopkg/validation/config.gopkg/validation/config_test.gopkg/validation/test.gopkg/validation/test_test.go
|
/test e2e |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
Sure.
Good point. The {
"cluster_profile_sets": {
"openshift-org-aws": [
"aws",
"aws-2",
"aws-3",
"aws-4",
"aws-5"
],
"openshift-org-azure": [
"azure-2",
"azure4"
],
"openshift-org-gcp": [
"gcp",
"gcp-arm64",
"gcp-openshift-gce-devel-ci-2",
"gcp-3"
]
},
"tests_whitelist": {
"openshift/openshift-tests-private": {
"release-4.22": {
"multi-nightly": [
"aws-ipi-public-ipv4-pool-byo-subnet-mini-perm-arm-f14"
]
}
}
}
}I can think of several approaches to address your, very valid, concerns:
"tests_whitelist": {
"openshift(-priv)?/openshift-tests-private": { // openshift or openshift-priv
".+": { // Any branch
".*": [ // Any or no variant
"aws-ipi-public-ipv4-pool-byo-subnet-mini-perm-arm-f14"
]
}
}
}
tests:
- as: aws-ipi-public-ipv4-pool-byo-subnet-mini-perm-arm-f14
cron: 33 8 4,18 * *
steps:
cluster_profile:
name: aws
enforce_cluster_profile_set_policy: false
To me, the least complex that keeps enforcing the policy and requires little to no long term, manual, maintenance is (1). |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
danilo-gemoli
force-pushed
the
feat/ci-operator-checkconfig/cps-ignore-list
branch
from
5d52c72 to
287468d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
pkg/api/types_test.go (1)
454-516: ⚡ Quick winAdd migration coverage for allowlist key aliases
This test only covers old map vs
cluster_profile_sets. Please add cases forallowlist,tests_allowlist, andtests_whitelistso schema migration behavior is locked in.As per coding guidelines
pkg/api/**: "Backward compatibility matters — new fields should have zero-value defaults that preserve existing behavior."🤖 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 `@pkg/api/types_test.go` around lines 454 - 516, Extend TestUnmarshalClusterProfileSetDetails to include additional test cases that verify the schema migration handles legacy key aliases: "allowlist", "tests_allowlist", and "tests_whitelist"; for each new case (add to the tc slice in TestUnmarshalClusterProfileSetDetails) provide JSON input using the legacy key with the same array value as the existing cases and set wantClusterProfileSetDetails to the same ClusterProfileSetDetails/ClusterProfileSetDetailsNew value with ClusterProfileSets populated for "openshift-org-aws"; ensure the test still unmarshals into ClusterProfileSetDetails and compares via cmp.Diff so backwards-compatibility for those aliases is asserted.pkg/api/types.go (1)
3118-3161: ⚡ Quick winAdd GoDoc for new exported API surface
ClusterProfileSetDetails,ClusterProfileSetDetailsNew,FindSetByProfile, andIsTestAllowlistedare exported and should have doc comments.As per coding guidelines
**/*.go: "Ensure Go documentation on functions, classes, and fields are written properly."🤖 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 `@pkg/api/types.go` around lines 3118 - 3161, Add GoDoc comments for the newly exported API: document ClusterProfileSetDetails and ClusterProfileSetDetailsNew (describe purpose and JSON field meanings for ClusterProfileSets and Allowlist), and document the methods FindSetByProfile and IsTestAllowlisted (describe parameters, return values and behavior). Use proper GoDoc style (each comment begins with the symbol name, concise description, mention JSON semantics for ClusterProfileSets and the nested allowlist structure) and place comments immediately above the type/func declarations for ClusterProfileSetDetails, ClusterProfileSetDetailsNew, ClusterProfileSets, Allowlist, FindSetByProfile, and IsTestAllowlisted.
🤖 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 `@pkg/api/types.go`:
- Around line 3122-3136: The UnmarshalJSON for ClusterProfileSetDetails should
not rely on strings.Contains; instead first unmarshal the top-level JSON into a
map[string]json.RawMessage (an "envelope") and inspect whether the
"cluster_profile_sets" key exists, then unmarshal the corresponding raw message
into the old cpsDetails map or into ClusterProfileSetDetailsNew accordingly;
update the ClusterProfileSetDetails.UnmarshalJSON function to use this
envelope-based detection and then set cps.ClusterProfileSets or
cps.ClusterProfileSetDetailsNew as before.
In `@pkg/load/load.go`:
- Around line 317-318: The json.Unmarshal error for cpsDetailsBytes into cpsd
doesn't include which file failed to parse; update the error wrapping (the
fmt.Errorf call) to include the source filename or path used to produce
cpsDetailsBytes (e.g., the variable that holds the path or the function
parameter that names the file) so the message becomes something like "unmarshal
<filename>: %w"; locate the json.Unmarshal call and the fmt.Errorf return in the
function handling cpsDetailsBytes/cpsd and augment the error string to include
that filename variable.
---
Nitpick comments:
In `@pkg/api/types_test.go`:
- Around line 454-516: Extend TestUnmarshalClusterProfileSetDetails to include
additional test cases that verify the schema migration handles legacy key
aliases: "allowlist", "tests_allowlist", and "tests_whitelist"; for each new
case (add to the tc slice in TestUnmarshalClusterProfileSetDetails) provide JSON
input using the legacy key with the same array value as the existing cases and
set wantClusterProfileSetDetails to the same
ClusterProfileSetDetails/ClusterProfileSetDetailsNew value with
ClusterProfileSets populated for "openshift-org-aws"; ensure the test still
unmarshals into ClusterProfileSetDetails and compares via cmp.Diff so
backwards-compatibility for those aliases is asserted.
In `@pkg/api/types.go`:
- Around line 3118-3161: Add GoDoc comments for the newly exported API: document
ClusterProfileSetDetails and ClusterProfileSetDetailsNew (describe purpose and
JSON field meanings for ClusterProfileSets and Allowlist), and document the
methods FindSetByProfile and IsTestAllowlisted (describe parameters, return
values and behavior). Use proper GoDoc style (each comment begins with the
symbol name, concise description, mention JSON semantics for ClusterProfileSets
and the nested allowlist structure) and place comments immediately above the
type/func declarations for ClusterProfileSetDetails,
ClusterProfileSetDetailsNew, ClusterProfileSets, Allowlist, FindSetByProfile,
and IsTestAllowlisted.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7a69c4cc-3842-494a-b5d6-22f8d7a52222
📒 Files selected for processing (9)
cmd/ci-operator-checkconfig/main.gopkg/api/types.gopkg/api/types_test.gopkg/load/load.gopkg/util/regexp/regexp.gopkg/util/regexp/regexp_test.gopkg/validation/config.gopkg/validation/test.gopkg/validation/test_test.go
✅ Files skipped from review due to trivial changes (2)
- pkg/util/regexp/regexp.go
- pkg/util/regexp/regexp_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/validation/test_test.go
- pkg/validation/test.go
| func (cps *ClusterProfileSetDetails) UnmarshalJSON(data []byte) error { | ||
| cpsDetails := make(map[ClusterProfile][]string) | ||
|
|
||
| if strings.Contains(string(data), `"cluster_profile_sets"`) { | ||
| newCPSDetails := ClusterProfileSetDetailsNew{} | ||
| if err := json.Unmarshal(data, &newCPSDetails); err != nil { | ||
| return fmt.Errorf("new ClusterProfileSetDetails schema: %w", err) | ||
| } | ||
| cps.ClusterProfileSetDetailsNew = newCPSDetails | ||
| } else { | ||
| if err := json.Unmarshal(data, &cpsDetails); err != nil { | ||
| return fmt.Errorf("old ClusterProfileSetDetails schema: %w", err) | ||
| } | ||
| cps.ClusterProfileSets = cpsDetails | ||
| } |
There was a problem hiding this comment.
Avoid substring-based schema detection in UnmarshalJSON
Using strings.Contains on raw JSON can select the wrong branch based on content text rather than actual top-level keys. Parse an envelope key instead.
💡 Safer schema detection
func (cps *ClusterProfileSetDetails) UnmarshalJSON(data []byte) error {
- cpsDetails := make(map[ClusterProfile][]string)
-
- if strings.Contains(string(data), `"cluster_profile_sets"`) {
+ cpsDetails := make(map[ClusterProfile][]string)
+ var probe struct {
+ ClusterProfileSets json.RawMessage `json:"cluster_profile_sets"`
+ }
+ if err := json.Unmarshal(data, &probe); err != nil {
+ return fmt.Errorf("probe ClusterProfileSetDetails schema: %w", err)
+ }
+
+ if probe.ClusterProfileSets != nil {
newCPSDetails := ClusterProfileSetDetailsNew{}
if err := json.Unmarshal(data, &newCPSDetails); err != nil {
return fmt.Errorf("new ClusterProfileSetDetails schema: %w", err)
}
+ newCPSDetails.normalizeAllowlist()
cps.ClusterProfileSetDetailsNew = newCPSDetails
} else {
if err := json.Unmarshal(data, &cpsDetails); err != nil {
return fmt.Errorf("old ClusterProfileSetDetails schema: %w", err)
}
- cps.ClusterProfileSets = cpsDetails
+ cps.ClusterProfileSetDetailsNew = ClusterProfileSetDetailsNew{
+ ClusterProfileSets: cpsDetails,
+ }
}🤖 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 `@pkg/api/types.go` around lines 3122 - 3136, The UnmarshalJSON for
ClusterProfileSetDetails should not rely on strings.Contains; instead first
unmarshal the top-level JSON into a map[string]json.RawMessage (an "envelope")
and inspect whether the "cluster_profile_sets" key exists, then unmarshal the
corresponding raw message into the old cpsDetails map or into
ClusterProfileSetDetailsNew accordingly; update the
ClusterProfileSetDetails.UnmarshalJSON function to use this envelope-based
detection and then set cps.ClusterProfileSets or cps.ClusterProfileSetDetailsNew
as before.
| if err := json.Unmarshal(cpsDetailsBytes, &cpsd); err != nil { | ||
| return cpsd, fmt.Errorf("unmarshal: %w", err) |
There was a problem hiding this comment.
Include file path in unmarshal error context
The current message loses which file failed JSON parsing.
💡 Small fix
- if err := json.Unmarshal(cpsDetailsBytes, &cpsd); err != nil {
- return cpsd, fmt.Errorf("unmarshal: %w", err)
+ if err := json.Unmarshal(cpsDetailsBytes, &cpsd); err != nil {
+ return cpsd, fmt.Errorf("unmarshal file %s: %w", p, err)
}🤖 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 `@pkg/load/load.go` around lines 317 - 318, The json.Unmarshal error for
cpsDetailsBytes into cpsd doesn't include which file failed to parse; update the
error wrapping (the fmt.Errorf call) to include the source filename or path used
to produce cpsDetailsBytes (e.g., the variable that holds the path or the
function parameter that names the file) so the message becomes something like
"unmarshal <filename>: %w"; locate the json.Unmarshal call and the fmt.Errorf
return in the function handling cpsDetailsBytes/cpsd and augment the error
string to include that filename variable.
287468d to
c1b5e76
Compare
danilo-gemoli
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
pkg/load/load.go (1)
309-309: ⚡ Quick winAdd GoDoc for exported
ClusterProfileSetDetailsPlease add a doc comment starting with
ClusterProfileSetDetailsabove the declaration so intent is discoverable and Go tooling stays clean.As per coding guidelines "Ensure Go documentation on functions, classes, and fields are written properly".
Suggested patch
+// ClusterProfileSetDetails loads cluster profile set details from a JSON file. func ClusterProfileSetDetails(path string) (api.ClusterProfileSetDetails, error) { cpsd := api.ClusterProfileSetDetails{}🤖 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 `@pkg/load/load.go` at line 309, Add a GoDoc comment for the exported function ClusterProfileSetDetails: place a comment immediately above the function declaration that begins with "ClusterProfileSetDetails" and briefly describes what the function does, the meaning of the path parameter, and what the returned api.ClusterProfileSetDetails and error represent (e.g., successful load vs. failure). Keep it concise and idiomatic (one or two sentences starting with the function name) so Go tooling and users can discover the intent.pkg/util/regexp/regexp.go (1)
7-10: ⚡ Quick winAdd GoDoc for exported API elements
Pattern(exported field) andCompile(exported function) need explicit doc comments to meet package documentation standards.As per coding guidelines Ensure Go documentation on functions, classes, and fields are written properly.
Also applies to: 34-40
🤖 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 `@pkg/util/regexp/regexp.go` around lines 7 - 10, Add GoDoc comments for the exported Regexp type, its exported field Pattern, and the exported Compile function: write concise package-style comments above the type Regexp describing its purpose, above the Pattern field describing that it holds the compiled *regexp.Regexp for the expression, and above the Compile function describing its behavior, inputs, return values, and any error semantics so the exported API is documented for godoc consumers.pkg/util/regexp/regexp_test.go (1)
142-150: ⚡ Quick winOverlap test does not verify the declared “first wins” behavior
This case currently skips value assertion, so it can’t detect resolution-order regressions. Once precedence is defined in
LookupByMatch, assert the exact winner here.🤖 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 `@pkg/util/regexp/regexp_test.go` around lines 142 - 150, The test "multiple patterns could match - first wins" currently sets skipValueCheck so it cannot catch precedence regressions; update the case to assert the exact winner from LookupByMatch instead of skipping: remove skipValueCheck and add wantValue (expect "specific" for input "test-cluster" since "^test-.*" should win over ".*"). If LookupByMatch defines precedence by input order rather than map iteration, change patternPairs from a map to an ordered structure (e.g., a slice of pattern/value pairs used by the test) so the test reliably encodes the intended order.
🤖 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 `@pkg/util/regexp/regexp.go`:
- Around line 44-49: LookupByMatch is nondeterministic because it iterates a map
and returns the first matching Regexp, so overlapping patterns can yield random
results; fix by making matching deterministic: collect the map entries (keys or
key/value pairs) into a slice, sort that slice by a stable key (e.g., the Regexp
pattern string via re.Pattern.String() or a priority field on the Regexp type if
present), then iterate the sorted slice and return the first match. Update
LookupByMatch to use sort.Slice on the slice of patterns before performing
re.Pattern.Match to ensure stable, repeatable matching.
---
Nitpick comments:
In `@pkg/load/load.go`:
- Line 309: Add a GoDoc comment for the exported function
ClusterProfileSetDetails: place a comment immediately above the function
declaration that begins with "ClusterProfileSetDetails" and briefly describes
what the function does, the meaning of the path parameter, and what the returned
api.ClusterProfileSetDetails and error represent (e.g., successful load vs.
failure). Keep it concise and idiomatic (one or two sentences starting with the
function name) so Go tooling and users can discover the intent.
In `@pkg/util/regexp/regexp_test.go`:
- Around line 142-150: The test "multiple patterns could match - first wins"
currently sets skipValueCheck so it cannot catch precedence regressions; update
the case to assert the exact winner from LookupByMatch instead of skipping:
remove skipValueCheck and add wantValue (expect "specific" for input
"test-cluster" since "^test-.*" should win over ".*"). If LookupByMatch defines
precedence by input order rather than map iteration, change patternPairs from a
map to an ordered structure (e.g., a slice of pattern/value pairs used by the
test) so the test reliably encodes the intended order.
In `@pkg/util/regexp/regexp.go`:
- Around line 7-10: Add GoDoc comments for the exported Regexp type, its
exported field Pattern, and the exported Compile function: write concise
package-style comments above the type Regexp describing its purpose, above the
Pattern field describing that it holds the compiled *regexp.Regexp for the
expression, and above the Compile function describing its behavior, inputs,
return values, and any error semantics so the exported API is documented for
godoc consumers.
🪄 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: 65a935c7-4755-4074-81f5-cd9179fb8c8c
📒 Files selected for processing (9)
cmd/ci-operator-checkconfig/main.gopkg/api/types.gopkg/api/types_test.gopkg/load/load.gopkg/util/regexp/regexp.gopkg/util/regexp/regexp_test.gopkg/validation/config.gopkg/validation/test.gopkg/validation/test_test.go
✅ Files skipped from review due to trivial changes (1)
- pkg/api/types_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- pkg/validation/config.go
- pkg/validation/test.go
- pkg/validation/test_test.go
- cmd/ci-operator-checkconfig/main.go
- pkg/api/types.go
| func LookupByMatch[T any](patterns map[Regexp]T, s string) (T, bool) { | ||
| for re, val := range patterns { | ||
| if re.Pattern.Match([]byte(s)) { | ||
| return val, true | ||
| } | ||
| } |
There was a problem hiding this comment.
Lookup result is nondeterministic when multiple regexes match
LookupByMatch returns the first match from a Go map iteration, which is random. If allowlist patterns overlap, behavior can flip between runs and incorrectly allow/deny entries.
💡 Deterministic matching sketch
+import "sort"
...
func LookupByMatch[T any](patterns map[Regexp]T, s string) (T, bool) {
- for re, val := range patterns {
- if re.Pattern.Match([]byte(s)) {
- return val, true
- }
- }
+ keys := make([]Regexp, 0, len(patterns))
+ for re := range patterns {
+ keys = append(keys, re)
+ }
+ sort.Slice(keys, func(i, j int) bool { return keys[i].expr < keys[j].expr })
+ for _, re := range keys {
+ if re.Pattern.MatchString(s) {
+ return patterns[re], true
+ }
+ }📝 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.
| func LookupByMatch[T any](patterns map[Regexp]T, s string) (T, bool) { | |
| for re, val := range patterns { | |
| if re.Pattern.Match([]byte(s)) { | |
| return val, true | |
| } | |
| } | |
| func LookupByMatch[T any](patterns map[Regexp]T, s string) (T, bool) { | |
| keys := make([]Regexp, 0, len(patterns)) | |
| for re := range patterns { | |
| keys = append(keys, re) | |
| } | |
| sort.Slice(keys, func(i, j int) bool { return keys[i].expr < keys[j].expr }) | |
| for _, re := range keys { | |
| if re.Pattern.MatchString(s) { | |
| return patterns[re], true | |
| } | |
| } | |
| var zero T | |
| return zero, false | |
| } |
🤖 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 `@pkg/util/regexp/regexp.go` around lines 44 - 49, LookupByMatch is
nondeterministic because it iterates a map and returns the first matching
Regexp, so overlapping patterns can yield random results; fix by making matching
deterministic: collect the map entries (keys or key/value pairs) into a slice,
sort that slice by a stable key (e.g., the Regexp pattern string via
re.Pattern.String() or a priority field on the Regexp type if present), then
iterate the sorted slice and return the first match. Update LookupByMatch to use
sort.Slice on the slice of patterns before performing re.Pattern.Match to ensure
stable, repeatable matching.
|
Scheduling tests matching the |
|
/test e2e |
|
@danilo-gemoli: The following test failed, say
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. |
|
I'm not a greatest fan of regexp in configuration files but here it seems like a pragmatic choice. I think eventually someone will ask you for a regex in the job names too (the list items) ;) /hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
Neither am I.
and I won't do anything until then because:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danilo-gemoli, Prucek The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit aeb8c9d
into
openshift:main
3 participants
Revisit the Cluster Profile Sets policy that has been introduced with #5087 by adding a whitelist that allows some tests to use the underlying cluster profile rather than the cluster profile set.
With the actual policy, the following test in
ci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-4.22__multi-nightly.yamldoesn't pass the validation since it is usingawsrather thanopenshift-org-aws:After we merge this PR along with another one in
openshift/releasethat would produce an allowlist like the following one, the test will pass the validation:{ "cluster_profile_sets": { "openshift-org-aws": [ "aws", "aws-2", "aws-3", "aws-4", "aws-5" ], "tests_allowlist": { "openshift/openshift-tests-private": { "release-4.22": { "multi-nightly": [ "aws-ipi-public-ipv4-pool-byo-subnet-mini-perm-arm-f14" ] } } } }The
tests_allowlistconsists of:{ "tests_allowlist" : { "${ORG}/${REPO}": { // Regex allowed "${BRANCH}": { // Regex allowed "${VARIANT}": [ // Regex allowed "${TEST_NAME}" // Plain text, NO regex allowed ] } } } }The type
ClusterProfileSetDetailsNewis temporary and will be deleted once we merge a PR inopenshift/releasethat will modify thecluster-profile-set-details.jsonschema to accomodate the newtests_whiteliststanza.The combination of both
ClusterProfileSetDetailsandClusterProfileSetDetailsNewmakes sure we are backward compatible until the migration is complete.Summary by CodeRabbit