r/aws_api_gateway_stage: Support canary release deployment #2793

erran · 2017-12-28T18:52:20Z

Description

Fix #2727 by adding the new canary_settings argument to api_gateway_stage. (Initially opened as #2792 after a bad merge/rebase)

Motivation

With the release of canary deployments in API gateway I'd like to be able to:

Create a canary deployment through terraform.
Disable/delete a canary deployment through terraform.
Update canary deployments through terraform.
~~Promote a canary deployment through terraform~~.

I've implemented Create/Update/Delete in this pull request but did not attempt to implement promote, which I think can be accomplished today by promoting through AWS console/CLI then updating config and importing the promoted stage into terraform management.

Documentation

Updated docs/r/aws_api_gateway_stage to include the new option/nested options.

Acceptance test run

Create canary settings
Update canary settings
Disable/delete canary settings
- The overall delete successfully removes these settings but I did not test deleting settings seperately.

make testacc TESTARGS="-run 'TestAccAWSAPIGatewayStage_*'"
==> Checking that code complies with gofmt requirements...              
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run 'TestAccAWSAPIGatewayStage_*' -timeout 120m
?       github.com/terraform-providers/terraform-provider-aws   [no test files]
                                                                        
=== RUN   TestAccAWSAPIGatewayStage_basic                               
--- PASS: TestAccAWSAPIGatewayStage_basic (622.91s)                    
PASS                                                                    
ok      github.com/terraform-providers/terraform-provider-aws/aws       631.351s

Fix hashicorp#2727 by adding the new `canary_settings` argument to `api_gateway_stage`.

…ary settings This field can be used to attach canary settings to a specific deployment. It defaults to the same as the stage but can change.

bflad

Hi @erran 👋 Thanks for this contribution and sorry for the delayed review. Can you please see the items below and rebase? Let us know if you have questions or if you do not have time to implement the feedback.

bflad · 2018-04-26T16:32:37Z

aws/resource_aws_api_gateway_stage.go

+			"canary_settings": {
+				Type:     schema.TypeList,
+				Optional: true,
+				MinItems: 0,


Nitpick: MinItems: 0 is extraneous here 😄

bflad · 2018-04-26T16:40:18Z

aws/resource_aws_api_gateway_stage.go

+		canarySettings := v.([]interface{})
+		canarySetting, ok := canarySettings[0].(map[string]interface{})
+		if !ok {
+			return fmt.Errorf("At least one field is expected inside canary_settings")


We do not need to error check the schema here and the other ones below. It can be confusing to see this code and we can rely on a panic during acceptance testing if a future schema change breaks a reference.

d.GetOk() will ensure there's an element in canarySettings

stage_variable_overrides will always default to an empty map given the existing schema

percent_traffic will always default to 0.0 given the existing schema

use_stage_cache will always default to false given the existing schema

bflad · 2018-04-26T16:44:03Z

aws/resource_aws_api_gateway_stage_test.go

@@ -26,6 +26,9 @@ func TestAccAWSAPIGatewayStage_basic(t *testing.T) {
 					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", "canary_settings.0.percent_traffic", "33.33"),


Can you please move the canary_settings testing to its own acceptance test(s)/configuration(s) and leave just a simple check in the basic test?

resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "canary_settings.#", "0"),

bflad · 2018-04-26T16:46:11Z

aws/structure.go

+		return nil
+	}
+
+	for k, v := range canarySettings.StageVariableOverrides {


The AWS SDK provides a convenience method which can replace this logic: aws.StringValueMap()

bflad · 2018-04-26T16:47:07Z

website/docs/r/api_gateway_stage.html.markdown

+
+* `percent_traffic` - (Optional) The percent `0.0` - `100.0` of traffic to divert to the canary deployment.
+* `stage_variable_overrides` - (Optional) A map of overridden stage `variables` (including new variables) for the canary deployment.
+* `use_stage_cache` - (Optional) Whether the canary deployment uses the stage cache.


We should include Defaults to false here

erran · 2018-05-08T15:50:07Z

@bflad I'll take a stab at finishing up the review feedback this week. I recently moved teams and haven't looked at my open source queue in a few weeks. 😅 I'll respond back here but your comments seem simple enough to address, thank you. 😃

bflad · 2018-10-09T03:51:42Z

Closing this PR due to lack of activity and its non-trivial merge conflicts at this point. We can absolutely revisit this if someone has the desire. 😄

ghost · 2020-04-03T17:14:09Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

erran added 2 commits December 28, 2017 18:50

r/aws_api_gateway_stage: Support canary release deployment

4a09f01

Fix hashicorp#2727 by adding the new `canary_settings` argument to `api_gateway_stage`.

r/aws_api_gateway_stage: Remove optional deployment id field from can…

d38a43b

…ary settings This field can be used to attach canary settings to a specific deployment. It defaults to the same as the stage but can change.

erran mentioned this pull request Dec 28, 2017

r/aws_api_gateway_stage: Support canary release deployment #2792

Closed

jen20 added the enhancement Requests to existing resources that expand the functionality or scope. label Dec 29, 2017

bflad added the service/apigateway Issues and PRs that pertain to the apigateway service. label Jan 28, 2018

bflad reviewed Apr 26, 2018

View reviewed changes

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Apr 26, 2018

bflad closed this Oct 9, 2018

GiPPe mentioned this pull request Oct 11, 2019

r/aws_api_gateway_stage: Support canary release deployment #10474

Closed

ghost locked and limited conversation to collaborators Apr 3, 2020

breathingdust removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

r/aws_api_gateway_stage: Support canary release deployment #2793

r/aws_api_gateway_stage: Support canary release deployment #2793

erran commented Dec 28, 2017 •

edited

Loading

bflad left a comment

bflad Apr 26, 2018

bflad Apr 26, 2018

bflad Apr 26, 2018

bflad Apr 26, 2018

bflad Apr 26, 2018

erran commented May 8, 2018

bflad commented Oct 9, 2018

ghost commented Apr 3, 2020

r/aws_api_gateway_stage: Support canary release deployment #2793

r/aws_api_gateway_stage: Support canary release deployment #2793

Conversation

erran commented Dec 28, 2017 • edited Loading

Description

Motivation

Documentation

Acceptance test run

bflad left a comment

Choose a reason for hiding this comment

bflad Apr 26, 2018

Choose a reason for hiding this comment

bflad Apr 26, 2018

Choose a reason for hiding this comment

bflad Apr 26, 2018

Choose a reason for hiding this comment

bflad Apr 26, 2018

Choose a reason for hiding this comment

bflad Apr 26, 2018

Choose a reason for hiding this comment

erran commented May 8, 2018

bflad commented Oct 9, 2018

ghost commented Apr 3, 2020

erran commented Dec 28, 2017 •

edited

Loading