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

r/aws_api_gateway_stage: Support canary release deployment #2793

Closed
wants to merge 2 commits into from
Closed

r/aws_api_gateway_stage: Support canary release deployment #2793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a09f01
r/aws_api_gateway_stage: Support canary release deployment
erran Dec 28, 2017
d38a43b
r/aws_api_gateway_stage: Remove optional deployment id field from can…
erran Dec 28, 2017
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
120 changes: 120 additions & 0 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 @@ -28,6 +28,30 @@ func resourceAwsApiGatewayStage() *schema.Resource {
Type: schema.TypeString,
Optional: true,
},
"canary_settings": {
Type: schema.TypeList,
Optional: true,
MinItems: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: MinItems: 0 is extraneous here 😄

MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"percent_traffic": {
Type: schema.TypeFloat,
Optional: true,
Default: 0.0,
},
"stage_variable_overrides": {
Type: schema.TypeMap,
Elem: schema.TypeString,
Optional: true,
},
"use_stage_cache": {
Type: schema.TypeBool,
Optional: true,
},
},
},
},
"client_certificate_id": {
Type: schema.TypeString,
Optional: true,
Expand Down Expand Up @@ -62,6 +86,62 @@ func resourceAwsApiGatewayStage() *schema.Resource {
}
}

func appendCanarySettingsPatchOperations(operations []*apigateway.PatchOperation, oldCanarySettingsRaw, newCanarySettingsRaw []interface{}) []*apigateway.PatchOperation {
if len(newCanarySettingsRaw) == 0 { // Schema guarantees either 0 or 1
return append(operations, &apigateway.PatchOperation{
Op: aws.String("remove"),
Path: aws.String("/canarySettings"),
})
}
newSettings := newCanarySettingsRaw[0].(map[string]interface{})

var oldSettings map[string]interface{}
if len(oldCanarySettingsRaw) == 1 { // Schema guarantees either 0 or 1
oldSettings = oldCanarySettingsRaw[0].(map[string]interface{})
} else {
oldSettings = map[string]interface{}{
"percent_traffic": 0.0,
"stage_variable_overrides": make(map[string]interface{}),
"use_stage_cache": false,
}
}

oldOverrides := oldSettings["stage_variable_overrides"].(map[string]interface{})
newOverrides := newSettings["stage_variable_overrides"].(map[string]interface{})
operations = append(operations, diffVariablesOps("/canarySettings/stageVariableOverrides/", oldOverrides, newOverrides)...)

oldPercentTraffic := oldSettings["percent_traffic"].(float64)
newPercentTraffic := newSettings["percent_traffic"].(float64)
if oldPercentTraffic != newPercentTraffic {
operations = append(operations, &apigateway.PatchOperation{
Op: aws.String("replace"),
Path: aws.String("/canarySettings/percentTraffic"),
Value: aws.String(fmt.Sprintf("%f", newPercentTraffic)),
})
}

oldUseStageCache := oldSettings["use_stage_cache"].(bool)
newUseStageCache := newSettings["use_stage_cache"].(bool)
if oldUseStageCache != newUseStageCache {
operations = append(operations, &apigateway.PatchOperation{
Op: aws.String("replace"),
Path: aws.String("/canarySettings/useStageCache"),
Value: aws.String(fmt.Sprintf("%t", newUseStageCache)),
})
}

return operations
}

func readStageVariableOverrides(overrides map[string]interface{}) map[string]string {
result := make(map[string]string)
for k, v := range overrides {
result[k] = v.(string)
}

return result
}

func resourceAwsApiGatewayStageCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).apigateway

Expand All @@ -82,6 +162,36 @@ func resourceAwsApiGatewayStageCreate(d *schema.ResourceData, meta interface{})
input.CacheClusterSize = aws.String(v.(string))
waitForCache = true
}
if v, ok := d.GetOk("canary_settings"); ok {
canarySettings := v.([]interface{})
canarySetting, ok := canarySettings[0].(map[string]interface{})
if !ok {
return fmt.Errorf("At least one field is expected inside canary_settings")
Copy link
Contributor

Choose a reason for hiding this comment

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

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

}

var stageVariableOverrides map[string]string
if canarySettingVariables, ok := canarySetting["stage_variable_overrides"]; ok {
stageVariableOverrides = readStageVariableOverrides(canarySettingVariables.(map[string]interface{}))
} else {
return fmt.Errorf("stage_variable_overrides must be set in canary_settings")
}

percentTraffic, ok := canarySetting["percent_traffic"]
if !ok {
return fmt.Errorf("percent_traffic must be set in canary_settings")
}

useStageCache, ok := canarySetting["use_stage_cache"]
if !ok {
return fmt.Errorf("use_stage_cache must be set in canary_settings")
}

input.CanarySettings = &apigateway.CanarySettings{
StageVariableOverrides: aws.StringMap(stageVariableOverrides),
PercentTraffic: aws.Float64(percentTraffic.(float64)),
UseStageCache: aws.Bool(useStageCache.(bool)),
}
}
if v, ok := d.GetOk("description"); ok {
input.Description = aws.String(v.(string))
}
Expand Down Expand Up @@ -129,6 +239,7 @@ func resourceAwsApiGatewayStageCreate(d *schema.ResourceData, meta interface{})
}
}

d.SetPartial("canary_settings")
d.SetPartial("cache_cluster_enabled")
d.SetPartial("cache_cluster_size")
d.Partial(false)
Expand Down Expand Up @@ -168,6 +279,10 @@ func resourceAwsApiGatewayStageRead(d *schema.ResourceData, meta interface{}) er
d.Set("cache_cluster_size", stage.CacheClusterSize)
}

