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.

Sign up for GitHub

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

Jump to bottom

tests/data-source/aws_region: Remove hardcoded us-east-1 handling #16024

Merged
merged 1 commit into from
Nov 11, 2020
Merged

tests/data-source/aws_region: Remove hardcoded us-east-1 handling #16024

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
185 changes: 46 additions & 139 deletions aws/data_source_aws_region_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ package aws

import (
"fmt"
"os"
"regexp"
"testing"

"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
)

func TestFindRegionByEc2Endpoint(t *testing.T) {
Expand Down Expand Up @@ -75,12 +74,7 @@ func TestFindRegionByName(t *testing.T) {
}

func TestAccDataSourceAwsRegion_basic(t *testing.T) {
// Ensure we always get a consistent result
oldvar := os.Getenv("AWS_DEFAULT_REGION")
os.Setenv("AWS_DEFAULT_REGION", "us-east-1")
defer os.Setenv("AWS_DEFAULT_REGION", oldvar)

resourceName := "data.aws_region.test"
dataSourceName := "data.aws_region.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -89,197 +83,110 @@ func TestAccDataSourceAwsRegion_basic(t *testing.T) {
{
Config: testAccDataSourceAwsRegionConfig_empty,
Check: resource.ComposeTestCheckFunc(
testAccDataSourceAwsRegionCheck(resourceName),
resource.TestCheckResourceAttr(resourceName, "endpoint", "ec2.us-east-1.amazonaws.com"),
resource.TestCheckResourceAttr(resourceName, "name", "us-east-1"),
resource.TestCheckResourceAttr(resourceName, "description", "US East (N. Virginia)"),
resource.TestMatchResourceAttr(dataSourceName, "description", regexp.MustCompile(`^.+$`)),
Copy link
Member

Choose a reason for hiding this comment

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

Why not?

Suggested change
resource.TestMatchResourceAttr(dataSourceName, "description", regexp.MustCompile(`^.+$`)),
resource.TestMatchResourceAttrSet(dataSourceName, "description"),

Copy link
Contributor Author

@bflad bflad Nov 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

resource.TestMatchResourceAttr(dataSourceName, "description", regexp.MustCompile(`^.+$`)),

Ensures that the value has at least one character, while TestCheckResourceAttrSet() would accept an empty string. We could potentially introduce another testing helper into the SDK, for lack of a better quick name TestCheckResourceAttrNonEmptyString(), but there are probably some other helpers that should be considered along side anything like that similar in nature to some of our validation functions.

testAccCheckResourceAttrRegionalHostnameService(dataSourceName, "endpoint", ec2.EndpointsID),
resource.TestCheckResourceAttr(dataSourceName, "name", testAccGetRegion()),
),
},
},
})
}

