Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
rix0rrr committed Jan 8, 2019
1 parent 996f998 commit e0ec521
Show file tree
Hide file tree
Showing 24 changed files with 103 additions and 111 deletions.
8 changes: 0 additions & 8 deletions packages/@aws-cdk/aws-apigateway/lib/deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,6 @@ class LatestDeploymentResource extends CfnDeployment {
}

return this.logicalIdToken.toString();

/*
if (!this.lazyLogicalId) {
throw new Error('This resource has a lazy logical ID which is calculated just before synthesis. Use a cdk.Token to evaluate');
}
return this.lazyLogicalId;
*/
}

/**
Expand Down
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-apigateway/lib/integrations/aws.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export interface AwsIntegrationProps {
* technology.
*/
export class AwsIntegration extends Integration {
private _anchor?: cdk.IConstruct;
private scope?: cdk.IConstruct;

constructor(props: AwsIntegrationProps) {
const backend = props.subdomain ? `${props.subdomain}.${props.service}` : props.service;
Expand All @@ -72,20 +72,20 @@ export class AwsIntegration extends Integration {
type,
integrationHttpMethod: 'POST',
uri: new cdk.Token(() => {
if (!this._anchor) { throw new Error('AwsIntegration must be used in API'); }
if (!this.scope) { throw new Error('AwsIntegration must be used in API'); }
return cdk.ArnUtils.fromComponents({
service: 'apigateway',
account: backend,
resource: apiType,
sep: '/',
resourceName: apiValue,
}, this._anchor);
}, this.scope);
}),
options: props.options,
});
}

public bind(_method: Method) {
this._anchor = _method;
public bind(method: Method) {
this.scope = method;
}
}
2 changes: 0 additions & 2 deletions packages/@aws-cdk/aws-apigateway/lib/restapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,6 @@ export enum EndpointType {
Private = 'PRIVATE'
}

export class RestApiUrl extends cdk.Token { }

class ImportedRestApi extends cdk.Construct implements IRestApi {
public restApiId: string;

Expand Down
28 changes: 14 additions & 14 deletions packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ class SingletonPolicy extends cdk.Construct {
this.statementFor({
actions: ['cloudformation:ExecuteChangeSet'],
conditions: { StringEquals: { 'cloudformation:ChangeSetName': props.changeSetName } },
}).addResource(stackArnFromProps(props, this));
}).addResource(this.stackArnFromProps(props));
}

public grantCreateReplaceChangeSet(props: { stackName: string, changeSetName: string, region?: string }): void {
Expand All @@ -422,7 +422,7 @@ class SingletonPolicy extends cdk.Construct {
'cloudformation:DescribeStacks',
],
conditions: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': props.changeSetName } },
}).addResource(stackArnFromProps(props, this));
}).addResource(this.stackArnFromProps(props));
}

public grantCreateUpdateStack(props: { stackName: string, replaceOnFailure?: boolean, region?: string }): void {
Expand All @@ -438,7 +438,7 @@ class SingletonPolicy extends cdk.Construct {
if (props.replaceOnFailure) {
actions.push('cloudformation:DeleteStack');
}
this.statementFor({ actions }).addResource(stackArnFromProps(props, this));
this.statementFor({ actions }).addResource(this.stackArnFromProps(props));
}

public grantDeleteStack(props: { stackName: string, region?: string }): void {
Expand All @@ -447,7 +447,7 @@ class SingletonPolicy extends cdk.Construct {
'cloudformation:DescribeStack*',
'cloudformation:DeleteStack',
]
}).addResource(stackArnFromProps(props, this));
}).addResource(this.stackArnFromProps(props));
}

public grantPassRole(role: iam.IRole): void {
Expand Down Expand Up @@ -485,20 +485,20 @@ class SingletonPolicy extends cdk.Construct {
}
}
}

private stackArnFromProps(props: { stackName: string, region?: string }): string {
return cdk.ArnUtils.fromComponents({
region: props.region,
service: 'cloudformation',
resource: 'stack',
resourceName: `${props.stackName}/*`
}, this);
}
}

