aws_apigateway: remove auto-generated CfnOutput for RestApi

[garysassano]

Describe the feature

There's an inconsistency in how CDK handles deployments for API constructs in the two API Gateway modules:

• aws_apigateway (v1): When creating a RestApi, CDK automatically generates a CfnOutput during deployment. This behavior is unexpected.
• aws_apigatewayv2: When creating a HttpApi or WebSocketApi, no such output is generated.

Expectation

I expected the RestApi from aws_apigateway to function similarly to HttpApi and WebSocketApi from aws_apigatewayv2.

Proposed Solution

I propose aligning the behavior of all API constructs. Ideally, the RestApi construct from aws_apigateway module should not generate a CfnOutput by default, mirroring the behaviour of HttpApi and WebSocketApi constructs from aws_apigatewayv2 module. This would provide a more consistent development experience when using CDK with different API types.

Other Information

No response

Acknowledgements

• I may be able to implement this feature request
• This feature might incur a breaking change

CDK version used

2.77

Environment details (OS name and version, etc.)

Ubuntu 22.04 LTS

[pahud]

Making this a p2 feature request. Please help us prioritize by giving upvotes on this issue.

[peterwoodworth]

Why is this an antipattern / why doesn't it make sense?

[garysassano]

Because AWS documentation defines Outputs as an optional section, but then you enforce it in this specific case, thus making it not optional? And even if we consider that it's ok to enforce it, why doing it only for RestApi and not for HttpApi?

[peterwoodworth]

Our L2 constructs often do a lot of things that are optional when using CloudFormation, I don't think I understand the reasoning here. Are you hitting limits, do you have security concerns, is it just unwanted extra noise in the output?

HttpApi is still within one of our alpha modules, so if anything, I think we would standardize the alpha module to be the same as the stable module. But, they're different constructs in different packages, so they can be different anyway 🙂

[peterwoodworth]

Keep in mind, if we were to outright remove this CfnOutput it would more than likely break some customers who have come to rely on it. I think the most we would do here is provide an option to remove the output, which you can do now already by removing the resource from the construct tree

[garysassano]

It would be indeed nice to have a parameter called EndpointOutput (or different name) which you can set to True or False. Then you could set True as the default value to not disrupt existing configurations. And the same structure could be used for HttpApi as well so that there is a linear developer experience across the two different versions of API Gateway.

[kylelaker]

It is definitely surprising as an end user to have an Output added to the stack and atypical across the library. It was added very early on in #665. In #17239, there was work to remove it under a feature flag but that was changed to instead prefer a prop; unfortunately work ended. This was also back when feature flags weren't quite as numerous as they are today so perhaps a feature flag would be viable if this were to be attempted again.

There is a fairly limited set of places that create a CfnOutput.

git grep 'new CfnOutput' ./packages/aws-cdk-lib | grep -v test | grep -v '.md'

