From dde30916556e4b70606b0c0da59dfc1b897cbf02 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 8 Dec 2022 23:26:34 -0800 Subject: [PATCH] fix(ecr): grants for cross-account principals result in failed deployments (#16376) When performing grants in ECR's Repository class for principals from other accounts, we put the ARN of the principal inside the Resource Policy of the Repository. However, ECR validates that all principals included in its Policy exist at the time of deploying the Repository, so if this cross-account principal was not created before the Repository, its deployment would fail. Detect that situation in the Repository class, and trust the entiure account of the principal if this situation happens. This was spotted by a customer when using the `TagParameterContainerImage` class. Fixes #15070 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-ecr/jest.config.js | 1 + packages/@aws-cdk/aws-ecr/lib/repository.ts | 86 +++++++++-- .../@aws-cdk/aws-ecr/test/repository.test.ts | 30 ++++ .../tag-parameter-container-image.test.ts | 77 +++++----- packages/@aws-cdk/aws-iam/lib/group.ts | 2 +- packages/@aws-cdk/aws-iam/lib/index.ts | 1 + .../@aws-cdk/aws-iam/lib/managed-policy.ts | 2 +- .../@aws-cdk/aws-iam/lib/policy-statement.ts | 2 +- packages/@aws-cdk/aws-iam/lib/policy.ts | 2 +- packages/@aws-cdk/aws-iam/lib/principals.ts | 2 +- packages/@aws-cdk/aws-iam/lib/private/util.ts | 143 ++++++++++++++++++ packages/@aws-cdk/aws-iam/lib/role.ts | 2 +- packages/@aws-cdk/aws-iam/lib/user.ts | 2 +- packages/@aws-cdk/aws-iam/lib/utils.ts | 38 +++++ packages/@aws-cdk/aws-kms/lib/key.ts | 30 +--- 15 files changed, 341 insertions(+), 79 deletions(-) create mode 100644 packages/@aws-cdk/aws-iam/lib/private/util.ts create mode 100644 packages/@aws-cdk/aws-iam/lib/utils.ts diff --git a/packages/@aws-cdk/aws-ecr/jest.config.js b/packages/@aws-cdk/aws-ecr/jest.config.js index f4f042f9d2c29..09efb83e68e72 100644 --- a/packages/@aws-cdk/aws-ecr/jest.config.js +++ b/packages/@aws-cdk/aws-ecr/jest.config.js @@ -5,6 +5,7 @@ module.exports = { global: { ...baseConfig.coverageThreshold.global, branches: 70, + statements: 78, }, }, }; diff --git a/packages/@aws-cdk/aws-ecr/lib/repository.ts b/packages/@aws-cdk/aws-ecr/lib/repository.ts index 1955a3447e18b..d32f3430e3c62 100644 --- a/packages/@aws-cdk/aws-ecr/lib/repository.ts +++ b/packages/@aws-cdk/aws-ecr/lib/repository.ts @@ -2,7 +2,7 @@ import { EOL } from 'os'; import * as events from '@aws-cdk/aws-events'; import * as iam from '@aws-cdk/aws-iam'; import * as kms from '@aws-cdk/aws-kms'; -import { ArnFormat, IResource, Lazy, RemovalPolicy, Resource, Stack, Token } from '@aws-cdk/core'; +import { ArnFormat, IResource, Lazy, RemovalPolicy, Resource, Stack, Tags, Token, TokenComparison } from '@aws-cdk/core'; import { IConstruct, Construct } from 'constructs'; import { CfnRepository } from './ecr.generated'; import { LifecycleRule, TagStatus } from './lifecycle'; @@ -279,17 +279,49 @@ export abstract class RepositoryBase extends Resource implements IRepository { rule.addTarget(options.target); return rule; } + /** * Grant the given principal identity permissions to perform the actions on this repository */ public grant(grantee: iam.IGrantable, ...actions: string[]) { - return iam.Grant.addToPrincipalOrResource({ - grantee, - actions, - resourceArns: [this.repositoryArn], - resourceSelfArns: [], - resource: this, - }); + const crossAccountPrincipal = this.unsafeCrossAccountResourcePolicyPrincipal(grantee); + if (crossAccountPrincipal) { + // If the principal is from a different account, + // that means addToPrincipalOrResource() will update the Resource Policy of this repo to trust that principal. + // However, ECR verifies that the principal used in the Policy exists, + // and will error out if it doesn't. + // Because of that, if the principal is a newly created resource, + // and there is not a dependency relationship between the Stacks of this repo and the principal, + // trust the entire account of the principal instead + // (otherwise, deploying this repo will fail). + // To scope down the permissions as much as possible, + // only trust principals from that account with a specific tag + const crossAccountPrincipalStack = Stack.of(crossAccountPrincipal); + const roleTag = `${crossAccountPrincipalStack.stackName}_${crossAccountPrincipal.node.addr}`; + Tags.of(crossAccountPrincipal).add('aws-cdk:id', roleTag); + this.addToResourcePolicy(new iam.PolicyStatement({ + actions, + principals: [new iam.AccountPrincipal(crossAccountPrincipalStack.account)], + conditions: { + StringEquals: { 'aws:PrincipalTag/aws-cdk:id': roleTag }, + }, + })); + + return iam.Grant.addToPrincipal({ + grantee, + actions, + resourceArns: [this.repositoryArn], + scope: this, + }); + } else { + return iam.Grant.addToPrincipalOrResource({ + grantee, + actions, + resourceArns: [this.repositoryArn], + resourceSelfArns: [], + resource: this, + }); + } } /** @@ -319,6 +351,43 @@ export abstract class RepositoryBase extends Resource implements IRepository { 'ecr:UploadLayerPart', 'ecr:CompleteLayerUpload'); } + + /** + * Returns the resource that backs the given IAM grantee if we cannot put a direct reference + * to the grantee in the resource policy of this ECR repository, + * and 'undefined' in case we can. + */ + private unsafeCrossAccountResourcePolicyPrincipal(grantee: iam.IGrantable): IConstruct | undefined { + // A principal cannot be safely added to the Resource Policy of this ECR repository, if: + // 1. The principal is from a different account, and + // 2. The principal is a new resource (meaning, not just referenced), and + // 3. The Stack this repo belongs to doesn't depend on the Stack the principal belongs to. + + // condition #1 + const principal = grantee.grantPrincipal; + const principalAccount = principal.principalAccount; + if (!principalAccount) { + return undefined; + } + const repoAndPrincipalAccountCompare = Token.compareStrings(this.env.account, principalAccount); + if (repoAndPrincipalAccountCompare === TokenComparison.BOTH_UNRESOLVED || + repoAndPrincipalAccountCompare === TokenComparison.SAME) { + return undefined; + } + + // condition #2 + if (!iam.principalIsOwnedResource(principal)) { + return undefined; + } + + // condition #3 + const principalStack = Stack.of(principal); + if (this.stack.dependencies.includes(principalStack)) { + return undefined; + } + + return principal; + } } /** @@ -491,7 +560,6 @@ export class Repository extends RepositoryBase { }); } - private static validateRepositoryName(physicalName: string) { const repositoryName = physicalName; if (!repositoryName || Token.isUnresolved(repositoryName)) { diff --git a/packages/@aws-cdk/aws-ecr/test/repository.test.ts b/packages/@aws-cdk/aws-ecr/test/repository.test.ts index 432ab41a2e8c1..84ea9d1a9930f 100644 --- a/packages/@aws-cdk/aws-ecr/test/repository.test.ts +++ b/packages/@aws-cdk/aws-ecr/test/repository.test.ts @@ -444,6 +444,36 @@ describe('repository', () => { }).toThrow('encryptionKey is specified, so \'encryption\' must be set to KMS (value: AES256)'); }); + test('removal policy is "Retain" by default', () => { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + new ecr.Repository(stack, 'Repo'); + + // THEN + Template.fromStack(stack).hasResource('AWS::ECR::Repository', { + 'Type': 'AWS::ECR::Repository', + 'DeletionPolicy': 'Retain', + }); + }); + + test('"Delete" removal policy can be set explicitly', () => { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + new ecr.Repository(stack, 'Repo', { + removalPolicy: cdk.RemovalPolicy.DESTROY, + }); + + // THEN + Template.fromStack(stack).hasResource('AWS::ECR::Repository', { + 'Type': 'AWS::ECR::Repository', + 'DeletionPolicy': 'Delete', + }); + }); + describe('events', () => { test('onImagePushed without imageTag creates the correct event', () => { const stack = new cdk.Stack(); diff --git a/packages/@aws-cdk/aws-ecs/test/images/tag-parameter-container-image.test.ts b/packages/@aws-cdk/aws-ecs/test/images/tag-parameter-container-image.test.ts index 9ff3023b1e965..5f09864f5f3c7 100644 --- a/packages/@aws-cdk/aws-ecs/test/images/tag-parameter-container-image.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/images/tag-parameter-container-image.test.ts @@ -77,51 +77,52 @@ describe('tag parameter container image', () => { 'Fn::Join': ['', [ 'arn:', { Ref: 'AWS::Partition' }, - ':iam::service-account:role/servicestackionexecutionrolee7e2d9a783a54eb795f4', + ':iam::service-account:root', ]], }, }, + Condition: { + StringEquals: { + 'aws:PrincipalTag/aws-cdk:id': 'ServiceStack_c8a38b9d3ed0e8d960dd0d679c0bab1612dafa96f5', + }, + }, }], }, }); - Template.fromStack(serviceStack).hasResourceProperties('AWS::IAM::Role', { - RoleName: 'servicestackionexecutionrolee7e2d9a783a54eb795f4', + + Template.fromStack(serviceStack).hasResourceProperties('AWS::IAM::Policy', { + PolicyDocument: Match.objectLike({ + Statement: Match.arrayWith([ + Match.objectLike({ + Action: [ + 'ecr:BatchCheckLayerAvailability', + 'ecr:GetDownloadUrlForLayer', + 'ecr:BatchGetImage', + ], + Effect: 'Allow', + Resource: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + `:ecr:us-west-1:pipeline-account:repository/${repositoryName}`, + ]], + }, + }), + Match.objectLike({ + Action: 'ecr:GetAuthorizationToken', + Effect: 'Allow', + Resource: '*', + }), + ]), + }), }); - Template.fromStack(serviceStack).hasResourceProperties('AWS::ECS::TaskDefinition', { - ContainerDefinitions: [ - Match.objectLike({ - Image: { - 'Fn::Join': ['', [ - { - 'Fn::Select': [4, { - 'Fn::Split': [':', { - 'Fn::Join': ['', [ - 'arn:', - { Ref: 'AWS::Partition' }, - `:ecr:us-west-1:pipeline-account:repository/${repositoryName}`, - ]], - }], - }], - }, - '.dkr.ecr.', - { - 'Fn::Select': [3, { - 'Fn::Split': [':', { - 'Fn::Join': ['', [ - 'arn:', - { Ref: 'AWS::Partition' }, - `:ecr:us-west-1:pipeline-account:repository/${repositoryName}`, - ]], - }], - }], - }, - '.', - { Ref: 'AWS::URLSuffix' }, - `/${repositoryName}:`, - { Ref: 'ServiceTaskDefinitionContainerImageTagParamCEC9D0BA' }, - ]], - }, - }), + + Template.fromStack(serviceStack).hasResourceProperties('AWS::IAM::Role', { + Tags: [ + { + Key: 'aws-cdk:id', + Value: 'ServiceStack_c8a38b9d3ed0e8d960dd0d679c0bab1612dafa96f5', + }, ], }); }); diff --git a/packages/@aws-cdk/aws-iam/lib/group.ts b/packages/@aws-cdk/aws-iam/lib/group.ts index ea8784524c5c1..0b81c6c572158 100644 --- a/packages/@aws-cdk/aws-iam/lib/group.ts +++ b/packages/@aws-cdk/aws-iam/lib/group.ts @@ -6,8 +6,8 @@ import { IManagedPolicy } from './managed-policy'; import { Policy } from './policy'; import { PolicyStatement } from './policy-statement'; import { AddToPrincipalPolicyResult, ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals'; +import { AttachedPolicies } from './private/util'; import { IUser } from './user'; -import { AttachedPolicies } from './util'; /** * Represents an IAM Group. diff --git a/packages/@aws-cdk/aws-iam/lib/index.ts b/packages/@aws-cdk/aws-iam/lib/index.ts index 7b13245b49f39..bf402b8037830 100644 --- a/packages/@aws-cdk/aws-iam/lib/index.ts +++ b/packages/@aws-cdk/aws-iam/lib/index.ts @@ -14,6 +14,7 @@ export * from './oidc-provider'; export * from './permissions-boundary'; export * from './saml-provider'; export * from './access-key'; +export * from './utils'; // AWS::IAM CloudFormation Resources: export * from './iam.generated'; diff --git a/packages/@aws-cdk/aws-iam/lib/managed-policy.ts b/packages/@aws-cdk/aws-iam/lib/managed-policy.ts index 4aab13017d20b..0b6285cd56323 100644 --- a/packages/@aws-cdk/aws-iam/lib/managed-policy.ts +++ b/packages/@aws-cdk/aws-iam/lib/managed-policy.ts @@ -5,9 +5,9 @@ import { IGroup } from './group'; import { CfnManagedPolicy } from './iam.generated'; import { PolicyDocument } from './policy-document'; import { PolicyStatement } from './policy-statement'; +import { undefinedIfEmpty } from './private/util'; import { IRole } from './role'; import { IUser } from './user'; -import { undefinedIfEmpty } from './util'; /** * A managed policy diff --git a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts index 53eeede5a0e9b..ebba378b92e3a 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts @@ -6,7 +6,7 @@ import { FederatedPrincipal, IPrincipal, PrincipalBase, PrincipalPolicyFragment, ServicePrincipal, ServicePrincipalOpts, validateConditionObject, } from './principals'; import { normalizeStatement } from './private/postprocess-policy-document'; -import { LITERAL_STRING_KEY, mergePrincipal, sum } from './util'; +import { LITERAL_STRING_KEY, mergePrincipal, sum } from './private/util'; const ensureArrayOrUndefined = (field: any) => { if (field === undefined) { diff --git a/packages/@aws-cdk/aws-iam/lib/policy.ts b/packages/@aws-cdk/aws-iam/lib/policy.ts index 2de04f0990b73..f46afbc151ec3 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy.ts @@ -4,9 +4,9 @@ import { IGroup } from './group'; import { CfnPolicy } from './iam.generated'; import { PolicyDocument } from './policy-document'; import { PolicyStatement } from './policy-statement'; +import { generatePolicyName, undefinedIfEmpty } from './private/util'; import { IRole } from './role'; import { IUser } from './user'; -import { generatePolicyName, undefinedIfEmpty } from './util'; /** * Represents an IAM Policy diff --git a/packages/@aws-cdk/aws-iam/lib/principals.ts b/packages/@aws-cdk/aws-iam/lib/principals.ts index d782355902d04..bfdc8a2369644 100644 --- a/packages/@aws-cdk/aws-iam/lib/principals.ts +++ b/packages/@aws-cdk/aws-iam/lib/principals.ts @@ -6,8 +6,8 @@ import { IOpenIdConnectProvider } from './oidc-provider'; import { PolicyDocument } from './policy-document'; import { Condition, Conditions, PolicyStatement } from './policy-statement'; import { defaultAddPrincipalToAssumeRole } from './private/assume-role-policy'; +import { LITERAL_STRING_KEY, mergePrincipal } from './private/util'; import { ISamlProvider } from './saml-provider'; -import { LITERAL_STRING_KEY, mergePrincipal } from './util'; /** * Any object that has an associated principal that a permission can be granted to diff --git a/packages/@aws-cdk/aws-iam/lib/private/util.ts b/packages/@aws-cdk/aws-iam/lib/private/util.ts new file mode 100644 index 0000000000000..aa4bb0a1288a4 --- /dev/null +++ b/packages/@aws-cdk/aws-iam/lib/private/util.ts @@ -0,0 +1,143 @@ +import { captureStackTrace, DefaultTokenResolver, IPostProcessor, IResolvable, IResolveContext, Lazy, StringConcat, Token, Tokenization } from '@aws-cdk/core'; +import { IConstruct } from 'constructs'; +import { IPolicy } from '../policy'; + +const MAX_POLICY_NAME_LEN = 128; + +export const LITERAL_STRING_KEY = 'LiteralString'; + +export function undefinedIfEmpty(f: () => string[]): string[] { + return Lazy.list({ + produce: () => { + const array = f(); + return (array && array.length > 0) ? array : undefined; + }, + }); +} + +/** + * Used to generate a unique policy name based on the policy resource construct. + * The logical ID of the resource is a great candidate as long as it doesn't exceed + * 128 characters, so we take the last 128 characters (in order to make sure the hash + * is there). + */ +export function generatePolicyName(scope: IConstruct, logicalId: string): string { + // as logicalId is itself a Token, resolve it first + const resolvedLogicalId = Tokenization.resolve(logicalId, { + scope, + resolver: new DefaultTokenResolver(new StringConcat()), + }); + return lastNCharacters(resolvedLogicalId, MAX_POLICY_NAME_LEN); +} + +/** + * Returns a string composed of the last n characters of str. + * If str is shorter than n, returns str. + * + * @param str the string to return the last n characters of + * @param n how many characters to return + */ +function lastNCharacters(str: string, n: number) { + const startIndex = Math.max(str.length - n, 0); + return str.substring(startIndex, str.length); +} + +/** + * Helper class that maintains the set of attached policies for a principal. + */ +export class AttachedPolicies { + private policies = new Array(); + + /** + * Adds a policy to the list of attached policies. + * + * If this policy is already, attached, returns false. + * If there is another policy attached with the same name, throws an exception. + */ + public attach(policy: IPolicy) { + if (this.policies.find(p => p === policy)) { + return; // already attached + } + + if (this.policies.find(p => p.policyName === policy.policyName)) { + throw new Error(`A policy named "${policy.policyName}" is already attached`); + } + + this.policies.push(policy); + } +} + +/** + * Merge two dictionaries that represent IAM principals + * + * Does an in-place merge. + */ +export function mergePrincipal(target: { [key: string]: string[] }, source: { [key: string]: string[] }) { + // If one represents a literal string, the other one must be empty + const sourceKeys = Object.keys(source); + const targetKeys = Object.keys(target); + + if ((LITERAL_STRING_KEY in source && targetKeys.some(k => k !== LITERAL_STRING_KEY)) || + (LITERAL_STRING_KEY in target && sourceKeys.some(k => k !== LITERAL_STRING_KEY))) { + throw new Error(`Cannot merge principals ${JSON.stringify(target)} and ${JSON.stringify(source)}; if one uses a literal principal string the other one must be empty`); + } + + for (const key of sourceKeys) { + target[key] = target[key] ?? []; + + let value = source[key]; + if (!Array.isArray(value)) { + value = [value]; + } + + target[key].push(...value); + } + + return target; +} + +/** + * Lazy string set token that dedupes entries + * + * Needs to operate post-resolve, because the inputs could be + * `[ '${Token[TOKEN.9]}', '${Token[TOKEN.10]}', '${Token[TOKEN.20]}' ]`, which + * still all resolve to the same string value. + * + * Needs to JSON.stringify() results because strings could resolve to literal + * strings but could also resolve to `{ Fn::Join: [...] }`. + */ +export class UniqueStringSet implements IResolvable, IPostProcessor { + public static from(fn: () => string[]) { + return Token.asList(new UniqueStringSet(fn)); + } + + public readonly creationStack: string[]; + + private constructor(private readonly fn: () => string[]) { + this.creationStack = captureStackTrace(); + } + + public resolve(context: IResolveContext) { + context.registerPostProcessor(this); + return this.fn(); + } + + public postProcess(input: any, _context: IResolveContext) { + if (!Array.isArray(input)) { return input; } + if (input.length === 0) { return undefined; } + + const uniq: Record = {}; + for (const el of input) { + uniq[JSON.stringify(el)] = el; + } + return Object.values(uniq); + } + + public toString(): string { + return Token.asString(this); + } +} + +export function sum(xs: number[]) { + return xs.reduce((a, b) => a + b, 0); +} diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index c2c0f4c68598e..ff83e13e611ad 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -14,7 +14,7 @@ import { ImmutableRole } from './private/immutable-role'; import { ImportedRole } from './private/imported-role'; import { MutatingPolicyDocumentAdapter } from './private/policydoc-adapter'; import { PrecreatedRole } from './private/precreated-role'; -import { AttachedPolicies, UniqueStringSet } from './util'; +import { AttachedPolicies, UniqueStringSet } from './private/util'; const MAX_INLINE_SIZE = 10000; const MAX_MANAGEDPOL_SIZE = 6000; diff --git a/packages/@aws-cdk/aws-iam/lib/user.ts b/packages/@aws-cdk/aws-iam/lib/user.ts index ece9c206a8c56..7909fd3d92404 100644 --- a/packages/@aws-cdk/aws-iam/lib/user.ts +++ b/packages/@aws-cdk/aws-iam/lib/user.ts @@ -7,7 +7,7 @@ import { IManagedPolicy } from './managed-policy'; import { Policy } from './policy'; import { PolicyStatement } from './policy-statement'; import { AddToPrincipalPolicyResult, ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals'; -import { AttachedPolicies, undefinedIfEmpty } from './util'; +import { AttachedPolicies, undefinedIfEmpty } from './private/util'; /** * Represents an IAM user diff --git a/packages/@aws-cdk/aws-iam/lib/utils.ts b/packages/@aws-cdk/aws-iam/lib/utils.ts new file mode 100644 index 0000000000000..45ae5754c8fdc --- /dev/null +++ b/packages/@aws-cdk/aws-iam/lib/utils.ts @@ -0,0 +1,38 @@ +import { Resource } from '@aws-cdk/core'; +import { Construct, IConstruct } from 'constructs'; +import { IPrincipal } from './principals'; + +/** + * Determines whether the given Principal is a newly created resource managed by the CDK, + * or if it's a referenced existing resource. + * + * @param principal the Principal to check + * @returns true if the Principal is a newly created resource, false otherwise. + * Additionally, the type of the principal will now also be IConstruct + * (because a newly created resource must be a construct) + */ +export function principalIsOwnedResource(principal: IPrincipal): principal is IPrincipal & IConstruct { + // a newly created resource will for sure be a construct + if (!isConstruct(principal)) { + return false; + } + + return Resource.isOwnedResource(principal); +} + +/** + * Whether the given object is a Construct + * + * Normally we'd do `x instanceof Construct`, but that is not robust against + * multiple copies of the `constructs` library on disk. This can happen + * when upgrading and downgrading between v2 and v1, and in the use of CDK + * Pipelines is going to an error that says "Can't use Pipeline/Pipeline/Role in + * a cross-environment fashion", which is very confusing. + */ +function isConstruct(x: any): x is IConstruct { + const sym = Symbol.for('constructs.Construct.node'); + return (typeof x === 'object' && x && + (x instanceof Construct // happy fast case + || !!(x as any).node // constructs v10 + || !!(x as any)[sym])); // constructs v3 +} diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts index 0dc523fb73c37..2eb86115dd534 100644 --- a/packages/@aws-cdk/aws-kms/lib/key.ts +++ b/packages/@aws-cdk/aws-kms/lib/key.ts @@ -203,12 +203,9 @@ abstract class KeyBase extends Resource implements IKey { */ private granteeStackDependsOnKeyStack(grantee: iam.IGrantable): string | undefined { const grantPrincipal = grantee.grantPrincipal; - if (!isConstruct(grantPrincipal)) { - return undefined; - } // this logic should only apply to newly created // (= not imported) resources - if (!Resource.isOwnedResource(grantPrincipal)) { + if (!iam.principalIsOwnedResource(grantPrincipal)) { return undefined; } const keyStack = Stack.of(this); @@ -223,20 +220,20 @@ abstract class KeyBase extends Resource implements IKey { } private isGranteeFromAnotherRegion(grantee: iam.IGrantable): boolean { - if (!isConstruct(grantee)) { + if (!iam.principalIsOwnedResource(grantee.grantPrincipal)) { return false; } const bucketStack = Stack.of(this); - const identityStack = Stack.of(grantee); + const identityStack = Stack.of(grantee.grantPrincipal); return bucketStack.region !== identityStack.region; } private isGranteeFromAnotherAccount(grantee: iam.IGrantable): boolean { - if (!isConstruct(grantee)) { + if (!iam.principalIsOwnedResource(grantee.grantPrincipal)) { return false; } const bucketStack = Stack.of(this); - const identityStack = Stack.of(grantee); + const identityStack = Stack.of(grantee.grantPrincipal); return bucketStack.account !== identityStack.account; } } @@ -725,20 +722,3 @@ export class Key extends KeyBase { })); } } - -/** - * Whether the given object is a Construct - * - * Normally we'd do `x instanceof Construct`, but that is not robust against - * multiple copies of the `constructs` library on disk. This can happen - * when upgrading and downgrading between v2 and v1, and in the use of CDK - * Pipelines is going to an error that says "Can't use Pipeline/Pipeline/Role in - * a cross-environment fashion", which is very confusing. - */ -function isConstruct(x: any): x is Construct { - const sym = Symbol.for('constructs.Construct.node'); - return (typeof x === 'object' && x && - (x instanceof Construct // happy fast case - || !!(x as any).node // constructs v10 - || !!(x as any)[sym])); // constructs v3 -} \ No newline at end of file