Skip to content

Commit

Permalink
resource/aws_api_gateway_rest_api: Additional OpenAPI specification a…
Browse files Browse the repository at this point in the history 
…cceptance testing and fix various attributes after import (#17099)

* fix bug with rest api openapi removing policy

* resource/aws_api_gateway_rest_api: Additional OpenAPI specification acceptance testing and fix various attributes after import

Reference: #5364
Reference: #7161
Reference: #9722
Reference: #10766
Reference: #12432
Reference: #13841
Reference: #14290
Reference: #14660

Changes:

```
* resource/aws_api_gateway_rest_api: Ensure `api_key_source`, `binary_media_types`, `description`, `minimum_compression_size`, `name`, and `policy` configuration values are correctly applied as an override after OpenAPI specification import (`body` argument)
* resource/aws_api_gateway_rest_api: Allow `api_key_source`, `binary_media_types`, and `description` arguments to be omitted from configuration with OpenAPI specification import (`body` argument)
```

The overall testing changes are:

* Ensuring the basic test covers all attributes
* Refactoring the basic test into per-attribute testing
* Adding per-attribute tests to cover OpenAPI specificiations (`body` argument) being set without Terraform configurations -- these should be allowed with Terraform showing a planned difference
* Adding per-attribute tests to cover OpenAPI specificiations (`body` argument) being set with Terraform configurations -- these should be allowed with the Terraform configuration value overriding the OpenAPI specification
* Removing extraneous API object `TestCheckFunc` (covered by `ImportStateVerify` testing)

It is worth mentioning that this does not cover the `disable_execute_api_endpoint` or `tags` attributes that can also be specified by OpenAPI since this change set is already very large. The `minimum_compression_size` attribute also needs an additional update to support OpenAPI-only configuration. Further updates can improve on this effort.

Before code updates, these new acceptance tests show how the Terraform configuration value would not be applied if an OpenAPI specification was imported:

```
=== CONT  TestAccAWSAPIGatewayRestApi_ApiKeySource_OverrideBody
    resource_aws_api_gateway_rest_api_test.go:428: Step 1/4 error: Check failed: 1 error occurred:
        	* Check 2/2 error: aws_api_gateway_rest_api.test: Attribute 'api_key_source' expected "AUTHORIZER", got "HEADER"

--- FAIL: TestAccAWSAPIGatewayRestApi_ApiKeySource_OverrideBody (8.82s)

=== CONT  TestAccAWSAPIGatewayRestApi_BinaryMediaTypes_OverrideBody
    resource_aws_api_gateway_rest_api_test.go:464: Step 1/4 error: Check failed: 1 error occurred:
        	* Check 3/3 error: aws_api_gateway_rest_api.test: Attribute 'binary_media_types.0' expected "application/octet-stream", got "image/jpeg"

=== CONT  TestAccAWSAPIGatewayRestApi_Description_OverrideBody
    resource_aws_api_gateway_rest_api_test.go:527: Step 1/4 error: Check failed: 1 error occurred:
        	* Check 2/2 error: aws_api_gateway_rest_api.test: Attribute 'description' expected "tfdescription1", got "oasdescription1"

--- FAIL: TestAccAWSAPIGatewayRestApi_Description_OverrideBody (9.60s)

=== CONT  TestAccAWSAPIGatewayRestApi_MinimumCompressionSize_OverrideBody
    resource_aws_api_gateway_rest_api_test.go:688: Step 1/4 error: Check failed: 1 error occurred:
        	* Check 2/2 error: aws_api_gateway_rest_api.test: Attribute 'minimum_compression_size' expected "1", got "5242880"

--- FAIL: TestAccAWSAPIGatewayRestApi_MinimumCompressionSize_OverrideBody (8.41s)

=== CONT  TestAccAWSAPIGatewayRestApi_Name_OverrideBody
    resource_aws_api_gateway_rest_api_test.go:528: Step 1/4 error: Check failed: 1 error occurred:
        	* Check 2/2 error: aws_api_gateway_rest_api.test: Attribute 'name' expected "tf-acc-test-4252368909257291928", got "title1"

--- FAIL: TestAccAWSAPIGatewayRestApi_Name_OverrideBody (8.57s)

=== CONT  TestAccAWSAPIGatewayRestApi_Policy_OverrideBody
    resource_aws_api_gateway_rest_api_test.go:593: Step 1/4 error: Check failed: 1 error occurred:
        	* Check 4/4 error: aws_api_gateway_rest_api.test: Attribute 'policy' didn't match "\"Allow\"", got ""

--- FAIL: TestAccAWSAPIGatewayRestApi_Policy_OverrideBody (9.37s)
```

Before code updates, these acceptance tests show how the Terraform resource would report an unexpected difference for missing configurations that were imported by the OpenAPI specification:

```
=== CONT  TestAccAWSAPIGatewayRestApi_ApiKeySource_SetByBody
    resource_aws_api_gateway_rest_api_test.go:471: Step 1/2 error: After applying this test step, the plan was not empty.
        stdout:

        An execution plan has been generated and is shown below.
        Resource actions are indicated with the following symbols:
          ~ update in-place

        Terraform will perform the following actions:

          # aws_api_gateway_rest_api.test will be updated in-place
          ~ resource "aws_api_gateway_rest_api" "test" {
              ~ api_key_source               = "AUTHORIZER" -> "HEADER"
                id                           = "5ja4mnzxta"
                name                         = "tf-acc-test-4415455482847955650"
                # (8 unchanged attributes hidden)

                # (1 unchanged block hidden)
            }

        Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccAWSAPIGatewayRestApi_ApiKeySource_SetByBody (20.65s)

=== CONT  TestAccAWSAPIGatewayRestApi_BinaryMediaTypes_SetByBody
    resource_aws_api_gateway_rest_api_test.go:510: Step 1/2 error: After applying this test step, the plan was not empty.
        stdout:

        An execution plan has been generated and is shown below.
        Resource actions are indicated with the following symbols:
          ~ update in-place

        Terraform will perform the following actions:

          # aws_api_gateway_rest_api.test will be updated in-place
          ~ resource "aws_api_gateway_rest_api" "test" {
              ~ binary_media_types           = [
                  - "application/octet-stream",
                ]
                id                           = "7we4bv4s8b"
                name                         = "tf-acc-test-2053199682951305540"
                # (8 unchanged attributes hidden)

                # (1 unchanged block hidden)
            }

        Plan: 0 to add, 1 to change, 0 to destroy.

=== CONT  TestAccAWSAPIGatewayRestApi_Description_SetByBody
    resource_aws_api_gateway_rest_api_test.go:570: Step 1/2 error: After applying this test step, the plan was not empty.
        stdout:

        An execution plan has been generated and is shown below.
        Resource actions are indicated with the following symbols:
          ~ update in-place

        Terraform will perform the following actions:

          # aws_api_gateway_rest_api.test will be updated in-place
          ~ resource "aws_api_gateway_rest_api" "test" {
              - description                  = "oasdescription1" -> null
                id                           = "3k0fykhp76"
                name                         = "tf-acc-test-2107985362088533117"
                # (8 unchanged attributes hidden)

                # (1 unchanged block hidden)
            }

        Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccAWSAPIGatewayRestApi_Description_SetByBody (10.02s)

=== CONT  TestAccAWSAPIGatewayRestApi_MinimumCompressionSize_SetByBody
    resource_aws_api_gateway_rest_api_test.go:731: Step 1/2 error: After applying this test step, the plan was not empty.
        stdout:

        An execution plan has been generated and is shown below.
        Resource actions are indicated with the following symbols:
          ~ update in-place

        Terraform will perform the following actions:

          # aws_api_gateway_rest_api.test will be updated in-place
          ~ resource "aws_api_gateway_rest_api" "test" {
                id                           = "bcmvzz0jfi"
              ~ minimum_compression_size     = 1048576 -> -1
                name                         = "tf-acc-test-2006611344091675720"
                # (7 unchanged attributes hidden)

                # (1 unchanged block hidden)
            }

        Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccAWSAPIGatewayRestApi_MinimumCompressionSize_SetByBody (10.99s)
```

Additionally these new acceptance tests show how the Terraform resource already respected missing configurations that were imported by the OpenAPI specification:

```
--- PASS: TestAccAWSAPIGatewayRestApi_Policy_SetByBody (15.03s)
```

Output from acceptance testing:

```
--- PASS: TestAccAWSAPIGatewayRestApi_ApiKeySource (28.57s)
--- PASS: TestAccAWSAPIGatewayRestApi_ApiKeySource_OverrideBody (52.53s)
--- PASS: TestAccAWSAPIGatewayRestApi_ApiKeySource_SetByBody (25.48s)
--- PASS: TestAccAWSAPIGatewayRestApi_basic (23.16s)
--- PASS: TestAccAWSAPIGatewayRestApi_BinaryMediaTypes (80.33s)
--- PASS: TestAccAWSAPIGatewayRestApi_BinaryMediaTypes_OverrideBody (34.45s)
--- PASS: TestAccAWSAPIGatewayRestApi_BinaryMediaTypes_SetByBody (24.16s)
--- PASS: TestAccAWSAPIGatewayRestApi_Body (26.69s)
--- PASS: TestAccAWSAPIGatewayRestApi_Description (765.29s)
--- PASS: TestAccAWSAPIGatewayRestApi_Description_OverrideBody (32.87s)
--- PASS: TestAccAWSAPIGatewayRestApi_Description_SetByBody (51.31s)
--- PASS: TestAccAWSAPIGatewayRestApi_DisableExecuteApiEndpoint (30.21s)
--- PASS: TestAccAWSAPIGatewayRestApi_disappears (38.64s)
--- PASS: TestAccAWSAPIGatewayRestApi_EndpointConfiguration (58.23s)
--- PASS: TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private (15.02s)
--- PASS: TestAccAWSAPIGatewayRestApi_EndpointConfiguration_VPCEndpoint (305.78s)
--- PASS: TestAccAWSAPIGatewayRestApi_MinimumCompressionSize (42.89s)
--- PASS: TestAccAWSAPIGatewayRestApi_MinimumCompressionSize_OverrideBody (35.97s)
--- PASS: TestAccAWSAPIGatewayRestApi_MinimumCompressionSize_SetByBody (106.39s)
--- PASS: TestAccAWSAPIGatewayRestApi_Name_OverrideBody (86.16s)
--- PASS: TestAccAWSAPIGatewayRestApi_Parameters (39.90s)
--- PASS: TestAccAWSAPIGatewayRestApi_Policy (683.47s)
--- PASS: TestAccAWSAPIGatewayRestApi_Policy_OverrideBody (905.68s)
--- PASS: TestAccAWSAPIGatewayRestApi_Policy_SetByBody (28.12s)
--- PASS: TestAccAWSAPIGatewayRestApi_tags (32.94s)
```

* tests/resource/aws_api_gateway_rest_api: terrafmt fixes

* tests/resource/aws_api_gateway_rest_api: Remove extraneous minimum_compression_size testing from basic test

* docs/resource/aws_api_gateway_rest_api: Fix misspell

* Apply suggestions from code review

Co-authored-by: Dirk Avery <31492422+YakDriver@users.noreply.github.com>

Co-authored-by: james.warren <james.warren@digital.justice.gov.uk>
Co-authored-by: Dirk Avery <31492422+YakDriver@users.noreply.github.com>
  • Loading branch information
@bflad @jamesrwarren @YakDriver
3 people authored Jan 20, 2021
1 parent c2b2fda commit 61dfaa9
Show file tree
Hide file tree
Showing 3 changed files with 1,290 additions and 288 deletions.
168 changes: 157 additions & 11 deletions aws/resource_aws_api_gateway_rest_api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,14 @@ func resourceAwsApiGatewayRestApi() *schema.Resource {
"description": {
Type: schema.TypeString,
Optional: true,
Computed: true,
},

"api_key_source": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringInSlice([]string{
apigateway.ApiKeySourceTypeAuthorizer,
apigateway.ApiKeySourceTypeHeader,
}, true),
Default: apigateway.ApiKeySourceTypeHeader,
Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validation.StringInSlice(apigateway.ApiKeySourceType_Values(), false),
},

"policy": {
Expand All @@ -57,6 +55,7 @@ func resourceAwsApiGatewayRestApi() *schema.Resource {
"binary_media_types": {
Type: schema.TypeList,
Optional: true,
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
},

Expand Down Expand Up @@ -204,11 +203,84 @@ func resourceAwsApiGatewayRestApiCreate(d *schema.ResourceData, meta interface{}
input.Parameters = stringMapToPointers(v.(map[string]interface{}))
}

_, err := conn.PutRestApi(input)
output, err := conn.PutRestApi(input)

if err != nil {
return fmt.Errorf("error creating API Gateway specification: %s", err)
}

// Using PutRestApi with mode overwrite will remove any configuration
// that was done with CreateRestApi. Reconcile these changes by having
// any Terraform configured values overwrite imported configuration.

updateInput := &apigateway.UpdateRestApiInput{
RestApiId: aws.String(d.Id()),
PatchOperations: []*apigateway.PatchOperation{},
}

if v, ok := d.GetOk("api_key_source"); ok && v.(string) != aws.StringValue(output.ApiKeySource) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/apiKeySource"),
Value: aws.String(v.(string)),
})
}