func TestAccDataSourceAwsRegion_endpoint(t *testing.T) {
// Ensure we always get a consistent result
oldvar := os.Getenv("AWS_DEFAULT_REGION")
os.Setenv("AWS_DEFAULT_REGION", "us-east-1")
defer os.Setenv("AWS_DEFAULT_REGION", oldvar)

endpoint1 := "ec2.us-east-1.amazonaws.com"
endpoint2 := "ec2.us-east-2.amazonaws.com"
name1 := "us-east-1"
name2 := "us-east-2"
description1 := "US East (N. Virginia)"
description2 := "US East (Ohio)"
resourceName := "data.aws_region.test"
dataSourceName := "data.aws_region.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccDataSourceAwsRegionConfig_endpoint(endpoint1),
Check: resource.ComposeTestCheckFunc(
testAccDataSourceAwsRegionCheck(resourceName),
resource.TestCheckResourceAttr(resourceName, "endpoint", endpoint1),
resource.TestCheckResourceAttr(resourceName, "name", name1),
resource.TestCheckResourceAttr(resourceName, "description", description1),
),
},
{
Config: testAccDataSourceAwsRegionConfig_endpoint(endpoint2),
Config: testAccDataSourceAwsRegionConfig_endpoint(),
Check: resource.ComposeTestCheckFunc(
testAccDataSourceAwsRegionCheck(resourceName),
resource.TestCheckResourceAttr(resourceName, "endpoint", endpoint2),
resource.TestCheckResourceAttr(resourceName, "name", name2),
resource.TestCheckResourceAttr(resourceName, "description", description2),
resource.TestMatchResourceAttr(dataSourceName, "description", regexp.MustCompile(`^.+$`)),
Copy link
Member

Choose a reason for hiding this comment

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

Same question.

resource.TestMatchResourceAttr(dataSourceName, "endpoint", regexp.MustCompile(fmt.Sprintf("^ec2\\.[^.]+\\.%s$", testAccGetPartitionDNSSuffix()))),
Copy link
Member

Choose a reason for hiding this comment

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

Why not?

Suggested change
resource.TestMatchResourceAttr(dataSourceName, "endpoint", regexp.MustCompile(fmt.Sprintf("^ec2\\.[^.]+\\.%s$", testAccGetPartitionDNSSuffix()))),
rtestAccCheckResourceAttrRegionalHostnameService(dataSourceName, "endpoint", ec2.EndpointsID),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The configuration uses the following to allow it to be region-agnostic:

endpoint = "ec2.${tolist(data.aws_regions.test.names)[0]}.${data.aws_partition.test.dns_suffix}"

The first region is not guaranteed to match the region of the test provider instance due to how the DescribeRegions API works and we probably shouldn't be using aws_region in the testing for aws_region to try and align things. 😄

resource.TestMatchResourceAttr(dataSourceName, "name", regexp.MustCompile(`^.+$`)),
),
},
{
Config: testAccDataSourceAwsRegionConfig_endpoint("does-not-exist"),
ExpectError: regexp.MustCompile(`region not found for endpoint: does-not-exist`),
},
},
})
}

func TestAccDataSourceAwsRegion_endpointAndName(t *testing.T) {
// Ensure we always get a consistent result
oldvar := os.Getenv("AWS_DEFAULT_REGION")
os.Setenv("AWS_DEFAULT_REGION", "us-east-1")
defer os.Setenv("AWS_DEFAULT_REGION", oldvar)

endpoint1 := "ec2.us-east-1.amazonaws.com"
endpoint2 := "ec2.us-east-2.amazonaws.com"
name1 := "us-east-1"
name2 := "us-east-2"
description1 := "US East (N. Virginia)"
description2 := "US East (Ohio)"
resourceName := "data.aws_region.test"
dataSourceName := "data.aws_region.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccDataSourceAwsRegionConfig_endpointAndName(endpoint1, name1),
Check: resource.ComposeTestCheckFunc(
testAccDataSourceAwsRegionCheck(resourceName),
resource.TestCheckResourceAttr(resourceName, "endpoint", endpoint1),
resource.TestCheckResourceAttr(resourceName, "name", name1),
resource.TestCheckResourceAttr(resourceName, "description", description1),
),
},
{
Config: testAccDataSourceAwsRegionConfig_endpointAndName(endpoint2, name2),
Check: resource.ComposeTestCheckFunc(
testAccDataSourceAwsRegionCheck(resourceName),
resource.TestCheckResourceAttr(resourceName, "endpoint", endpoint2),
resource.TestCheckResourceAttr(resourceName, "name", name2),
resource.TestCheckResourceAttr(resourceName, "description", description2),
),
},
{
Config: testAccDataSourceAwsRegionConfig_endpointAndName(endpoint1, name1),
Config: testAccDataSourceAwsRegionConfig_endpointAndName(),
Check: resource.ComposeTestCheckFunc(
testAccDataSourceAwsRegionCheck(resourceName),
resource.TestCheckResourceAttr(resourceName, "endpoint", endpoint1),
resource.TestCheckResourceAttr(resourceName, "name", name1),
resource.TestCheckResourceAttr(resourceName, "description", description1),
resource.TestMatchResourceAttr(dataSourceName, "description", regexp.MustCompile(`^.+$`)),
resource.TestMatchResourceAttr(dataSourceName, "endpoint", regexp.MustCompile(fmt.Sprintf("^ec2\\.[^.]+\\.%s$", testAccGetPartitionDNSSuffix()))),
resource.TestMatchResourceAttr(dataSourceName, "name", regexp.MustCompile(`^.+$`)),
),
},
{
Config: testAccDataSourceAwsRegionConfig_endpointAndName(endpoint1, name2),
ExpectError: regexp.MustCompile(`multiple regions matched`),
},
{
Config: testAccDataSourceAwsRegionConfig_endpointAndName(endpoint2, name1),
ExpectError: regexp.MustCompile(`multiple regions matched`),
},
},
})
}

