From 87f375524635dd45acce7d42a873b82ad8db7b5c Mon Sep 17 00:00:00 2001 From: Cory Hall <43035978+corymhall@users.noreply.github.com> Date: Thu, 14 Nov 2024 07:59:49 -0500 Subject: [PATCH] Create aws ccapi resources by using the SDK (#217) This PR updates our mapping to create resources using the SDK directly instead of creating a `pulumi.CustomResource`. By using a `pulumi.CustomResource` we were missing out on all the resource specific things from the generated SDK. (`replaceOnChanges`, required property validation, etc). closes #216 --- integration/examples_nodejs_test.go | 15 ++++++ integration/replace-on-changes/Pulumi.yaml | 3 ++ integration/replace-on-changes/index.ts | 24 +++++++++ integration/replace-on-changes/package.json | 15 ++++++ integration/replace-on-changes/step2/index.ts | 26 ++++++++++ integration/replace-on-changes/tsconfig.json | 18 +++++++ src/cfn-resource-mappings.ts | 18 +++---- src/interop.ts | 28 +---------- src/naming.ts | 2 +- tests/cfn-resource-mappings.test.ts | 50 ++++++++++++------- tests/converters/app-converter.test.ts | 4 ++ tests/utils.ts | 5 +- 12 files changed, 150 insertions(+), 58 deletions(-) create mode 100644 integration/replace-on-changes/Pulumi.yaml create mode 100644 integration/replace-on-changes/index.ts create mode 100644 integration/replace-on-changes/package.json create mode 100644 integration/replace-on-changes/step2/index.ts create mode 100644 integration/replace-on-changes/tsconfig.json diff --git a/integration/examples_nodejs_test.go b/integration/examples_nodejs_test.go index 2a40ec4c..8abf56b4 100644 --- a/integration/examples_nodejs_test.go +++ b/integration/examples_nodejs_test.go @@ -132,6 +132,21 @@ func TestErrors(t *testing.T) { assert.Containsf(t, buf.String(), "Error: Event Bus policy statements must have a sid", "Expected error message not found in pulumi up output") } +func TestReplaceOnChanges(t *testing.T) { + test := getJSBaseOptions(t). + With(integration.ProgramTestOptions{ + Dir: filepath.Join(getCwd(t), "replace-on-changes"), + EditDirs: []integration.EditDir{ + { + Dir: filepath.Join(getCwd(t), "replace-on-changes/step2"), + Additive: true, + }, + }, + }) + + integration.ProgramTest(t, &test) +} + func getJSBaseOptions(t *testing.T) integration.ProgramTestOptions { base := getBaseOptions(t) baseJS := base.With(integration.ProgramTestOptions{ diff --git a/integration/replace-on-changes/Pulumi.yaml b/integration/replace-on-changes/Pulumi.yaml new file mode 100644 index 00000000..433e4bed --- /dev/null +++ b/integration/replace-on-changes/Pulumi.yaml @@ -0,0 +1,3 @@ +name: pulumi-aws-replaceOnChanges +runtime: nodejs +description: replaceOnChanges integration test diff --git a/integration/replace-on-changes/index.ts b/integration/replace-on-changes/index.ts new file mode 100644 index 00000000..b3d44f85 --- /dev/null +++ b/integration/replace-on-changes/index.ts @@ -0,0 +1,24 @@ +import * as aws from '@pulumi/aws'; +import * as pulumicdk from '@pulumi/cdk'; +import * as ec2 from 'aws-cdk-lib/aws-ec2'; + +class ReplaceOnChangesStack extends pulumicdk.Stack { + constructor(app: pulumicdk.App, id: string, options?: pulumicdk.StackOptions) { + super(app, id, options); + const vpc = aws.ec2.getVpcOutput({ + default: true, + }).id; + const azs = aws.getAvailabilityZonesOutput({}).names; + new ec2.SecurityGroup(this, 'sg', { + description: 'Some Description', + vpc: ec2.Vpc.fromVpcAttributes(this, 'vpc', { + vpcId: pulumicdk.asString(vpc), + availabilityZones: pulumicdk.asList(azs), + }), + }); + } +} + +new pulumicdk.App('app', (scope: pulumicdk.App) => { + new ReplaceOnChangesStack(scope, 'teststack'); +}); diff --git a/integration/replace-on-changes/package.json b/integration/replace-on-changes/package.json new file mode 100644 index 00000000..888282a9 --- /dev/null +++ b/integration/replace-on-changes/package.json @@ -0,0 +1,15 @@ +{ + "name": "pulumi-aws-cdk", + "devDependencies": { + "@types/node": "^10.0.0" + }, + "dependencies": { + "@pulumi/aws": "^6.0.0", + "@pulumi/aws-native": "^1.5.0", + "@pulumi/cdk": "^0.5.0", + "@pulumi/pulumi": "^3.0.0", + "aws-cdk-lib": "2.149.0", + "constructs": "10.3.0", + "esbuild": "^0.24.0" + } +} diff --git a/integration/replace-on-changes/step2/index.ts b/integration/replace-on-changes/step2/index.ts new file mode 100644 index 00000000..9070bb4a --- /dev/null +++ b/integration/replace-on-changes/step2/index.ts @@ -0,0 +1,26 @@ +import * as aws from '@pulumi/aws'; +import * as pulumicdk from '@pulumi/cdk'; +import * as ec2 from 'aws-cdk-lib/aws-ec2'; + +class ReplaceOnChangesStack extends pulumicdk.Stack { + constructor(app: pulumicdk.App, id: string, options?: pulumicdk.StackOptions) { + super(app, id, options); + const vpc = aws.ec2.getVpcOutput({ + default: true, + }).id; + const azs = aws.getAvailabilityZonesOutput({}).names; + new ec2.SecurityGroup(this, 'sg', { + // description is a createOnlyProperty which means this would fail + // if `replaceOnChanges` was not working + description: 'Some New Description', + vpc: ec2.Vpc.fromVpcAttributes(this, 'vpc', { + vpcId: pulumicdk.asString(vpc), + availabilityZones: pulumicdk.asList(azs), + }), + }); + } +} + +new pulumicdk.App('app', (scope: pulumicdk.App) => { + new ReplaceOnChangesStack(scope, 'teststack'); +}); diff --git a/integration/replace-on-changes/tsconfig.json b/integration/replace-on-changes/tsconfig.json new file mode 100644 index 00000000..eac442cb --- /dev/null +++ b/integration/replace-on-changes/tsconfig.json @@ -0,0 +1,18 @@ +{ + "compilerOptions": { + "strict": true, + "outDir": "bin", + "target": "es2019", + "module": "commonjs", + "moduleResolution": "node", + "sourceMap": true, + "experimentalDecorators": true, + "pretty": true, + "noFallthroughCasesInSwitch": true, + "noImplicitReturns": true, + "forceConsistentCasingInFileNames": true + }, + "include": [ + "./*.ts" + ] +} diff --git a/src/cfn-resource-mappings.ts b/src/cfn-resource-mappings.ts index 6187cf6b..3618434e 100644 --- a/src/cfn-resource-mappings.ts +++ b/src/cfn-resource-mappings.ts @@ -14,11 +14,9 @@ import * as pulumi from '@pulumi/pulumi'; import * as aws from '@pulumi/aws-native'; -import { CfnResource, ResourceMapping, normalize } from './interop'; +import { ResourceMapping, normalize } from './interop'; import { debug } from '@pulumi/pulumi/log'; -import { toSdkName } from './naming'; -import { Metadata } from './pulumi-metadata'; -import { PulumiProvider } from './types'; +import { toSdkName, typeName as pulumiTypeName, moduleName } from './naming'; export function mapToCfnResource( logicalId: string, @@ -115,14 +113,10 @@ export function mapToCfnResource( } default: { - // When creating a generic `CfnResource` we don't have any information on the - // attributes attached to the resource. We need to populate them by looking up the - // `output` in the metadata - const metadata = new Metadata(PulumiProvider.AWS_NATIVE); - const resource = metadata.findResource(typeName); - const attributes = Object.keys(resource.outputs); - - return new CfnResource(logicalId, typeName, props, attributes, options); + const mName = moduleName(typeName).toLowerCase(); + const pType = pulumiTypeName(typeName); + const awsModule = aws as any; + return new awsModule[mName][pType](logicalId, props, options); } } } diff --git a/src/interop.ts b/src/interop.ts index 452e0da3..919fbe91 100644 --- a/src/interop.ts +++ b/src/interop.ts @@ -13,9 +13,8 @@ // limitations under the License. import * as pulumi from '@pulumi/pulumi'; -import { debug } from '@pulumi/pulumi/log'; import { normalizeObject } from './pulumi-metadata'; -import { toSdkName, typeToken } from './naming'; +import { toSdkName } from './naming'; import { PulumiProvider } from './types'; import { PulumiResourceType } from './graph'; @@ -84,31 +83,6 @@ export type ResourceAttributeMappingArray = (ResourceAttributeMapping & { logica export type ResourceMapping = ResourceAttributeMapping | pulumi.Resource | ResourceAttributeMappingArray; -export class CfnResource extends pulumi.CustomResource { - constructor( - name: string, - type: string, - properties: any, - attributes: string[], - opts?: pulumi.CustomResourceOptions, - ) { - const resourceName = typeToken(type); - - debug(`Constructing CfnResource ${name} of type ${resourceName} with attributes=${JSON.stringify(attributes)}`); - const propertiesDebugString = pulumi.output(properties).apply(JSON.stringify); - pulumi.interpolate`CfnResource ${name} input properties: ${propertiesDebugString}`.apply(debug); - - // Prepare an args bag with placeholders for output attributes. - const args: any = {}; - for (const k of attributes) { - args[k] = undefined; - } - Object.assign(args, properties); - - super(resourceName, name, args, opts); - } -} - export class CdkConstruct extends pulumi.ComponentResource { constructor(public readonly name: PulumiResourceType, type?: string, options?: pulumi.ComponentResourceOptions) { const constructType = type ?? 'Construct'; diff --git a/src/naming.ts b/src/naming.ts index 862cde34..0fb6431c 100644 --- a/src/naming.ts +++ b/src/naming.ts @@ -24,7 +24,7 @@ export function moduleName(resourceType: string): string { return lowerAcronyms(mName); } -function typeName(typ: string): string { +export function typeName(typ: string): string { const resourceTypeComponents = typ.split('::'); if (resourceTypeComponents.length !== 3) { throw new Error(`expected three parts in type ${resourceTypeComponents}`); diff --git a/tests/cfn-resource-mappings.test.ts b/tests/cfn-resource-mappings.test.ts index 3b935ad0..636270d4 100644 --- a/tests/cfn-resource-mappings.test.ts +++ b/tests/cfn-resource-mappings.test.ts @@ -1,6 +1,7 @@ import { CustomResource } from '@pulumi/pulumi'; import { mapToCfnResource } from '../src/cfn-resource-mappings'; import * as aws from '@pulumi/aws-native'; +import { moduleName, typeName } from '../src/naming'; class MockResource { constructor(args: { [key: string]: any }) { @@ -31,6 +32,27 @@ jest.mock('@pulumi/aws-native', () => { return {}; }), }, + apprunner: { + Service: jest.fn().mockImplementation(() => { + return {}; + }), + }, + ecs: { + Cluster: jest.fn().mockImplementation(() => { + return {}; + }), + TaskDefinition: jest.fn().mockImplementation(() => { + return {}; + }), + Service: jest.fn().mockImplementation(() => { + return {}; + }), + }, + ec2: { + Vpc: jest.fn().mockImplementation(() => { + return {}; + }), + }, iam: { Role: jest.fn().mockImplementation(() => { return {}; @@ -118,8 +140,7 @@ describe('Cfn Resource Mappings', () => { // WHEN mapToCfnResource(logicalId, cfnType, cfnProps, {}); // THEN - expect(CustomResource).toHaveBeenCalledWith( - 'aws-native:s3objectlambda:AccessPoint', + expect(aws.s3objectlambda.AccessPoint).toHaveBeenCalledWith( logicalId, { objectLambdaConfiguration: { @@ -154,8 +175,7 @@ describe('Cfn Resource Mappings', () => { // WHEN mapToCfnResource(logicalId, cfnType, cfnProps, {}); // THEN - expect(CustomResource).toHaveBeenCalledWith( - 'aws-native:lambda:Function', + expect(aws.lambda.Function).toHaveBeenCalledWith( logicalId, { environment: { @@ -207,8 +227,7 @@ describe('Cfn Resource Mappings', () => { // WHEN mapToCfnResource(logicalId, cfnType, cfnProps, {}); // THEN - expect(CustomResource).toHaveBeenCalledWith( - 'aws-native:iam:Role', + expect(aws.iam.Role).toHaveBeenCalledWith( logicalId, { description: 'desc', @@ -268,8 +287,7 @@ describe('Cfn Resource Mappings', () => { // WHEN mapToCfnResource(logicalId, cfnType, cfnProps, {}); // THEN - expect(CustomResource).toHaveBeenCalledWith( - 'aws-native:s3:AccessPoint', + expect(aws.s3.AccessPoint).toHaveBeenCalledWith( logicalId, { policy: { @@ -300,8 +318,7 @@ describe('Cfn Resource Mappings', () => { // WHEN mapToCfnResource(logicalId, cfnType, cfnProps, {}); // THEN - expect(CustomResource).toHaveBeenCalledWith( - 'aws-native:ec2:Vpc', + expect(aws.ec2.Vpc).toHaveBeenCalledWith( logicalId, { cidrBlock: '10.0.0.0/16', @@ -367,10 +384,10 @@ describe('Cfn Resource Mappings', () => { }); test.each([ - ['AWS::AppRunner::Service', 'aws-native:apprunner:Service'], - ['AWS::ECS::Cluster', 'aws-native:ecs:Cluster'], - ['AWS::ECS::TaskDefinition', 'aws-native:ecs:TaskDefinition'], - ])('successfully maps %p to %p', (cfnType, pulumiType) => { + ['AWS::AppRunner::Service', aws.apprunner.Service], + ['AWS::ECS::Cluster', aws.ecs.Cluster], + ['AWS::ECS::TaskDefinition', aws.ecs.TaskDefinition], + ])('successfully maps %p to %p', (cfnType, called) => { // GIVEN const logicalId = 'my-resource'; const cfnProps = {}; @@ -379,7 +396,7 @@ describe('Cfn Resource Mappings', () => { mapToCfnResource(logicalId, cfnType, cfnProps, {}); // THEN - expect(CustomResource).toHaveBeenCalledWith(pulumiType, logicalId, {}, {}); + expect(called).toHaveBeenCalledWith(logicalId, {}, {}); }); test('successfully maps ECS Service resource', () => { @@ -404,8 +421,7 @@ describe('Cfn Resource Mappings', () => { // WHEN mapToCfnResource(logicalId, cfnType, cfnProps, {}); // THEN - expect(CustomResource).toHaveBeenCalledWith( - 'aws-native:ecs:Service', + expect(aws.ecs.Service).toHaveBeenCalledWith( logicalId, { tags: [ diff --git a/tests/converters/app-converter.test.ts b/tests/converters/app-converter.test.ts index 9dc6596a..60faf817 100644 --- a/tests/converters/app-converter.test.ts +++ b/tests/converters/app-converter.test.ts @@ -114,6 +114,7 @@ describe('App Converter', () => { examplebucketPolicyE09B485E: { Type: 'AWS::S3::BucketPolicy', Properties: { + PolicyDocument: {}, Bucket: { Ref: 'examplebucketC9DFA43E', }, @@ -310,6 +311,7 @@ describe('Stack Converter', () => { other: { Type: 'AWS::EC2::Subnet', Properties: { + VpcId: { Ref: 'vpc' }, Ipv6CidrBlock: { 'Fn::Select': [0, { 'Fn::GetAtt': ['vpc', 'Ipv6CidrBlocks'] }] }, }, }, @@ -404,6 +406,7 @@ describe('Stack Converter', () => { other: { Type: 'AWS::EC2::Subnet', Properties: { + VpcId: { Ref: 'vpc' }, Ipv6CidrBlock: { 'Fn::Select': [0, { 'Fn::GetAtt': ['vpc', 'Ipv6CidrBlocks'] }] }, }, }, @@ -421,6 +424,7 @@ describe('Stack Converter', () => { other2: { Type: 'AWS::EC2::Subnet', Properties: { + VpcId: { Ref: 'vpc2' }, Ipv6CidrBlock: { 'Fn::Select': [0, { 'Fn::GetAtt': ['vpc2', 'Ipv6CidrBlocks'] }] }, }, }, diff --git a/tests/utils.ts b/tests/utils.ts index e760a99f..7168081b 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -42,7 +42,10 @@ export function createStackManifest( }, resource2: { Type: 'AWS::S3::BucketPolicy', - Properties: resource2Props, + Properties: { + policyDocument: {}, + ...resource2Props, + }, DependsOn: resource2DependsOn, }, },