packages/aws-cdk-lib/aws-apigateway/lib/restapi.ts:606:      new CfnOutput(this, 'Endpoint', { exportName: props.endpointExportName, value: this.urlForPath() });
packages/aws-cdk-lib/aws-ec2/lib/bastion-host.ts:211:    new CfnOutput(this, 'BastionHostId', {
packages/aws-cdk-lib/aws-ec2/lib/client-vpn-endpoint.ts:370:      new CfnOutput(this, 'SelfServicePortalUrl', {
packages/aws-cdk-lib/aws-ecs-patterns/lib/base/application-multiple-target-groups-service-base.ts:451:        new CfnOutput(this, `LoadBalancerDNS${lb.node.id}`, { value: lb.loadBalancerDnsName });
packages/aws-cdk-lib/aws-ecs-patterns/lib/base/application-multiple-target-groups-service-base.ts:453:          new CfnOutput(this, `ServiceURL${lb.node.id}${protocol.toLowerCase()}`, { value: protocol.toLowerCase() + '://' + domainName });
packages/aws-cdk-lib/aws-ecs-patterns/lib/base/application-multiple-target-groups-service-base.ts:468:      new CfnOutput(this, 'LoadBalancerDNS', { value: this.loadBalancer.loadBalancerDnsName });
packages/aws-cdk-lib/aws-ecs-patterns/lib/base/application-multiple-target-groups-service-base.ts:469:      new CfnOutput(this, 'ServiceURL', { value: protocol.toLowerCase() + '://' + domainName });
packages/aws-cdk-lib/aws-ecs-patterns/lib/base/network-multiple-target-groups-service-base.ts:347:        new CfnOutput(this, `LoadBalancerDNS${lb.node.id}`, { value: lb.loadBalancerDnsName });
packages/aws-cdk-lib/aws-ecs-patterns/lib/base/network-multiple-target-groups-service-base.ts:357:      new CfnOutput(this, 'LoadBalancerDNS', { value: this.loadBalancer.loadBalancerDnsName });
packages/aws-cdk-lib/aws-ecs-patterns/lib/base/queue-processing-service-base.ts:313:      new CfnOutput(this, 'SQSDeadLetterQueue', { value: this.deadLetterQueue.queueName });
packages/aws-cdk-lib/aws-ecs-patterns/lib/base/queue-processing-service-base.ts:314:      new CfnOutput(this, 'SQSDeadLetterQueueArn', { value: this.deadLetterQueue.queueArn });
packages/aws-cdk-lib/aws-ecs-patterns/lib/base/queue-processing-service-base.ts:349:    new CfnOutput(this, 'SQSQueue', { value: this.sqsQueue.queueName });
packages/aws-cdk-lib/aws-ecs-patterns/lib/base/queue-processing-service-base.ts:350:    new CfnOutput(this, 'SQSQueueArn', { value: this.sqsQueue.queueArn });
packages/aws-cdk-lib/aws-eks/lib/cluster.ts:1127:      new CfnOutput(autoScalingGroup, 'InstanceRoleARN', {
packages/aws-cdk-lib/aws-eks/lib/cluster.ts:1587:      new CfnOutput(this, 'ClusterName', { value: this.clusterName });
packages/aws-cdk-lib/aws-eks/lib/cluster.ts:1601:      new CfnOutput(this, 'MastersRoleArn', { value: mastersRole.roleArn });
packages/aws-cdk-lib/aws-eks/lib/cluster.ts:1624:      new CfnOutput(this, 'ConfigCommand', { value: `${updateConfigCommandPrefix} ${postfix}` });
packages/aws-cdk-lib/aws-eks/lib/cluster.ts:1625:      new CfnOutput(this, 'GetTokenCommand', { value: `${getTokenCommandPrefix} ${postfix}` });
packages/aws-cdk-lib/core/lib/custom-resource-provider/cross-region-export-providers/export-writer-provider.ts:35: * new CfnOutput(stack1, 'Output', { value: 'someValue', exportName: 'someName' });
packages/aws-cdk-lib/core/lib/private/refs.ts:313:    output = new CfnOutput(producer, outputId, { value: Token.asString(reference) });
packages/aws-cdk-lib/core/lib/stack.ts:1150:      new CfnOutput(this, `Export${options.name}`, {
packages/aws-cdk-lib/core/lib/stack.ts:1161:      new CfnOutput(exportsScope, id, {
packages/aws-cdk-lib/core/lib/stack.ts:1199:      new CfnOutput(this, `Export${options.name}`, {
packages/aws-cdk-lib/core/lib/stack.ts:1210:      new CfnOutput(exportsScope, id, {

Primarily, this is EKS cluster and ECS patterns (and support code in stack.ts). Outside of that, it's EC2's bastion host and client VPN constructs and the RestApi.

Perhaps it'd be good for there to be an actual standard for this to make it less surprising when it happens (documented in CONTRIBUTING.md?). Personally, I expect a construct to not impact much outside itself and to alter things "higher" in the tree (there are still some arguments here like SingletonFunction and a few creations of Mappings).

Specifically in the case of RestApi though, it is possible to disable the default Execute API endpoint that the output emits (using disableExecuteApiEndpoint); so it at least shouldn't be created in that case.

[peterwoodworth]

Thanks for the feedback, and for the context @kylelaker. This is something we'd need a contributor to help us with since this is a p2 issue with workarounds available; we'd be happy to review a PR

Perhaps it'd be good for there to be an actual standard for this to make it less surprising when it happens (documented in CONTRIBUTING.md?)

I'm not sure we'd set any "standard" for this unless we opt to not use CfnOutputs at all when not required because...

Personally, I expect a construct to not impact much outside itself and to alter things "higher" in the tree (there are still some arguments here like SingletonFunction and a few creations of Mappings).

I tend to agree with this

[advdv]

Keep in mind, if we were to outright remove this CfnOutput it would more than likely break some customers who have come to rely on it. I think the most we would do here is provide an option to remove the output, which you can do now already by removing the resource from the construct tree

@peterwoodworth I was unable to get this workaround to work. What should the "childName" be? I've tried the name of the output but that doesn't work. I've also tried to iterate over all nodes using findAll() but it doesn't show the output, which makes me think it is not possible to remove it from the tree in this way.

In that case the priority for this might be higher because there is no workaround. I agree with the arguments of @kylelaker , it is akin to a function deep in your code modifying global state.

Besides, I think it is possible that a role/user HAS been biven permission to read a Cloudformation's outputs, but NOT the resources in the stack. In that case it could lead to unexpected permission escalation because all of a sudden this user can now see the gateway's endpoint.

[Goradux]

@advanderveer The tryRemoveChild() workaround worked well for me.

The docs are a bit unclear about what is a "name", but apparently the name is just Endpoint if you are accessing the RestApi node.

Example (Kotlin via Java CDK):

import software.amazon.awscdk.services.apigateway.RestApi
import software.amazon.awscdk.CfnOutput

val restApi = RestApi(this, "Api", RestApiProps.builder().restApiName("some-name").build())
restApi.node.tryRemoveChild("Endpoint")

// add your own output
CfnOutput(this, "ApiUrlOutput", CfnOutputProps.builder().key("api-url").value(restApi.url).build())

Btw, I figured the name out via simple iteration over the children nodes of RestApi:

for (child in restApi.node.children) {
    println(child)
    println(child.javaClass.name)
}