Skip to content

Commit

Permalink
r/aws_elasticsearch_domain: omit iops/throughput when not supported
Browse files Browse the repository at this point in the history
  • Loading branch information
@gracevivi523
gracevivi523 committed Jul 24, 2023
1 parent be9d5ac commit a544d5e
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 10 deletions.
29 changes: 29 additions & 0 deletions internal/service/elasticsearch/domain.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/elasticsearchservice"
elasticsearch "github.com/aws/aws-sdk-go/service/elasticsearchservice"
"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
awspolicy "github.com/hashicorp/awspolicyequivalence"
Expand Down Expand Up @@ -1249,3 +1250,31 @@ func advancedOptionsIgnoreDefault(o map[string]interface{}, n map[string]interfa

return n
}

// 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{elasticsearchservice.VolumeTypeGp3, elasticsearchservice.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{elasticsearchservice.VolumeTypeGp3}
for _, t := range permittedTypes {
if volumeType == t {
return true
}
}
return false
}
144 changes: 141 additions & 3 deletions internal/service/elasticsearch/domain_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import (

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/cognitoidentityprovider"
"github.com/aws/aws-sdk-go/service/elasticsearchservice"
elasticsearch "github.com/aws/aws-sdk-go/service/elasticsearchservice"
"github.com/aws/aws-sdk-go/service/opensearchservice"
sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/terraform"
Expand All @@ -22,6 +24,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", elasticsearchservice.VolumeTypeGp2, false},
{"gp3", elasticsearchservice.VolumeTypeGp3, true},
{"io1", elasticsearchservice.VolumeTypeIo1, true},
{"standard", elasticsearchservice.VolumeTypeStandard, false},
}
for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

if got := tfelasticsearch.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", elasticsearchservice.VolumeTypeGp2, false},
{"gp3", elasticsearchservice.VolumeTypeGp3, true},
{"io1", elasticsearchservice.VolumeTypeIo1, false},
{"standard", elasticsearchservice.VolumeTypeStandard, false},
}
for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

if got := tfelasticsearch.EBSVolumeTypePermitsThroughputInput(testCase.volumeType); got != testCase.want {
t.Errorf("EBSVolumeTypePermitsThroughputInput() = %v, want %v", got, testCase.want)
}
})
}
}

func TestAccElasticsearchDomain_basic(t *testing.T) {
ctx := acctest.Context(t)
if testing.Short() {
Expand Down Expand Up @@ -1429,6 +1483,55 @@ func TestAccElasticsearchDomain_VolumeType_update(t *testing.T) {
}})
}

// Verifies that EBS volume_type can be 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/32613
func TestAccElasticsearchDomain_VolumeType_gp3ToGP2(t *testing.T) {
ctx := acctest.Context(t)
if testing.Short() {
t.Skip("skipping long-running test in short mode")
}

var input elasticsearch.ElasticsearchDomainStatus
rName := testAccRandomDomainName()
resourceName := "aws_elasticsearch_domain.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheckIAMServiceLinkedRole(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, opensearchservice.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckDomainDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccDomainConfig_clusterEBSVolumeGP3DefaultIopsThroughput(rName, 10),
Check: resource.ComposeTestCheckFunc(
testAccCheckDomainExists(ctx, 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(ctx, 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 TestAccElasticsearchDomain_VolumeType_missing(t *testing.T) {
if testing.Short() {
Expand Down Expand Up @@ -2117,6 +2220,41 @@ resource "aws_elasticsearch_domain" "test" {
`, rName, volumeSize, volumeThroughput, volumeIops)
}

func testAccDomainConfig_clusterEBSVolumeGP3DefaultIopsThroughput(rName string, volumeSize int) string {
return fmt.Sprintf(`
resource "aws_elasticsearch_domain" "test" {
domain_name = %[1]q
elasticsearch_version = "7.10"
ebs_options {
ebs_enabled = true
volume_size = %[2]d
volume_type = "gp3"
}
cluster_config {
instance_type = "t3.small.elasticsearch"
}
}
`, rName, volumeSize)
}

func testAccDomainConfig_clusterEBSVolumeGP2(rName string, volumeSize int) string {
return fmt.Sprintf(`
resource "aws_elasticsearch_domain" "test" {
domain_name = %[1]q
elasticsearch_version = "7.10"
ebs_options {
ebs_enabled = true
volume_size = %[2]d
volume_type = "gp2"
}
cluster_config {
instance_type = "t3.small.elasticsearch"
}
}
`, rName, volumeSize)
}

func testAccDomainConfig_clusterUpdateVersion(rName, version string) string {
return fmt.Sprintf(`
resource "aws_elasticsearch_domain" "test" {
Expand Down Expand Up @@ -2915,16 +3053,16 @@ func testAccDomainConfig_logPublishingOptions(rName, logType string) string {
master_user_password = "Barbarbarbar1!"
}
}
domain_endpoint_options {
enforce_https = true
tls_security_policy = "Policy-Min-TLS-1-2-2019-07"
}
encrypt_at_rest {
enabled = true
}
node_to_node_encryption {
enabled = true
}`
Expand Down
17 changes: 10 additions & 7 deletions internal/service/elasticsearch/flex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,21 @@ func expandEBSOptions(m map[string]interface{}) *elasticsearch.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)))
}

}
}

Expand Down

0 comments on commit a544d5e

Please sign in to comment.