Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): cross stack references to string lists break #22873

Merged
merged 38 commits into from Dec 8, 2022
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
5cce6a1
working linked app example, but still needs testing + edge cases
comcalvi Nov 7, 2022
dcc3465
tmp
comcalvi Nov 7, 2022
f177221
working intra-env
comcalvi Nov 8, 2022
c2364dd
integ test + ssm bugfix
comcalvi Nov 10, 2022
1491766
integ
comcalvi Nov 10, 2022
063590a
undo
comcalvi Nov 10, 2022
bc71472
edge case
comcalvi Nov 11, 2022
183d70c
changed return type of to any
comcalvi Nov 11, 2022
3b9fa84
remove breaking change
comcalvi Nov 14, 2022
9a055ce
docs
comcalvi Nov 14, 2022
e065be7
synta
comcalvi Nov 14, 2022
4129840
docs
comcalvi Nov 14, 2022
a7b9e1f
comments
comcalvi Nov 15, 2022
468cd82
leftover
comcalvi Nov 15, 2022
ecc6aab
remove Fn::Ref (it's not valid CFN)
comcalvi Nov 16, 2022
9e454c3
added test for name
comcalvi Nov 16, 2022
8b5239c
global const
comcalvi Nov 16, 2022
c6110c6
outputs will throw if given string lists
comcalvi Nov 16, 2022
a75181e
comment
comcalvi Nov 16, 2022
d31ff3c
extra comments
comcalvi Nov 16, 2022
d1796d2
stash
comcalvi Nov 28, 2022
6ea0622
type hints for tokens!
comcalvi Nov 29, 2022
1876530
build errors
comcalvi Nov 30, 2022
4666004
Merge branch 'main' into crossStack
comcalvi Nov 30, 2022
d4455bb
clean up
comcalvi Dec 1, 2022
51b86a3
test
comcalvi Dec 1, 2022
cd2f636
remove more unused type hints
comcalvi Dec 2, 2022
3d600db
clean up
comcalvi Dec 2, 2022
95914ff
rename
comcalvi Dec 2, 2022
8933739
remove from iam
comcalvi Dec 2, 2022
d324356
clean up
comcalvi Dec 3, 2022
e6ae434
comments
comcalvi Dec 3, 2022
2cfa06a
update genspec to use correct enum
comcalvi Dec 5, 2022
b571cd9
comment
comcalvi Dec 5, 2022
c1e1c25
update output error message
comcalvi Dec 5, 2022
caf451f
comments
comcalvi Dec 6, 2022
f23fbdb
Merge branch 'main' into crossStack
mergify[bot] Dec 7, 2022
8bef0bb
Merge branch 'main' into crossStack
TheRealAmazonKendra Dec 7, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,72 @@
import * as ssm from '@aws-cdk/aws-ssm';
import { App, CfnParameter, Stack, StackProps } from '@aws-cdk/core';
import { IntegTest } from '@aws-cdk/integ-tests';
import { Construct } from 'constructs';
import { InterfaceVpcEndpoint, InterfaceVpcEndpointAwsService, Vpc } from '../lib';

// GIVEN
const app = new App({
treeMetadata: false,
});

class ProducerStack extends Stack {
public stringListGetAtt: string[];
public stringListRef: CfnParameter;
public manualExport: string[];

constructor(scope: Construct, id: string, props?: StackProps) {
super(scope, id, props);

const vpc = new Vpc(this, 'vpc');
this.stringListGetAtt = new InterfaceVpcEndpoint(this, 'endpoint', {
vpc,
service: InterfaceVpcEndpointAwsService.SECRETS_MANAGER,
}).vpcEndpointDnsEntries;

this.stringListRef = new CfnParameter(this, 'stringListParam', {
default: 'BLAT,BLAH',
type: 'List<String>',
});

this.manualExport = this.exportListValue(['string1', 'string2'], {
name: 'ManualExport',
});
}
}

export interface consumerDeployProps extends StackProps {
stringListGetAtt: string[],
stringListRef: CfnParameter,
manualStringList: string[]
}

class ConsumerStack extends Stack {
constructor(scope: Construct, id: string, props: consumerDeployProps) {
super(scope, id, props);

new ssm.StringListParameter(this, 'GetAtt', {
stringListValue: props.stringListGetAtt,
});

new ssm.StringListParameter(this, 'Ref', {
stringListValue: props.stringListRef.valueAsList,
});

new ssm.StringListParameter(this, 'Manual', {
stringListValue: props.manualStringList,
});
}
}

const producer = new ProducerStack(app, 'producer');
const consumer = new ConsumerStack(app, 'consumer', {
stringListGetAtt: producer.stringListGetAtt,
stringListRef: producer.stringListRef,
manualStringList: producer.manualExport,
});

// THEN
new IntegTest(app, 'cross-region-references', {
testCases: [producer, consumer],
stackUpdateWorkflow: false,
});
63 changes: 62 additions & 1 deletion packages/@aws-cdk/aws-ec2/test/vpc.test.ts
@@ -1,6 +1,6 @@
import { Annotations, Match, Template } from '@aws-cdk/assertions';
import { testDeprecated } from '@aws-cdk/cdk-build-tools';
import { CfnOutput, CfnResource, Fn, Lazy, Stack, Tags } from '@aws-cdk/core';
import { App, CfnOutput, CfnResource, Fn, Lazy, Stack, Tags } from '@aws-cdk/core';
import {
AclCidr,
AclTraffic,
Expand All @@ -27,6 +27,7 @@ import {
TrafficDirection,
Vpc,
IpAddresses,
InterfaceVpcEndpointAwsService,
} from '../lib';

describe('vpc', () => {
Expand Down Expand Up @@ -2115,6 +2116,66 @@ describe('vpc', () => {
}
});
});

describe('can reference vpcEndpointDnsEntries across stacks', () => {
test('can reference an actual string list across stacks', () => {
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
const app = new App();
const stack1 = new Stack(app, 'Stack1');
const vpc = new Vpc(stack1, 'Vpc');
const endpoint = new InterfaceVpcEndpoint(stack1, 'interfaceVpcEndpoint', {
vpc,
service: InterfaceVpcEndpointAwsService.SECRETS_MANAGER,
});

const stack2 = new Stack(app, 'Stack2');
new CfnOutput(stack2, 'endpoint', {
value: Fn.select(0, endpoint.vpcEndpointDnsEntries),
});

const assembly = app.synth();
const template1 = assembly.getStackByName(stack1.stackName).template;
const template2 = assembly.getStackByName(stack2.stackName).template;

// THEN
expect(template1).toMatchObject({
Outputs: {
ExportsOutputFnGetAttinterfaceVpcEndpoint89C99945DnsEntriesB1872F7A: {
Value: {
'Fn::Join': [
'||', {
'Fn::GetAtt': [
'interfaceVpcEndpoint89C99945',
'DnsEntries',
],
},
],
},
Export: { Name: 'Stack1:ExportsOutputFnGetAttinterfaceVpcEndpoint89C99945DnsEntriesB1872F7A' },
},
},
});

expect(template2).toMatchObject({
Outputs: {
endpoint: {
Value: {
'Fn::Select': [
0,
{
'Fn::Split': [
'||',
{
'Fn::ImportValue': 'Stack1:ExportsOutputFnGetAttinterfaceVpcEndpoint89C99945DnsEntriesB1872F7A',
},
],
},
],
},
},
},
});
});
});
});

