[WIP] AWS APIGateway Custom Authorizer #6731

johnjelinek · 2016-05-17T23:59:50Z

I see that there is already an AWS APIGateway Authorizer. I'm going to try to wire up the resource to an AWS APIGateway method. Looking for feedback along the way :). This is a WIP placeholder unless someone else can beat me to it.

johnjelinek · 2016-05-18T02:01:55Z

I'm trying to figure out how to handle the following:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAPIGatewayMethod_customauthorizer'
==> Checking that code complies with gofmt requirements...
/Users/administrator/go/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
2016/05/17 20:42:37 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAPIGatewayMethod_customauthorizer -timeout 120m
=== RUN   TestAccAWSAPIGatewayMethod_customauthorizer
--- FAIL: TestAccAWSAPIGatewayMethod_customauthorizer (28.45s)
    testing.go:234: Step 1 error: Error applying: 1 error(s) occurred:

        * aws_api_gateway_authorizer.test: Deleting API Gateway Authorizer failed: ConflictException: Cannot delete authorizer 'tf-acc-test-authorizer', is referenced in method: GET//test
            status code: 409, request id: e66e977f-1c99-11e6-ab1a-5d2160b20552
    testing.go:295: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: Error applying: 1 error(s) occurred:

        * aws_api_gateway_authorizer.test: Deleting API Gateway Authorizer failed: ConflictException: Cannot delete authorizer 'tf-acc-test-authorizer', is referenced in method: GET//test
            status code: 409, request id: e871449c-1c99-11e6-af10-131e5fb9adc0

        State: aws_api_gateway_authorizer.test:
          ID = 87kvkh
          authorizer_credentials = arn:aws:iam::116738426468:role/tf_acc_api_gateway_auth_invocation_role
          authorizer_result_ttl_in_seconds = 0
          authorizer_uri = arn:aws:apigateway:region:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:116738426468:function:tf_acc_api_gateway_authorizer/invocations
          identity_source = method.request.header.Authorization
          identity_validation_expression =
          name = tf-acc-test-authorizer
          rest_api_id = opatedcbg0
          type = TOKEN

          Dependencies:
            aws_api_gateway_rest_api.test
            aws_lambda_function.authorizer
            aws_iam_role.invocation_role
        aws_api_gateway_rest_api.test:
          ID = opatedcbg0
          description =
          name = test
          root_resource_id = 3vmcy4llok
        aws_iam_role.iam_for_lambda:
          ID = tf_acc_iam_for_lambda_api_gateway_authorizer
          arn = arn:aws:iam::116738426468:role/tf_acc_iam_for_lambda_api_gateway_authorizer
          assume_role_policy = {
          "Version": "2012-10-17",
          "Statement": [
            {
              "Action": "sts:AssumeRole",
              "Principal": {
                "Service": "lambda.amazonaws.com"
              },
              "Effect": "Allow",
              "Sid": ""
            }
          ]
        }

          name = tf_acc_iam_for_lambda_api_gateway_authorizer
          path = /
          unique_id = AROAJJNV5T5L7XCZIN5VO
        aws_iam_role.invocation_role:
          ID = tf_acc_api_gateway_auth_invocation_role
          arn = arn:aws:iam::116738426468:role/tf_acc_api_gateway_auth_invocation_role
          assume_role_policy = {
          "Version": "2012-10-17",
          "Statement": [
            {
              "Action": "sts:AssumeRole",
              "Principal": {
                "Service": "apigateway.amazonaws.com"
              },
              "Effect": "Allow",
              "Sid": ""
            }
          ]
        }

          name = tf_acc_api_gateway_auth_invocation_role
          path = /
          unique_id = AROAIGQR5VMFQTQX4U43A
        aws_lambda_function.authorizer:
          ID = tf_acc_api_gateway_authorizer
          arn = arn:aws:lambda:us-west-2:116738426468:function:tf_acc_api_gateway_authorizer
          description =
          filename = test-fixtures/lambdatest.zip
          function_name = tf_acc_api_gateway_authorizer
          handler = exports.example
          last_modified = 2016-05-18T01:43:05.135+0000
          memory_size = 128
          role = arn:aws:iam::116738426468:role/tf_acc_iam_for_lambda_api_gateway_authorizer

any tips?

johnjelinek · 2016-05-18T02:24:24Z

@radeksimko: could you have a look here?

johnjelinek · 2016-05-18T02:44:49Z

I got a repro, where if the authorizer is deleted prior to the apigateway method, then it results in the conflict exception:

aws apigateway delete-authorizer --rest-api-id opatedcbg0 --region=us-west-2 --authorizer-id 87kvkh

A client error (ConflictException) occurred when calling the DeleteAuthorizer operation: Cannot delete authorizer 'tf-acc-test-authorizer', is referenced in method: GET//

johnjelinek · 2016-05-18T02:50:51Z

builtin/providers/aws/resource_aws_api_gateway_method_test.go

+  resource_id = "${aws_api_gateway_resource.test.id}"
+  http_method = "GET"
+  authorization = "CUSTOM"
+  authorizer_id = "${aws_api_gateway_authorizer.test.id}"


I think this is the tricky part. Because I have a reference to the authorizer_id here, terraform wants to delete the authorizer before deleting the method, but the AWS SDK API expects all methods to be deleted first. Maybe instead, the authorizer should have a collection of methods it should attach to, instead. What do you think?

terraform wants to delete the authorizer before deleting the method

in what situation exactly? on complete terraform destroy? I believe the opposite is reality:
https://gist.github.com/radeksimko/3d9b0637ad821b34de7be4e7e2c993fe - snippet below:

aws_api_gateway_method.MyDemoMethod: Destruction complete ... aws_api_gateway_authorizer.demo: Destroying... aws_api_gateway_authorizer.demo: Destruction complete

johnjelinek · 2016-05-18T03:03:10Z

If I set an explicit dependency like this:

resource "aws_api_gateway_authorizer" "test" {
  name = "tf-acc-test-authorizer"
  rest_api_id = "${aws_api_gateway_rest_api.test.id}"
  authorizer_uri = "arn:aws:apigateway:region:lambda:path/2015-03-31/functions/${aws_lambda_function.authorizer.arn}/invocations"
  authorizer_credentials = "${aws_iam_role.invocation_role.arn}"
  depends_on = ["aws_api_gateway_method.test"]
}

then I get an error like this:

--- FAIL: TestAccAWSAPIGatewayMethod_customauthorizer (0.01s)
    testing.go:234: Step 0 error: Configuration is invalid.

        Warnings: []string(nil)

        Errors: []string{"1 error(s) occurred:\n\n* Cycle: aws_api_gateway_authorizer.test, aws_api_gateway_method.test"}

johnjelinek · 2016-05-18T14:57:52Z

I added a TODO for anyone who knows how to change the destroy order without setting cyclical dependencies. I'm gonna update the docs and then, I guess this will be ready for merging.

johnjelinek · 2016-05-18T15:04:49Z

@radeksimko: docs are in place for review.

johnjelinek · 2016-05-19T20:36:04Z

any feedback?

gthole · 2016-07-13T11:09:46Z

