Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

feat: enable system-assigned identity by default #3856

Merged
merged 12 commits into from
Oct 13, 2020
Merged

feat: enable system-assigned identity by default #3856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
acc8aa9
feat: enable system-assigned identity by default
jackfrancis Sep 22, 2020
beb78e7
fix: don't change defaults for master vmss or customCloudProfile
jackfrancis Sep 22, 2020
c3d176b
don't change val during upgrade/scale
jackfrancis Sep 22, 2020
6b22340
docs and example updates
jackfrancis Oct 2, 2020
ed25e4c
typo
jackfrancis Oct 2, 2020
4424538
don't retry E2E if PrincipalNotFound
jackfrancis Oct 2, 2020
f4facf4
default to cli auth method, update docs
jackfrancis Oct 2, 2020
36e3244
fix UT
jackfrancis Oct 2, 2020
a92b91d
remove unnecessary client_secret auth examples
jackfrancis Oct 2, 2020
5c1332b
Update docs/tutorials/quickstart.md
jackfrancis Oct 13, 2020
f75f4bc
just remove this retry
jackfrancis Oct 13, 2020
2ca66f4
Merge branch 'system-assigned-identity-default' of https://github.com…
jackfrancis Oct 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,13 @@ func autofillApimodel(dc *deployCmd) error {
if dc.dnsPrefix == "" {
return errors.New("apimodel: missing masterProfile.dnsPrefix and --dns-prefix was not specified")
}
log.Warnf("apimodel: missing masterProfile.dnsPrefix will use %q", dc.dnsPrefix)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be a warning, it's literally a CLI feature

dc.containerService.Properties.MasterProfile.DNSPrefix = dc.dnsPrefix
}

if dc.autoSuffix {
suffix := strconv.FormatInt(time.Now().Unix(), 16)
dc.containerService.Properties.MasterProfile.DNSPrefix += "-" + suffix
log.Infof("Generated random suffix %s, DNS Prefix is %s", suffix, dc.containerService.Properties.MasterProfile.DNSPrefix)
}

