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.

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

provider/aws: Respect 'selection_pattern' in api_gateway_integration_response #5893

Merged
merged 4 commits into from
Apr 27, 2016

Conversation

radeksimko
Copy link
Member

Fixes #5891

@@ -72,6 +72,7 @@ func resourceAwsApiGatewayIntegrationResponseCreate(d *schema.ResourceData, meta
RestApiId: aws.String(d.Get("rest_api_id").(string)),
StatusCode: aws.String(d.Get("status_code").(string)),
ResponseTemplates: aws.StringMap(templates),
SelectionPattern: aws.String(d.Get("selection_pattern").(string)),
Copy link
Contributor

Choose a reason for hiding this comment

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

@radeksimko, this is an optional parameter. What happens if we don't set anything for this? Shouldn't this be wrapped in an if v, ok := d.GetOk( style code block?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ha, very good point. Thanks for spotting it. I will add d.GetOk.

@stack72
Copy link
Contributor

stack72 commented Mar 31, 2016

1 question on this, but the tests pass as expected:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAPIGatewayIntegrationResponse' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAPIGatewayIntegrationResponse -timeout 120m
=== RUN   TestAccAWSAPIGatewayIntegrationResponse_basic
--- PASS: TestAccAWSAPIGatewayIntegrationResponse_basic (13.41s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    13.426s

@stack72 stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Mar 31, 2016
@ijin
Copy link
Contributor

ijin commented Apr 2, 2016

I've tried testing with this branch, but can't seem to get it to work.

resource "aws_api_gateway_integration_response" "ip_get_200" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  resource_id = "${aws_api_gateway_resource.ip.id}"
  http_method = "${aws_api_gateway_method.ip_get.http_method}"
  status_code = "${aws_api_gateway_method_response.ip_200.status_code}"
  response_templates = {
    "application/json" = "$input.path('$')"
  }
}

resource "aws_api_gateway_integration_response" "ip_get_400" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  resource_id = "${aws_api_gateway_resource.ip.id}"
  http_method = "${aws_api_gateway_method.ip_get.http_method}"
  status_code = "${aws_api_gateway_method_response.ip_400.status_code}"
  selection_pattern = "[^0-9](.|\n)*"
  response_templates = {
    "application/json" = "$input.path('$').errorMessage"
  }
}

gives me,

* aws_api_gateway_integration_response.ip_get_400: Error creating API Gateway Integration Response: ConflictException: Integration response with selection pattern default already exists
        status code: 409, request id: d63d9f93-f890-12e3-b34a-31d1f71db609

Currently, I have to manually call the API afterwards as a workaround.

$ aws apigateway put-integration-response --rest-api-id somgjn2ehe --resource-id 1kijnf --http-method GET --status-code 400 --response-templates '{"application/json": "$input.path('$').errorMessage"}' --selection-pattern "[^0-9](.|\n)*"                                                                                                                           
{
    "statusCode": "400", 
    "selectionPattern": "[^0-9](.|\\n)*", 
    "responseTemplates": {
        "application/json": "$input.path().errorMessage"
    }
}

@stack72 stack72 self-assigned this Apr 5, 2016
@dsalazar-carecloud
Copy link

+1 Would love to have the selection pattern option available for use.

@IRGraham
Copy link

Bump. Any changes made in the last 20 days?

@radeksimko
Copy link
Member Author

radeksimko commented Apr 23, 2016

@ijin

I've tried testing with this branch, but can't seem to get it to work.

I was not able to reproduce this - can you provide more context - i.e. more Terraform configs (specifically for aws_api_gateway_integration, aws_api_gateway_method & aws_api_gateway_method_response + related resources) + ideally output from terraform show (minus any secrets)? Or at least say which integration type you are using?

I was testing apply via current stable version -> plan+apply via custom built version from this branch.

I had three configs with different aws_api_gateway_integration - one with type = MOCK, another one with type = HTTP and last one with type = AWS pointing to Lambda.

aws_api_gateway_integration_response correctly updated the selection_pattern in all three cases.


Side note: Whether it's causing issues or not I think we should probably be using Update instead of Put for updating the integration response - it would be more efficient and predictable I think. That's probably outside of scope for this PR though.

@stack72
Copy link
Contributor

stack72 commented Apr 25, 2016

@radeksimko how are we getting on with this PR? Nearly in a state for a review again? 👍

@radeksimko
Copy link
Member Author

@stack72 I will modify it per your comments either today or in the following days.

I was hoping @ijin would come up with repro case for the issue he mentioned, but if not, I think I'll just modify it and make it ready for final review.

@stack72
Copy link
Contributor

stack72 commented Apr 25, 2016

👍

@ijin
Copy link
Contributor

ijin commented Apr 26, 2016

@radeksimko @stack72 sorry for the late reply, but here is the whole .tf that fails.

Same error.

* aws_api_gateway_integration_response.ip_get_400: Error creating API Gateway Integration Response: ConflictException: Integration response with selection pattern default already exists
        status code: 409, request id: eb2a130e-0b93-11e6-87c3-afa3aa40b6be
provider "aws" {
    region = "ap-northeast-1"
    allowed_account_ids = ["${var.names.account_id}"]
}

variable "names" {
    default = {
        account_id = "$ACCOUNT_ID"
    }
}


# REST API
resource "aws_api_gateway_rest_api" "EB" {
  name = "EB"
  description = "get EB info"
}

# Resources
resource "aws_api_gateway_resource" "eb" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  parent_id = "${aws_api_gateway_rest_api.EB.root_resource_id}"
  path_part = "eb"
}

