diff --git a/builder/azure/arm/authenticate.go b/builder/azure/arm/authenticate.go index 10b8e4bd7..8204660de 100644 --- a/builder/azure/arm/authenticate.go +++ b/builder/azure/arm/authenticate.go @@ -2,44 +2,9 @@ package arm import ( "github.com/Azure/go-autorest/autorest/adal" - "github.com/Azure/go-autorest/autorest/azure" ) -type Authenticate struct { - env azure.Environment - clientID string - clientSecret string - tenantID string -} - -func NewAuthenticate(env azure.Environment, clientID, clientSecret, tenantID string) *Authenticate { - return &Authenticate{ - env: env, - clientID: clientID, - clientSecret: clientSecret, - tenantID: tenantID, - } -} - -func (a *Authenticate) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) { - return a.getServicePrincipalTokenWithResource(a.env.ResourceManagerEndpoint) -} - -func (a *Authenticate) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) { - oauthConfig, err := adal.NewOAuthConfig(a.env.ActiveDirectoryEndpoint, a.tenantID) - if err != nil { - return nil, err - } - - if a.clientID == "" && a.clientSecret == "" { - return adal.NewServicePrincipalTokenFromMSI("http://169.254.169.254/metadata/identity/oauth2/token", resource) - } - - spt, err := adal.NewServicePrincipalToken( - *oauthConfig, - a.clientID, - a.clientSecret, - resource) - - return spt, err +type oAuthTokenProvider interface { + getServicePrincipalToken() (*adal.ServicePrincipalToken, error) + getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) } diff --git a/builder/azure/arm/authenticate_devicewflow.go b/builder/azure/arm/authenticate_devicewflow.go new file mode 100644 index 000000000..48a6d2f69 --- /dev/null +++ b/builder/azure/arm/authenticate_devicewflow.go @@ -0,0 +1,36 @@ +package arm + +import ( + "fmt" + "strings" + + "github.com/Azure/go-autorest/autorest/adal" + "github.com/Azure/go-autorest/autorest/azure" + packerAzureCommon "github.com/hashicorp/packer/builder/azure/common" +) + +func NewDeviceFlowOAuthTokenProvider(env azure.Environment, say func(string), tenantID string) oAuthTokenProvider { + return &deviceflowOauthTokenProvider{} +} + +type deviceflowOauthTokenProvider struct { + env azure.Environment + say func(string) + tenantID string +} + +func (tp *deviceflowOauthTokenProvider) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) { + return tp.getServicePrincipalTokenWithResource(tp.env.ResourceManagerEndpoint) +} + +func (tp *deviceflowOauthTokenProvider) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) { + if resource == tp.env.ServiceManagementEndpoint { + tp.say("Getting auth token for Service management endpoint") + } else if resource == strings.TrimRight(tp.env.KeyVaultEndpoint, "/") { + tp.say("Getting token for Vault resource") + } else { + tp.say(fmt.Sprintf("Getting token for %s", resource)) + } + + return packerAzureCommon.Authenticate(tp.env, tp.tenantID, tp.say, resource) +} diff --git a/builder/azure/arm/authenticate_msi.go b/builder/azure/arm/authenticate_msi.go new file mode 100644 index 000000000..b9f551eeb --- /dev/null +++ b/builder/azure/arm/authenticate_msi.go @@ -0,0 +1,23 @@ +package arm + +import ( + "github.com/Azure/go-autorest/autorest/adal" + "github.com/Azure/go-autorest/autorest/azure" +) + +// for managed identity auth +type msiOAuthTokenProvider struct { + env azure.Environment +} + +func NewMSIOAuthTokenProvider(env azure.Environment) oAuthTokenProvider { + return &msiOAuthTokenProvider{env} +} + +func (tp *msiOAuthTokenProvider) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) { + return tp.getServicePrincipalTokenWithResource(tp.env.ResourceManagerEndpoint) +} + +func (tp *msiOAuthTokenProvider) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) { + return adal.NewServicePrincipalTokenFromMSI("http://169.254.169.254/metadata/identity/oauth2/token", resource) +} diff --git a/builder/azure/arm/authenticate_secret.go b/builder/azure/arm/authenticate_secret.go new file mode 100644 index 000000000..6a434591a --- /dev/null +++ b/builder/azure/arm/authenticate_secret.go @@ -0,0 +1,35 @@ +package arm + +import ( + "github.com/Azure/go-autorest/autorest/adal" + "github.com/Azure/go-autorest/autorest/azure" +) + +// for clientID/secret auth +type secretOAuthTokenProvider struct { + env azure.Environment + clientID, clientSecret, tenantID string +} + +func NewSecretOAuthTokenProvider(env azure.Environment, clientID, clientSecret, tenantID string) oAuthTokenProvider { + return &secretOAuthTokenProvider{env, clientID, clientSecret, tenantID} +} + +func (tp *secretOAuthTokenProvider) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) { + return tp.getServicePrincipalTokenWithResource(tp.env.ResourceManagerEndpoint) +} + +func (tp *secretOAuthTokenProvider) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) { + oauthConfig, err := adal.NewOAuthConfig(tp.env.ActiveDirectoryEndpoint, tp.tenantID) + if err != nil { + return nil, err + } + + spt, err := adal.NewServicePrincipalToken( + *oauthConfig, + tp.clientID, + tp.clientSecret, + resource) + + return spt, err +} diff --git a/builder/azure/arm/authenticate_test.go b/builder/azure/arm/authenticate_secret_test.go similarity index 88% rename from builder/azure/arm/authenticate_test.go rename to builder/azure/arm/authenticate_secret_test.go index 63fd1c3aa..532883b4f 100644 --- a/builder/azure/arm/authenticate_test.go +++ b/builder/azure/arm/authenticate_secret_test.go @@ -9,8 +9,8 @@ import ( // Behavior is the most important thing to assert for ServicePrincipalToken, but // that cannot be done in a unit test because it involves network access. Instead, // I assert the expected inertness of this class. -func TestNewAuthenticate(t *testing.T) { - testSubject := NewAuthenticate(azure.PublicCloud, "clientID", "clientString", "tenantID") +func TestNewSecretOAuthTokenProvider(t *testing.T) { + testSubject := NewSecretOAuthTokenProvider(azure.PublicCloud, "clientID", "clientString", "tenantID") spn, err := testSubject.getServicePrincipalToken() if err != nil { t.Fatalf(err.Error()) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 1dfd66fa5..48c19263e 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -385,52 +385,7 @@ func (b *Builder) setImageParameters(stateBag multistep.StateBag) { } func (b *Builder) getServicePrincipalTokens(say func(string)) (*adal.ServicePrincipalToken, *adal.ServicePrincipalToken, error) { - var servicePrincipalToken *adal.ServicePrincipalToken - var servicePrincipalTokenVault *adal.ServicePrincipalToken - - var err error - - if b.config.useDeviceLogin { - say("Getting auth token for Service management endpoint") - servicePrincipalToken, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.TenantID, say, b.config.cloudEnvironment.ServiceManagementEndpoint) - if err != nil { - return nil, nil, err - } - say("Getting token for Vault resource") - servicePrincipalTokenVault, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.TenantID, say, strings.TrimRight(b.config.cloudEnvironment.KeyVaultEndpoint, "/")) - if err != nil { - return nil, nil, err - } - - } else { - auth := NewAuthenticate(*b.config.cloudEnvironment, b.config.ClientID, b.config.ClientSecret, b.config.TenantID) - - servicePrincipalToken, err = auth.getServicePrincipalToken() - if err != nil { - return nil, nil, err - } - - servicePrincipalTokenVault, err = auth.getServicePrincipalTokenWithResource( - strings.TrimRight(b.config.cloudEnvironment.KeyVaultEndpoint, "/")) - if err != nil { - return nil, nil, err - } - - } - - err = servicePrincipalToken.EnsureFresh() - - if err != nil { - return nil, nil, err - } - - err = servicePrincipalTokenVault.EnsureFresh() - - if err != nil { - return nil, nil, err - } - - return servicePrincipalToken, servicePrincipalTokenVault, nil + return b.config.ClientConfig.getServicePrincipalTokens(say) } func getObjectIdFromToken(ui packer.Ui, token *adal.ServicePrincipalToken) string { diff --git a/builder/azure/arm/clientconfig.go b/builder/azure/arm/clientconfig.go new file mode 100644 index 000000000..d4daff259 --- /dev/null +++ b/builder/azure/arm/clientconfig.go @@ -0,0 +1,165 @@ +package arm + +import ( + "fmt" + "strings" + + "github.com/Azure/go-autorest/autorest/adal" + "github.com/Azure/go-autorest/autorest/azure" + "github.com/hashicorp/packer/packer" +) + +// ClientConfig allows for various ways to authenticate Azure clients +type ClientConfig struct { + // Describes where API's are + + CloudEnvironmentName string `mapstructure:"cloud_environment_name"` + cloudEnvironment *azure.Environment + + // Authentication fields + + // Client ID + ClientID string `mapstructure:"client_id"` + // Client secret/password + ClientSecret string `mapstructure:"client_secret"` + ObjectID string `mapstructure:"object_id"` + TenantID string `mapstructure:"tenant_id"` + SubscriptionID string `mapstructure:"subscription_id"` +} + +const DefaultCloudEnvironmentName = "Public" + +func (c *ClientConfig) provideDefaultValues() { + if c.CloudEnvironmentName == "" { + c.CloudEnvironmentName = DefaultCloudEnvironmentName + } +} + +func (c *ClientConfig) setCloudEnvironment() error { + lookup := map[string]string{ + "CHINA": "AzureChinaCloud", + "CHINACLOUD": "AzureChinaCloud", + "AZURECHINACLOUD": "AzureChinaCloud", + + "GERMAN": "AzureGermanCloud", + "GERMANCLOUD": "AzureGermanCloud", + "AZUREGERMANCLOUD": "AzureGermanCloud", + + "GERMANY": "AzureGermanCloud", + "GERMANYCLOUD": "AzureGermanCloud", + "AZUREGERMANYCLOUD": "AzureGermanCloud", + + "PUBLIC": "AzurePublicCloud", + "PUBLICCLOUD": "AzurePublicCloud", + "AZUREPUBLICCLOUD": "AzurePublicCloud", + + "USGOVERNMENT": "AzureUSGovernmentCloud", + "USGOVERNMENTCLOUD": "AzureUSGovernmentCloud", + "AZUREUSGOVERNMENTCLOUD": "AzureUSGovernmentCloud", + } + + name := strings.ToUpper(c.CloudEnvironmentName) + envName, ok := lookup[name] + if !ok { + return fmt.Errorf("There is no cloud environment matching the name '%s'!", c.CloudEnvironmentName) + } + + env, err := azure.EnvironmentFromName(envName) + c.cloudEnvironment = &env + return err +} + +func (c ClientConfig) assertRequiredParametersSet(errs *packer.MultiError) { + ///////////////////////////////////////////// + // Authentication via OAUTH + + // Check if device login is being asked for, and is allowed. + // + // Device login is enabled if the user only defines SubscriptionID and not + // ClientID, ClientSecret, and TenantID. + // + // Device login is not enabled for Windows because the WinRM certificate is + // readable by the ObjectID of the App. There may be another way to handle + // this case, but I am not currently aware of it - send feedback. + + if c.useMSI() { + return + } + + if c.SubscriptionID == "" { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("A subscription_id must be specified")) + } + + if c.useDeviceLogin() { + return + } + + if c.SubscriptionID != "" && c.ClientID != "" && c.ClientSecret != "" { + // Service principal using secret + return + } + + errs = packer.MultiErrorAppend(errs, fmt.Errorf("No valid set of authentication values specified:\n"+ + "* to use the Managed Identity of teh current machine, do not specify any of the fields below\n"+ + "* to use interactive user authentication, specify only subscription_id\n"+ + "* to use an Azure Active Directory service principal, specify subscription_id, client_id and client_secret.")) +} + +func (c ClientConfig) useDeviceLogin() bool { + return c.SubscriptionID != "" && + c.ClientID == "" && + c.ClientSecret == "" && + c.TenantID == "" +} + +func (c ClientConfig) useMSI() bool { + return c.SubscriptionID == "" && + c.ClientID == "" && + c.ClientSecret == "" && + c.TenantID == "" +} + +func (c ClientConfig) getServicePrincipalTokens( + say func(string)) ( + servicePrincipalToken *adal.ServicePrincipalToken, + servicePrincipalTokenVault *adal.ServicePrincipalToken, + err error) { + + tenantID := c.TenantID + + var auth oAuthTokenProvider + + if c.useDeviceLogin() { + say("Getting tokens using device flow") + auth = NewDeviceFlowOAuthTokenProvider(*c.cloudEnvironment, say, tenantID) + } else if c.useMSI() { + say("Getting tokens using Managed Identity for Azure") + auth = NewMSIOAuthTokenProvider(*c.cloudEnvironment) + } else { + say("Getting tokens using client secret") + auth = NewSecretOAuthTokenProvider(*c.cloudEnvironment, c.ClientID, c.ClientSecret, tenantID) + } + + servicePrincipalToken, err = auth.getServicePrincipalToken() + if err != nil { + return nil, nil, err + } + + err = servicePrincipalToken.EnsureFresh() + if err != nil { + return nil, nil, err + } + + servicePrincipalTokenVault, err = auth.getServicePrincipalTokenWithResource( + strings.TrimRight(c.cloudEnvironment.KeyVaultEndpoint, "/")) + if err != nil { + return nil, nil, err + } + + err = servicePrincipalTokenVault.EnsureFresh() + if err != nil { + return nil, nil, err + } + + return servicePrincipalToken, servicePrincipalTokenVault, nil +} diff --git a/builder/azure/arm/clientconfig_test.go b/builder/azure/arm/clientconfig_test.go new file mode 100644 index 000000000..87fefae09 --- /dev/null +++ b/builder/azure/arm/clientconfig_test.go @@ -0,0 +1,199 @@ +package arm + +import ( + "fmt" + "os" + "testing" + + "github.com/Azure/go-autorest/autorest/azure" + "github.com/hashicorp/packer/packer" +) + +func Test_ClientConfig_RequiredParametersSet(t *testing.T) { + + tests := []struct { + name string + config ClientConfig + wantErr bool + }{ + { + name: "no client_id, client_secret or subscription_id should enable MSI auth", + config: ClientConfig{}, + wantErr: false, + }, + { + name: "subscription_id is set will trigger device flow", + config: ClientConfig{ + SubscriptionID: "error", + }, + wantErr: false, + }, + { + name: "client_id without client_secret should error", + config: ClientConfig{ + ClientID: "error", + }, + wantErr: true, + }, + { + name: "client_secret without client_id should error", + config: ClientConfig{ + ClientSecret: "error", + }, + wantErr: true, + }, + { + name: "missing subscription_id when using secret", + config: ClientConfig{ + ClientID: "ok", + ClientSecret: "ok", + }, + wantErr: true, + }, + { + name: "tenant_id alone should fail", + config: ClientConfig{ + TenantID: "ok", + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + errs := &packer.MultiError{} + tt.config.assertRequiredParametersSet(errs) + if (len(errs.Errors) != 0) != tt.wantErr { + t.Errorf("newConfig() error = %v, wantErr %v", errs, tt.wantErr) + return + } + }) + } +} + +func Test_ClientConfig_DeviceLogin(t *testing.T) { + getEnvOrSkip(t, "AZURE_DEVICE_LOGIN") + cfg := ClientConfig{ + SubscriptionID: getEnvOrSkip(t, "AZURE_SUBSCRIPTION"), + cloudEnvironment: getCloud(), + } + assertValid(t, cfg) + + spt, sptkv, err := cfg.getServicePrincipalTokens( + func(s string) { fmt.Printf("SAY: %s\n", s) }) + if err != nil { + t.Fatalf("Expected nil err, but got: %v", err) + } + token := spt.Token() + if token.AccessToken == "" { + t.Fatal("Expected management token to have non-nil access token") + } + if token.RefreshToken == "" { + t.Fatal("Expected management token to have non-nil refresh token") + } + kvtoken := sptkv.Token() + if kvtoken.AccessToken == "" { + t.Fatal("Expected keyvault token to have non-nil access token") + } + if kvtoken.RefreshToken == "" { + t.Fatal("Expected keyvault token to have non-nil refresh token") + } +} + +func Test_ClientConfig_ClientPassword(t *testing.T) { + cfg := ClientConfig{ + SubscriptionID: getEnvOrSkip(t, "AZURE_SUBSCRIPTION"), + ClientID: getEnvOrSkip(t, "AZURE_CLIENTID"), + ClientSecret: getEnvOrSkip(t, "AZURE_CLIENTSECRET"), + TenantID: getEnvOrSkip(t, "AZURE_TENANTID"), + cloudEnvironment: getCloud(), + } + assertValid(t, cfg) + + spt, sptkv, err := cfg.getServicePrincipalTokens(func(s string) { fmt.Printf("SAY: %s\n", s) }) + if err != nil { + t.Fatalf("Expected nil err, but got: %v", err) + } + token := spt.Token() + if token.AccessToken == "" { + t.Fatal("Expected management token to have non-nil access token") + } + if token.RefreshToken != "" { + t.Fatal("Expected management token to have no refresh token") + } + kvtoken := sptkv.Token() + if kvtoken.AccessToken == "" { + t.Fatal("Expected keyvault token to have non-nil access token") + } + if kvtoken.RefreshToken != "" { + t.Fatal("Expected keyvault token to have no refresh token") + } +} + +func getEnvOrSkip(t *testing.T, envVar string) string { + v := os.Getenv(envVar) + if v == "" { + t.Skipf("%s is empty, skipping", envVar) + } + return v +} + +func getCloud() *azure.Environment { + cloudName := os.Getenv("AZURE_CLOUD") + if cloudName == "" { + cloudName = "AZUREPUBLICCLOUD" + } + c, _ := azure.EnvironmentFromName(cloudName) + return &c +} + +// tests for assertRequiredParametersSet + +func Test_ClientConfig_CanUseDeviceCode(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + // TenantID is optional + + assertValid(t, cfg) +} + +func assertValid(t *testing.T, cfg ClientConfig) { + errs := &packer.MultiError{} + cfg.assertRequiredParametersSet(errs) + if len(errs.Errors) != 0 { + t.Fatal("Expected errs to be empty: ", errs) + } +} + +func assertInvalid(t *testing.T, cfg ClientConfig) { + errs := &packer.MultiError{} + cfg.assertRequiredParametersSet(errs) + if len(errs.Errors) == 0 { + t.Fatal("Expected errs to be non-empty") + } +} + +func Test_ClientConfig_CanUseClientSecret(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientSecret = "12345" + + assertValid(t, cfg) +} + +func Test_ClientConfig_CanUseClientSecretWithTenantID(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientSecret = "12345" + cfg.TenantID = "12345" + + assertValid(t, cfg) +} + +func emptyClientConfig() ClientConfig { + cfg := ClientConfig{} + _ = cfg.setCloudEnvironment() + return cfg +} diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index 35f79e6f8..862226245 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -15,7 +15,6 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute" - "github.com/Azure/go-autorest/autorest/azure" "github.com/Azure/go-autorest/autorest/to" "github.com/masterzen/winrm" @@ -32,7 +31,6 @@ import ( ) const ( - DefaultCloudEnvironmentName = "Public" DefaultImageVersion = "latest" DefaultUserName = "packer" DefaultPrivateVirtualNetworkWithPublicIp = false @@ -79,11 +77,7 @@ type Config struct { common.PackerConfig `mapstructure:",squash"` // Authentication via OAUTH - ClientID string `mapstructure:"client_id"` - ClientSecret string `mapstructure:"client_secret"` - ObjectID string `mapstructure:"object_id"` - TenantID string `mapstructure:"tenant_id"` - SubscriptionID string `mapstructure:"subscription_id"` + ClientConfig `mapstructure:",squash"` // Capture CaptureNamePrefix string `mapstructure:"capture_name_prefix"` @@ -122,8 +116,6 @@ type Config struct { TempResourceGroupName string `mapstructure:"temp_resource_group_name"` BuildResourceGroupName string `mapstructure:"build_resource_group_name"` storageAccountBlobEndpoint string - CloudEnvironmentName string `mapstructure:"cloud_environment_name"` - cloudEnvironment *azure.Environment PrivateVirtualNetworkWithPublicIp bool `mapstructure:"private_virtual_network_with_public_ip"` VirtualNetworkName string `mapstructure:"virtual_network_name"` VirtualNetworkSubnetName string `mapstructure:"virtual_network_subnet_name"` @@ -157,8 +149,6 @@ type Config struct { tmpVirtualNetworkName string tmpWinRMCertificateUrl string - useDeviceLogin bool - // Authentication with the VM via SSH sshAuthorizedKey string @@ -287,7 +277,7 @@ func newConfig(raws ...interface{}) (*Config, []string, error) { provideDefaultValues(&c) setRuntimeValues(&c) setUserNamePassword(&c) - err = setCloudEnvironment(&c) + err = c.ClientConfig.setCloudEnvironment() if err != nil { return nil, nil, err } @@ -416,40 +406,6 @@ func setUserNamePassword(c *Config) { } } -func setCloudEnvironment(c *Config) error { - lookup := map[string]string{ - "CHINA": "AzureChinaCloud", - "CHINACLOUD": "AzureChinaCloud", - "AZURECHINACLOUD": "AzureChinaCloud", - - "GERMAN": "AzureGermanCloud", - "GERMANCLOUD": "AzureGermanCloud", - "AZUREGERMANCLOUD": "AzureGermanCloud", - - "GERMANY": "AzureGermanCloud", - "GERMANYCLOUD": "AzureGermanCloud", - "AZUREGERMANYCLOUD": "AzureGermanCloud", - - "PUBLIC": "AzurePublicCloud", - "PUBLICCLOUD": "AzurePublicCloud", - "AZUREPUBLICCLOUD": "AzurePublicCloud", - - "USGOVERNMENT": "AzureUSGovernmentCloud", - "USGOVERNMENTCLOUD": "AzureUSGovernmentCloud", - "AZUREUSGOVERNMENTCLOUD": "AzureUSGovernmentCloud", - } - - name := strings.ToUpper(c.CloudEnvironmentName) - envName, ok := lookup[name] - if !ok { - return fmt.Errorf("There is no cloud environment matching the name '%s'!", c.CloudEnvironmentName) - } - - env, err := azure.EnvironmentFromName(envName) - c.cloudEnvironment = &env - return err -} - func setCustomData(c *Config) error { if c.CustomDataFile == "" { return nil @@ -481,9 +437,7 @@ func provideDefaultValues(c *Config) { c.ImageVersion = DefaultImageVersion } - if c.CloudEnvironmentName == "" { - c.CloudEnvironmentName = DefaultCloudEnvironmentName - } + c.provideDefaultValues() } func assertTagProperties(c *Config, errs *packer.MultiError) { @@ -502,37 +456,7 @@ func assertTagProperties(c *Config, errs *packer.MultiError) { } func assertRequiredParametersSet(c *Config, errs *packer.MultiError) { - ///////////////////////////////////////////// - // Authentication via OAUTH - - // Check if device login is being asked for, and is allowed. - // - // Device login is enabled if the user only defines SubscriptionID and not - // ClientID, ClientSecret, and TenantID. - // - // Device login is not enabled for Windows because the WinRM certificate is - // readable by the ObjectID of the App. There may be another way to handle - // this case, but I am not currently aware of it - send feedback. - isUseDeviceLogin := func(c *Config) bool { - - return c.SubscriptionID != "" && - c.ClientID == "" && - c.ClientSecret == "" && - c.TenantID == "" - } - - if isUseDeviceLogin(c) { - c.useDeviceLogin = true - } else { - if c.ClientID == "" && c.ClientSecret != "" || c.ClientID != "" && c.ClientSecret == "" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("A client_id and client_secret must be specified together or not specified at all")) - } - - if c.ClientID != "" && c.SubscriptionID == "" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("A subscription_id must be specified when client_id & client_secret are")) - } - - } + c.ClientConfig.assertRequiredParametersSet(errs) ///////////////////////////////////////////// // Capture diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index e5b7e6ece..bf1bd5023 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -133,87 +133,6 @@ func TestConfigShouldNotDefaultImageVersionIfCustomImage(t *testing.T) { } } -func Test_newConfig_MSI(t *testing.T) { - baseConfig := map[string]string{ - "capture_name_prefix": "ignore", - "capture_container_name": "ignore", - "location": "ignore", - "image_url": "ignore", - "storage_account": "ignore", - "resource_group_name": "ignore", - "os_type": constants.Target_Linux, - } - - tests := []struct { - name string - args []interface{} - wantErr bool - }{ - { - name: "no client_id and no client_secret should enable MSI auth", - args: []interface{}{ - baseConfig, - getPackerConfiguration(), - }, - wantErr: false, - }, - { - name: "subscription_id is will be taken from MSI", - args: []interface{}{ - baseConfig, - map[string]string{ - "subscription_id": "error", - }, - getPackerConfiguration(), - }, - wantErr: false, - }, - { - name: "client_id without client_secret should error", - args: []interface{}{ - baseConfig, - map[string]string{ - "client_id": "error", - }, - getPackerConfiguration(), - }, - wantErr: true, - }, - { - name: "client_secret without client_id should error", - args: []interface{}{ - baseConfig, - map[string]string{ - "client_secret": "error", - }, - getPackerConfiguration(), - }, - wantErr: true, - }, - { - name: "missing subscription_id", - args: []interface{}{ - baseConfig, - map[string]string{ - "client_id": "ok", - "client_secret": "ok", - }, - getPackerConfiguration(), - }, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _, _, err := newConfig(tt.args...) - if (err != nil) != tt.wantErr { - t.Errorf("newConfig() error = %v, wantErr %v", err, tt.wantErr) - return - } - }) - } -} - func TestConfigShouldNormalizeOSTypeCase(t *testing.T) { config := map[string]string{ "capture_name_prefix": "ignore",