From 969a889f38e94c7ff3d8fb4348d2303b952ee9bd Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Tue, 21 Jul 2020 02:19:26 -0400 Subject: [PATCH 1/2] keep throttling disabled by default in api gateway method settings resource --- ...esource_aws_api_gateway_method_settings.go | 24 ++- ...ce_aws_api_gateway_method_settings_test.go | 161 ++++++++++++++++++ website/docs/guides/version-3-upgrade.html.md | 7 + .../api_gateway_method_settings.html.markdown | 12 +- 4 files changed, 200 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_api_gateway_method_settings.go b/aws/resource_aws_api_gateway_method_settings.go index e7312199215..110291a2e07 100644 --- a/aws/resource_aws_api_gateway_method_settings.go +++ b/aws/resource_aws_api_gateway_method_settings.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" @@ -17,6 +18,10 @@ func resourceAwsApiGatewayMethodSettings() *schema.Resource { Update: resourceAwsApiGatewayMethodSettingsUpdate, Delete: resourceAwsApiGatewayMethodSettingsDelete, + Importer: &schema.ResourceImporter{ + State: resourceAwsApiGatewayMethodSettingsImport, + }, + Schema: map[string]*schema.Schema{ "rest_api_id": { Type: schema.TypeString, @@ -57,12 +62,12 @@ func resourceAwsApiGatewayMethodSettings() *schema.Resource { "throttling_burst_limit": { Type: schema.TypeInt, Optional: true, - Computed: true, + Default: -1, }, "throttling_rate_limit": { Type: schema.TypeFloat, Optional: true, - Computed: true, + Default: -1, }, "caching_enabled": { Type: schema.TypeBool, @@ -266,3 +271,18 @@ func resourceAwsApiGatewayMethodSettingsDelete(d *schema.ResourceData, meta inte return nil } + +func resourceAwsApiGatewayMethodSettingsImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { + idParts := strings.Split(d.Id(), ":") + if len(idParts) != 3 || idParts[0] == "" || idParts[1] == "" || idParts[2] == "" { + return nil, fmt.Errorf("Unexpected format of ID (%q), expected REST-API-ID:STAGE-NAME:METHOD-PATH", d.Id()) + } + restApiID := idParts[0] + stageName := idParts[1] + methodPath := idParts[2] + d.Set("rest_api_id", restApiID) + d.Set("stage_name", stageName) + d.Set("method_path", methodPath) + d.SetId(fmt.Sprintf("%s-%s-%s", restApiID, stageName, methodPath)) + return []*schema.ResourceData{d}, nil +} diff --git a/aws/resource_aws_api_gateway_method_settings_test.go b/aws/resource_aws_api_gateway_method_settings_test.go index a0040e0acd5..a59abdc1367 100644 --- a/aws/resource_aws_api_gateway_method_settings_test.go +++ b/aws/resource_aws_api_gateway_method_settings_test.go @@ -30,6 +30,12 @@ func TestAccAWSAPIGatewayMethodSettings_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "settings.0.logging_level", "INFO"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, }, }) } @@ -60,6 +66,12 @@ func TestAccAWSAPIGatewayMethodSettings_Settings_CacheDataEncrypted(t *testing.T resource.TestCheckResourceAttr(resourceName, "settings.0.cache_data_encrypted", "false"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, }, }) } @@ -90,6 +102,12 @@ func TestAccAWSAPIGatewayMethodSettings_Settings_CacheTtlInSeconds(t *testing.T) resource.TestCheckResourceAttr(resourceName, "settings.0.cache_ttl_in_seconds", "2"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, }, }) } @@ -120,6 +138,12 @@ func TestAccAWSAPIGatewayMethodSettings_Settings_CachingEnabled(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "settings.0.caching_enabled", "false"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, }, }) } @@ -150,6 +174,12 @@ func TestAccAWSAPIGatewayMethodSettings_Settings_DataTraceEnabled(t *testing.T) resource.TestCheckResourceAttr(resourceName, "settings.0.data_trace_enabled", "false"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, }, }) } @@ -182,6 +212,12 @@ func TestAccAWSAPIGatewayMethodSettings_Settings_LoggingLevel(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "settings.0.logging_level", "OFF"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, }, }) } @@ -214,6 +250,12 @@ func TestAccAWSAPIGatewayMethodSettings_Settings_MetricsEnabled(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "settings.0.metrics_enabled", "false"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, }, }) } @@ -250,6 +292,12 @@ func TestAccAWSAPIGatewayMethodSettings_Settings_Multiple(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "settings.0.logging_level", "OFF"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, }, }) } @@ -280,6 +328,12 @@ func TestAccAWSAPIGatewayMethodSettings_Settings_RequireAuthorizationForCacheCon resource.TestCheckResourceAttr(resourceName, "settings.0.require_authorization_for_cache_control", "false"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, }, }) } @@ -310,6 +364,49 @@ func TestAccAWSAPIGatewayMethodSettings_Settings_ThrottlingBurstLimit(t *testing resource.TestCheckResourceAttr(resourceName, "settings.0.throttling_burst_limit", "2"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, + }, + }) +} + +// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/5690 +func TestAccAWSAPIGatewayMethodSettings_Settings_ThrottlingBurstLimitDisabledByDefault(t *testing.T) { + var stage1, stage2 apigateway.Stage + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_api_gateway_method_settings.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAPIGatewayMethodSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSAPIGatewayMethodSettingsConfigSettingsLoggingLevel(rName, "INFO"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAPIGatewayMethodSettingsExists(resourceName, &stage2), + resource.TestCheckResourceAttr(resourceName, "settings.#", "1"), + resource.TestCheckResourceAttr(resourceName, "settings.0.throttling_burst_limit", "-1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, + { + Config: testAccAWSAPIGatewayMethodSettingsConfigSettingsThrottlingBurstLimit(rName, 1), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAPIGatewayMethodSettingsExists(resourceName, &stage1), + resource.TestCheckResourceAttr(resourceName, "settings.#", "1"), + resource.TestCheckResourceAttr(resourceName, "settings.0.throttling_burst_limit", "1"), + ), + }, }, }) } @@ -340,6 +437,49 @@ func TestAccAWSAPIGatewayMethodSettings_Settings_ThrottlingRateLimit(t *testing. resource.TestCheckResourceAttr(resourceName, "settings.0.throttling_rate_limit", "2.2"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, + }, + }) +} + +// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/5690 +func TestAccAWSAPIGatewayMethodSettings_Settings_ThrottlingRateLimitDisabledByDefault(t *testing.T) { + var stage1, stage2 apigateway.Stage + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_api_gateway_method_settings.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAPIGatewayMethodSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSAPIGatewayMethodSettingsConfigSettingsLoggingLevel(rName, "INFO"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAPIGatewayMethodSettingsExists(resourceName, &stage1), + resource.TestCheckResourceAttr(resourceName, "settings.#", "1"), + resource.TestCheckResourceAttr(resourceName, "settings.0.throttling_rate_limit", "-1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, + { + Config: testAccAWSAPIGatewayMethodSettingsConfigSettingsThrottlingRateLimit(rName, 1.1), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAPIGatewayMethodSettingsExists(resourceName, &stage2), + resource.TestCheckResourceAttr(resourceName, "settings.#", "1"), + resource.TestCheckResourceAttr(resourceName, "settings.0.throttling_rate_limit", "1.1"), + ), + }, }, }) } @@ -370,6 +510,12 @@ func TestAccAWSAPIGatewayMethodSettings_Settings_UnauthorizedCacheControlHeaderS resource.TestCheckResourceAttr(resourceName, "settings.0.unauthorized_cache_control_header_strategy", "SUCCEED_WITHOUT_RESPONSE_HEADER"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, }, }) } @@ -464,6 +610,21 @@ func testAccCheckAWSAPIGatewayMethodSettingsDestroy(s *terraform.State) error { return nil } +func testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName string) resource.ImportStateIdFunc { + return func(s *terraform.State) (string, error) { + rs, ok := s.RootModule().Resources[resourceName] + if !ok { + return "", fmt.Errorf("not found: %s", resourceName) + } + + restApiID := rs.Primary.Attributes["rest_api_id"] + stageName := rs.Primary.Attributes["stage_name"] + methodPath := rs.Primary.Attributes["method_path"] + + return fmt.Sprintf("%s:%s:%s", restApiID, stageName, methodPath), nil + } +} + func testAccAWSAPIGatewayMethodSettingsConfigBase(rName string) string { return fmt.Sprintf(` resource "aws_api_gateway_rest_api" "test" { diff --git a/website/docs/guides/version-3-upgrade.html.md b/website/docs/guides/version-3-upgrade.html.md index 9e75070736f..b3320bb077d 100644 --- a/website/docs/guides/version-3-upgrade.html.md +++ b/website/docs/guides/version-3-upgrade.html.md @@ -23,6 +23,7 @@ Upgrade topics: - [Data Source: aws_availability_zones](#data-source-aws_availability_zones) - [Data Source: aws_lambda_invocation](#data-source-aws_lambda_invocation) - [Resource: aws_acm_certificate](#resource-aws_acm_certificate) +- [Resource: aws_api_gateway_method_settings](#resource-aws_api_gateway_method_settings) - [Resource: aws_autoscaling_group](#resource-aws_autoscaling_group) - [Resource: aws_dx_gateway](#resource-aws_dx_gateway) - [Resource: aws_elastic_transcoder_preset](#resource-aws_elastic_transcoder_preset) @@ -171,6 +172,12 @@ Previously the `subject_alternative_names` argument was stored in the Terraform Previously when the `certificate_body`, `certificate_chain`, and `private_key` arguments were stored in state, they were stored as a hash of the actual value. This prevented Terraform from properly updating the resource when necessary and the hashing has been removed. The Terraform AWS Provider will show an update to these arguments on the first apply after upgrading to version 3.0.0, which is fixing the Terraform state to remove the hash. Since the `private_key` attribute is marked as sensitive, the values in the update will not be visible in the Terraform output. If the non-hashed values have not changed, then no update is occurring other than the Terraform state update. If these arguments are the only updates and they all match the hash removal, the apply will occur without submitting API calls. +## Resource: aws_api_gateway_method_settings + +### throttling_burst_limit and throttling_rate_limit Arguments Now Default to -1 + +Previously when the `throttling_burst_limit` or `throttling_rate_limit` argument was not configured, the resource would enable throttling and set the limit value to the AWS API Gateway default. In addition, as these arguments were marked as `Computed`, Terraform ignored any subsequent changes made to these arguments in the resource. These behaviors have been removed and, by default, the `throttling_burst_limit` and `throttling_rate_limit` arguments will be disabled in the resource with a value of `-1`. + ## Resource: aws_autoscaling_group ### availability_zones and vpc_zone_identifier Arguments Now Report Plan-Time Conflict diff --git a/website/docs/r/api_gateway_method_settings.html.markdown b/website/docs/r/api_gateway_method_settings.html.markdown index 7241d56a45d..e4ed0cfdf51 100644 --- a/website/docs/r/api_gateway_method_settings.html.markdown +++ b/website/docs/r/api_gateway_method_settings.html.markdown @@ -84,10 +84,18 @@ The following arguments are supported: * `metrics_enabled` - (Optional) Specifies whether Amazon CloudWatch metrics are enabled for this method. * `logging_level` - (Optional) Specifies the logging level for this method, which effects the log entries pushed to Amazon CloudWatch Logs. The available levels are `OFF`, `ERROR`, and `INFO`. * `data_trace_enabled` - (Optional) Specifies whether data trace logging is enabled for this method, which effects the log entries pushed to Amazon CloudWatch Logs. -* `throttling_burst_limit` - (Optional) Specifies the throttling burst limit. -* `throttling_rate_limit` - (Optional) Specifies the throttling rate limit. +* `throttling_burst_limit` - (Optional) Specifies the throttling burst limit. Default: `-1` (throttling disabled). +* `throttling_rate_limit` - (Optional) Specifies the throttling rate limit. Default: `-1` (throttling disabled). * `caching_enabled` - (Optional) Specifies whether responses should be cached and returned for requests. A cache cluster must be enabled on the stage for responses to be cached. * `cache_ttl_in_seconds` - (Optional) Specifies the time to live (TTL), in seconds, for cached responses. The higher the TTL, the longer the response will be cached. * `cache_data_encrypted` - (Optional) Specifies whether the cached responses are encrypted. * `require_authorization_for_cache_control` - (Optional) Specifies whether authorization is required for a cache invalidation request. * `unauthorized_cache_control_header_strategy` - (Optional) Specifies how to handle unauthorized requests for cache invalidation. The available values are `FAIL_WITH_403`, `SUCCEED_WITH_RESPONSE_HEADER`, `SUCCEED_WITHOUT_RESPONSE_HEADER`. + +## Import + +`aws_api_gateway_method_settings` can be imported using `REST-API-ID:STAGE-NAME:METHOD-PATH`, e.g. + +``` +$ terraform import aws_api_gateway_method_settings.example 12345abcde:example:test/GET +``` From 9cbd77b814460d28b84745bdec80413a11fc4103 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Tue, 21 Jul 2020 15:27:37 -0400 Subject: [PATCH 2/2] update import ID pattern --- aws/resource_aws_api_gateway_method_settings.go | 4 ++-- aws/resource_aws_api_gateway_method_settings_test.go | 2 +- website/docs/r/api_gateway_method_settings.html.markdown | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/aws/resource_aws_api_gateway_method_settings.go b/aws/resource_aws_api_gateway_method_settings.go index 110291a2e07..17d541c33ed 100644 --- a/aws/resource_aws_api_gateway_method_settings.go +++ b/aws/resource_aws_api_gateway_method_settings.go @@ -273,9 +273,9 @@ func resourceAwsApiGatewayMethodSettingsDelete(d *schema.ResourceData, meta inte } func resourceAwsApiGatewayMethodSettingsImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - idParts := strings.Split(d.Id(), ":") + idParts := strings.SplitN(d.Id(), "/", 3) if len(idParts) != 3 || idParts[0] == "" || idParts[1] == "" || idParts[2] == "" { - return nil, fmt.Errorf("Unexpected format of ID (%q), expected REST-API-ID:STAGE-NAME:METHOD-PATH", d.Id()) + return nil, fmt.Errorf("Unexpected format of ID (%q), expected REST-API-ID/STAGE-NAME/METHOD-PATH", d.Id()) } restApiID := idParts[0] stageName := idParts[1] diff --git a/aws/resource_aws_api_gateway_method_settings_test.go b/aws/resource_aws_api_gateway_method_settings_test.go index a59abdc1367..0c51f52b1b8 100644 --- a/aws/resource_aws_api_gateway_method_settings_test.go +++ b/aws/resource_aws_api_gateway_method_settings_test.go @@ -621,7 +621,7 @@ func testAccAWSAPIGatewayMethodSettingsImportStateIdFunc(resourceName string) re stageName := rs.Primary.Attributes["stage_name"] methodPath := rs.Primary.Attributes["method_path"] - return fmt.Sprintf("%s:%s:%s", restApiID, stageName, methodPath), nil + return fmt.Sprintf("%s/%s/%s", restApiID, stageName, methodPath), nil } } diff --git a/website/docs/r/api_gateway_method_settings.html.markdown b/website/docs/r/api_gateway_method_settings.html.markdown index e4ed0cfdf51..5ee6c77e12d 100644 --- a/website/docs/r/api_gateway_method_settings.html.markdown +++ b/website/docs/r/api_gateway_method_settings.html.markdown @@ -94,8 +94,8 @@ The following arguments are supported: ## Import -`aws_api_gateway_method_settings` can be imported using `REST-API-ID:STAGE-NAME:METHOD-PATH`, e.g. +`aws_api_gateway_method_settings` can be imported using `REST-API-ID/STAGE-NAME/METHOD-PATH`, e.g. ``` -$ terraform import aws_api_gateway_method_settings.example 12345abcde:example:test/GET +$ terraform import aws_api_gateway_method_settings.example 12345abcde/example/test/GET ```