From 6d6b94d515d86012a453d449a8c73aff5a73e9bd Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 23 Jan 2020 14:28:54 -0800 Subject: [PATCH 1/4] Add ability to use custom keyvault into azure builds --- builder/azure/arm/builder.go | 23 +++++++-- builder/azure/arm/config.go | 5 +- .../azure/arm/step_certificate_in_keyvault.go | 50 +++++++++++++++++++ .../azure/arm/step_delete_resource_group.go | 9 ++-- builder/azure/arm/step_deploy_template.go | 4 ++ builder/azure/common/constants/stateBag.go | 1 + builder/azure/common/vault.go | 44 +++++++++++++++- 7 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 builder/azure/arm/step_certificate_in_keyvault.go diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 799659c50..1bd63f426 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -231,8 +231,16 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack keyVaultDeploymentName := b.stateBag.Get(constants.ArmKeyVaultDeploymentName).(string) steps = []multistep.Step{ NewStepCreateResourceGroup(azureClient, ui), - NewStepValidateTemplate(azureClient, ui, &b.config, GetKeyVaultDeployment), - NewStepDeployTemplate(azureClient, ui, &b.config, keyVaultDeploymentName, GetKeyVaultDeployment), + } + if b.config.BuildKeyVaultName == "" { + steps = append(steps, + NewStepValidateTemplate(azureClient, ui, &b.config, GetKeyVaultDeployment), + NewStepDeployTemplate(azureClient, ui, &b.config, keyVaultDeploymentName, GetKeyVaultDeployment), + ) + } else { + steps = append(steps, NewStepCertificateInKeyVault(azureClient, ui, &b.config)) + } + steps = append(steps, NewStepGetCertificate(azureClient, ui), NewStepSetCertificate(&b.config, ui), NewStepValidateTemplate(azureClient, ui, &b.config, GetVirtualMachineDeployment), @@ -261,7 +269,7 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack NewStepDeleteResourceGroup(azureClient, ui), NewStepDeleteOSDisk(azureClient, ui), NewStepDeleteAdditionalDisks(azureClient, ui), - } + ) } else { return nil, fmt.Errorf("Builder does not support the os_type '%s'", b.config.OSType) } @@ -395,7 +403,14 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) { stateBag.Put(constants.ArmKeyVaultDeploymentName, fmt.Sprintf("kv%s", b.config.tmpDeploymentName)) } - stateBag.Put(constants.ArmKeyVaultName, b.config.tmpKeyVaultName) + if b.config.BuildKeyVaultName != "" { + stateBag.Put(constants.ArmKeyVaultName, b.config.BuildKeyVaultName) + b.config.tmpKeyVaultName = b.config.BuildKeyVaultName + stateBag.Put(constants.ArmIsExistingKeyVault, false) + } else { + stateBag.Put(constants.ArmKeyVaultName, b.config.tmpKeyVaultName) + stateBag.Put(constants.ArmIsExistingKeyVault, true) + } stateBag.Put(constants.ArmNicName, b.config.tmpNicName) stateBag.Put(constants.ArmPublicIPAddressName, b.config.tmpPublicIPAddressName) stateBag.Put(constants.ArmResourceGroupName, b.config.BuildResourceGroupName) diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index 18b426e19..3b701eae7 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -253,7 +253,10 @@ type Config struct { // group is deleted at the end of the build. TempResourceGroupName string `mapstructure:"temp_resource_group_name"` // Specify an existing resource group to run the build in. - BuildResourceGroupName string `mapstructure:"build_resource_group_name"` + BuildResourceGroupName string `mapstructure:"build_resource_group_name"` + // Specify an existing key vault to use for uploading certificates to the + // instance to connect. + BuildKeyVaultName string `mapstructure:"build_key_vault_name"` storageAccountBlobEndpoint string // This value allows you to // set a virtual_network_name and obtain a public IP. If this value is not diff --git a/builder/azure/arm/step_certificate_in_keyvault.go b/builder/azure/arm/step_certificate_in_keyvault.go new file mode 100644 index 000000000..a46b160ce --- /dev/null +++ b/builder/azure/arm/step_certificate_in_keyvault.go @@ -0,0 +1,50 @@ +package arm + +import ( + "context" + "fmt" + + "github.com/hashicorp/packer/builder/azure/common/constants" + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +type StepCertificateInKeyVault struct { + config *Config + client *AzureClient + say func(message string) + error func(e error) +} + +func NewStepCertificateInKeyVault(cli *AzureClient, ui packer.Ui, config *Config) *StepCertificateInKeyVault { + var step = &StepCertificateInKeyVault{ + client: cli, + config: config, + say: func(message string) { ui.Say(message) }, + error: func(e error) { ui.Error(e.Error()) }, + } + + return step +} + +func (s *StepCertificateInKeyVault) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { + s.say("Setting the certificate in the KeyVault...") + + var keyVaultName = state.Get(constants.ArmKeyVaultName).(string) + // err := s.client.CreateKey(keyVaultName, DefaultSecretName) + // if err != nil { + // s.error(fmt.Errorf("Error setting winrm cert in custom keyvault: %s", err)) + // return multistep.ActionHalt + // } + + err := s.client.SetSecret(keyVaultName, DefaultSecretName, s.config.winrmCertificate) + if err != nil { + s.error(fmt.Errorf("Error setting winrm cert in custom keyvault: %s", err)) + return multistep.ActionHalt + } + + return multistep.ActionContinue +} + +func (*StepCertificateInKeyVault) Cleanup(multistep.StateBag) { +} diff --git a/builder/azure/arm/step_delete_resource_group.go b/builder/azure/arm/step_delete_resource_group.go index aac1c4b91..80564b1c6 100644 --- a/builder/azure/arm/step_delete_resource_group.go +++ b/builder/azure/arm/step_delete_resource_group.go @@ -44,9 +44,12 @@ func (s *StepDeleteResourceGroup) deleteResourceGroup(ctx context.Context, state } if keyVaultDeploymentName, ok := state.GetOk(constants.ArmKeyVaultDeploymentName); ok { - err = s.deleteDeploymentResources(ctx, keyVaultDeploymentName.(string), resourceGroupName) - if err != nil { - return err + // Only delete if custom keyvault was not provided. + if exists := state.Get(constants.ArmIsExistingKeyVault).(bool); exists { + err = s.deleteDeploymentResources(ctx, keyVaultDeploymentName.(string), resourceGroupName) + if err != nil { + return err + } } } diff --git a/builder/azure/arm/step_deploy_template.go b/builder/azure/arm/step_deploy_template.go index a8626bd00..15d19394f 100644 --- a/builder/azure/arm/step_deploy_template.go +++ b/builder/azure/arm/step_deploy_template.go @@ -76,6 +76,10 @@ func (s *StepDeployTemplate) deleteTemplate(ctx context.Context, state multistep } func (s *StepDeployTemplate) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { + if s.config.BuildKeyVaultName != "" { + // Deployment already exists + + } s.say("Deploying deployment template ...") var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string) diff --git a/builder/azure/common/constants/stateBag.go b/builder/azure/common/constants/stateBag.go index 0bf0ad1ae..b18e5f152 100644 --- a/builder/azure/common/constants/stateBag.go +++ b/builder/azure/common/constants/stateBag.go @@ -36,6 +36,7 @@ const ( ArmTags string = "arm.Tags" ArmVirtualMachineCaptureParameters string = "arm.VirtualMachineCaptureParameters" ArmIsExistingResourceGroup string = "arm.IsExistingResourceGroup" + ArmIsExistingKeyVault string = "arm.IsExistingKeyVault" ArmIsManagedImage string = "arm.IsManagedImage" ArmManagedImageResourceGroupName string = "arm.ManagedImageResourceGroupName" diff --git a/builder/azure/common/vault.go b/builder/azure/common/vault.go index 1bda604a2..3c53c0074 100644 --- a/builder/azure/common/vault.go +++ b/builder/azure/common/vault.go @@ -54,7 +54,8 @@ func (client *VaultClient) GetSecret(vaultName, secretName string) (*Secret, err autorest.AsGet(), autorest.WithBaseURL(client.getVaultUrl(vaultName)), autorest.WithPathParameters("/secrets/{secret-name}", p), - autorest.WithQueryParameters(q)) + autorest.WithQueryParameters(q), + ) if err != nil { return nil, err @@ -86,6 +87,47 @@ func (client *VaultClient) GetSecret(vaultName, secretName string) (*Secret, err return &secret, nil } +func (client *VaultClient) SetSecret(vaultName, secretName string, secretValue string) error { + p := map[string]interface{}{ + "secret-name": autorest.Encode("path", secretName), + } + q := map[string]interface{}{ + "api-version": AzureVaultApiVersion, + } + + jsonBody := fmt.Sprintf(`{"value": "%s"}`, secretValue) + + req, err := autorest.Prepare( + &http.Request{}, + autorest.AsPut(), + autorest.AsContentType("application/json; charset=utf-8"), + autorest.WithBaseURL(client.getVaultUrl(vaultName)), + autorest.WithPathParameters("/secrets/{secret-name}", p), + autorest.WithQueryParameters(q), + autorest.WithString(jsonBody), + ) + + if err != nil { + return err + } + + resp, err := autorest.SendWithSender(client, req) + if err != nil { + return err + } + + if resp.StatusCode != 200 { + return fmt.Errorf( + "Failed to set secret to %s/%s, HTTP status code=%d (%s)", + vaultName, + secretName, + resp.StatusCode, + http.StatusText(resp.StatusCode)) + } + + return nil +} + // Delete deletes the specified Azure key vault. // // resourceGroupName is the name of the Resource Group to which the vault belongs. vaultName is the name of the vault From 9643ad35f1afd1aa365b48e54bfb45dadf7b8ad7 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 6 Feb 2020 16:35:39 -0800 Subject: [PATCH 2/4] add tests --- builder/azure/arm/builder.go | 2 +- .../azure/arm/step_certificate_in_keyvault.go | 11 +--- .../arm/step_certificate_in_keyvault_test.go | 66 +++++++++++++++++++ builder/azure/arm/step_deploy_template.go | 4 -- builder/azure/common/vault.go | 9 +++ builder/azure/common/vault_client_mock.go | 56 ++++++++++++++++ 6 files changed, 135 insertions(+), 13 deletions(-) create mode 100644 builder/azure/arm/step_certificate_in_keyvault_test.go create mode 100644 builder/azure/common/vault_client_mock.go diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 1bd63f426..55e889fe6 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -238,7 +238,7 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack NewStepDeployTemplate(azureClient, ui, &b.config, keyVaultDeploymentName, GetKeyVaultDeployment), ) } else { - steps = append(steps, NewStepCertificateInKeyVault(azureClient, ui, &b.config)) + steps = append(steps, NewStepCertificateInKeyVault(&azureClient.VaultClient, ui, &b.config)) } steps = append(steps, NewStepGetCertificate(azureClient, ui), diff --git a/builder/azure/arm/step_certificate_in_keyvault.go b/builder/azure/arm/step_certificate_in_keyvault.go index a46b160ce..d646acead 100644 --- a/builder/azure/arm/step_certificate_in_keyvault.go +++ b/builder/azure/arm/step_certificate_in_keyvault.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/hashicorp/packer/builder/azure/common" "github.com/hashicorp/packer/builder/azure/common/constants" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -11,12 +12,12 @@ import ( type StepCertificateInKeyVault struct { config *Config - client *AzureClient + client common.AZVaultClientIface say func(message string) error func(e error) } -func NewStepCertificateInKeyVault(cli *AzureClient, ui packer.Ui, config *Config) *StepCertificateInKeyVault { +func NewStepCertificateInKeyVault(cli common.AZVaultClientIface, ui packer.Ui, config *Config) *StepCertificateInKeyVault { var step = &StepCertificateInKeyVault{ client: cli, config: config, @@ -29,13 +30,7 @@ func NewStepCertificateInKeyVault(cli *AzureClient, ui packer.Ui, config *Config func (s *StepCertificateInKeyVault) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { s.say("Setting the certificate in the KeyVault...") - var keyVaultName = state.Get(constants.ArmKeyVaultName).(string) - // err := s.client.CreateKey(keyVaultName, DefaultSecretName) - // if err != nil { - // s.error(fmt.Errorf("Error setting winrm cert in custom keyvault: %s", err)) - // return multistep.ActionHalt - // } err := s.client.SetSecret(keyVaultName, DefaultSecretName, s.config.winrmCertificate) if err != nil { diff --git a/builder/azure/arm/step_certificate_in_keyvault_test.go b/builder/azure/arm/step_certificate_in_keyvault_test.go new file mode 100644 index 000000000..a246a7746 --- /dev/null +++ b/builder/azure/arm/step_certificate_in_keyvault_test.go @@ -0,0 +1,66 @@ +package arm + +import ( + "bytes" + "context" + "testing" + + azcommon "github.com/hashicorp/packer/builder/azure/common" + "github.com/hashicorp/packer/builder/azure/common/constants" + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +func TestNewStepCertificateInKeyVault(t *testing.T) { + cli := azcommon.MockAZVaultClient{} + ui := &packer.BasicUi{ + Reader: new(bytes.Buffer), + Writer: new(bytes.Buffer), + } + state := new(multistep.BasicStateBag) + state.Put(constants.ArmKeyVaultName, "testKeyVaultName") + + config := &Config{ + winrmCertificate: "testCertificateString", + } + + certKVStep := NewStepCertificateInKeyVault(&cli, ui, config) + stepAction := certKVStep.Run(context.TODO(), state) + + if stepAction == multistep.ActionHalt { + t.Fatalf("step should have succeeded.") + } + if !cli.SetSecretCalled { + t.Fatalf("Step should have called SetSecret on Azure client.") + } + if cli.SetSecretCert != "testCertificateString" { + t.Fatalf("Step should have read cert from winRMCertificate field on config.") + } + if cli.SetSecretVaultName != "testKeyVaultName" { + t.Fatalf("step should have read keyvault name from state.") + } +} + +func TestNewStepCertificateInKeyVault_error(t *testing.T) { + // Tell mock to return an error + cli := azcommon.MockAZVaultClient{} + cli.IsError = true + + ui := &packer.BasicUi{ + Reader: new(bytes.Buffer), + Writer: new(bytes.Buffer), + } + state := new(multistep.BasicStateBag) + state.Put(constants.ArmKeyVaultName, "testKeyVaultName") + + config := &Config{ + winrmCertificate: "testCertificateString", + } + + certKVStep := NewStepCertificateInKeyVault(&cli, ui, config) + stepAction := certKVStep.Run(context.TODO(), state) + + if stepAction != multistep.ActionHalt { + t.Fatalf("step should have failed.") + } +} diff --git a/builder/azure/arm/step_deploy_template.go b/builder/azure/arm/step_deploy_template.go index 15d19394f..a8626bd00 100644 --- a/builder/azure/arm/step_deploy_template.go +++ b/builder/azure/arm/step_deploy_template.go @@ -76,10 +76,6 @@ func (s *StepDeployTemplate) deleteTemplate(ctx context.Context, state multistep } func (s *StepDeployTemplate) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { - if s.config.BuildKeyVaultName != "" { - // Deployment already exists - - } s.say("Deploying deployment template ...") var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string) diff --git a/builder/azure/common/vault.go b/builder/azure/common/vault.go index 3c53c0074..2be3e8a36 100644 --- a/builder/azure/common/vault.go +++ b/builder/azure/common/vault.go @@ -16,6 +16,15 @@ const ( AzureVaultApiVersion = "2016-10-01" ) +// Enables us to test steps that access this cli +type AZVaultClientIface interface { + GetSecret(string, string) (*Secret, error) + SetSecret(string, string, string) error + DeletePreparer(string, string) (*http.Request, error) + DeleteResponder(*http.Response) (autorest.Response, error) + DeleteSender(*http.Request) (*http.Response, error) +} + type VaultClient struct { autorest.Client keyVaultEndpoint url.URL diff --git a/builder/azure/common/vault_client_mock.go b/builder/azure/common/vault_client_mock.go new file mode 100644 index 000000000..57bbd1c11 --- /dev/null +++ b/builder/azure/common/vault_client_mock.go @@ -0,0 +1,56 @@ +package common + +import ( + "fmt" + "net/http" + + "github.com/Azure/go-autorest/autorest" +) + +type MockAZVaultClient struct { + GetSecretCalled bool + SetSecretCalled bool + SetSecretVaultName string + SetSecretSecretName string + SetSecretCert string + DeleteResponderCalled bool + DeletePreparerCalled bool + DeleteSenderCalled bool + + IsError bool +} + +func (m *MockAZVaultClient) GetSecret(vaultName, secretName string) (*Secret, error) { + m.GetSecretCalled = true + var secret Secret + return &secret, nil +} + +func (m *MockAZVaultClient) SetSecret(vaultName, secretName string, secretValue string) error { + m.SetSecretCalled = true + m.SetSecretVaultName = vaultName + m.SetSecretSecretName = secretName + m.SetSecretCert = secretValue + + if m.IsError { + return fmt.Errorf("generic error!!") + } + + return nil +} + +func (m *MockAZVaultClient) DeletePreparer(resourceGroupName string, vaultName string) (*http.Request, error) { + m.DeletePreparerCalled = true + return nil, nil +} + +func (m *MockAZVaultClient) DeleteResponder(resp *http.Response) (autorest.Response, error) { + m.DeleteResponderCalled = true + var result autorest.Response + return result, nil +} + +func (m *MockAZVaultClient) DeleteSender(req *http.Request) (*http.Response, error) { + m.DeleteSenderCalled = true + return nil, nil +} From 7dd1fa44db2cc3b5bfeb2f740456273910531975 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 6 Feb 2020 16:39:41 -0800 Subject: [PATCH 3/4] regenerate code --- builder/azure/arm/config.hcl2spec.go | 2 ++ .../partials/builder/azure/arm/_Config-not-required.html.md | 3 +++ 2 files changed, 5 insertions(+) diff --git a/builder/azure/arm/config.hcl2spec.go b/builder/azure/arm/config.hcl2spec.go index 826100044..c3fbc8036 100644 --- a/builder/azure/arm/config.hcl2spec.go +++ b/builder/azure/arm/config.hcl2spec.go @@ -53,6 +53,7 @@ type FlatConfig struct { TempComputeName *string `mapstructure:"temp_compute_name" required:"false" cty:"temp_compute_name"` TempResourceGroupName *string `mapstructure:"temp_resource_group_name" cty:"temp_resource_group_name"` BuildResourceGroupName *string `mapstructure:"build_resource_group_name" cty:"build_resource_group_name"` + BuildKeyVaultName *string `mapstructure:"build_key_vault_name" cty:"build_key_vault_name"` PrivateVirtualNetworkWithPublicIp *bool `mapstructure:"private_virtual_network_with_public_ip" required:"false" cty:"private_virtual_network_with_public_ip"` VirtualNetworkName *string `mapstructure:"virtual_network_name" required:"false" cty:"virtual_network_name"` VirtualNetworkSubnetName *string `mapstructure:"virtual_network_subnet_name" required:"false" cty:"virtual_network_subnet_name"` @@ -166,6 +167,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "temp_compute_name": &hcldec.AttrSpec{Name: "temp_compute_name", Type: cty.String, Required: false}, "temp_resource_group_name": &hcldec.AttrSpec{Name: "temp_resource_group_name", Type: cty.String, Required: false}, "build_resource_group_name": &hcldec.AttrSpec{Name: "build_resource_group_name", Type: cty.String, Required: false}, + "build_key_vault_name": &hcldec.AttrSpec{Name: "build_key_vault_name", Type: cty.String, Required: false}, "private_virtual_network_with_public_ip": &hcldec.AttrSpec{Name: "private_virtual_network_with_public_ip", Type: cty.Bool, Required: false}, "virtual_network_name": &hcldec.AttrSpec{Name: "virtual_network_name", Type: cty.String, Required: false}, "virtual_network_subnet_name": &hcldec.AttrSpec{Name: "virtual_network_subnet_name", Type: cty.String, Required: false}, diff --git a/website/source/partials/builder/azure/arm/_Config-not-required.html.md b/website/source/partials/builder/azure/arm/_Config-not-required.html.md index 01ab45847..7ed6fe482 100644 --- a/website/source/partials/builder/azure/arm/_Config-not-required.html.md +++ b/website/source/partials/builder/azure/arm/_Config-not-required.html.md @@ -132,6 +132,9 @@ - `build_resource_group_name` (string) - Specify an existing resource group to run the build in. +- `build_key_vault_name` (string) - Specify an existing key vault to use for uploading certificates to the + instance to connect. + - `private_virtual_network_with_public_ip` (bool) - This value allows you to set a virtual_network_name and obtain a public IP. If this value is not set and virtual_network_name is defined Packer is only allowed to be From 2181f10e79b1fdcf7abfd3da462b30930007c57e Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 6 Feb 2020 16:54:07 -0800 Subject: [PATCH 4/4] fix statebag setup; simplify conditional --- builder/azure/arm/builder.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 55e889fe6..c891d3fe1 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -403,14 +403,14 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) { stateBag.Put(constants.ArmKeyVaultDeploymentName, fmt.Sprintf("kv%s", b.config.tmpDeploymentName)) } + stateBag.Put(constants.ArmKeyVaultName, b.config.tmpKeyVaultName) + stateBag.Put(constants.ArmIsExistingKeyVault, false) if b.config.BuildKeyVaultName != "" { stateBag.Put(constants.ArmKeyVaultName, b.config.BuildKeyVaultName) b.config.tmpKeyVaultName = b.config.BuildKeyVaultName - stateBag.Put(constants.ArmIsExistingKeyVault, false) - } else { - stateBag.Put(constants.ArmKeyVaultName, b.config.tmpKeyVaultName) stateBag.Put(constants.ArmIsExistingKeyVault, true) } + stateBag.Put(constants.ArmNicName, b.config.tmpNicName) stateBag.Put(constants.ArmPublicIPAddressName, b.config.tmpPublicIPAddressName) stateBag.Put(constants.ArmResourceGroupName, b.config.BuildResourceGroupName)