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

(aws-appmesh): priority cannot be configured in gateway routes #16821

Closed
desoss opened this issue Oct 6, 2021 · 10 comments · Fixed by #17694
Closed

(aws-appmesh): priority cannot be configured in gateway routes #16821

desoss opened this issue Oct 6, 2021 · 10 comments · Fixed by #17694
Assignees
Labels
@aws-cdk/aws-appmesh Related to AWS App Mesh bug This issue is a bug. p1

Comments

@desoss
Copy link

desoss commented Oct 6, 2021

What is the problem?

Even if the AWS console allows to specify the gateway route priority in the gateway route configuration this is not possible in CDK.

Reproduction Steps

Here I've no way to specify the priority of a gateway route.

virtualGateway.addGatewayRoute('trial-route', {
    gatewayRouteName: 'trial-route',
    routeSpec: appmesh.GatewayRouteSpec.http({
        match: {
            path: HttpGatewayRoutePathMatch.startsWith('/'),
            rewriteRequestHostname: false,
        },
        routeTarget: props.targetService,
    })
});

What did you expect to happen?

I expect to be able to configure the gateway route priority as in the console:

image

What actually happened?

Currently, the priority cannot be configured in the gateway routes.

CDK CLI Version

1.126.0

Framework Version

No response

Node.js Version

16.4.1

OS

macOS

Language

Typescript

Language Version

No response

Other information

No response

@desoss desoss added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 6, 2021
@github-actions github-actions bot added the @aws-cdk/aws-appmesh Related to AWS App Mesh label Oct 6, 2021
@Seiya6329
Copy link
Contributor

@desoss - thank you very much for reporting the issue and I am very sorry for the inconvenience you are facing.

I am taking a looking a look at this issue we will respond by end of today with updates.

@peterwoodworth peterwoodworth removed the needs-triage This issue or PR still needs to be triaged. label Oct 6, 2021
@Seiya6329
Copy link
Contributor

Seiya6329 commented Oct 7, 2021

I am confirming CDK is currently not allowing Gateway Route to configure Priority.

I verified that the App Mesh console as well as AWS CLI does allow configuring Gateway Route with Priority.

However, CloudFormation GAtewayRouteSpec does not contain a field for Priority.

I am further getting confirmation if this is the bug/miss.

@Seiya6329
Copy link
Contributor

@desoss - We've verified Priority field is missing from CloudFormation spec and thus not showing in CDK as well.
We are working on fixing this issue and I will update this thread again once we identify estimated date of completion.

Meanwhile, as a workaround, please consider taking one of the below steps:

  • Create Gateway Route using CDK and configure Priority field using AWS console
  • Create Gateway Route using either AWS console or AWS CLI

Again, we deeply apologize for the inconvenience.

@Seiya6329 Seiya6329 added the p1 label Oct 7, 2021
@desoss
Copy link
Author

desoss commented Oct 11, 2021

Thanks, @Seiya6329 .
Is it possible to have an estimation on when the fix will be released?

So that we can decide if it's worth investing some time in an AWS CLI script or not.

@Seiya6329
Copy link
Contributor

@desoss - we are looking at 1.0 - 1.5 month to deploy all changes.

@Seiya6329
Copy link
Contributor

Quick update: We are deploying changes to CloudFormation to include Priority field.

I think we are looking at end of November as a target for full CDK update completion.

@Seiya6329
Copy link
Contributor

Seiya6329 commented Nov 16, 2021

Priority field is now available in CloudFormation. (link)
It should be becoming available for CDK L1 this week.

For CDK L2 support, we are still at end of November.

@Seiya6329
Copy link
Contributor

Submitted the PR for supporting Gateway Route Priority in CDK L2 construct: #17694

@mergify mergify bot closed this as completed in #17694 Dec 14, 2021
mergify bot pushed a commit that referenced this issue Dec 14, 2021

Verified

This commit was signed with the committer’s verified signature.
renovate-bot Mend Renovate
Adding the Gateway Route `Priority` support. This is not a new feature but it was missed from the implementation.

The implementation method is mimicking how Route's `Priority` is implemented: 
 - [route-spec.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-appmesh/lib/route-spec.ts)
 - [route.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-appmesh/lib/route.ts)

Fixes #16821

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@Seiya6329
Copy link
Contributor

@desoss - we have pushed the support with the gateway route priority. This feature should become available on 12/16 (Thu).

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Adding the Gateway Route `Priority` support. This is not a new feature but it was missed from the implementation.

The implementation method is mimicking how Route's `Priority` is implemented: 
 - [route-spec.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-appmesh/lib/route-spec.ts)
 - [route.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-appmesh/lib/route.ts)

Fixes aws#16821

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-appmesh Related to AWS App Mesh bug This issue is a bug. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants