-
Notifications
You must be signed in to change notification settings - Fork 106
API-1835: test/oauth-server-configobserver: adds a test to show which input resources are required to run the operator and the configObserver controller #785
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.
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds a new ApplyConfiguration test suite for the OAuth server config observer: input fixtures (cluster-scoped config resources and a Secret), expected outputs (Authentication Update/ApplyStatus and many Event creations), controller-results, and the test driver YAML. All files are new test data under oauth-server-configobserver. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
🧹 Nitpick comments (18)
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/oauths.yaml (1)
11-29
: Remove unnecessary volatile metadata for deterministic tests.Same as consoles; these fields are not needed by observers and make fixtures brittle.
Apply this diff:
- creationTimestamp: "2025-08-01T18:29:57Z" - generation: 1 - managedFields: - - apiVersion: config.openshift.io/v1 - fieldsType: FieldsV1 - fieldsV1: - f:metadata: - f:annotations: - .: {} - f:include.release.openshift.io/ibm-cloud-managed: {} - f:include.release.openshift.io/self-managed-high-availability: {} - f:release.openshift.io/create-only: {} - f:ownerReferences: - .: {} - k:{"uid":"fd412cff-9592-4cb6-b0e9-97a5c376f29e"}: {} - f:spec: {} - manager: cluster-version-operator - operation: Update - time: "2025-08-01T18:29:57Z" @@ - resourceVersion: "1775" - uid: ffaa4e11-4ed6-49c4-b520-c9d79c19d6ffAlso applies to: 36-37
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ea65-body-authentication-operator.18599d2230299800.2df24af9.yaml (1)
11-11
: Event message contains Go fmt artifacts (%!d(float64=...)
).If the controller now formats numbers cleanly, this fixture will drift. Verify current output; update either the controller message or the expected text accordingly.
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/namespaces/openshift-authentication/core/secrets.yaml (3)
6-6
: Suppress false-positive secret leak in test data.Gitleaks flags this. Since it’s synthetic, consider using a neutral placeholder (e.g., "VEVTVA==") or add an allowlist for test-data to keep CI quiet.
- apps.ci-op-gn2pz6q7-69aee.XXXXXXXXXXXXXXXXXXXXXX: NDEwNCBieXRlcyBsb25n + apps.ci-op-gn2pz6q7-69aee.XXXXXXXXXXXXXXXXXXXXXX: VEVTVA==Alternatively, add a repo-level gitleaks allowlist for this path.
28-28
: Add trailing newline.YAMLlint complaint; add a final newline to the file.
resourceVersion: "269056" +
10-20
: Trim managedFields from fixtures to reduce churn.ManagedFields are noisy and can change across k8s versions; unless asserted, drop them from test inputs.
- managedFields: - - apiVersion: v1 - fieldsType: FieldsV1 - fieldsV1: - f:data: - .: {} - f:apps.ci-op-gn2pz6q7-69aee.XXXXXXXXXXXXXXXXXXXXXX: {} - f:type: {} - manager: authentication-operator - operation: Update - time: "2025-08-01T18:34:45Z"test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/apiservers.yaml (1)
1-51
: Fixture looks correct for APIServerList; minor ordering nitContent matches a typical APIServerList response. If your harness does strict text diffs, consider moving "kind: APIServerList" next to apiVersion for readability and consistency across fixtures. Functionally fine as-is.
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-body-authentication-operator.18599d2230299800.49fb0e36.yaml (1)
11-14
: Avoid multiline wrap in message to prevent whitespace driftYAML will fold these lines into spaces, but tiny spacing differences can cause brittle comparisons. Prefer a single-line or a quoted block to lock exact text.
Apply this diff if you want a single-line:
-message: cipherSuites changed to ["TLS_AES_128_GCM_SHA256" "TLS_AES_256_GCM_SHA384" - "TLS_CHACHA20_POLY1305_SHA256" "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256" "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384" "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384" - "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256" "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256"] +message: cipherSuites changed to ["TLS_AES_128_GCM_SHA256" "TLS_AES_256_GCM_SHA384" "TLS_CHACHA20_POLY1305_SHA256" "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256" "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384" "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384" "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256" "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256"]test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ae22-body-authentication-operator.18599d2230299800.a8ecfbce.yaml (1)
11-12
: Lock the message formatting (optional)Same nit as the TLS event: keep this on one line or use an explicit quoted block for stable diffs.
-message: AuditProfile changed from '%!s(<nil>)' to 'map[audit-log-format:[json] audit-log-maxbackup:[10] - audit-log-maxsize:[100] audit-log-path:[/var/log/oauth-server/audit.log] audit-policy-file:[/var/run/configmaps/audit/audit.yaml]]' +message: AuditProfile changed from '%!s(<nil>)' to 'map[audit-log-format:[json] audit-log-maxbackup:[10] audit-log-maxsize:[100] audit-log-path:[/var/log/oauth-server/audit.log] audit-policy-file:[/var/run/configmaps/audit/audit.yaml]]'test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-body-authentication-operator.18599d2230299800.5f2cc1a1.yaml (1)
11-22
: Improve message readability with a block scalar.The escaped Go dump is hard to diff. Consider using a YAML block scalar (|) to store the exact message content without escape noise; it won’t change semantics.
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/clusterversions.yaml (1)
165-168
: Minor: co-locate kind with apiVersion for consistency.Move
kind: ClusterVersionList
next toapiVersion
at the top-level map for readability (key order doesn’t affect parsing).---- apiVersion: config.openshift.io/v1 +kind: ClusterVersionList items: ... -kind: ClusterVersionList metadata:test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/consoles.yaml (1)
51-54
: Minor: co-locate kind with apiVersion at the top-level.Same readability nit as other List files.
apiVersion: config.openshift.io/v1 +kind: ConsoleList items: ... -kind: ConsoleList metadata:test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/infrastructures.yaml (1)
1-75
: Confirm “-old” vs current inputs are intentionally identical.If the test aims to demonstrate deltas, consider trimming or differing the “old” to reflect a prior state; if stability is the goal, keep as is.
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-metadata-cluster.yaml (1)
2-3
: Verify controller/fieldManager identifiers are final.“TODO-configObserver” and “oauth-server-ConfigObserver” casing should match what the controller emits to avoid brittle assertions.
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/apiservers.yaml (1)
14-35
: Trim noisy, irrelevant fields from input fixture to reduce churn.managedFields/resourceVersion/uid are not needed for controller inputs and add maintenance burden.
- managedFields: - - apiVersion: config.openshift.io/v1 - fieldsType: FieldsV1 - fieldsV1: - f:metadata: - f:annotations: - .: {} - f:include.release.openshift.io/ibm-cloud-managed: {} - f:include.release.openshift.io/self-managed-high-availability: {} - f:oauth-apiserver.openshift.io/secure-token-storage: {} - f:release.openshift.io/create-only: {} - f:ownerReferences: - .: {} - k:{"uid":"fd412cff-9592-4cb6-b0e9-97a5c376f29e"}: {} - f:spec: - .: {} - f:audit: - .: {} - f:profile: {} - manager: cluster-version-operator - operation: Update - time: "2025-08-01T18:29:39Z" @@ - resourceVersion: "910" - uid: 5ae211a9-1169-4caa-a7b5-86a60f3aa057 + # resourceVersion/uid intentionally omitted in test inputAlso applies to: 42-43
test-data/apply-configuration/overall/oauth-server-configobserver/test.yaml (1)
7-7
: YAML lint fixes: remove trailing spaces and add newline at EOF.Prevents CI/style failures.
- + @@ - + @@ -now: 2025-08-07T22:38:20Z +now: 2025-08-07T22:38:20Z +Also applies to: 9-9, 18-18
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-body-cluster.yaml (3)
17-19
: Reduce test brittleness by avoiding environment-specific hostnamesHardcoded CI domain fragments can make fixtures fragile. Prefer stable placeholders to keep expected output deterministic.
Apply a consistent placeholder domain:
- assetPublicURL: https://console-openshift-console.apps.ci-op-gn2pz6q7-69aee.XXXXXXXXXXXXXXXXXXXXXX - loginURL: https://api.ci-op-gn2pz6q7-69aee.XXXXXXXXXXXXXXXXXXXXXX:6443 + assetPublicURL: https://console-openshift-console.apps.<cluster-domain.example> + loginURL: https://api.<cluster-domain.example>:6443 @@ - - certFile: /var/config/system/secrets/v4-0-config-system-router-certs/apps.ci-op-gn2pz6q7-69aee.XXXXXXXXXXXXXXXXXXXXXX - keyFile: /var/config/system/secrets/v4-0-config-system-router-certs/apps.ci-op-gn2pz6q7-69aee.XXXXXXXXXXXXXXXXXXXXXX + - certFile: /var/config/system/secrets/v4-0-config-system-router-certs/apps.<cluster-domain.example> + keyFile: /var/config/system/secrets/v4-0-config-system-router-certs/apps.<cluster-domain.example> @@ - - '*.apps.ci-op-gn2pz6q7-69aee.XXXXXXXXXXXXXXXXXXXXXX' + - '*.apps.<cluster-domain.example>'Also applies to: 46-49
34-44
: TLS ciphers list may be over-specifiedTLS 1.3 ciphers are not configurable in Go; including them is harmless but can cause churn if upstream defaults/order change. Consider limiting assertions in tests to minTLSVersion and TLS 1.2 ciphers, or make the check tolerant to order.
50-51
: Confirm type of identityProvidersidentityProviders is set to the string '{}' rather than an object. If the consumer expects a map, prefer an empty object.
- identityProviders: '{}' + identityProviders: {}If a literal string is intentionally required by downstream templating, keep as-is and add a brief comment in the test fixture explaining why.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (36)
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-body-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-metadata-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-options-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-body-authentication-operator.18599d2230299800.5f2cc1a1.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-metadata-authentication-operator.18599d2230299800.5f2cc1a1.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/1125-body-authentication-operator.18599d2230299800.6182ed8c.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/1125-metadata-authentication-operator.18599d2230299800.6182ed8c.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/5d38-body-authentication-operator.18599d2230299800.1d05f9ac.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/5d38-metadata-authentication-operator.18599d2230299800.1d05f9ac.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/75ea-body-authentication-operator.18599d2230299800.6d9ca9b2.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/75ea-metadata-authentication-operator.18599d2230299800.6d9ca9b2.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/aa39-body-authentication-operator.18599d2230299800.44a05c38.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/aa39-metadata-authentication-operator.18599d2230299800.44a05c38.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ae22-body-authentication-operator.18599d2230299800.a8ecfbce.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ae22-metadata-authentication-operator.18599d2230299800.a8ecfbce.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-body-authentication-operator.18599d2230299800.49fb0e36.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-metadata-authentication-operator.18599d2230299800.49fb0e36.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ea65-body-authentication-operator.18599d2230299800.2df24af9.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ea65-metadata-authentication-operator.18599d2230299800.2df24af9.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-body-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-metadata-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/controller-results.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/apiservers.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/authentications.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/clusterversions.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/consoles.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/infrastructures.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/oauths.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/apiservers.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/clusterversions.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/consoles.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/infrastructures.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/oauths.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/namespaces/openshift-authentication/core/secrets.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/test.yaml
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
test-data/apply-configuration/overall/oauth-server-configobserver/test.yaml
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-file)
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml
[error] 14-14: no new line character at the end of file
(new-line-at-end-of-file)
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/namespaces/openshift-authentication/core/secrets.yaml
[error] 28-28: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Gitleaks (8.27.2)
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/namespaces/openshift-authentication/core/secrets.yaml
5-7: Possible Kubernetes Secret detected, posing a risk of leaking credentials/tokens from your deployments
(kubernetes-secret-yaml)
🔇 Additional comments (21)
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-options-cluster.yaml (1)
1-2
: LGTM on ApplyOptions; fieldManager consistency verified. Both fixtures use “oauth-server-ConfigObserver”.test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/consoles.yaml (1)
11-39
: Trim volatile metadata fields from test fixtures
RemovecreationTimestamp
,generation
,managedFields
,resourceVersion
, anduid
to reduce fixture churn; grep shows these across many fixtures—confirm the harness doesn’t assert on any List-level or object-levelresourceVersion
/uid
before trimming.test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/aa39-body-authentication-operator.18599d2230299800.44a05c38.yaml (1)
1-21
: LGTM: event body matches expected observer output.Fields (reason, message, involvedObject, timestamps) look consistent for a legacy core/v1 Event.
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/75ea-body-authentication-operator.18599d2230299800.6d9ca9b2.yaml (1)
4-4
: Confirm deterministic timestamps or ignore them in assertions.If these Event timestamps are recomputed during test runs, they’ll cause brittle diffs. Ensure the harness ignores first/lastTimestamp comparisons or fix them to stable sentinel values.
Also applies to: 10-10
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/clusterversions.yaml (1)
1-169
: LGTM: ClusterVersionList fixture is comprehensiveFields and conditions look realistic and consistent with CVO status. No changes needed.
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-body-authentication-operator.18599d2230299800.5f2cc1a1.yaml (2)
1-1
: Confirm intent to use core/v1 Event (not events.k8s.io/v1).If the operator now emits events via events.k8s.io/v1 in newer payloads, this fixture will drift. Please confirm the API group/version targeted by the test.
5-9
: ObjectReference fields are minimal; verify stability across clusters.Only kind/name/namespace are set. If the harness ever compares uid/resourceVersion/apiVersion, this may flake. Confirm the comparator ignores those.
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/clusterversions.yaml (1)
80-85
: LGTM: Owner/UID and generation look coherent.The ClusterVersion metadata (name/version UID and generations) is internally consistent for use as an ownerRef across fixtures.
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/consoles.yaml (1)
40-51
: LGTM: Console status URL present and matches test hostnames.This should satisfy observers depending on consoleURL.
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/authentications.yaml (2)
75-78
: LGTM: integratedOAuthMetadata present.Observer inputs for integrated OAuth are populated.
67-75
: Confirm empty-string values forspec.type
andspec.serviceAccountIssuer
are valid.
spec.type: ""
andspec.serviceAccountIssuer: ""
differ from omitting the field—verify that your CRD schema and observer logic accept empty strings (rather than rejecting or defaulting them).test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io-old/oauths.yaml (1)
38-42
: LGTM: Minimal OAuth spec is intentionally empty.Sufficient for exercising observers that read oauth config presence.
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/5d38-body-authentication-operator.18599d2230299800.1d05f9ac.yaml (1)
1-21
: LGTM — event fixture looks consistent.Fields align with core/v1 Event expectations for this harness (involvedObject, reason, message, timestamps).
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/infrastructures.yaml (1)
1-75
: LGTM — InfrastructureList input is well-formed and redacted appropriately.test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-body-cluster.yaml (1)
1-9
: LGTM — status condition captures non-degraded observation.test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/controller-results.yaml (1)
1-82
: Verified guard against drift conditions: exactly one Succeeded entry (TODO-configObserver) and no duplicate controller names.test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/1125-body-authentication-operator.18599d2230299800.6182ed8c.yaml (1)
1-22
: LGTM: Event body matches expected ObserveConsoleURL signal.test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-body-cluster.yaml (4)
1-12
: Structure and types look correct for Authentication.observedConfigResource shape, managementState, and observedConfig nesting match operator expectations. No blockers.
14-16
: CORS origin regexes look standardPatterns for localhost and 127.0.0.1 align with typical oauth-server defaults. No change needed.
23-32
: Audit args: types and formatting are compatibleValues are arrays of strings as expected by serverArguments. Paths look sane. No action.
52-52
: unsupportedConfigOverrides: null is fineExplicit null keeps intent clear and avoids accidental merges. LGTM.
...us/cluster-scoped-resources/operator.openshift.io/authentications/9509-metadata-cluster.yaml
Outdated
Show resolved
Hide resolved
...on-operator/core/events/0dc4-metadata-authentication-operator.18599d2230299800.5f2cc1a1.yaml
Outdated
Show resolved
Hide resolved
...on-operator/core/events/1125-metadata-authentication-operator.18599d2230299800.6182ed8c.yaml
Outdated
Show resolved
Hide resolved
...on-operator/core/events/5d38-metadata-authentication-operator.18599d2230299800.1d05f9ac.yaml
Outdated
Show resolved
Hide resolved
...on-operator/core/events/75ea-metadata-authentication-operator.18599d2230299800.6d9ca9b2.yaml
Outdated
Show resolved
Hide resolved
action: Create | ||
controllerInstanceName: "" | ||
generateName: "" | ||
mame: authentication-operator.18599d2230299800.49fb0e36 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Fix typo: use 'name', not 'mame'.
The metadata key should be 'name'. Typo will break consumers that parse these metadata stubs.
Apply this diff:
- mame: authentication-operator.18599d2230299800.49fb0e36
+ name: authentication-operator.18599d2230299800.49fb0e36
Run to verify no other occurrences remain:
🏁 Script executed:
#!/bin/bash
rg -nP '\bmame\s*:' test-data/apply-configuration/overall/oauth-server-configobserver
Length of output: 2776
Fix 'mame' typo in metadata keys across oauth-server-configobserver fixtures
rg -nP '\bmame\s*:' reports 10 occurrences in test-data/apply-configuration/overall/oauth-server-configobserver/expected-output. Replace each mame:
with name:
to ensure metadata parses correctly.
🤖 Prompt for AI Agents
In
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-metadata-authentication-operator.18599d2230299800.49fb0e36.yaml
around line 4, and in the other 9 occurrences under
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output,
the metadata key is misspelled as "mame:"; replace each "mame:" with the correct
"name:" so Kubernetes metadata parses correctly, and run a quick grep (rg -nP
'\bmame\s*:') afterwards to verify no remaining instances.
action: Create | ||
controllerInstanceName: "" | ||
generateName: "" | ||
mame: authentication-operator.18599d2230299800.2df24af9 | ||
namespace: openshift-authentication-operator | ||
resourceType: | ||
Group: "" | ||
Resource: events | ||
Version: v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Search-and-fix across the repo for 'mame:' in fixtures.
To avoid missing siblings, run this and patch all occurrences.
I can generate a bulk patch once you confirm the paths.
🏁 Script executed:
#!/bin/bash
rg -nP '^\s*mame:\s' test-data | sed -n '1,200p'
Length of output: 21010
Fix all ‘mame:’ typos to ‘name:’ in test fixtures.
Run
rg -nP '^\s*mame:\s' test-data
and replace each occurrence; I can supply a bulk patch for these corrections.
🤖 Prompt for AI Agents
In
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ea65-metadata-authentication-operator.18599d2230299800.2df24af9.yaml
lines 1-9, there is a typo "mame:" that should be "name:"; update the key to
"name:" and then scan the test-data directory for other occurrences (eg. rg -nP
'^\s*mame:\s') and replace each with "name:" so all fixtures use the correct
key.
...on-operator/core/events/ea65-metadata-authentication-operator.18599d2230299800.2df24af9.yaml
Outdated
Show resolved
Hide resolved
action: Update | ||
controllerInstanceName: TODO-configObserver | ||
generateName: "" | ||
mame: cluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Typo in metadata key breaks matcher ("mame" → "name").
This will prevent the Update from targeting the "cluster" Authentication.
- mame: cluster
+ name: cluster
🏁 Script executed:
#!/bin/bash
# Find other occurrences of the same typo in repo
rg -nP '^\s*mame:\s' -C1
Length of output: 60901
Fix ‘mame’ → ‘name’ typo in metadata across all test fixtures
Found dozens of occurrences of the mame:
typo in expected-output YAML under test-data/apply-configuration (including both minimal-cluster and oauth-server-configobserver scenarios); replace every mame:
key with name:
to ensure matchers target resources correctly.
🤖 Prompt for AI Agents
In
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-metadata-cluster.yaml
around line 4 there is a typo "mame:" that should be "name:"; locate and replace
this occurrence and then scan all files under test-data/apply-configuration
(including minimal-cluster and oauth-server-configobserver expected-output
fixtures) to replace every `mame:` key with `name:` so test matchers find the
correct metadata field.
managementState: Managed | ||
observedConfig: | ||
oauthServer: | ||
serverArguments: No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use empty object for serverArguments and add newline at EOF.
“serverArguments:” alone is null, not {}. Use an empty map to match expected schema, and satisfy YAMLlint’s newline check.
- serverArguments:
+ serverArguments: {}
Also add a trailing newline to the file.
📝 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.
serverArguments: | |
serverArguments: {} |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 14-14: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml
around line 14, replace the bare "serverArguments:" which YAML treats as null
with an explicit empty mapping "serverArguments: {}" and ensure you add a
trailing newline at EOF; this will match the expected schema and satisfy
YAMLLint's newline requirement.
/assign @benluddy @bertinatto |
b620652
to
9f399ff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml (1)
14-14
: Use an empty mapping for serverArguments and add a trailing newline.Bare "serverArguments:" is null; prefer an explicit empty map and end file with a newline.
oauthServer: - serverArguments: + serverArguments: {}
🧹 Nitpick comments (4)
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/namespaces/openshift-authentication/core/secrets.yaml (2)
28-28
: Add trailing newline at EOF.Fixes YAMLlint new-line-at-end-of-file.
1-28
: Docs vs fixtures: test lists a ConfigMap, fixture provides a Secret.The test.yaml says “core/configmaps/.../v4-0-config-system-router-certs” but this file is a SecretList with Secret v4-0-config-system-router-certs. Align the test description to “core/secrets/...”.
test-data/apply-configuration/overall/oauth-server-configobserver/test.yaml (2)
7-9
: Remove trailing spaces on blank lines.Keeps YAMLlint happy.
18-18
: Add trailing newline at EOF.Fixes YAMLlint new-line-at-end-of-file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (30)
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-body-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-metadata-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-options-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-body-authentication-operator.18599d2230299800.5f2cc1a1.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-metadata-authentication-operator.18599d2230299800.5f2cc1a1.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/1125-body-authentication-operator.18599d2230299800.6182ed8c.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/1125-metadata-authentication-operator.18599d2230299800.6182ed8c.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/5d38-body-authentication-operator.18599d2230299800.1d05f9ac.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/5d38-metadata-authentication-operator.18599d2230299800.1d05f9ac.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/75ea-body-authentication-operator.18599d2230299800.6d9ca9b2.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/75ea-metadata-authentication-operator.18599d2230299800.6d9ca9b2.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/aa39-body-authentication-operator.18599d2230299800.44a05c38.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/aa39-metadata-authentication-operator.18599d2230299800.44a05c38.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ae22-body-authentication-operator.18599d2230299800.a8ecfbce.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ae22-metadata-authentication-operator.18599d2230299800.a8ecfbce.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-body-authentication-operator.18599d2230299800.49fb0e36.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-metadata-authentication-operator.18599d2230299800.49fb0e36.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ea65-body-authentication-operator.18599d2230299800.2df24af9.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ea65-metadata-authentication-operator.18599d2230299800.2df24af9.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-body-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-metadata-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/controller-results.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/apiservers.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/clusterversions.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/consoles.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/infrastructures.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/oauths.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/namespaces/openshift-authentication/core/secrets.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/test.yaml
(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/controller-results.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ea65-body-authentication-operator.18599d2230299800.2df24af9.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/1125-body-authentication-operator.18599d2230299800.6182ed8c.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/75ea-body-authentication-operator.18599d2230299800.6d9ca9b2.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/5d38-body-authentication-operator.18599d2230299800.1d05f9ac.yaml
🚧 Files skipped from review as they are similar to previous changes (20)
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/aa39-metadata-authentication-operator.18599d2230299800.44a05c38.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-metadata-cluster.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-options-cluster.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/aa39-body-authentication-operator.18599d2230299800.44a05c38.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-body-cluster.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/75ea-metadata-authentication-operator.18599d2230299800.6d9ca9b2.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/oauths.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-body-authentication-operator.18599d2230299800.49fb0e36.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ae22-metadata-authentication-operator.18599d2230299800.a8ecfbce.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-metadata-authentication-operator.18599d2230299800.5f2cc1a1.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-metadata-cluster.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-body-authentication-operator.18599d2230299800.5f2cc1a1.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/infrastructures.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ea65-metadata-authentication-operator.18599d2230299800.2df24af9.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/5d38-metadata-authentication-operator.18599d2230299800.1d05f9ac.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/apiservers.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/consoles.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ae22-body-authentication-operator.18599d2230299800.a8ecfbce.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/1125-metadata-authentication-operator.18599d2230299800.6182ed8c.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-metadata-authentication-operator.18599d2230299800.49fb0e36.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml
[error] 14-14: no new line character at the end of file
(new-line-at-end-of-file)
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/namespaces/openshift-authentication/core/secrets.yaml
[error] 28-28: no new line character at the end of file
(new-line-at-end-of-file)
test-data/apply-configuration/overall/oauth-server-configobserver/test.yaml
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (2)
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-body-cluster.yaml (1)
1-52
: LGTM: observedConfig shape and types look correct.Arrays-for-args, TLS profile, and named certs align with observer outputs.
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/clusterversions.yaml (1)
1-169
: LGTM: sufficient fixture for CVO-dependent startup.No blocking issues spotted.
/test test-operator-integration |
/retest-required |
@p0lyn0mial: This pull request references API-1835 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.21.0" version, but no target version was set. In response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/lgtm |
/hold @p0lyn0mial coderabbit has detected some typos, namely mame instead of name -- I think these should be fixed, right? |
will be addressed in |
…ources are required to run the operator and the configObserver controller
9f399ff
to
a180796
Compare
/hold cancel fixed :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
@p0lyn0mial: This pull request references API-1835 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.21.0" version, but no target version was set. In response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bertinatto, liouk, p0lyn0mial The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml (1)
12-14
: SetserverArguments
to an empty map.Leaving the key bare makes it
null
, which mismatches the schema and the earlier review request—please render it as an empty mapping (serverArguments: {}
) so consumers still see an object.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base
setting
📒 Files selected for processing (31)
test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/90a1-body-system-COLON-openshift-COLON-openshift-authenticator-.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-body-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-metadata-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-options-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-body-authentication-operator.18599d2230299800.5f2cc1a1.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-metadata-authentication-operator.18599d2230299800.5f2cc1a1.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/1125-body-authentication-operator.18599d2230299800.6182ed8c.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/1125-metadata-authentication-operator.18599d2230299800.6182ed8c.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/5d38-body-authentication-operator.18599d2230299800.1d05f9ac.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/5d38-metadata-authentication-operator.18599d2230299800.1d05f9ac.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/75ea-body-authentication-operator.18599d2230299800.6d9ca9b2.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/75ea-metadata-authentication-operator.18599d2230299800.6d9ca9b2.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/aa39-body-authentication-operator.18599d2230299800.44a05c38.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/aa39-metadata-authentication-operator.18599d2230299800.44a05c38.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ae22-body-authentication-operator.18599d2230299800.a8ecfbce.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ae22-metadata-authentication-operator.18599d2230299800.a8ecfbce.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-body-authentication-operator.18599d2230299800.49fb0e36.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-metadata-authentication-operator.18599d2230299800.49fb0e36.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ea65-body-authentication-operator.18599d2230299800.2df24af9.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ea65-metadata-authentication-operator.18599d2230299800.2df24af9.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-body-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-metadata-cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/controller-results.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/apiservers.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/clusterversions.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/consoles.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/infrastructures.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/oauths.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/namespaces/openshift-authentication/core/secrets.yaml
(1 hunks)test-data/apply-configuration/overall/oauth-server-configobserver/test.yaml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/5d38-metadata-authentication-operator.18599d2230299800.1d05f9ac.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/ea65-body-authentication-operator.18599d2230299800.2df24af9.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/9509-metadata-cluster.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/aa39-body-authentication-operator.18599d2230299800.44a05c38.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/5d38-body-authentication-operator.18599d2230299800.1d05f9ac.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Update/cluster-scoped-resources/operator.openshift.io/authentications/ad90-metadata-cluster.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/1125-body-authentication-operator.18599d2230299800.6182ed8c.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/1125-metadata-authentication-operator.18599d2230299800.6182ed8c.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/cluster-scoped-resources/config.openshift.io/consoles.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/controller-results.yaml
- test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-body-authentication-operator.18599d2230299800.49fb0e36.yaml
🔇 Additional comments (6)
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/75ea-metadata-authentication-operator.18599d2230299800.6d9ca9b2.yaml (1)
1-9
: Metadata entry looks good.
name
, namespace, and resourceType match the companion body fixture; no further action needed.test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/75ea-body-authentication-operator.18599d2230299800.6d9ca9b2.yaml (1)
1-23
: Event payload consistent with observer output.Fields line up with expected router secret observation; nothing to fix.
test-data/apply-configuration/overall/oauth-server-configobserver/input-dir/namespaces/openshift-authentication/core/secrets.yaml (1)
1-28
: Secret fixture matches expected schema.Data/key layout and metadata look correct for the router cert secret.
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/cfad-metadata-authentication-operator.18599d2230299800.49fb0e36.yaml (1)
1-9
: Metadata stub looks good.
name
is corrected and the resource envelope matches the other event fixtures. Thanks for cleaning this up.test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-metadata-authentication-operator.18599d2230299800.5f2cc1a1.yaml (1)
1-9
: Second metadata fixture also reads correctly.Key naming and resource envelope align with the rest of the suite.
test-data/apply-configuration/overall/oauth-server-configobserver/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/0dc4-body-authentication-operator.18599d2230299800.5f2cc1a1.yaml (1)
1-33
: Event body fixture matches the expected diff output.All required fields are present and the diff text aligns with the observed-config change scenario.
input-dir: | ||
- config.openshift.io/clusterversions: required to "start the operator" (CreateOperatorStarter/prepareOauthOperator) | ||
- config.openshift.io/oauths/cluster: required by observe_idps.go, observe_tokenconfig.go | ||
- config.openshift.io/apiservers/cluster: required by observe_cors.go, observe_tlssecurityprofile.go | ||
- config.openshift.io/consoles/cluster: required by some observer | ||
- config.openshift.io/infrastructures/cluster: required by some observer | ||
- core/configmaps/openshift-authentication/v4-0-config-system-router-certs: required by some observer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct the listed resource kind (ConfigMap → Secret).
The fixture under input-dir
ships the router cert as core/secrets/openshift-authentication/v4-0-config-system-router-certs.yaml
. Keeping the list entry as core/configmaps/...
misstates the required input and will confuse anyone copying it. Please update the line to use core/secrets/openshift-authentication/v4-0-config-system-router-certs
.
🤖 Prompt for AI Agents
In test-data/apply-configuration/overall/oauth-server-configobserver/test.yaml
around lines 10 to 16, the input-dir entry incorrectly lists the router cert as
core/configmaps/openshift-authentication/v4-0-config-system-router-certs; update
that single list item to
core/secrets/openshift-authentication/v4-0-config-system-router-certs so the
fixture matches the actual file path and resource kind used by the tests.
/verified by @p0lyn0mial |
@p0lyn0mial: This PR has been marked as verified by In response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
@p0lyn0mial: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions 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. |
Uh oh!
There was an error while loading. Please reload this page.