Skip to content

Conversation

@devguyio
Copy link
Contributor

@devguyio devguyio commented Oct 7, 2025

Note

This PR is stacked on top of #1052

Summary

Refactors console-operator authentication controllers to improve separation of concerns and eliminate duplicate resource management by splitting controllers by authentication type.

Fixes CONSOLE-4822

Problem

  • oauthClientSecretController handled both IntegratedOAuth AND OIDC (despite OAuth-only name)
  • oidcSetupController validated OIDC secret but didn't sync it

Solution

Split Controllers by Auth Type

integratedOAuthController (pkg/console/controllers/integratedoauth/)

  • Handles only IntegratedOAuth and None authentication
  • Generates random client secrets

oidcController (pkg/console/controllers/oidc/)

  • Handles all OIDC authentication resources
  • NEW: Syncs OIDC client secret from openshift-config to openshift-console/console-oauth-config
  • Syncs CA configmaps and updates Authentication CR status

Use Constants

Replaced hardcoded "console-oauth-config" strings with deploymentsub.ConsoleOauthConfigName constant.

Benefits

  • Clear separation of concerns by authentication type
  • Eliminates duplicate OIDC client secret fetching
  • Controller names accurately reflect responsibilities
  • Single source of truth for secret names
  • Easier maintenance and debugging

devguyio and others added 3 commits October 7, 2025 21:08
This commit addresses two issues related to OIDC authentication:

1. Fixed OIDC client secret lookup in oidcsetup controller to use
   the correct informer, namespace (openshift-config) and dynamic
   secret name from the Authentication CR, instead of hardcoded values.

2. Added condition cleanup in sync_v400 to properly clear the
   OIDCProviderTrustedAuthorityConfigGet degraded condition when
   authentication type changes from OIDC to non-OIDC (e.g.,
   IntegratedOAuth). This prevents the Console Operator
   from remaining in a Degraded state indefinitely during rollback
   scenarios.

The second fix follows the same pattern used in the oidcsetup
controller for clearing conditions when auth type is not OIDC.

Assisted-by: Claude Code 2.0.5, claude-sonnet-4-5@20250929
Signed-off-by: Ahmed Abdalla <[email protected]>
Split oauthClientSecretController and oidcSetupController into two
clearly-separated controllers based on authentication type to improve
code organization and eliminate duplicate resource management.

Changes:
- Renamed oauthClientSecretController → integratedOAuthController
  - Handles only IntegratedOAuth and None authentication types
  - Removed OIDC client secret fetching logic
  - Location: pkg/console/controllers/integratedoauth/

- Renamed oidcSetupController → oidcController
  - Handles all OIDC authentication resources
  - Added OIDC client secret syncing from openshift-config to
    openshift-console/console-oauth-config
  - Now owns complete OIDC resource lifecycle
  - Location: pkg/console/controllers/oidc/

- Updated pkg/console/starter/starter.go with new controller names

Benefits:
- Clear separation of concerns by authentication type
- Eliminates duplicate OIDC client secret fetching
- Controller names accurately reflect their responsibilities
- Easier maintenance and debugging

🤖 Generated with Claude Code 2.0.5, claude-sonnet-4-5@20250929

Co-Authored-By: Claude <[email protected]>
Replace all hardcoded "console-oauth-config" strings with the existing
constant deploymentsub.ConsoleOauthConfigName for consistency and
maintainability.

Changes:
- pkg/console/controllers/oidc/oidc.go
  - Added deploymentsub import
  - Use constant for secret name lookup

- pkg/console/controllers/integratedoauth/integratedoauth.go
  - Added deploymentsub import
  - Use constant in informer filter, sync(), and syncSecret()

- pkg/console/controllers/oauthclients/oauthclients.go
  - Added deploymentsub import
  - Use constant for secret lookup

Benefits:
- Single source of truth for the secret name
- Easier refactoring if name needs to change
- Consistent with existing codebase patterns

🤖 Generated with Claude Code 2.0.9, claude-sonnet-4-5@20250929

Co-Authored-By: Claude <[email protected]>
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 7, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 7, 2025

@devguyio: This pull request references CONSOLE-4822 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 story to target the "4.21.0" version, but no target version was set.

In response to this:

Summary

