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
elastic · Oct 5, 2022 · c061f5f · c061f5f
1 parent b6c5d6d
commit c061f5f
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 1 deletion.
diff --git a/.changelog/546.txt 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/...deployment_elasticsearch_kesytore_test.go → ...deployment_elasticsearch_keystore_test.go b/...deployment_elasticsearch_kesytore_test.go → ...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
@@ -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{