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

Refactor API Gateway Stage Resource - use keyvaluetags + enums #10570

Merged
merged 6 commits into from
Nov 8, 2019
Merged

Refactor API Gateway Stage Resource - use keyvaluetags + enums #10570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
31f9449
Merge pull request #1 from terraform-providers/master
DrFaust92 Oct 8, 2019
d4284c7
Merge pull request #2 from terraform-providers/master
DrFaust92 Oct 11, 2019
aca775e
Merge pull request #3 from terraform-providers/master
DrFaust92 Oct 11, 2019
60418c6
Add tag support to api gateway usage plan
DrFaust92 Oct 20, 2019
61eddee
Revert "Add tag support to api gateway usage plan"
DrFaust92 Oct 20, 2019
60046c8
refactor api gateway stage to use keyvaluetags
DrFaust92 Oct 20, 2019
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
20 changes: 20 additions & 0 deletions aws/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,26 @@ func testAccMatchResourceAttrRegionalARN(resourceName, attributeName, arnService
}
}

// testAccMatchResourceAttrRegionalARN ensures the Terraform state regexp matches a formatted ARN with region and no account id
func testAccMatchResourceAttrRegionalARNNoAccount(resourceName, attributeName, arnService string, arnResourceRegexp *regexp.Regexp) resource.TestCheckFunc {
return func(s *terraform.State) error {
arnRegexp := arn.ARN{
Partition: testAccGetPartition(),
Region: testAccGetRegion(),
Resource: arnResourceRegexp.String(),
Service: arnService,
}.String()

attributeMatch, err := regexp.Compile(arnRegexp)

if err != nil {
return fmt.Errorf("Unable to compile ARN regexp (%s): %s", arnRegexp, err)
}

return resource.TestMatchResourceAttr(resourceName, attributeName, attributeMatch)(s)
}
}

