feat(apigateway): feature flag to disable the default endpoint Output #17239

jamesorlakin · 2021-10-29T22:41:41Z

The RestApi construct creates a CfnOutput by default with no exportName. This seems slightly peculiar, and in stacks of my own I've often resorted to myApi.node.tryRemoveChild('Endpoint') to avoid confusion when the deployed APIs are private (and need accessing via a VPC endpoint instead of the normally public HTTPS URL).

It was contemplated in #3662 (comment) to remove this entirely. With CDK v2 providing an opportunity to allow breaking behaviours I've created a new feature flag to gradually introduce this new behaviour. I appreciate breaking changes are never desired, so I'm not disheartened if this is unwanted (feel free to close!).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…(under feature flag)

gitpod-io · 2021-10-29T22:41:44Z

…n test

…tion test

nija-at

Thanks for taking the effort to do this.
This is excellent!

Just a couple of small comments.

nija-at · 2021-11-05T16:50:24Z

packages/@aws-cdk/aws-apigateway/lib/restapi.ts

-      new CfnOutput(this, 'Endpoint', { exportName: props.endpointExportName, value: this.urlForPath() });
+      // If the context is true, don't create an unnamed output.
+      // If false - or the endpointExportName is set, do create a CfnOutput.
+      const avoidEmptyExport: boolean | undefined = Node.of(this).tryGetContext(APIGATEWAY_REST_API_NO_CFN_OUTPUT);


I believe you should use the FeatureFlags API - https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/core/lib/feature-flags.ts#L11 - instead.

Ooh, nice spot!

nija-at · 2021-11-05T16:51:19Z

packages/@aws-cdk/cx-api/lib/features.ts

@@ -160,6 +160,15 @@ export const LAMBDA_RECOGNIZE_VERSION_PROPS = '@aws-cdk/aws-lambda:recognizeVers
 */
 export const CLOUDFRONT_DEFAULT_SECURITY_POLICY_TLS_V1_2_2021 = '@aws-cdk/aws-cloudfront:defaultSecurityPolicyTLSv1.2_2021';