function getTestStack(): Stack {
Expand Down
4 changes: 3 additions & 1 deletion packages/@aws-cdk/aws-iam/lib/util.ts
@@ -1,4 +1,4 @@
import { captureStackTrace, DefaultTokenResolver, IPostProcessor, IResolvable, IResolveContext, Lazy, StringConcat, Token, Tokenization } from '@aws-cdk/core';
import { captureStackTrace, DefaultTokenResolver, IPostProcessor, IResolvable, IResolveContext, Lazy, ResolutionTypeHint, StringConcat, Token, Tokenization } from '@aws-cdk/core';
import { IConstruct } from 'constructs';
import { IPolicy } from './policy';

Expand Down Expand Up @@ -112,9 +112,11 @@ export class UniqueStringSet implements IResolvable, IPostProcessor {
}

public readonly creationStack: string[];
public readonly typeHint: ResolutionTypeHint;

private constructor(private readonly fn: () => string[]) {
this.creationStack = captureStackTrace();
this.typeHint = ResolutionTypeHint.LIST;
}

public resolve(context: IResolveContext) {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ssm/lib/parameter.ts
Expand Up @@ -729,7 +729,7 @@ export class StringListParameter extends ParameterBase implements IStringListPar
name: this.physicalName,
tier: props.tier,
type: ParameterType.STRING_LIST,
value: props.stringListValue.join(','),
value: Fn.join(',', props.stringListValue),
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
});
this.parameterName = this.getResourceNameAttribute(resource.ref);
this.parameterArn = arnForParameterName(this, this.parameterName, {
Expand Down
4 changes: 3 additions & 1 deletion packages/@aws-cdk/aws-stepfunctions/lib/private/json-path.ts
@@ -1,4 +1,4 @@
import { captureStackTrace, IResolvable, IResolveContext, Token, Tokenization } from '@aws-cdk/core';
import { captureStackTrace, IResolvable, IResolveContext, ResolutionTypeHint, Token, Tokenization } from '@aws-cdk/core';
import { IntrinsicParser, IntrinsicExpression } from './intrinstics';

const JSON_PATH_TOKEN_SYMBOL = Symbol.for('@aws-cdk/aws-stepfunctions.JsonPathToken');
Expand All @@ -9,10 +9,12 @@ export class JsonPathToken implements IResolvable {
}

public readonly creationStack: string[];
public readonly typeHint: ResolutionTypeHint;
public displayHint: string;

constructor(public readonly path: string) {
this.creationStack = captureStackTrace();
this.typeHint = ResolutionTypeHint.STRING;
this.displayHint = path.replace(/^[^a-zA-Z]+/, '');
Object.defineProperty(this, JSON_PATH_TOKEN_SYMBOL, { value: true });
}
Expand Down
7 changes: 7 additions & 0 deletions packages/@aws-cdk/core/lib/cfn-fn.ts
Expand Up @@ -6,6 +6,7 @@ import { IResolvable, IResolveContext } from './resolvable';
import { Stack } from './stack';
import { captureStackTrace } from './stack-trace';
import { Token } from './token';
import { ResolutionTypeHint } from './type-hints';

/* eslint-disable max-len */

Expand Down Expand Up @@ -810,6 +811,7 @@ class FnValueOfAll extends FnBase {
*/
class FnJoin implements IResolvable {
public readonly creationStack: string[];
public readonly typeHint: ResolutionTypeHint;

private readonly delimiter: string;
private readonly listOfValues: any[];
Expand All @@ -828,6 +830,7 @@ class FnJoin implements IResolvable {
this.delimiter = delimiter;
this.listOfValues = listOfValues;
this.creationStack = captureStackTrace();
this.typeHint = ResolutionTypeHint.STRING;
}

public resolve(context: IResolveContext): any {
Expand Down Expand Up @@ -867,12 +870,14 @@ class FnJoin implements IResolvable {
*/
class FnToJsonString implements IResolvable {
public readonly creationStack: string[];
public readonly typeHint: ResolutionTypeHint;

private readonly object: any;

constructor(object: any) {
this.object = object;
this.creationStack = captureStackTrace();
this.typeHint = ResolutionTypeHint.STRING;
}

public resolve(context: IResolveContext): any {
Expand All @@ -895,12 +900,14 @@ class FnToJsonString implements IResolvable {
*/
class FnLength implements IResolvable {
public readonly creationStack: string[];
public readonly typeHint: ResolutionTypeHint;

private readonly array: any;

constructor(array: any) {
this.array = array;
this.creationStack = captureStackTrace();
this.typeHint = ResolutionTypeHint.NUMBER;
}

public resolve(context: IResolveContext): any {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/core/lib/cfn-mapping.ts
Expand Up @@ -130,7 +130,7 @@ export class CfnMapping extends CfnRefElement {
class CfnMappingEmbedder implements IResolvable {
readonly creationStack: string[] = [];

constructor(private readonly cfnMapping: CfnMapping, readonly mapping: Mapping, private readonly key1: string, private readonly key2: string) { }
constructor(private readonly cfnMapping: CfnMapping, readonly mapping: Mapping, private readonly key1: string, private readonly key2: string) {}

public resolve(context: IResolveContext): string {
const consumingStack = Stack.of(context.scope);
Expand Down
4 changes: 4 additions & 0 deletions packages/@aws-cdk/core/lib/cfn-output.ts
Expand Up @@ -51,6 +51,10 @@ export class CfnOutput extends CfnElement {

if (props.value === undefined) {
throw new Error(`Missing value for CloudFormation output at path "${this.node.path}"`);
} else if (Array.isArray(props.value)) {
// `props.value` is a string, but because cross-stack exports allow passing any,
// we need to check for lists here.
throw new Error(`CloudFormation output was given a string list instead of a string or number at path "${this.node.path}"`);
}

this._description = props.description;
Expand Down
16 changes: 15 additions & 1 deletion packages/@aws-cdk/core/lib/cfn-parameter.ts
Expand Up @@ -3,6 +3,7 @@ import { CfnElement } from './cfn-element';
import { CfnReference } from './private/cfn-reference';
import { IResolvable, IResolveContext } from './resolvable';
import { Token } from './token';
import { ResolutionTypeHint } from './type-hints';

export interface CfnParameterProps {
/**
Expand Down Expand Up @@ -108,6 +109,7 @@ export class CfnParameter extends CfnElement {
private _minLength?: number;
private _minValue?: number;
private _noEcho?: boolean;
private typeHint: ResolutionTypeHint;

/**
* Creates a parameter construct.
Expand All @@ -131,6 +133,7 @@ export class CfnParameter extends CfnElement {
this._minLength = props.minLength;
this._minValue = props.minValue;
this._noEcho = props.noEcho;
this.typeHint = typeToTypeHint(this._type);
}

/**
Expand All @@ -144,6 +147,7 @@ export class CfnParameter extends CfnElement {

public set type(type: string) {
this._type = type;
this.typeHint = typeToTypeHint(this._type);
}

/**
Expand Down Expand Up @@ -282,7 +286,7 @@ export class CfnParameter extends CfnElement {
* The parameter value as a Token
*/
public get value(): IResolvable {
return CfnReference.for(this, 'Ref');
return CfnReference.for(this, 'Ref', undefined, this.typeHint);
}

/**
Expand Down Expand Up @@ -363,3 +367,13 @@ function isNumberType(type: string) {
function isStringType(type: string) {
return !isListType(type) && !isNumberType(type);
}

function typeToTypeHint(type: string): ResolutionTypeHint {
if (isListType(type)) {
return ResolutionTypeHint.LIST;
} else if (isNumberType(type)) {
return ResolutionTypeHint.NUMBER;
}

return ResolutionTypeHint.STRING;
}
5 changes: 3 additions & 2 deletions packages/@aws-cdk/core/lib/cfn-resource.ts
Expand Up @@ -15,6 +15,7 @@ import { TagManager } from './tag-manager';
import { Tokenization } from './token';
import { capitalizePropertyNames, ignoreEmpty, PostResolveToken } from './util';
import { FeatureFlags } from './feature-flags';
import { ResolutionTypeHint } from './type-hints';

export interface CfnResourceProps {
/**
Expand Down Expand Up @@ -172,8 +173,8 @@ export class CfnResource extends CfnRefElement {
* in case there is no generated attribute.
* @param attributeName The name of the attribute.
*/
public getAtt(attributeName: string): Reference {
return CfnReference.for(this, attributeName);
public getAtt(attributeName: string, typeHint?: ResolutionTypeHint): Reference {
return CfnReference.for(this, attributeName, undefined, typeHint);
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/core/lib/index.ts
Expand Up @@ -3,6 +3,7 @@ export * from './tag-aspect';

export * from './token';
export * from './resolvable';
export * from './type-hints';
export * from './lazy';
export * from './tag-manager';
export * from './string-fragments';
Expand Down