if v, ok := d.GetOk("binary_media_types"); ok && len(v.([]interface{})) > 0 {
for _, elem := range aws.StringValueSlice(output.BinaryMediaTypes) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpRemove),
Path: aws.String("/binaryMediaTypes/" + escapeJsonPointer(elem)),
})
}

for _, elem := range v.([]interface{}) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpAdd),
Path: aws.String("/binaryMediaTypes/" + escapeJsonPointer(elem.(string))),
})
}
}

if v, ok := d.GetOk("description"); ok && v.(string) != aws.StringValue(output.Description) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/description"),
Value: aws.String(v.(string)),
})
}

if v := d.Get("minimum_compression_size").(int); v > -1 && int64(v) != aws.Int64Value(output.MinimumCompressionSize) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/minimumCompressionSize"),
Value: aws.String(strconv.Itoa(v)),
})
}

if v, ok := d.GetOk("name"); ok && v.(string) != aws.StringValue(output.Name) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/name"),
Value: aws.String(v.(string)),
})
}

if v, ok := d.GetOk("policy"); ok && v.(string) != aws.StringValue(output.Policy) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/policy"),
Value: aws.String(v.(string)),
})
}

if len(updateInput.PatchOperations) > 0 {
_, err := conn.UpdateRestApi(updateInput)

if err != nil {
return fmt.Errorf("error updating REST API (%s) after OpenAPI import: %w", d.Id(), err)
}
}
}

