From c061f5ffe1591d12d2e83c9da101ef5ea9877a79 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 5 Oct 2022 19:57:31 +1100 Subject: [PATCH] Actually delete keystore items as requested. (#546) * Actually delete keystore items as requested. Don't just update the value on a copy of the keystore item and them do nothing like a boss * Lint * Add changelog --- .changelog/546.txt | 3 + ...deployment_elasticsearch_keystore_test.go} | 58 +++++++++++++++++++ .../elasticsearchkeystoreresource/delete.go | 4 +- 3 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 .changelog/546.txt rename ec/acc/{deployment_elasticsearch_kesytore_test.go => deployment_elasticsearch_keystore_test.go} (80%) diff --git a/.changelog/546.txt b/.changelog/546.txt new file mode 100644 index 000000000..d29952178 --- /dev/null +++ b/.changelog/546.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/elasticsearchkeystore: Correctly delete keystore items when removed from the module definition. +``` diff --git a/ec/acc/deployment_elasticsearch_kesytore_test.go b/ec/acc/deployment_elasticsearch_keystore_test.go similarity index 80% rename from ec/acc/deployment_elasticsearch_kesytore_test.go rename to ec/acc/deployment_elasticsearch_keystore_test.go index 5870bc2a6..d2e97ece0 100644 --- a/ec/acc/deployment_elasticsearch_kesytore_test.go +++ b/ec/acc/deployment_elasticsearch_keystore_test.go @@ -21,7 +21,9 @@ import ( "fmt" "testing" + "github.com/elastic/cloud-sdk-go/pkg/api/deploymentapi/eskeystoreapi" "github.com/elastic/cloud-sdk-go/pkg/multierror" + "github.com/elastic/cloud-sdk-go/pkg/util/slice" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" @@ -31,6 +33,7 @@ func TestAccDeploymentElasticsearchKeystore_full(t *testing.T) { var previousID, currentID string resType := "ec_deployment_elasticsearch_keystore" + deploymentResName := "ec_deployment.keystore" firstResName := resType + ".test" secondResName := resType + ".gcs_creds" randomName := prefix + acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) @@ -65,6 +68,8 @@ func TestAccDeploymentElasticsearchKeystore_full(t *testing.T) { resource.TestCheckResourceAttr(secondResName, "value", "{\n \"type\": \"service_account\",\n \"project_id\": \"project-id\",\n \"private_key_id\": \"key-id\",\n \"private_key\": \"-----BEGIN PRIVATE KEY-----\\nprivate-key\\n-----END PRIVATE KEY-----\\n\",\n \"client_email\": \"service-account-email\",\n \"client_id\": \"client-id\",\n \"auth_uri\": \"https://accounts.google.com/o/oauth2/auth\",\n \"token_uri\": \"https://accounts.google.com/o/oauth2/token\",\n \"auth_provider_x509_cert_url\": \"https://www.googleapis.com/oauth2/v1/certs\",\n \"client_x509_cert_url\": \"https://www.googleapis.com/robot/v1/metadata/x509/service-account-email\"\n}"), resource.TestCheckResourceAttr(secondResName, "as_file", "false"), resource.TestCheckResourceAttrSet(secondResName, "deployment_id"), + + checkExpectedKeystoreKeysExist(deploymentResName, "xpack.notification.slack.account.hello.secure_url", "gcs.client.secondary.credentials_file"), ), }, { @@ -81,6 +86,8 @@ func TestAccDeploymentElasticsearchKeystore_full(t *testing.T) { resource.TestCheckResourceAttr(secondResName, "value", "{\n \"type\": \"service_account\",\n \"project_id\": \"project-id\",\n \"private_key_id\": \"key-id\",\n \"private_key\": \"-----BEGIN PRIVATE KEY-----\\nprivate-key\\n-----END PRIVATE KEY-----\\n\",\n \"client_email\": \"service-account-email\",\n \"client_id\": \"client-id\",\n \"auth_uri\": \"https://accounts.google.com/o/oauth2/auth\",\n \"token_uri\": \"https://accounts.google.com/o/oauth2/token\",\n \"auth_provider_x509_cert_url\": \"https://www.googleapis.com/oauth2/v1/certs\",\n \"client_x509_cert_url\": \"https://www.googleapis.com/robot/v1/metadata/x509/service-account-email\"\n}"), resource.TestCheckResourceAttr(secondResName, "as_file", "false"), resource.TestCheckResourceAttrSet(secondResName, "deployment_id"), + + checkExpectedKeystoreKeysExist(deploymentResName, "xpack.notification.slack.account.hello.secure_url", "gcs.client.secondary.credentials_file"), ), }, { @@ -97,6 +104,8 @@ func TestAccDeploymentElasticsearchKeystore_full(t *testing.T) { resource.TestCheckResourceAttr(secondResName, "value", "{\n \"type\": \"service_account\",\n \"project_id\": \"project-id\",\n \"private_key_id\": \"key-id\",\n \"private_key\": \"-----BEGIN PRIVATE KEY-----\\nprivate-key\\n-----END PRIVATE KEY-----\\n\",\n \"client_email\": \"service-account-email\",\n \"client_id\": \"client-id\",\n \"auth_uri\": \"https://accounts.google.com/o/oauth2/auth\",\n \"token_uri\": \"https://accounts.google.com/o/oauth2/token\",\n \"auth_provider_x509_cert_url\": \"https://www.googleapis.com/oauth2/v1/certs\",\n \"client_x509_cert_url\": \"https://www.googleapis.com/robot/v1/metadata/x509/service-account-email\"\n}"), resource.TestCheckResourceAttr(secondResName, "as_file", "false"), resource.TestCheckResourceAttrSet(secondResName, "deployment_id"), + + checkExpectedKeystoreKeysExist(deploymentResName, "xpack.notification.slack.account.hello.secure_urla", "gcs.client.secondary.credentials_file"), ), }, { @@ -117,6 +126,55 @@ func TestAccDeploymentElasticsearchKeystore_full(t *testing.T) { }) } +func checkExpectedKeystoreKeysExist(deploymentResource string, expectedKeys ...string) resource.TestCheckFunc { + return func(s *terraform.State) error { + client, err := newAPI() + if err != nil { + return err + } + + deployment, ok := s.RootModule().Resources[deploymentResource] + if !ok { + return fmt.Errorf("Not found: %s", deploymentResource) + } + + deploymentID := deployment.Primary.ID + + keystoreContents, err := eskeystoreapi.Get(eskeystoreapi.GetParams{ + API: client, + DeploymentID: deploymentID, + }) + if err != nil { + return err + } + + var missingKeys, extraKeys []string + for _, expectedKey := range expectedKeys { + if _, ok := keystoreContents.Secrets[expectedKey]; !ok { + missingKeys = append(missingKeys, expectedKey) + } + } + + for key := range keystoreContents.Secrets { + if !slice.HasString(expectedKeys, key) { + extraKeys = append(extraKeys, key) + } + } + + mErr := multierror.NewPrefixed("unexpected keystore contents") + + if len(missingKeys) > 0 { + mErr = mErr.Append(fmt.Errorf("keys missing from the deployment keystore %v", missingKeys)) + } + + if len(extraKeys) > 0 { + mErr = mErr.Append(fmt.Errorf("extra keys present in the deployment keystore: %v", extraKeys)) + } + + return mErr.ErrorOrNil() + } +} + func checkESKeystoreResourceID(resourceName string, id *string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] diff --git a/ec/ecresource/elasticsearchkeystoreresource/delete.go b/ec/ecresource/elasticsearchkeystoreresource/delete.go index b6c6d7e63..a4fcd145f 100644 --- a/ec/ecresource/elasticsearchkeystoreresource/delete.go +++ b/ec/ecresource/elasticsearchkeystoreresource/delete.go @@ -31,11 +31,13 @@ import ( func delete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { client := meta.(*api.API) contents := expandModel(d) + settingName := d.Get("setting_name").(string) // Since we're using the Update API (PATCH method), we need to se the Value // field to nil for the keystore setting to be unset. - if secret, ok := contents.Secrets[d.Get("setting_name").(string)]; ok { + if secret, ok := contents.Secrets[settingName]; ok { secret.Value = nil + contents.Secrets[settingName] = secret } if _, err := eskeystoreapi.Update(eskeystoreapi.UpdateParams{