Skip to content

Conversation

@liouk
Copy link
Member

@liouk liouk commented Sep 23, 2025

In order to be able to run openshift's conformance e2e test suite on a cluster with external OIDC configured, we must skip any tests that are inherently irrelevant to OIDC (for example, tests against the OAuth APIs must be skipped, as these APIs do not exist in a cluster with external OIDC).

However, there's a number of tests that we want to avoid skipping (e.g. checking apiserver availability); this PR makes adaptations to such tests that currently break when OIDC is configured but we don't want to skip completely.

Summary of changes

  • authorization_rbac_proxy: when the users API is not present, the oc user created in this test comes from client.go; the order of the groups is different than the one the test expects, so we must make the test check expect both orders
  • apiserver-external-availability monitor test: this test checks all API servers, including the oauth apiserver; we adapt this test to skip the oauth apiserver when OIDC is configured (as it does not exist)
  • management_plane_operators: when OIDC is configured, the authentication operator does not have some conditions that are listed as always required in this test; this PR introduces a mechanism to determine some cluster-runtime conditions depending on cluster config/state and moves the respective auth operator ones there

Example failed run of conformance suite with OIDC configured: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_release/66981/rehearse-66981-periodic-ci-openshift-cluster-authentication-operator-release-4.21-periodics-e2e-aws-external-oidc-conformance-parallel-techpreview/1970076671268622336

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 23, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Sep 23, 2025

@liouk: This pull request references CNTRLPLANE-947 which is a valid jira issue.

In response to this:

This PR makes e2e test adaptations for the case of a cluster with external OIDC authentication configured. These are tests we do not want to skip completely.

Summary of changes

  • authorization_rbac_proxy: when the users API is not present, the oc user created in this test comes from client.go; the order of the groups is different than the one the test expects, so we must make the test check more flexible.
  • apiserver-external-availability monitor test: this test checks all API servers, including the oauth apiserver; we adapt this test to skip the oauth apiserver when OIDC is configured (as it does not exist)
  • management_plane_operators: when OIDC is configured, the authentication operator does not have some conditions that are listed as always required in this test; this PR introduces a mechanism to determine some cluster-runtime conditions depending on cluster config/state and moves the respective auth operator ones there

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 openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 23, 2025
@liouk liouk force-pushed the e2e-oidc-adaptations branch 2 times, most recently from f2f53d9 to c109961 Compare September 23, 2025 09:10
@openshift-ci-robot
Copy link

openshift-ci-robot commented Sep 23, 2025

@liouk: This pull request references CNTRLPLANE-947 which is a valid jira issue.

In response to this:

This PR makes e2e test adaptations for the case of a cluster with external OIDC authentication configured. These are tests we do not want to skip completely.

Summary of changes

  • authorization_rbac_proxy: when the users API is not present, the oc user created in this test comes from client.go; the order of the groups is different than the one the test expects, so we must make the test check expect both orders
  • apiserver-external-availability monitor test: this test checks all API servers, including the oauth apiserver; we adapt this test to skip the oauth apiserver when OIDC is configured (as it does not exist)
  • management_plane_operators: when OIDC is configured, the authentication operator does not have some conditions that are listed as always required in this test; this PR introduces a mechanism to determine some cluster-runtime conditions depending on cluster config/state and moves the respective auth operator ones there

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.

@liouk liouk changed the title WIP: CNTRLPLANE-947: E2E test adaptations for OIDC CNTRLPLANE-947: E2E test adaptations for OIDC Sep 23, 2025
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 23, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Sep 23, 2025

@liouk: This pull request references CNTRLPLANE-947 which is a valid jira issue.

In response to this:

This PR makes e2e test adaptations for the case of a cluster with external OIDC authentication configured. These are tests we do not want to skip completely.

Summary of changes

  • authorization_rbac_proxy: when the users API is not present, the oc user created in this test comes from client.go; the order of the groups is different than the one the test expects, so we must make the test check expect both orders
  • apiserver-external-availability monitor test: this test checks all API servers, including the oauth apiserver; we adapt this test to skip the oauth apiserver when OIDC is configured (as it does not exist)
  • management_plane_operators: when OIDC is configured, the authentication operator does not have some conditions that are listed as always required in this test; this PR introduces a mechanism to determine some cluster-runtime conditions depending on cluster config/state and moves the respective auth operator ones there

Example failed run of conformance suite with OIDC configured: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_release/66981/rehearse-66981-periodic-ci-openshift-cluster-authentication-operator-release-4.21-periodics-e2e-aws-external-oidc-conformance-parallel-techpreview/1970076671268622336

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.

@liouk liouk changed the title CNTRLPLANE-947: E2E test adaptations for OIDC WIP: CNTRLPLANE-947: E2E test adaptations for OIDC Sep 23, 2025
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 23, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Sep 30, 2025

@liouk: This pull request references CNTRLPLANE-947 which is a valid jira issue.

In response to this:

In order to be able to run openshift's conformance e2e test suite on a cluster with external OIDC configured, we must skip any tests that are inherently irrelevant to OIDC (for example, tests against the OAuth APIs must be skipped, as these APIs do not exist in a cluster with external OIDC).

However, there's a number of tests that we want to avoid skipping (e.g. checking apiserver availability); this PR makes adaptations to such tests that currently break when OIDC is configured but we don't want to skip completely.