interface StatementTemplate {
actions: string[];
conditions?: StatementCondition;
}

type StatementCondition = { [op: string]: { [attribute: string]: string } };

function stackArnFromProps(props: { stackName: string, region?: string }, anchor: cdk.IConstruct): string {
return cdk.ArnUtils.fromComponents({
region: props.region,
service: 'cloudformation',
resource: 'stack',
resourceName: `${props.stackName}/*`
}, anchor);
}
type StatementCondition = { [op: string]: { [attribute: string]: string } };
Original file line number Diff line number Diff line change
Expand Up @@ -271,12 +271,12 @@ function _isOrContains(entity: string | string[], value: string): boolean {
return false;
}

function _stackArn(stackName: string, anchor: cdk.IConstruct): string {
function _stackArn(stackName: string, scope: cdk.IConstruct): string {
return cdk.ArnUtils.fromComponents({
service: 'cloudformation',
resource: 'stack',
resourceName: `${stackName}/*`,
}, anchor);
}, scope);
}

class PipelineDouble extends cdk.Construct implements cpapi.IPipeline {
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-codedeploy/lib/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ export class ServerApplication extends cdk.Construct implements IServerApplicati
}
}

function applicationNameToArn(applicationName: string, anchor: cdk.IConstruct): string {
function applicationNameToArn(applicationName: string, scope: cdk.IConstruct): string {
return cdk.ArnUtils.fromComponents({
service: 'codedeploy',
resource: 'application',
resourceName: applicationName,
sep: ':',
}, anchor);
}, scope);
}
18 changes: 9 additions & 9 deletions packages/@aws-cdk/aws-codedeploy/lib/deployment-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { CfnDeploymentConfig } from './codedeploy.generated';
*/
export interface IServerDeploymentConfig {
readonly deploymentConfigName: string;
deploymentConfigArn(anchor: cdk.IConstruct): string;
deploymentConfigArn(scope: cdk.IConstruct): string;
export(): ServerDeploymentConfigImportProps;
}

Expand All @@ -37,8 +37,8 @@ class ImportedServerDeploymentConfig extends cdk.Construct implements IServerDep
this.deploymentConfigName = props.deploymentConfigName;
}