return resourceAwsApiGatewayRestApiRead(d, meta)
Expand Down Expand Up @@ -458,11 +530,86 @@ func resourceAwsApiGatewayRestApiUpdate(d *schema.ResourceData, meta interface{}
input.Parameters = stringMapToPointers(v.(map[string]interface{}))
}

_, err := conn.PutRestApi(input)
output, err := conn.PutRestApi(input)

if err != nil {
return fmt.Errorf("error updating API Gateway specification: %s", err)
}

// Using PutRestApi with mode overwrite will remove any configuration
// that was done previously. Reconcile these changes by having
// any Terraform configured values overwrite imported configuration.

updateInput := &apigateway.UpdateRestApiInput{
RestApiId: aws.String(d.Id()),
PatchOperations: []*apigateway.PatchOperation{},
}

if v, ok := d.GetOk("api_key_source"); ok && v.(string) != aws.StringValue(output.ApiKeySource) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/apiKeySource"),
Value: aws.String(v.(string)),
})
}

if v, ok := d.GetOk("binary_media_types"); ok && len(v.([]interface{})) > 0 {
for _, elem := range aws.StringValueSlice(output.BinaryMediaTypes) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpRemove),
Path: aws.String("/binaryMediaTypes/" + escapeJsonPointer(elem)),
})
}

