CORS-4516: Support setting universe domain in client options#131
CORS-4516: Support setting universe domain in client options#131patrickdillon wants to merge 4 commits into
Conversation
Adds support for non-standard universe domains, such as Google Cloud Dedicated's sovereign clouds. Custom token sources specified in the cloud config are still preferred to maintain consistent behavior. Otherwise, FindDefaultCredentials will discover creds based on the priority defined in the SDK. The WithCredentialsJSON function is preferred as it uses a self-signed JWT--not oauth token exchange, which may fail with custom universe domains.
This commit changes the default token source behavior, so that when no token source is set the credentials are discovered based on the priorities of FindDefaultCredentials. For example, prior to this commit, specifying the env var GOOGLE_APPLICATION_CREDENTIALS pointing to a service account key file would be ignored, unless token-uri is explicitly set to "nil" and metadata-service-based authentication would be preferred. Now, GOOGLE_APPLICATION_CREDENTIAlS will be checked, along with other methods, ultimately falling back to metadata-based authentication.
Pull in most modern authentication packages.
go mod tidy && go work vendor
|
@patrickdillon: This pull request references CORS-4516 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 "5.0.0" version, but no target version was set. DetailsIn 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (524)
📒 Files selected for processing (3)
WalkthroughGCE cloud configuration now defers token source selection and centralizes Google client option creation. Default credentials can be discovered and configured from credential JSON, while Compute and Container clients reuse the resulting options. Related dependency versions and tests were updated. ChangesGCE authentication client options
Estimated code review effort: 3 (Moderate) | ~20 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@patrickdillon, |
|
/testwith openshift/installer/main/e2e-gcd-ovn-private-techpreview openshift/gcp-pd-csi-driver#126 openshift/cluster-image-registry-operator#1356 openshift/machine-api-operator#1523 openshift/cluster-ingress-operator#1515 openshift/cloud-network-config-controller#249 openshift/machine-api-provider-gcp#180 openshift/image-registry#474 openshift/cloud-credential-operator#1068 openshift/cluster-cloud-controller-manager-operator#493 openshift/installer#10696 Including arbitrary installer pr to access the test job... |
|
/testwith openshift/installer/main/e2e-gcd-ovn-private-techpreview openshift/gcp-pd-csi-driver#126 openshift/cluster-image-registry-operator#1356 openshift/machine-api-operator#1523 openshift/cluster-ingress-operator#1515 openshift/cloud-network-config-controller#249 openshift/machine-api-provider-gcp#180 openshift/image-registry#474 openshift/cloud-credential-operator#1068 openshift/cluster-cloud-controller-manager-operator#493 openshift/installer#10687 Seems like there was some kind of merge conflict |
1 similar comment
|
attempting more targeted multi-pr run here: openshift/cloud-credential-operator#1068 (comment) |
2 similar comments
|
/retest As this changes the default credentials to use those mounted in the pod, and the current credsrequests are inadequate without openshift/cluster-cloud-controller-manager-operator#493, we should expect e2e-gcp to fail |
|
@patrickdillon: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
The earlier multi-pr testwith run verifies the behavior works correctly. Master nodes joined the cluster. the cluster was unable to pull the capi-controllers image. /testwith openshift/installer/main/e2e-gcd-ovn-private-techpreview openshift/gcp-pd-csi-driver#126 openshift/cluster-image-registry-operator#1356 openshift/machine-api-operator#1523 openshift/cluster-ingress-operator#1515 openshift/cloud-network-config-controller#249 openshift/machine-api-provider-gcp#180 openshift/image-registry#474 openshift/cloud-credential-operator#1068 openshift/cluster-cloud-controller-manager-operator#493 openshift/installer#10706 |
1 similar comment
|
hm, it seems that if you repeatedly run /testwith commands that you may lose the history. I wanted to check the details of the most recent run, not the active run, and I can't find it |
Upstream pull request kubernetes#1250 & kubernetes#1261
See those descriptions and openshift/enhancements#1977 for more details
This includes a change in the default credentials, so we would actually start using the mounted credentials. Those credentials are inadequate, so I would expect e2e-gcp to fail without openshift/cluster-cloud-controller-manager-operator#493
Summary by CodeRabbit
New Features
Bug Fixes
Chores