public deploymentConfigArn(anchor: cdk.IConstruct): string {
return arnForDeploymentConfigName(this.deploymentConfigName, anchor);
public deploymentConfigArn(scope: cdk.IConstruct): string {
return arnForDeploymentConfigName(this.deploymentConfigName, scope);
}

public export() {
Expand All @@ -53,8 +53,8 @@ class DefaultServerDeploymentConfig implements IServerDeploymentConfig {
this.deploymentConfigName = deploymentConfigName;
}

public deploymentConfigArn(anchor: cdk.IConstruct): string {
return arnForDeploymentConfigName(this.deploymentConfigName, anchor);
public deploymentConfigArn(scope: cdk.IConstruct): string {
return arnForDeploymentConfigName(this.deploymentConfigName, scope);
}

public export(): ServerDeploymentConfigImportProps {
Expand Down Expand Up @@ -126,8 +126,8 @@ export class ServerDeploymentConfig extends cdk.Construct implements IServerDepl
this.deploymentConfigName = resource.ref.toString();
}

public deploymentConfigArn(anchor: cdk.IConstruct): string {
return arnForDeploymentConfigName(this.deploymentConfigName, anchor);
public deploymentConfigArn(scope: cdk.IConstruct): string {
return arnForDeploymentConfigName(this.deploymentConfigName, scope);
}

public export(): ServerDeploymentConfigImportProps {
Expand Down Expand Up @@ -156,11 +156,11 @@ export class ServerDeploymentConfig extends cdk.Construct implements IServerDepl
}
}

function arnForDeploymentConfigName(name: string, anchor: cdk.IConstruct): string {
function arnForDeploymentConfigName(name: string, scope: cdk.IConstruct): string {
return cdk.ArnUtils.fromComponents({
service: 'codedeploy',
resource: 'deploymentconfig',
resourceName: name,
sep: ':',
}, anchor);
}, scope);
}
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-codedeploy/lib/deployment-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -560,11 +560,11 @@ export class ServerDeploymentGroup extends ServerDeploymentGroupBase {
}
}

function deploymentGroupNameToArn(applicationName: string, deploymentGroupName: string, anchor: cdk.IConstruct): string {
function deploymentGroupNameToArn(applicationName: string, deploymentGroupName: string, scope: cdk.IConstruct): string {
return cdk.ArnUtils.fromComponents({
service: 'codedeploy',
resource: 'deploymentgroup',
resourceName: `${applicationName}/${deploymentGroupName}`,
sep: ':',
}, anchor);
}, scope);
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function generateStackName(props: CrossRegionScaffoldStackProps): string {
}

function generateUniqueName(baseName: string, region: string, account: string,
toUpperCase: boolean, hashPartLen: number = 8): string {
toUpperCase: boolean, hashPartLen: e = 8): string {
const sha256 = crypto.createHash('sha256')
.update(baseName)
.update(region)
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ export class Pipeline extends cdk.Construct implements cpapi.IPipeline {
/**
* Get the number of Stages in this Pipeline.
*/
public get stageCount(): number {
public get stageCount(): e {

This comment has been minimized.

Copy link
@eladb

eladb Jan 8, 2019

Contributor

e?

return this.stages.length;
}

Expand Down Expand Up @@ -369,7 +369,7 @@ export class Pipeline extends cdk.Construct implements cpapi.IPipeline {
'Please provide it explicitly with the inputArtifact property.');
}

private calculateInsertIndexFromPlacement(placement: StagePlacement): number {
private calculateInsertIndexFromPlacement(placement: StagePlacement): e {
// check if at most one placement property was provided
const providedPlacementProps = ['rightBefore', 'justAfter', 'atIndex']
.filter((prop) => (placement as any)[prop] !== undefined);
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codepipeline/lib/stage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export interface StagePlacement {
* The maximum allowed value is {@link Pipeline#stageCount},
* which will insert the new Stage at the end of the Pipeline.
*/
readonly atIndex?: number;
readonly atIndex?: e;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codepipeline/test/test.action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export = {
},
};

function boundsValidationResult(numberOfArtifacts: number, min: number, max: number): string[] {
function boundsValidationResult(numberOfArtifacts: e, min: e, max: e): string[] {
const stack = new cdk.Stack();
const pipeline = new codepipeline.Pipeline(stack, 'pipeline');
const stage = new codepipeline.Stage(stack, 'stage', { pipeline });
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-ecr/lib/repository-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,12 @@ export abstract class RepositoryBase extends cdk.Construct implements IRepositor
* Returns an ECR ARN for a repository that resides in the same account/region
* as the current stack.
*/
public static arnForLocalRepository(repositoryName: string, anchor: cdk.IConstruct): string {
public static arnForLocalRepository(repositoryName: string, scope: cdk.IConstruct): string {
return cdk.ArnUtils.fromComponents({
service: 'ecr',
resource: 'repository',
resourceName: repositoryName
}, anchor);
}, scope);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-iam/lib/managed-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ export class AwsManagedPolicy {
/**
* The Arn of this managed policy
*/
public policyArn(anchor: cdk.IConstruct): string {
public policyArn(scope: cdk.IConstruct): string {
// the arn is in the form of - arn:aws:iam::aws:policy/<policyName>
return cdk.ArnUtils.fromComponents({
service: "iam",
region: "", // no region for managed policy
account: "aws", // the account for a managed policy is 'aws'
resource: "policy",
resourceName: this.managedPolicyName
}, anchor);
}, scope);
}
}
20 changes: 10 additions & 10 deletions packages/@aws-cdk/aws-iam/lib/policy-document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export class PolicyDocument extends cdk.Token {
super();
}

public resolve(_context: cdk.ResolveContext): any {
public scope(_context: cdk.ResolveContext): any {
if (this.isEmpty) {
return undefined;
}
Expand Down Expand Up @@ -81,8 +81,8 @@ export class ArnPrincipal extends PolicyPrincipal {
}

export class AccountPrincipal extends ArnPrincipal {
constructor(public readonly anchor: cdk.Construct, public readonly accountId: any) {
super(`arn:${new cdk.Aws(anchor).partition}:iam::${accountId}:root`);
constructor(public readonly scope: cdk.Construct, public readonly accountId: any) {
super(`arn:${new cdk.Aws(scope).partition}:iam::${accountId}:root`);
}
}

Expand Down Expand Up @@ -136,8 +136,8 @@ export class FederatedPrincipal extends PolicyPrincipal {
}

export class AccountRootPrincipal extends AccountPrincipal {
constructor(anchor: cdk.Construct) {
super(anchor, new cdk.Aws(anchor).accountId);
constructor(scope: cdk.Construct) {
super(scope, new cdk.Aws(scope).scope);
}
}

Expand Down Expand Up @@ -250,8 +250,8 @@ export class PolicyStatement extends cdk.Token {
return this.addPrincipal(new ArnPrincipal(arn));
}

public addAwsAccountPrincipal(anchor: cdk.Construct, accountId: string): this {
return this.addPrincipal(new AccountPrincipal(anchor, accountId));
public addAwsAccountPrincipal(scope: cdk.Construct, accountId: string): this {
return this.addPrincipal(new AccountPrincipal(scope, accountId));
}

public addArnPrincipal(arn: string): this {
Expand All @@ -266,8 +266,8 @@ export class PolicyStatement extends cdk.Token {
return this.addPrincipal(new FederatedPrincipal(federated, conditions));
}

public addAccountRootPrincipal(anchor: cdk.Construct): this {
return this.addPrincipal(new AccountRootPrincipal(anchor));
public addAccountRootPrincipal(scope: cdk.Construct): this {
return this.addPrincipal(new AccountRootPrincipal(scope));
}

public addCanonicalUserPrincipal(canonicalUserId: string): this {
Expand Down Expand Up @@ -372,7 +372,7 @@ export class PolicyStatement extends cdk.Token {
// Serialization
//

public resolve(_context: cdk.ResolveContext): any {
public scope(_context: cdk.ResolveContext): any {
return this.toJson();
}

Expand Down
12 changes: 6 additions & 6 deletions packages/@aws-cdk/cdk/lib/cloudformation/arn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ export class ArnUtils {
* arn:{partition}:{service}:{region}:{account}:{resource}{sep}}{resource-name}
*
* The required ARN pieces that are omitted will be taken from the stack that
* the 'anchor' is attached to. If all ARN pieces are supplied, the supplied anchor
* the 'scope' is attached to. If all ARN pieces are supplied, the supplied scope
* can be 'undefined'.
*/
public static fromComponents(components: ArnComponents, anchor: IConstruct | undefined): string {
public static fromComponents(components: ArnComponents, scope: IConstruct | undefined): string {
const partition = components.partition !== undefined ? components.partition : theStack('partition').partition;
const region = components.region !== undefined ? components.region : theStack('region').region;
const account = components.account !== undefined ? components.account : theStack('account').accountId;
Expand All @@ -45,13 +45,13 @@ export class ArnUtils {
return values.join('');

/**
* Return the anchored stack (so the caller can get an attribute from it), throw a descriptive error if we don't have an anchor
* Return the stack we're scoped to (so the caller can get an attribute from it), throw a descriptive error if we don't have a scope
*/
function theStack(attribute: string) {
if (!anchor) {
throw new Error(`Must provide anchor when using implicit ${attribute}`);
if (!scope) {
throw new Error(`Must provide scope when using implicit ${attribute}`);
}
return Stack.find(anchor);
return Stack.find(scope);

}
}
Expand Down
Loading

0 comments on commit e0ec521

Please sign in to comment.