if err := d.Set("canary_settings", flattenApiGatewayStageCanarySettings(stage.CanarySettings)); err != nil {
log.Printf("[ERR] Error setting canary settings for api gateway stage (%s): %s", d.Id(), err)
}

d.Set("deployment_id", stage.DeploymentId)
d.Set("description", stage.Description)
d.Set("documentation_version", stage.DocumentationVersion)
Expand Down Expand Up @@ -198,6 +313,10 @@ func resourceAwsApiGatewayStageUpdate(d *schema.ResourceData, meta interface{})
})
waitForCache = true
}
if d.HasChange("canary_settings") {
oldCanarySettingsRaw, newCanarySettingsRaw := d.GetChange("canary_settings")
operations = appendCanarySettingsPatchOperations(operations, oldCanarySettingsRaw.([]interface{}), newCanarySettingsRaw.([]interface{}))
}
if d.HasChange("client_certificate_id") {
operations = append(operations, &apigateway.PatchOperation{
Op: aws.String("replace"),
Expand Down Expand Up @@ -273,6 +392,7 @@ func resourceAwsApiGatewayStageUpdate(d *schema.ResourceData, meta interface{})
}
}

d.SetPartial("canary_settings")
d.SetPartial("cache_cluster_enabled")
d.SetPartial("cache_cluster_size")
d.Partial(false)
Expand Down
23 changes: 23 additions & 0 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 @@ -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"),
Copy link
Contributor

Choose a reason for hiding this comment

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

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"),

resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "canary_settings.0.stage_variable_overrides.four", "4"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "canary_settings.0.use_stage_cache", "true"),
),
},
resource.TestStep{
Expand All @@ -34,6 +37,9 @@ func TestAccAWSAPIGatewayStage_basic(t *testing.T) {
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", "canary_settings.0.percent_traffic", "50.5"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "canary_settings.0.stage_variable_overrides.four", "5"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "canary_settings.0.use_stage_cache", "false"),
),
},
resource.TestStep{
Expand All @@ -43,6 +49,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"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "canary_settings.0.stage_variable_overrides.four", "4"),
resource.TestCheckResourceAttr("aws_api_gateway_stage.test", "canary_settings.0.use_stage_cache", "true"),
),
},
},
Expand Down Expand Up @@ -171,6 +180,13 @@ resource "aws_api_gateway_stage" "test" {
deployment_id = "${aws_api_gateway_deployment.dev.id}"
cache_cluster_enabled = true
cache_cluster_size = "0.5"
canary_settings {
percent_traffic = 33.33
stage_variable_overrides = {
four = "4"
}
use_stage_cache = "true"
}
variables {
one = "1"
two = "2"
Expand All @@ -186,6 +202,13 @@ resource "aws_api_gateway_stage" "test" {
stage_name = "prod"
deployment_id = "${aws_api_gateway_deployment.dev.id}"
cache_cluster_enabled = false
canary_settings {
percent_traffic = 50.5
stage_variable_overrides = {
four = "5"
}
use_stage_cache = "false"
}
description = "Hello world"
variables {
one = "1"
Expand Down
20 changes: 20 additions & 0 deletions aws/structure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3059,3 +3059,23 @@ func flattenMqBrokerInstances(instances []*mq.BrokerInstance) []interface{} {

return l
}

func flattenApiGatewayStageCanarySettings(canarySettings *apigateway.CanarySettings) []interface{} {
settings := make(map[string]interface{})
overrides := make(map[string]string)

if canarySettings == nil {
return nil
}

for k, v := range canarySettings.StageVariableOverrides {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

overrides[k] = *v
}
if len(overrides) > 0 {
settings["stage_variable_overrides"] = overrides
}
settings["percent_traffic"] = canarySettings.PercentTraffic
settings["use_stage_cache"] = canarySettings.UseStageCache

return []interface{}{settings}
}
22 changes: 22 additions & 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 @@ -17,6 +17,19 @@ resource "aws_api_gateway_stage" "test" {
stage_name = "prod"
rest_api_id = "${aws_api_gateway_rest_api.test.id}"
deployment_id = "${aws_api_gateway_deployment.test.id}"

variables = {
my_var = "normal value"
}

canary_settings {
percent_traffic = 33.33

stage_variable_overrides = {
my_var = "overridden value"
my_new_var = "true"
}
}
}

resource "aws_api_gateway_rest_api" "test" {
Expand Down Expand Up @@ -72,7 +85,16 @@ The following arguments are supported:
* `cache_cluster_enabled` - (Optional) Specifies whether a cache cluster is enabled for the stage
* `cache_cluster_size` - (Optional) The size of the cache cluster for the stage, if enabled.
Allowed values include `0.5`, `1.6`, `6.1`, `13.5`, `28.4`, `58.2`, `118` and `237`.
* `canary_settings` - (Optional) A map of settings for a [canary deployment][0]. Fields documented below.
* `client_certificate_id` - (Optional) The identifier of a client certificate for the stage.
* `description` - (Optional) The description of the stage
* `documentation_version` - (Optional) The version of the associated API documentation
* `variables` - (Optional) A map that defines the stage variables

**canary_settings** supports the following:

* `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.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should include Defaults to false here


[0]: https://docs.aws.amazon.com/apigateway/latest/developerguide/canary-release.html