From f10eaa7974494ff89e3f575b49bdd1eb1cf5c21a Mon Sep 17 00:00:00 2001 From: corymhall <43035978+corymhall@users.noreply.github.com> Date: Mon, 18 Nov 2024 15:06:05 -0500 Subject: [PATCH 1/3] Enable `autoTrim` behavior by default This PR enables the aws-native `AutoNaming.autoTrim` feature by default. It does this by creating an aws-native provider by default if the user does not provide one. I've updated tests to remove the workarounds. **Alternatives** 1. We could require that the user configure this themselves. 2. Is there a way to set config values programatically? closes #62, closes #70 --- README.md | 28 ++++++++++++++++++++++++++++ examples/alb/index.ts | 7 +------ integration/apigateway/sfn-api.ts | 6 ------ integration/examples_nodejs_test.go | 2 -- src/cfn-resource-mappings.ts | 13 +------------ src/stack.ts | 20 +++++++++++++++++++- 6 files changed, 49 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index faf3b30f..a812723e 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,34 @@ Try the workshop at https://apprunnerworkshop.com Read the docs at https://docs.aws.amazon.com/apprunner ``` +## AWS Cloud Control AutoNaming Config + +Sometimes CDK constructs can create resource names that are too long for the +[AWS Cloud Control provider](https://www.pulumi.com/registry/packages/aws-native/). +When this happens you can configure the `autoTrim` feature to have the generated +names be automatically trimmed to fit within the name requirements. If you are +not configuring your own `aws-native` provider then this feature is enabled by +default. If you _are_ configuring your own `aws-native` provider then you will +have to enable this. + +```ts +const nativeProvider = new aws_native.Provider('cdk-native-provider', { + region: 'us-east-2', + autoNaming: { + autoTrim: true, + randomSuffixMinLength: 7, + }, +}); +const app = new pulumicdk.App('app', (scope: pulumicdk.App): pulumicdk.AppOutputs => { + const stack = new AppRunnerStack('teststack'); + return { + url: stack.url, + }; +}, { + providers: [ nativeProvider ], +}); +``` + ## Bootstrapping CDK has the concept of [bootstrapping](https://docs.aws.amazon.com/cdk/v2/guide/bootstrapping.html) diff --git a/examples/alb/index.ts b/examples/alb/index.ts index 22af2cbe..51922be6 100644 --- a/examples/alb/index.ts +++ b/examples/alb/index.ts @@ -3,7 +3,6 @@ import * as ec2 from 'aws-cdk-lib/aws-ec2'; import * as elbv2 from 'aws-cdk-lib/aws-elasticloadbalancingv2'; import * as pulumi from '@pulumi/pulumi'; import * as pulumicdk from '@pulumi/cdk'; -import { CfnOutput } from 'aws-cdk-lib'; class AlbStack extends pulumicdk.Stack { url: pulumi.Output; @@ -28,15 +27,11 @@ class AlbStack extends pulumicdk.Stack { port: 80, }); - const tg = listener.addTargets('Target', { + listener.addTargets('Target', { port: 80, targets: [asg], }); - // workaround for https://github.com/pulumi/pulumi-cdk/issues/62 - const cfnTargetGroup = tg.node.defaultChild as elbv2.CfnTargetGroup; - cfnTargetGroup.overrideLogicalId('LBListenerTG'); - listener.connections.allowDefaultPortFromAnyIpv4('Open to the world'); asg.scaleOnRequestCount('AModestLoad', { diff --git a/integration/apigateway/sfn-api.ts b/integration/apigateway/sfn-api.ts index 58ec2180..8af44cc4 100644 --- a/integration/apigateway/sfn-api.ts +++ b/integration/apigateway/sfn-api.ts @@ -1,4 +1,3 @@ -import * as iam from 'aws-cdk-lib/aws-iam'; import { Construct } from 'constructs'; import * as sfn from 'aws-cdk-lib/aws-stepfunctions'; import * as apigw from 'aws-cdk-lib/aws-apigateway'; @@ -15,13 +14,8 @@ export class SfnApi extends Construct { stateMachineType: sfn.StateMachineType.EXPRESS, }); - // TODO[pulumi/pulumi-cdk#62] The auto created role has too long name - const role = new iam.Role(this, 'StartRole', { - assumedBy: new iam.ServicePrincipal('apigateway.amazonaws.com'), - }); const api = new apigw.StepFunctionsRestApi(this, 'StepFunctionsRestApi', { deploy: false, - role, stateMachine: stateMachine, headers: true, path: false, diff --git a/integration/examples_nodejs_test.go b/integration/examples_nodejs_test.go index 5992ac88..77a8262d 100644 --- a/integration/examples_nodejs_test.go +++ b/integration/examples_nodejs_test.go @@ -148,8 +148,6 @@ func TestCustomResource(t *testing.T) { test := getJSBaseOptions(t). With(integration.ProgramTestOptions{ Dir: filepath.Join(getCwd(t), "custom-resource"), - // Workaround until TODO[pulumi/pulumi-aws-native#1816] is resolved. - Env: []string{"PULUMI_CDK_EXPERIMENTAL_MAX_NAME_LENGTH=56"}, ExtraRuntimeValidation: func(t *testing.T, stack integration.RuntimeValidationStackInfo) { t.Logf("Outputs: %v", stack.Outputs) url := stack.Outputs["websiteUrl"].(string) diff --git a/src/cfn-resource-mappings.ts b/src/cfn-resource-mappings.ts index 480135dc..b334615f 100644 --- a/src/cfn-resource-mappings.ts +++ b/src/cfn-resource-mappings.ts @@ -117,18 +117,7 @@ export function mapToCfnResource( const pType = pulumiTypeName(typeName); const awsModule = aws as any; - // Workaround until TODO[pulumi/pulumi-aws-native#1816] is resolved. - // Not expected to be exposed to users - let name = logicalId; - const maxNameLength = process.env.PULUMI_CDK_EXPERIMENTAL_MAX_NAME_LENGTH; - if (maxNameLength) { - const maxLength = parseInt(maxNameLength, 10); - if (name.length > maxLength) { - name = name.substring(0, maxLength); - } - } - - return new awsModule[mName][pType](name, props, options); + return new awsModule[mName][pType](logicalId, props, options); } } } diff --git a/src/stack.ts b/src/stack.ts index c6533dda..ea9542cc 100644 --- a/src/stack.ts +++ b/src/stack.ts @@ -19,6 +19,7 @@ import { PulumiSynthesizer, PulumiSynthesizerBase } from './synthesizer'; import { AwsCdkCli, ICloudAssemblyDirectoryProducer } from '@aws-cdk/cli-lib-alpha'; import { CdkConstruct } from './interop'; import { makeUniqueId } from './cdk-logical-id'; +import * as native from '@pulumi/aws-native'; export type AppOutputs = { [outputId: string]: pulumi.Output }; @@ -93,7 +94,24 @@ export class App private appProps?: cdk.AppProps; constructor(id: string, createFunc: (scope: App) => void | AppOutputs, props?: AppResourceOptions) { - super('cdk:index:App', id, props?.appOptions, props); + // This matches the logic found in aws-native. If all of these are undefined the provider + // will throw an error + const region = native.config.region ?? process.env.AWS_REGION ?? process.env.AWS_DEFAULT_REGION!; + // If the user has not provided any providers, we will create an aws-native one by default in order + // to enable the autoNaming feature. + const providers = props?.providers ?? [ + new native.Provider('cdk-aws-native', { + region: region as native.Region, + autoNaming: { + randomSuffixMinLength: 7, + autoTrim: true, + }, + }), + ]; + super('cdk:index:App', id, props?.appOptions, { + ...props, + providers, + }); this.appOptions = props?.appOptions; this.createFunc = createFunc; this.component = this; From 42113c6c2e9afa9ea162060cc9df8bf01f86926d Mon Sep 17 00:00:00 2001 From: corymhall <43035978+corymhall@users.noreply.github.com> Date: Mon, 18 Nov 2024 15:24:58 -0500 Subject: [PATCH 2/3] fixing tests --- tests/basic.test.ts | 7 +++++++ tests/cdk-resource.test.ts | 4 ++++ tests/synthesizer.test.ts | 20 ++++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/tests/basic.test.ts b/tests/basic.test.ts index 1970347d..28037c7e 100644 --- a/tests/basic.test.ts +++ b/tests/basic.test.ts @@ -20,6 +20,13 @@ import { Vpc } from 'aws-cdk-lib/aws-ec2'; import { aws_ssm } from 'aws-cdk-lib'; import { Construct } from 'constructs'; +beforeAll(() => { + process.env.AWS_REGION = 'us-east-2'; +}); +afterAll(() => { + process.env.AWS_REGION = undefined; +}); + describe('Basic tests', () => { setMocks(); test('Checking single resource registration', async () => { diff --git a/tests/cdk-resource.test.ts b/tests/cdk-resource.test.ts index 19d00cfa..c26ec912 100644 --- a/tests/cdk-resource.test.ts +++ b/tests/cdk-resource.test.ts @@ -13,9 +13,13 @@ import { Construct } from 'constructs'; describe('CDK Construct tests', () => { let resources: MockResourceArgs[] = []; beforeAll(() => { + process.env.AWS_REGION = 'us-east-2'; resources = []; setMocks(resources); }); + afterAll(() => { + process.env.AWS_REGION = undefined; + }); // DynamoDB table was previously mapped to the `aws` provider // otherwise this level of testing wouldn't be necessary. // We also don't need to do this type of testing for _every_ resource diff --git a/tests/synthesizer.test.ts b/tests/synthesizer.test.ts index 6fd6ca4c..4539068f 100644 --- a/tests/synthesizer.test.ts +++ b/tests/synthesizer.test.ts @@ -7,6 +7,13 @@ import { asNetworkMode, asPlatforms } from '../src/synthesizer'; import { NetworkMode, Platform as DockerPlatform } from '@pulumi/docker-build'; import { DockerImageAsset, Platform, NetworkMode as Network } from 'aws-cdk-lib/aws-ecr-assets'; +beforeAll(() => { + process.env.AWS_REGION = 'us-east-2'; +}); +afterAll(() => { + process.env.AWS_REGION = undefined; +}); + describe('Synthesizer File Assets', () => { test('no assets = no staging resources', async () => { const resources: MockResourceArgs[] = []; @@ -16,6 +23,19 @@ describe('Synthesizer File Assets', () => { new CfnBucket(scope, 'Bucket'); }); expect(resources).toEqual([ + expect.objectContaining({ + inputs: { + autoNaming: '{"randomSuffixMinLength":7,"autoTrim":true}', + region: 'us-east-2', + skipCredentialsValidation: 'true', + skipGetEc2Platforms: 'true', + skipMetadataApiCheck: 'true', + skipRegionValidation: 'true', + }, + name: 'cdk-aws-native', + provider: '', + type: 'pulumi:providers:aws-native', + }), expect.objectContaining({ name: 'staging-stack-project-stack', type: 'cdk:construct:StagingStack', From e10371334099cd15db0fa7c7b679691014918140 Mon Sep 17 00:00:00 2001 From: corymhall <43035978+corymhall@users.noreply.github.com> Date: Tue, 19 Nov 2024 08:38:59 -0500 Subject: [PATCH 3/3] Handle case where other providers are provided --- src/stack.ts | 42 +++++++----- tests/basic.test.ts | 159 +++++++++++++++++++++++++++++++++++++++++++- tests/mocks.ts | 29 +++++--- 3 files changed, 205 insertions(+), 25 deletions(-) diff --git a/src/stack.ts b/src/stack.ts index ea9542cc..260067b5 100644 --- a/src/stack.ts +++ b/src/stack.ts @@ -94,23 +94,9 @@ export class App private appProps?: cdk.AppProps; constructor(id: string, createFunc: (scope: App) => void | AppOutputs, props?: AppResourceOptions) { - // This matches the logic found in aws-native. If all of these are undefined the provider - // will throw an error - const region = native.config.region ?? process.env.AWS_REGION ?? process.env.AWS_DEFAULT_REGION!; - // If the user has not provided any providers, we will create an aws-native one by default in order - // to enable the autoNaming feature. - const providers = props?.providers ?? [ - new native.Provider('cdk-aws-native', { - region: region as native.Region, - autoNaming: { - randomSuffixMinLength: 7, - autoTrim: true, - }, - }), - ]; super('cdk:index:App', id, props?.appOptions, { ...props, - providers, + providers: createDefaultNativeProvider(props?.providers), }); this.appOptions = props?.appOptions; this.createFunc = createFunc; @@ -344,3 +330,29 @@ function generateAppId(): string { .replace(/[^a-z0-9-.]/g, '-') .slice(-17); } + +/** + * If the user has not provided the aws-native provider, we will create one by default in order + * to enable the autoNaming feature. + */ +function createDefaultNativeProvider( + providers?: pulumi.ProviderResource[] | Record, +): pulumi.ProviderResource[] { + // This matches the logic found in aws-native. If all of these are undefined the provider + // will throw an error + const region = native.config.region ?? process.env.AWS_REGION ?? process.env.AWS_DEFAULT_REGION!; + + const newProviders = providers && !Array.isArray(providers) ? Object.values(providers) : providers ?? []; + if (!newProviders.find((p) => native.Provider.isInstance(p))) { + newProviders.push( + new native.Provider('cdk-aws-native', { + region: region as native.Region, + autoNaming: { + randomSuffixMinLength: 7, + autoTrim: true, + }, + }), + ); + } + return newProviders; +} diff --git a/tests/basic.test.ts b/tests/basic.test.ts index 28037c7e..23e57c4a 100644 --- a/tests/basic.test.ts +++ b/tests/basic.test.ts @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. import * as pulumi from '@pulumi/pulumi'; +import * as aws from '@pulumi/aws'; +import * as native from '@pulumi/aws-native'; import * as s3 from 'aws-cdk-lib/aws-s3'; import * as output from '../src/output'; import { setMocks, testApp } from './mocks'; @@ -28,14 +30,15 @@ afterAll(() => { }); describe('Basic tests', () => { - setMocks(); test('Checking single resource registration', async () => { + setMocks(); await testApp((scope: Construct) => { new s3.Bucket(scope, 'MyFirstBucket', { versioned: true }); }); }); test('LoadBalancer dnsName attribute does not throw', async () => { + setMocks(); await testApp((scope: Construct) => { const vpc = new Vpc(scope, 'vpc'); const alb = new ApplicationLoadBalancer(scope, 'alb', { @@ -51,9 +54,163 @@ describe('Basic tests', () => { }); }); test('Supports Output', async () => { + setMocks(); const o = pulumi.output('the-bucket-name'); await testApp((scope: Construct) => { new s3.Bucket(scope, 'MyFirstBucket', { bucketName: output.asString(o) }); }); }); + + test('Creates native provider by default', async () => { + const resources: pulumi.runtime.MockResourceArgs[] = []; + setMocks(resources); + await testApp((scope: Construct) => { + new s3.Bucket(scope, 'MyFirstBucket', { versioned: true }); + }); + const providers = resources.filter((r) => r.type === 'pulumi:providers:aws-native'); + expect(providers).toHaveLength(1); + expect(providers[0]).toEqual( + expect.objectContaining({ + inputs: { + autoNaming: '{"randomSuffixMinLength":7,"autoTrim":true}', + region: 'us-east-2', + skipCredentialsValidation: 'true', + skipGetEc2Platforms: 'true', + skipMetadataApiCheck: 'true', + skipRegionValidation: 'true', + }, + name: 'cdk-aws-native', + provider: '', + type: 'pulumi:providers:aws-native', + }), + ); + }); + + test('Creates native provider when classic provided', async () => { + const resources: pulumi.runtime.MockResourceArgs[] = []; + setMocks(resources); + await testApp( + (scope: Construct) => { + new s3.Bucket(scope, 'MyFirstBucket', { versioned: true }); + }, + { + providers: [new aws.Provider('test-aws', {})], + }, + ); + const providers = resources.filter((r) => r.type === 'pulumi:providers:aws-native'); + expect(providers).toHaveLength(1); + expect(providers[0]).toEqual( + expect.objectContaining({ + inputs: { + autoNaming: '{"randomSuffixMinLength":7,"autoTrim":true}', + region: 'us-east-2', + skipCredentialsValidation: 'true', + skipGetEc2Platforms: 'true', + skipMetadataApiCheck: 'true', + skipRegionValidation: 'true', + }, + name: 'cdk-aws-native', + provider: '', + type: 'pulumi:providers:aws-native', + }), + ); + }); + + test('Creates native provider when classic provided object', async () => { + const resources: pulumi.runtime.MockResourceArgs[] = []; + setMocks(resources); + await testApp( + (scope: Construct) => { + new s3.Bucket(scope, 'MyFirstBucket', { versioned: true }); + }, + { + providers: { + aws: new aws.Provider('test-aws', {}), + }, + }, + ); + const providers = resources.filter((r) => r.type === 'pulumi:providers:aws-native'); + expect(providers).toHaveLength(1); + expect(providers[0]).toEqual( + expect.objectContaining({ + inputs: { + autoNaming: '{"randomSuffixMinLength":7,"autoTrim":true}', + region: 'us-east-2', + skipCredentialsValidation: 'true', + skipGetEc2Platforms: 'true', + skipMetadataApiCheck: 'true', + skipRegionValidation: 'true', + }, + name: 'cdk-aws-native', + provider: '', + type: 'pulumi:providers:aws-native', + }), + ); + }); + + test('does not create native provider when one is provided', async () => { + const resources: pulumi.runtime.MockResourceArgs[] = []; + setMocks(resources); + await testApp( + (scope: Construct) => { + new s3.Bucket(scope, 'MyFirstBucket', { versioned: true }); + }, + { + providers: [ + new native.Provider('test-native', { + region: 'us-west-2', + }), + ], + }, + ); + const providers = resources.filter((r) => r.type === 'pulumi:providers:aws-native'); + expect(providers).toHaveLength(1); + expect(providers[0]).toEqual( + expect.objectContaining({ + inputs: { + region: 'us-west-2', + skipCredentialsValidation: 'true', + skipGetEc2Platforms: 'true', + skipMetadataApiCheck: 'true', + skipRegionValidation: 'true', + }, + name: 'test-native', + provider: '', + type: 'pulumi:providers:aws-native', + }), + ); + }); + + test('does not create native provider when one is provided object', async () => { + const resources: pulumi.runtime.MockResourceArgs[] = []; + setMocks(resources); + await testApp( + (scope: Construct) => { + new s3.Bucket(scope, 'MyFirstBucket', { versioned: true }); + }, + { + providers: { + 'aws-native': new native.Provider('test-native', { + region: 'us-west-2', + }), + }, + }, + ); + const providers = resources.filter((r) => r.type === 'pulumi:providers:aws-native'); + expect(providers).toHaveLength(1); + expect(providers[0]).toEqual( + expect.objectContaining({ + inputs: { + region: 'us-west-2', + skipCredentialsValidation: 'true', + skipGetEc2Platforms: 'true', + skipMetadataApiCheck: 'true', + skipRegionValidation: 'true', + }, + name: 'test-native', + provider: '', + type: 'pulumi:providers:aws-native', + }), + ); + }); }); diff --git a/tests/mocks.ts b/tests/mocks.ts index 802867e6..c97a21dd 100644 --- a/tests/mocks.ts +++ b/tests/mocks.ts @@ -1,6 +1,13 @@ import * as pulumi from '@pulumi/pulumi'; import { CdkConstruct } from '../src/interop'; -import { Stack as CdkStack, DockerImageAssetLocation, DockerImageAssetSource, FileAssetLocation, FileAssetSource, ISynthesisSession } from 'aws-cdk-lib/core'; +import { + Stack as CdkStack, + DockerImageAssetLocation, + DockerImageAssetSource, + FileAssetLocation, + FileAssetSource, + ISynthesisSession, +} from 'aws-cdk-lib/core'; import { AppComponent, AppOptions } from '../src/types'; import { MockCallArgs, MockResourceArgs } from '@pulumi/pulumi/runtime'; import { Construct } from 'constructs'; @@ -28,7 +35,7 @@ export class MockAppComponent extends pulumi.ComponentResource implements AppCom } } -export async function testApp(fn: (scope: Construct) => void) { +export async function testApp(fn: (scope: Construct) => void, options?: pulumi.ComponentResourceOptions) { class TestStack extends Stack { constructor(app: App, id: string) { super(app, id, { @@ -48,9 +55,15 @@ export async function testApp(fn: (scope: Construct) => void) { } } - const app = new App('testapp', (scope: App) => { - new TestStack(scope, 'teststack'); - }); + const app = new App( + 'testapp', + (scope: App) => { + new TestStack(scope, 'teststack'); + }, + { + ...options, + }, + ); const converter = await app.converter; await Promise.all( Array.from(converter.stacks.values()).flatMap((stackConverter) => { @@ -151,8 +164,8 @@ export function setMocks(resources?: MockResourceArgs[]) { ...args.inputs, id: args.inputs.logicalId + '_id', data: { - "DestinationBucketArn": `arn:aws:s3:::${args.inputs.bucketName}` - } + DestinationBucketArn: `arn:aws:s3:::${args.inputs.bucketName}`, + }, }, }; default: @@ -173,7 +186,6 @@ export function setMocks(resources?: MockResourceArgs[]) { } export class MockSynth extends PulumiSynthesizerBase { - constructor(readonly bucket: string, readonly prefix: string) { super(); } @@ -195,4 +207,3 @@ export class MockSynth extends PulumiSynthesizerBase { throw new Error('Method not implemented.'); } } -