API-1835: test/oauth-server-payloadConfigController: adds a test to show which input resources are required to run the operator and the payloadConfigController #786
Conversation
WalkthroughUpdated ManagementResources.ExactResources in runOutputResources to add the ConfigMap Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
/assign @benluddy @bertinatto |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/c450-body-authentication-operator.18599d2230299800.18b41977.yaml (1)
1-23: Fix event metadata key typo (“mame” → “name”)
Replace everymame:entry withname:in the YAML files undertest-data/apply-configuration/**/expected-output/**/events/**/*-metadata-*.yaml.
🧹 Nitpick comments (16)
test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/core/secrets.yaml (2)
12-26: YAML indentation nit (yamllint) under metadata.managedFieldsAlign list dash with the key to satisfy the linter.
- managedFields: - - apiVersion: v1 - fieldsType: FieldsV1 - fieldsV1: + managedFields: + - apiVersion: v1 + fieldsType: FieldsV1 + fieldsV1:
34-34: Add trailing newlineSilences “no newline at end of file”.
- resourceVersion: "269056" + resourceVersion: "269056" +test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/core/services/oauth-openshift.yaml (2)
10-10: Trim volatile fields from fixture to reduce flakinesscreationTimestamp, managedFields, resourceVersion, uid, and status are environment-specific and unnecessary for this input snapshot. Consider removing them to keep the fixture minimal and stable.
Apply this diff:
- creationTimestamp: "2025-08-01T18:34:31Z" - managedFields: - - apiVersion: v1 - fieldsType: FieldsV1 - fieldsV1: - ... - manager: authentication-operator - operation: Update - time: "2025-08-01T18:34:31Z" - - apiVersion: v1 - fieldsType: FieldsV1 - fieldsV1: - ... - manager: service-ca-operator - operation: Update - time: "2025-08-01T18:34:40Z" - resourceVersion: "8642" - uid: 751fd500-5c7a-4b59-8c64-ea8aad587954 -status: - loadBalancer: {}Also applies to: 13-50, 53-55, 72-73
56-58: Avoid pinning clusterIP/clusterIPs in a create-time manifestIf this object is ever applied (vs. only loaded), specifying concrete clusterIP/clusterIPs can be rejected or cause churn. Prefer omitting them and letting the apiserver allocate.
-spec: - clusterIP: 172.30.228.83 - clusterIPs: - - 172.30.228.83 +spec:If the test harness never applies these and only uses them as existing state, feel free to keep—but please confirm.
test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/route.openshift.io/routes.yaml (1)
7-7: Strip mutable metadata/status to stabilize the route fixturecreationTimestamp, managedFields, resourceVersion, uid, and status.ingress are cluster-derived and not needed for the controller to read spec.host/target. Removing them will reduce fixture churn.
- creationTimestamp: "2025-08-01T18:44:36Z" - managedFields: - ... - resourceVersion: "20297" - uid: f3652ccc-9da2-49a3-8819-cf9b3cbc1fb2 - status: - ingress: - - conditions: - - lastTransitionTime: "2025-08-01T18:44:36Z" - status: "True" - type: Admitted - host: oauth-openshift.apps.ci-op-gn2pz6q7-69aee.XXXXXXXXXXXXXXXXXXXXXX - routerCanonicalHostname: router-default.apps.ci-op-gn2pz6q7-69aee.XXXXXXXXXXXXXXXXXXXXXX - routerName: default - wildcardPolicy: None -... -metadata: - resourceVersion: "269050"Also applies to: 11-43, 46-47, 60-69, 72-72
test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/controller-results.yaml (1)
1-82: Reduce assertion surface of controller resultsHard-coding statuses for many unrelated controllers is brittle. Prefer asserting only the presence and Succeeded status for TODO-payloadConfigController (and ignoring others) to avoid churn when controller sets change.
I can help adjust the harness to support partial/contains assertions if desired.
test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/c450-body-authentication-operator.18599d2230299800.18b41977.yaml (1)
1-23: Optional: add involvedObject.apiVersion/uid for completenessNot required, but adding apiVersion and uid can make the event more realistic.
test-data/apply-configuration/overall/oauth-server-payloadcontroller/test.yaml (2)
5-16: Tighten wording in descriptionMinor copyedits for clarity and consistency (“config” over “cfg”, “artifact” over “artefact”) while keeping this as documentation.
description: > This test runs only the controller, which produces the configuration for oauth-server. The purpose of this test is to show which input resources are required to run the operator and the controller. input-dir: - config.openshift.io/clusterversions: required to "start the operator" (CreateOperatorStarter/prepareOauthOperator) - config.openshift.io/authentications/cluster: required by the controller - - operator.openshift.io/authentications/cluster: required by the controller, otherwise OAuthConfigDegraded is put and no cfg - - openshift-authentication/route.openshift.io/routes/oauth-openshift: required by the controller, otherwise OAuthConfigRouteDegraded is put and no cfg - - openshift-authentication/core/services/oauth-openshift: required by the controller, otherwise OAuthConfigServiceDegraded is put and no cfg - - openshift-authentication/core/secrets/v4-0-config-system-session: not strictly required but it makes the test stable and on a real system this artefact will be created only once (?) and then reused + - operator.openshift.io/authentications/cluster: required by the controller; otherwise OAuthConfigDegraded is set and no config is produced + - openshift-authentication/route.openshift.io/routes/oauth-openshift: required by the controller; otherwise OAuthConfigRouteDegraded is set and no config is produced + - openshift-authentication/core/services/oauth-openshift: required by the controller; otherwise OAuthConfigServiceDegraded is set and no config is produced + - openshift-authentication/core/secrets/v4-0-config-system-session: not strictly required, but it makes the test stable; on a real system this artifact is typically created once and reused
18-18: Add trailing newlineYAML linter reports missing newline at EOF. Please add one.
test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/config.openshift.io/authentications.yaml (1)
6-13: Trim server-populated metadata to reduce fixture brittlenesscreationTimestamp, generation, managedFields, resourceVersion, and uid are server-filled and can cause needless churn in golden tests. Prefer omitting them in input fixtures.
Apply targeted removals:
metadata: - creationTimestamp: "2025-08-01T18:29:45Z" - generation: 2 - managedFields: - - apiVersion: config.openshift.io/v1 - ... - - apiVersion: config.openshift.io/v1 - ... - - apiVersion: config.openshift.io/v1 - ... name: cluster ownerReferences: - apiVersion: config.openshift.io/v1 kind: ClusterVersion name: version uid: fd412cff-9592-4cb6-b0e9-97a5c376f29e - resourceVersion: "20310" - uid: 7adc5a7e-47eb-41c7-af2e-9faa138dccc5 @@ metadata: - continue: "" - resourceVersion: "269028" + {}Also applies to: 29-58, 65-66, 78-81
test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/config.openshift.io/clusterversions.yaml (2)
6-19: Remove nondeterministic metadata for stabilityStrip creationTimestamp, generation, managedFields, resourceVersion, and uid from test inputs to avoid fragile diffs.
metadata: - creationTimestamp: "2025-08-01T18:29:12Z" - generation: 2 - managedFields: - - apiVersion: config.openshift.io/v1 - ... - - apiVersion: config.openshift.io/v1 - ... name: version - resourceVersion: "31637" - uid: fd412cff-9592-4cb6-b0e9-97a5c376f29e @@ -metadata: - continue: "" - resourceVersion: "269027" +metadata: {}Also applies to: 20-82, 165-168
83-124: Signal/status payload is comprehensive; consider slimming to essentialsOnly include status fields the payloadConfigController actually depends on (e.g., none, if purely config generation). This keeps fixtures readable and resilient.
Also applies to: 125-164
test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml (4)
5-11: Cull server-managed metadata fieldsRemove creationTimestamp, generation, large managedFields blob, resourceVersion, and uid to prevent flaky golden updates.
metadata: - creationTimestamp: "2025-08-01T18:29:46Z" - generation: 9 - managedFields: - - apiVersion: operator.openshift.io/v1 - ... @@ - resourceVersion: "30999" - uid: fa91c2c0-b4a8-44f5-b2bf-1d34f1ffba2dAlso applies to: 12-29, 711-787
788-860: Minimize observedConfig to controller-relevant bitsIf payloadConfigController only needs route/service/session secret to build oauth-server config, consider pruning observedConfig (etcd-servers, cipher suites, domains, audit settings). Reduces noise and future churn.
861-1095: Status conditions are exhaustive; keep only those impacting the controller under testRetain only OAuthConfig*, OAuthSessionSecret*, and Route/Service related conditions if the test assertions don’t read the rest. Shrinks fixture size without losing signal.
780-786: Optional: drop OwnerReference.uid to avoid hard-couplingSince the referenced ClusterVersion exists in the same inputs, name/kind/apiVersion are sufficient for intent. Dropping uid can improve portability across regenerated fixtures.
📜 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 (13)
pkg/cmd/mom/output_resources_command.go(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/c450-body-authentication-operator.18599d2230299800.18b41977.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/c450-metadata-authentication-operator.18599d2230299800.18b41977.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication/core/configmaps/82b1-body-v4-0-config-system-cliconfig.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication/core/configmaps/82b1-metadata-v4-0-config-system-cliconfig.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/controller-results.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/config.openshift.io/authentications.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/config.openshift.io/clusterversions.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/core/secrets.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/core/services/oauth-openshift.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/route.openshift.io/routes.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/test.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
test-data/apply-configuration/overall/oauth-server-payloadcontroller/test.yaml
[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-payloadcontroller/input-dir/namespaces/openshift-authentication/core/secrets.yaml
[warning] 13-13: wrong indentation: expected 4 but found 6
(indentation)
[error] 34-34: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (11)
test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication/core/configmaps/82b1-metadata-v4-0-config-system-cliconfig.yaml (1)
1-9: Potential schema typo: mame → nameConfirm whether the payload schema expects “name”. If so, fix the key.
- mame: v4-0-config-system-cliconfig + name: v4-0-config-system-cliconfigtest-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/c450-metadata-authentication-operator.18599d2230299800.18b41977.yaml (1)
1-9: Same possible typo: mame → nameVerify the schema and adjust if required.
- mame: authentication-operator.18599d2230299800.18b41977 + name: authentication-operator.18599d2230299800.18b41977test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication/core/configmaps/82b1-body-v4-0-config-system-cliconfig.yaml (1)
1-10: LGTM for expected body contentConfigMap structure and key name align with the new managed resource.
test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/core/services/oauth-openshift.yaml (1)
63-69: Port mapping looks correctService name/port to targetPort 6443 matches oauth-server expectations and the Route’s targetPort. LGTM.
test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/route.openshift.io/routes.yaml (1)
49-58: Spec alignment looks goodRoute passthrough TLS to targetPort 6443 and Service backend name match. LGTM.
Also applies to: 52-55
test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/config.openshift.io/authentications.yaml (2)
61-75: OwnerReference UID consistency looks goodAuthentication.ownerReferences.uid matches the ClusterVersion uid provided in the companion fixture. No action needed.
67-75: Confirm webhook kubeconfig name isn’t required by the focused controllerIf payloadConfigController doesn’t read spec.webhookTokenAuthenticator.kubeConfig, consider dropping it to keep inputs minimal. Otherwise, ensure any referenced Secret/ConfigMap appears in the input-dir if the controller dereferences it.
Would you like a quick script to assert that no controller in this test path reads this field?
test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/config.openshift.io/clusterversions.yaml (1)
80-85: Name alignment is correctClusterVersion name “version” matches all OwnerReferences elsewhere. Good.
test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml (3)
780-786: OwnerReference alignment is correctOwnerReferences point to ClusterVersion/name “version” with the matching uid from the CV fixture. Looks good.
837-844: Audit policy path references an audit ConfigMapobservedConfig.oauthServer.serverArguments.audit-policy-file points to /var/run/configmaps/audit/audit.yaml, but the PR switched management outputs to v4-0-config-system-cliconfig. Verify the controller under test doesn’t rely on an audit CM; if not, consider removing this to avoid confusion.
851-856: Masked domains: good practiceUse of XXXXXXXXXXXXXX placeholders prevents accidental environment coupling. Keep as-is.
| libraryoutputresources.ExactConfigMap("openshift-authentication", "audit"), | ||
| libraryoutputresources.ExactConfigMap("openshift-authentication", "v4-0-config-system-trusted-ca-bundle"), | ||
| libraryoutputresources.ExactConfigMap("openshift-authentication", "v4-0-config-system-cliconfig"), | ||
| libraryoutputresources.ExactDeployment("openshift-authentication", "oauth-openshift"), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Intended replacement not fully applied (old ConfigMaps still listed).
PR summary says “replace” audit and trusted-ca-bundle with cliconfig, but both old entries remain. Remove them to match intent and tests.
libraryoutputresources.ExactNamespace("openshift-oauth-apiserver"),
- libraryoutputresources.ExactConfigMap("openshift-authentication", "audit"),
- libraryoutputresources.ExactConfigMap("openshift-authentication", "v4-0-config-system-trusted-ca-bundle"),
libraryoutputresources.ExactConfigMap("openshift-authentication", "v4-0-config-system-cliconfig"),Run to double-check for lingering references:
🏁 Script executed:
#!/bin/bash
rg -nP 'ExactConfigMap\("openshift-authentication",\s*"(audit|v4-0-config-system-trusted-ca-bundle)"\)'Length of output: 655
Remove obsolete ConfigMap entries from input and output commands
- In pkg/cmd/mom/output_resources_command.go (lines 30–31), delete
libraryoutputresources.ExactConfigMap("openshift-authentication", "audit") libraryoutputresources.ExactConfigMap("openshift-authentication", "v4-0-config-system-trusted-ca-bundle")
- In pkg/cmd/mom/input_resources_command.go (lines 57–59), similarly remove
libraryinputresources.ExactConfigMap("openshift-authentication", "audit") libraryinputresources.ExactConfigMap("openshift-authentication", "v4-0-config-system-trusted-ca-bundle")
to match the PR summary and associated tests.
🤖 Prompt for AI Agents
In pkg/cmd/mom/output_resources_command.go around lines 30-33 and
pkg/cmd/mom/input_resources_command.go around lines 57-59, remove the two
obsolete ExactConfigMap entries for the openshift-authentication namespace
("audit" and "v4-0-config-system-trusted-ca-bundle") from the resource lists so
the input and output commands no longer include those configmaps; ensure commas
and surrounding list formatting remain valid after deletion.
| message: Created ConfigMap/v4-0-config-system-cliconfig -n openshift-authentication | ||
| because it was missing |
There was a problem hiding this comment.
Fix multi-line message (newline will not match expected string)
The message spans two YAML lines, producing an embedded newline. Use a folded scalar to keep it a single line.
-message: Created ConfigMap/v4-0-config-system-cliconfig -n openshift-authentication
- because it was missing
+message: >-
+ Created ConfigMap/v4-0-config-system-cliconfig -n openshift-authentication because it was missing📝 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.
| message: Created ConfigMap/v4-0-config-system-cliconfig -n openshift-authentication | |
| because it was missing | |
| message: >- | |
| Created ConfigMap/v4-0-config-system-cliconfig -n openshift-authentication because it was missing |
🤖 Prompt for AI Agents
In
test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/c450-body-authentication-operator.18599d2230299800.18b41977.yaml
around lines 11-12 the message value is split across two YAML lines creating an
embedded newline; change the YAML value to a folded scalar (using >) so the
message becomes a single-line string in the parsed/expected output, preserving
spaces between words and removing the literal newline.
|
/test test-operator-integration |
p0lyn0mial
changed the title
|
@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. |
openshift-ci-robot
added
the
jira/valid-reference
|
/approve |
openshift-ci
bot
added
the
approved
…input resources are required to run the operator and the payloadConfigController
p0lyn0mial
force-pushed
the
integration-test-oauth-server-payload-controller-new
branch
from
5f8e9f6 to
1614820
Compare
|
/verified by @p0lyn0mial |
openshift-ci-robot
added
the
verified
|
@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: 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. |
|
/lgtm |
|
[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.
Actionable comments posted: 1
📜 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 (16)
pkg/cmd/mom/output_resources_command.go(1 hunks)test-data/apply-configuration/overall/minimal-cluster/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/2e46-metadata-authentication-operator.17fe72c59b829800.b2cdb588.yaml(1 hunks)test-data/apply-configuration/overall/minimal-cluster/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/86b1-metadata-authentication-operator.17fe72c59b829800.5aa4f723.yaml(1 hunks)test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/3ed4-body-system-COLON-openshift-COLON-openshift-authenticator-.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/c450-body-authentication-operator.18599d2230299800.18b41977.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/c450-metadata-authentication-operator.18599d2230299800.18b41977.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication/core/configmaps/82b1-body-v4-0-config-system-cliconfig.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication/core/configmaps/82b1-metadata-v4-0-config-system-cliconfig.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/controller-results.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/config.openshift.io/authentications.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/config.openshift.io/clusterversions.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/core/secrets.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/core/services/oauth-openshift.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/route.openshift.io/routes.yaml(1 hunks)test-data/apply-configuration/overall/oauth-server-payloadcontroller/test.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/core/secrets.yaml
🚧 Files skipped from review as they are similar to previous changes (8)
- test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/c450-body-authentication-operator.18599d2230299800.18b41977.yaml
- test-data/apply-configuration/overall/oauth-server-payloadcontroller/test.yaml
- test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/config.openshift.io/clusterversions.yaml
- test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/config.openshift.io/authentications.yaml
- test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication/core/configmaps/82b1-metadata-v4-0-config-system-cliconfig.yaml
- test-data/apply-configuration/overall/oauth-server-payloadcontroller/expected-output/Management/Create/namespaces/openshift-authentication/core/configmaps/82b1-body-v4-0-config-system-cliconfig.yaml
- test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/namespaces/openshift-authentication/route.openshift.io/routes.yaml
- pkg/cmd/mom/output_resources_command.go
🔇 Additional comments (1)
test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/3ed4-body-system-COLON-openshift-COLON-openshift-authenticator-.yaml (1)
9-9: CSR payload update looks validThe new base64 decodes to a properly wrapped PEM CSR (
-----BEGIN CERTIFICATE REQUEST-----…-----END CERTIFICATE REQUEST-----), so the fixture remains well-formed.
| volumesToMount: | ||
| identityProviders: '{}' |
There was a problem hiding this comment.
Fix identityProviders to serialize as an object, not a string.
With the single quotes, this serializes to the literal string "{}" inside spec.observedConfig. Downstream consumers expect identityProviders to be an empty object (so they can treat it as a map); handing them a string can break JSON decoding and regress reconciliation. Drop the quotes so it stays an empty object.
- identityProviders: '{}'
+ identityProviders: {}📝 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.
| volumesToMount: | |
| identityProviders: '{}' | |
| volumesToMount: | |
| identityProviders: {} |
🤖 Prompt for AI Agents
In
test-data/apply-configuration/overall/oauth-server-payloadcontroller/input-dir/cluster-scoped-resources/operator.openshift.io/authentications/cluster.yaml
around lines 856-857, identityProviders is set to the quoted string '{}' which
serializes as a string; change it to an unquoted empty mapping so it becomes an
object (identityProviders: {}), ensuring spec.observedConfig contains an empty
object rather than the literal string.
|
/retest-required |
|
/hold Revision 1614820 was retested 3 times: holding |
openshift-ci
bot
added
the
do-not-merge/hold
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/retest-required |
1 similar comment
|
/retest-required |
|
@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. |
5 participants
Uh oh!
There was an error while loading. Please reload this page.