CORS-4516: Support setting universe domain in client options - #131
Conversation
|
@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. |
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 Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@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 test 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 |
|
And now for vanilla gcp, this PR changes the default so our mounted credentials will actually get used, and the credentialsrequest for GCP does not have the required permissions, so let's test it together with the permissions added in openshift/cluster-cloud-controller-manager-operator#493 /testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-gcp-ovn openshift/cluster-cloud-controller-manager-operator#493 |
2 similar comments
6 similar comments
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.
|
Rebased and dropped the upstream kubernetes#1261 part from this PR, as that PR is still open and this is the critical functionality. We can set the cloud provider to use the JSON creds through the cloud config |
|
/retest |
|
/testwith openshift/installer/main/e2e-gcd-ovn-private-techpreview openshift/gcp-pd-csi-driver#126 openshift/cluster-image-registry-operator#1356 openshift/cloud-network-config-controller#249 openshift/machine-api-provider-gcp#180 openshift/image-registry#474 openshift/cluster-cloud-controller-manager-operator#493 openshift/installer#10733 openshift/installer#10731 |
|
/verified by e2e and multi-pr test |
|
@patrickdillon: This PR has been marked as verified by 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. |
|
@patrickdillon: This pull request references CORS-4516 which is a valid jira issue. 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: theobarberbany The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
51c3264
into
openshift:main
Upstream pull request kubernetes#1250
& kubernetes#1261See that description 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#493UPDATE: Originally this PR included upstream 1261, but since that PR has not merged, it is removed from here
Summary by CodeRabbit
New Features
Bug Fixes
Chores