+/**
+ * API Gateway's REST API construct always creates a CfnOutput with a blank exportName. That name can be overridden


Suggested change

* API Gateway's REST API construct always creates a CfnOutput with a blank exportName. That name can be overridden

* API Gateway's RestApi construct always creates a CfnOutput with a blank exportName. That name can be overridden

Pull request has been modified.

aws-cdk-automation · 2021-11-08T11:30:55Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: d7f7887
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

nija-at

I'm curious about one thing - why did you pick the feature flag option, rather than just provide an option to disable this (enabled by default)?

jamesorlakin · 2021-11-09T18:26:18Z

That's an excellent question! I'll be honest and say I didn't think about doing that 🤦‍♂️

An extra boolean prop like avoidEndpointExport could work, then it could throw if provided a endpointExportName as well. While the default of having an output seems a little odd I guess it's not a breaking change by any stretch.

nija-at

@jamesorlakin - feels to me like the simpler solution than introducing a feature flag.

I can see you've put in a considerable amount of work to get the feature flag going, especially given that this is a tricky part of our code base, but would you consider switching up the solution?

jamesorlakin · 2021-11-12T10:06:37Z

Hey @nija-at, that's honestly fine and no worries - good thinking of a much simpler solution!

I'll close this one and maybe make something similar to the deploy prop (default true) when I get around to it.

olka-dev · 2022-02-25T14:58:19Z

@jamesorlakin you have not got around it in the end, haven`t you?...
You could have saved my day with this feature flag.
Unfortunately you were stopped before merging, @nija-at big thanks there... 🤬
Gotta have to go with node.findChild then.

jamesorlakin · 2022-03-02T22:05:56Z

...nope, it fell off my radar. You can always open a PR if you're that passionate about it. 👀

Otherwise a one-liner tryRemoveChild is far from the end of the world. Subclass with this behaviour if there's many RestApi instances and you don't want repetition in other constructs.

fix(aws-apigateway): no longer produce an empty CfnOutput by default …

c00cf8f

…(under feature flag)

github-actions bot added the @aws-cdk/aws-apigateway Related to Amazon API Gateway label Oct 29, 2021

github-actions bot assigned nija-at Oct 29, 2021

jamesorlakin added 4 commits October 30, 2021 10:08

fix(aws-route-53-targets): remove outputs from API Gateway integratio…

69bda9d

…n test

fix(aws-stepfunctions-tasks): remove outputs from API Gateway integra…

92090f3

…tion test

fix(decdk): Remove API Gateway output from Jest snapshot

6b4ac2e

Merge branch 'master' into refactor/api-gateway-no-default-cfnoutput

ba3c405

nija-at previously requested changes Nov 5, 2021

View reviewed changes

refactor(aws-apigateway): Use FeatureFlags API within restapi

7a042ca

chore(cx-api): Rename feature flag comment to RestApi

bf96867

jamesorlakin force-pushed the refactor/api-gateway-no-default-cfnoutput branch from 2796584 to bf96867 Compare November 8, 2021 09:38

Merge branch 'master' into refactor/api-gateway-no-default-cfnoutput

d7f7887

nija-at changed the title ~~fix(aws-apigateway): RestApi no longer creates a CfnOutput by default (under feature flag)~~ fix(apigateway): feature flag to disable the default endpoint Output Nov 9, 2021

nija-at changed the title ~~fix(apigateway): feature flag to disable the default endpoint Output~~ feat(apigateway): feature flag to disable the default endpoint Output Nov 9, 2021

nija-at reviewed Nov 9, 2021

View reviewed changes

nija-at suggested changes Nov 10, 2021

View reviewed changes

jamesorlakin closed this Nov 12, 2021

laurelmay mentioned this pull request May 3, 2023

aws_apigateway: remove auto-generated CfnOutput for RestApi #25373

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(apigateway): feature flag to disable the default endpoint Output #17239

feat(apigateway): feature flag to disable the default endpoint Output #17239

jamesorlakin commented Oct 29, 2021 •

edited by nija-at

Loading

gitpod-io bot commented Oct 29, 2021

nija-at left a comment

nija-at Nov 5, 2021

jamesorlakin Nov 8, 2021

nija-at Nov 5, 2021

aws-cdk-automation commented Nov 8, 2021

nija-at left a comment

jamesorlakin commented Nov 9, 2021

nija-at left a comment

jamesorlakin commented Nov 12, 2021

olka-dev commented Feb 25, 2022 •

edited

Loading

jamesorlakin commented Mar 2, 2022

	* API Gateway's REST API construct always creates a CfnOutput with a blank exportName. That name can be overridden
	* API Gateway's RestApi construct always creates a CfnOutput with a blank exportName. That name can be overridden

feat(apigateway): feature flag to disable the default endpoint Output #17239

feat(apigateway): feature flag to disable the default endpoint Output #17239

Conversation

jamesorlakin commented Oct 29, 2021 • edited by nija-at Loading

gitpod-io bot commented Oct 29, 2021

nija-at left a comment

Choose a reason for hiding this comment

nija-at Nov 5, 2021

Choose a reason for hiding this comment

jamesorlakin Nov 8, 2021

Choose a reason for hiding this comment

nija-at Nov 5, 2021

Choose a reason for hiding this comment

aws-cdk-automation commented Nov 8, 2021

AWS CodeBuild CI Report

nija-at left a comment

Choose a reason for hiding this comment

jamesorlakin commented Nov 9, 2021

nija-at left a comment

Choose a reason for hiding this comment

jamesorlakin commented Nov 12, 2021

olka-dev commented Feb 25, 2022 • edited Loading

jamesorlakin commented Mar 2, 2022

jamesorlakin commented Oct 29, 2021 •

edited by nija-at

Loading

olka-dev commented Feb 25, 2022 •

edited

Loading