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

[WIP] App Mesh preview - HTTP header based routing, route priorities, cookie based routing and retry policy #9468

Closed
wants to merge 23 commits into from

Conversation

ewbankkit
Copy link
Contributor

@ewbankkit ewbankkit commented Jul 23, 2019

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

WIP to add support for App Mesh HTTP Header based routing: aws/aws-app-mesh-roadmap#15.
Based on:

@ewbankkit ewbankkit requested a review from a team July 23, 2019 20:24
@ghost ghost added size/XXL Managed by automation to categorize the size of a PR. provider Pertains to the provider itself, rather than any interaction with AWS. service/appmesh Issues and PRs that pertain to the appmesh service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. dependencies Used to indicate dependency changes. documentation Introduces or discusses updates to documentation. labels Jul 23, 2019
@ewbankkit
Copy link
Contributor Author

ewbankkit commented Jul 23, 2019

Acceptance tests so far:

  • All AppMesh resources using the appmesh-preview service
  • Comment out tagging as it's not support in the current preview API
  • Minor tweaks
$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSAppmesh'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSAppmesh -timeout 120m
=== RUN   TestAccAWSAppmesh
=== RUN   TestAccAWSAppmesh/VirtualNode
=== RUN   TestAccAWSAppmesh/VirtualNode/cloudMapServiceDiscovery
=== RUN   TestAccAWSAppmesh/VirtualNode/listenerHealthChecks
=== RUN   TestAccAWSAppmesh/VirtualNode/logging
=== RUN   TestAccAWSAppmesh/VirtualNode/basic
=== RUN   TestAccAWSAppmesh/VirtualRouter
=== RUN   TestAccAWSAppmesh/VirtualRouter/basic
=== RUN   TestAccAWSAppmesh/VirtualService
=== RUN   TestAccAWSAppmesh/VirtualService/virtualNode
=== RUN   TestAccAWSAppmesh/VirtualService/virtualRouter
=== RUN   TestAccAWSAppmesh/Mesh
=== RUN   TestAccAWSAppmesh/Mesh/basic
=== RUN   TestAccAWSAppmesh/Mesh/egressFilter
=== RUN   TestAccAWSAppmesh/Route
=== RUN   TestAccAWSAppmesh/Route/httpRoute
=== RUN   TestAccAWSAppmesh/Route/tcpRoute
--- PASS: TestAccAWSAppmesh (458.26s)
    --- PASS: TestAccAWSAppmesh/VirtualNode (203.90s)
        --- PASS: TestAccAWSAppmesh/VirtualNode/cloudMapServiceDiscovery (110.57s)
        --- PASS: TestAccAWSAppmesh/VirtualNode/listenerHealthChecks (36.31s)
        --- PASS: TestAccAWSAppmesh/VirtualNode/logging (35.65s)
        --- PASS: TestAccAWSAppmesh/VirtualNode/basic (21.37s)
    --- PASS: TestAccAWSAppmesh/VirtualRouter (35.68s)
        --- PASS: TestAccAWSAppmesh/VirtualRouter/basic (35.68s)
    --- PASS: TestAccAWSAppmesh/VirtualService (77.54s)
        --- PASS: TestAccAWSAppmesh/VirtualService/virtualNode (39.38s)
        --- PASS: TestAccAWSAppmesh/VirtualService/virtualRouter (38.17s)
    --- PASS: TestAccAWSAppmesh/Mesh (60.29s)
        --- PASS: TestAccAWSAppmesh/Mesh/basic (19.52s)
        --- PASS: TestAccAWSAppmesh/Mesh/egressFilter (40.77s)
    --- PASS: TestAccAWSAppmesh/Route (80.85s)
        --- PASS: TestAccAWSAppmesh/Route/httpRoute (41.01s)
        --- PASS: TestAccAWSAppmesh/Route/tcpRoute (39.84s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	458.405s

@ewbankkit ewbankkit force-pushed the appmesh-preview-07-2019 branch from ac09664 to 3d8b25a Compare July 25, 2019 22:22
@ewbankkit ewbankkit force-pushed the appmesh-preview-07-2019 branch from 3d8b25a to b34cf1c Compare July 26, 2019 18:19
@ewbankkit
Copy link
Contributor Author

ewbankkit commented Jul 26, 2019

New acceptance test:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSAppmesh/Route/httpHeader'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSAppmesh/Route/httpHeader -timeout 120m
=== RUN   TestAccAWSAppmesh
=== RUN   TestAccAWSAppmesh/Route
=== RUN   TestAccAWSAppmesh/Route/httpHeader
=== RUN   TestAccAWSAppmesh/VirtualRouter
--- PASS: TestAccAWSAppmesh (35.64s)
    --- PASS: TestAccAWSAppmesh/Route (35.64s)
        --- PASS: TestAccAWSAppmesh/Route/httpHeader (35.64s)
    --- PASS: TestAccAWSAppmesh/VirtualRouter (0.00s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	35.677s

@ewbankkit
Copy link
Contributor Author

Once the feature is released I'll open a new PR, cherry pick the relevant commits and close this PR.

@ewbankkit ewbankkit changed the title [WIP] App Mesh preview - HTTP Header based routing [WIP] App Mesh preview - HTTP header based routing, route priorities, cookie routing and retry policy Aug 2, 2019
@ewbankkit ewbankkit changed the title [WIP] App Mesh preview - HTTP header based routing, route priorities, cookie routing and retry policy [WIP] App Mesh preview - HTTP header based routing, route priorities, cookie based routing and retry policy Aug 2, 2019
@ewbankkit ewbankkit force-pushed the appmesh-preview-07-2019 branch from bf13e71 to 14a78aa Compare August 4, 2019 22:34
@ewbankkit
Copy link
Contributor Author

ewbankkit commented Aug 7, 2019

@ewbankkit ewbankkit force-pushed the appmesh-preview-07-2019 branch from 519378b to 50183eb Compare August 10, 2019 22:42
@ewbankkit
Copy link
Contributor Author

Even though it's in the current service model, TLS is not yet supported in the preview channel:

ForbiddenException: TLS Certificates from ACM are not supported.

Current acceptance tests for virtual nodes:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSAppmesh/VirtualNode'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSAppmesh/VirtualNode -timeout 120m
=== RUN   TestAccAWSAppmesh
=== RUN   TestAccAWSAppmesh/VirtualNode
=== RUN   TestAccAWSAppmesh/VirtualNode/logging
=== RUN   TestAccAWSAppmesh/VirtualNode/basic
=== RUN   TestAccAWSAppmesh/VirtualNode/cloudMapServiceDiscovery
=== RUN   TestAccAWSAppmesh/VirtualNode/listenerHealthChecks
--- PASS: TestAccAWSAppmesh (183.95s)
    --- PASS: TestAccAWSAppmesh/VirtualNode (183.95s)
        --- PASS: TestAccAWSAppmesh/VirtualNode/logging (30.98s)
        --- PASS: TestAccAWSAppmesh/VirtualNode/basic (18.60s)
        --- PASS: TestAccAWSAppmesh/VirtualNode/cloudMapServiceDiscovery (103.84s)
        --- PASS: TestAccAWSAppmesh/VirtualNode/listenerHealthChecks (30.52s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	183.982s

@ewbankkit
Copy link
Contributor Author

It looks like HTTP header and cookie based routing plus route priorities are available with AWS SDK v1.23.2.

Requires:

@ewbankkit
Copy link
Contributor Author

ewbankkit commented Sep 8, 2019

TLS support has been re-enabled in the preview channel: aws/aws-app-mesh-roadmap#39 (comment).
Documentation.
Walkthrough.

It looks like the HTTP Retry model has also changed recently: aws/aws-app-mesh-roadmap@ab21aee#diff-bed927c299bc696566dfae64bae275e2.
Changes are cosmetic.

@ewbankkit ewbankkit force-pushed the appmesh-preview-07-2019 branch from 50183eb to 8e9c081 Compare September 8, 2019 22:28
@ewbankkit
Copy link
Contributor Author

ewbankkit commented Sep 8, 2019

Current Virtual Node acceptance tests:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSAppmesh/VirtualNode'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSAppmesh/VirtualNode -timeout 120m
=== RUN   TestAccAWSAppmesh
=== RUN   TestAccAWSAppmesh/VirtualNode
=== RUN   TestAccAWSAppmesh/VirtualNode/listenerHealthChecks
=== RUN   TestAccAWSAppmesh/VirtualNode/listenerTls
=== RUN   TestAccAWSAppmesh/VirtualNode/logging
=== RUN   TestAccAWSAppmesh/VirtualNode/basic
=== RUN   TestAccAWSAppmesh/VirtualNode/cloudMapServiceDiscovery
--- FAIL: TestAccAWSAppmesh (193.21s)
    --- FAIL: TestAccAWSAppmesh/VirtualNode (193.21s)
        --- PASS: TestAccAWSAppmesh/VirtualNode/listenerHealthChecks (30.63s)
        --- FAIL: TestAccAWSAppmesh/VirtualNode/listenerTls (11.24s)
            testing.go:568: Step 0 error: errors during apply:
                
                Error: error creating App Mesh virtual node: BadRequestException: Certificate arn:aws:acm:us-west-2:000000000000:certificate/39b1275a-1386-42fc-aceb-f551c62ea3ce is invalid. App Mesh accepts only certificates issued by AWS Certificate Manager Private Certificate Authority.
                	status code: 400, request id: 3c122985-7954-4a4b-a312-0b50558ee0e5
                
                  on /tmp/tf-test210645761/main.tf line 33:
                  (source code not available)
                
                
        --- PASS: TestAccAWSAppmesh/VirtualNode/logging (30.03s)
        --- PASS: TestAccAWSAppmesh/VirtualNode/basic (18.40s)
        --- PASS: TestAccAWSAppmesh/VirtualNode/cloudMapServiceDiscovery (102.90s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	193.231s
GNUmakefile:20: recipe for target 'testacc' failed
make: *** [testacc] Error 1

Need to change the acceptance test to use the aws_acmpca_certificate_authority resource.

@ewbankkit
Copy link
Contributor Author

Retry policy bug fixed: aws/aws-app-mesh-roadmap#89.
Verify in production service: #10075.

@ewbankkit
Copy link
Contributor Author

@ewbankkit
Copy link
Contributor Author

http2/gRPC support is now available in AWS SDK v1.25.19.
Requires:

@bflad
Copy link
Contributor

bflad commented Oct 29, 2019

AWS Go SDK v1.25.21 has been merged if that helps

@ewbankkit
Copy link
Contributor Author

http2/gRPC support issue: #10803

@ewbankkit
Copy link
Contributor Author

Closing in favour of #11850.

@ewbankkit ewbankkit closed this Feb 2, 2020
@ewbankkit ewbankkit deleted the appmesh-preview-07-2019 branch February 2, 2020 13:18
@ghost
Copy link

ghost commented Mar 27, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies Used to indicate dependency changes. documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/appmesh Issues and PRs that pertain to the appmesh service. size/XXL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants