From 4da1231370920bd44893ddf24db1d28e320f0dd5 Mon Sep 17 00:00:00 2001 From: DrFaust92 Date: Sat, 25 Jul 2020 13:07:00 +0300 Subject: [PATCH 1/9] change api_stages to set extract lists/set expansion to functions move usage plan flattners to usage plan file refactor tests --- aws/resource_aws_api_gateway_usage_plan.go | 242 ++++++++++++------ ...esource_aws_api_gateway_usage_plan_test.go | 225 ++++++++-------- aws/structure.go | 60 ----- 3 files changed, 284 insertions(+), 243 deletions(-) diff --git a/aws/resource_aws_api_gateway_usage_plan.go b/aws/resource_aws_api_gateway_usage_plan.go index a67076299d00..f6627a2ccade 100644 --- a/aws/resource_aws_api_gateway_usage_plan.go +++ b/aws/resource_aws_api_gateway_usage_plan.go @@ -36,7 +36,7 @@ func resourceAwsApiGatewayUsagePlan() *schema.Resource { }, "api_stages": { - Type: schema.TypeList, + Type: schema.TypeSet, Optional: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -90,15 +90,17 @@ func resourceAwsApiGatewayUsagePlan() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "burst_limit": { - Type: schema.TypeInt, - Default: 0, - Optional: true, + Type: schema.TypeInt, + Default: 0, + Optional: true, + AtLeastOneOf: []string{"throttle_settings.0.burst_limit", "throttle_settings.0.rate_limit"}, }, "rate_limit": { - Type: schema.TypeFloat, - Default: 0, - Optional: true, + Type: schema.TypeFloat, + Default: 0, + Optional: true, + AtLeastOneOf: []string{"throttle_settings.0.burst_limit", "throttle_settings.0.rate_limit"}, }, }, }, @@ -129,78 +131,27 @@ func resourceAwsApiGatewayUsagePlanCreate(d *schema.ResourceData, meta interface params.Description = aws.String(v.(string)) } - if s, ok := d.GetOk("api_stages"); ok { - stages := s.([]interface{}) - as := make([]*apigateway.ApiStage, 0) - - for _, v := range stages { - sv := v.(map[string]interface{}) - stage := &apigateway.ApiStage{} - - if v, ok := sv["api_id"].(string); ok && v != "" { - stage.ApiId = aws.String(v) - } - - if v, ok := sv["stage"].(string); ok && v != "" { - stage.Stage = aws.String(v) - } - - as = append(as, stage) - } - - if len(as) > 0 { - params.ApiStages = as - } + if v, ok := d.GetOk("api_stages"); ok && v.(*schema.Set).Len() > 0 { + params.ApiStages = expandApiGatewayUsageApiStages(v.(*schema.Set)) } if v, ok := d.GetOk("quota_settings"); ok { settings := v.([]interface{}) q, ok := settings[0].(map[string]interface{}) - if errors := validateApiGatewayUsagePlanQuotaSettings(q); len(errors) > 0 { - return fmt.Errorf("Error validating the quota settings: %v", errors) + if err := validateApiGatewayUsagePlanQuotaSettings(q); len(err) > 0 { + return fmt.Errorf("error validating the quota settings: %v", err) } if !ok { return errors.New("At least one field is expected inside quota_settings") } - qs := &apigateway.QuotaSettings{} - - if sv, ok := q["limit"].(int); ok { - qs.Limit = aws.Int64(int64(sv)) - } - - if sv, ok := q["offset"].(int); ok { - qs.Offset = aws.Int64(int64(sv)) - } - - if sv, ok := q["period"].(string); ok && sv != "" { - qs.Period = aws.String(sv) - } - - params.Quota = qs + params.Quota = expandApiGatewayUsageQuotaSettings(v.([]interface{})) } if v, ok := d.GetOk("throttle_settings"); ok { - settings := v.([]interface{}) - q, ok := settings[0].(map[string]interface{}) - - if !ok { - return errors.New("At least one field is expected inside throttle_settings") - } - - ts := &apigateway.ThrottleSettings{} - - if sv, ok := q["burst_limit"].(int); ok { - ts.BurstLimit = aws.Int64(int64(sv)) - } - - if sv, ok := q["rate_limit"].(float64); ok { - ts.RateLimit = aws.Float64(sv) - } - - params.Throttle = ts + params.Throttle = expandApiGatewayUsageThrottleSettings(v.([]interface{})) } if v, ok := d.GetOk("tags"); ok { @@ -209,10 +160,10 @@ func resourceAwsApiGatewayUsagePlanCreate(d *schema.ResourceData, meta interface up, err := conn.CreateUsagePlan(params) if err != nil { - return fmt.Errorf("Error creating API Gateway Usage Plan: %s", err) + return fmt.Errorf("error creating API Gateway Usage Plan: %w", err) } - d.SetId(*up.Id) + d.SetId(aws.StringValue(up.Id)) // Handle case of adding the product code since not addable when // creating the Usage Plan initially. @@ -230,7 +181,7 @@ func resourceAwsApiGatewayUsagePlanCreate(d *schema.ResourceData, meta interface _, err = conn.UpdateUsagePlan(updateParameters) if err != nil { - return fmt.Errorf("Error creating the API Gateway Usage Plan product code: %s", err) + return fmt.Errorf("error creating the API Gateway Usage Plan product code: %w", err) } } @@ -256,7 +207,7 @@ func resourceAwsApiGatewayUsagePlanRead(d *schema.ResourceData, meta interface{} } if err := d.Set("tags", keyvaluetags.ApigatewayKeyValueTags(up.Tags).IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { - return fmt.Errorf("error setting tags: %s", err) + return fmt.Errorf("error setting tags: %w", err) } arn := arn.ARN{ @@ -273,19 +224,19 @@ func resourceAwsApiGatewayUsagePlanRead(d *schema.ResourceData, meta interface{} if up.ApiStages != nil { if err := d.Set("api_stages", flattenApiGatewayUsageApiStages(up.ApiStages)); err != nil { - return fmt.Errorf("Error setting api_stages error: %#v", err) + return fmt.Errorf("error setting api_stages error: %w", err) } } if up.Throttle != nil { if err := d.Set("throttle_settings", flattenApiGatewayUsagePlanThrottling(up.Throttle)); err != nil { - return fmt.Errorf("Error setting throttle_settings error: %#v", err) + return fmt.Errorf("error setting throttle_settings error: %w", err) } } if up.Quota != nil { if err := d.Set("quota_settings", flattenApiGatewayUsagePlanQuota(up.Quota)); err != nil { - return fmt.Errorf("Error setting quota_settings error: %#v", err) + return fmt.Errorf("error setting quota_settings error: %w", err) } } @@ -333,12 +284,12 @@ func resourceAwsApiGatewayUsagePlanUpdate(d *schema.ResourceData, meta interface if d.HasChange("api_stages") { o, n := d.GetChange("api_stages") - old := o.([]interface{}) - new := n.([]interface{}) + os := o.(*schema.Set).List() + ns := n.(*schema.Set).List() - // Remove every stages associated. Simpler to remove and add new ones, + // Remove every stages associated. Simpler to remove and add ns ones, // since there are no replacings. - for _, v := range old { + for _, v := range os { m := v.(map[string]interface{}) operations = append(operations, &apigateway.PatchOperation{ Op: aws.String(apigateway.OpRemove), @@ -348,8 +299,8 @@ func resourceAwsApiGatewayUsagePlanUpdate(d *schema.ResourceData, meta interface } // Handle additions - if len(new) > 0 { - for _, v := range new { + if len(ns) > 0 { + for _, v := range ns { m := v.(map[string]interface{}) operations = append(operations, &apigateway.PatchOperation{ Op: aws.String(apigateway.OpAdd), @@ -467,7 +418,7 @@ func resourceAwsApiGatewayUsagePlanUpdate(d *schema.ResourceData, meta interface if d.HasChange("tags") { o, n := d.GetChange("tags") if err := keyvaluetags.ApigatewayUpdateTags(conn, d.Get("arn").(string), o, n); err != nil { - return fmt.Errorf("error updating tags: %s", err) + return fmt.Errorf("error updating tags: %w", err) } } @@ -478,7 +429,7 @@ func resourceAwsApiGatewayUsagePlanUpdate(d *schema.ResourceData, meta interface _, err := conn.UpdateUsagePlan(params) if err != nil { - return fmt.Errorf("Error updating API Gateway Usage Plan: %s", err) + return fmt.Errorf("error updating API Gateway Usage Plan: %w", err) } return resourceAwsApiGatewayUsagePlanRead(d, meta) @@ -490,10 +441,10 @@ func resourceAwsApiGatewayUsagePlanDelete(d *schema.ResourceData, meta interface // Removing existing api stages associated if apistages, ok := d.GetOk("api_stages"); ok { log.Printf("[DEBUG] Deleting API Stages associated with Usage Plan: %s", d.Id()) - stages := apistages.([]interface{}) + stages := apistages.(*schema.Set) operations := []*apigateway.PatchOperation{} - for _, v := range stages { + for _, v := range stages.List() { sv := v.(map[string]interface{}) operations = append(operations, &apigateway.PatchOperation{ @@ -508,7 +459,7 @@ func resourceAwsApiGatewayUsagePlanDelete(d *schema.ResourceData, meta interface PatchOperations: operations, }) if err != nil { - return fmt.Errorf("Error removing API Stages associated with Usage Plan: %s", err) + return fmt.Errorf("error removing API Stages associated with Usage Plan: %w", err) } } @@ -519,9 +470,134 @@ func resourceAwsApiGatewayUsagePlanDelete(d *schema.ResourceData, meta interface }) if err != nil { - return fmt.Errorf("Error deleting API gateway usage plan: %s", err) + return fmt.Errorf("error deleting API gateway usage plan: %w", err) + } + + return nil + +} + +func expandApiGatewayUsageApiStages(s *schema.Set) []*apigateway.ApiStage { + stages := []*apigateway.ApiStage{} + + for _, stageRaw := range s.List() { + stage := &apigateway.ApiStage{} + mStage := stageRaw.(map[string]interface{}) + + if v, ok := mStage["api_id"].(string); ok && v != "" { + stage.ApiId = aws.String(v) + } + + if v, ok := mStage["stage"].(string); ok && v != "" { + stage.Stage = aws.String(v) + } + + stages = append(stages, stage) + } + + return stages +} + +func expandApiGatewayUsageQuotaSettings(l []interface{}) *apigateway.QuotaSettings { + if len(l) == 0 { + return nil + } + + m := l[0].(map[string]interface{}) + + qs := &apigateway.QuotaSettings{} + + if v, ok := m["limit"].(int); ok { + qs.Limit = aws.Int64(int64(v)) + } + + if v, ok := m["offset"].(int); ok { + qs.Offset = aws.Int64(int64(v)) + } + + if v, ok := m["period"].(string); ok && v != "" { + qs.Period = aws.String(v) + } + + return qs +} + +func expandApiGatewayUsageThrottleSettings(l []interface{}) *apigateway.ThrottleSettings { + if len(l) == 0 { + return nil + } + + m := l[0].(map[string]interface{}) + + ts := &apigateway.ThrottleSettings{} + + if sv, ok := m["burst_limit"].(int); ok { + ts.BurstLimit = aws.Int64(int64(sv)) + } + + if sv, ok := m["rate_limit"].(float64); ok { + ts.RateLimit = aws.Float64(sv) + } + + return ts +} + +func flattenApiGatewayUsageApiStages(s []*apigateway.ApiStage) []map[string]interface{} { + stages := make([]map[string]interface{}, 0) + + for _, bd := range s { + if bd.ApiId != nil && bd.Stage != nil { + stage := make(map[string]interface{}) + stage["api_id"] = aws.StringValue(bd.ApiId) + stage["stage"] = aws.StringValue(bd.Stage) + + stages = append(stages, stage) + } + } + + if len(stages) > 0 { + return stages } return nil +} + +func flattenApiGatewayUsagePlanThrottling(s *apigateway.ThrottleSettings) []map[string]interface{} { + settings := make(map[string]interface{}) + + if s == nil { + return nil + } + + if s.BurstLimit != nil { + settings["burst_limit"] = aws.Int64Value(s.BurstLimit) + } + + if s.RateLimit != nil { + settings["rate_limit"] = aws.Float64Value(s.RateLimit) + } + + return []map[string]interface{}{settings} +} + +func flattenApiGatewayUsagePlanQuota(s *apigateway.QuotaSettings) []map[string]interface{} { + settings := make(map[string]interface{}) + + if s == nil { + return nil + } + + if s.Limit != nil { + settings["limit"] = aws.Int64Value(s.Limit) + } + + if s.Offset != nil { + settings["offset"] = aws.Int64Value(s.Offset) + } + + if s.Period != nil { + settings["period"] = aws.StringValue(s.Period) + } + return []map[string]interface{}{settings} } diff --git a/aws/resource_aws_api_gateway_usage_plan_test.go b/aws/resource_aws_api_gateway_usage_plan_test.go index 3e22639f49d1..0f63aae21611 100644 --- a/aws/resource_aws_api_gateway_usage_plan_test.go +++ b/aws/resource_aws_api_gateway_usage_plan_test.go @@ -5,17 +5,17 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/apigateway" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfawsresource" ) func TestAccAWSAPIGatewayUsagePlan_basic(t *testing.T) { var conf apigateway.UsagePlan - rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(10)) - updatedName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(10)) + rName := acctest.RandomWithPrefix("tf-acc-test") + updatedName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_api_gateway_usage_plan.test" resource.ParallelTest(t, resource.TestCase{ @@ -37,28 +37,20 @@ func TestAccAWSAPIGatewayUsagePlan_basic(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccAWSApiGatewayUsagePlanBasicUpdatedConfig(updatedName), + Config: testAccAWSApiGatewayUsagePlanBasicConfig(updatedName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "name", updatedName), resource.TestCheckResourceAttr(resourceName, "description", ""), ), }, - { - Config: testAccAWSApiGatewayUsagePlanBasicConfig(rName), - Check: resource.ComposeTestCheckFunc( - testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "name", rName), - resource.TestCheckResourceAttr(resourceName, "description", ""), - ), - }, }, }) } func TestAccAWSAPIGatewayUsagePlan_tags(t *testing.T) { var conf apigateway.UsagePlan - name := acctest.RandString(10) - resourceName := "aws_api_gateway_usage_plan.main" + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_api_gateway_usage_plan.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccAPIGatewayTypeEDGEPreCheck(t) }, @@ -66,10 +58,10 @@ func TestAccAWSAPIGatewayUsagePlan_tags(t *testing.T) { CheckDestroy: testAccCheckAWSAPIGatewayUsagePlanDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSApiGatewayUsagePlanBasicTags1(name, "key1", "value1"), + Config: testAccAWSApiGatewayUsagePlanBasicTags1(rName, "key1", "value1"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "name", name), + resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), ), @@ -80,19 +72,19 @@ func TestAccAWSAPIGatewayUsagePlan_tags(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccAWSApiGatewayUsagePlanBasicTags2(name, "key1", "value1updated", "key2", "value2"), + Config: testAccAWSApiGatewayUsagePlanBasicTags2(rName, "key1", "value1updated", "key2", "value2"), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "name", name), + resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), }, { - Config: testAccAWSApiGatewayUsagePlanBasicTags1(name, "key2", "value2"), + Config: testAccAWSApiGatewayUsagePlanBasicTags1(rName, "key2", "value2"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "name", name), + resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), @@ -103,7 +95,7 @@ func TestAccAWSAPIGatewayUsagePlan_tags(t *testing.T) { func TestAccAWSAPIGatewayUsagePlan_description(t *testing.T) { var conf apigateway.UsagePlan - rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(10)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_api_gateway_usage_plan.test" resource.ParallelTest(t, resource.TestCase{ @@ -124,20 +116,20 @@ func TestAccAWSAPIGatewayUsagePlan_description(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccAWSApiGatewayUsagePlanDescriptionConfig(rName), + Config: testAccAWSApiGatewayUsagePlanDescriptionConfig(rName, "This is a description"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), resource.TestCheckResourceAttr(resourceName, "description", "This is a description"), ), }, { - Config: testAccAWSApiGatewayUsagePlanDescriptionUpdatedConfig(rName), + Config: testAccAWSApiGatewayUsagePlanDescriptionConfig(rName, "This is a new description"), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "description", "This is a new description"), ), }, { - Config: testAccAWSApiGatewayUsagePlanDescriptionConfig(rName), + Config: testAccAWSApiGatewayUsagePlanDescriptionConfig(rName, "This is a description"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), resource.TestCheckResourceAttr(resourceName, "description", "This is a description"), @@ -156,7 +148,7 @@ func TestAccAWSAPIGatewayUsagePlan_description(t *testing.T) { func TestAccAWSAPIGatewayUsagePlan_productCode(t *testing.T) { var conf apigateway.UsagePlan - rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(10)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_api_gateway_usage_plan.test" resource.ParallelTest(t, resource.TestCase{ @@ -165,10 +157,10 @@ func TestAccAWSAPIGatewayUsagePlan_productCode(t *testing.T) { CheckDestroy: testAccCheckAWSAPIGatewayUsagePlanDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSApiGatewayUsagePlanBasicConfig(rName), + Config: testAccAWSApiGatewayUsagePlanProductCodeConfig(rName, "MYCODE"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "product_code", ""), + resource.TestCheckResourceAttr(resourceName, "product_code", "MYCODE"), ), }, { @@ -177,30 +169,24 @@ func TestAccAWSAPIGatewayUsagePlan_productCode(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccAWSApiGatewayUsagePlanProductCodeConfig(rName), + Config: testAccAWSApiGatewayUsagePlanProductCodeConfig(rName, "MYCODE2"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "product_code", "MYCODE"), - ), - }, - { - Config: testAccAWSApiGatewayUsagePlanProductCodeUpdatedConfig(rName), - Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "product_code", "MYCODE2"), ), }, { - Config: testAccAWSApiGatewayUsagePlanProductCodeConfig(rName), + Config: testAccAWSApiGatewayUsagePlanBasicConfig(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "product_code", "MYCODE"), + resource.TestCheckResourceAttr(resourceName, "product_code", ""), ), }, { - Config: testAccAWSApiGatewayUsagePlanBasicConfig(rName), + Config: testAccAWSApiGatewayUsagePlanProductCodeConfig(rName, "MYCODE"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), - resource.TestCheckResourceAttr(resourceName, "product_code", ""), + resource.TestCheckResourceAttr(resourceName, "product_code", "MYCODE"), ), }, }, @@ -209,7 +195,7 @@ func TestAccAWSAPIGatewayUsagePlan_productCode(t *testing.T) { func TestAccAWSAPIGatewayUsagePlan_throttling(t *testing.T) { var conf apigateway.UsagePlan - rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(10)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_api_gateway_usage_plan.test" resource.ParallelTest(t, resource.TestCase{ @@ -263,7 +249,7 @@ func TestAccAWSAPIGatewayUsagePlan_throttling(t *testing.T) { // https://github.com/terraform-providers/terraform-provider-aws/issues/2057 func TestAccAWSAPIGatewayUsagePlan_throttlingInitialRateLimit(t *testing.T) { var conf apigateway.UsagePlan - rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(10)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_api_gateway_usage_plan.test" resource.ParallelTest(t, resource.TestCase{ @@ -289,7 +275,7 @@ func TestAccAWSAPIGatewayUsagePlan_throttlingInitialRateLimit(t *testing.T) { func TestAccAWSAPIGatewayUsagePlan_quota(t *testing.T) { var conf apigateway.UsagePlan - rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(10)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_api_gateway_usage_plan.test" resource.ParallelTest(t, resource.TestCase{ @@ -344,7 +330,7 @@ func TestAccAWSAPIGatewayUsagePlan_quota(t *testing.T) { func TestAccAWSAPIGatewayUsagePlan_apiStages(t *testing.T) { var conf apigateway.UsagePlan - rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(10)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_api_gateway_usage_plan.test" resource.ParallelTest(t, resource.TestCase{ @@ -359,7 +345,9 @@ func TestAccAWSAPIGatewayUsagePlan_apiStages(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), resource.TestCheckResourceAttr(resourceName, "name", rName), - resource.TestCheckResourceAttr(resourceName, "api_stages.0.stage", "test"), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "api_stages.*", map[string]string{ + "stage": "test", + }), ), }, { @@ -382,16 +370,33 @@ func TestAccAWSAPIGatewayUsagePlan_apiStages(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), resource.TestCheckResourceAttr(resourceName, "name", rName), - resource.TestCheckResourceAttr(resourceName, "api_stages.0.stage", "test"), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "api_stages.*", map[string]string{ + "stage": "test", + }), ), }, // Handle api stages updates + { + Config: testAccAWSApiGatewayUsagePlanApiStagesMultipleConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "name", rName), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "api_stages.*", map[string]string{ + "stage": "foo", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "api_stages.*", map[string]string{ + "stage": "test", + }), + ), + }, { Config: testAccAWSApiGatewayUsagePlanApiStagesModifiedConfig(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), resource.TestCheckResourceAttr(resourceName, "name", rName), - resource.TestCheckResourceAttr(resourceName, "api_stages.0.stage", "foo"), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "api_stages.*", map[string]string{ + "stage": "foo", + }), ), }, { @@ -406,9 +411,41 @@ func TestAccAWSAPIGatewayUsagePlan_apiStages(t *testing.T) { }) } +func TestAccAWSAPIGatewayUsagePlan_apiStages_multiple(t *testing.T) { + var conf apigateway.UsagePlan + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_api_gateway_usage_plan.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAPIGatewayUsagePlanDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSApiGatewayUsagePlanApiStagesMultipleConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "name", rName), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "api_stages.*", map[string]string{ + "stage": "foo", + }), + tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "api_stages.*", map[string]string{ + "stage": "test", + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccAWSAPIGatewayUsagePlan_disappears(t *testing.T) { var conf apigateway.UsagePlan - rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(10)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_api_gateway_usage_plan.test" resource.ParallelTest(t, resource.TestCase{ @@ -449,7 +486,7 @@ func testAccCheckAWSAPIGatewayUsagePlanExists(n string, res *apigateway.UsagePla return err } - if *up.Id != rs.Primary.ID { + if aws.StringValue(up.Id) != rs.Primary.ID { return fmt.Errorf("APIGateway Usage Plan not found") } @@ -473,16 +510,12 @@ func testAccCheckAWSAPIGatewayUsagePlanDestroy(s *terraform.State) error { describe, err := conn.GetUsagePlan(req) if err == nil { - if describe.Id != nil && *describe.Id == rs.Primary.ID { + if describe.Id != nil && aws.StringValue(describe.Id) == rs.Primary.ID { return fmt.Errorf("API Gateway Usage Plan still exists") } } - aws2err, ok := err.(awserr.Error) - if !ok { - return err - } - if aws2err.Code() != apigateway.ErrCodeNotFoundException { + if !isAWSErr(err, apigateway.ErrCodeNotFoundException, "") { return err } @@ -561,7 +594,7 @@ resource "aws_api_gateway_deployment" "foo" { } func testAccAWSApiGatewayUsagePlanBasicConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` resource "aws_api_gateway_usage_plan" "test" { name = "%s" } @@ -569,8 +602,8 @@ resource "aws_api_gateway_usage_plan" "test" { } func testAccAWSApiGatewayUsagePlanBasicTags1(rName, tagKey1, tagValue1 string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` -resource "aws_api_gateway_usage_plan" "main" { + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` +resource "aws_api_gateway_usage_plan" "test" { name = "%s" tags = { @@ -581,8 +614,8 @@ resource "aws_api_gateway_usage_plan" "main" { } func testAccAWSApiGatewayUsagePlanBasicTags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` -resource "aws_api_gateway_usage_plan" "main" { + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` +resource "aws_api_gateway_usage_plan" "test" { name = "%s" tags = { @@ -593,52 +626,26 @@ resource "aws_api_gateway_usage_plan" "main" { `, rName, tagKey1, tagValue1, tagKey2, tagValue2) } -func testAccAWSApiGatewayUsagePlanDescriptionConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` +func testAccAWSApiGatewayUsagePlanDescriptionConfig(rName, desc string) string { + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` resource "aws_api_gateway_usage_plan" "test" { - name = "%s" - description = "This is a description" + name = %[1]q + description = %[2]q } -`, rName) +`, rName, desc) } -func testAccAWSApiGatewayUsagePlanDescriptionUpdatedConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` +func testAccAWSApiGatewayUsagePlanProductCodeConfig(rName, code string) string { + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` resource "aws_api_gateway_usage_plan" "test" { - name = "%s" - description = "This is a new description" + name = %[1]q + product_code = %[2]q } -`, rName) -} - -func testAccAWSApiGatewayUsagePlanProductCodeConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` -resource "aws_api_gateway_usage_plan" "test" { - name = "%s" - product_code = "MYCODE" -} -`, rName) -} - -func testAccAWSApiGatewayUsagePlanProductCodeUpdatedConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` -resource "aws_api_gateway_usage_plan" "test" { - name = "%s" - product_code = "MYCODE2" -} -`, rName) -} - -func testAccAWSApiGatewayUsagePlanBasicUpdatedConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` -resource "aws_api_gateway_usage_plan" "test" { - name = "%s" -} -`, rName) +`, rName, code) } func testAccAWSApiGatewayUsagePlanThrottlingConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` resource "aws_api_gateway_usage_plan" "test" { name = "%s" @@ -651,7 +658,7 @@ resource "aws_api_gateway_usage_plan" "test" { } func testAccAWSApiGatewayUsagePlanThrottlingModifiedConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` resource "aws_api_gateway_usage_plan" "test" { name = "%s" @@ -664,7 +671,7 @@ resource "aws_api_gateway_usage_plan" "test" { } func testAccAWSApiGatewayUsagePlanQuotaConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` resource "aws_api_gateway_usage_plan" "test" { name = "%s" @@ -678,7 +685,7 @@ resource "aws_api_gateway_usage_plan" "test" { } func testAccAWSApiGatewayUsagePlanQuotaModifiedConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` resource "aws_api_gateway_usage_plan" "test" { name = "%s" @@ -692,7 +699,7 @@ resource "aws_api_gateway_usage_plan" "test" { } func testAccAWSApiGatewayUsagePlanApiStagesConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` resource "aws_api_gateway_usage_plan" "test" { name = "%s" @@ -705,7 +712,20 @@ resource "aws_api_gateway_usage_plan" "test" { } func testAccAWSApiGatewayUsagePlanApiStagesModifiedConfig(rName string) string { - return fmt.Sprintf(testAccAWSAPIGatewayUsagePlanConfig(rName)+` + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` +resource "aws_api_gateway_usage_plan" "test" { + name = "%s" + + api_stages { + api_id = "${aws_api_gateway_rest_api.test.id}" + stage = "${aws_api_gateway_deployment.foo.stage_name}" + } +} +`, rName) +} + +func testAccAWSApiGatewayUsagePlanApiStagesMultipleConfig(rName string) string { + return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` resource "aws_api_gateway_usage_plan" "test" { name = "%s" @@ -713,6 +733,11 @@ resource "aws_api_gateway_usage_plan" "test" { api_id = aws_api_gateway_rest_api.test.id stage = aws_api_gateway_deployment.foo.stage_name } + + api_stages { + api_id = "${aws_api_gateway_rest_api.test.id}" + stage = "${aws_api_gateway_deployment.test.stage_name}" + } } `, rName) } diff --git a/aws/structure.go b/aws/structure.go index 585ef1a7b645..35e2001994e5 100644 --- a/aws/structure.go +++ b/aws/structure.go @@ -2275,66 +2275,6 @@ func normalizeJsonOrYamlString(templateString interface{}) (string, error) { return checkYamlString(templateString) } -func flattenApiGatewayUsageApiStages(s []*apigateway.ApiStage) []map[string]interface{} { - stages := make([]map[string]interface{}, 0) - - for _, bd := range s { - if bd.ApiId != nil && bd.Stage != nil { - stage := make(map[string]interface{}) - stage["api_id"] = *bd.ApiId - stage["stage"] = *bd.Stage - - stages = append(stages, stage) - } - } - - if len(stages) > 0 { - return stages - } - - return nil -} - -func flattenApiGatewayUsagePlanThrottling(s *apigateway.ThrottleSettings) []map[string]interface{} { - settings := make(map[string]interface{}) - - if s == nil { - return nil - } - - if s.BurstLimit != nil { - settings["burst_limit"] = *s.BurstLimit - } - - if s.RateLimit != nil { - settings["rate_limit"] = *s.RateLimit - } - - return []map[string]interface{}{settings} -} - -func flattenApiGatewayUsagePlanQuota(s *apigateway.QuotaSettings) []map[string]interface{} { - settings := make(map[string]interface{}) - - if s == nil { - return nil - } - - if s.Limit != nil { - settings["limit"] = *s.Limit - } - - if s.Offset != nil { - settings["offset"] = *s.Offset - } - - if s.Period != nil { - settings["period"] = *s.Period - } - - return []map[string]interface{}{settings} -} - func buildApiGatewayInvokeURL(client *AWSClient, restApiId, stageName string) string { hostname := client.RegionalHostname(fmt.Sprintf("%s.execute-api", restApiId)) return fmt.Sprintf("https://%s/%s", hostname, stageName) From cc6b5e02c87d04e5cd2ce36c5414c4d94ada4063 Mon Sep 17 00:00:00 2001 From: DrFaust92 Date: Sat, 25 Jul 2020 13:35:19 +0300 Subject: [PATCH 2/9] extra checks for basic test --- aws/resource_aws_api_gateway_usage_plan_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/aws/resource_aws_api_gateway_usage_plan_test.go b/aws/resource_aws_api_gateway_usage_plan_test.go index 0f63aae21611..151d53f89697 100644 --- a/aws/resource_aws_api_gateway_usage_plan_test.go +++ b/aws/resource_aws_api_gateway_usage_plan_test.go @@ -2,6 +2,7 @@ package aws import ( "fmt" + "regexp" "testing" "github.com/aws/aws-sdk-go/aws" @@ -28,7 +29,12 @@ func TestAccAWSAPIGatewayUsagePlan_basic(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckAWSAPIGatewayUsagePlanExists(resourceName, &conf), resource.TestCheckResourceAttr(resourceName, "name", rName), + testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/usageplans/+.`)), resource.TestCheckResourceAttr(resourceName, "description", ""), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "api_stages.%", "0"), + resource.TestCheckResourceAttr(resourceName, "quota_settings.%", "0"), + resource.TestCheckResourceAttr(resourceName, "throttle_settings.%", "0"), ), }, { From 42c97cf24d16f778642e3c1d837d14d92114477c Mon Sep 17 00:00:00 2001 From: DrFaust92 Date: Sun, 20 Sep 2020 13:52:39 +0300 Subject: [PATCH 3/9] tf12 syntax --- aws/resource_aws_api_gateway_usage_plan_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_api_gateway_usage_plan_test.go b/aws/resource_aws_api_gateway_usage_plan_test.go index 151d53f89697..1d0a44f39d8c 100644 --- a/aws/resource_aws_api_gateway_usage_plan_test.go +++ b/aws/resource_aws_api_gateway_usage_plan_test.go @@ -723,8 +723,8 @@ resource "aws_api_gateway_usage_plan" "test" { name = "%s" api_stages { - api_id = "${aws_api_gateway_rest_api.test.id}" - stage = "${aws_api_gateway_deployment.foo.stage_name}" + api_id = aws_api_gateway_rest_api.test.id + stage = aws_api_gateway_deployment.foo.stage_name } } `, rName) @@ -741,8 +741,8 @@ resource "aws_api_gateway_usage_plan" "test" { } api_stages { - api_id = "${aws_api_gateway_rest_api.test.id}" - stage = "${aws_api_gateway_deployment.test.stage_name}" + api_id = aws_api_gateway_rest_api.test.id + stage = aws_api_gateway_deployment.test.stage_name } } `, rName) From 2f393cc321ed9ff43634f796401bb780c1d47614 Mon Sep 17 00:00:00 2001 From: DrFaust92 Date: Mon, 21 Sep 2020 11:22:22 +0300 Subject: [PATCH 4/9] update test --- aws/resource_aws_api_gateway_usage_plan_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_api_gateway_usage_plan_test.go b/aws/resource_aws_api_gateway_usage_plan_test.go index 1d0a44f39d8c..d451849c4168 100644 --- a/aws/resource_aws_api_gateway_usage_plan_test.go +++ b/aws/resource_aws_api_gateway_usage_plan_test.go @@ -16,7 +16,7 @@ import ( func TestAccAWSAPIGatewayUsagePlan_basic(t *testing.T) { var conf apigateway.UsagePlan rName := acctest.RandomWithPrefix("tf-acc-test") - updatedName := acctest.RandomWithPrefix("tf-acc-test") + updatedName := acctest.RandomWithPrefix("tf-acc-test-2") resourceName := "aws_api_gateway_usage_plan.test" resource.ParallelTest(t, resource.TestCase{ From 0f016007a066fdb8a3c550b173a066ee3efe4336 Mon Sep 17 00:00:00 2001 From: DrFaust92 Date: Mon, 21 Sep 2020 11:29:59 +0300 Subject: [PATCH 5/9] use enum slice --- aws/resource_aws_api_gateway_usage_plan.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/aws/resource_aws_api_gateway_usage_plan.go b/aws/resource_aws_api_gateway_usage_plan.go index f6627a2ccade..af57f38d429e 100644 --- a/aws/resource_aws_api_gateway_usage_plan.go +++ b/aws/resource_aws_api_gateway_usage_plan.go @@ -71,13 +71,9 @@ func resourceAwsApiGatewayUsagePlan() *schema.Resource { }, "period": { - Type: schema.TypeString, - Required: true, // Required as not removable - ValidateFunc: validation.StringInSlice([]string{ - apigateway.QuotaPeriodTypeDay, - apigateway.QuotaPeriodTypeWeek, - apigateway.QuotaPeriodTypeMonth, - }, false), + Type: schema.TypeString, + Required: true, // Required as not removable + ValidateFunc: validation.StringInSlice(apigateway.QuotaPeriodType_Values(), false), }, }, }, From 21d15450b7b9b1aa465a1aed959b2ae3160ec7f2 Mon Sep 17 00:00:00 2001 From: DrFaust92 Date: Tue, 22 Sep 2020 00:15:58 +0300 Subject: [PATCH 6/9] fix destroy check --- aws/resource_aws_api_gateway_usage_plan_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_api_gateway_usage_plan_test.go b/aws/resource_aws_api_gateway_usage_plan_test.go index d451849c4168..b889b1f66e90 100644 --- a/aws/resource_aws_api_gateway_usage_plan_test.go +++ b/aws/resource_aws_api_gateway_usage_plan_test.go @@ -511,7 +511,7 @@ func testAccCheckAWSAPIGatewayUsagePlanDestroy(s *terraform.State) error { } req := &apigateway.GetUsagePlanInput{ - UsagePlanId: aws.String(s.RootModule().Resources["aws_api_gateway_rest_api.test"].Primary.ID), + UsagePlanId: aws.String(rs.Primary.ID), } describe, err := conn.GetUsagePlan(req) From 825aa83128b46c74bf96f82bd4e55e2bda4515e1 Mon Sep 17 00:00:00 2001 From: DrFaust92 Date: Tue, 22 Sep 2020 11:07:23 +0300 Subject: [PATCH 7/9] fmt --- aws/resource_aws_api_gateway_usage_plan_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_api_gateway_usage_plan_test.go b/aws/resource_aws_api_gateway_usage_plan_test.go index b889b1f66e90..02eef9169647 100644 --- a/aws/resource_aws_api_gateway_usage_plan_test.go +++ b/aws/resource_aws_api_gateway_usage_plan_test.go @@ -720,7 +720,7 @@ resource "aws_api_gateway_usage_plan" "test" { func testAccAWSApiGatewayUsagePlanApiStagesModifiedConfig(rName string) string { return testAccAWSAPIGatewayUsagePlanConfig(rName) + fmt.Sprintf(` resource "aws_api_gateway_usage_plan" "test" { - name = "%s" + name = "%s" api_stages { api_id = aws_api_gateway_rest_api.test.id From e0265ef174fdff4fdbd484828167d18a9e9c9f3d Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 22 Oct 2020 15:12:24 -0700 Subject: [PATCH 8/9] Update resource_aws_api_gateway_usage_plan.go A couple minor cleanups --- aws/resource_aws_api_gateway_usage_plan.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_api_gateway_usage_plan.go b/aws/resource_aws_api_gateway_usage_plan.go index af57f38d429e..8a4386301c8f 100644 --- a/aws/resource_aws_api_gateway_usage_plan.go +++ b/aws/resource_aws_api_gateway_usage_plan.go @@ -136,7 +136,7 @@ func resourceAwsApiGatewayUsagePlanCreate(d *schema.ResourceData, meta interface q, ok := settings[0].(map[string]interface{}) if err := validateApiGatewayUsagePlanQuotaSettings(q); len(err) > 0 { - return fmt.Errorf("error validating the quota settings: %v", err) + return fmt.Errorf("error validating the quota settings: %w", err) } if !ok { @@ -283,7 +283,7 @@ func resourceAwsApiGatewayUsagePlanUpdate(d *schema.ResourceData, meta interface os := o.(*schema.Set).List() ns := n.(*schema.Set).List() - // Remove every stages associated. Simpler to remove and add ns ones, + // Remove every stages associated. Simpler to remove and add new ones, // since there are no replacings. for _, v := range os { m := v.(map[string]interface{}) From 23eaafb1e9c0995609f58b6a536e1b3dcec7869e Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 22 Oct 2020 15:46:46 -0700 Subject: [PATCH 9/9] Update resource_aws_api_gateway_usage_plan.go `validateApiGatewayUsagePlanQuotaSettings()` returns a slice of errors --- aws/resource_aws_api_gateway_usage_plan.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_api_gateway_usage_plan.go b/aws/resource_aws_api_gateway_usage_plan.go index 8a4386301c8f..0de0d6924071 100644 --- a/aws/resource_aws_api_gateway_usage_plan.go +++ b/aws/resource_aws_api_gateway_usage_plan.go @@ -135,8 +135,8 @@ func resourceAwsApiGatewayUsagePlanCreate(d *schema.ResourceData, meta interface settings := v.([]interface{}) q, ok := settings[0].(map[string]interface{}) - if err := validateApiGatewayUsagePlanQuotaSettings(q); len(err) > 0 { - return fmt.Errorf("error validating the quota settings: %w", err) + if errs := validateApiGatewayUsagePlanQuotaSettings(q); len(errs) > 0 { + return fmt.Errorf("error validating the quota settings: %v", errs) } if !ok {