if dc.outputDirectory == "" {
Expand Down Expand Up @@ -324,7 +324,7 @@ func autofillApimodel(dc *deployCmd) error {

k8sConfig := dc.containerService.Properties.OrchestratorProfile.KubernetesConfig

useManagedIdentity := k8sConfig != nil && k8sConfig.UseManagedIdentity
useManagedIdentity := k8sConfig != nil && to.Bool(k8sConfig.UseManagedIdentity)

if !useManagedIdentity {
spp := dc.containerService.Properties.ServicePrincipalProfile
Expand Down
3 changes: 2 additions & 1 deletion cmd/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/Azure/aks-engine/pkg/engine/transform"
"github.com/Azure/aks-engine/pkg/helpers"
"github.com/Azure/aks-engine/pkg/i18n"
"github.com/Azure/go-autorest/autorest/to"
"github.com/google/uuid"
"github.com/leonelquinteros/gotext"
"github.com/pkg/errors"
Expand Down Expand Up @@ -191,7 +192,7 @@ func (gc *generateCmd) loadAPIModel() error {
func (gc *generateCmd) autofillApimodel() error {
// set the client id and client secret by command flags
k8sConfig := gc.containerService.Properties.OrchestratorProfile.KubernetesConfig
useManagedIdentity := k8sConfig != nil && k8sConfig.UseManagedIdentity
useManagedIdentity := k8sConfig != nil && to.Bool(k8sConfig.UseManagedIdentity)
if !useManagedIdentity {
if (gc.containerService.Properties.ServicePrincipalProfile == nil || ((gc.containerService.Properties.ServicePrincipalProfile.ClientID == "" || gc.containerService.Properties.ServicePrincipalProfile.ClientID == "00000000-0000-0000-0000-000000000000") && gc.containerService.Properties.ServicePrincipalProfile.Secret == "")) && gc.ClientID.String() != "" && gc.ClientSecret != "" {
gc.containerService.Properties.ServicePrincipalProfile = &api.ServicePrincipalProfile{
Expand Down
7 changes: 6 additions & 1 deletion cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ type authArgs struct {
func addAuthFlags(authArgs *authArgs, f *flag.FlagSet) {
f.StringVar(&authArgs.RawAzureEnvironment, "azure-env", "AzurePublicCloud", "the target Azure cloud")
f.StringVarP(&authArgs.rawSubscriptionID, "subscription-id", "s", "", "azure subscription id (required)")
f.StringVar(&authArgs.AuthMethod, "auth-method", "client_secret", "auth method (default:`client_secret`, `cli`, `client_certificate`, `device`)")
f.StringVar(&authArgs.AuthMethod, "auth-method", "cli", "auth method (default:`client_secret`, `cli`, `client_certificate`, `device`)")
f.StringVar(&authArgs.rawClientID, "client-id", "", "client id (used with --auth-method=[client_secret|client_certificate])")
f.StringVar(&authArgs.ClientSecret, "client-secret", "", "client secret (used with --auth-method=client_secret)")
f.StringVar(&authArgs.CertificatePath, "certificate-path", "", "path to client certificate (used with --auth-method=client_certificate)")
Expand All @@ -146,6 +146,11 @@ func (authArgs *authArgs) validateAuthArgs() error {
return errors.New("--auth-method is a required parameter")
}

// Back-compat to accommodate existing client usage patterns that assume that "client-secret" is the default
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jadarsie Do you have any concerns here for Azure Stack? Does the az CLI auth model work for Azure Stack? If not, are you comfortable with this back-compat solution that "re-sets" the default auth method to client_secret if the service principal ID and password are included in the command line arguments?

The goal for us is to default to CLI as the auth model because for most users it is easier (don't have to generate/maintain service principals, easier command statements). If there's a reason that a local az context isn't possible, then this back-compat solution should work for pre-existing scripts that pass in the id and pass but haven't ever bothered to set the --auth-method to client_secret explicitly, because it's always been the default.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @jadarsie @haofan-ms

if authArgs.AuthMethod == "cli" && authArgs.rawClientID != "" && authArgs.ClientSecret != "" {
authArgs.AuthMethod = "client_secret"
}

if authArgs.AuthMethod == "client_secret" || authArgs.AuthMethod == "client_certificate" {
authArgs.ClientID, err = uuid.Parse(authArgs.rawClientID)
if err != nil {
Expand Down
14 changes: 6 additions & 8 deletions docs/topics/addpool.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ To add a new pool to the cluster you will run a command like:
```sh
$ aks-engine addpool --subscription-id <subscription_id> \
--resource-group mycluster --location <location> \
--client-id '<service principal client ID>' \
--client-secret '<service principal client secret>' \
--api-model _output/mycluster/apimodel.json \
--node-pool ./pool.json
```
Expand Down Expand Up @@ -58,8 +56,8 @@ Some important considerations:
|--resource-group|yes|The resource group the cluster is deployed in.|
|--location|yes|The location the resource group is in.|
|--api-model|yes|Relative path to the generated API model for the cluster.|
|--client-id|depends| The Service Principal Client ID. This is required if the auth-method is set to service_principal/client_certificate|
|--client-secret|depends| The Service Principal Client secret. This is required if the auth-method is set to service_principal|
|--client-id|depends| The Service Principal Client ID. This is required if the auth-method is set to client_secret or client_certificate|
|--client-secret|depends| The Service Principal Client secret. This is required if the auth-method is set to client_secret|
|--certificate-path|depends| The path to the file which contains the client certificate. This is required if the auth-method is set to client_certificate|
|--node-pool|yes|Path to JSON file expressing the `agentPoolProfile` spec of the new node pool.|
|--auth-method|no|The authentication method used. Default value is `client_secret`. Other supported values are: `cli`, `client_certificate`, and `device`.|
Expand Down Expand Up @@ -133,15 +131,15 @@ $ grep orchestratorRelease -A 1 _output/kubernetes-westus2-1838/apimodel.json
We can now run addpool once per new pool to begin the process of validating v1.19.1 across our existing v1.18.8 cluster:

```sh
$ aks-engine addpool --subscription-id $TEST_AZURE_SUB_ID --api-model _output/kubernetes-westus2-1838/apimodel.json --node-pool newpool1.json --location westus2 --resource-group kubernetes-westus2-1838 --auth-method client_secret --client-id $TEST_AZURE_SP_ID --client-secret $TEST_AZURE_SP_PW
$ aks-engine addpool --subscription-id $TEST_AZURE_SUB_ID --api-model _output/kubernetes-westus2-1838/apimodel.json --node-pool newpool1.json --location westus2 --resource-group kubernetes-westus2-1838
WARN[0003] Any new nodes will have containerd version 1.3.7
INFO[0003] Starting ARM Deployment kubernetes-westus2-1838-1942811440 in resource group kubernetes-westus2-1838. This will take some time...
INFO[0158] Finished ARM Deployment (kubernetes-westus2-1838-1942811440). Succeeded
$ aks-engine addpool --subscription-id $TEST_AZURE_SUB_ID --api-model _output/kubernetes-westus2-1838/apimodel.json --node-pool newpool2.json --location westus2 --resource-group kubernetes-westus2-1838 --auth-method client_secret --client-id $TEST_AZURE_SP_ID --client-secret $TEST_AZURE_SP_PW
$ aks-engine addpool --subscription-id $TEST_AZURE_SUB_ID --api-model _output/kubernetes-westus2-1838/apimodel.json --node-pool newpool2.json --location westus2 --resource-group kubernetes-westus2-1838
WARN[0008] Any new nodes will have containerd version 1.3.7
INFO[0008] Starting ARM Deployment kubernetes-westus2-1838-25937475 in resource group kubernetes-westus2-1838. This will take some time...
INFO[0163] Finished ARM Deployment (kubernetes-westus2-1838-25937475). Succeeded
$ aks-engine addpool --subscription-id $TEST_AZURE_SUB_ID --api-model _output/kubernetes-westus2-1838/apimodel.json --node-pool newpool3.json --location westus2 --resource-group kubernetes-westus2-1838 --auth-method client_secret --client-id $TEST_AZURE_SP_ID --client-secret $TEST_AZURE_SP_PW
$ aks-engine addpool --subscription-id $TEST_AZURE_SUB_ID --api-model _output/kubernetes-westus2-1838/apimodel.json --node-pool newpool3.json --location westus2 --resource-group kubernetes-westus2-1838
WARN[0004] Any new nodes will have containerd version 1.3.7
INFO[0004] Starting ARM Deployment kubernetes-westus2-1838-1370618455 in resource group kubernetes-westus2-1838. This will take some time...
INFO[0174] Finished ARM Deployment (kubernetes-westus2-1838-1370618455). Succeeded
Expand Down Expand Up @@ -188,7 +186,7 @@ node/k8s-newpool3-26196714-vmss000000 tainted
Let's say we've validated the "pool1" replacement, which we've called "newpool1". Let's scale that pool out to match the original "pool1":

```sh
$ aks-engine scale --subscription-id $TEST_AZURE_SUB_ID --client-id $TEST_AZURE_SP_ID --client-secret $TEST_AZURE_SP_PW --api-model _output/kubernetes-westus2-1838/apimodel.json --location westus2 --resource-group kubernetes-westus2-1838 --apiserver kubernetes-westus2-1838.westus2.cloudapp.azure.com --node-pool newpool1 --new-node-count 3 --auth-method client_secret --identity-system azure_ad
$ aks-engine scale --api-model _output/kubernetes-westus2-1838/apimodel.json --location westus2 --resource-group kubernetes-westus2-1838 --apiserver kubernetes-westus2-1838.westus2.cloudapp.azure.com --node-pool newpool1 --new-node-count 3
INFO[0003] found VMSS k8s-newpool1-26196714-vmss in resource group kubernetes-westus2-1838 that correlates with node pool newpool1
WARN[0003] Any new nodes will have containerd version 1.3.7
INFO[0003] Removing singlePlacementGroup property from [variables('newpool1VMNamePrefix')]
Expand Down
8 changes: 3 additions & 5 deletions docs/topics/creating_new_clusters.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ $ aks-engine deploy --subscription-id $SUBSCRIPTION_ID \
--dns-prefix $CLUSTER_NAME \
--resource-group $RESOURCE_GROUP \
--location $LOCATION \
--api-model examples/kubernetes.json \
--client-id $SERVICE_PRINCIPAL_ID \
--client-secret $SERVICE_PRINCIPAL_PASSWORD
--api-model examples/kubernetes.json
```

`aks-engine deploy` is a long-running operation that creates Azure resources (e.g., Virtual Machine and/or Virtual Machine Scale Set [VMSS], Disk, Network Interface, Network Security Group, Public IP Address, Virtual Network, Load Balancer, and others) that will underly a Kubernetes cluster. All deployed VMs will be configured to run Kubernetes bootstrap scripts appropriate for the desired cluster configuration. The outcome of a successful `aks-engine deploy` operation is a fully operational Kubernetes cluster, ready for use immediately.
Expand All @@ -34,8 +32,8 @@ A more detailed walk-through of `aks-engine deploy` is in the [quickstart guide]
|--set|no|Set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2).|
|--ca-certificate-path|no|Path to the CA certificate to use for Kubernetes PKI assets.|
|--ca-private-key-path|no|Path to the CA private key to use for Kubernetes PKI assets.|
|--client-id|depends| The Service Principal Client ID. This is required if the auth-method is set to service_principal/client_certificate|
|--client-secret|depends| The Service Principal Client secret. This is required if the auth-method is set to service_principal|
|--client-id|depends| The Service Principal Client ID. This is required if the auth-method is set to client_secret or client_certificate|
|--client-secret|depends| The Service Principal Client secret. This is required if the auth-method is set to client_secret|
|--certificate-path|depends| The path to the file which contains the client certificate. This is required if the auth-method is set to client_certificate|
|--identity-system|no|Identity system (default is azure_ad)|
|--auth-method|no|The authentication method used. Default value is `client_secret`. Other supported values are: `cli`, `client_certificate`, and `device`.|
Expand Down
8 changes: 3 additions & 5 deletions docs/topics/scale.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ To scale the cluster you will run a command like:
```sh
$ aks-engine scale --subscription-id <subscription_id> \
--resource-group mycluster --location <location> \
--client-id '<service principal client ID>' \
--client-secret '<service principal client secret>' \
--api-model _output/mycluster/apimodel.json --new-node-count <desired node count> \
--node-pool agentpool1 --apiserver mycluster.<location>.cloudapp.azure.com
```
Expand All @@ -35,8 +33,8 @@ This command will re-use the `apimodel.json` file inside the output directory as
|--resource-group|yes|The resource group the cluster is deployed in.|
|--location|yes|The location the resource group is in.|
|--api-model|yes|Relative path to the generated API model for the cluster.|
|--client-id|depends| The Service Principal Client ID. This is required if the auth-method is set to service_principal/client_certificate|
|--client-secret|depends| The Service Principal Client secret. This is required if the auth-method is set to service_principal|
|--client-id|depends| The Service Principal Client ID. This is required if the auth-method is set to client_secret or client_certificate|
|--client-secret|depends| The Service Principal Client secret. This is required if the auth-method is set to client_secret|
|--certificate-path|depends| The path to the file which contains the client certificate. This is required if the auth-method is set to client_certificate|
|--node-pool|depends|Required if there is more than one node pool. Which node pool should be scaled.|
|--new-node-count|yes|Desired number of nodes in the node pool.|
Expand Down Expand Up @@ -185,7 +183,7 @@ $ grep orchestratorVersion _output/kubernetes-westus2-95121/apimodel.json
Now, let's try that scale operation again!

```sh
$ bin/aks-engine scale --subscription-id $AZURE_SUB_ID --client-id $AZURE_SP_ID --client-secret $AZURE_SP_PW --api-model _output/$RESOURCE_GROUP/apimodel.json --location westus2 --resource-group $RESOURCE_GROUP --apiserver $RESOURCE_GROUP.westus2.cloudapp.azure.com --node-pool agentpool1 --new-node-count 10 --auth-method client_secret --identity-system azure_ad
$ bin/aks-engine scale --api-model _output/$RESOURCE_GROUP/apimodel.json --location westus2 --resource-group $RESOURCE_GROUP --apiserver $RESOURCE_GROUP.westus2.cloudapp.azure.com --node-pool agentpool1 --new-node-count 10
INFO[0004] found VMSS k8s-agentpool1-10367588-vmss in resource group kubernetes-westus2-95121 that correlates with node pool agentpool1
WARN[0004] Any new nodes will have Moby version 19.03.12
WARN[0004] containerd will be upgraded to version 1.3.7
Expand Down
6 changes: 2 additions & 4 deletions docs/topics/update.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ To update the cluster you will run a command like:
```sh
$ aks-engine update --subscription-id <subscription_id> \
--resource-group mycluster --location <location> \
--client-id '<service principal client ID>' \
--client-secret '<service principal client secret>' \
--api-model _output/mycluster/apimodel.json \
--node-pool agentpool1
```
Expand All @@ -37,8 +35,8 @@ The above operation will complete rather quickly, as it is only updating the VMS
|--resource-group|yes|The resource group the cluster is deployed in.|
|--location|yes|The location the resource group is in.|
|--api-model|yes|Relative path to the generated API model for the cluster.|
|--client-id|depends| The Service Principal Client ID. This is required if the auth-method is set to service_principal/client_certificate|
|--client-secret|depends| The Service Principal Client secret. This is required if the auth-method is set to service_principal|
|--client-id|depends| The Service Principal Client ID. This is required if the auth-method is set to client_secret or client_certificate|
|--client-secret|depends| The Service Principal Client secret. This is required if the auth-method is set to client_secret|
|--certificate-path|depends| The path to the file which contains the client certificate. This is required if the auth-method is set to client_certificate|
|--node-pool|yes|Which node pool should be updated.|
|--auth-method|no|The authentication method used. Default value is `client_secret`. Other supported values are: `cli`, `client_certificate`, and `device`.|
Expand Down
18 changes: 5 additions & 13 deletions docs/topics/upgrade.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ In summary, using `aks-engine upgrade` means you will freshen and re-pave the en
|--subscription-id|yes|The subscription id the cluster is deployed in.|
|--resource-group|yes|The resource group the cluster is deployed in.|
|--location|yes|The location to deploy to.|\
|--client-id|depends| The Service Principal Client ID. This is required if the auth-method is set to service_principal/client_certificate|
|--client-secret|depends| The Service Principal Client secret. This is required if the auth-method is set to service_principal|
|--client-id|depends| The Service Principal Client ID. This is required if the auth-method is set to client_secret or client_certificate|
|--client-secret|depends| The Service Principal Client secret. This is required if the auth-method is set to client_secret|
|--certificate-path|depends| The path to the file which contains the client certificate. This is required if the auth-method is set to client_certificate|
|--identity-system|no|Identity system (default is azure_ad)|
|--auth-method|no|The authentication method used. Default value is `client_secret`. Other supported values are: `cli`, `client_certificate`, and `device`.|
Expand Down Expand Up @@ -106,10 +106,7 @@ Once you have read all the [requirements](#pre-requirements), run `aks-engine up
--api-model <generated apimodel.json> \
--location <resource group location> \
--resource-group <resource group name> \
--upgrade-version <desired Kubernetes version> \
--auth-method client_secret \
--client-id <service principal id> \
--client-secret <service principal secret>
--upgrade-version <desired Kubernetes version>
```

For example,
Expand All @@ -120,9 +117,7 @@ For example,
--api-model _output/mycluster/apimodel.json \
--location westus \
--resource-group test-upgrade \
--upgrade-version 1.8.7 \
--client-id xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx \
--client-secret xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
--upgrade-version 1.8.7
```

### Steps to run when using Key Vault for secrets
Expand All @@ -131,13 +126,10 @@ If you use Key Vault for secrets, you must specify a local [kubeconfig file](htt

```bash
./bin/aks-engine upgrade \
--subscription-id xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx \
--api-model _output/mycluster/apimodel.json \
--location westus \
--resource-group test-upgrade \
--upgrade-version 1.8.7 \
--client-id xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx \
--client-secret xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx \
--upgrade-version 1.18.7 \
--kubeconfig ./path/to/kubeconfig.json
```

Expand Down
Loading