Skip to content

Commit

Permalink
Revert "revert"
Browse files Browse the repository at this point in the history
This reverts commit 458e6b4.
  • Loading branch information
tmokmss committed May 7, 2023
1 parent 458e6b4 commit 905d5ed
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 93 deletions.
96 changes: 3 additions & 93 deletions packages/aws-cdk-lib/core/lib/cfn-resource.ts
Expand Up @@ -8,14 +8,13 @@ import { CfnCreationPolicy, CfnDeletionPolicy, CfnUpdatePolicy } from './cfn-res
import { Construct, IConstruct, Node } from 'constructs';
import { addDependency, obtainDependencies, removeDependency } from './deps';
import { CfnReference } from './private/cfn-reference';
import { CLOUDFORMATION_TOKEN_RESOLVER } from './private/cloudformation-lang';
import { Reference } from './reference';
import { RemovalPolicy, RemovalPolicyOptions } from './removal-policy';
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';
import { Intrinsic } from './private/intrinsic';

export interface CfnResourceProps {
/**
Expand Down Expand Up @@ -438,14 +437,7 @@ export class CfnResource extends CfnRefElement {
const hasDefined = Object.values(renderedProps).find(v => v !== undefined);
resourceDef.Properties = hasDefined !== undefined ? renderedProps : undefined;
}
const resolvedRawOverrides = Tokenization.resolve(this.rawOverrides, {
scope: this,
resolver: CLOUDFORMATION_TOKEN_RESOLVER,
// we need to preserve the empty elements here,
// as that's how removing overrides are represented as
removeEmpty: false,
});
return deepMerge(resourceDef, resolvedRawOverrides);
return deepMerge(resourceDef, this.rawOverrides);
}),
},
};
Expand Down Expand Up @@ -605,33 +597,6 @@ export interface ICfnResourceOptions {
metadata?: { [key: string]: any };
}

/**
* Object keys that deepMerge should not consider. Currently these include
* CloudFormation intrinsics
*
* @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html
*/

const MERGE_EXCLUDE_KEYS: string[] = [
'Ref',
'Fn::Base64',
'Fn::Cidr',
'Fn::FindInMap',
'Fn::GetAtt',
'Fn::GetAZs',
'Fn::ImportValue',
'Fn::Join',
'Fn::Select',
'Fn::Split',
'Fn::Sub',
'Fn::Transform',
'Fn::And',
'Fn::Equals',
'Fn::If',
'Fn::Not',
'Fn::Or',
];

/**
* Merges `source` into `target`, overriding any existing values.
* `null`s will cause a value to be deleted.
Expand All @@ -644,66 +609,11 @@ function deepMerge(target: any, ...sources: any[]) {

for (const key of Object.keys(source)) {
const value = source[key];
if (typeof(value) === 'object' && value != null && !Array.isArray(value)) {
if (typeof(value) === 'object' && value != null && !Array.isArray(value) && !(value instanceof Intrinsic)) {
// if the value at the target is not an object, override it with an
// object so we can continue the recursion
if (typeof(target[key]) !== 'object') {
target[key] = {};

/**
* If we have something that looks like:
*
* target: { Type: 'MyResourceType', Properties: { prop1: { Ref: 'Param' } } }
* sources: [ { Properties: { prop1: [ 'Fn::Join': ['-', 'hello', 'world'] ] } } ]
*
* Eventually we will get to the point where we have
*
* target: { prop1: { Ref: 'Param' } }
* sources: [ { prop1: { 'Fn::Join': ['-', 'hello', 'world'] } } ]
*
* We need to recurse 1 more time, but if we do we will end up with
* { prop1: { Ref: 'Param', 'Fn::Join': ['-', 'hello', 'world'] } }
* which is not what we want.
*
* Instead we check to see whether the `target` value (i.e. target.prop1)
* is an object that contains a key that we don't want to recurse on. If it does
* then we essentially drop it and end up with:
*
* { prop1: { 'Fn::Join': ['-', 'hello', 'world'] } }
*/
} else if (Object.keys(target[key]).length === 1) {
if (MERGE_EXCLUDE_KEYS.includes(Object.keys(target[key])[0])) {
target[key] = {};
}
}

/**
* There might also be the case where the source is an intrinsic
*
* target: {
* Type: 'MyResourceType',
* Properties: {
* prop1: { subprop: { name: { 'Fn::GetAtt': 'abc' } } }
* }
* }
* sources: [ {
* Properties: {
* prop1: { subprop: { 'Fn::If': ['SomeCondition', {...}, {...}] }}
* }
* } ]
*
* We end up in a place that is the reverse of the above check, the source
* becomes an intrinsic before the target
*
* target: { subprop: { name: { 'Fn::GetAtt': 'abc' } } }
* sources: [{
* 'Fn::If': [ 'MyCondition', {...}, {...} ]
* }]
*/
if (Object.keys(value).length === 1) {
if (MERGE_EXCLUDE_KEYS.includes(Object.keys(value)[0])) {
target[key] = {};
}
}

deepMerge(target[key], value);
Expand Down
2 changes: 2 additions & 0 deletions packages/aws-cdk-lib/core/lib/resolvable.ts
Expand Up @@ -158,6 +158,8 @@ export class DefaultTokenResolver implements ITokenResolver {
// The token might have returned more values that need resolving, recurse
resolved = context.resolve(resolved);
resolved = postProcessor.postProcess(resolved, context);
// resolve again since postProcess might have added more tokens (e.g. overriding)
resolved = context.resolve(resolved);
return resolved;
} catch (e: any) {
let message = `Resolution error: ${e.message}.`;
Expand Down

0 comments on commit 905d5ed

Please sign in to comment.