resource "aws_api_gateway_resource" "env_name" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  parent_id = "${aws_api_gateway_resource.eb.id}"
  path_part = "{env_name}"
}

resource "aws_api_gateway_resource" "ip" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  parent_id = "${aws_api_gateway_resource.env_name.id}"
  path_part = "ip"
}

# Methods
resource "aws_api_gateway_method" "ip_get" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  resource_id = "${aws_api_gateway_resource.ip.id}"
  http_method = "GET"
  authorization = "NONE"
}

# Integrations
resource "aws_api_gateway_integration" "ip_get" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  resource_id = "${aws_api_gateway_resource.ip.id}"
  http_method = "${aws_api_gateway_method.ip_get.http_method}"
  integration_http_method = "POST"
  type = "AWS"
  uri = "arn:aws:apigateway:ap-northeast-1:lambda:path/2015-03-31/functions/arn:aws:lambda:ap-northeast-1:${var.names.account_id}:function:eb_ip/invocations"
  request_templates = {
    "application/json" = "{ \"env_name\": \"$input.params('env_name')\" }"
  }
  depends_on = ["aws_api_gateway_method.ip_get"]
}

# Method responses
resource "aws_api_gateway_method_response" "ip_200" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  resource_id = "${aws_api_gateway_resource.ip.id}"
  http_method = "${aws_api_gateway_method.ip_get.http_method}"
  status_code = "200"
}

resource "aws_api_gateway_method_response" "ip_400" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  resource_id = "${aws_api_gateway_resource.ip.id}"
  http_method = "${aws_api_gateway_method.ip_get.http_method}"
  status_code = "400"
}

# Integration responses
resource "aws_api_gateway_integration_response" "ip_get_200" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  resource_id = "${aws_api_gateway_resource.ip.id}"
  http_method = "${aws_api_gateway_method.ip_get.http_method}"
  status_code = "${aws_api_gateway_method_response.ip_200.status_code}"
  response_templates = {
    "application/json" = "$input.path('$')"
  }
}

resource "aws_api_gateway_integration_response" "ip_get_400" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  resource_id = "${aws_api_gateway_resource.ip.id}"
  http_method = "${aws_api_gateway_method.ip_get.http_method}"
  status_code = "${aws_api_gateway_method_response.ip_400.status_code}"
  selection_pattern = "[^0-9](.|\n)*"
  response_templates = {
    "application/json" = "$input.path('$').errorMessage"
  }
  depends_on = ["aws_api_gateway_integration_response.ip_get_200"]
}

