Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove last empty string validation #22954

Merged
merged 11 commits into from
Feb 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changelog/22954.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
```release-note:breaking-change
resource/aws_efs_mount_target: `ip_address` can no longer be set to `""`; instead, remove or set to `null`
```

```release-note:breaking-change
resource/aws_elasticsearch_domain: `ebs_options.0.volume_type` can no longer be set to `""`; instead, remove or set to `null`
```

```release-note:breaking-change
resource/aws_cloudwatch_event_target: `ecs_target.0.launch_type` can no longer be set to `""`; instead, remove or set to `null`
```
10 changes: 10 additions & 0 deletions .semgrep.yml
Original file line number Diff line number Diff line change
Expand Up @@ -683,3 +683,13 @@ rules:
metavariable: "$FUNC"
regex: "^resource\\w*(Delete|Disable)$"
severity: WARNING

- id: avoid-string-is-empty-validation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

languages: [go]
message: Empty strings should not be included in validation
paths:
include:
- internal/
patterns:
- pattern: validation.Any(..., validation.StringIsEmpty, ...)
severity: ERROR
13 changes: 5 additions & 8 deletions internal/service/efs/mount_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,11 @@ func ResourceMountTarget() *schema.Resource {
},

"ip_address": {
Type: schema.TypeString,
Computed: true,
Optional: true,
ForceNew: true,
ValidateFunc: validation.Any(
validation.IsIPv4Address,
validation.StringIsEmpty,
),
Type: schema.TypeString,
Computed: true,
Optional: true,
ForceNew: true,
ValidateFunc: validation.IsIPv4Address,
},

"security_groups": {
Expand Down
61 changes: 53 additions & 8 deletions internal/service/efs/mount_target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ func TestAccEFSMountTarget_disappears(t *testing.T) {
func TestAccEFSMountTarget_ipAddress(t *testing.T) {
var mount efs.MountTargetDescription
resourceName := "aws_efs_mount_target.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
Expand All @@ -96,7 +97,7 @@ func TestAccEFSMountTarget_ipAddress(t *testing.T) {
CheckDestroy: testAccCheckEfsMountTargetDestroy,
Steps: []resource.TestStep{
{
Config: testAccMountTargetIPAddressConfig("10.0.0.100"),
Config: testAccMountTargetIPAddressConfig(rName, "10.0.0.100"),
Check: resource.ComposeTestCheckFunc(
testAccCheckEfsMountTarget(resourceName, &mount),
resource.TestCheckResourceAttr(resourceName, "ip_address", "10.0.0.100"),
Expand All @@ -115,6 +116,7 @@ func TestAccEFSMountTarget_ipAddress(t *testing.T) {
func TestAccEFSMountTarget_IPAddress_emptyString(t *testing.T) {
var mount efs.MountTargetDescription
resourceName := "aws_efs_mount_target.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
Expand All @@ -123,7 +125,7 @@ func TestAccEFSMountTarget_IPAddress_emptyString(t *testing.T) {
CheckDestroy: testAccCheckEfsMountTargetDestroy,
Steps: []resource.TestStep{
{
Config: testAccMountTargetIPAddressConfig(""),
Config: testAccMountTargetIPAddressConfigNullIP(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckEfsMountTarget(resourceName, &mount),
resource.TestMatchResourceAttr(resourceName, "ip_address", regexp.MustCompile(`\d+\.\d+\.\d+\.\d+`)),
Expand Down Expand Up @@ -320,7 +322,7 @@ resource "aws_subnet" "test2" {
`, ct)
}

func testAccMountTargetIPAddressConfig(ipAddress string) string {
func testAccMountTargetIPAddressConfig(rName, ipAddress string) string {
return fmt.Sprintf(`
data "aws_availability_zones" "available" {
state = "available"
Expand All @@ -335,7 +337,7 @@ resource "aws_vpc" "test" {
cidr_block = "10.0.0.0/16"

tags = {
Name = "tf-acc-efs-mount-target-test"
Name = %[1]q
}
}

Expand All @@ -345,22 +347,65 @@ resource "aws_subnet" "test" {
cidr_block = "10.0.0.0/24"

tags = {
Name = "tf-acc-efs-mount-target-test"
Name = %[1]q
}
}

resource "aws_efs_file_system" "test" {
tags = {
Name = "tf-acc-efs-mount-target-test"
Name = %[1]q
}
}

resource "aws_efs_mount_target" "test" {
file_system_id = aws_efs_file_system.test.id
ip_address = %[2]q
subnet_id = aws_subnet.test.id
}
`, rName, ipAddress)
}

func testAccMountTargetIPAddressConfigNullIP(rName string) string {
return fmt.Sprintf(`
data "aws_availability_zones" "available" {
state = "available"

filter {
name = "opt-in-status"
values = ["opt-in-not-required"]
}
}

resource "aws_vpc" "test" {
cidr_block = "10.0.0.0/16"

tags = {
Name = %[1]q
}
}

resource "aws_subnet" "test" {
vpc_id = aws_vpc.test.id
availability_zone = data.aws_availability_zones.available.names[0]
cidr_block = "10.0.0.0/24"

tags = {
Name = %[1]q
}
}

resource "aws_efs_file_system" "test" {
tags = {
Name = %[1]q
}
}

resource "aws_efs_mount_target" "test" {
file_system_id = aws_efs_file_system.test.id
ip_address = %[1]q
ip_address = null
subnet_id = aws_subnet.test.id
}
`, ipAddress)
`, rName)
}

func testAccCheckVpnGatewayDestroy(s *terraform.State) error {
Expand Down
11 changes: 4 additions & 7 deletions internal/service/elasticsearch/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,13 +264,10 @@ func ResourceDomain() *schema.Resource {
Optional: true,
},
"volume_type": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validation.Any(
validation.StringIsEmpty,
validation.StringInSlice(elasticsearch.VolumeType_Values(), false),
),
Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validation.StringInSlice(elasticsearch.VolumeType_Values(), false),
},
},
},
Expand Down
8 changes: 8 additions & 0 deletions internal/service/elasticsearch/domain_data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import (
)

func TestAccElasticsearchDomainDataSource_Data_basic(t *testing.T) {
if testing.Short() {
t.Skip("skipping long-running test in short mode")
}

rInt := sdkacctest.RandInt()
autoTuneStartAtTime := testAccGetValidStartAtTime(t, "24h")
datasourceName := "data.aws_elasticsearch_domain.test"
Expand Down Expand Up @@ -49,6 +53,10 @@ func TestAccElasticsearchDomainDataSource_Data_basic(t *testing.T) {
}

func TestAccElasticsearchDomainDataSource_Data_advanced(t *testing.T) {
if testing.Short() {
t.Skip("skipping long-running test in short mode")
}

rInt := sdkacctest.RandInt()
autoTuneStartAtTime := testAccGetValidStartAtTime(t, "24h")
datasourceName := "data.aws_elasticsearch_domain.test"
Expand Down
Loading