Refactors console-operator authentication controllers to improve separation of concerns and eliminate duplicate resource management by splitting controllers by authentication type.

Fixes CONSOLE-4822

Problem

  • oauthClientSecretController handled both IntegratedOAuth AND OIDC (despite OAuth-only name)
  • oidcSetupController validated OIDC secret but didn't sync it

Solution

Split Controllers by Auth Type

integratedOAuthController (pkg/console/controllers/integratedoauth/)

  • Handles only IntegratedOAuth and None authentication
  • Generates random client secrets

oidcController (pkg/console/controllers/oidc/)

  • Handles all OIDC authentication resources
  • NEW: Syncs OIDC client secret from openshift-config to openshift-console/console-oauth-config
  • Syncs CA configmaps and updates Authentication CR status

Use Constants

Replaced hardcoded "console-oauth-config" strings with deploymentsub.ConsoleOauthConfigName constant.

Benefits

  • Clear separation of concerns by authentication type
  • Eliminates duplicate OIDC client secret fetching
  • Controller names accurately reflect responsibilities
  • Single source of truth for secret names
  • Easier maintenance and debugging

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.

1 similar comment
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 7, 2025

@devguyio: This pull request references CONSOLE-4822 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 story to target the "4.21.0" version, but no target version was set.

In response to this:

Summary

Refactors console-operator authentication controllers to improve separation of concerns and eliminate duplicate resource management by splitting controllers by authentication type.

Fixes CONSOLE-4822

Problem

  • oauthClientSecretController handled both IntegratedOAuth AND OIDC (despite OAuth-only name)
  • oidcSetupController validated OIDC secret but didn't sync it

Solution

Split Controllers by Auth Type

integratedOAuthController (pkg/console/controllers/integratedoauth/)

  • Handles only IntegratedOAuth and None authentication
  • Generates random client secrets

oidcController (pkg/console/controllers/oidc/)

  • Handles all OIDC authentication resources
  • NEW: Syncs OIDC client secret from openshift-config to openshift-console/console-oauth-config
  • Syncs CA configmaps and updates Authentication CR status

Use Constants

Replaced hardcoded "console-oauth-config" strings with deploymentsub.ConsoleOauthConfigName constant.

Benefits

  • Clear separation of concerns by authentication type
  • Eliminates duplicate OIDC client secret fetching
  • Controller names accurately reflect responsibilities
  • Single source of truth for secret names
  • Easier maintenance and debugging

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 requested review from TheRealJon and jhadvig October 7, 2025 21:17
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 7, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: devguyio
Once this PR has been reviewed and has the lgtm label, please assign spadgett for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 7, 2025

@devguyio: This pull request references CONSOLE-4822 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 story to target the "4.21.0" version, but no target version was set.

In response to this:

[!NOTE]
This PR is stacked on top of #1052

Summary

Refactors console-operator authentication controllers to improve separation of concerns and eliminate duplicate resource management by splitting controllers by authentication type.

Fixes CONSOLE-4822

Problem

  • oauthClientSecretController handled both IntegratedOAuth AND OIDC (despite OAuth-only name)
  • oidcSetupController validated OIDC secret but didn't sync it

Solution

Split Controllers by Auth Type

integratedOAuthController (pkg/console/controllers/integratedoauth/)

  • Handles only IntegratedOAuth and None authentication
  • Generates random client secrets

oidcController (pkg/console/controllers/oidc/)

  • Handles all OIDC authentication resources
  • NEW: Syncs OIDC client secret from openshift-config to openshift-console/console-oauth-config
  • Syncs CA configmaps and updates Authentication CR status

Use Constants

Replaced hardcoded "console-oauth-config" strings with deploymentsub.ConsoleOauthConfigName constant.

Benefits

  • Clear separation of concerns by authentication type
  • Eliminates duplicate OIDC client secret fetching
  • Controller names accurately reflect responsibilities
  • Single source of truth for secret names
  • Easier maintenance and debugging

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.

@devguyio
Copy link
Contributor Author

devguyio commented Oct 7, 2025

/hold
#1052 should be merged first.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 7, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 8, 2025

@devguyio: The following test 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 178b96e link false /test okd-scos-e2e-aws-ovn

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.

@devguyio devguyio marked this pull request as draft October 24, 2025 19:06
@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 Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants