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

Jump to bottom

provider/aws: API Gateway Custom Authorizer #8535

Merged
merged 4 commits into from
Aug 29, 2016
Merged

provider/aws: API Gateway Custom Authorizer #8535

merged 4 commits into from
Aug 29, 2016

Conversation

radeksimko
Copy link
Member

Replaces #6731

Thanks @johnjelinek for most of the work here. I just added a few missing bits.

Test plan

One test may actually fail until #8533 is merged.

make testacc TEST=./builtin/providers/aws TESTARGS='-run=AWSAPIGateway'

TF_ACC=1 go test ./builtin/providers/aws -v -run=AWSAPIGateway -timeout 120m
=== RUN   TestAccAWSAPIGatewayAccount_importBasic
--- PASS: TestAccAWSAPIGatewayAccount_importBasic (16.81s)
=== RUN   TestAccAWSAPIGatewayApiKey_importBasic
--- PASS: TestAccAWSAPIGatewayApiKey_importBasic (43.76s)
=== RUN   TestAccAWSAPIGatewayAccount_basic
--- PASS: TestAccAWSAPIGatewayAccount_basic (120.88s)
=== RUN   TestAccAWSAPIGatewayApiKey_basic
--- PASS: TestAccAWSAPIGatewayApiKey_basic (42.53s)
=== RUN   TestAccAWSAPIGatewayAuthorizer_basic
--- PASS: TestAccAWSAPIGatewayAuthorizer_basic (52.22s)
=== RUN   TestAccAWSAPIGatewayBasePath_basic
--- PASS: TestAccAWSAPIGatewayBasePath_basic (40.37s)
=== RUN   TestAccAWSAPIGatewayDeployment_basic
--- PASS: TestAccAWSAPIGatewayDeployment_basic (37.23s)
=== RUN   TestAccAWSAPIGatewayDomainName_basic
--- PASS: TestAccAWSAPIGatewayDomainName_basic (20.89s)
=== RUN   TestAccAWSAPIGatewayIntegrationResponse_basic
--- PASS: TestAccAWSAPIGatewayIntegrationResponse_basic (58.66s)
=== RUN   TestAccAWSAPIGatewayIntegration_basic
--- PASS: TestAccAWSAPIGatewayIntegration_basic (48.09s)
=== RUN   TestAccAWSAPIGatewayMethodResponse_basic
--- PASS: TestAccAWSAPIGatewayMethodResponse_basic (55.87s)
=== RUN   TestAccAWSAPIGatewayMethod_basic
--- PASS: TestAccAWSAPIGatewayMethod_basic (43.93s)
=== RUN   TestAccAWSAPIGatewayMethod_customauthorizer
--- PASS: TestAccAWSAPIGatewayMethod_customauthorizer (59.41s)
=== RUN   TestAccAWSAPIGatewayModel_basic
--- PASS: TestAccAWSAPIGatewayModel_basic (24.93s)
=== RUN   TestAccAWSAPIGatewayResource_basic
--- PASS: TestAccAWSAPIGatewayResource_basic (29.13s)
=== RUN   TestAccAWSAPIGatewayRestApi_basic
--- PASS: TestAccAWSAPIGatewayRestApi_basic (43.80s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    738.540s

stack72
stack72 reviewed Aug 29, 2016
View reviewed changes
builtin/providers/aws/resource_aws_api_gateway_method.go
@@ -107,6 +112,7 @@ func resourceAwsApiGatewayMethodCreate(d *schema.ResourceData, meta interface{})
RequestModels: aws.StringMap(models),
RequestParameters: aws.BoolMap(parameters),
ApiKeyRequired: aws.Bool(d.Get("api_key_required").(bool)),
AuthorizerId: aws.String(d.Get("authorizer_id").(string)),
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is optional, shouldn't we check that it is present before passing it to the API?

Copy link
Member Author

Choose a reason for hiding this comment

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

True, we could do that, although based on tests it has the same effect as passing an empty string. Admittedly I'm in favour of changes that make resulting HTTP requests smaller so I'll update the PR. 😉 👍

@stack72 stack72 mentioned this pull request Aug 29, 2016
Closed
@radeksimko radeksimko force-pushed the f-aws-apig-custom-auth branch from c29cec7 to 0a32edf Compare August 29, 2016 19:43
@radeksimko
Copy link
Member Author

PR Updated + rebased. Also I reran the acceptance tests and they're all passing now. 🎉

@stack72
Copy link
Contributor

stack72 commented Aug 29, 2016

@radeksimko

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=AWSAPIGateway'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=AWSAPIGateway -timeout 120m
=== RUN   TestAccAWSAPIGatewayAccount_importBasic
--- PASS: TestAccAWSAPIGatewayAccount_importBasic (33.40s)
=== RUN   TestAccAWSAPIGatewayApiKey_importBasic
--- PASS: TestAccAWSAPIGatewayApiKey_importBasic (39.00s)
=== RUN   TestAccAWSAPIGatewayAccount_basic
--- PASS: TestAccAWSAPIGatewayAccount_basic (99.72s)
=== RUN   TestAccAWSAPIGatewayApiKey_basic
--- PASS: TestAccAWSAPIGatewayApiKey_basic (39.76s)
=== RUN   TestAccAWSAPIGatewayAuthorizer_basic
--- PASS: TestAccAWSAPIGatewayAuthorizer_basic (51.75s)
=== RUN   TestAccAWSAPIGatewayBasePath_basic
--- PASS: TestAccAWSAPIGatewayBasePath_basic (37.72s)
=== RUN   TestAccAWSAPIGatewayDeployment_basic
--- PASS: TestAccAWSAPIGatewayDeployment_basic (33.24s)
=== RUN   TestAccAWSAPIGatewayDomainName_basic
--- PASS: TestAccAWSAPIGatewayDomainName_basic (20.19s)
=== RUN   TestAccAWSAPIGatewayIntegrationResponse_basic
--- PASS: TestAccAWSAPIGatewayIntegrationResponse_basic (51.22s)
=== RUN   TestAccAWSAPIGatewayIntegration_basic
--- PASS: TestAccAWSAPIGatewayIntegration_basic (43.42s)
=== RUN   TestAccAWSAPIGatewayMethodResponse_basic
--- PASS: TestAccAWSAPIGatewayMethodResponse_basic (44.99s)
=== RUN   TestAccAWSAPIGatewayMethod_basic
--- PASS: TestAccAWSAPIGatewayMethod_basic (38.66s)
=== RUN   TestAccAWSAPIGatewayMethod_customauthorizer
--- PASS: TestAccAWSAPIGatewayMethod_customauthorizer (56.44s)
=== RUN   TestAccAWSAPIGatewayModel_basic
--- PASS: TestAccAWSAPIGatewayModel_basic (20.66s)
=== RUN   TestAccAWSAPIGatewayResource_basic
--- PASS: TestAccAWSAPIGatewayResource_basic (24.99s)
=== RUN   TestAccAWSAPIGatewayRestApi_basic
--- PASS: TestAccAWSAPIGatewayRestApi_basic (39.83s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    675.007s

@stack72 stack72 merged commit cc38378 into hashicorp:master Aug 29, 2016
@radeksimko radeksimko deleted the f-aws-apig-custom-auth branch August 29, 2016 20:08
@zanona zanona mentioned this pull request Aug 30, 2016
Closed
swestcott
swestcott reviewed Aug 30, 2016
View reviewed changes
website/source/docs/providers/aws/r/api_gateway_method.html.markdown
@@ -39,7 +39,8 @@ The following arguments are supported:
* `rest_api_id` - (Required) The ID of the associated REST API
* `resource_id` - (Required) The API resource ID
* `http_method` - (Required) The HTTP Method (`GET`, `POST`, `PUT`, `DELETE`, `HEAD`, `OPTION`)
* `authorization` - (Required) The type of authorization used for the method
* `authorization` - (Required) The type of authorization used for the method (`NONE`, `CUSTOM`)
Copy link
Contributor

Choose a reason for hiding this comment

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

AWS_IAM is also a valid authorisation type (authorizer_id not required for this)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the note @swestcott , would you mind sending a PR with this small fix?

@ghost
Copy link

ghost commented Apr 22, 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 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement provider/aws
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@radeksimko @stack72 @swestcott @johnjelinek