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(servicecatalogappregistry): RAM Share is replaced on every change to Application #24760

Merged
merged 5 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions packages/@aws-cdk/aws-servicecatalogappregistry/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,8 @@ import * as iam from '@aws-cdk/aws-iam';
declare const application: appreg.Application;
declare const myRole: iam.IRole;
declare const myUser: iam.IUser;
application.shareApplication({
application.shareApplication('MyShareId', {
name:'MyShare',
accounts: ['123456789012'],
organizationArns: ['arn:aws:organizations::123456789012:organization/o-my-org-id'],
roles: [myRole],
Expand All @@ -302,7 +303,8 @@ E.g., sharing an application with multiple accounts and allowing the accounts to
```ts
import * as iam from '@aws-cdk/aws-iam';
declare const application: appreg.Application;
application.shareApplication({
application.shareApplication('MyShareId', {
name: 'MyShare',
accounts: ['123456789012', '234567890123'],
sharePermission: appreg.SharePermission.ALLOW_ACCESS,
});
Expand All @@ -315,7 +317,8 @@ import * as iam from '@aws-cdk/aws-iam';
declare const attributeGroup: appreg.AttributeGroup;
declare const myRole: iam.IRole;
declare const myUser: iam.IUser;
attributeGroup.shareAttributeGroup({
attributeGroup.shareAttributeGroup('MyShareId', {
name: 'MyShare',
accounts: ['123456789012'],
organizationArns: ['arn:aws:organizations::123456789012:organization/o-my-org-id'],
roles: [myRole],
Expand All @@ -328,7 +331,8 @@ E.g., sharing an application with multiple accounts and allowing the accounts to
```ts
import * as iam from '@aws-cdk/aws-iam';
declare const attributeGroup: appreg.AttributeGroup;
attributeGroup.shareAttributeGroup({
attributeGroup.shareAttributeGroup('MyShareId', {
name: 'MyShare',
accounts: ['123456789012', '234567890123'],
sharePermission: appreg.SharePermission.ALLOW_ACCESS,
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { CfnResourceShare } from '@aws-cdk/aws-ram';
import * as cdk from '@aws-cdk/core';
import { Names } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { StageStackAssociator } from './aspects/stack-associator';
import { AttributeGroup, IAttributeGroup } from './attribute-group';
Expand Down Expand Up @@ -90,9 +89,10 @@ export interface IApplication extends cdk.IResource {
/**
* Share this application with other IAM entities, accounts, or OUs.
*
* @param id The construct name for the share.
* @param shareOptions The options for the share.
*/
shareApplication(shareOptions: ShareOptions): void;
shareApplication(id: string, shareOptions: ShareOptions): void;

/**
* Associate this application with all stacks under the construct node.
Expand Down Expand Up @@ -205,13 +205,13 @@ abstract class ApplicationBase extends cdk.Resource implements IApplication {
* Share an application with accounts, organizations and OUs, and IAM roles and users.
* The application will become available to end users within those principals.
*
* @param id The construct name for the share.
* @param shareOptions The options for the share.
*/
public shareApplication(shareOptions: ShareOptions): void {
public shareApplication(id: string, shareOptions: ShareOptions): void {
const principals = getPrincipalsforSharing(shareOptions);
const shareName = `RAMShare${hashValues(Names.nodeUniqueId(this.node), this.node.children.length.toString())}`;
new CfnResourceShare(this, shareName, {
name: shareName,
new CfnResourceShare(this, id, {
name: shareOptions.name,
allowExternalPrincipals: false,
principals: principals,
resourceArns: [this.applicationArn],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { IAspect, Stack, Stage, Annotations } from '@aws-cdk/core';
import { IAspect, Stack, Stage, Annotations, Names } from '@aws-cdk/core';
import { IConstruct } from 'constructs';
import { IApplication } from '../application';
import { ApplicationAssociator } from '../application-associator';
import { SharePermission } from '../common';
import { hashValues, SharePermission } from '../common';
import { isRegionUnresolved, isAccountUnresolved } from '../private/utils';

export interface StackAssociatorBaseProps {
Expand Down Expand Up @@ -112,7 +112,9 @@ abstract class StackAssociatorBase implements IAspect {

if (node.account != this.application.env.account && !this.sharedAccounts.has(node.account)) {
if (this.associateCrossAccountStacks) {
this.application.shareApplication({
const shareId = `ApplicationShare${hashValues(Names.nodeUniqueId(this.application.node), Names.nodeUniqueId(node.node))}`;
this.application.shareApplication(shareId, {
name: shareId,
accounts: [node.account],
sharePermission: SharePermission.ALLOW_ACCESS,
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { CfnResourceShare } from '@aws-cdk/aws-ram';
import * as cdk from '@aws-cdk/core';
import { Names } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { IApplication } from './application';
import { getPrincipalsforSharing, hashValues, ShareOptions, SharePermission } from './common';
Expand Down Expand Up @@ -29,9 +28,10 @@ export interface IAttributeGroup extends cdk.IResource {
/**
* Share the attribute group resource with other IAM entities, accounts, or OUs.
*
* @param id The construct name for the share.
* @param shareOptions The options for the share.
*/
shareAttributeGroup(shareOptions: ShareOptions): void;
shareAttributeGroup(id: string, shareOptions: ShareOptions): void;
}

/**
Expand Down Expand Up @@ -77,11 +77,10 @@ abstract class AttributeGroupBase extends cdk.Resource implements IAttributeGrou
}
}

public shareAttributeGroup(shareOptions: ShareOptions): void {
public shareAttributeGroup(id: string, shareOptions: ShareOptions): void {
const principals = getPrincipalsforSharing(shareOptions);
const shareName = `RAMShare${hashValues(Names.nodeUniqueId(this.node), this.node.children.length.toString())}`;
new CfnResourceShare(this, shareName, {
name: shareName,
new CfnResourceShare(this, id, {
name: shareOptions.name,
allowExternalPrincipals: false,
principals: principals,
resourceArns: [this.attributeGroupArn],
Expand Down
5 changes: 5 additions & 0 deletions packages/@aws-cdk/aws-servicecatalogappregistry/lib/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ export enum SharePermission {
* The options that are passed into a share of an Application or Attribute Group.
*/
export interface ShareOptions {
/**
* Name of the share.
*
*/
readonly name: string;
/**
* A list of AWS accounts that the application will be shared with.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,32 +269,36 @@ describe('Application', () => {

test('fails for sharing application without principals', () => {
expect(() => {
application.shareApplication({});
application.shareApplication('MyShareId', {
name: 'MyShare',
});
}).toThrow(/An entity must be provided for the share/);
});

test('share application with an organization', () => {
application.shareApplication({
application.shareApplication('MyShareId', {
name: 'MyShare',
organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'MyShare',
Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
});
});

test('share application with an account', () => {
application.shareApplication({
application.shareApplication('MyShareId', {
name: 'MyShare',
accounts: ['123456789012'],
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'MyShare',
Principals: ['123456789012'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
Expand All @@ -304,13 +308,14 @@ describe('Application', () => {
test('share application with an IAM role', () => {
const myRole = iam.Role.fromRoleArn(stack, 'MyRole', 'arn:aws:iam::123456789012:role/myRole');

application.shareApplication({
application.shareApplication('MyShareId', {
name: 'MyShare',
roles: [myRole],
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'MyShare',
Principals: ['arn:aws:iam::123456789012:role/myRole'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
Expand All @@ -320,43 +325,46 @@ describe('Application', () => {
test('share application with an IAM user', () => {
const myUser = iam.User.fromUserArn(stack, 'MyUser', 'arn:aws:iam::123456789012:user/myUser');

application.shareApplication({
application.shareApplication('MyShareId', {
name: 'MyShare',
users: [myUser],
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'MyShare',
Principals: ['arn:aws:iam::123456789012:user/myUser'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
});
});

test('share application with organization, give explicit read only access to an application', () => {
application.shareApplication({
application.shareApplication('MyShareId', {
name: 'MyShare',
organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
sharePermission: appreg.SharePermission.READ_ONLY,
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'MyShare',
Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
});
});

test('share application with organization, allow access to associate resources and attribute group with an application', () => {
application.shareApplication({
application.shareApplication('MyShareId', {
name: 'MyShare',
organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
sharePermission: appreg.SharePermission.ALLOW_ACCESS,
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'MyShare',
Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationAllowAssociation'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,32 +212,36 @@ describe('Attribute Group', () => {

test('fails for sharing attribute group without principals', () => {
expect(() => {
attributeGroup.shareAttributeGroup({});
attributeGroup.shareAttributeGroup('MyShareId', {
name: 'MyShare',
});
}).toThrow(/An entity must be provided for the share/);
});

test('share attribute group with an organization', () => {
attributeGroup.shareAttributeGroup({
attributeGroup.shareAttributeGroup('MyShareId', {
name: 'MyShare',
organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMShare76d2681489c0',
Name: 'MyShare',
Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupReadOnly'],
});
});

test('share attribute group with an account', () => {
attributeGroup.shareAttributeGroup({
attributeGroup.shareAttributeGroup('MyShareId', {
name: 'MyShare',
accounts: ['123456789012'],
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMShare76d2681489c0',
Name: 'MyShare',
Principals: ['123456789012'],
ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupReadOnly'],
Expand All @@ -247,13 +251,14 @@ describe('Attribute Group', () => {
test('share attribute group with an IAM role', () => {
const myRole = iam.Role.fromRoleArn(stack, 'MyRole', 'arn:aws:iam::123456789012:role/myRole');

attributeGroup.shareAttributeGroup({
attributeGroup.shareAttributeGroup('MyShareId', {
name: 'MyShare',
roles: [myRole],
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMShare76d2681489c0',
Name: 'MyShare',
Principals: ['arn:aws:iam::123456789012:role/myRole'],
ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupReadOnly'],
Expand All @@ -263,43 +268,46 @@ describe('Attribute Group', () => {
test('share attribute group with an IAM user', () => {
const myUser = iam.User.fromUserArn(stack, 'MyUser', 'arn:aws:iam::123456789012:user/myUser');

attributeGroup.shareAttributeGroup({
attributeGroup.shareAttributeGroup('MyShareId', {
name: 'MyShare',
users: [myUser],
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMShare76d2681489c0',
Name: 'MyShare',
Principals: ['arn:aws:iam::123456789012:user/myUser'],
ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupReadOnly'],
});
});

test('share attribute group with organization, give explicit read only access to the attribute group', () => {
attributeGroup.shareAttributeGroup({
attributeGroup.shareAttributeGroup('MyShareId', {
name: 'MyShare',
organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
sharePermission: appreg.SharePermission.READ_ONLY,
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMShare76d2681489c0',
Name: 'MyShare',
Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupReadOnly'],
});
});

test('share attribute group with organization, give access to mutate attribute groups', () => {
attributeGroup.shareAttributeGroup({
attributeGroup.shareAttributeGroup('MyShareId', {
name: 'MyShare',
organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
sharePermission: appreg.SharePermission.ALLOW_ACCESS,
});

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMShare76d2681489c0',
Name: 'MyShare',
Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupAllowAssociation'],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"version": "31.0.0",
"files": {
"5fbf2a286122f4bc412b1730f96351e289444b1122006f36e4ade8fae8442765": {
"461d235e9497deb16b9209be4a927c7d0dc7aa06d668e38bfb19a90db8e4a4b2": {
"source": {
"path": "integ-servicecatalogappregistry-application.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "5fbf2a286122f4bc412b1730f96351e289444b1122006f36e4ade8fae8442765.json",
"objectKey": "461d235e9497deb16b9209be4a927c7d0dc7aa06d668e38bfb19a90db8e4a4b2.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@
}
}
},
"TestApplicationRAMShare004736f08f8e57044D5D": {
"TestApplicationMyShareIdE1044482": {
"Type": "AWS::RAM::ResourceShare",
"Properties": {
"Name": "RAMShare004736f08f8e",
"Name": "MyShare",
"AllowExternalPrincipals": false,
"PermissionArns": [
"arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly"
Expand Down
Loading