WIP: Azure Stack store credentials in a secret #5046
Conversation
The AzureStack cloud provider config differs from that of public Azure. This provides the appropriate values for the following keys when using ASH: useManagedIdentityExtension should be false useInstanceMetadata should be false loadBalancerSku should be basic
This is a temporary addition to add the client credentials to the cloud provider config to support bootstrapping the kubelet with the legacy cloud provider. Once the Azure out-of-tree provider has been implemented we can utilize a merged cloud provider config similar to ARO so that the client credentials are saved in a secret rather than in plaintext on the nodes. From my reading of the legacy provider in the kubelet, the merged config is not supported when bootstrapping (hence the need for this commit). The call for bootstrapping is made here: https://github.com/openshift/kubernetes/blob/master/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go#L360 But that code never calls getConfigFromSecret, which creates the merged config: https://github.com/openshift/kubernetes/blob/master/staging/src/k8s.io/legacy-cloud-providers/azure/azure_config.go#L66 Instead, getConfigFromSecret is called from Initialize: https://github.com/openshift/kubernetes/blob/master/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go#L675 which appears to only be called from the kube-controller-manager: https://github.com/openshift/kubernetes/blob/master/cmd/kube-controller-manager/app/controllermanager.go#L601 This is not a problem for Public Azure because it uses managed identity, which is not supported in Azure Stack.
…fig" This reverts commit d210770. With the introduction of the CCM at bootstrap we can use a secret to store credentials and the CCM will create a merged CPC.
This change creates the resources needed to store client credentials in a secret and produce a merged cloud provider config.
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
[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: PR needs rebase. 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/test-infra repository. |
openshift-ci
bot
added
the
needs-rebase
|
@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/test-infra repository. I understand the commands that are listed here. |
|
/close |
|
@patrickdillon: 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/test-infra repository. |
1 participant
Depends on #5042
Also depends on the introduction of the cloud controller manager at bootstrap. Once the cloud controller manager has been introduced, we should be able to test this to see whether we can store credentials in a merged cloud provider config similar to ARO.