Summary of changes

  • authorization_rbac_proxy: when the users API is not present, the oc user created in this test comes from client.go; the order of the groups is different than the one the test expects, so we must make the test check expect both orders
  • apiserver-external-availability monitor test: this test checks all API servers, including the oauth apiserver; we adapt this test to skip the oauth apiserver when OIDC is configured (as it does not exist)
  • management_plane_operators: when OIDC is configured, the authentication operator does not have some conditions that are listed as always required in this test; this PR introduces a mechanism to determine some cluster-runtime conditions depending on cluster config/state and moves the respective auth operator ones there

Example failed run of conformance suite with OIDC configured: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_release/66981/rehearse-66981-periodic-ci-openshift-cluster-authentication-operator-release-4.21-periodics-e2e-aws-external-oidc-conformance-parallel-techpreview/1970076671268622336

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.

@liouk
Copy link
Member Author

liouk commented Oct 1, 2025

/retest-required

@sdodson
Copy link
Member

sdodson commented Oct 1, 2025

@liouk What's the minimal set of required tests you'd like to see pass here before this merges. I don't want to get stuck in a retest quagmire.

@liouk
Copy link
Member Author

liouk commented Oct 2, 2025

@sdodson when it comes to making sure that the changes in this PR do not break existing tests, I've already seen enough successful runs of the tests changed in the jobs that have already run successfully.

From the current failing jobs, the ones that contain the updated tests are the following:

  • e2e-aws-ovn
  • e2e-aws-ovn-single-node-upgrade
  • e2e-aws-proxy
  • e2e-openstack-ovn

However these aren't required anyway, so I doubt we should block this PR until these succeed.

Apart from verifying we're not breaking any existing tests, I would like to see the results of the jobs introduced with openshift/release#66981 and I'm planning on running the conformance suites at a local cluster -- however I am not aware of any way to run those jobs on the CI and include the changes of this PR before it merges (let me know if there's a way!).

@liouk liouk changed the title WIP: CNTRLPLANE-947: E2E test adaptations for OIDC CNTRLPLANE-947: E2E test adaptations for OIDC Oct 2, 2025
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 2, 2025
@sdodson
Copy link
Member

sdodson commented Oct 6, 2025

/approve

@liouk
Copy link
Member Author

liouk commented Oct 23, 2025

/retest-required

@liouk
Copy link
Member Author

liouk commented Nov 26, 2025

/retest

@openshift-trt
Copy link

openshift-trt bot commented Nov 26, 2025

Job Failure Risk Analysis for sha: c109961

Job Name Failure Risk
pull-ci-openshift-origin-main-e2e-aws-disruptive High
Job run should complete before timeout
This test has passed 98.08% of 5312 runs on release 4.21 [Overall] in the last week.

Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

These changes seem reasonable to me.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 1, 2025
}
escalationFormat := `%s %q is forbidden: user %q (groups=["system:authenticated:oauth" "system:authenticated"]) is attempting to grant RBAC permissions not currently held:`
// the two groups appear in different order in different cases
escalationFormatRegex := `^%s %q is forbidden: user %q \(groups=\[("system:authenticated" "system:authenticated:oauth"|"system:authenticated:oauth" "system:authenticated")\]\) is attempting to grant RBAC permissions not currently held:`
Copy link
Member

Choose a reason for hiding this comment

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

nit: another way of doing this would be to extract the groups from the string into a slice and compare. This works, though!

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good idea, makes it more future-proof 👍 Fixed in latest push.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 2, 2025
liouk added 3 commits December 3, 2025 10:43
…is not configured

For example, when external OIDC authentication is configured.
…untime

Also define what conditions to expect for the authentication operator depending on
configured auth type.
@liouk liouk force-pushed the e2e-oidc-adaptations branch from c109961 to 63b9922 Compare December 3, 2025 09:43
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 3, 2025
@openshift-ci-robot
Copy link

Scheduling required tests:
/test e2e-aws-csi
/test e2e-aws-ovn-fips
/test e2e-aws-ovn-microshift
/test e2e-aws-ovn-microshift-serial
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-gcp-csi
/test e2e-gcp-ovn
/test e2e-gcp-ovn-upgrade
/test e2e-metal-ipi-ovn-ipv6
/test e2e-vsphere-ovn
/test e2e-vsphere-ovn-upi

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 3, 2025

@liouk: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-scos-e2e-aws-ovn c109961 link false /test okd-scos-e2e-aws-ovn
ci/prow/e2e-aws-ovn-single-node-serial c109961 link false /test e2e-aws-ovn-single-node-serial
ci/prow/e2e-aws-disruptive c109961 link false /test e2e-aws-disruptive
ci/prow/e2e-aws-ovn-cgroupsv2 c109961 link false /test e2e-aws-ovn-cgroupsv2
ci/prow/e2e-openstack-ovn c109961 link false /test e2e-openstack-ovn
ci/prow/e2e-aws-ovn-fips 63b9922 link true /test e2e-aws-ovn-fips
ci/prow/e2e-aws-ovn-serial-2of2 63b9922 link true /test e2e-aws-ovn-serial-2of2
ci/prow/e2e-metal-ipi-ovn-ipv6 63b9922 link true /test e2e-metal-ipi-ovn-ipv6

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.

Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 3, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 3, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bertinatto, everettraven, liouk, sdodson

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants