From dea601e7bcf8748e5814db53207a8e465de79d91 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Thu, 12 Jan 2023 15:09:13 -0500 Subject: [PATCH] r/aws_opensearch_domain: omit iops/throughput when not supported --- internal/service/opensearch/domain.go | 28 +++++ internal/service/opensearch/domain_test.go | 136 +++++++++++++++++++++ internal/service/opensearch/flex.go | 17 +-- 3 files changed, 174 insertions(+), 7 deletions(-) diff --git a/internal/service/opensearch/domain.go b/internal/service/opensearch/domain.go index 8373e774db38..be1b3ad5ca2a 100644 --- a/internal/service/opensearch/domain.go +++ b/internal/service/opensearch/domain.go @@ -1318,3 +1318,31 @@ func ParseEngineVersion(engineVersion string) (string, string, error) { return parts[0], parts[1], nil } + +// EBSVolumeTypePermitsIopsInput returns true if the volume type supports the Iops input +// +// This check prevents a ValidationException when updating EBS volume types from a value +// that supports IOPS (ex. gp3) to one that doesn't (ex. gp2). +func EBSVolumeTypePermitsIopsInput(volumeType string) bool { + permittedTypes := []string{opensearchservice.VolumeTypeGp3, opensearchservice.VolumeTypeIo1} + for _, t := range permittedTypes { + if volumeType == t { + return true + } + } + return false +} + +// EBSVolumeTypePermitsIopsInput returns true if the volume type supports the Throughput input +// +// This check prevents a ValidationException when updating EBS volume types from a value +// that supports Throughput (ex. gp3) to one that doesn't (ex. gp2). +func EBSVolumeTypePermitsThroughputInput(volumeType string) bool { + permittedTypes := []string{opensearchservice.VolumeTypeGp3} + for _, t := range permittedTypes { + if volumeType == t { + return true + } + } + return false +} diff --git a/internal/service/opensearch/domain_test.go b/internal/service/opensearch/domain_test.go index 066e167a8f65..b86f61a8044c 100644 --- a/internal/service/opensearch/domain_test.go +++ b/internal/service/opensearch/domain_test.go @@ -20,6 +20,58 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) +func TestEBSVolumeTypePermitsIopsInput(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + volumeType string + want bool + }{ + {"empty", "", false}, + {"gp2", opensearchservice.VolumeTypeGp2, false}, + {"gp3", opensearchservice.VolumeTypeGp3, true}, + {"io1", opensearchservice.VolumeTypeIo1, true}, + {"standard", opensearchservice.VolumeTypeStandard, false}, + } + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + if got := tfopensearch.EBSVolumeTypePermitsIopsInput(testCase.volumeType); got != testCase.want { + t.Errorf("EBSVolumeTypePermitsIopsInput() = %v, want %v", got, testCase.want) + } + }) + } +} + +func TestEBSVolumeTypePermitsThroughputInput(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + volumeType string + want bool + }{ + {"empty", "", false}, + {"gp2", opensearchservice.VolumeTypeGp2, false}, + {"gp3", opensearchservice.VolumeTypeGp3, true}, + {"io1", opensearchservice.VolumeTypeIo1, false}, + {"standard", opensearchservice.VolumeTypeStandard, false}, + } + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + if got := tfopensearch.EBSVolumeTypePermitsThroughputInput(testCase.volumeType); got != testCase.want { + t.Errorf("EBSVolumeTypePermitsThroughputInput() = %v, want %v", got, testCase.want) + } + }) + } +} + func TestParseEngineVersion(t *testing.T) { t.Parallel() @@ -1493,6 +1545,54 @@ func TestAccOpenSearchDomain_VolumeType_update(t *testing.T) { }}) } +// Verifies that EBS volume_type can be sucessfully changed from gp3 to a type which +// does not support the throughput and iops input values (ex. gp2) +// +// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/27467 +func TestAccOpenSearchDomain_VolumeType_gp3ToStandard(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var input opensearchservice.DomainStatus + rName := testAccRandomDomainName() + resourceName := "aws_opensearch_domain.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckIAMServiceLinkedRole(t) }, + ErrorCheck: acctest.ErrorCheck(t, opensearchservice.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckDomainDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDomainConfig_clusterEBSVolumeGp3DefaultIopsThroughput(rName, 10), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &input), + resource.TestCheckResourceAttr(resourceName, "ebs_options.#", "1"), + resource.TestCheckResourceAttr(resourceName, "ebs_options.0.ebs_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "ebs_options.0.volume_size", "10"), + resource.TestCheckResourceAttr(resourceName, "ebs_options.0.volume_type", "gp3"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateId: rName, + ImportStateVerify: true, + }, + { + Config: testAccDomainConfig_clusterEBSVolumeGp2(rName, 10), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(resourceName, &input), + resource.TestCheckResourceAttr(resourceName, "ebs_options.#", "1"), + resource.TestCheckResourceAttr(resourceName, "ebs_options.0.ebs_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "ebs_options.0.volume_size", "10"), + resource.TestCheckResourceAttr(resourceName, "ebs_options.0.volume_type", "gp2"), + ), + }, + }}) +} + // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/13867 func TestAccOpenSearchDomain_VolumeType_missing(t *testing.T) { if testing.Short() { @@ -2241,6 +2341,42 @@ resource "aws_opensearch_domain" "test" { `, rName, volumeSize, volumeThroughput, volumeIops) } +func testAccDomainConfig_clusterEBSVolumeGp3DefaultIopsThroughput(rName string, volumeSize int) string { + return fmt.Sprintf(` +resource "aws_opensearch_domain" "test" { + domain_name = %[1]q + + ebs_options { + ebs_enabled = true + volume_size = %d + volume_type = "gp3" + } + + cluster_config { + instance_type = "t3.small.search" + } +} +`, rName, volumeSize) +} + +func testAccDomainConfig_clusterEBSVolumeGp2(rName string, volumeSize int) string { + return fmt.Sprintf(` +resource "aws_opensearch_domain" "test" { + domain_name = %[1]q + + ebs_options { + ebs_enabled = true + volume_size = %d + volume_type = "gp2" + } + + cluster_config { + instance_type = "t3.small.search" + } +} +`, rName, volumeSize) +} + func testAccDomainConfig_clusterUpdateVersion(rName, version string) string { return fmt.Sprintf(` resource "aws_opensearch_domain" "test" { diff --git a/internal/service/opensearch/flex.go b/internal/service/opensearch/flex.go index 4bf5a2822a71..77db447d7655 100644 --- a/internal/service/opensearch/flex.go +++ b/internal/service/opensearch/flex.go @@ -76,17 +76,20 @@ func expandEBSOptions(m map[string]interface{}) *opensearchservice.EBSOptions { options.EBSEnabled = aws.Bool(ebsEnabled.(bool)) if ebsEnabled.(bool) { - if v, ok := m["iops"]; ok && v.(int) > 0 { - options.Iops = aws.Int64(int64(v.(int))) - } - if v, ok := m["throughput"]; ok && v.(int) > 0 { - options.Throughput = aws.Int64(int64(v.(int))) - } if v, ok := m["volume_size"]; ok && v.(int) > 0 { options.VolumeSize = aws.Int64(int64(v.(int))) } + var volumeType string if v, ok := m["volume_type"]; ok && v.(string) != "" { - options.VolumeType = aws.String(v.(string)) + volumeType = v.(string) + options.VolumeType = aws.String(volumeType) + } + + if v, ok := m["iops"]; ok && v.(int) > 0 && EBSVolumeTypePermitsIopsInput(volumeType) { + options.Iops = aws.Int64(int64(v.(int))) + } + if v, ok := m["throughput"]; ok && v.(int) > 0 && EBSVolumeTypePermitsThroughputInput(volumeType) { + options.Throughput = aws.Int64(int64(v.(int))) } } }