for _, elem := range v.([]interface{}) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpAdd),
Path: aws.String("/binaryMediaTypes/" + escapeJsonPointer(elem.(string))),
})
}
}

if v, ok := d.GetOk("description"); ok && v.(string) != aws.StringValue(output.Description) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/description"),
Value: aws.String(v.(string)),
})
}

if v := d.Get("minimum_compression_size").(int); v > -1 && int64(v) != aws.Int64Value(output.MinimumCompressionSize) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/minimumCompressionSize"),
Value: aws.String(strconv.Itoa(v)),
})
}

if v, ok := d.GetOk("name"); ok && v.(string) != aws.StringValue(output.Name) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/name"),
Value: aws.String(v.(string)),
})
}

if v, ok := d.GetOk("policy"); ok && v.(string) != aws.StringValue(output.Policy) {
updateInput.PatchOperations = append(updateInput.PatchOperations, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/policy"),
Value: aws.String(v.(string)),
})
}

if len(updateInput.PatchOperations) > 0 {
_, err := conn.UpdateRestApi(updateInput)

if err != nil {
return fmt.Errorf("error updating REST API (%s) after OpenAPI import: %w", d.Id(), err)
}
}

return resourceAwsApiGatewayRestApiRead(d, meta)
}
}

Expand All @@ -472,9 +619,8 @@ func resourceAwsApiGatewayRestApiUpdate(d *schema.ResourceData, meta interface{}
})

if err != nil {
return err
return fmt.Errorf("error updating REST API (%s): %w", d.Id(), err)
}
log.Printf("[DEBUG] Updated API Gateway %s", d.Id())

return resourceAwsApiGatewayRestApiRead(d, meta)
}
Expand Down
Loading

0 comments on commit 61dfaa9

Please sign in to comment.