-
Notifications
You must be signed in to change notification settings - Fork 315
Fix IAP OAuth credentials not being fetched when IAP is disabled #2939
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?
Fix IAP OAuth credentials not being fetched when IAP is disabled #2939
Conversation
The behavior in validateIAP() of treating .spec.iap.enabled=false as equivalent to .spec.iap=nil and abort all validation is incongruent with that in pkg/backends/features/iap.go, where a full sync of the IAP configuration is triggered as long as .spec.iap!=nil. The latter behavior is the correct one, as it allows disabling IAP on an ingress where it was previously enabled by setting .spec.spec.enabled=false in the BackendConfig. However, because pkg/backendconfig/validation.go skips the entire validation and with that the lookup of the OAuth credentials referenced in .spec.iap.oauthclientCredentials.secretName, the desired BackendConfig's OAuth Client ID will be uninitialized and interpreted as empty (""). And in the case that IAP was previously configured with a non-default Client ID, this will trigger a switchingToDefaultError.
The committers listed above are authorized under a signed CLA. |
Welcome @sysedwinistrator! |
Hi @sysedwinistrator. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sysedwinistrator 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 |
The behavior in
validateIAP()
of treating.spec.iap.enabled=false
as equivalent to.spec.iap=nil
and abort all validation is incongruent with that inpkg/backends/features/iap.go
, where a full sync of the IAP configuration is triggered as long as.spec.iap!=nil
.The latter behavior is the correct one, as it allows disabling IAP on an ingress where it was previously enabled by setting
.spec.spec.enabled=false
in the BackendConfig.However, because
pkg/backendconfig/validation.go
skips the entire validation and with that the lookup of the OAuth credentials referenced in.spec.iap.oauthclientCredentials.secretName
, the desired BackendConfig's OAuth Client ID will be uninitialized and interpreted as empty (""
). And in the case that IAP was previously configured with a non-default Client ID, this will trigger aswitchingToDefaultError
.TL;DR: This allows disabling IAP on BackendConfig when a non-default OAuth Client has been configured before. It's still necessary to keep the OAuth client credentials in the BackendConfig while IAP is being disabled.