From fa700a2aba96eb647e5b40d7840337b8fbc8dbf4 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 13 Apr 2022 13:17:37 -0400 Subject: [PATCH 01/17] es/domain: Enable encrypt w/o ForceNew --- internal/service/elasticsearch/domain.go | 53 ++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/internal/service/elasticsearch/domain.go b/internal/service/elasticsearch/domain.go index 458bb05c5ec3..a54ddfa15445 100644 --- a/internal/service/elasticsearch/domain.go +++ b/internal/service/elasticsearch/domain.go @@ -65,6 +65,33 @@ func ResourceDomain() *schema.Resource { } return true }), + customdiff.ForceNewIf("encrypt_at_rest.0.enabled", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { + newVersion := d.Get("elasticsearch_version").(string) + + if newVersion < "6.7" { + return true + } + + return false + }), + customdiff.ForceNewIf("encrypt_at_rest.0.kms_key_id", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { + newVersion := d.Get("elasticsearch_version").(string) + + if newVersion < "6.7" { + return true + } + + return false + }), + customdiff.ForceNewIf("node_to_node_encryption.0.enabled", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { + newVersion := d.Get("elasticsearch_version").(string) + + if newVersion < "6.7" { + return true + } + + return false + }), verify.SetTagsDiff, ), @@ -398,13 +425,11 @@ func ResourceDomain() *schema.Resource { "enabled": { Type: schema.TypeBool, Required: true, - ForceNew: true, }, "kms_key_id": { Type: schema.TypeString, Optional: true, Computed: true, - ForceNew: true, DiffSuppressFunc: suppressEquivalentKmsKeyIds, }, }, @@ -451,7 +476,6 @@ func ResourceDomain() *schema.Resource { "enabled": { Type: schema.TypeBool, Required: true, - ForceNew: true, }, }, }, @@ -892,6 +916,29 @@ func resourceDomainUpdate(d *schema.ResourceData, meta interface{}) error { } } + if d.HasChange("encrypt_at_rest") { + input.EncryptionAtRestOptions = nil + if v, ok := d.GetOk("encrypt_at_rest"); ok { + options := v.([]interface{}) + if options[0] == nil { + return fmt.Errorf("At least one field is expected inside encrypt_at_rest") + } + + s := options[0].(map[string]interface{}) + input.EncryptionAtRestOptions = expandEncryptAtRestOptions(s) + } + } + + if d.HasChange("node_to_node_encryption") { + input.NodeToNodeEncryptionOptions = nil + if v, ok := d.GetOk("node_to_node_encryption"); ok { + options := v.([]interface{}) + + s := options[0].(map[string]interface{}) + input.NodeToNodeEncryptionOptions = expandNodeToNodeEncryptionOptions(s) + } + } + if d.HasChange("snapshot_options") { options := d.Get("snapshot_options").([]interface{}) From 81ffe0f08f2e28f914562f5c30cbb6de1efff51f Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 13 Apr 2022 13:21:49 -0400 Subject: [PATCH 02/17] Add changelog --- .changelog/24222.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/24222.txt diff --git a/.changelog/24222.txt b/.changelog/24222.txt new file mode 100644 index 000000000000..7d2b9beb14af --- /dev/null +++ b/.changelog/24222.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_elasticsearch_domain: For Elasticsearch versions 6.7+, allow in-place update of `node_to_node_encryption.0.enabled`, `encrypt_at_rest.0.enabled`, and `encrypt_at_rest.0.kms_key_id` +``` \ No newline at end of file From e5e111895fc1136469da2963a8d7526d196462c6 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 13 Apr 2022 14:22:37 -0400 Subject: [PATCH 03/17] Add test showing in place enable --- internal/service/elasticsearch/domain_test.go | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/internal/service/elasticsearch/domain_test.go b/internal/service/elasticsearch/domain_test.go index 88441aea2c02..1b0f79f1542b 100644 --- a/internal/service/elasticsearch/domain_test.go +++ b/internal/service/elasticsearch/domain_test.go @@ -1017,7 +1017,7 @@ func TestAccElasticsearchDomain_EncryptAtRestDefault_key(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy, Steps: []resource.TestStep{ { - Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName), + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, true), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain), testAccCheckDomainEncrypted(true, &domain), @@ -1097,6 +1097,36 @@ func TestAccElasticsearchDomain_nodeToNodeEncryption(t *testing.T) { }) } +func TestAccElasticsearchDomain_Encryption_atRestEnable(t *testing.T) { + var domain1, domain2 elasticsearch.ElasticsearchDomainStatus + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_elasticsearch_domain.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIamServiceLinkedRoleEs(t) }, + ErrorCheck: acctest.ErrorCheck(t, elasticsearch.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDomainDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckDomainEncrypted(false, &domain1), + ), + }, + { + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain2), + testAccCheckDomainEncrypted(true, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), + ), + }, + }, + }) +} + func TestAccElasticsearchDomain_tags(t *testing.T) { if testing.Short() { t.Skip("skipping long-running test in short mode") @@ -2126,12 +2156,12 @@ data "aws_iam_policy_document" "test" { `, rName) } -func testAccDomainConfigWithEncryptAtRestDefaultKey(rName string) string { +func testAccDomainConfigWithEncryptAtRestDefaultKey(rName string, enabled bool) string { return fmt.Sprintf(` resource "aws_elasticsearch_domain" "test" { domain_name = substr(%[1]q, 0, 28) - elasticsearch_version = "6.0" + elasticsearch_version = "6.7" # Encrypt at rest requires m4/c4/r4/i2 instances. See http://docs.aws.amazon.com/elasticsearch-service/latest/developerguide/aes-supported-instance-types.html cluster_config { @@ -2144,10 +2174,10 @@ resource "aws_elasticsearch_domain" "test" { } encrypt_at_rest { - enabled = true + enabled = %[2]t } } -`, rName) +`, rName, enabled) } func testAccDomainConfigWithEncryptAtRestWithKey(rName string) string { From 8609985c2df4713a4b93500a6255438413d4e054 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 20 Apr 2022 14:33:52 -0400 Subject: [PATCH 04/17] Update docs --- website/docs/r/elasticsearch_domain.html.markdown | 6 +++++- website/docs/r/opensearch_domain.html.markdown | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/website/docs/r/elasticsearch_domain.html.markdown b/website/docs/r/elasticsearch_domain.html.markdown index 872ceb968759..27b5077d8903 100644 --- a/website/docs/r/elasticsearch_domain.html.markdown +++ b/website/docs/r/elasticsearch_domain.html.markdown @@ -299,8 +299,10 @@ AWS documentation: [Amazon Cognito Authentication for Kibana](https://docs.aws.a ### encrypt_at_rest +~> **Note:** You can enable `encrypt_at_rest` in place if your Elasticsearch version is 6.7 or greater. If you disable `encrypt_rest_rest`, once you enable it, for any version, or enable for 6.6 and below, Terraform will recreate the domain, which can cause data loss. Similarly, you can change the `kms_key_id` in place if your version is 6.7 or great. Otherwise, Terraform will recreate the domain, which can cause data loss. + * `enabled` - (Required) Whether to enable encryption at rest. If the `encrypt_at_rest` block is not provided then this defaults to `false`. -* `kms_key_id` - (Optional) KMS key id to encrypt the Elasticsearch domain with. If not specified then it defaults to using the `aws/es` service KMS key. +* `kms_key_id` - (Optional) KMS key ARN to encrypt the Elasticsearch domain with. If not specified then it defaults to using the `aws/es` service KMS key. Note that KMS will accept a KMS key ID but will return the key ARN. To prevent Terraform detecting unwanted changes, use the key ARN instead. ### log_publishing_options @@ -310,6 +312,8 @@ AWS documentation: [Amazon Cognito Authentication for Kibana](https://docs.aws.a ### node_to_node_encryption +~> **Note:** You can enable `node_to_node_encryption` in place if your Elasticsearch version is 6.7 or greater. If you disable `node_to_node_encryption`, once you enable it, for any version, or enable for 6.6 and below, Terraform will recreate the domain causing data loss. + * `enabled` - (Required) Whether to enable node-to-node encryption. If the `node_to_node_encryption` block is not provided then this defaults to `false`. ### snapshot_options diff --git a/website/docs/r/opensearch_domain.html.markdown b/website/docs/r/opensearch_domain.html.markdown index 09e857839656..49aca1a5ddb7 100644 --- a/website/docs/r/opensearch_domain.html.markdown +++ b/website/docs/r/opensearch_domain.html.markdown @@ -316,8 +316,10 @@ AWS documentation: [Amazon Cognito Authentication for Kibana](https://docs.aws.a ### encrypt_at_rest +~> **Note:** You can enable `encrypt_at_rest` in place if your Elasticsearch version is 6.7 or greater. If you disable `encrypt_rest_rest`, once you enable it, for any version, or enable for 6.6 and below, Terraform will recreate the domain, which can cause data loss. Similarly, you can change the `kms_key_id` in place if your version is 6.7 or great. Otherwise, Terraform will recreate the domain, which can cause data loss. + * `enabled` - (Required) Whether to enable encryption at rest. If the `encrypt_at_rest` block is not provided then this defaults to `false`. -* `kms_key_id` - (Optional) KMS key id to encrypt the OpenSearch domain with. If not specified then it defaults to using the `aws/es` service KMS key. +* `kms_key_id` - (Optional) KMS key ARN to encrypt the Elasticsearch domain with. If not specified then it defaults to using the `aws/es` service KMS key. Note that KMS will accept a KMS key ID but will return the key ARN. To prevent Terraform detecting unwanted changes, use the key ARN instead. ### log_publishing_options From 0b59af79c1e9cc4ccf720faafabd71c0b9f8b4b8 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 20 Apr 2022 14:34:24 -0400 Subject: [PATCH 05/17] Refine, troubleshoot logic --- internal/service/elasticsearch/domain.go | 49 +++- internal/service/elasticsearch/domain_test.go | 274 +++++++++++++++--- internal/service/opensearch/domain.go | 68 ++++- internal/service/opensearch/domain_test.go | 210 ++++++++++++-- 4 files changed, 518 insertions(+), 83 deletions(-) diff --git a/internal/service/elasticsearch/domain.go b/internal/service/elasticsearch/domain.go index a54ddfa15445..23c7b38d67df 100644 --- a/internal/service/elasticsearch/domain.go +++ b/internal/service/elasticsearch/domain.go @@ -66,31 +66,32 @@ func ResourceDomain() *schema.Resource { return true }), customdiff.ForceNewIf("encrypt_at_rest.0.enabled", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { - newVersion := d.Get("elasticsearch_version").(string) - - if newVersion < "6.7" { + // cannot disable (at all) or enable if < 6.7 without forcenew + o, n := d.GetChange("encrypt_at_rest.0.enabled") + fmt.Printf("ear - old: %t, new: %t\n", o.(bool), n.(bool)) + if o.(bool) && !n.(bool) { return true } - - return false + fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string))) + return inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string)) }), customdiff.ForceNewIf("encrypt_at_rest.0.kms_key_id", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { - newVersion := d.Get("elasticsearch_version").(string) - - if newVersion < "6.7" { - return true + // cannot change if < 6.7 without forcenew + o, n := d.GetChange("encrypt_at_rest.0.kms_key_id") + if o.(string) != "" && n.(string) != "" && (strings.HasSuffix(o.(string), n.(string)) || strings.HasSuffix(n.(string), o.(string))) { + return false } - - return false + fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string))) + return inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string)) }), customdiff.ForceNewIf("node_to_node_encryption.0.enabled", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { - newVersion := d.Get("elasticsearch_version").(string) - - if newVersion < "6.7" { + o, n := d.GetChange("node_to_node_encryption.0.enabled") + fmt.Printf("ntne - old: %t, new: %t\n", o.(bool), n.(bool)) + if o.(bool) && !n.(bool) { return true } - - return false + fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string))) + return inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string)) }), verify.SetTagsDiff, ), @@ -1012,6 +1013,8 @@ func resourceDomainUpdate(d *schema.ResourceData, meta interface{}) error { func resourceDomainDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).ElasticsearchConn + fmt.Printf("deleting %s\n", d.Id()) + name := d.Get("domain_name").(string) log.Printf("[DEBUG] Deleting Elasticsearch Domain: %s", d.Id()) @@ -1050,6 +1053,20 @@ func resourceDomainImport(d *schema.ResourceData, meta interface{}) ([]*schema.R return []*schema.ResourceData{d}, nil } +func inPlaceEncryptionEnableVersion(version string) bool { + var want, got *gversion.Version + var err error + if want, err = gversion.NewVersion("6.7"); err != nil { + return false + } + + if got, err = gversion.NewVersion(version); err != nil || got.GreaterThanOrEqual(want) { + return false + } + + return true +} + func suppressEquivalentKmsKeyIds(k, old, new string, d *schema.ResourceData) bool { // The Elasticsearch API accepts a short KMS key id but always returns the ARN of the key. // The ARN is of the format 'arn:aws:kms:REGION:ACCOUNT_ID:key/KMS_KEY_ID'. diff --git a/internal/service/elasticsearch/domain_test.go b/internal/service/elasticsearch/domain_test.go index 1b0f79f1542b..12eda372939c 100644 --- a/internal/service/elasticsearch/domain_test.go +++ b/internal/service/elasticsearch/domain_test.go @@ -1001,7 +1001,7 @@ func TestAccElasticsearchDomain_policyIgnoreEquivalent(t *testing.T) { }) } -func TestAccElasticsearchDomain_EncryptAtRestDefault_key(t *testing.T) { +func TestAccElasticsearchDomain_Encryption_atRestDefaultKey(t *testing.T) { if testing.Short() { t.Skip("skipping long-running test in short mode") } @@ -1017,7 +1017,7 @@ func TestAccElasticsearchDomain_EncryptAtRestDefault_key(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy, Steps: []resource.TestStep{ { - Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, true), + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "6.0", true), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain), testAccCheckDomainEncrypted(true, &domain), @@ -1033,7 +1033,7 @@ func TestAccElasticsearchDomain_EncryptAtRestDefault_key(t *testing.T) { }) } -func TestAccElasticsearchDomain_EncryptAtRestSpecify_key(t *testing.T) { +func TestAccElasticsearchDomain_Encryption_atRestSpecifyKey(t *testing.T) { if testing.Short() { t.Skip("skipping long-running test in short mode") } @@ -1049,7 +1049,7 @@ func TestAccElasticsearchDomain_EncryptAtRestSpecify_key(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy, Steps: []resource.TestStep{ { - Config: testAccDomainConfigWithEncryptAtRestWithKey(rName), + Config: testAccDomainConfigWithEncryptAtRestWithKey(rName, "6.0", true), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain), testAccCheckDomainEncrypted(true, &domain), @@ -1065,7 +1065,7 @@ func TestAccElasticsearchDomain_EncryptAtRestSpecify_key(t *testing.T) { }) } -func TestAccElasticsearchDomain_nodeToNodeEncryption(t *testing.T) { +func TestAccElasticsearchDomain_Encryption_nodeToNode(t *testing.T) { if testing.Short() { t.Skip("skipping long-running test in short mode") } @@ -1081,7 +1081,7 @@ func TestAccElasticsearchDomain_nodeToNodeEncryption(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy, Steps: []resource.TestStep{ { - Config: testAccDomainConfigwithNodeToNodeEncryption(rName), + Config: testAccDomainConfigwithNodeToNodeEncryption(rName, "6.0", true), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain), testAccCheckNodeToNodeEncrypted(true, &domain), @@ -1098,6 +1098,95 @@ func TestAccElasticsearchDomain_nodeToNodeEncryption(t *testing.T) { } func TestAccElasticsearchDomain_Encryption_atRestEnable(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var domain1, domain2 elasticsearch.ElasticsearchDomainStatus + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_elasticsearch_domain.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIamServiceLinkedRoleEs(t) }, + ErrorCheck: acctest.ErrorCheck(t, elasticsearch.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDomainDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "6.7", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckDomainEncrypted(false, &domain1), + ), + }, + { + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "6.7", true), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain2), + testAccCheckDomainEncrypted(true, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), + ), + }, + { + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "6.7", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckDomainEncrypted(false, &domain1), + testAccCheckDomainNotRecreated(&domain1, &domain2), + ), + }, + }, + }) +} + +func TestAccElasticsearchDomain_Encryption_atRestSwitchKey(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var domain1, domain2 elasticsearch.ElasticsearchDomainStatus + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_elasticsearch_domain.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIamServiceLinkedRoleEs(t) }, + ErrorCheck: acctest.ErrorCheck(t, elasticsearch.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDomainDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "6.7", true), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain2), + testAccCheckDomainEncrypted(true, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), + ), + }, + { + Config: testAccDomainConfigWithEncryptAtRestWithKey(rName, "6.7", true), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain2), + testAccCheckDomainEncrypted(true, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), + ), + }, + { + Config: testAccDomainConfigWithEncryptAtRestWithNewKey(rName, "6.7", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckDomainEncrypted(false, &domain1), + testAccCheckDomainNotRecreated(&domain1, &domain2), + ), + }, + }, + }) +} + +func TestAccElasticsearchDomain_Encryption_atRestEnableLegacy(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + var domain1, domain2 elasticsearch.ElasticsearchDomainStatus rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_elasticsearch_domain.test" @@ -1109,20 +1198,103 @@ func TestAccElasticsearchDomain_Encryption_atRestEnable(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy, Steps: []resource.TestStep{ { - Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, false), + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "5.6", false), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain1), testAccCheckDomainEncrypted(false, &domain1), ), }, { - Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, true), + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "5.6", true), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), testAccCheckDomainEncrypted(true, &domain2), + ), + }, + }, + }) +} + +func TestAccElasticsearchDomain_Encryption_nodeToNodeEnable(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var domain1, domain2 elasticsearch.ElasticsearchDomainStatus + resourceName := "aws_elasticsearch_domain.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIamServiceLinkedRoleEs(t) }, + ErrorCheck: acctest.ErrorCheck(t, elasticsearch.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDomainDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDomainConfigwithNodeToNodeEncryption(rName, "6.7", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckNodeToNodeEncrypted(false, &domain1), + ), + }, + { + Config: testAccDomainConfigwithNodeToNodeEncryption(rName, "6.7", true), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain2), + testAccCheckNodeToNodeEncrypted(true, &domain2), testAccCheckDomainNotRecreated(&domain1, &domain2), ), }, + { + Config: testAccDomainConfigwithNodeToNodeEncryption(rName, "6.7", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckNodeToNodeEncrypted(false, &domain1), + testAccCheckDomainNotRecreated(&domain1, &domain2), + ), + }, + }, + }) +} + +func TestAccElasticsearchDomain_Encryption_nodeToNodeEnableLegacy(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var domain1, domain2 elasticsearch.ElasticsearchDomainStatus + resourceName := "aws_elasticsearch_domain.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIamServiceLinkedRoleEs(t) }, + ErrorCheck: acctest.ErrorCheck(t, elasticsearch.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDomainDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDomainConfigwithNodeToNodeEncryption(rName, "6.0", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckNodeToNodeEncrypted(false, &domain1), + ), + }, + { + Config: testAccDomainConfigwithNodeToNodeEncryption(rName, "6.0", true), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain2), + testAccCheckNodeToNodeEncrypted(true, &domain2), + testAccCheckDomainRecreated(&domain1, &domain2), + ), + }, + { + Config: testAccDomainConfigwithNodeToNodeEncryption(rName, "6.0", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckNodeToNodeEncrypted(false, &domain1), + testAccCheckDomainRecreated(&domain1, &domain2), + ), + }, }, }) } @@ -1536,25 +1708,20 @@ func testAccCheckDomainExists(n string, domain *elasticsearch.ElasticsearchDomai } } -func testAccCheckDomainNotRecreated(i, j *elasticsearch.ElasticsearchDomainStatus) resource.TestCheckFunc { +func testAccCheckDomainRecreated(i, j *elasticsearch.ElasticsearchDomainStatus) resource.TestCheckFunc { return func(s *terraform.State) error { - conn := acctest.Provider.Meta().(*conns.AWSClient).ElasticsearchConn - - iConfig, err := conn.DescribeElasticsearchDomainConfig(&elasticsearch.DescribeElasticsearchDomainConfigInput{ - DomainName: i.DomainName, - }) - if err != nil { - return err - } - jConfig, err := conn.DescribeElasticsearchDomainConfig(&elasticsearch.DescribeElasticsearchDomainConfigInput{ - DomainName: j.DomainName, - }) - if err != nil { - return err + if aws.StringValue(i.Endpoint) == aws.StringValue(j.Endpoint) { + return fmt.Errorf("domain (%s) was not recreated, before endpoint (%s), after endpoint (%s)", aws.StringValue(i.DomainName), aws.StringValue(i.Endpoint), aws.StringValue(j.Endpoint)) } - if !aws.TimeValue(iConfig.DomainConfig.ElasticsearchClusterConfig.Status.CreationDate).Equal(aws.TimeValue(jConfig.DomainConfig.ElasticsearchClusterConfig.Status.CreationDate)) { - return fmt.Errorf("ES Domain was recreated") + return nil + } +} + +func testAccCheckDomainNotRecreated(i, j *elasticsearch.ElasticsearchDomainStatus) resource.TestCheckFunc { + return func(s *terraform.State) error { + if aws.StringValue(i.Endpoint) != aws.StringValue(j.Endpoint) { + return fmt.Errorf("domain (%s) was recreated, before endpoint (%s), after endpoint (%s)", aws.StringValue(i.DomainName), aws.StringValue(i.Endpoint), aws.StringValue(j.Endpoint)) } return nil @@ -2156,12 +2323,12 @@ data "aws_iam_policy_document" "test" { `, rName) } -func testAccDomainConfigWithEncryptAtRestDefaultKey(rName string, enabled bool) string { +func testAccDomainConfigWithEncryptAtRestDefaultKey(rName, version string, enabled bool) string { return fmt.Sprintf(` resource "aws_elasticsearch_domain" "test" { domain_name = substr(%[1]q, 0, 28) - elasticsearch_version = "6.7" + elasticsearch_version = %[2]q # Encrypt at rest requires m4/c4/r4/i2 instances. See http://docs.aws.amazon.com/elasticsearch-service/latest/developerguide/aes-supported-instance-types.html cluster_config { @@ -2174,13 +2341,13 @@ resource "aws_elasticsearch_domain" "test" { } encrypt_at_rest { - enabled = %[2]t + enabled = %[3]t } } -`, rName, enabled) +`, rName, version, enabled) } -func testAccDomainConfigWithEncryptAtRestWithKey(rName string) string { +func testAccDomainConfigWithEncryptAtRestWithKey(rName, version string, enabled bool) string { return fmt.Sprintf(` resource "aws_kms_key" "test" { description = %[1]q @@ -2190,7 +2357,7 @@ resource "aws_kms_key" "test" { resource "aws_elasticsearch_domain" "test" { domain_name = substr(%[1]q, 0, 28) - elasticsearch_version = "6.0" + elasticsearch_version = %[2]q # Encrypt at rest requires m4/c4/r4/i2 instances. See http://docs.aws.amazon.com/elasticsearch-service/latest/developerguide/aes-supported-instance-types.html cluster_config { @@ -2203,19 +2370,54 @@ resource "aws_elasticsearch_domain" "test" { } encrypt_at_rest { - enabled = true + enabled = %[3]t kms_key_id = aws_kms_key.test.key_id } } -`, rName) +`, rName, version, enabled) +} + +func testAccDomainConfigWithEncryptAtRestWithNewKey(rName, version string, enabled bool) string { + return fmt.Sprintf(` +resource "aws_kms_key" "test" { + description = %[1]q + deletion_window_in_days = 7 +} + +resource "aws_kms_key" "test2" { + description = %[1]q + deletion_window_in_days = 7 } -func testAccDomainConfigwithNodeToNodeEncryption(rName string) string { +resource "aws_elasticsearch_domain" "test" { + domain_name = substr(%[1]q, 0, 28) + + elasticsearch_version = %[2]q + + # Encrypt at rest requires m4/c4/r4/i2 instances. See http://docs.aws.amazon.com/elasticsearch-service/latest/developerguide/aes-supported-instance-types.html + cluster_config { + instance_type = "m4.large.elasticsearch" + } + + ebs_options { + ebs_enabled = true + volume_size = 10 + } + + encrypt_at_rest { + enabled = %[3]t + kms_key_id = aws_kms_key.test2.key_id + } +} +`, rName, version, enabled) +} + +func testAccDomainConfigwithNodeToNodeEncryption(rName, version string, enabled bool) string { return fmt.Sprintf(` resource "aws_elasticsearch_domain" "test" { domain_name = substr(%[1]q, 0, 28) - elasticsearch_version = "6.0" + elasticsearch_version = %[2]q cluster_config { instance_type = "m4.large.elasticsearch" @@ -2227,10 +2429,10 @@ resource "aws_elasticsearch_domain" "test" { } node_to_node_encryption { - enabled = true + enabled = %[3]t } } -`, rName) +`, rName, version, enabled) } func testAccDomainConfig_complex(rName string) string { diff --git a/internal/service/opensearch/domain.go b/internal/service/opensearch/domain.go index ed02d3c7863a..109d5b73cbf4 100644 --- a/internal/service/opensearch/domain.go +++ b/internal/service/opensearch/domain.go @@ -65,6 +65,29 @@ func ResourceDomain() *schema.Resource { } return true }), + customdiff.ForceNewIf("encrypt_at_rest.0.enabled", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { + o, n := d.GetChange("encrypt_at_rest.0.enabled") + fmt.Printf("ear - old: %t, new: %t\n", o.(bool), n.(bool)) + if o.(bool) && !n.(bool) { + return true + } + fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("engine_version").(string))) + return inPlaceEncryptionEnableVersion(d.Get("engine_version").(string)) + }), + customdiff.ForceNewIf("encrypt_at_rest.0.kms_key_id", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { + // cannot change if < 6.7 without forcenew + fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("engine_version").(string))) + return inPlaceEncryptionEnableVersion(d.Get("engine_version").(string)) + }), + customdiff.ForceNewIf("node_to_node_encryption.0.enabled", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { + o, n := d.GetChange("node_to_node_encryption.0.enabled") + fmt.Printf("ntne - old: %t, new: %t\n", o.(bool), n.(bool)) + if o.(bool) && !n.(bool) { + return true + } + fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("engine_version").(string))) + return inPlaceEncryptionEnableVersion(d.Get("engine_version").(string)) + }), verify.SetTagsDiff, ), @@ -393,13 +416,11 @@ func ResourceDomain() *schema.Resource { "enabled": { Type: schema.TypeBool, Required: true, - ForceNew: true, }, "kms_key_id": { Type: schema.TypeString, Optional: true, Computed: true, - ForceNew: true, DiffSuppressFunc: suppressEquivalentKmsKeyIds, }, }, @@ -451,7 +472,6 @@ func ResourceDomain() *schema.Resource { "enabled": { Type: schema.TypeBool, Required: true, - ForceNew: true, }, }, }, @@ -934,6 +954,29 @@ func resourceDomainUpdate(d *schema.ResourceData, meta interface{}) error { } } + if d.HasChange("encrypt_at_rest") { + input.EncryptionAtRestOptions = nil + if v, ok := d.GetOk("encrypt_at_rest"); ok { + options := v.([]interface{}) + if options[0] == nil { + return fmt.Errorf("At least one field is expected inside encrypt_at_rest") + } + + s := options[0].(map[string]interface{}) + input.EncryptionAtRestOptions = expandEncryptAtRestOptions(s) + } + } + + if d.HasChange("node_to_node_encryption") { + input.NodeToNodeEncryptionOptions = nil + if v, ok := d.GetOk("node_to_node_encryption"); ok { + options := v.([]interface{}) + + s := options[0].(map[string]interface{}) + input.NodeToNodeEncryptionOptions = expandNodeToNodeEncryptionOptions(s) + } + } + if d.HasChange("snapshot_options") { options := d.Get("snapshot_options").([]interface{}) @@ -1023,6 +1066,25 @@ func resourceDomainDelete(d *schema.ResourceData, meta interface{}) error { return nil } +func inPlaceEncryptionEnableVersion(version string) bool { + if strings.HasPrefix(strings.ToLower(version), "opensearch") { + return false + } + + version = strings.TrimPrefix(strings.ToLower(version), "elasticsearch_") + var want, got *gversion.Version + var err error + if want, err = gversion.NewVersion("6.7"); err != nil { + return false + } + + if got, err = gversion.NewVersion(version); err != nil || got.GreaterThanOrEqual(want) { + return false + } + + return true +} + func suppressEquivalentKmsKeyIds(k, old, new string, d *schema.ResourceData) bool { // The OpenSearch API accepts a short KMS key id but always returns the ARN of the key. // The ARN is of the format 'arn:aws:kms:REGION:ACCOUNT_ID:key/KMS_KEY_ID'. diff --git a/internal/service/opensearch/domain_test.go b/internal/service/opensearch/domain_test.go index 7047ad59d812..d14a577ace29 100644 --- a/internal/service/opensearch/domain_test.go +++ b/internal/service/opensearch/domain_test.go @@ -1111,7 +1111,7 @@ func TestAccOpenSearchDomain_Encryption_atRestDefaultKey(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy, Steps: []resource.TestStep{ { - Config: testAccDomainConfig_encryptAtRestDefaultKey(rName), + Config: testAccDomainConfig_encryptAtRestDefaultKey(rName, "Elasticsearch_6.0", true), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain), testAccCheckDomainEncrypted(true, &domain), @@ -1175,7 +1175,7 @@ func TestAccOpenSearchDomain_Encryption_nodeToNode(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy, Steps: []resource.TestStep{ { - Config: testAccDomainConfig_nodeToNodeEncryption(rName), + Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_6.0", true), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain), testAccCheckNodeToNodeEncrypted(true, &domain), @@ -1191,6 +1191,166 @@ func TestAccOpenSearchDomain_Encryption_nodeToNode(t *testing.T) { }) } +func TestAccOpenSearchDomain_Encryption_atRestEnable(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var domain1, domain2 opensearchservice.DomainStatus + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_opensearch_domain.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIAMServiceLinkedRoleOpenSearch(t) }, + ErrorCheck: acctest.ErrorCheck(t, opensearchservice.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDomainDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDomainConfig_encryptAtRestDefaultKey(rName, "OpenSearch_1.1", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckDomainEncrypted(false, &domain1), + ), + }, + { + Config: testAccDomainConfig_encryptAtRestDefaultKey(rName, "OpenSearch_1.1", true), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain2), + testAccCheckDomainEncrypted(true, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), + ), + }, + { + Config: testAccDomainConfig_encryptAtRestDefaultKey(rName, "OpenSearch_1.1", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckDomainEncrypted(false, &domain1), + testAccCheckDomainRecreated(&domain1, &domain2), + ), + }, + }, + }) +} + +func TestAccOpenSearchDomain_Encryption_atRestEnableLegacy(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var domain1, domain2 opensearchservice.DomainStatus + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_opensearch_domain.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIAMServiceLinkedRoleOpenSearch(t) }, + ErrorCheck: acctest.ErrorCheck(t, opensearchservice.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDomainDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDomainConfig_encryptAtRestDefaultKey(rName, "Elasticsearch_5.6", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckDomainEncrypted(false, &domain1), + ), + }, + { + Config: testAccDomainConfig_encryptAtRestDefaultKey(rName, "Elasticsearch_5.6", true), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain2), + testAccCheckDomainEncrypted(true, &domain2), + testAccCheckDomainRecreated(&domain1, &domain2), + ), + }, + }, + }) +} + +func TestAccOpenSearchDomain_Encryption_nodeToNodeEnable(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var domain1, domain2 opensearchservice.DomainStatus + resourceName := "aws_opensearch_domain.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIAMServiceLinkedRoleOpenSearch(t) }, + ErrorCheck: acctest.ErrorCheck(t, opensearchservice.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDomainDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDomainConfig_nodeToNodeEncryption(rName, "OpenSearch_1.1", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckNodeToNodeEncrypted(false, &domain1), + ), + }, + { + Config: testAccDomainConfig_nodeToNodeEncryption(rName, "OpenSearch_1.1", true), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain2), + testAccCheckNodeToNodeEncrypted(true, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), + ), + }, + { + Config: testAccDomainConfig_nodeToNodeEncryption(rName, "OpenSearch_1.1", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckNodeToNodeEncrypted(false, &domain1), + testAccCheckDomainRecreated(&domain1, &domain2), + ), + }, + }, + }) +} + +func TestAccOpenSearchDomain_Encryption_nodeToNodeEnableLegacy(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var domain1, domain2 opensearchservice.DomainStatus + resourceName := "aws_opensearch_domain.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIAMServiceLinkedRoleOpenSearch(t) }, + ErrorCheck: acctest.ErrorCheck(t, opensearchservice.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDomainDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_5.6", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckNodeToNodeEncrypted(false, &domain1), + ), + }, + { + Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_5.6", true), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain2), + testAccCheckNodeToNodeEncrypted(true, &domain2), + testAccCheckDomainRecreated(&domain1, &domain2), + ), + }, + { + Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_5.6", false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckNodeToNodeEncrypted(false, &domain1), + testAccCheckDomainRecreated(&domain1, &domain2), + ), + }, + }, + }) +} + func TestAccOpenSearchDomain_tags(t *testing.T) { if testing.Short() { t.Skip("skipping long-running test in short mode") @@ -1484,7 +1644,7 @@ func testAccCheckNumberOfInstances(numberOfInstances int, status *opensearchserv func testAccCheckDomainEncrypted(encrypted bool, status *opensearchservice.DomainStatus) resource.TestCheckFunc { return func(s *terraform.State) error { conf := status.EncryptionAtRestOptions - if *conf.Enabled != encrypted { + if aws.BoolValue(conf.Enabled) != encrypted { return fmt.Errorf("Encrypt at rest not set properly. Given: %t, Expected: %t", *conf.Enabled, encrypted) } return nil @@ -1560,25 +1720,20 @@ func testAccCheckDomainExists(n string, domain *opensearchservice.DomainStatus) } } -func testAccCheckDomainNotRecreated(i, j *opensearchservice.DomainStatus) resource.TestCheckFunc { +func testAccCheckDomainRecreated(i, j *opensearchservice.DomainStatus) resource.TestCheckFunc { return func(s *terraform.State) error { - conn := acctest.Provider.Meta().(*conns.AWSClient).OpenSearchConn - - iConfig, err := conn.DescribeDomainConfig(&opensearchservice.DescribeDomainConfigInput{ - DomainName: i.DomainName, - }) - if err != nil { - return err - } - jConfig, err := conn.DescribeDomainConfig(&opensearchservice.DescribeDomainConfigInput{ - DomainName: j.DomainName, - }) - if err != nil { - return err + if aws.StringValue(i.DomainId) == aws.StringValue(j.DomainId) { + return fmt.Errorf("domain (%s) was not recreated, before ID (%s), after ID (%s)", aws.StringValue(i.DomainName), aws.StringValue(i.DomainId), aws.StringValue(j.DomainId)) } - if !aws.TimeValue(iConfig.DomainConfig.ClusterConfig.Status.CreationDate).Equal(aws.TimeValue(jConfig.DomainConfig.ClusterConfig.Status.CreationDate)) { - return fmt.Errorf("OpenSearch Domain was recreated") + return nil + } +} + +func testAccCheckDomainNotRecreated(i, j *opensearchservice.DomainStatus) resource.TestCheckFunc { + return func(s *terraform.State) error { + if aws.StringValue(i.DomainId) != aws.StringValue(j.DomainId) { + return fmt.Errorf("domain (%s) was recreated, before ID (%s), after ID (%s)", aws.StringValue(i.DomainName), aws.StringValue(i.DomainId), aws.StringValue(j.DomainId)) } return nil @@ -2237,14 +2392,13 @@ data "aws_iam_policy_document" "test" { `, rName) } -func testAccDomainConfig_encryptAtRestDefaultKey(rName string) string { +func testAccDomainConfig_encryptAtRestDefaultKey(rName, version string, enabled bool) string { return fmt.Sprintf(` resource "aws_opensearch_domain" "test" { domain_name = substr(%[1]q, 0, 28) - engine_version = "Elasticsearch_6.0" + engine_version = %[2]q - # Encrypt at rest requires m4/c4/r4/i2 instances. See http://docs.aws.amazon.com/opensearch-service/latest/developerguide/aes-supported-instance-types.html cluster_config { instance_type = "m4.large.search" } @@ -2255,10 +2409,10 @@ resource "aws_opensearch_domain" "test" { } encrypt_at_rest { - enabled = true + enabled = %[3]t } } -`, rName) +`, rName, version, enabled) } func testAccDomainConfig_encryptAtRestWithKey(rName string) string { @@ -2291,12 +2445,12 @@ resource "aws_opensearch_domain" "test" { `, rName) } -func testAccDomainConfig_nodeToNodeEncryption(rName string) string { +func testAccDomainConfig_nodeToNodeEncryption(rName, version string, enabled bool) string { return fmt.Sprintf(` resource "aws_opensearch_domain" "test" { domain_name = substr(%[1]q, 0, 28) - engine_version = "Elasticsearch_6.0" + engine_version = %[2]q cluster_config { instance_type = "m4.large.search" @@ -2308,10 +2462,10 @@ resource "aws_opensearch_domain" "test" { } node_to_node_encryption { - enabled = true + enabled = %[3]t } } -`, rName) +`, rName, version, enabled) } func testAccDomainConfig_complex(rName string) string { From 29c21bfe1ea46ce3714eb3c14505de584160287a Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 12:53:46 -0400 Subject: [PATCH 06/17] Update changelog --- .changelog/24222.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.changelog/24222.txt b/.changelog/24222.txt index 7d2b9beb14af..9b85807f8e55 100644 --- a/.changelog/24222.txt +++ b/.changelog/24222.txt @@ -1,3 +1,7 @@ ```release-note:enhancement resource/aws_elasticsearch_domain: For Elasticsearch versions 6.7+, allow in-place update of `node_to_node_encryption.0.enabled`, `encrypt_at_rest.0.enabled`, and `encrypt_at_rest.0.kms_key_id` +``` + +```release-note:enhancement +resource/aws_opensearch_domain: For Elasticsearch versions 6.7+, allow in-place update of `node_to_node_encryption.0.enabled`, `encrypt_at_rest.0.enabled`, and `encrypt_at_rest.0.kms_key_id` ``` \ No newline at end of file From 60ce2d64c6f48ffa78dbfdbcb351c5417960745d Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 12:54:00 -0400 Subject: [PATCH 07/17] Update docs --- website/docs/r/elasticsearch_domain.html.markdown | 2 +- website/docs/r/opensearch_domain.html.markdown | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/website/docs/r/elasticsearch_domain.html.markdown b/website/docs/r/elasticsearch_domain.html.markdown index 27b5077d8903..139c5bdf7d6f 100644 --- a/website/docs/r/elasticsearch_domain.html.markdown +++ b/website/docs/r/elasticsearch_domain.html.markdown @@ -299,7 +299,7 @@ AWS documentation: [Amazon Cognito Authentication for Kibana](https://docs.aws.a ### encrypt_at_rest -~> **Note:** You can enable `encrypt_at_rest` in place if your Elasticsearch version is 6.7 or greater. If you disable `encrypt_rest_rest`, once you enable it, for any version, or enable for 6.6 and below, Terraform will recreate the domain, which can cause data loss. Similarly, you can change the `kms_key_id` in place if your version is 6.7 or great. Otherwise, Terraform will recreate the domain, which can cause data loss. +~> **Note:** You can enable `encrypt_at_rest` in place if your Elasticsearch version is 6.7 or greater. If you disable `encrypt_rest_rest`, once you enable it, for any version, or enable for 6.6 and below, Terraform will recreate the domain, which can cause data loss. In contrast, AWS does not support changing the `kms_key_id`. If you change the key, Terraform will recreate the domain, which can cause data loss. * `enabled` - (Required) Whether to enable encryption at rest. If the `encrypt_at_rest` block is not provided then this defaults to `false`. * `kms_key_id` - (Optional) KMS key ARN to encrypt the Elasticsearch domain with. If not specified then it defaults to using the `aws/es` service KMS key. Note that KMS will accept a KMS key ID but will return the key ARN. To prevent Terraform detecting unwanted changes, use the key ARN instead. diff --git a/website/docs/r/opensearch_domain.html.markdown b/website/docs/r/opensearch_domain.html.markdown index 49aca1a5ddb7..23a1b943855b 100644 --- a/website/docs/r/opensearch_domain.html.markdown +++ b/website/docs/r/opensearch_domain.html.markdown @@ -316,7 +316,7 @@ AWS documentation: [Amazon Cognito Authentication for Kibana](https://docs.aws.a ### encrypt_at_rest -~> **Note:** You can enable `encrypt_at_rest` in place if your Elasticsearch version is 6.7 or greater. If you disable `encrypt_rest_rest`, once you enable it, for any version, or enable for 6.6 and below, Terraform will recreate the domain, which can cause data loss. Similarly, you can change the `kms_key_id` in place if your version is 6.7 or great. Otherwise, Terraform will recreate the domain, which can cause data loss. +~> **Note:** You can enable `encrypt_at_rest` in place if your Elasticsearch version is 6.7 or greater. If you disable `encrypt_rest_rest`, once you enable it, for any version, or enable for 6.6 and below, Terraform will recreate the domain, which can cause data loss. In contrast, AWS does not support changing the `kms_key_id`. If you change the key, Terraform will recreate the domain, which can cause data loss. * `enabled` - (Required) Whether to enable encryption at rest. If the `encrypt_at_rest` block is not provided then this defaults to `false`. * `kms_key_id` - (Optional) KMS key ARN to encrypt the Elasticsearch domain with. If not specified then it defaults to using the `aws/es` service KMS key. Note that KMS will accept a KMS key ID but will return the key ARN. To prevent Terraform detecting unwanted changes, use the key ARN instead. From f81dc915b3a6f61223497ace6e87e12becc35590 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 12:54:13 -0400 Subject: [PATCH 08/17] Align es and opensearch --- internal/service/elasticsearch/domain.go | 26 ++-- internal/service/elasticsearch/domain_test.go | 118 ++++++---------- internal/service/elasticsearch/status.go | 23 ++++ internal/service/elasticsearch/wait.go | 14 ++ internal/service/opensearch/domain.go | 26 ++-- internal/service/opensearch/domain_test.go | 126 ++++++++++-------- internal/service/opensearch/status.go | 23 ++++ internal/service/opensearch/wait.go | 20 +++ 8 files changed, 212 insertions(+), 164 deletions(-) diff --git a/internal/service/elasticsearch/domain.go b/internal/service/elasticsearch/domain.go index 23c7b38d67df..afbc0cd45cfb 100644 --- a/internal/service/elasticsearch/domain.go +++ b/internal/service/elasticsearch/domain.go @@ -68,30 +68,19 @@ func ResourceDomain() *schema.Resource { customdiff.ForceNewIf("encrypt_at_rest.0.enabled", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { // cannot disable (at all) or enable if < 6.7 without forcenew o, n := d.GetChange("encrypt_at_rest.0.enabled") - fmt.Printf("ear - old: %t, new: %t\n", o.(bool), n.(bool)) if o.(bool) && !n.(bool) { return true } - fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string))) - return inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string)) - }), - customdiff.ForceNewIf("encrypt_at_rest.0.kms_key_id", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { - // cannot change if < 6.7 without forcenew - o, n := d.GetChange("encrypt_at_rest.0.kms_key_id") - if o.(string) != "" && n.(string) != "" && (strings.HasSuffix(o.(string), n.(string)) || strings.HasSuffix(n.(string), o.(string))) { - return false - } - fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string))) - return inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string)) + + return !inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string)) }), customdiff.ForceNewIf("node_to_node_encryption.0.enabled", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { o, n := d.GetChange("node_to_node_encryption.0.enabled") - fmt.Printf("ntne - old: %t, new: %t\n", o.(bool), n.(bool)) if o.(bool) && !n.(bool) { return true } - fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string))) - return inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string)) + + return !inPlaceEncryptionEnableVersion(d.Get("elasticsearch_version").(string)) }), verify.SetTagsDiff, ), @@ -431,6 +420,7 @@ func ResourceDomain() *schema.Resource { Type: schema.TypeString, Optional: true, Computed: true, + ForceNew: true, DiffSuppressFunc: suppressEquivalentKmsKeyIds, }, }, @@ -1013,8 +1003,6 @@ func resourceDomainUpdate(d *schema.ResourceData, meta interface{}) error { func resourceDomainDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).ElasticsearchConn - fmt.Printf("deleting %s\n", d.Id()) - name := d.Get("domain_name").(string) log.Printf("[DEBUG] Deleting Elasticsearch Domain: %s", d.Id()) @@ -1053,6 +1041,8 @@ func resourceDomainImport(d *schema.ResourceData, meta interface{}) ([]*schema.R return []*schema.ResourceData{d}, nil } +// inPlaceEncryptionEnableVersion returns true if, based on version, encryption +// can be enabled in place (without ForceNew) func inPlaceEncryptionEnableVersion(version string) bool { var want, got *gversion.Version var err error @@ -1060,7 +1050,7 @@ func inPlaceEncryptionEnableVersion(version string) bool { return false } - if got, err = gversion.NewVersion(version); err != nil || got.GreaterThanOrEqual(want) { + if got, err = gversion.NewVersion(version); err != nil || got.LessThan(want) { return false } diff --git a/internal/service/elasticsearch/domain_test.go b/internal/service/elasticsearch/domain_test.go index 12eda372939c..0d0cb0dfe1fa 100644 --- a/internal/service/elasticsearch/domain_test.go +++ b/internal/service/elasticsearch/domain_test.go @@ -1065,38 +1065,6 @@ func TestAccElasticsearchDomain_Encryption_atRestSpecifyKey(t *testing.T) { }) } -func TestAccElasticsearchDomain_Encryption_nodeToNode(t *testing.T) { - if testing.Short() { - t.Skip("skipping long-running test in short mode") - } - - var domain elasticsearch.ElasticsearchDomainStatus - resourceName := "aws_elasticsearch_domain.test" - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIamServiceLinkedRoleEs(t) }, - ErrorCheck: acctest.ErrorCheck(t, elasticsearch.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckDomainDestroy, - Steps: []resource.TestStep{ - { - Config: testAccDomainConfigwithNodeToNodeEncryption(rName, "6.0", true), - Check: resource.ComposeTestCheckFunc( - testAccCheckDomainExists(resourceName, &domain), - testAccCheckNodeToNodeEncrypted(true, &domain), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateId: rName[:28], - ImportStateVerify: true, - }, - }, - }) -} - func TestAccElasticsearchDomain_Encryption_atRestEnable(t *testing.T) { if testing.Short() { t.Skip("skipping long-running test in short mode") @@ -1132,14 +1100,13 @@ func TestAccElasticsearchDomain_Encryption_atRestEnable(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain1), testAccCheckDomainEncrypted(false, &domain1), - testAccCheckDomainNotRecreated(&domain1, &domain2), ), }, }, }) } -func TestAccElasticsearchDomain_Encryption_atRestSwitchKey(t *testing.T) { +func TestAccElasticsearchDomain_Encryption_atRestEnableLegacy(t *testing.T) { if testing.Short() { t.Skip("skipping long-running test in short mode") } @@ -1155,41 +1122,31 @@ func TestAccElasticsearchDomain_Encryption_atRestSwitchKey(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy, Steps: []resource.TestStep{ { - Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "6.7", true), + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "5.6", false), Check: resource.ComposeTestCheckFunc( - testAccCheckDomainExists(resourceName, &domain2), - testAccCheckDomainEncrypted(true, &domain2), - testAccCheckDomainNotRecreated(&domain1, &domain2), + testAccCheckDomainExists(resourceName, &domain1), + testAccCheckDomainEncrypted(false, &domain1), ), }, { - Config: testAccDomainConfigWithEncryptAtRestWithKey(rName, "6.7", true), + Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "5.6", true), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), testAccCheckDomainEncrypted(true, &domain2), - testAccCheckDomainNotRecreated(&domain1, &domain2), - ), - }, - { - Config: testAccDomainConfigWithEncryptAtRestWithNewKey(rName, "6.7", false), - Check: resource.ComposeTestCheckFunc( - testAccCheckDomainExists(resourceName, &domain1), - testAccCheckDomainEncrypted(false, &domain1), - testAccCheckDomainNotRecreated(&domain1, &domain2), ), }, }, }) } -func TestAccElasticsearchDomain_Encryption_atRestEnableLegacy(t *testing.T) { +func TestAccElasticsearchDomain_Encryption_nodeToNode(t *testing.T) { if testing.Short() { t.Skip("skipping long-running test in short mode") } - var domain1, domain2 elasticsearch.ElasticsearchDomainStatus - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + var domain elasticsearch.ElasticsearchDomainStatus resourceName := "aws_elasticsearch_domain.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIamServiceLinkedRoleEs(t) }, @@ -1198,18 +1155,17 @@ func TestAccElasticsearchDomain_Encryption_atRestEnableLegacy(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy, Steps: []resource.TestStep{ { - Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "5.6", false), + Config: testAccDomainConfigwithNodeToNodeEncryption(rName, "6.0", true), Check: resource.ComposeTestCheckFunc( - testAccCheckDomainExists(resourceName, &domain1), - testAccCheckDomainEncrypted(false, &domain1), + testAccCheckDomainExists(resourceName, &domain), + testAccCheckNodeToNodeEncrypted(true, &domain), ), }, { - Config: testAccDomainConfigWithEncryptAtRestDefaultKey(rName, "5.6", true), - Check: resource.ComposeTestCheckFunc( - testAccCheckDomainExists(resourceName, &domain2), - testAccCheckDomainEncrypted(true, &domain2), - ), + ResourceName: resourceName, + ImportState: true, + ImportStateId: rName[:28], + ImportStateVerify: true, }, }, }) @@ -1250,7 +1206,6 @@ func TestAccElasticsearchDomain_Encryption_nodeToNodeEnable(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain1), testAccCheckNodeToNodeEncrypted(false, &domain1), - testAccCheckDomainNotRecreated(&domain1, &domain2), ), }, }, @@ -1284,7 +1239,6 @@ func TestAccElasticsearchDomain_Encryption_nodeToNodeEnableLegacy(t *testing.T) Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), testAccCheckNodeToNodeEncrypted(true, &domain2), - testAccCheckDomainRecreated(&domain1, &domain2), ), }, { @@ -1292,7 +1246,6 @@ func TestAccElasticsearchDomain_Encryption_nodeToNodeEnableLegacy(t *testing.T) Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain1), testAccCheckNodeToNodeEncrypted(false, &domain1), - testAccCheckDomainRecreated(&domain1, &domain2), ), }, }, @@ -1708,21 +1661,38 @@ func testAccCheckDomainExists(n string, domain *elasticsearch.ElasticsearchDomai } } -func testAccCheckDomainRecreated(i, j *elasticsearch.ElasticsearchDomainStatus) resource.TestCheckFunc { +// testAccCheckDomainNotRecreated does not work. Inexplicably, a deleted +// domain's create time (& endpoint) carry over to a newly created domain with +// the same name, if it's created within any reasonable time after deletion. +func testAccCheckDomainNotRecreated(domain1, domain2 *elasticsearch.ElasticsearchDomainStatus) resource.TestCheckFunc { return func(s *terraform.State) error { - if aws.StringValue(i.Endpoint) == aws.StringValue(j.Endpoint) { - return fmt.Errorf("domain (%s) was not recreated, before endpoint (%s), after endpoint (%s)", aws.StringValue(i.DomainName), aws.StringValue(i.Endpoint), aws.StringValue(j.Endpoint)) - } + /* + conn := acctest.Provider.Meta().(*conns.AWSClient).ElasticsearchConn - return nil - } -} + ic, err := conn.DescribeElasticsearchDomainConfig(&elasticsearch.DescribeElasticsearchDomainConfigInput{ + DomainName: domain1.DomainName, + }) + if err != nil { + return fmt.Errorf("while checking if domain (%s) was not recreated, describing domain config: %w", aws.StringValue(domain1.DomainName), err) + } -func testAccCheckDomainNotRecreated(i, j *elasticsearch.ElasticsearchDomainStatus) resource.TestCheckFunc { - return func(s *terraform.State) error { - if aws.StringValue(i.Endpoint) != aws.StringValue(j.Endpoint) { - return fmt.Errorf("domain (%s) was recreated, before endpoint (%s), after endpoint (%s)", aws.StringValue(i.DomainName), aws.StringValue(i.Endpoint), aws.StringValue(j.Endpoint)) - } + jc, err := conn.DescribeElasticsearchDomainConfig(&elasticsearch.DescribeElasticsearchDomainConfigInput{ + DomainName: domain2.DomainName, + }) + if err != nil { + return fmt.Errorf("while checking if domain (%s) was not recreated, describing domain config: %w", aws.StringValue(domain2.DomainName), err) + } + + if aws.StringValue(domain1.Endpoint) != aws.StringValue(domain2.Endpoint) || !aws.TimeValue(ic.DomainConfig.ElasticsearchClusterConfig.Status.CreationDate).Equal(aws.TimeValue(jc.DomainConfig.ElasticsearchClusterConfig.Status.CreationDate)) { + return fmt.Errorf("domain (%s) was recreated, before endpoint (%s, create time: %s), after endpoint (%s, create time: %s)", + aws.StringValue(domain1.DomainName), + aws.StringValue(domain1.Endpoint), + aws.TimeValue(ic.DomainConfig.ElasticsearchClusterConfig.Status.CreationDate), + aws.StringValue(domain2.Endpoint), + aws.TimeValue(jc.DomainConfig.ElasticsearchClusterConfig.Status.CreationDate), + ) + } + */ return nil } diff --git a/internal/service/elasticsearch/status.go b/internal/service/elasticsearch/status.go index 5c7757ace88f..6967a8626abd 100644 --- a/internal/service/elasticsearch/status.go +++ b/internal/service/elasticsearch/status.go @@ -3,11 +3,15 @@ package elasticsearch import ( "github.com/aws/aws-sdk-go/aws" elasticsearch "github.com/aws/aws-sdk-go/service/elasticsearchservice" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) const ( UpgradeStatusUnknown = "Unknown" + ConfigStatusNotFound = "NotFound" + ConfigStatusUnknown = "Unknown" + ConfigStatusExists = "Exists" ) func statusUpgradeStatus(conn *elasticsearch.ElasticsearchService, name string) resource.StateRefreshFunc { @@ -29,3 +33,22 @@ func statusUpgradeStatus(conn *elasticsearch.ElasticsearchService, name string) return out, aws.StringValue(out.StepStatus), nil } } + +func domainConfigStatus(conn *elasticsearch.ElasticsearchService, name string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + out, err := conn.DescribeElasticsearchDomainConfig(&elasticsearch.DescribeElasticsearchDomainConfigInput{ + DomainName: aws.String(name), + }) + + if tfawserr.ErrCodeEquals(err, elasticsearch.ErrCodeResourceNotFoundException) { + // if first return value is nil, WaitForState treats as not found - here not found is treated differently + return "not nil", ConfigStatusNotFound, nil + } + + if err != nil { + return nil, ConfigStatusUnknown, err + } + + return out, ConfigStatusExists, nil + } +} diff --git a/internal/service/elasticsearch/wait.go b/internal/service/elasticsearch/wait.go index 30d01697ca7a..ca7d1c9ed659 100644 --- a/internal/service/elasticsearch/wait.go +++ b/internal/service/elasticsearch/wait.go @@ -137,5 +137,19 @@ func waitForDomainDelete(conn *elasticsearch.ElasticsearchService, domainName st return err } + stateConf := &resource.StateChangeConf{ + Pending: []string{ConfigStatusUnknown, ConfigStatusExists}, + Target: []string{ConfigStatusNotFound}, + Refresh: domainConfigStatus(conn, domainName), + Timeout: timeout, + MinTimeout: 10 * time.Second, + ContinuousTargetOccurence: 3, + } + + _, err = stateConf.WaitForState() + if err != nil { + return err + } + return nil } diff --git a/internal/service/opensearch/domain.go b/internal/service/opensearch/domain.go index 109d5b73cbf4..dc54a21380b7 100644 --- a/internal/service/opensearch/domain.go +++ b/internal/service/opensearch/domain.go @@ -67,26 +67,19 @@ func ResourceDomain() *schema.Resource { }), customdiff.ForceNewIf("encrypt_at_rest.0.enabled", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { o, n := d.GetChange("encrypt_at_rest.0.enabled") - fmt.Printf("ear - old: %t, new: %t\n", o.(bool), n.(bool)) if o.(bool) && !n.(bool) { return true } - fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("engine_version").(string))) - return inPlaceEncryptionEnableVersion(d.Get("engine_version").(string)) - }), - customdiff.ForceNewIf("encrypt_at_rest.0.kms_key_id", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { - // cannot change if < 6.7 without forcenew - fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("engine_version").(string))) - return inPlaceEncryptionEnableVersion(d.Get("engine_version").(string)) + + return !inPlaceEncryptionEnableVersion(d.Get("engine_version").(string)) }), customdiff.ForceNewIf("node_to_node_encryption.0.enabled", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { o, n := d.GetChange("node_to_node_encryption.0.enabled") - fmt.Printf("ntne - old: %t, new: %t\n", o.(bool), n.(bool)) if o.(bool) && !n.(bool) { return true } - fmt.Printf("force new? %t\n", inPlaceEncryptionEnableVersion(d.Get("engine_version").(string))) - return inPlaceEncryptionEnableVersion(d.Get("engine_version").(string)) + + return !inPlaceEncryptionEnableVersion(d.Get("engine_version").(string)) }), verify.SetTagsDiff, ), @@ -421,6 +414,7 @@ func ResourceDomain() *schema.Resource { Type: schema.TypeString, Optional: true, Computed: true, + ForceNew: true, DiffSuppressFunc: suppressEquivalentKmsKeyIds, }, }, @@ -959,7 +953,7 @@ func resourceDomainUpdate(d *schema.ResourceData, meta interface{}) error { if v, ok := d.GetOk("encrypt_at_rest"); ok { options := v.([]interface{}) if options[0] == nil { - return fmt.Errorf("At least one field is expected inside encrypt_at_rest") + return fmt.Errorf("at least one field is expected inside encrypt_at_rest") } s := options[0].(map[string]interface{}) @@ -1066,9 +1060,11 @@ func resourceDomainDelete(d *schema.ResourceData, meta interface{}) error { return nil } +// inPlaceEncryptionEnableVersion returns true if, based on version, encryption +// can be enabled in place (without ForceNew) func inPlaceEncryptionEnableVersion(version string) bool { if strings.HasPrefix(strings.ToLower(version), "opensearch") { - return false + return true } version = strings.TrimPrefix(strings.ToLower(version), "elasticsearch_") @@ -1079,10 +1075,10 @@ func inPlaceEncryptionEnableVersion(version string) bool { } if got, err = gversion.NewVersion(version); err != nil || got.GreaterThanOrEqual(want) { - return false + return true } - return true + return false } func suppressEquivalentKmsKeyIds(k, old, new string, d *schema.ResourceData) bool { diff --git a/internal/service/opensearch/domain_test.go b/internal/service/opensearch/domain_test.go index d14a577ace29..bb88589a3a77 100644 --- a/internal/service/opensearch/domain_test.go +++ b/internal/service/opensearch/domain_test.go @@ -1143,7 +1143,7 @@ func TestAccOpenSearchDomain_Encryption_atRestSpecifyKey(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy, Steps: []resource.TestStep{ { - Config: testAccDomainConfig_encryptAtRestWithKey(rName), + Config: testAccDomainConfig_encryptAtRestWithKey(rName, "Elasticsearch_6.0", true), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain), testAccCheckDomainEncrypted(true, &domain), @@ -1159,38 +1159,6 @@ func TestAccOpenSearchDomain_Encryption_atRestSpecifyKey(t *testing.T) { }) } -func TestAccOpenSearchDomain_Encryption_nodeToNode(t *testing.T) { - if testing.Short() { - t.Skip("skipping long-running test in short mode") - } - - var domain opensearchservice.DomainStatus - resourceName := "aws_opensearch_domain.test" - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIAMServiceLinkedRoleOpenSearch(t) }, - ErrorCheck: acctest.ErrorCheck(t, opensearchservice.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckDomainDestroy, - Steps: []resource.TestStep{ - { - Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_6.0", true), - Check: resource.ComposeTestCheckFunc( - testAccCheckDomainExists(resourceName, &domain), - testAccCheckNodeToNodeEncrypted(true, &domain), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateId: rName[:28], - ImportStateVerify: true, - }, - }, - }) -} - func TestAccOpenSearchDomain_Encryption_atRestEnable(t *testing.T) { if testing.Short() { t.Skip("skipping long-running test in short mode") @@ -1226,7 +1194,6 @@ func TestAccOpenSearchDomain_Encryption_atRestEnable(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain1), testAccCheckDomainEncrypted(false, &domain1), - testAccCheckDomainRecreated(&domain1, &domain2), ), }, }, @@ -1260,13 +1227,44 @@ func TestAccOpenSearchDomain_Encryption_atRestEnableLegacy(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), testAccCheckDomainEncrypted(true, &domain2), - testAccCheckDomainRecreated(&domain1, &domain2), ), }, }, }) } +func TestAccOpenSearchDomain_Encryption_nodeToNode(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var domain opensearchservice.DomainStatus + resourceName := "aws_opensearch_domain.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIAMServiceLinkedRoleOpenSearch(t) }, + ErrorCheck: acctest.ErrorCheck(t, opensearchservice.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDomainDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_6.0", true), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &domain), + testAccCheckNodeToNodeEncrypted(true, &domain), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateId: rName[:28], + ImportStateVerify: true, + }, + }, + }) +} + func TestAccOpenSearchDomain_Encryption_nodeToNodeEnable(t *testing.T) { if testing.Short() { t.Skip("skipping long-running test in short mode") @@ -1302,7 +1300,6 @@ func TestAccOpenSearchDomain_Encryption_nodeToNodeEnable(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain1), testAccCheckNodeToNodeEncrypted(false, &domain1), - testAccCheckDomainRecreated(&domain1, &domain2), ), }, }, @@ -1325,26 +1322,24 @@ func TestAccOpenSearchDomain_Encryption_nodeToNodeEnableLegacy(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy, Steps: []resource.TestStep{ { - Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_5.6", false), + Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_6.0", false), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain1), testAccCheckNodeToNodeEncrypted(false, &domain1), ), }, { - Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_5.6", true), + Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_6.0", true), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), testAccCheckNodeToNodeEncrypted(true, &domain2), - testAccCheckDomainRecreated(&domain1, &domain2), ), }, { - Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_5.6", false), + Config: testAccDomainConfig_nodeToNodeEncryption(rName, "Elasticsearch_6.0", false), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain1), testAccCheckNodeToNodeEncrypted(false, &domain1), - testAccCheckDomainRecreated(&domain1, &domain2), ), }, }, @@ -1720,21 +1715,38 @@ func testAccCheckDomainExists(n string, domain *opensearchservice.DomainStatus) } } -func testAccCheckDomainRecreated(i, j *opensearchservice.DomainStatus) resource.TestCheckFunc { +// testAccCheckDomainNotRecreated does not work. Inexplicably, a deleted +// domain's create time (& endpoint) carry over to a newly created domain with +// the same name, if it's created within any reasonable time after deletion. +func testAccCheckDomainNotRecreated(domain1, domain2 *opensearchservice.DomainStatus) resource.TestCheckFunc { return func(s *terraform.State) error { - if aws.StringValue(i.DomainId) == aws.StringValue(j.DomainId) { - return fmt.Errorf("domain (%s) was not recreated, before ID (%s), after ID (%s)", aws.StringValue(i.DomainName), aws.StringValue(i.DomainId), aws.StringValue(j.DomainId)) - } + /* + conn := acctest.Provider.Meta().(*conns.AWSClient).OpenSearchConn - return nil - } -} + ic, err := conn.DescribeDomainConfig(&opensearchservice.DescribeDomainConfigInput{ + DomainName: domain1.DomainName, + }) + if err != nil { + return fmt.Errorf("while checking if domain (%s) was not recreated, describing domain config: %w", aws.StringValue(domain1.DomainName), err) + } -func testAccCheckDomainNotRecreated(i, j *opensearchservice.DomainStatus) resource.TestCheckFunc { - return func(s *terraform.State) error { - if aws.StringValue(i.DomainId) != aws.StringValue(j.DomainId) { - return fmt.Errorf("domain (%s) was recreated, before ID (%s), after ID (%s)", aws.StringValue(i.DomainName), aws.StringValue(i.DomainId), aws.StringValue(j.DomainId)) - } + jc, err := conn.DescribeDomainConfig(&opensearchservice.DescribeDomainConfigInput{ + DomainName: domain2.DomainName, + }) + if err != nil { + return fmt.Errorf("while checking if domain (%s) was not recreated, describing domain config: %w", aws.StringValue(domain2.DomainName), err) + } + + if aws.StringValue(domain1.Endpoint) != aws.StringValue(domain2.Endpoint) || !aws.TimeValue(ic.DomainConfig.ClusterConfig.Status.CreationDate).Equal(aws.TimeValue(jc.DomainConfig.ClusterConfig.Status.CreationDate)) { + return fmt.Errorf("domain (%s) was recreated, before endpoint (%s, create time: %s), after endpoint (%s, create time: %s)", + aws.StringValue(domain1.DomainName), + aws.StringValue(domain1.Endpoint), + aws.TimeValue(ic.DomainConfig.ClusterConfig.Status.CreationDate), + aws.StringValue(domain2.Endpoint), + aws.TimeValue(jc.DomainConfig.ClusterConfig.Status.CreationDate), + ) + } + */ return nil } @@ -2415,7 +2427,7 @@ resource "aws_opensearch_domain" "test" { `, rName, version, enabled) } -func testAccDomainConfig_encryptAtRestWithKey(rName string) string { +func testAccDomainConfig_encryptAtRestWithKey(rName, version string, enabled bool) string { return fmt.Sprintf(` resource "aws_kms_key" "test" { description = %[1]q @@ -2425,7 +2437,7 @@ resource "aws_kms_key" "test" { resource "aws_opensearch_domain" "test" { domain_name = substr(%[1]q, 0, 28) - engine_version = "Elasticsearch_6.0" + engine_version = %[2]q # Encrypt at rest requires m4/c4/r4/i2 instances. See http://docs.aws.amazon.com/opensearch-service/latest/developerguide/aes-supported-instance-types.html cluster_config { @@ -2438,11 +2450,11 @@ resource "aws_opensearch_domain" "test" { } encrypt_at_rest { - enabled = true + enabled = %[3]t kms_key_id = aws_kms_key.test.key_id } } -`, rName) +`, rName, version, enabled) } func testAccDomainConfig_nodeToNodeEncryption(rName, version string, enabled bool) string { diff --git a/internal/service/opensearch/status.go b/internal/service/opensearch/status.go index c167daf39dce..ecd3b6eadafb 100644 --- a/internal/service/opensearch/status.go +++ b/internal/service/opensearch/status.go @@ -3,11 +3,15 @@ package opensearch import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/opensearchservice" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) const ( UpgradeStatusUnknown = "Unknown" + ConfigStatusNotFound = "NotFound" + ConfigStatusUnknown = "Unknown" + ConfigStatusExists = "Exists" ) func statusUpgradeStatus(conn *opensearchservice.OpenSearchService, name string) resource.StateRefreshFunc { @@ -29,3 +33,22 @@ func statusUpgradeStatus(conn *opensearchservice.OpenSearchService, name string) return out, aws.StringValue(out.StepStatus), nil } } + +func domainConfigStatus(conn *opensearchservice.OpenSearchService, name string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + out, err := conn.DescribeDomainConfig(&opensearchservice.DescribeDomainConfigInput{ + DomainName: aws.String(name), + }) + + if tfawserr.ErrCodeEquals(err, opensearchservice.ErrCodeResourceNotFoundException) { + // if first return value is nil, WaitForState treats as not found - here not found is treated differently + return "not nil", ConfigStatusNotFound, nil + } + + if err != nil { + return nil, ConfigStatusUnknown, err + } + + return out, ConfigStatusExists, nil + } +} diff --git a/internal/service/opensearch/wait.go b/internal/service/opensearch/wait.go index fe4b037f72d1..0d84d38ca017 100644 --- a/internal/service/opensearch/wait.go +++ b/internal/service/opensearch/wait.go @@ -116,6 +116,7 @@ func waitForDomainDelete(conn *opensearchservice.OpenSearchService, domainName s return resource.RetryableError(fmt.Errorf("timeout while waiting for the OpenSearch domain %q to be deleted", domainName)) }) + if tfresource.TimedOut(err) { out, err = FindDomainByName(conn, domainName) if err != nil { @@ -128,8 +129,27 @@ func waitForDomainDelete(conn *opensearchservice.OpenSearchService, domainName s return nil } } + if err != nil { return fmt.Errorf("Error waiting for OpenSearch domain to be deleted: %s", err) } + + // opensearch maintains information about the domain in multiple (at least 2) places that need + // to clear before it is really deleted - otherwise, requesting information about domain immediately + // after delete will return info about just deleted domain + stateConf := &resource.StateChangeConf{ + Pending: []string{ConfigStatusUnknown, ConfigStatusExists}, + Target: []string{ConfigStatusNotFound}, + Refresh: domainConfigStatus(conn, domainName), + Timeout: timeout, + MinTimeout: 10 * time.Second, + ContinuousTargetOccurence: 3, + } + + _, err = stateConf.WaitForState() + if err != nil { + return err + } + return nil } From f705321ee3911b6ac2f3c44343970275c0080cf1 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 13:46:16 -0400 Subject: [PATCH 09/17] cilint: Deadcode --- internal/service/elasticsearch/domain_test.go | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/internal/service/elasticsearch/domain_test.go b/internal/service/elasticsearch/domain_test.go index 0d0cb0dfe1fa..4204281203f0 100644 --- a/internal/service/elasticsearch/domain_test.go +++ b/internal/service/elasticsearch/domain_test.go @@ -2347,41 +2347,6 @@ resource "aws_elasticsearch_domain" "test" { `, rName, version, enabled) } -func testAccDomainConfigWithEncryptAtRestWithNewKey(rName, version string, enabled bool) string { - return fmt.Sprintf(` -resource "aws_kms_key" "test" { - description = %[1]q - deletion_window_in_days = 7 -} - -resource "aws_kms_key" "test2" { - description = %[1]q - deletion_window_in_days = 7 -} - -resource "aws_elasticsearch_domain" "test" { - domain_name = substr(%[1]q, 0, 28) - - elasticsearch_version = %[2]q - - # Encrypt at rest requires m4/c4/r4/i2 instances. See http://docs.aws.amazon.com/elasticsearch-service/latest/developerguide/aes-supported-instance-types.html - cluster_config { - instance_type = "m4.large.elasticsearch" - } - - ebs_options { - ebs_enabled = true - volume_size = 10 - } - - encrypt_at_rest { - enabled = %[3]t - kms_key_id = aws_kms_key.test2.key_id - } -} -`, rName, version, enabled) -} - func testAccDomainConfigwithNodeToNodeEncryption(rName, version string, enabled bool) string { return fmt.Sprintf(` resource "aws_elasticsearch_domain" "test" { From bc153da3111b5a1bf1a4b21afaf85a8912e92a2b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 14:00:35 -0400 Subject: [PATCH 10/17] Align es and os --- internal/service/opensearch/domain.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/opensearch/domain.go b/internal/service/opensearch/domain.go index dc54a21380b7..9948f319db35 100644 --- a/internal/service/opensearch/domain.go +++ b/internal/service/opensearch/domain.go @@ -1074,11 +1074,11 @@ func inPlaceEncryptionEnableVersion(version string) bool { return false } - if got, err = gversion.NewVersion(version); err != nil || got.GreaterThanOrEqual(want) { - return true + if got, err = gversion.NewVersion(version); err != nil || got.LessThan(want) { + return false } - return false + return true } func suppressEquivalentKmsKeyIds(k, old, new string, d *schema.ResourceData) bool { From a03116112738e2cf48fc57ebc6c74bd97f391099 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 14:05:15 -0400 Subject: [PATCH 11/17] Add code comments --- internal/service/elasticsearch/domain_test.go | 14 +++++++------- internal/service/opensearch/domain_test.go | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/internal/service/elasticsearch/domain_test.go b/internal/service/elasticsearch/domain_test.go index 4204281203f0..fcee95945c81 100644 --- a/internal/service/elasticsearch/domain_test.go +++ b/internal/service/elasticsearch/domain_test.go @@ -176,7 +176,7 @@ func TestAccElasticsearchDomain_Cluster_zoneAwareness(t *testing.T) { Config: testAccDomainConfig_ClusterConfig_ZoneAwarenessConfig_AvailabilityZoneCount(rName, 2), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), - testAccCheckDomainNotRecreated(&domain1, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), // note: this check does not work and always passes resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_config.#", "1"), resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_config.0.availability_zone_count", "2"), resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_enabled", "true"), @@ -186,7 +186,7 @@ func TestAccElasticsearchDomain_Cluster_zoneAwareness(t *testing.T) { Config: testAccDomainConfig_ClusterConfig_ZoneAwarenessEnabled(rName, false), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain3), - testAccCheckDomainNotRecreated(&domain2, &domain3), + testAccCheckDomainNotRecreated(&domain2, &domain3), // note: this check does not work and always passes resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_config.#", "0"), ), @@ -195,7 +195,7 @@ func TestAccElasticsearchDomain_Cluster_zoneAwareness(t *testing.T) { Config: testAccDomainConfig_ClusterConfig_ZoneAwarenessConfig_AvailabilityZoneCount(rName, 3), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain4), - testAccCheckDomainNotRecreated(&domain3, &domain4), + testAccCheckDomainNotRecreated(&domain3, &domain4), // note: this check does not work and always passes resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_config.#", "1"), resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_config.0.availability_zone_count", "3"), resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_enabled", "true"), @@ -1092,7 +1092,7 @@ func TestAccElasticsearchDomain_Encryption_atRestEnable(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), testAccCheckDomainEncrypted(true, &domain2), - testAccCheckDomainNotRecreated(&domain1, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), // note: this check does not work and always passes ), }, { @@ -1198,7 +1198,7 @@ func TestAccElasticsearchDomain_Encryption_nodeToNodeEnable(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), testAccCheckNodeToNodeEncrypted(true, &domain2), - testAccCheckDomainNotRecreated(&domain1, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), // note: this check does not work and always passes ), }, { @@ -1456,7 +1456,7 @@ func TestAccElasticsearchDomain_Update_version(t *testing.T) { Config: testAccDomainConfig_ClusterUpdateVersion(rName, "5.6"), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), - testAccCheckDomainNotRecreated(&domain1, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), // note: this check does not work and always passes resource.TestCheckResourceAttr(resourceName, "elasticsearch_version", "5.6"), ), }, @@ -1464,7 +1464,7 @@ func TestAccElasticsearchDomain_Update_version(t *testing.T) { Config: testAccDomainConfig_ClusterUpdateVersion(rName, "6.3"), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain3), - testAccCheckDomainNotRecreated(&domain2, &domain3), + testAccCheckDomainNotRecreated(&domain2, &domain3), // note: this check does not work and always passes resource.TestCheckResourceAttr(resourceName, "elasticsearch_version", "6.3"), ), }, diff --git a/internal/service/opensearch/domain_test.go b/internal/service/opensearch/domain_test.go index bb88589a3a77..e0000dd20b71 100644 --- a/internal/service/opensearch/domain_test.go +++ b/internal/service/opensearch/domain_test.go @@ -229,7 +229,7 @@ func TestAccOpenSearchDomain_Cluster_zoneAwareness(t *testing.T) { Config: testAccDomainConfig_clusterZoneAwarenessAZCount(rName, 2), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), - testAccCheckDomainNotRecreated(&domain1, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), // note: this check does not work and always passes resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_config.#", "1"), resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_config.0.availability_zone_count", "2"), resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_enabled", "true"), @@ -239,7 +239,7 @@ func TestAccOpenSearchDomain_Cluster_zoneAwareness(t *testing.T) { Config: testAccDomainConfig_clusterZoneAwarenessEnabled(rName, false), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain3), - testAccCheckDomainNotRecreated(&domain2, &domain3), + testAccCheckDomainNotRecreated(&domain2, &domain3), // note: this check does not work and always passes resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_config.#", "0"), ), @@ -248,7 +248,7 @@ func TestAccOpenSearchDomain_Cluster_zoneAwareness(t *testing.T) { Config: testAccDomainConfig_clusterZoneAwarenessAZCount(rName, 3), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain4), - testAccCheckDomainNotRecreated(&domain3, &domain4), + testAccCheckDomainNotRecreated(&domain3, &domain4), // note: this check does not work and always passes resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_config.#", "1"), resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_config.0.availability_zone_count", "3"), resource.TestCheckResourceAttr(resourceName, "cluster_config.0.zone_awareness_enabled", "true"), @@ -1186,7 +1186,7 @@ func TestAccOpenSearchDomain_Encryption_atRestEnable(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), testAccCheckDomainEncrypted(true, &domain2), - testAccCheckDomainNotRecreated(&domain1, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), // note: this check does not work and always passes ), }, { @@ -1292,7 +1292,7 @@ func TestAccOpenSearchDomain_Encryption_nodeToNodeEnable(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), testAccCheckNodeToNodeEncrypted(true, &domain2), - testAccCheckDomainNotRecreated(&domain1, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), // note: this check does not work and always passes ), }, { @@ -1510,7 +1510,7 @@ func TestAccOpenSearchDomain_versionUpdate(t *testing.T) { Config: testAccDomainConfig_clusterUpdateVersion(rName, "Elasticsearch_5.6"), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain2), - testAccCheckDomainNotRecreated(&domain1, &domain2), + testAccCheckDomainNotRecreated(&domain1, &domain2), // note: this check does not work and always passes resource.TestCheckResourceAttr(resourceName, "engine_version", "Elasticsearch_5.6"), ), }, @@ -1518,7 +1518,7 @@ func TestAccOpenSearchDomain_versionUpdate(t *testing.T) { Config: testAccDomainConfig_clusterUpdateVersion(rName, "Elasticsearch_6.3"), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(resourceName, &domain3), - testAccCheckDomainNotRecreated(&domain2, &domain3), + testAccCheckDomainNotRecreated(&domain2, &domain3), // note: this check does not work and always passes resource.TestCheckResourceAttr(resourceName, "engine_version", "Elasticsearch_6.3"), ), }, From 36da4c039bc264f9e2e12455839040bf150c5c3a Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 14:08:47 -0400 Subject: [PATCH 12/17] Add comments --- internal/service/elasticsearch/domain_test.go | 2 ++ internal/service/opensearch/domain_test.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/internal/service/elasticsearch/domain_test.go b/internal/service/elasticsearch/domain_test.go index fcee95945c81..2025bd482e48 100644 --- a/internal/service/elasticsearch/domain_test.go +++ b/internal/service/elasticsearch/domain_test.go @@ -1664,6 +1664,8 @@ func testAccCheckDomainExists(n string, domain *elasticsearch.ElasticsearchDomai // testAccCheckDomainNotRecreated does not work. Inexplicably, a deleted // domain's create time (& endpoint) carry over to a newly created domain with // the same name, if it's created within any reasonable time after deletion. +// Also, domain ID is not unique and is simply the domain name so won't work +// for this check either. func testAccCheckDomainNotRecreated(domain1, domain2 *elasticsearch.ElasticsearchDomainStatus) resource.TestCheckFunc { return func(s *terraform.State) error { /* diff --git a/internal/service/opensearch/domain_test.go b/internal/service/opensearch/domain_test.go index e0000dd20b71..97e3a191e0ae 100644 --- a/internal/service/opensearch/domain_test.go +++ b/internal/service/opensearch/domain_test.go @@ -1718,6 +1718,8 @@ func testAccCheckDomainExists(n string, domain *opensearchservice.DomainStatus) // testAccCheckDomainNotRecreated does not work. Inexplicably, a deleted // domain's create time (& endpoint) carry over to a newly created domain with // the same name, if it's created within any reasonable time after deletion. +// Also, domain ID is not unique and is simply the domain name so won't work +// for this check either. func testAccCheckDomainNotRecreated(domain1, domain2 *opensearchservice.DomainStatus) resource.TestCheckFunc { return func(s *terraform.State) error { /* From e7c85124f1eef6ca3080d0820c395c3a63047f93 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 14:26:44 -0400 Subject: [PATCH 13/17] Clarify docs --- website/docs/r/elasticsearch_domain.html.markdown | 4 ++-- website/docs/r/opensearch_domain.html.markdown | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/website/docs/r/elasticsearch_domain.html.markdown b/website/docs/r/elasticsearch_domain.html.markdown index 139c5bdf7d6f..7c7f8698e769 100644 --- a/website/docs/r/elasticsearch_domain.html.markdown +++ b/website/docs/r/elasticsearch_domain.html.markdown @@ -299,9 +299,9 @@ AWS documentation: [Amazon Cognito Authentication for Kibana](https://docs.aws.a ### encrypt_at_rest -~> **Note:** You can enable `encrypt_at_rest` in place if your Elasticsearch version is 6.7 or greater. If you disable `encrypt_rest_rest`, once you enable it, for any version, or enable for 6.6 and below, Terraform will recreate the domain, which can cause data loss. In contrast, AWS does not support changing the `kms_key_id`. If you change the key, Terraform will recreate the domain, which can cause data loss. +~> **Note:** You can enable `encrypt_at_rest` _in place_ for an existing, unencrypted domain only if your Elasticsearch version is 6.7 or greater. For lower versions, if you enable `encrypt_at_rest`, Terraform with recreate the domain, potentially causing data loss. For any version, if you disable `encrypt_at_rest` for an existing, encrypted domain, Terraform will recreate the domain, potentially causing data loss. If you change the `kms_key_id`, Terraform will also recreate the domain, potentially causing data loss. -* `enabled` - (Required) Whether to enable encryption at rest. If the `encrypt_at_rest` block is not provided then this defaults to `false`. +* `enabled` - (Required) Whether to enable encryption at rest. If the `encrypt_at_rest` block is not provided then this defaults to `false`. Enabling encryption on new domains requires `elasticsearch_version` 5.1 or greater. * `kms_key_id` - (Optional) KMS key ARN to encrypt the Elasticsearch domain with. If not specified then it defaults to using the `aws/es` service KMS key. Note that KMS will accept a KMS key ID but will return the key ARN. To prevent Terraform detecting unwanted changes, use the key ARN instead. ### log_publishing_options diff --git a/website/docs/r/opensearch_domain.html.markdown b/website/docs/r/opensearch_domain.html.markdown index 23a1b943855b..de8fcf486ecd 100644 --- a/website/docs/r/opensearch_domain.html.markdown +++ b/website/docs/r/opensearch_domain.html.markdown @@ -316,9 +316,9 @@ AWS documentation: [Amazon Cognito Authentication for Kibana](https://docs.aws.a ### encrypt_at_rest -~> **Note:** You can enable `encrypt_at_rest` in place if your Elasticsearch version is 6.7 or greater. If you disable `encrypt_rest_rest`, once you enable it, for any version, or enable for 6.6 and below, Terraform will recreate the domain, which can cause data loss. In contrast, AWS does not support changing the `kms_key_id`. If you change the key, Terraform will recreate the domain, which can cause data loss. +~> **Note:** You can enable `encrypt_at_rest` _in place_ for an existing, unencrypted domain only if you are using OpenSearch or your Elasticsearch version is 6.7 or greater. For other versions, if you enable `encrypt_at_rest`, Terraform with recreate the domain, potentially causing data loss. For any version, if you disable `encrypt_at_rest` for an existing, encrypted domain, Terraform will recreate the domain, potentially causing data loss. If you change the `kms_key_id`, Terraform will also recreate the domain, potentially causing data loss. -* `enabled` - (Required) Whether to enable encryption at rest. If the `encrypt_at_rest` block is not provided then this defaults to `false`. +* `enabled` - (Required) Whether to enable encryption at rest. If the `encrypt_at_rest` block is not provided then this defaults to `false`. Enabling encryption on new domains requires an `engine_version` of `OpenSearch_X.Y` or `Elasticsearch_5.1` or greater. * `kms_key_id` - (Optional) KMS key ARN to encrypt the Elasticsearch domain with. If not specified then it defaults to using the `aws/es` service KMS key. Note that KMS will accept a KMS key ID but will return the key ARN. To prevent Terraform detecting unwanted changes, use the key ARN instead. ### log_publishing_options From 08296b930857dd3c7622a623168af049842a9541 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 14:29:10 -0400 Subject: [PATCH 14/17] Update changelog --- .changelog/24222.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changelog/24222.txt b/.changelog/24222.txt index 9b85807f8e55..334077e19127 100644 --- a/.changelog/24222.txt +++ b/.changelog/24222.txt @@ -1,7 +1,7 @@ ```release-note:enhancement -resource/aws_elasticsearch_domain: For Elasticsearch versions 6.7+, allow in-place update of `node_to_node_encryption.0.enabled`, `encrypt_at_rest.0.enabled`, and `encrypt_at_rest.0.kms_key_id` +resource/aws_elasticsearch_domain: For Elasticsearch versions 6.7+, allow in-place update of `node_to_node_encryption.0.enabled` and `encrypt_at_rest.0.enabled`. ``` ```release-note:enhancement -resource/aws_opensearch_domain: For Elasticsearch versions 6.7+, allow in-place update of `node_to_node_encryption.0.enabled`, `encrypt_at_rest.0.enabled`, and `encrypt_at_rest.0.kms_key_id` +resource/aws_opensearch_domain: For Elasticsearch versions 6.7+, allow in-place update of `node_to_node_encryption.0.enabled` and `encrypt_at_rest.0.enabled`. ``` \ No newline at end of file From 10c79851e6a6fef421dc4278405b9c25980158a5 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 14:36:19 -0400 Subject: [PATCH 15/17] Fix docs --- website/docs/r/elasticsearch_domain.html.markdown | 2 +- website/docs/r/opensearch_domain.html.markdown | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/website/docs/r/elasticsearch_domain.html.markdown b/website/docs/r/elasticsearch_domain.html.markdown index 7c7f8698e769..25ef025c8b81 100644 --- a/website/docs/r/elasticsearch_domain.html.markdown +++ b/website/docs/r/elasticsearch_domain.html.markdown @@ -312,7 +312,7 @@ AWS documentation: [Amazon Cognito Authentication for Kibana](https://docs.aws.a ### node_to_node_encryption -~> **Note:** You can enable `node_to_node_encryption` in place if your Elasticsearch version is 6.7 or greater. If you disable `node_to_node_encryption`, once you enable it, for any version, or enable for 6.6 and below, Terraform will recreate the domain causing data loss. +~> **Note:** You can enable `node_to_node_encryption` _in place_ for an existing, unencrypted domain only if your Elasticsearch version is 6.7 or greater. For lower versions, if you enable `node_to_node_encryption`, Terraform will recreate the domain, potentially causing data loss. For any version, if you disable `node_to_node_encryption` for an existing, node-to-node encrypted domain, Terraform will recreate the domain, potentially causing data loss. * `enabled` - (Required) Whether to enable node-to-node encryption. If the `node_to_node_encryption` block is not provided then this defaults to `false`. diff --git a/website/docs/r/opensearch_domain.html.markdown b/website/docs/r/opensearch_domain.html.markdown index de8fcf486ecd..0155a5f53138 100644 --- a/website/docs/r/opensearch_domain.html.markdown +++ b/website/docs/r/opensearch_domain.html.markdown @@ -329,6 +329,8 @@ AWS documentation: [Amazon Cognito Authentication for Kibana](https://docs.aws.a ### node_to_node_encryption +~> **Note:** You can enable `node_to_node_encryption` _in place_ for an existing, unencrypted domain only if you are using OpenSearch or your Elasticsearch version is 6.7 or greater. For other versions, if you enable `node_to_node_encryption`, Terraform will recreate the domain, potentially causing data loss. For any version, if you disable `node_to_node_encryption` for an existing, node-to-node encrypted domain, Terraform will recreate the domain, potentially causing data loss. + * `enabled` - (Required) Whether to enable node-to-node encryption. If the `node_to_node_encryption` block is not provided then this defaults to `false`. ### snapshot_options From 1faed8484b5df95a1e796b76b3ca31ff386c0377 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 14:44:27 -0400 Subject: [PATCH 16/17] es/os: Docs clarification --- website/docs/r/elasticsearch_domain.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/elasticsearch_domain.html.markdown b/website/docs/r/elasticsearch_domain.html.markdown index 25ef025c8b81..3a7880535884 100644 --- a/website/docs/r/elasticsearch_domain.html.markdown +++ b/website/docs/r/elasticsearch_domain.html.markdown @@ -314,7 +314,7 @@ AWS documentation: [Amazon Cognito Authentication for Kibana](https://docs.aws.a ~> **Note:** You can enable `node_to_node_encryption` _in place_ for an existing, unencrypted domain only if your Elasticsearch version is 6.7 or greater. For lower versions, if you enable `node_to_node_encryption`, Terraform will recreate the domain, potentially causing data loss. For any version, if you disable `node_to_node_encryption` for an existing, node-to-node encrypted domain, Terraform will recreate the domain, potentially causing data loss. -* `enabled` - (Required) Whether to enable node-to-node encryption. If the `node_to_node_encryption` block is not provided then this defaults to `false`. +* `enabled` - (Required) Whether to enable node-to-node encryption. If the `node_to_node_encryption` block is not provided then this defaults to `false`. Enabling node-to-node encryption of a new domain requires an `elasticsearch_version` of `6.0` or greater. ### snapshot_options From 5a21d228ab6a0433c2978c9f7b913f3063d7552a Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 21 Apr 2022 14:46:24 -0400 Subject: [PATCH 17/17] es/os: Docs clarification --- website/docs/r/opensearch_domain.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/opensearch_domain.html.markdown b/website/docs/r/opensearch_domain.html.markdown index 0155a5f53138..2264602d371d 100644 --- a/website/docs/r/opensearch_domain.html.markdown +++ b/website/docs/r/opensearch_domain.html.markdown @@ -331,7 +331,7 @@ AWS documentation: [Amazon Cognito Authentication for Kibana](https://docs.aws.a ~> **Note:** You can enable `node_to_node_encryption` _in place_ for an existing, unencrypted domain only if you are using OpenSearch or your Elasticsearch version is 6.7 or greater. For other versions, if you enable `node_to_node_encryption`, Terraform will recreate the domain, potentially causing data loss. For any version, if you disable `node_to_node_encryption` for an existing, node-to-node encrypted domain, Terraform will recreate the domain, potentially causing data loss. -* `enabled` - (Required) Whether to enable node-to-node encryption. If the `node_to_node_encryption` block is not provided then this defaults to `false`. +* `enabled` - (Required) Whether to enable node-to-node encryption. If the `node_to_node_encryption` block is not provided then this defaults to `false`. Enabling node-to-node encryption of a new domain requires an `engine_version` of `OpenSearch_X.Y` or `Elasticsearch_6.0` or greater. ### snapshot_options