# Wait for https://github.com/hashicorp/terraform/pull/5893
# until then, use 
# aws apigateway get-rest-apis --output table
# aws apigateway get-resources --rest-api-id `aws apigateway get-rest-apis | jq .items[].id -r` --output table
# aws apigateway put-integration-response --rest-api-id somgjn6eha --resource-id g901ec --http-method GET --status-code 400 --response-templates '{"application/json": "$input.path('$').errorMessage"}' --selection-pattern "[^0-9](.|\n)*" 


resource "aws_api_gateway_deployment" "eb_deployment" {
  rest_api_id = "${aws_api_gateway_rest_api.EB.id}"
  depends_on = ["aws_api_gateway_integration.ip_get"]
  stage_name = "prod"
}

## Permissions
#resource "aws_lambda_permission" "apigw_ip" {
#  statement_id = "apigw_for_ip"
#  action = "lambda:InvokeFunction"
#  function_name = "eb_ip"
#  principal = "apigateway.amazonaws.com"
#  source_arn = "arn:aws:execute-api:ap-northeast-1:${var.names.account_id}:${aws_api_gateway_rest_api.EB.id}/*/GET/eb/*/${aws_api_gateway_resource.ip.path_part}"
#}
#

Replace $ACCOUNT_ID.

Sometimes I was getting a BadRequestException: Unable to complete operation due to concurrent modification. Please try again later. so I added a depends_on to one of the aws_api_gateway_integration_response method.

@radeksimko
Copy link
Member Author

@ijin Reproduced w/ unmodified v0.6.14 release by applying the config you provided. Thanks.

Error creating API Gateway Integration Response: ConflictException: Integration response with selection pattern default already exists
status code: 409

Just looking into it.

@radeksimko
Copy link
Member Author

If I compile a patched version from this PR and use it to apply your config from scratch, it works ok, so theoretically you would be able to upgrade Terraform to patched version & just taint the aws_api_gateway_integration_response.ip_get_400.

I will check if there's any automated migration we can do for you and potentially other users that are already using selection_pattern, so taint isn't necessary.

@radeksimko radeksimko removed the waiting-response An issue/pull request is waiting for a response from the community label Apr 27, 2016
@radeksimko
Copy link
Member Author

I was able to do the upgrade smoothly without having to taint anything, see the steps I performed in my gist: https://gist.github.com/radeksimko/bcdfa64147f9c617d628cc0f81e2e0c3

@ijin Can you confirm the patched version is working for you?

@radeksimko radeksimko added the waiting-response An issue/pull request is waiting for a response from the community label Apr 27, 2016
@ijin
Copy link
Contributor

ijin commented Apr 27, 2016

@radeksimko ok, I got it working with commit a9dc48158d, confirmed.

Thanks!

On a side note, I had to add another depends_on = ["aws_api_gateway_method_response.ip_200"] in the aws_api_gateway_method_response.ip_400 to circumvent concurrent operations errors. Otherwise, I had to apply twice. But perhaps this should be another issue.

@radeksimko
Copy link
Member Author

On a side note, I had to add another depends_on = ["aws_api_gateway_method_response.ip_200"] in the aws_api_gateway_method_response.ip_400 to circumvent concurrent operations errors.

Right, that's believable, I think that's a separate issue though which can be fixed separately via PR to docs.

Terraform is always trying to run many operations in parallel to CRUD resources quickly, but some need to be created first, even though they may not contain any references between each other, so the dependency is not obvious for Terraform, hence it doesn't know that certain operations cannot run in parallel hence the explicit depends_on is necessary in some cases.

@radeksimko radeksimko removed the waiting-response An issue/pull request is waiting for a response from the community label Apr 27, 2016
@stack72
Copy link
Contributor

stack72 commented Apr 27, 2016

@radeksimko this LGTM! Merge at will :)

@radeksimko radeksimko merged commit 7642fa0 into hashicorp:master Apr 27, 2016
@radeksimko radeksimko deleted the b-aws-api-gateway-fields branch April 27, 2016 18:46
@ijin
Copy link
Contributor

ijin commented Apr 28, 2016

👍

@ghost
Copy link

ghost commented Apr 26, 2020

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_api_gateway_integration_response selection_pattern not sent
5 participants