Skip to content

Commit

Permalink
refactor: construct props struct normalization (#2321)
Browse files Browse the repository at this point in the history
Resolves #2268 

Added awslint rule:
- props-struct-name: verifies that all constructs have a corresponding "Props" struct.
- construct-ctor-prop-type: the "props" type in construct ctor uses the right type.
- construct-ctor-props-optional: if all props are optional, "props" should be optional

Exclusions:
- `construct-ctor-props-optional:@aws-cdk/aws-codebuild.ProjectProps`: a project with no source requires `buildSpec`, so we still have all optional props. We will revisit.

BREAKING CHANGE: the `cdk.Root` construct has been removed. Use `cdk.App` instead.
* In `stepfunctions.WaitProps`: the props `seconds`, `timestamp`, secondsPath` and `timestampPath` are now `duration` of a union-like class `WaitDuration` (e.g. `duration: WaitDuration.seconds(n)`)
* In `codedeploy.ServerDeploymentConfigProps`: the props `minHealthyHostCount` and `minHealthyHostPercentage` are now `minimumHealthyHosts` of union-like class `MinimumHealthyHosts` (e.g. `minimumHealthyHosts: MinimumHealthyHosts.percentage(50)`)
* In `cloudformation.CustomResourceProps`: the props `topicProvider` and `lambdaProvider` are now `provider` of union-like class `CustomResourceProvider` (e.g. `CustomResourceProvider.lambda(fn)`
  • Loading branch information
Elad Ben-Israel authored Apr 18, 2019
1 parent d94c1cc commit 3ff4f1f
Show file tree
Hide file tree
Showing 37 changed files with 4,559 additions and 200 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
*.tsbuildinfo

.vscode
# VSCode extension
/.favorites.json
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/assets-docker/lib/adopted-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export class AdoptedRepository extends ecr.RepositoryBase {

const adopter = new cfn.CustomResource(this, 'Resource', {
resourceType: 'Custom::ECRAdoptedRepository',
lambdaProvider: fn,
provider: cfn.CustomResourceProvider.lambda(fn),
properties: {
RepositoryName: props.repositoryName,
PolicyDocument: this.policyDocument
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/assets/lib/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export enum AssetPackaging {
File = 'file',
}

export interface GenericAssetProps {
export interface AssetProps {
/**
* The disk location of the asset.
*/
Expand Down Expand Up @@ -85,7 +85,7 @@ export class Asset extends cdk.Construct {
*/
private readonly s3Prefix: string;

constructor(scope: cdk.Construct, id: string, props: GenericAssetProps) {
constructor(scope: cdk.Construct, id: string, props: AssetProps) {
super(scope, id);

// stage the asset source (conditionally).
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/assets/lib/staging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import fs = require('fs');
import path = require('path');
import { copyDirectory, fingerprint } from './fs';

export interface StageProps {
export interface StagingProps {
readonly sourcePath: string;
}

Expand Down Expand Up @@ -49,7 +49,7 @@ export class Staging extends Construct {
*/
private _preparedAssetPath?: string;

constructor(scope: Construct, id: string, props: StageProps) {
constructor(scope: Construct, id: string, props: StagingProps) {
super(scope, id);

this.sourcePath = props.sourcePath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export class DnsValidatedCertificate extends cdk.Construct implements ICertifica
);

const certificate = new cfn.CustomResource(this, 'CertificateRequestorResource', {
lambdaProvider: requestorFunction,
provider: cfn.CustomResourceProvider.lambda(requestorFunction),
properties: {
DomainName: props.domainName,
SubjectAlternativeNames: props.subjectAlternativeNames,
Expand Down
34 changes: 20 additions & 14 deletions packages/@aws-cdk/aws-cloudformation/lib/custom-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,35 @@ import { CfnCustomResource } from './cloudformation.generated';
*/
export type Properties = {[key: string]: any};

/**
* Properties to provide a Lambda-backed custom resource
*/
export interface CustomResourceProps {
export class CustomResourceProvider {
/**
* The Lambda provider that implements this custom resource.
*
* We recommend using a lambda.SingletonFunction for this.
*
* Optional, exactly one of lamdaProvider or topicProvider must be set.
*/
readonly lambdaProvider?: lambda.IFunction;
public static lambda(handler: lambda.IFunction) { return new CustomResourceProvider(handler.functionArn); }

/**
* The SNS Topic for the provider that implements this custom resource.
*/
public static topic(topic: sns.ITopic) { return new CustomResourceProvider(topic.topicArn); }

private constructor(public readonly serviceToken: string) {

}
}

/**
* Properties to provide a Lambda-backed custom resource
*/
export interface CustomResourceProps {
/**
* The provider which implements the custom resource
*
* Optional, exactly one of lamdaProvider or topicProvider must be set.
* @example CustomResourceProvider.lambda(myFunction)
* @example CustomResourceProvider.topic(myTopic)
*/
readonly topicProvider?: sns.ITopic;
readonly provider: CustomResourceProvider;

/**
* Properties to pass to the Lambda
Expand Down Expand Up @@ -68,16 +78,12 @@ export class CustomResource extends Resource {
constructor(scope: Construct, id: string, props: CustomResourceProps) {
super(scope, id);

if (!!props.lambdaProvider === !!props.topicProvider) {
throw new Error('Exactly one of "lambdaProvider" or "topicProvider" must be set.');
}

const type = renderResourceType(props.resourceType);

this.resource = new CfnResource(this, 'Default', {
type,
properties: {
ServiceToken: props.lambdaProvider ? props.lambdaProvider.functionArn : props.topicProvider!.topicArn,
ServiceToken: props.provider.serviceToken,
...uppercaseProperties(props.properties || {})
}
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import lambda = require('@aws-cdk/aws-lambda');
import cdk = require('@aws-cdk/cdk');
import fs = require('fs');
import { CustomResource } from '../lib';
import { CustomResource, CustomResourceProvider } from '../lib';

interface DemoResourceProps {
/**
Expand All @@ -22,14 +22,14 @@ class DemoResource extends cdk.Construct {
super(scope, id);

const resource = new CustomResource(this, 'Resource', {
lambdaProvider: new lambda.SingletonFunction(this, 'Singleton', {
provider: CustomResourceProvider.lambda(new lambda.SingletonFunction(this, 'Singleton', {
uuid: 'f7d4f730-4ee1-11e8-9c2d-fa7ae01bbebc',
// This makes the demo only work as top-level TypeScript program, but that's fine for now
code: new lambda.InlineCode(fs.readFileSync('integ.trivial-lambda-provider.py', { encoding: 'utf-8' })),
handler: 'index.main',
timeout: 300,
runtime: lambda.Runtime.Python27,
}),
})),
properties: props
});

Expand Down
12 changes: 6 additions & 6 deletions packages/@aws-cdk/aws-cloudformation/test/test.resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import lambda = require('@aws-cdk/aws-lambda');
import sns = require('@aws-cdk/aws-sns');
import cdk = require('@aws-cdk/cdk');
import { Test } from 'nodeunit';
import { CustomResource } from '../lib';
import { CustomResource, CustomResourceProvider } from '../lib';

// tslint:disable:object-literal-key-quotes

Expand Down Expand Up @@ -91,7 +91,7 @@ export = {
const stack = new cdk.Stack();
new CustomResource(stack, 'MyCustomResource', {
resourceType: 'Custom::MyCustomResourceType',
topicProvider: new sns.Topic(stack, 'Provider')
provider: CustomResourceProvider.topic(new sns.Topic(stack, 'Provider'))
});
expect(stack).to(haveResource('Custom::MyCustomResourceType'));
test.done();
Expand All @@ -104,7 +104,7 @@ export = {
test.throws(() => {
new CustomResource(stack, 'MyCustomResource', {
resourceType: 'NoCustom::MyCustomResourceType',
topicProvider: new sns.Topic(stack, 'Provider')
provider: CustomResourceProvider.topic(new sns.Topic(stack, 'Provider'))
});
}, /Custom resource type must begin with "Custom::"/);

Expand All @@ -117,7 +117,7 @@ export = {
test.throws(() => {
new CustomResource(stack, 'MyCustomResource', {
resourceType: 'Custom::My Custom?ResourceType',
topicProvider: new sns.Topic(stack, 'Provider')
provider: CustomResourceProvider.topic(new sns.Topic(stack, 'Provider'))
});
}, /Custom resource type name can only include alphanumeric characters and/);

Expand All @@ -130,7 +130,7 @@ export = {
test.throws(() => {
new CustomResource(stack, 'MyCustomResource', {
resourceType: 'Custom::0123456789012345678901234567890123456789012345678901234567891',
topicProvider: new sns.Topic(stack, 'Provider')
provider: CustomResourceProvider.topic(new sns.Topic(stack, 'Provider'))
});
}, /Custom resource type length > 60/);

Expand All @@ -153,7 +153,7 @@ class TestCustomResource extends cdk.Construct {
});

new CustomResource(this, 'Resource', {
lambdaProvider: singletonLambda
provider: CustomResourceProvider.lambda(singletonLambda)
});
}
}
5 changes: 5 additions & 0 deletions packages/@aws-cdk/aws-codebuild/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,5 +101,10 @@
},
"engines": {
"node": ">= 8.10.0"
},
"awslint": {
"exclude": [
"construct-ctor-props-optional:@aws-cdk/aws-codebuild.Project"
]
}
}
66 changes: 35 additions & 31 deletions packages/@aws-cdk/aws-codedeploy/lib/server/deployment-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,38 @@ class DefaultServerDeploymentConfig implements IServerDeploymentConfig {
}
}

export class MinimumHealthyHosts {

/**
* The minimum healhty hosts threshold expressed as an absolute number.
*/
public static count(value: number): MinimumHealthyHosts {
return new MinimumHealthyHosts({
type: 'HOST_COUNT',
value
});
}

/**
* The minmum healhty hosts threshold expressed as a percentage of the fleet.
*/
public static percentage(value: number): MinimumHealthyHosts {
return new MinimumHealthyHosts({
type: 'FLEET_PERCENT',
value
});
}

private constructor(private readonly json: CfnDeploymentConfig.MinimumHealthyHostsProperty) { }

/**
* @internal
*/
public get _json() {
return this.json;
}
}

/**
* Construction properties of {@link ServerDeploymentConfig}.
*/
Expand All @@ -77,20 +109,9 @@ export interface ServerDeploymentConfigProps {
readonly deploymentConfigName?: string;

/**
* The minimum healhty hosts threshold expressed as an absolute number.
* If you've specified this value,
* you can't specify {@link #minHealthyHostPercentage},
* however one of this or {@link #minHealthyHostPercentage} is required.
*/
readonly minHealthyHostCount?: number;

/**
* The minmum healhty hosts threshold expressed as a percentage of the fleet.
* If you've specified this value,
* you can't specify {@link #minHealthyHostCount},
* however one of this or {@link #minHealthyHostCount} is required.
* Minimum number of healthy hosts.
*/
readonly minHealthyHostPercentage?: number;
readonly minimumHealthyHosts: MinimumHealthyHosts;
}

/**
Expand Down Expand Up @@ -121,7 +142,7 @@ export class ServerDeploymentConfig extends cdk.Construct implements IServerDepl

const resource = new CfnDeploymentConfig(this, 'Resource', {
deploymentConfigName: props.deploymentConfigName,
minimumHealthyHosts: this.minimumHealthyHosts(props),
minimumHealthyHosts: props.minimumHealthyHosts._json,
});

this.deploymentConfigName = resource.ref.toString();
Expand All @@ -138,21 +159,4 @@ export class ServerDeploymentConfig extends cdk.Construct implements IServerDepl
}).makeImportValue().toString(),
};
}

private minimumHealthyHosts(props: ServerDeploymentConfigProps):
CfnDeploymentConfig.MinimumHealthyHostsProperty {
if (props.minHealthyHostCount === undefined && props.minHealthyHostPercentage === undefined) {
throw new Error('At least one of minHealthyHostCount or minHealthyHostPercentage must be specified when creating ' +
'a custom Server DeploymentConfig');
}
if (props.minHealthyHostCount !== undefined && props.minHealthyHostPercentage !== undefined) {
throw new Error('Both minHealthyHostCount and minHealthyHostPercentage cannot be specified when creating ' +
'a custom Server DeploymentConfig');
}

return {
type: props.minHealthyHostCount !== undefined ? 'HOST_COUNT' : 'FLEET_PERCENT',
value: props.minHealthyHostCount !== undefined ? props.minHealthyHostCount : props.minHealthyHostPercentage!,
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,35 +7,11 @@ import codedeploy = require('../../lib');

export = {
'CodeDeploy DeploymentConfig': {
"cannot be created without specifying minHealthyHostCount or minHealthyHostPercentage"(test: Test) {
const stack = new cdk.Stack();

test.throws(() => {
new codedeploy.ServerDeploymentConfig(stack, 'DeploymentConfig', {
});
}, /minHealthyHost/i);

test.done();
},

"cannot be created specifying both minHealthyHostCount and minHealthyHostPercentage"(test: Test) {
const stack = new cdk.Stack();

test.throws(() => {
new codedeploy.ServerDeploymentConfig(stack, 'DeploymentConfig', {
minHealthyHostCount: 1,
minHealthyHostPercentage: 1,
});
}, /minHealthyHost/i);

test.done();
},

"can be created by specifying only minHealthyHostCount"(test: Test) {
const stack = new cdk.Stack();

new codedeploy.ServerDeploymentConfig(stack, 'DeploymentConfig', {
minHealthyHostCount: 1,
minimumHealthyHosts: codedeploy.MinimumHealthyHosts.count(1),
});

expect(stack).to(haveResource('AWS::CodeDeploy::DeploymentConfig', {
Expand All @@ -52,7 +28,7 @@ export = {
const stack = new cdk.Stack();

new codedeploy.ServerDeploymentConfig(stack, 'DeploymentConfig', {
minHealthyHostPercentage: 75,
minimumHealthyHosts: codedeploy.MinimumHealthyHosts.percentage(75),
});

expect(stack).to(haveResource('AWS::CodeDeploy::DeploymentConfig', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const application = new codedeploy.ServerApplication(stack, 'CodeDeployApplicati
});

const deploymentConfig = new codedeploy.ServerDeploymentConfig(stack, 'CustomDeployConfig', {
minHealthyHostCount: 0,
minimumHealthyHosts: codedeploy.MinimumHealthyHosts.count(0),
});

const deploymentGroup = new codedeploy.ServerDeploymentGroup(stack, 'CodeDeployGroup', {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cognito/lib/user-pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ export class UserPool extends Resource implements IUserPool {

private triggers: CfnUserPool.LambdaConfigProperty = { };

constructor(scope: Construct, id: string, props: UserPoolProps) {
constructor(scope: Construct, id: string, props: UserPoolProps = {}) {
super(scope, id);

let aliasAttributes: UserPoolAttribute[] | undefined;
Expand Down
Loading

0 comments on commit 3ff4f1f

Please sign in to comment.