// testAccCheckResourceAttrGlobalARN ensures the Terraform state exactly matches a formatted ARN without region
func testAccCheckResourceAttrGlobalARN(resourceName, attributeName, arnService, arnResource string) resource.TestCheckFunc {
return func(s *terraform.State) error {
Expand Down
66 changes: 44 additions & 22 deletions aws/resource_aws_api_gateway_stage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/aws/aws-sdk-go/service/apigateway"
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

func resourceAwsApiGatewayStage() *schema.Resource {
Expand Down Expand Up @@ -64,6 +66,16 @@ func resourceAwsApiGatewayStage() *schema.Resource {
"cache_cluster_size": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringInSlice([]string{
apigateway.CacheClusterSize05,
apigateway.CacheClusterSize16,
apigateway.CacheClusterSize61,
apigateway.CacheClusterSize118,
apigateway.CacheClusterSize135,
apigateway.CacheClusterSize237,
apigateway.CacheClusterSize284,
apigateway.CacheClusterSize582,
}, true),
},
"client_certificate_id": {
Type: schema.TypeString,
Expand Down Expand Up @@ -108,6 +120,10 @@ func resourceAwsApiGatewayStage() *schema.Resource {
Type: schema.TypeBool,
Optional: true,
},
"arn": {
Type: schema.TypeString,
Computed: true,
},
},
}
}
Expand Down Expand Up @@ -148,12 +164,8 @@ func resourceAwsApiGatewayStageCreate(d *schema.ResourceData, meta interface{})
}
input.Variables = aws.StringMap(variables)
}
if vars, ok := d.GetOk("tags"); ok {
newMap := make(map[string]string, len(vars.(map[string]interface{})))
for k, v := range vars.(map[string]interface{}) {
newMap[k] = v.(string)
}
input.Tags = aws.StringMap(newMap)
if v, ok := d.GetOk("tags"); ok {
input.Tags = keyvaluetags.New(v.(map[string]interface{})).IgnoreAws().ApigatewayTags()
}

out, err := conn.CreateStage(&input)
Expand All @@ -170,14 +182,14 @@ func resourceAwsApiGatewayStageCreate(d *schema.ResourceData, meta interface{})
d.SetPartial("variables")
d.SetPartial("xray_tracing_enabled")

if waitForCache && *out.CacheClusterStatus != "NOT_AVAILABLE" {
if waitForCache && *out.CacheClusterStatus != apigateway.CacheClusterStatusNotAvailable {
stateConf := &resource.StateChangeConf{
Pending: []string{
"CREATE_IN_PROGRESS",
"DELETE_IN_PROGRESS",
"FLUSH_IN_PROGRESS",
apigateway.CacheClusterStatusCreateInProgress,
apigateway.CacheClusterStatusDeleteInProgress,
apigateway.CacheClusterStatusFlushInProgress,
},
Target: []string{"AVAILABLE"},
Target: []string{apigateway.CacheClusterStatusAvailable},
Refresh: apiGatewayStageCacheRefreshFunc(conn,
d.Get("rest_api_id").(string),
d.Get("stage_name").(string)),
Expand Down Expand Up @@ -215,7 +227,7 @@ func resourceAwsApiGatewayStageRead(d *schema.ResourceData, meta interface{}) er
}
stage, err := conn.GetStage(&input)
if err != nil {
if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "NotFoundException" {
if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == apigateway.ErrCodeNotFoundException {
log.Printf("[WARN] API Gateway Stage (%s) not found, removing from state", d.Id())
d.SetId("")
return nil
Expand All @@ -230,7 +242,7 @@ func resourceAwsApiGatewayStageRead(d *schema.ResourceData, meta interface{}) er

d.Set("client_certificate_id", stage.ClientCertificateId)

if stage.CacheClusterStatus != nil && *stage.CacheClusterStatus == "DELETE_IN_PROGRESS" {
if stage.CacheClusterStatus != nil && *stage.CacheClusterStatus == apigateway.CacheClusterStatusDeleteInProgress {
d.Set("cache_cluster_enabled", false)
d.Set("cache_cluster_size", nil)
} else {
Expand All @@ -243,10 +255,18 @@ func resourceAwsApiGatewayStageRead(d *schema.ResourceData, meta interface{}) er
d.Set("documentation_version", stage.DocumentationVersion)
d.Set("xray_tracing_enabled", stage.TracingEnabled)

if err := d.Set("tags", aws.StringValueMap(stage.Tags)); err != nil {
if err := d.Set("tags", keyvaluetags.ApigatewayKeyValueTags(stage.Tags).IgnoreAws().Map()); err != nil {
return fmt.Errorf("error setting tags: %s", err)
}

stageArn := arn.ARN{
Partition: meta.(*AWSClient).partition,
Region: meta.(*AWSClient).region,
Service: "apigateway",
Resource: fmt.Sprintf("/restapis/%s/stages/%s", d.Get("rest_api_id").(string), d.Get("stage_name").(string)),
}.String()
d.Set("arn", stageArn)

if err := d.Set("variables", aws.StringValueMap(stage.Variables)); err != nil {
return fmt.Errorf("error setting variables: %s", err)
}
Expand Down Expand Up @@ -277,10 +297,12 @@ func resourceAwsApiGatewayStageUpdate(d *schema.ResourceData, meta interface{})
Service: "apigateway",
Resource: fmt.Sprintf("/restapis/%s/stages/%s", d.Get("rest_api_id").(string), d.Get("stage_name").(string)),
}.String()
if tagErr := setTagsAPIGatewayStage(conn, d, stageArn); tagErr != nil {
return tagErr
if d.HasChange("tags") {
o, n := d.GetChange("tags")
if err := keyvaluetags.ApigatewayUpdateTags(conn, stageArn, o, n); err != nil {
return fmt.Errorf("error updating tags: %s", err)
}
}
d.SetPartial("tags")

operations := make([]*apigateway.PatchOperation, 0)
waitForCache := false
Expand Down Expand Up @@ -380,17 +402,17 @@ func resourceAwsApiGatewayStageUpdate(d *schema.ResourceData, meta interface{})
d.SetPartial("xray_tracing_enabled")
d.SetPartial("variables")

if waitForCache && *out.CacheClusterStatus != "NOT_AVAILABLE" {
if waitForCache && *out.CacheClusterStatus != apigateway.CacheClusterStatusNotAvailable {
stateConf := &resource.StateChangeConf{
Pending: []string{
"CREATE_IN_PROGRESS",
"FLUSH_IN_PROGRESS",
apigateway.CacheClusterStatusCreateInProgress,
apigateway.CacheClusterStatusFlushInProgress,
},
Target: []string{
"AVAILABLE",
apigateway.CacheClusterStatusAvailable,
// There's an AWS API bug (raised & confirmed in Sep 2016 by support)
// which causes the stage to remain in deletion state forever
"DELETE_IN_PROGRESS",
apigateway.CacheClusterStatusDeleteInProgress,
},
Refresh: apiGatewayStageCacheRefreshFunc(conn,
d.Get("rest_api_id").(string),
Expand Down
99 changes: 57 additions & 42 deletions aws/resource_aws_api_gateway_stage_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
func TestAccAWSAPIGatewayStage_basic(t *testing.T) {
var conf apigateway.Stage
rName := acctest.RandString(5)
resourceName := "aws_api_gateway_stage.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -25,43 +26,51 @@ func TestAccAWSAPIGatewayStage_basic(t *testing.T) {
{
Config: testAccAWSAPIGatewayStageConfig_basic(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAPIGatewayStageExists("aws_api_gateway_stage.test", &conf),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "stage_name", "prod"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "cache_cluster_enabled", "true"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "cache_cluster_size", "0.5"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "tags.%", "1"),
resource.TestCheckResourceAttrSet("aws_api_gateway_stage.test", "execution_arn"),
resource.TestCheckResourceAttrSet("aws_api_gateway_stage.test", "invoke_url"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "xray_tracing_enabled", "true"),
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "stage_name", "prod"),
resource.TestCheckResourceAttr(resourceName, "cache_cluster_enabled", "true"),
resource.TestCheckResourceAttr(resourceName, "cache_cluster_size", "0.5"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", "tf-test"),
resource.TestCheckResourceAttrSet(resourceName, "execution_arn"),
resource.TestCheckResourceAttrSet(resourceName, "invoke_url"),
resource.TestCheckResourceAttr(resourceName, "xray_tracing_enabled", "true"),
),
},
{
ResourceName: "aws_api_gateway_stage.test",
ResourceName: resourceName,
ImportState: true,
ImportStateIdFunc: testAccAWSAPIGatewayStageImportStateIdFunc("aws_api_gateway_stage.test"),
ImportStateIdFunc: testAccAWSAPIGatewayStageImportStateIdFunc(resourceName),
ImportStateVerify: true,
},
{
Config: testAccAWSAPIGatewayStageConfig_updated(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAPIGatewayStageExists("aws_api_gateway_stage.test", &conf),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "stage_name", "prod"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "cache_cluster_enabled", "false"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "tags.%", "2"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "xray_tracing_enabled", "false"),
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "stage_name", "prod"),
resource.TestCheckResourceAttr(resourceName, "cache_cluster_enabled", "false"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", "tf-test"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", "tf-test"),
resource.TestCheckResourceAttr(resourceName, "tags.ExtraName", "tf-test"),
resource.TestCheckResourceAttr(resourceName, "xray_tracing_enabled", "false"),
),
},
{
Config: testAccAWSAPIGatewayStageConfig_basic(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAPIGatewayStageExists("aws_api_gateway_stage.test", &conf),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "stage_name", "prod"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "cache_cluster_enabled", "true"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "cache_cluster_size", "0.5"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "tags.%", "1"),
resource.TestCheckResourceAttrSet("aws_api_gateway_stage.test", "execution_arn"),
resource.TestCheckResourceAttrSet("aws_api_gateway_stage.test", "invoke_url"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "xray_tracing_enabled", "true"),
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "stage_name", "prod"),
resource.TestCheckResourceAttr(resourceName, "cache_cluster_enabled", "true"),
resource.TestCheckResourceAttr(resourceName, "cache_cluster_size", "0.5"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", "tf-test"),
resource.TestCheckResourceAttrSet(resourceName, "execution_arn"),
resource.TestCheckResourceAttrSet(resourceName, "invoke_url"),
resource.TestCheckResourceAttr(resourceName, "xray_tracing_enabled", "true"),
),
},
},
Expand All @@ -71,6 +80,7 @@ func TestAccAWSAPIGatewayStage_basic(t *testing.T) {
func TestAccAWSAPIGatewayStage_accessLogSettings(t *testing.T) {
var conf apigateway.Stage
rName := acctest.RandString(5)
resourceName := "aws_api_gateway_stage.test"
logGroupArnRegex := regexp.MustCompile(fmt.Sprintf("^arn:[^:]+:logs:[^:]+:[^:]+:log-group:foo-bar-%s$", rName))
clf := `$context.identity.sourceIp $context.identity.caller $context.identity.user [$context.requestTime] "$context.httpMethod $context.resourcePath $context.protocol" $context.status $context.responseLength $context.requestId`
json := `{ "requestId":"$context.requestId", "ip": "$context.identity.sourceIp", "caller":"$context.identity.caller", "user":"$context.identity.user", "requestTime":"$context.requestTime", "httpMethod":"$context.httpMethod", "resourcePath":"$context.resourcePath", "status":"$context.status", "protocol":"$context.protocol", "responseLength":"$context.responseLength" }`
Expand All @@ -85,45 +95,50 @@ func TestAccAWSAPIGatewayStage_accessLogSettings(t *testing.T) {
{
Config: testAccAWSAPIGatewayStageConfig_accessLogSettings(rName, clf),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAPIGatewayStageExists("aws_api_gateway_stage.test", &conf),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "access_log_settings.#", "1"),
resource.TestMatchResourceAttr("aws_api_gateway_stage.test", "access_log_settings.0.destination_arn", logGroupArnRegex),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "access_log_settings.0.format", clf),
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.#", "1"),
resource.TestMatchResourceAttr(resourceName, "access_log_settings.0.destination_arn", logGroupArnRegex),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.0.format", clf),
),
},

{
Config: testAccAWSAPIGatewayStageConfig_accessLogSettings(rName, json),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAPIGatewayStageExists("aws_api_gateway_stage.test", &conf),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "access_log_settings.#", "1"),
resource.TestMatchResourceAttr("aws_api_gateway_stage.test", "access_log_settings.0.destination_arn", logGroupArnRegex),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "access_log_settings.0.format", json),
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.#", "1"),
resource.TestMatchResourceAttr(resourceName, "access_log_settings.0.destination_arn", logGroupArnRegex),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.0.format", json),
),
},
{
Config: testAccAWSAPIGatewayStageConfig_accessLogSettings(rName, xml),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAPIGatewayStageExists("aws_api_gateway_stage.test", &conf),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "access_log_settings.#", "1"),
resource.TestMatchResourceAttr("aws_api_gateway_stage.test", "access_log_settings.0.destination_arn", logGroupArnRegex),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "access_log_settings.0.format", xml),
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.#", "1"),
resource.TestMatchResourceAttr(resourceName, "access_log_settings.0.destination_arn", logGroupArnRegex),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.0.format", xml),
),
},
{
Config: testAccAWSAPIGatewayStageConfig_accessLogSettings(rName, csv),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAPIGatewayStageExists("aws_api_gateway_stage.test", &conf),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "access_log_settings.#", "1"),
resource.TestMatchResourceAttr("aws_api_gateway_stage.test", "access_log_settings.0.destination_arn", logGroupArnRegex),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "access_log_settings.0.format", csv),
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.#", "1"),
resource.TestMatchResourceAttr(resourceName, "access_log_settings.0.destination_arn", logGroupArnRegex),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.0.format", csv),
),
},
{
Config: testAccAWSAPIGatewayStageConfig_basic(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAPIGatewayStageExists("aws_api_gateway_stage.test", &conf),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "access_log_settings.#", "0"),
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.#", "0"),
),
},
},
Expand Down Expand Up @@ -179,7 +194,7 @@ func testAccCheckAWSAPIGatewayStageDestroy(s *terraform.State) error {
if !ok {
return err
}
if awsErr.Code() != "NotFoundException" {
if awsErr.Code() != apigateway.ErrCodeNotFoundException {
return err
}

Expand Down
44 changes: 0 additions & 44 deletions aws/tags_apigateway.go
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions website/docs/r/api_gateway_stage.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ In addition to all arguments above, the following attributes are exported:
* `execution_arn` - The execution ARN to be used in [`lambda_permission`](/docs/providers/aws/r/lambda_permission.html)'s `source_arn`
when allowing API Gateway to invoke a Lambda function,
e.g. `arn:aws:execute-api:eu-west-2:123456789012:z4675bid1j/prod`
* `arn` - Amazon Resource Name (ARN)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Looks like this is indented. Will fix on merge along with alphabetizing in the list.


## Import

Expand Down