Enable Authenticating with Azure with Certificates using Azure SDK for Go's Generic NewDefaultAzureCredential#1694
Enable Authenticating with Azure with Certificates using Azure SDK for Go's Generic NewDefaultAzureCredential#1694bryan-cox wants to merge 6 commits into
Conversation
|
[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 |
bryan-cox
force-pushed
the
HOSTEDCP-1994
branch
from
52974c9 to
3b9cdb4
Compare
Signed-off-by: Bryan Cox <brcox@redhat.com>
bryan-cox
force-pushed
the
HOSTEDCP-1994
branch
from
3b9cdb4 to
ed18676
Compare
bennerv
left a comment
bennerv
left a comment
There was a problem hiding this comment.
Also should capture AZURE_CLIENT_SEND_CERTIFICATE_CHAIN but you said there's be followup.
Otherwise lgtm
Signed-off-by: Bryan Cox <brcox@redhat.com>
Co-authored-by: Ben Vesel <10840174+bennerv@users.noreply.github.com>
flavianmissi
left a comment
flavianmissi
left a comment
There was a problem hiding this comment.
Left a question about the image registry (operand). Looks good otherwise!
| HyperShift would pass the following environment variables - AZURE_CLIENT_ID, AZURE_TENANT_ID, and | ||
| AZURE_CLIENT_CERTIFICATE_PATH - to its deployments of image registry, ingress, cloud network config, and storage | ||
| operators (azure-file and azure-disk) on the hosted control plane. Each of these components would then pass these | ||
| variables along to NewDefaultAzureCredential. |
There was a problem hiding this comment.
When configured with workload identity, the image registry operator will configure the image registry in a similar fashion to what's described in this EP (source).
Do we need to consider this when changing the operator code to use these env vars? In other words, do we also want the image registry (operand!) to authenticate using this proposed mechanism?
There was a problem hiding this comment.
Yeah probably so. On ARO HCP, on the HCP side image registry will authenticate with the cert method, while the operand would use workload identity.
Signed-off-by: Bryan Cox <brcox@redhat.com>
Signed-off-by: Bryan Cox <brcox@redhat.com>
bryan-cox
force-pushed
the
HOSTEDCP-1994
branch
from
3ba8d36 to
667e461
Compare
Signed-off-by: Bryan Cox <brcox@redhat.com>
|
@bryan-cox: 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. |
|
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
openshift-ci
Bot
added
the
lifecycle/stale
|
Stale enhancement proposals rot after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle rotten |
openshift-ci
Bot
added
lifecycle/rotten
|
Rotten enhancement proposals close after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Reopen the proposal by commenting /close |
|
@openshift-bot: Closed this PR. 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 kubernetes-sigs/prow repository. |
4 participants
This enhancement proposes enabling image registry, ingress, cloud network config, and storage operators(azure-file and azure-disk) to accept authenticating with Azure with certificates using Azure SDK for Go's generic function NewDefaultAzureCredential.