func TestAccDataSourceAwsRegion_name(t *testing.T) {
// Ensure we always get a consistent result
oldvar := os.Getenv("AWS_DEFAULT_REGION")
os.Setenv("AWS_DEFAULT_REGION", "us-east-1")
defer os.Setenv("AWS_DEFAULT_REGION", oldvar)

endpoint1 := "ec2.us-east-1.amazonaws.com"
endpoint2 := "ec2.us-east-2.amazonaws.com"
name1 := "us-east-1"
name2 := "us-east-2"
description1 := "US East (N. Virginia)"
description2 := "US East (Ohio)"
resourceName := "data.aws_region.test"
dataSourceName := "data.aws_region.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccDataSourceAwsRegionConfig_name(name1),
Config: testAccDataSourceAwsRegionConfig_name(),
Check: resource.ComposeTestCheckFunc(
testAccDataSourceAwsRegionCheck(resourceName),
resource.TestCheckResourceAttr(resourceName, "endpoint", endpoint1),
resource.TestCheckResourceAttr(resourceName, "name", name1),
resource.TestCheckResourceAttr(resourceName, "description", description1),
resource.TestMatchResourceAttr(dataSourceName, "description", regexp.MustCompile(`^.+$`)),
resource.TestMatchResourceAttr(dataSourceName, "endpoint", regexp.MustCompile(fmt.Sprintf("^ec2\\.[^.]+\\.%s$", testAccGetPartitionDNSSuffix()))),
resource.TestMatchResourceAttr(dataSourceName, "name", regexp.MustCompile(`^.+$`)),
),
},
{
Config: testAccDataSourceAwsRegionConfig_name(name2),
Check: resource.ComposeTestCheckFunc(
testAccDataSourceAwsRegionCheck(resourceName),
resource.TestCheckResourceAttr(resourceName, "endpoint", endpoint2),
resource.TestCheckResourceAttr(resourceName, "name", name2),
resource.TestCheckResourceAttr(resourceName, "description", description2),
),
},
{
Config: testAccDataSourceAwsRegionConfig_name("does-not-exist"),
ExpectError: regexp.MustCompile(`region not found for name: does-not-exist`),
},
},
})
}

func testAccDataSourceAwsRegionCheck(name string) resource.TestCheckFunc {
return func(s *terraform.State) error {
_, ok := s.RootModule().Resources[name]
if !ok {
return fmt.Errorf("root module has no resource called %s", name)
}

return nil
}
}

const testAccDataSourceAwsRegionConfig_empty = `
data "aws_region" "test" {}
`

func testAccDataSourceAwsRegionConfig_endpoint(endpoint string) string {
return fmt.Sprintf(`
func testAccDataSourceAwsRegionConfig_endpoint() string {
return `
data "aws_partition" "test" {}

data "aws_regions" "test" {
}

data "aws_region" "test" {
endpoint = "%s"
endpoint = "ec2.${tolist(data.aws_regions.test.names)[0]}.${data.aws_partition.test.dns_suffix}"
}
`, endpoint)
`
}

func testAccDataSourceAwsRegionConfig_endpointAndName() string {
return `
data "aws_partition" "test" {}

data "aws_regions" "test" {
}

func testAccDataSourceAwsRegionConfig_endpointAndName(endpoint, name string) string {
return fmt.Sprintf(`
data "aws_region" "test" {
endpoint = "%s"
name = "%s"
endpoint = "ec2.${tolist(data.aws_regions.test.names)[0]}.${data.aws_partition.test.dns_suffix}"
name = tolist(data.aws_regions.test.names)[0]
}
`, endpoint, name)
`
}

func testAccDataSourceAwsRegionConfig_name() string {
return `
data "aws_regions" "test" {
}

func testAccDataSourceAwsRegionConfig_name(name string) string {
return fmt.Sprintf(`
data "aws_region" "test" {
name = "%s"
name = tolist(data.aws_regions.test.names)[0]
}
`, name)
`
}