Skip to content

Commit

Permalink
fix(appconfig): fromDeploymentStrategyId takes an enum-like class rat…
Browse files Browse the repository at this point in the history
…her 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*
  • Loading branch information
chenjane-dev authored Jan 17, 2024
1 parent 3960720 commit 2b59ed1
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 16 deletions.
16 changes: 16 additions & 0 deletions packages/@aws-cdk/aws-appconfig-alpha/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-appconfig-alpha/awslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -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]"
]
}
34 changes: 25 additions & 9 deletions packages/@aws-cdk/aws-appconfig-alpha/lib/deployment-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand Down

0 comments on commit 2b59ed1

Please sign in to comment.