From 2b59ed1b54b5b83f22020ed5f2c4b77c6a1292f8 Mon Sep 17 00:00:00 2001 From: Jane Chen <125300057+chenjane-dev@users.noreply.github.com> Date: Wed, 17 Jan 2024 17:35:44 -0500 Subject: [PATCH] fix(appconfig): fromDeploymentStrategyId takes an enum-like class rather than a string (#28743) Previously, we were typing this as a `string` and providing an enum for `PredefinedDeploymentStrategyId`s. This is a CDK anti-pattern because this makes the enum undiscoverable, since users see that it is typed only as a `string`. It also may not work in non-TS languages. Instead, we are moving the type to explicitly be an enum-like class. Follow up from #28671. BREAKING CHANGE: `deploymentStrategyId` prop in `fromDeploymentStrategyId` now takes a `DeploymentStrategyId` rather than a `string`. To import a predefined deployment strategy id, use `DeploymentStrategyId.CANARY_10_PERCENT_20_MINUTES`. Otherwise, use `DeploymentStrategyId.fromString('abc123')`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-appconfig-alpha/README.md | 16 +++++++++ .../@aws-cdk/aws-appconfig-alpha/awslint.json | 4 +-- .../lib/deployment-strategy.ts | 34 ++++++++++++++----- .../test/deployment-strategy.test.ts | 10 +++--- 4 files changed, 48 insertions(+), 16 deletions(-) diff --git a/packages/@aws-cdk/aws-appconfig-alpha/README.md b/packages/@aws-cdk/aws-appconfig-alpha/README.md index d96b8dfc6a7f6..f7a658fdc5ab8 100644 --- a/packages/@aws-cdk/aws-appconfig-alpha/README.md +++ b/packages/@aws-cdk/aws-appconfig-alpha/README.md @@ -70,6 +70,22 @@ new appconfig.DeploymentStrategy(this, 'MyDeploymentStrategy', { }); ``` +Importing a deployment strategy by ID: + +```ts +appconfig.DeploymentStrategy.fromDeploymentStrategyId(this, 'MyImportedDeploymentStrategy', appconfig.DeploymentStrategyId.fromString('abc123')); +``` + +Importing an AWS AppConfig predefined deployment strategy by ID: + +```ts +appconfig.DeploymentStrategy.fromDeploymentStrategyId( + this, + 'MyImportedPredefinedDeploymentStrategy', + appconfig.DeploymentStrategyId.CANARY_10_PERCENT_20_MINUTES, +); +``` + ## Configuration A configuration is a higher-level construct that can either be a `HostedConfiguration` (stored internally through AWS diff --git a/packages/@aws-cdk/aws-appconfig-alpha/awslint.json b/packages/@aws-cdk/aws-appconfig-alpha/awslint.json index 4355a6e6a41b1..74fa11fd7c925 100644 --- a/packages/@aws-cdk/aws-appconfig-alpha/awslint.json +++ b/packages/@aws-cdk/aws-appconfig-alpha/awslint.json @@ -25,10 +25,10 @@ "docs-public-apis:@aws-cdk/aws-appconfig-alpha.IConfiguration", "docs-public-apis:@aws-cdk/aws-appconfig-alpha.IApplication", - "no-unused-type:@aws-cdk/aws-appconfig-alpha.PredefinedDeploymentStrategyId", "ref-via-interface:@aws-cdk/aws-appconfig-alpha.Application.addAgentToEcs.taskDef", "events-in-interface", "events-method-signature", - "events-generic" + "events-generic", + "from-signature:@aws-cdk/aws-appconfig-alpha.DeploymentStrategy.fromDeploymentStrategyId.params[2]" ] } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-appconfig-alpha/lib/deployment-strategy.ts b/packages/@aws-cdk/aws-appconfig-alpha/lib/deployment-strategy.ts index 0ebb40b59ac1a..83b3e9c2a88bd 100644 --- a/packages/@aws-cdk/aws-appconfig-alpha/lib/deployment-strategy.ts +++ b/packages/@aws-cdk/aws-appconfig-alpha/lib/deployment-strategy.ts @@ -66,16 +66,16 @@ export class DeploymentStrategy extends Resource implements IDeploymentStrategy * @param id The name of the deployment strategy construct * @param deploymentStrategyId The ID of the deployment strategy */ - public static fromDeploymentStrategyId(scope: Construct, id: string, deploymentStrategyId: string): IDeploymentStrategy { + public static fromDeploymentStrategyId(scope: Construct, id: string, deploymentStrategyId: DeploymentStrategyId): IDeploymentStrategy { const stack = Stack.of(scope); const deploymentStrategyArn = stack.formatArn({ service: 'appconfig', resource: 'deploymentstrategy', - resourceName: deploymentStrategyId, + resourceName: deploymentStrategyId.id, }); class Import extends Resource implements IDeploymentStrategy { - public readonly deploymentStrategyId = deploymentStrategyId; + public readonly deploymentStrategyId = deploymentStrategyId.id; public readonly deploymentStrategyArn = deploymentStrategyArn; } @@ -182,36 +182,52 @@ export enum GrowthType { } /** - * Defines the deployment strategy ID's of AWS AppConfig predefined strategies. + * Defines the deployment strategy ID's of AWS AppConfig deployment strategies. * * @see https://docs.aws.amazon.com/appconfig/latest/userguide/appconfig-creating-deployment-strategy.html */ -export enum PredefinedDeploymentStrategyId { +export abstract class DeploymentStrategyId { /** * **AWS Recommended**. This strategy processes the deployment exponentially using a 10% growth factor over 20 minutes. * AWS AppConfig recommends using this strategy for production deployments because it aligns with AWS best practices * for configuration deployments. */ - CANARY_10_PERCENT_20_MINUTES = 'AppConfig.Canary10Percent20Minutes', + public static readonly CANARY_10_PERCENT_20_MINUTES = DeploymentStrategyId.fromString('AppConfig.Canary10Percent20Minutes'); /** * **Testing/Demonstration**. This strategy deploys the configuration to half of all targets every 30 seconds for a * one-minute deployment. AWS AppConfig recommends using this strategy only for testing or demonstration purposes because * it has a short duration and bake time. */ - LINEAR_50_PERCENT_EVERY_30_SECONDS = 'AppConfig.Linear50PercentEvery30Seconds', + public static readonly LINEAR_50_PERCENT_EVERY_30_SECONDS = DeploymentStrategyId.fromString('AppConfig.Linear50PercentEvery30Seconds'); /** * **AWS Recommended**. This strategy deploys the configuration to 20% of all targets every six minutes for a 30 minute deployment. * AWS AppConfig recommends using this strategy for production deployments because it aligns with AWS best practices * for configuration deployments. */ - LINEAR_20_PERCENT_EVERY_6_MINUTES = 'AppConfig.Linear20PercentEvery6Minutes', + public static readonly LINEAR_20_PERCENT_EVERY_6_MINUTES = DeploymentStrategyId.fromString('AppConfig.Linear20PercentEvery6Minutes'); /** * **Quick**. This strategy deploys the configuration to all targets immediately. */ - ALL_AT_ONCE = 'AppConfig.AllAtOnce', + public static readonly ALL_AT_ONCE = DeploymentStrategyId.fromString('AppConfig.AllAtOnce'); + + /** + * Builds a deployment strategy ID from a string. + * + * @param deploymentStrategyId The deployment strategy ID. + */ + public static fromString(deploymentStrategyId: string): DeploymentStrategyId { + return { + id: deploymentStrategyId, + }; + } + + /** + * The deployment strategy ID. + */ + public abstract readonly id: string; } /** diff --git a/packages/@aws-cdk/aws-appconfig-alpha/test/deployment-strategy.test.ts b/packages/@aws-cdk/aws-appconfig-alpha/test/deployment-strategy.test.ts index d4076f587b697..2e0d51e1d966f 100644 --- a/packages/@aws-cdk/aws-appconfig-alpha/test/deployment-strategy.test.ts +++ b/packages/@aws-cdk/aws-appconfig-alpha/test/deployment-strategy.test.ts @@ -1,7 +1,7 @@ import * as cdk from 'aws-cdk-lib'; import { App } from 'aws-cdk-lib'; import { Template } from 'aws-cdk-lib/assertions'; -import { DeploymentStrategy, PredefinedDeploymentStrategyId, RolloutStrategy } from '../lib'; +import { DeploymentStrategy, DeploymentStrategyId, RolloutStrategy } from '../lib'; describe('deployment strategy', () => { test('default deployment strategy', () => { @@ -166,7 +166,7 @@ describe('deployment strategy', () => { account: '123456789012', }, }); - const deploymentStrategy = DeploymentStrategy.fromDeploymentStrategyId(stack, 'MyDeploymentStrategy', 'abc123'); + const deploymentStrategy = DeploymentStrategy.fromDeploymentStrategyId(stack, 'MyDeploymentStrategy', DeploymentStrategyId.fromString('abc123')); expect(deploymentStrategy.deploymentStrategyId).toEqual('abc123'); expect(deploymentStrategy.env.account).toEqual('123456789012'); @@ -181,7 +181,7 @@ describe('deployment strategy', () => { account: '123456789012', }, }); - const deploymentStrategy = DeploymentStrategy.fromDeploymentStrategyId(stack, 'MyDeploymentStrategy', PredefinedDeploymentStrategyId.ALL_AT_ONCE); + const deploymentStrategy = DeploymentStrategy.fromDeploymentStrategyId(stack, 'MyDeploymentStrategy', DeploymentStrategyId.ALL_AT_ONCE); expect(deploymentStrategy.deploymentStrategyId).toEqual('AppConfig.AllAtOnce'); expect(deploymentStrategy.env.account).toEqual('123456789012'); @@ -196,7 +196,7 @@ describe('deployment strategy', () => { account: '123456789012', }, }); - const deploymentStrategy = DeploymentStrategy.fromDeploymentStrategyId(stack, 'MyDeploymentStrategy', PredefinedDeploymentStrategyId.CANARY_10_PERCENT_20_MINUTES); + const deploymentStrategy = DeploymentStrategy.fromDeploymentStrategyId(stack, 'MyDeploymentStrategy', DeploymentStrategyId.CANARY_10_PERCENT_20_MINUTES); expect(deploymentStrategy.deploymentStrategyId).toEqual('AppConfig.Canary10Percent20Minutes'); expect(deploymentStrategy.env.account).toEqual('123456789012'); @@ -211,7 +211,7 @@ describe('deployment strategy', () => { account: '123456789012', }, }); - const deploymentStrategy = DeploymentStrategy.fromDeploymentStrategyId(stack, 'MyDeploymentStrategy', PredefinedDeploymentStrategyId.LINEAR_50_PERCENT_EVERY_30_SECONDS); + const deploymentStrategy = DeploymentStrategy.fromDeploymentStrategyId(stack, 'MyDeploymentStrategy', DeploymentStrategyId.LINEAR_50_PERCENT_EVERY_30_SECONDS); expect(deploymentStrategy.deploymentStrategyId).toEqual('AppConfig.Linear50PercentEvery30Seconds'); expect(deploymentStrategy.env.account).toEqual('123456789012');