chore: remove support for CLI & DEVICE auth methods (#244)

Makefile

@@ -133,7 +133,7 @@ ginkgoBuild: generate
 	make -C ./test/e2e ginkgo-build
 
 test: generate
-	ginkgo -mod=vendor -junit-report test/junit/junit.xml -skip-package test/e2e -fail-fast -r -v -tags=fast .
+	ginkgo -mod=vendor -junit-report junit.xml -skip-package test/e2e -fail-fast -r -v -tags=fast .
 
 .PHONY: test-style
 test-style: validate-go validate-shell validate-copyright-headers

cmd/deploy.go

@@ -28,7 +28,6 @@ import (
 	"github.com/Azure/aks-engine-azurestack/pkg/engine/transform"
 	"github.com/Azure/aks-engine-azurestack/pkg/helpers"
 	"github.com/Azure/aks-engine-azurestack/pkg/i18n"
-	"github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac"
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/pkg/errors"
 )
@@ -348,33 +347,7 @@ func autofillApimodel(dc *deployCmd) error {
 	if !useManagedIdentity {
 		spp := dc.containerService.Properties.ServicePrincipalProfile
 		if spp != nil && spp.ClientID == "" && spp.Secret == "" && spp.KeyvaultSecretRef == nil && (dc.getAuthArgs().ClientID.String() == "" || dc.getAuthArgs().ClientID.String() == "00000000-0000-0000-0000-000000000000") && dc.getAuthArgs().ClientSecret == "" {
-			log.Warnln("apimodel: ServicePrincipalProfile was missing or empty, creating application...")
-
-			// TODO: consider caching the creds here so they persist between subsequent runs of 'deploy'
-			appName := dc.containerService.Properties.MasterProfile.DNSPrefix
-			appURL := fmt.Sprintf("https://%s/", appName)
-			var replyURLs *[]string
-			var requiredResourceAccess *[]graphrbac.RequiredResourceAccess
-			applicationResp, servicePrincipalObjectID, secret, createErr := dc.client.CreateApp(ctx, appName, appURL, replyURLs, requiredResourceAccess)
-			if createErr != nil {
-				return errors.Wrap(createErr, "apimodel invalid: ServicePrincipalProfile was empty, and we failed to create valid credentials")
-			}
-			applicationID := to.String(applicationResp.AppID)
-			log.Warnf("created application with applicationID (%s) and servicePrincipalObjectID (%s).", applicationID, servicePrincipalObjectID)
-
-			log.Warnln("apimodel: ServicePrincipalProfile was empty, assigning role to application...")
-
-			err = dc.client.CreateRoleAssignmentSimple(ctx, dc.resourceGroup, servicePrincipalObjectID)
-			if err != nil {
-				return errors.Wrap(err, "apimodel: could not create or assign ServicePrincipal")
-
-			}
-
-			dc.containerService.Properties.ServicePrincipalProfile = &api.ServicePrincipalProfile{
-				ClientID: applicationID,
-				Secret:   secret,
-				ObjectID: servicePrincipalObjectID,
-			}
+			log.Warnln("apimodel: ServicePrincipalProfile missing or empty...")
 		} else if (dc.containerService.Properties.ServicePrincipalProfile == nil || ((dc.containerService.Properties.ServicePrincipalProfile.ClientID == "" || dc.containerService.Properties.ServicePrincipalProfile.ClientID == "00000000-0000-0000-0000-000000000000") && dc.containerService.Properties.ServicePrincipalProfile.Secret == "")) && dc.getAuthArgs().ClientID.String() != "" && dc.getAuthArgs().ClientSecret != "" {
 			dc.containerService.Properties.ServicePrincipalProfile = &api.ServicePrincipalProfile{
 				ClientID: dc.getAuthArgs().ClientID.String(),

cmd/deploy_test.go

@@ -266,12 +266,6 @@ func TestValidate(t *testing.T) {
 	}
 }
 
-func TestAutofillApimodelWithoutManagedIdentityCreatesCreds(t *testing.T) {
-	t.Parallel()
-
-	testAutodeployCredentialHandling(t, false, "", "")
-}
-
 func TestAutofillApimodelWithManagedIdentitySkipsCreds(t *testing.T) {
 	t.Parallel()
 
@@ -480,57 +474,6 @@ func TestAPIModelWithoutServicePrincipalProfileAndWithoutClientIdAndSecretInCmd(
 
 }
 
-func TestAPIModelWithEmptyServicePrincipalProfileAndWithoutClientIdAndSecretInCmd(t *testing.T) {
-	t.Parallel()
-
-	apiloader := &api.Apiloader{
-		Translator: nil,
-	}
-
-	apimodel := getAPIModel(ExampleAPIModelWithDNSPrefix, false, "", "")
-
-	cs, ver, err := apiloader.DeserializeContainerService([]byte(apimodel), false, false, nil)
-	if err != nil {
-		t.Fatalf("unexpected error deserializing the example apimodel: %s", err)
-	}
-
-	outDir, del := makeTmpDir(t)
-	defer del()
-
-	deployCmd := &deployCmd{
-		apimodelPath:     "./this/is/unused.json",
-		outputDirectory:  outDir,
-		forceOverwrite:   true,
-		location:         "westus",
-		containerService: cs,
-		apiVersion:       ver,
-
-		client: &armhelpers.MockAKSEngineClient{},
-		authProvider: &mockAuthProvider{
-			authArgs: &authArgs{},
-		},
-	}
-	err = autofillApimodel(deployCmd)
-	if err != nil {
-		t.Fatalf("unexpected error autofilling the example apimodel: %s", err)
-	}
-
-	if deployCmd.containerService.Properties.ServicePrincipalProfile == nil {
-		t.Fatalf("expected service principal profile to be Empty and not nil for unmanaged identity, where client id and secret are not supplied in api model and deployment command")
-	}
-
-	// mockclient returns "app-id" for ClientID when empty
-	if deployCmd.containerService.Properties.ServicePrincipalProfile.ClientID != "app-id" {
-		t.Fatalf("expected service principal profile client id to be empty but got %s", deployCmd.containerService.Properties.ServicePrincipalProfile.ClientID)
-	}
-
-	// mockcliet returns "client-secret" when empty
-	if deployCmd.containerService.Properties.ServicePrincipalProfile.Secret != "client-secret" {
-		t.Fatalf("expected service principal profile client secret to be empty but got %s", deployCmd.containerService.Properties.ServicePrincipalProfile.Secret)
-	}
-
-}
-
 func testAutodeployCredentialHandling(t *testing.T, useManagedIdentity bool, clientID, clientSecret string) {
 	apiloader := &api.Apiloader{
 		Translator: nil,

cmd/root.go

@@ -122,7 +122,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", "cli", "auth method (default:`client_secret`, `cli`, `client_certificate`, `device`)")
+	f.StringVar(&authArgs.AuthMethod, "auth-method", "client_secret", "auth method (default:`client_secret`, `client_certificate`)")
 	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)")
@@ -146,11 +146,6 @@ 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
-	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 {
@@ -237,10 +232,6 @@ func (authArgs *authArgs) getAzureClient() (armhelpers.AKSEngineClient, error) {
 		return nil, err
 	}
 	switch authArgs.AuthMethod {
-	case "cli":
-		client, err = armhelpers.NewAzureClientWithCLI(env, authArgs.SubscriptionID.String())
-	case "device":
-		client, err = armhelpers.NewAzureClientWithDeviceAuth(env, authArgs.SubscriptionID.String())
 	case "client_secret":
 		client, err = armhelpers.NewAzureClientWithClientSecret(env, authArgs.SubscriptionID.String(), authArgs.ClientID.String(), authArgs.ClientSecret)
 	case "client_certificate":

docs/topics/addpool.md

@@ -60,7 +60,7 @@ Some important considerations:
 |--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`.|
+|--auth-method|no|The authentication method used. Default value is `client_secret`. Other supported values are: `client_certificate`.|
 |--language|no|Language to return error message in. Default value is "en-us").|
 
 ## Frequently Asked Questions

docs/topics/creating_new_clusters.md

@@ -36,7 +36,7 @@ A more detailed walk-through of `aks-engine-azurestack deploy` is in the [quicks
 |--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`.|
+|--auth-method|no|The authentication method used. Default value is `client_secret`. Other supported values are: `client_certificate`.|
 |--private-key-path|no|Path to private key (used with --auth-method=client_certificate).|
 |--language|no|Language to return error message in. Default value is "en-us").|
 

docs/topics/scale.md

@@ -39,7 +39,7 @@ This command will re-use the `apimodel.json` file inside the output directory as
 |--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.|
 |--apiserver|when scaling down|apiserver endpoint (required to cordon and drain nodes). This should be output as part of the create template or it can be found by looking at the public ip addresses in the resource group.|
-|--auth-method|no|The authentication method used. Default value is `client_secret`. Other supported values are: `cli`, `client_certificate`, and `device`.|
+|--auth-method|no|The authentication method used. Default value is `client_secret`. Other supported values are: `client_certificate`.|
 |--language|no|Language to return error message in. Default value is "en-us").|
 
 ## Frequently Asked Questions

docs/topics/upgrade.md

@@ -72,7 +72,7 @@ In summary, using `aks-engine-azurestack upgrade` means you will freshen and re-
 |--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`.|
+|--auth-method|no|The authentication method used. Default value is `client_secret`. Other supported values are: `client_certificate`.|
 |--private-key-path|no|Path to private key (used with --auth-method=client_certificate).|
 |--language|no|Language to return error message in. Default value is "en-us").|
 

docs/tutorials/cli-overview.md

@@ -50,7 +50,7 @@ Usage:
 
 Flags:
   -m, --api-model string             path to your cluster definition file
-      --auth-method client_secret    auth method (default:client_secret, `cli`, `client_certificate`, `device`) (default "cli")
+      --auth-method client_secret    auth method (default:client_secret, `client_certificate`)
       --auto-suffix                  automatically append a compressed timestamp to the dnsPrefix to ensure unique cluster name automatically
       --azure-env string             the target Azure cloud (default "AzurePublicCloud")
       --ca-certificate-path string   path to the CA certificate to use for Kubernetes PKI assets
@@ -90,7 +90,7 @@ Usage:
 Flags:
   -m, --api-model string            path to the generated apimodel.json file
       --apiserver string            apiserver endpoint (required to cordon and drain nodes)
-      --auth-method client_secret   auth method (default:client_secret, `cli`, `client_certificate`, `device`) (default "cli")
+      --auth-method client_secret   auth method (default:client_secret, `client_certificate`)
       --azure-env string            the target Azure cloud (default "AzurePublicCloud")
       --certificate-path string     path to client certificate (used with --auth-method=client_certificate)
       --client-id string            client id (used with --auth-method=[client_secret|client_certificate])
@@ -131,7 +131,7 @@ Usage:
 
 Flags:
   -m, --api-model string            path to the generated apimodel.json file
-      --auth-method client_secret   auth method (default:client_secret, `cli`, `client_certificate`, `device`) (default "cli")
+      --auth-method client_secret   auth method (default:client_secret, `client_certificate`)
       --azure-env string            the target Azure cloud (default "AzurePublicCloud")
       --certificate-path string     path to client certificate (used with --auth-method=client_certificate)
       --client-id string            client id (used with --auth-method=[client_secret|client_certificate])
@@ -164,7 +164,7 @@ Usage:
 
 Flags:
   -m, --api-model string            path to the generated apimodel.json file
-      --auth-method client_secret   auth method (default:client_secret, `cli`, `client_certificate`, `device`) (default "cli")
+      --auth-method client_secret   auth method (default:client_secret, `client_certificate`)
       --azure-env string            the target Azure cloud (default "AzurePublicCloud")
       --certificate-path string     path to client certificate (used with --auth-method=client_certificate)
       --client-id string            client id (used with --auth-method=[client_secret|client_certificate])

go.mod

@@ -7,8 +7,6 @@ require (
 	github.com/Azure/azure-storage-blob-go v0.7.0
 	github.com/Azure/go-autorest/autorest v0.11.12
 	github.com/Azure/go-autorest/autorest/adal v0.9.10
-	github.com/Azure/go-autorest/autorest/azure/cli v0.3.0
-	github.com/Azure/go-autorest/autorest/date v0.3.0
 	github.com/Azure/go-autorest/autorest/to v0.3.0
 	github.com/BurntSushi/toml v0.3.1
 	github.com/Jeffail/gabs v1.1.1
@@ -22,7 +20,6 @@ require (
 	github.com/jarcoal/httpmock v1.0.1
 	github.com/leonelquinteros/gotext v1.4.0
 	github.com/mattn/go-colorable v0.0.9
-	github.com/mitchellh/go-homedir v1.1.0
 	github.com/onsi/ginkgo/v2 v2.17.1
 	github.com/onsi/gomega v1.30.0
 	github.com/pkg/errors v0.9.1
@@ -43,10 +40,10 @@ require (
 require (
 	github.com/Azure/azure-pipeline-go v0.2.1 // indirect
 	github.com/Azure/go-autorest v14.2.0+incompatible // indirect
+	github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
 	github.com/Azure/go-autorest/autorest/validation v0.2.0 // indirect
 	github.com/Azure/go-autorest/logger v0.2.0 // indirect
 	github.com/Azure/go-autorest/tracing v0.6.0 // indirect
-	github.com/dimchansky/utfbom v1.1.0 // indirect
 	github.com/dnaeon/go-vcr v1.0.1 // indirect
 	github.com/emicklei/go-restful/v3 v3.9.0 // indirect
 	github.com/form3tech-oss/jwt-go v3.2.2+incompatible // indirect

go.sum

@@ -11,19 +11,14 @@ github.com/Azure/go-autorest/autorest v0.9.0/go.mod h1:xyHB1BMZT0cuDHU7I0+g046+B
 github.com/Azure/go-autorest/autorest v0.11.12 h1:gI8ytXbxMfI+IVbI9mP2JGCTXIuhHLgRlvQ9X4PsnHE=
 github.com/Azure/go-autorest/autorest v0.11.12/go.mod h1:eipySxLmqSyC5s5k1CLupqet0PSENBEDP93LQ9a8QYw=
 github.com/Azure/go-autorest/autorest/adal v0.5.0/go.mod h1:8Z9fGy2MpX0PvDjB1pEgQTmVqjGhiHBW7RJJEciWzS0=
-github.com/Azure/go-autorest/autorest/adal v0.6.0/go.mod h1:Z6vX6WXXuyieHAXwMj0S6HY6e6wcHn37qQMBQlvY3lc=
 github.com/Azure/go-autorest/autorest/adal v0.9.5/go.mod h1:B7KF7jKIeC9Mct5spmyCB/A8CG/sEz1vwIRGv/bbw7A=
 github.com/Azure/go-autorest/autorest/adal v0.9.10 h1:r6fZHMaHD8B6LDCn0o5vyBFHIHrM6Ywwx7mb49lPItI=
 github.com/Azure/go-autorest/autorest/adal v0.9.10/go.mod h1:B7KF7jKIeC9Mct5spmyCB/A8CG/sEz1vwIRGv/bbw7A=
-github.com/Azure/go-autorest/autorest/azure/cli v0.3.0 h1:5PAqnv+CSTwW9mlZWZAizmzrazFWEgZykEZXpr2hDtY=
-github.com/Azure/go-autorest/autorest/azure/cli v0.3.0/go.mod h1:rNYMNAefZMRowqCV0cVhr/YDW5dD7afFq9nXAXL4ykE=
 github.com/Azure/go-autorest/autorest/date v0.1.0/go.mod h1:plvfp3oPSKwf2DNjlBjWF/7vwR+cUD/ELuzDCXwHUVA=
-github.com/Azure/go-autorest/autorest/date v0.2.0/go.mod h1:vcORJHLJEh643/Ioh9+vPmf1Ij9AEBM5FuBIXLmIy0g=
 github.com/Azure/go-autorest/autorest/date v0.3.0 h1:7gUk1U5M/CQbp9WoqinNzJar+8KY+LPI6wiWrP/myHw=
 github.com/Azure/go-autorest/autorest/date v0.3.0/go.mod h1:BI0uouVdmngYNUzGWeSYnokU+TrmwEsOqdt8Y6sso74=
 github.com/Azure/go-autorest/autorest/mocks v0.1.0/go.mod h1:OTyCOPRA2IgIlWxVYxBee2F5Gr4kF2zd2J5cFRaIDN0=
 github.com/Azure/go-autorest/autorest/mocks v0.2.0/go.mod h1:OTyCOPRA2IgIlWxVYxBee2F5Gr4kF2zd2J5cFRaIDN0=
-github.com/Azure/go-autorest/autorest/mocks v0.3.0/go.mod h1:a8FDP3DYzQ4RYfVAxAN3SVSiiO77gL2j2ronKKP0syM=
 github.com/Azure/go-autorest/autorest/mocks v0.4.1 h1:K0laFcLE6VLTOwNgSxaGbUcLPuGXlNkbVvq4cW4nIHk=
 github.com/Azure/go-autorest/autorest/mocks v0.4.1/go.mod h1:LTp+uSrOhSkaKrUy935gNZuuIPPVsHlr9DSOxSayd+k=
 github.com/Azure/go-autorest/autorest/to v0.3.0 h1:zebkZaadz7+wIQYgC7GXaz3Wb28yKYfVkkBKwc38VF8=
@@ -52,8 +47,6 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
-github.com/dimchansky/utfbom v1.1.0 h1:FcM3g+nofKgUteL8dm/UpdRXNC9KmADgTpLKsu0TRo4=
-github.com/dimchansky/utfbom v1.1.0/go.mod h1:rO41eb7gLfo8SF1jd9F8HplJm1Fewwi4mQvIirEdv+8=
 github.com/dnaeon/go-vcr v1.0.1 h1:r8L/HqC0Hje5AXMu1ooW8oyQyOFv4GxqpL0nRP7SLLY=
 github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyGc8n1E=
 github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE=
@@ -161,8 +154,6 @@ github.com/mattn/go-isatty v0.0.10 h1:qxFzApOv4WsAL965uUPIsXzAKCZxN2p9UqdhFS4ZW1
 github.com/mattn/go-isatty v0.0.10/go.mod h1:qgIWMr58cqv1PHHyhnkY9lrL7etaEgOFcMEpPG5Rm84=
 github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b h1:j7+1HpAFS1zy5+Q4qx1fWh90gTKwiN4QCGoY9TWyyO4=
 github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE=
-github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
-github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
 github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg=
 github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=