This is pretty important for our use case, and the authorizer resource is useless without completing the integration to the method like this. Can this be reviewed please? (@johnjelinek possibly remove the WIP tag if it's ready for review.)

I'm going to cherry-pick this into my fork and test it against our stack. I'm happy to report back here if it will be helpful.

johnjelinek · 2016-07-13T11:30:12Z

That would be super helpful if you could test. I can remove WIP also.

On Wednesday, July 13, 2016, Greg Thole notifications@github.com wrote:

This is pretty important for our use case, and the authorizer resource is
useless without completing the integration to the method like this. Can
this be reviewed please? (@johnjelink possibly remove the WIP tag if it's
ready for review.)

I'm going to cherry-pick this into my fork and test it against our stack.
I'm happy to report back here if it will be helpful.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#6731 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA1UihTHjevcDorw3Oalf9m3zMBBfGlLks5qVMeHgaJpZM4Ig0Qt
.

gthole · 2016-07-13T17:21:57Z

I built the binaries with this commit on top of v0.6.16 (make dev apparently doesn't create all the plugins on master anymore, which is the only reason I used the v0.6.16 tag as my base). It worked great for me and I've been able to actually hook up the custom authorizer resource to something.

johnjelinek · 2016-07-15T01:32:47Z

That's great! Glad to hear it. Can we get this merged?

On Wednesday, July 13, 2016, Greg Thole notifications@github.com wrote:

I built the binaries with this commit on top of v0.6.16 (make dev
apparently doesn't create all the plugins on master anymore, which is the
only reason I used the v0.6.16 tag as my base). It worked great for me and
I've been able to actually hook up the custom authorizer resource to
something.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#6731 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA1UigOOLboLO5iz3Y7bYu7f5EqrzRt6ks5qVR7EgaJpZM4Ig0Qt
.

radeksimko · 2016-07-25T12:26:01Z

builtin/providers/aws/resource_aws_api_gateway_authorizer.go

-		return fmt.Errorf("Deleting API Gateway Authorizer failed: %s", err)
+		// XXX: Figure out a way to delete the method that depends on the authorizer first
+		// otherwise the authorizer will be dangling until the API is deleted
+		if !strings.Contains(err.Error(), "ConflictException") {


As long as the authorizer is managed in the same terraform scope and is referenced properly (i.e. not hardcoded authorizer IDs in methods) Terraform will schedule the deletion correctly by default - i.e. for complete destruction 1st method => 2nd authorizer. Have a look at the dependency graph via terraform graph. 😉

The only issue that may theoretically arise is eventual consistency of the AWS API (or the implementation of it) - i.e. the authorizer_id change may take time to propagate. We usually just retry the deletion in such cases with a reasonable timeout.

Did you experience this problem yourself @johnjelinek ?

radeksimko · 2016-07-25T13:38:43Z

Hi @johnjelinek ,
I did try to run attached acceptance test couple of times (5x to be exact) and it always passed. I was not able to reproduce the ConflictException mentioned in your initial post.

If you saw this error while using the exact same config, it must be eventual consistency and we should add Retry block in that case. If you do decide to add such retry block, I'd suggest you check the error code via exact match instead of using strings.Contains().

I left you one more comment about reading + updating the field. Once that's resolved I'm happy to merge this 😉

zanona · 2016-08-23T23:34:23Z

+1 for merging.

stack72 · 2016-08-29T19:19:43Z

Closed in favour of #8535

ghost · 2020-04-22T02:31:18Z

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.

johnjelinek force-pushed the master branch from a1c8bc6 to b41d4b8 Compare May 18, 2016 01:52

johnjelinek reviewed May 18, 2016
View reviewed changes

catsby added enhancement provider/aws labels May 18, 2016

johnjelinek force-pushed the master branch from b41d4b8 to 6176383 Compare May 18, 2016 14:55

radeksimko self-assigned this May 18, 2016

johnjelinek force-pushed the master branch from 6176383 to 402e318 Compare May 18, 2016 15:04

johnjelinek force-pushed the master branch from 402e318 to 0fb5ddb Compare May 18, 2016 15:06

[WIP] AWS APIGateway Custom Authorizer

b47122f

johnjelinek force-pushed the master branch from 0fb5ddb to b47122f Compare May 18, 2016 15:06

radeksimko reviewed Jul 25, 2016
View reviewed changes

radeksimko added the waiting-response An issue/pull request is waiting for a response from the community label Jul 25, 2016

radeksimko mentioned this pull request Aug 29, 2016

provider/aws: API Gateway Custom Authorizer #8535

Merged

stack72 closed this Aug 29, 2016

ghost locked and limited conversation to collaborators Apr 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] AWS APIGateway Custom Authorizer #6731

[WIP] AWS APIGateway Custom Authorizer #6731

johnjelinek commented May 17, 2016

johnjelinek commented May 18, 2016

johnjelinek commented May 18, 2016

johnjelinek commented May 18, 2016

johnjelinek May 18, 2016

radeksimko Jul 25, 2016

johnjelinek commented May 18, 2016

johnjelinek commented May 18, 2016

johnjelinek commented May 18, 2016

johnjelinek commented May 19, 2016

gthole commented Jul 13, 2016 •

edited

Loading

johnjelinek commented Jul 13, 2016

gthole commented Jul 13, 2016

johnjelinek commented Jul 15, 2016

radeksimko Jul 25, 2016

radeksimko commented Jul 25, 2016

zanona commented Aug 23, 2016

stack72 commented Aug 29, 2016

ghost commented Apr 22, 2020

[WIP] AWS APIGateway Custom Authorizer #6731

[WIP] AWS APIGateway Custom Authorizer #6731

Conversation

johnjelinek commented May 17, 2016

johnjelinek commented May 18, 2016

johnjelinek commented May 18, 2016

johnjelinek commented May 18, 2016

johnjelinek May 18, 2016

Choose a reason for hiding this comment

radeksimko Jul 25, 2016

Choose a reason for hiding this comment

johnjelinek commented May 18, 2016

johnjelinek commented May 18, 2016

johnjelinek commented May 18, 2016

johnjelinek commented May 19, 2016

gthole commented Jul 13, 2016 • edited Loading

johnjelinek commented Jul 13, 2016

gthole commented Jul 13, 2016

johnjelinek commented Jul 15, 2016

radeksimko Jul 25, 2016

Choose a reason for hiding this comment

radeksimko commented Jul 25, 2016

zanona commented Aug 23, 2016

stack72 commented Aug 29, 2016

ghost commented Apr 22, 2020

gthole commented Jul 13, 2016 •

edited

Loading