diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ea507302..3ff411956 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ * builder/amazon: don't Cleanup Temp Keys when there is no communicator to avoid a panic [GH-7100] [GH-7095] * builder/azure: allow to configure disk caching [GH-7061] +* builder/azure: add certificate authentication [GH-7189] +* builder/azure: use deallocate instead of just power-off [GH-7203] * builder/openstack: Don't require network v2 [GH-6933] * builder/openstack: Support for tagging new images [GH-7037] * core/shell: Add env vars "PACKER_HTTP_IP" and "PACKER_HTTP_PORT" to shell 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_cert.go b/builder/azure/arm/authenticate_cert.go new file mode 100644 index 000000000..0c1fb6e49 --- /dev/null +++ b/builder/azure/arm/authenticate_cert.go @@ -0,0 +1,155 @@ +package arm + +import ( + "crypto/ecdsa" + "crypto/rand" + "crypto/rsa" + "crypto/sha1" + "crypto/x509" + "encoding/base64" + "encoding/pem" + "fmt" + "io/ioutil" + "os" + "time" + + "github.com/Azure/go-autorest/autorest/azure" + jwt "github.com/dgrijalva/jwt-go" +) + +func NewCertOAuthTokenProvider(env azure.Environment, clientID, clientCertPath, tenantID string) (oAuthTokenProvider, error) { + cert, key, err := readCert(clientCertPath) + if err != nil { + return nil, fmt.Errorf("Error reading certificate: %v", err) + } + + audience := fmt.Sprintf("%s%s/oauth2/token", env.ActiveDirectoryEndpoint, tenantID) + jwt, err := makeJWT(clientID, audience, cert, key, time.Hour, true) + if err != nil { + return nil, fmt.Errorf("Error generating JWT: %v", err) + } + + return NewJWTOAuthTokenProvider(env, clientID, jwt, tenantID), nil +} + +// Creates a new JSON Web Token to be used as bearer JWT to authenticate +// to the Azure AD token endpoint to retrieve an access token for `audience`. +// If the full certificate is included in the token, then issuer/subject name +// could be used to authenticate if configured by the identity provider (AAD). +func makeJWT(clientID string, audience string, + cert *x509.Certificate, privatekey interface{}, + validFor time.Duration, includeFullCertificate bool) (string, error) { + + // The jti (JWT ID) claim provides a unique identifier for the JWT. + // See https://tools.ietf.org/html/rfc7519#section-4.1.7 + jti := make([]byte, 20) + _, err := rand.Read(jti) + if err != nil { + return "", err + } + + var token *jwt.Token + if cert.PublicKeyAlgorithm == x509.RSA { + token = jwt.New(jwt.SigningMethodRS256) + } else if cert.PublicKeyAlgorithm == x509.ECDSA { + token = jwt.New(jwt.SigningMethodES256) + } else { + return "", fmt.Errorf("Don't know how to handle this type of key algorithm: %v", cert.PublicKeyAlgorithm) + } + + hasher := sha1.New() + if _, err := hasher.Write(cert.Raw); err != nil { + return "", err + } + thumbprint := base64.URLEncoding.EncodeToString(hasher.Sum(nil)) + + // X.509 thumbprint, see https://tools.ietf.org/html/rfc7515#section-4.1.7 + token.Header["x5t"] = thumbprint + if includeFullCertificate { + // X.509 certificate (chain), see https://tools.ietf.org/html/rfc7515#section-4.1.6 + token.Header["x5c"] = []string{base64.StdEncoding.EncodeToString(cert.Raw)} + } + + token.Claims = jwt.MapClaims{ + // See https://tools.ietf.org/html/rfc7519#section-4.1 + "aud": audience, + "iss": clientID, + "sub": clientID, + "jti": base64.URLEncoding.EncodeToString(jti), + "nbf": time.Now().Unix(), + "exp": time.Now().Add(validFor).Unix(), + } + + return token.SignedString(privatekey) +} + +func readCert(file string) (cert *x509.Certificate, key interface{}, err error) { + f, err := os.Open(file) + if err != nil { + return nil, nil, err + } + defer f.Close() + d, err := ioutil.ReadAll(f) + if err != nil { + return nil, nil, err + } + + blocks := []*pem.Block{} + for len(d) > 0 { + var b *pem.Block + b, d = pem.Decode(d) + if b == nil { + break + } + blocks = append(blocks, b) + } + + certs := []*x509.Certificate{} + for _, block := range blocks { + if block.Type == "CERTIFICATE" { + c, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, nil, fmt.Errorf( + "Failed to read certificate block: %v", err) + } + certs = append(certs, c) + } else if block.Type == "PRIVATE KEY" { + key, err = x509.ParsePKCS8PrivateKey(block.Bytes) + if err != nil { + return nil, nil, fmt.Errorf( + "Failed to read private key block: %v", err) + } + } + // Don't care about other types of blocks, ignore + } + + if key == nil { + return nil, nil, fmt.Errorf("Did not find private key in pem file") + } + + // find the certificate that belongs to the private key by comparing the public keys + switch key := key.(type) { + case *rsa.PrivateKey: + for _, c := range certs { + if cp, ok := c.PublicKey.(*rsa.PublicKey); ok && + (cp.N.Cmp(key.PublicKey.N) == 0) { + cert = c + } + } + + case *ecdsa.PrivateKey: + for _, c := range certs { + if cp, ok := c.PublicKey.(*ecdsa.PublicKey); ok && + (cp.X.Cmp(key.PublicKey.X) == 0) && + (cp.Y.Cmp(key.PublicKey.Y) == 0) { + cert = c + } + } + } + + if cert == nil { + return nil, nil, fmt.Errorf("Did not find certificate belonging to private key in pem file") + } + + return cert, key, nil +} 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_jwt.go b/builder/azure/arm/authenticate_jwt.go new file mode 100644 index 000000000..d4bb41748 --- /dev/null +++ b/builder/azure/arm/authenticate_jwt.go @@ -0,0 +1,43 @@ +package arm + +import ( + "net/url" + + "github.com/Azure/go-autorest/autorest/adal" + "github.com/Azure/go-autorest/autorest/azure" +) + +// for clientID/bearer JWT auth +type jwtOAuthTokenProvider struct { + env azure.Environment + clientID, clientJWT, tenantID string +} + +func NewJWTOAuthTokenProvider(env azure.Environment, clientID, clientJWT, tenantID string) oAuthTokenProvider { + return &jwtOAuthTokenProvider{env, clientID, clientJWT, tenantID} +} + +func (tp *jwtOAuthTokenProvider) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) { + return tp.getServicePrincipalTokenWithResource(tp.env.ResourceManagerEndpoint) +} + +func (tp *jwtOAuthTokenProvider) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) { + oauthConfig, err := adal.NewOAuthConfig(tp.env.ActiveDirectoryEndpoint, tp.tenantID) + if err != nil { + return nil, err + } + + return adal.NewServicePrincipalTokenWithSecret( + *oauthConfig, + tp.clientID, + resource, + tp) +} + +// implements github.com/Azure/go-autorest/autorest/adal.ServicePrincipalSecret +func (tp *jwtOAuthTokenProvider) SetAuthenticationValues( + t *adal.ServicePrincipalToken, v *url.Values) error { + v.Set("client_assertion", tp.clientJWT) + v.Set("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer") + return nil +} 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..69dfffbea --- /dev/null +++ b/builder/azure/arm/clientconfig.go @@ -0,0 +1,223 @@ +package arm + +import ( + "fmt" + "os" + "strings" + "time" + + "github.com/Azure/go-autorest/autorest/adal" + "github.com/Azure/go-autorest/autorest/azure" + jwt "github.com/dgrijalva/jwt-go" + "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"` + // Certificate path for client auth + ClientCertPath string `mapstructure:"client_cert_path"` + // JWT bearer token for client auth (RFC 7523, Sec. 2.2) + ClientJWT string `mapstructure:"client_jwt"` + 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.useDeviceLogin() { + return + } + + if c.SubscriptionID != "" && c.ClientID != "" && + c.ClientSecret != "" && + c.ClientCertPath == "" && + c.ClientJWT == "" { + // Service principal using secret + return + } + + if c.SubscriptionID != "" && c.ClientID != "" && + c.ClientSecret == "" && + c.ClientCertPath != "" && + c.ClientJWT == "" { + // Service principal using certificate + + if _, err := os.Stat(c.ClientCertPath); err != nil { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("client_cert_path is not an accessible file: %v", err)) + } + return + } + + if c.SubscriptionID != "" && c.ClientID != "" && + c.ClientSecret == "" && + c.ClientCertPath == "" && + c.ClientJWT != "" { + // Service principal using JWT + // Check that JWT is valid for at least 5 more minutes + + p := jwt.Parser{} + claims := jwt.StandardClaims{} + token, _, err := p.ParseUnverified(c.ClientJWT, &claims) + if err != nil { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("client_jwt is not a JWT: %v", err)) + } else { + if claims.ExpiresAt < time.Now().Add(5*time.Minute).Unix() { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("client_jwt will expire within 5 minutes, please use a JWT that is valid for at least 5 minutes")) + } + if t, ok := token.Header["x5t"]; !ok || t == "" { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("client_jwt is missing the x5t header value, which is required for bearer JWT client authentication to Azure")) + } + } + + return + } + + errs = packer.MultiErrorAppend(errs, fmt.Errorf("No valid set of authentication values specified:\n"+ + " to use the Managed Identity of the 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 either:\n"+ + " - subscription_id, client_id and client_secret\n"+ + " - subscription_id, client_id and client_cert_path\n"+ + " - subscription_id, client_id and client_jwt.")) +} + +func (c ClientConfig) useDeviceLogin() bool { + return c.SubscriptionID != "" && + c.ClientID == "" && + c.ClientSecret == "" && + c.ClientJWT == "" && + c.ClientCertPath == "" && + c.TenantID == "" +} + +func (c ClientConfig) useMSI() bool { + return c.SubscriptionID == "" && + c.ClientID == "" && + c.ClientSecret == "" && + c.ClientJWT == "" && + c.ClientCertPath == "" && + 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 if c.ClientSecret != "" { + say("Getting tokens using client secret") + auth = NewSecretOAuthTokenProvider(*c.cloudEnvironment, c.ClientID, c.ClientSecret, tenantID) + } else if c.ClientCertPath != "" { + say("Getting tokens using client certificate") + auth, err = NewCertOAuthTokenProvider(*c.cloudEnvironment, c.ClientID, c.ClientCertPath, tenantID) + if err != nil { + return nil, nil, err + } + } else { + say("Getting tokens using client bearer JWT") + auth = NewJWTOAuthTokenProvider(*c.cloudEnvironment, c.ClientID, c.ClientJWT, 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..f9fafd2f2 --- /dev/null +++ b/builder/azure/arm/clientconfig_test.go @@ -0,0 +1,397 @@ +package arm + +import ( + crand "crypto/rand" + "crypto/rsa" + "encoding/base64" + "encoding/binary" + "fmt" + "io" + mrand "math/rand" + "os" + "testing" + "time" + + "github.com/Azure/go-autorest/autorest/azure" + jwt "github.com/dgrijalva/jwt-go" + "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, client_cert_path or client_jwt should error", + config: ClientConfig{ + ClientID: "error", + }, + wantErr: true, + }, + { + name: "client_secret without client_id should error", + config: ClientConfig{ + ClientSecret: "error", + }, + wantErr: true, + }, + { + name: "client_cert_path without client_id should error", + config: ClientConfig{ + ClientCertPath: "/dev/null", + }, + wantErr: true, + }, + { + name: "client_jwt without client_id should error", + config: ClientConfig{ + ClientJWT: "error", + }, + wantErr: true, + }, + { + name: "missing subscription_id when using secret", + config: ClientConfig{ + ClientID: "ok", + ClientSecret: "ok", + }, + wantErr: true, + }, + { + name: "missing subscription_id when using certificate", + config: ClientConfig{ + ClientID: "ok", + ClientCertPath: "ok", + }, + wantErr: true, + }, + { + name: "missing subscription_id when using JWT", + config: ClientConfig{ + ClientID: "ok", + ClientJWT: "ok", + }, + wantErr: true, + }, + { + name: "too many client_* values", + config: ClientConfig{ + SubscriptionID: "ok", + ClientID: "ok", + ClientSecret: "ok", + ClientCertPath: "error", + }, + wantErr: true, + }, + { + name: "too many client_* values (2)", + config: ClientConfig{ + SubscriptionID: "ok", + ClientID: "ok", + ClientSecret: "ok", + ClientJWT: "error", + }, + 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 Test_ClientConfig_ClientCert(t *testing.T) { + cfg := ClientConfig{ + SubscriptionID: getEnvOrSkip(t, "AZURE_SUBSCRIPTION"), + ClientID: getEnvOrSkip(t, "AZURE_CLIENTID"), + ClientCertPath: getEnvOrSkip(t, "AZURE_CLIENTCERT"), + 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 Test_ClientConfig_ClientJWT(t *testing.T) { + cfg := ClientConfig{ + SubscriptionID: getEnvOrSkip(t, "AZURE_SUBSCRIPTION"), + ClientID: getEnvOrSkip(t, "AZURE_CLIENTID"), + ClientJWT: getEnvOrSkip(t, "AZURE_CLIENTJWT"), + 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 Test_ClientConfig_CanUseClientJWT(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientJWT = getJWT(10*time.Minute, true) + + assertValid(t, cfg) +} + +func Test_ClientConfig_CanUseClientJWTWithTenantID(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientJWT = getJWT(10*time.Minute, true) + cfg.TenantID = "12345" + + assertValid(t, cfg) +} + +func Test_ClientConfig_CannotUseBothClientJWTAndSecret(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientSecret = "12345" + cfg.ClientJWT = getJWT(10*time.Minute, true) + + assertInvalid(t, cfg) +} + +func Test_ClientConfig_ClientJWTShouldBeValidForAtLeast5Minutes(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientJWT = getJWT(time.Minute, true) + + assertInvalid(t, cfg) +} + +func Test_ClientConfig_ClientJWTShouldHaveThumbprint(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientJWT = getJWT(10*time.Minute, false) + + assertInvalid(t, cfg) +} + +func emptyClientConfig() ClientConfig { + cfg := ClientConfig{} + _ = cfg.setCloudEnvironment() + return cfg +} + +func Test_getJWT(t *testing.T) { + if getJWT(time.Minute, true) == "" { + t.Fatalf("getJWT is broken") + } +} + +func newRandReader() io.Reader { + var seed int64 + binary.Read(crand.Reader, binary.LittleEndian, &seed) + + return mrand.New(mrand.NewSource(seed)) +} + +func getJWT(validFor time.Duration, withX5tHeader bool) string { + token := jwt.New(jwt.SigningMethodRS256) + key, _ := rsa.GenerateKey(newRandReader(), 2048) + + token.Claims = jwt.MapClaims{ + "aud": "https://login.microsoftonline.com/tenant.onmicrosoft.com/oauth2/token?api-version=1.0", + "iss": "355dff10-cd78-11e8-89fe-000d3afd16e3", + "sub": "355dff10-cd78-11e8-89fe-000d3afd16e3", + "jti": base64.URLEncoding.EncodeToString([]byte{0}), + "nbf": time.Now().Unix(), + "exp": time.Now().Add(validFor).Unix(), + } + if withX5tHeader { + token.Header["x5t"] = base64.URLEncoding.EncodeToString([]byte("thumbprint")) + } + + jwt, _ := token.SignedString(key) + return jwt +} 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", diff --git a/builder/azure/arm/step_power_off_compute.go b/builder/azure/arm/step_power_off_compute.go index da0ede214..4f03adefd 100644 --- a/builder/azure/arm/step_power_off_compute.go +++ b/builder/azure/arm/step_power_off_compute.go @@ -28,7 +28,7 @@ func NewStepPowerOffCompute(client *AzureClient, ui packer.Ui) *StepPowerOffComp } func (s *StepPowerOffCompute) powerOffCompute(ctx context.Context, resourceGroupName string, computeName string) error { - f, err := s.client.VirtualMachinesClient.PowerOff(ctx, resourceGroupName, computeName) + f, err := s.client.VirtualMachinesClient.Deallocate(ctx, resourceGroupName, computeName) if err == nil { err = f.WaitForCompletion(ctx, s.client.VirtualMachinesClient.Client) } diff --git a/builder/hyperv/common/step_output_dir_test.go b/builder/hyperv/common/step_output_dir_test.go deleted file mode 100644 index 5933a5fe9..000000000 --- a/builder/hyperv/common/step_output_dir_test.go +++ /dev/null @@ -1,146 +0,0 @@ -package common - -import ( - "context" - "os" - "testing" - - "github.com/hashicorp/packer/helper/multistep" -) - -func TestStepOutputDir_imp(t *testing.T) { - var _ multistep.Step = new(StepOutputDir) -} - -func TestStepOutputDir_Default(t *testing.T) { - state := testState(t) - step := new(StepOutputDir) - - step.Path = genTestDirPath("packerHypervOutput") - - // Test the run - if action := step.Run(context.Background(), state); action != multistep.ActionContinue { - t.Fatalf("Bad action: %v", action) - } - if _, ok := state.GetOk("error"); ok { - t.Fatal("Should NOT have error") - } - - // The directory should have been created - if _, err := os.Stat(step.Path); err != nil { - t.Fatal("Should have created output directory") - } - - // Remove the directory created due to test - err := os.RemoveAll(step.Path) - if err != nil { - t.Fatalf("Error encountered removing directory created by test: %s", err) - } -} - -func TestStepOutputDir_DirectoryAlreadyExistsNoForce(t *testing.T) { - state := testState(t) - step := new(StepOutputDir) - - step.Path = genTestDirPath("packerHypervOutput") - - // Create the directory so that we can test - err := os.Mkdir(step.Path, 0755) - if err != nil { - t.Fatal("Test failed to create directory for test of Cancel and Cleanup") - } - defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong - - step.Force = false // Default - // Test the run - if action := step.Run(context.Background(), state); action != multistep.ActionHalt { - t.Fatalf("Should halt when directory exists and 'Force' is false. Bad action: %v", action) - } - if _, ok := state.GetOk("error"); !ok { - t.Fatal("Should error when directory exists and 'Force' is false") - } -} - -func TestStepOutputDir_DirectoryAlreadyExistsForce(t *testing.T) { - state := testState(t) - step := new(StepOutputDir) - - step.Path = genTestDirPath("packerHypervOutput") - - // Create the directory so that we can test - err := os.Mkdir(step.Path, 0755) - if err != nil { - t.Fatal("Test failed to create directory for test of Cancel and Cleanup") - } - defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong - - step.Force = true // User specified that existing directory and contents should be discarded - if action := step.Run(context.Background(), state); action != multistep.ActionContinue { - t.Fatalf("Should complete when directory exists and 'Force' is true. Bad action: %v", action) - } - if _, ok := state.GetOk("error"); ok { - t.Fatalf("Should NOT error when directory exists and 'Force' is true: %s", err) - } -} - -func TestStepOutputDir_CleanupBuildCancelled(t *testing.T) { - state := testState(t) - step := new(StepOutputDir) - - step.Path = genTestDirPath("packerHypervOutput") - - // Create the directory so that we can test the cleanup - err := os.Mkdir(step.Path, 0755) - if err != nil { - t.Fatal("Test failed to create directory for test of Cancel and Cleanup") - } - defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong - - // 'Cancel' the build - state.Put(multistep.StateCancelled, true) - - // Ensure the directory isn't removed if the cleanup flag is false - step.cleanup = false - step.Cleanup(state) - if _, err := os.Stat(step.Path); err != nil { - t.Fatal("Output dir should NOT be removed if on 'Cancel' if cleanup flag is unset/false") - } - - // Ensure the directory is removed if the cleanup flag is true - step.cleanup = true - step.Cleanup(state) - if _, err := os.Stat(step.Path); err == nil { - t.Fatalf("Output directory should NOT exist after 'Cancel' and Cleanup: %s", step.Path) - } -} - -func TestStepOutputDir_CleanupBuildHalted(t *testing.T) { - state := testState(t) - step := new(StepOutputDir) - - step.Path = genTestDirPath("packerHypervOutput") - - // Create the directory so that we can test the cleanup - err := os.Mkdir(step.Path, 0755) - if err != nil { - t.Fatal("Test failed to create directory for test of Cancel and Cleanup") - } - defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong - - // 'Halt' the build and test the directory is removed - state.Put(multistep.StateHalted, true) - - // Ensure the directory isn't removed if the cleanup flag is false - step.cleanup = false - step.Cleanup(state) - if _, err := os.Stat(step.Path); err != nil { - t.Fatal("Output dir should NOT be removed if on 'Halt' if cleanup flag is unset/false") - } - - // Ensure the directory is removed if the cleanup flag is true - step.cleanup = true - step.Cleanup(state) - if _, err := os.Stat(step.Path); err == nil { - t.Fatalf("Output directory should NOT exist after 'Halt' and Cleanup: %s", step.Path) - } -} diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index 7fccf0634..08a010268 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -376,7 +376,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe &hypervcommon.StepCreateBuildDir{ TempPath: b.config.TempPath, }, - &hypervcommon.StepOutputDir{ + &common.StepOutputDir{ Force: b.config.PackerForce, Path: b.config.OutputDir, }, diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index d3eb5f261..13b733ba2 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -392,7 +392,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe &hypervcommon.StepCreateBuildDir{ TempPath: b.config.TempPath, }, - &hypervcommon.StepOutputDir{ + &common.StepOutputDir{ Force: b.config.PackerForce, Path: b.config.OutputDir, }, diff --git a/builder/parallels/common/driver_9.go b/builder/parallels/common/driver_9.go index 77fa3ed5b..5a5d9393f 100644 --- a/builder/parallels/common/driver_9.go +++ b/builder/parallels/common/driver_9.go @@ -154,6 +154,7 @@ func (d *Parallels9Driver) DeviceAddCDROM(name string, image string) (string, er "set", name, "--device-add", "cdrom", "--image", image, + "--enable", "--connect", } out, err := exec.Command(d.PrlctlPath, command...).Output() diff --git a/builder/virtualbox/common/step_output_dir.go b/builder/virtualbox/common/step_output_dir.go deleted file mode 100644 index fc39d08b2..000000000 --- a/builder/virtualbox/common/step_output_dir.go +++ /dev/null @@ -1,86 +0,0 @@ -package common - -import ( - "context" - "fmt" - "log" - "os" - "path/filepath" - "time" - - "github.com/hashicorp/packer/helper/multistep" - "github.com/hashicorp/packer/packer" -) - -// StepOutputDir sets up the output directory by creating it if it does -// not exist, deleting it if it does exist and we're forcing, and cleaning -// it up when we're done with it. -type StepOutputDir struct { - Force bool - Path string - - cleanup bool -} - -func (s *StepOutputDir) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { - ui := state.Get("ui").(packer.Ui) - - if _, err := os.Stat(s.Path); err == nil { - if !s.Force { - err := fmt.Errorf( - "Output directory exists: %s\n\n"+ - "Use the force flag to delete it prior to building.", - s.Path) - state.Put("error", err) - return multistep.ActionHalt - } - - ui.Say("Deleting previous output directory...") - os.RemoveAll(s.Path) - } - - // Enable cleanup - s.cleanup = true - - // Create the directory - if err := os.MkdirAll(s.Path, 0755); err != nil { - state.Put("error", err) - return multistep.ActionHalt - } - - // Make sure we can write in the directory - f, err := os.Create(filepath.Join(s.Path, "_packer_perm_check")) - if err != nil { - err = fmt.Errorf("Couldn't write to output directory: %s", err) - state.Put("error", err) - return multistep.ActionHalt - } - f.Close() - os.Remove(f.Name()) - - return multistep.ActionContinue -} - -func (s *StepOutputDir) Cleanup(state multistep.StateBag) { - if !s.cleanup { - return - } - - _, cancelled := state.GetOk(multistep.StateCancelled) - _, halted := state.GetOk(multistep.StateHalted) - - if cancelled || halted { - ui := state.Get("ui").(packer.Ui) - - ui.Say("Deleting output directory...") - for i := 0; i < 5; i++ { - err := os.RemoveAll(s.Path) - if err == nil { - break - } - - log.Printf("Error removing output dir: %s", err) - time.Sleep(2 * time.Second) - } - } -} diff --git a/builder/virtualbox/iso/builder.go b/builder/virtualbox/iso/builder.go index 603a36aa8..55aa602a9 100644 --- a/builder/virtualbox/iso/builder.go +++ b/builder/virtualbox/iso/builder.go @@ -209,7 +209,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe TargetPath: b.config.TargetPath, Url: b.config.ISOUrls, }, - &vboxcommon.StepOutputDir{ + &common.StepOutputDir{ Force: b.config.PackerForce, Path: b.config.OutputDir, }, diff --git a/builder/virtualbox/ovf/builder.go b/builder/virtualbox/ovf/builder.go index c87a16cb0..60142728f 100644 --- a/builder/virtualbox/ovf/builder.go +++ b/builder/virtualbox/ovf/builder.go @@ -50,7 +50,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe // Build the steps. steps := []multistep.Step{ - &vboxcommon.StepOutputDir{ + &common.StepOutputDir{ Force: b.config.PackerForce, Path: b.config.OutputDir, }, diff --git a/common/download.go b/common/download.go index 78f31f25d..a5b217a62 100644 --- a/common/download.go +++ b/common/download.go @@ -191,8 +191,9 @@ func (d *DownloadClient) Get() (string, error) { } err = fmt.Errorf( - "checksums didn't match expected: %s", - hex.EncodeToString(d.config.Checksum)) + "checksums didn't match. expected %s and got %s", + hex.EncodeToString(d.config.Checksum), + hex.EncodeToString(d.config.Hash.Sum(nil))) } } diff --git a/common/download_test.go b/common/download_test.go index dd41d6cc1..be4382393 100644 --- a/common/download_test.go +++ b/common/download_test.go @@ -411,7 +411,7 @@ func TestDownloadFileUrl(t *testing.T) { // Verify that we fail to match the checksum _, err = client.Get() - if err.Error() != "checksums didn't match expected: 6e6f7065" { + if err.Error() != "checksums didn't match. expected 6e6f7065 and got 606f1945f81a022d0ed0bd99edfd4f99081c1cb1f97fae087291ee14e945e608" { t.Fatalf("Unexpected failure; expected checksum not to match. Error was \"%v\"", err) } @@ -442,7 +442,7 @@ func SimulateFileUriDownload(t *testing.T, uri string) (string, error) { path, err := client.Get() // ignore any non-important checksum errors if it's not a unc path - if !strings.HasPrefix(path, "\\\\") && err.Error() != "checksums didn't match expected: 6e6f7065" { + if !strings.HasPrefix(path, "\\\\") && err.Error() != "checksums didn't match. expected 6e6f7065 and got 606f1945f81a022d0ed0bd99edfd4f99081c1cb1f97fae087291ee14e945e608" { t.Fatalf("Unexpected failure; expected checksum not to match") } diff --git a/builder/hyperv/common/step_output_dir.go b/common/step_output_dir.go similarity index 100% rename from builder/hyperv/common/step_output_dir.go rename to common/step_output_dir.go diff --git a/builder/virtualbox/common/step_output_dir_test.go b/common/step_output_dir_test.go similarity index 91% rename from builder/virtualbox/common/step_output_dir_test.go rename to common/step_output_dir_test.go index 766154ca9..197977151 100644 --- a/builder/virtualbox/common/step_output_dir_test.go +++ b/common/step_output_dir_test.go @@ -1,14 +1,25 @@ package common import ( + "bytes" "context" "io/ioutil" "os" "testing" "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" ) +func testState(t *testing.T) multistep.StateBag { + state := new(multistep.BasicStateBag) + state.Put("ui", &packer.BasicUi{ + Reader: new(bytes.Buffer), + Writer: new(bytes.Buffer), + }) + return state +} + func testStepOutputDir(t *testing.T) *StepOutputDir { td, err := ioutil.TempDir("", "packer") if err != nil { diff --git a/post-processor/amazon-import/post-processor.go b/post-processor/amazon-import/post-processor.go index 1bc8508d5..58e91c9e8 100644 --- a/post-processor/amazon-import/post-processor.go +++ b/post-processor/amazon-import/post-processor.go @@ -203,7 +203,20 @@ func (p *PostProcessor) PostProcess(ui packer.Ui, artifact packer.Artifact) (pac ui.Message(fmt.Sprintf("Waiting for task %s to complete (may take a while)", *import_start.ImportTaskId)) err = awscommon.WaitUntilImageImported(aws.BackgroundContext(), ec2conn, *import_start.ImportTaskId) if err != nil { - return nil, false, fmt.Errorf("Import task %s failed with error: %s", *import_start.ImportTaskId, err) + + // Retrieve the status message + import_result, err2 := ec2conn.DescribeImportImageTasks(&ec2.DescribeImportImageTasksInput{ + ImportTaskIds: []*string{ + import_start.ImportTaskId, + }, + }) + + statusMessage := "Error retrieving status message" + + if err2 == nil { + statusMessage = *import_result.ImportImageTasks[0].StatusMessage + } + return nil, false, fmt.Errorf("Import task %s failed with status message: %s, error: %s", *import_start.ImportTaskId, statusMessage, err) } // Retrieve what the outcome was for the import task @@ -216,7 +229,6 @@ func (p *PostProcessor) PostProcess(ui packer.Ui, artifact packer.Artifact) (pac if err != nil { return nil, false, fmt.Errorf("Failed to find import task %s: %s", *import_start.ImportTaskId, err) } - // Check it was actually completed if *import_result.ImportImageTasks[0].Status != "completed" { // The most useful error message is from the job itself diff --git a/website/source/docs/builders/azure-setup.html.md b/website/source/docs/builders/azure-setup.html.md index de107a78c..99173fe5e 100644 --- a/website/source/docs/builders/azure-setup.html.md +++ b/website/source/docs/builders/azure-setup.html.md @@ -204,37 +204,12 @@ Make sure that `GROUPNAME` and `LOCATION` are the same as above. Also, ensure that `GROUPNAME` is less than 24 characters long and contains only lowercase letters and numbers. -### Create an Application - -An application represents a way to authorize access to the Azure API. Note that -you will need to specify a URL for your application (this is intended to be -used for OAuth callbacks) but these do not actually need to be valid URLs. - -First pick APPNAME, APPURL and PASSWORD: - -``` shell -APPNAME=packer.test -APPURL=packer.test -PASSWORD=xxx -``` - -Password is your `client_secret` and can be anything you like. I recommend -using `openssl rand -base64 24`. - -``` shell -$ az ad app create \ - --display-name $APPNAME \ - --identifier-uris $APPURL \ - --homepage $APPURL \ - --password $PASSWORD -``` - ### Create a Service Principal -You cannot directly grant permissions to an application. Instead, you create a -service principal and assign permissions to the service principal. To create a -service principal for use with Packer, run the below command specifying the -subscription. This will grant Packer the contributor role to the subscription. +A service principal acts on behalf of an application (Packer) on your Azure +subscription. To create an application and service principal for use with +Packer, run the below command specifying the subscription. This will grant +Packer the contributor role to the subscription. The output of this command is your service principal credentials, save these in a safe place as you will need these to configure Packer. @@ -263,8 +238,13 @@ pre-configured roles via: $ az role definition list --output json | jq ".[] | {name:.roleName, description:.description}" ``` +If you would rather use a certificate to autenticate your service principal, +please follow the [Azure Active Directory documentation](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-certificate-credentials#register-your-certificate-with-azure-ad). + ### Configuring Packer Now (finally) everything has been setup in Azure and our service principal has been created. You can use the output from creating your service principal in -your template. +your template. Use the value from the `appId` field above as a value for +`client_id` in your configuration and set `client_secret` to the `password` +value from above. diff --git a/website/source/docs/builders/azure.html.md b/website/source/docs/builders/azure.html.md index 3ed470369..8caf44848 100644 --- a/website/source/docs/builders/azure.html.md +++ b/website/source/docs/builders/azure.html.md @@ -35,12 +35,15 @@ addition to the options listed here, a [communicator](/docs/templates/communicator.html) can be configured for this builder. -### Required ( unless instance has [managed identities](/docs/builders/azure-setup.html#managed-identities-for-azure-resources) enabled): - -- `client_id` (string) The Active Directory service principal associated with - your builder. - -- `client_secret` (string) The password or secret for your service principal. +### Required options for authentication: +If you're running packer on an Azure VM with a [managed identity](/docs/builders/azure-setup.html#managed-identities-for-azure-resources) +you don't need to specify any additional configuration options. +If you would like to use interactive user authentication, you should specify +`subscription_id` only. Packer will use cached credentials or redirect you +to a website to log in. +If you want to use a [service principal](/docs/builders/azure-setup.html#create-a-service-principal) +you should specify `subscription_id`, `client_id` and one of `client_secret`, +`client_cert_path` or `client_jwt`. - `subscription_id` (string) Subscription under which the build will be performed. **The service principal specified in `client_id` must have full @@ -48,6 +51,19 @@ builder. specified in which case it needs to have owner access to the existing resource group specified in build\_resource\_group\_name parameter.** +- `client_id` (string) The Active Directory service principal associated with + your builder. + +- `client_secret` (string) The password or secret for your service principal. + +- `client_cert_path` (string) The location of a PEM file containing a + certificate and private key for service principal. + +- `client_jwt` (string) The bearer JWT assertion signed using a certificate + associated with your service principal principal. See [Azure Active + Directory docs](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-certificate-credentials) + for more information. + ### Required: - `image_publisher` (string) PublisherName for your base image. See