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

feat(servicecatalog): Add Product Stack Asset Support #22857

Merged
merged 15 commits into from Dec 28, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
70 changes: 70 additions & 0 deletions packages/@aws-cdk/aws-servicecatalog/README.md
Expand Up @@ -22,6 +22,7 @@ enables organizations to create and manage catalogs of products for their end us
- [Product](#product)
- [Creating a product from a local asset](#creating-a-product-from-local-asset)
- [Creating a product from a stack](#creating-a-product-from-a-stack)
- [Using Assets in your Product Stack](#using-aseets-in-your-product-stack)
- [Creating a Product from a stack with a history of previous versions](#creating-a-product-from-a-stack-with-a-history-of-all-previous-versions)
- [Adding a product to a portfolio](#adding-a-product-to-a-portfolio)
- [TagOptions](#tag-options)
Expand Down Expand Up @@ -185,6 +186,75 @@ const product = new servicecatalog.CloudFormationProduct(this, 'Product', {
});
```

### Using Assets in your Product Stack

You can reference assets in a Product Stack. For example, we can add a handler to a Lambda function or a S3 Asset directly from a local asset file.
In this case, you must provide a S3 Bucket with a bucketName to store your assets.

```ts
import * as lambda from '@aws-cdk/aws-lambda';
import * as cdk from '@aws-cdk/core';
import { Bucket } from "@aws-cdk/aws-s3";

class LambdaProduct extends servicecatalog.ProductStack {
constructor(scope: Construct, id: string) {
super(scope, id);

new lambda.Function(this, 'LambdaProduct', {
runtime: lambda.Runtime.PYTHON_3_9,
code: lambda.Code.fromAsset("./assets"),
handler: 'index.handler'
});
}
}

const userDefinedBucket = new Bucket(this, `UserDefinedBucket`, {
assetBucketName: 'user-defined-bucket-for-product-stack-assets',
wanjacki marked this conversation as resolved.
Show resolved Hide resolved
});

const product = new servicecatalog.CloudFormationProduct(this, 'Product', {
productName: "My Product",
owner: "Product Owner",
productVersions: [
{
productVersionName: "v1",
cloudFormationTemplate: servicecatalog.CloudFormationTemplate.fromProductStack(new LambdaProduct(this, 'LambdaFunctionProduct', {
assetBucket: userDefinedBucket,
})),
},
],
});
```

When a product containing an asset is shared with a spoke account, the corresponding asset bucket
will automatically grant read permissions to the spoke account.
Note, it is not recommended using a referenced bucket as permissions cannot be added from CDK.
In this case, it will be your responsibility to grant read permissions for the asset bucket to
the spoke account.

Furthermore, in order for a spoke account to provision a product with an asset, the role launching
the product needs permissions to read from the asset bucket.
We recommend you utilize a launch role with permissions to read from the asset bucket.
For example your launch role would need to include at least the following policy:
Comment on lines +268 to +271
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make the permissions more specific if they are using a launch role?
For example if they provide a launch role via portfolio.setLocalLaunchRoleName() then we could specify that as part of the bucket policy. From my testing it looks like the most specific we can get is something like this:

new iam.PolicyStatement({
	actions: [
		's3:GetObject*',
		's3:GetBucket*',
		's3:List*', ],
	effect: iam.Effect.ALLOW,
	resources: [
		bucket.bucketArn,
		bucket.arnForObjects('*'),
	],
	principals: [
		...this.sharedAccounts.map(account => new iam.ArnPrincipal(cdk.Stack.of(this).formatArn({
			service: 'iam',
			region: '',
			account,
			resource: 'role',
			resourceName: launchRoleName,
		}))),
	],
	conditions: {
		'ForAnyValue:StringEquals': {
			'aws:CalledVia': ['cloudformation.amazonaws.com'],
		},
		'Bool': {
			'aws:ViaAWSService': true,
		},
	},
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean actually adding that PolicyStatement to their launch role. We would have to save the LaunchRole in portfolio when it is set and add it our aspect, so we can propagate the sharedAccounts regardless of the order it is called. There can also be multiple bucket and multiple accounts and other possible complications trying to generate that PolicyStatement. I'm not if it worth the effort or if the customer wants us adding permissions to their launchRole for them.

Or do you mean adding this to the documentation as a recommended policy statement to use for launch role when deploying assets? I think this a good alternative as they can craft a PolicyStatement using that more specific template you provided and easily add it to their LaunchRole if needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No not to the launch role. This would be the policy that we add to the bucket. Rather than allowing read from any principal in the shared accounts, this only allows read from the launch role (if one is specified).

If this isn't going to cover all use cases though then we can leave the bucket policy as is and just add this as an example policy to the docs if users want to scope down the access more.


```json
{
"Statement": [
{
"Effect": "Allow",
"Action": [
"s3:GetObject"
],
"Resource": "*"
}
]
}
```

Please refer to [Set launch role](#set-launch-role) for additional details about launch roles.
See [Launch Constraint](https://docs.aws.amazon.com/servicecatalog/latest/adminguide/constraints-launch.html) documentation
to understand the permissions that launch roles need.

### Creating a Product from a stack with a history of previous versions

The default behavior of Service Catalog is to overwrite each product version upon deployment.
Expand Down
@@ -1,3 +1,4 @@
import { IBucket } from '@aws-cdk/aws-s3';
import * as s3_assets from '@aws-cdk/aws-s3-assets';
import { Construct } from 'constructs';
import { hashValues } from './private/util';
Expand Down Expand Up @@ -46,9 +47,16 @@ export abstract class CloudFormationTemplate {
*/
export interface CloudFormationTemplateConfig {
/**
* The http url of the template in S3.
*/
* The http url of the template in S3.
*/
readonly httpUrl: string;

/**
* The S3 bucket containing product stack assets.
* @default - None - no assets are used in this product
*/
readonly assetBucket?: IBucket;

}

/**
Expand Down Expand Up @@ -108,6 +116,7 @@ class CloudFormationProductStackTemplate extends CloudFormationTemplate {
public bind(_scope: Construct): CloudFormationTemplateConfig {
return {
httpUrl: this.productStack._getTemplateUrl(),
assetBucket: this.productStack._getAssetBucket(),
};
}
}
34 changes: 32 additions & 2 deletions packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
@@ -1,7 +1,8 @@
import * as iam from '@aws-cdk/aws-iam';
import { IBucket } from '@aws-cdk/aws-s3';
import * as sns from '@aws-cdk/aws-sns';
import * as cdk from '@aws-cdk/core';
import { Construct } from 'constructs';
import { Construct, IConstruct } from 'constructs';
import { MessageLanguage } from './common';
import {
CloudFormationRuleConstraintOptions, CommonConstraintOptions,
Expand Down Expand Up @@ -105,7 +106,7 @@ export interface IPortfolio extends cdk.IResource {
* @param product A service catalog product.
* @param options options for the constraint.
*/
constrainCloudFormationParameters(product:IProduct, options: CloudFormationRuleConstraintOptions): void;
constrainCloudFormationParameters(product: IProduct, options: CloudFormationRuleConstraintOptions): void;

/**
* Force users to assume a certain role when launching a product.
Expand Down Expand Up @@ -155,6 +156,8 @@ abstract class PortfolioBase extends cdk.Resource implements IPortfolio {
public abstract readonly portfolioArn: string;
public abstract readonly portfolioId: string;
private readonly associatedPrincipals: Set<string> = new Set();
private readonly assetBuckets: Set<IBucket> = new Set<IBucket>();
private readonly sharedAccounts: string[] = [];

public giveAccessToRole(role: iam.IRole): void {
this.associatePrincipal(role.roleArn, role.node.addr);
Expand All @@ -169,11 +172,17 @@ abstract class PortfolioBase extends cdk.Resource implements IPortfolio {
}

public addProduct(product: IProduct): void {
if (product.assetBuckets) {
for (const bucket of product.assetBuckets) {
this.assetBuckets.add(bucket);
}
}
AssociationManager.associateProductWithPortfolio(this, product, undefined);
}

public shareWithAccount(accountId: string, options: PortfolioShareOptions = {}): void {
const hashId = this.generateUniqueHash(accountId);
this.sharedAccounts.push(accountId);
new CfnPortfolioShare(this, `PortfolioShare${hashId}`, {
portfolioId: this.portfolioId,
accountId: accountId,
Expand Down Expand Up @@ -236,6 +245,19 @@ abstract class PortfolioBase extends cdk.Resource implements IPortfolio {
}
}

/**
* Gives access to Asset Buckets to Shared Accounts.
*
*/
protected addBucketPermissionsToSharedAccounts() {
if (this.sharedAccounts.length > 0) {
for (const bucket of this.assetBuckets) {
bucket.grantRead(new iam.CompositePrincipal(...this.sharedAccounts.map(account => new iam.AccountPrincipal(account))),
);
}
}
}

/**
* Create a unique id based off the L1 CfnPortfolio or the arn of an imported portfolio.
*/
Expand Down Expand Up @@ -336,6 +358,14 @@ export class Portfolio extends PortfolioBase {
if (props.tagOptions !== undefined) {
this.associateTagOptions(props.tagOptions);
}

cdk.Aspects.of(this).add({
visit(c: IConstruct) {
if (c instanceof Portfolio) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of instanceof can you try doing something like this instead?
instanceof has some issues and we are trying to stop using
it
.

cdk.Aspects.of(this).add({
  visit(c: IConstruct) {
    if (c.node.id === this.node.id) {
      ...
    };
  };
});

I think something like this should ensure it only runs on this construct.

c.addBucketPermissionsToSharedAccounts();
};
},
});
}

protected generateUniqueHash(value: string): string {
Expand Down
@@ -1,13 +1,44 @@
import { CfnBucket, IBucket } from '@aws-cdk/aws-s3';
import * as cdk from '@aws-cdk/core';
import { ProductStack } from '../product-stack';

/**
* Deployment environment for an AWS Service Catalog product stack.
*
* Interoperates with the StackSynthesizer of the parent stack.
*/
export class ProductStackSynthesizer extends cdk.StackSynthesizer {
public addFileAsset(_asset: cdk.FileAssetSource): cdk.FileAssetLocation {
throw new Error('Service Catalog Product Stacks cannot use Assets');
private readonly assetBucket?: IBucket;

constructor(assetBucket?: IBucket) {
super();
this.assetBucket = assetBucket;
}

public addFileAsset(asset: cdk.FileAssetSource): cdk.FileAssetLocation {
if (!this.assetBucket) {
throw new Error('An Asset Bucket must be provided to use Assets');
}

const physicalName = this.physicalNameOfBucket(this.assetBucket);

(this.boundStack as ProductStack)._addAsset(asset);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like we should manage the logic for _addAsset and _deployAssets
here instead of within ProductStack.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved them both over as suggested.


const bucketName = physicalName;
const s3Filename = asset.fileName?.split('.')[1] + '.zip';
const objectKey = `${s3Filename}`;
const s3ObjectUrl = `s3://${bucketName}/${objectKey}`;
const httpUrl = `https://s3.${bucketName}/${objectKey}`;

return { bucketName, objectKey, httpUrl, s3ObjectUrl, s3Url: httpUrl };
}

private physicalNameOfBucket(bucket: IBucket) {
const resolvedName = cdk.Stack.of(bucket).resolve((bucket.node.defaultChild as CfnBucket).bucketName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should just be bucket.bucketName right?

const bucketName = bucket.bucketName;
// if the bucketName is undefined or is a token
if (!bucketName || Token.isUnresolved(bucketName)) {
  throw new Error(...);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bucketName is always a token regardless of if we pass a bucketName or not.
I asked Rico about a solution and he suggested his comment here: #21487 (comment).
Implementing his suggestion we access the underlying CfnBucket and retrieve the bucketName.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh ok. In that case I think you will just have to handle the case where someone passes an IBucket and there is no default child. Maybe

if (Resource.isOwnedResource(bucket)) {
  cdk.Stack.of(bucket).resolve((bucket.node.defaultChild as CfnBucket).bucketName);
} else {
  bucket.bucketName;
}

if (resolvedName === undefined) {
throw new Error('A bucketName must be provided to use Assets');
}
return resolvedName;
}

public addDockerImageAsset(_asset: cdk.DockerImageAssetSource): cdk.DockerImageAssetLocation {
Expand Down
63 changes: 61 additions & 2 deletions packages/@aws-cdk/aws-servicecatalog/lib/product-stack.ts
@@ -1,11 +1,24 @@
import * as crypto from 'crypto';
import * as fs from 'fs';
import * as path from 'path';
import { IBucket } from '@aws-cdk/aws-s3';
import { BucketDeployment, ISource, Source } from '@aws-cdk/aws-s3-deployment';
import * as cdk from '@aws-cdk/core';
import { Construct } from 'constructs';
import { ProductStackSynthesizer } from './private/product-stack-synthesizer';
import { ProductStackHistory } from './product-stack-history';

/**
* Product stack props.
*/
export interface ProductStackProps {
/**
* A Bucket can be passed to store assets, enabling ProductStack Asset support
* @default No Bucket provided and Assets will not be supported.
*/
readonly assetBucket?: IBucket;
}

/**
* A Service Catalog product stack, which is similar in form to a Cloudformation nested stack.
* You can add the resources to this stack that you want to define for your service catalog product.
Expand All @@ -21,15 +34,21 @@ export class ProductStack extends cdk.Stack {
private _templateUrl?: string;
private _parentStack: cdk.Stack;

constructor(scope: Construct, id: string) {
private readonly assets: ISource[];
private assetBucket?: IBucket;

constructor(scope: Construct, id: string, props: ProductStackProps = {}) {
super(scope, id, {
synthesizer: new ProductStackSynthesizer(),
synthesizer: new ProductStackSynthesizer(props.assetBucket),
});

this._parentStack = findParentStack(scope);

// this is the file name of the synthesized template file within the cloud assembly
this.templateFile = `${cdk.Names.uniqueId(this)}.product.template.json`;

this.assets = [];
this.assetBucket = props.assetBucket;
}

/**
Expand All @@ -50,6 +69,44 @@ export class ProductStack extends cdk.Stack {
return cdk.Lazy.uncachedString({ produce: () => this._templateUrl });
}

/**
* Fetch the asset bucket.
*
* @internal
*/
public _getAssetBucket(): IBucket | undefined {
return this.assetBucket;
}

/**
* Asset are prepared for bulk deployment to S3.
* @internal
*/
public _addAsset(asset: cdk.FileAssetSource): void {
const assetPath = './cdk.out/' + asset.fileName;
wanjacki marked this conversation as resolved.
Show resolved Hide resolved
this.assets.push(Source.asset(assetPath));
}

/**
* Deploy all assets to S3.
* @internal
*/
private _deployAssets() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my other comment about doing this within the synthesizer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved them both over as suggested.

if (this.assetBucket && this.assets.length > 0) {
if (!cdk.Resource.isOwnedResource(this.assetBucket)) {
// eslint-disable-next-line no-console
console.warn('[WARNING]', 'Bucket Policy Permissions cannot be added to' +
wanjacki marked this conversation as resolved.
Show resolved Hide resolved
' referenced Bucket. Please make sure your bucket has the correct permissions');
}
new BucketDeployment(this._parentStack, 'AssetsBucketDeployment', {
sources: this.assets,
destinationBucket: this.assetBucket,
extract: false,
prune: false,
});
}
}

/**
* Synthesize the product stack template, overrides the `super` class method.
*
Expand All @@ -72,6 +129,8 @@ export class ProductStack extends cdk.Stack {
this._parentProductStackHistory._writeTemplateToSnapshot(cfn);
}

this._deployAssets();

fs.writeFileSync(path.join(session.assembly.outdir, this.templateFile), cfn);
}
}
Expand Down
16 changes: 16 additions & 0 deletions packages/@aws-cdk/aws-servicecatalog/lib/product.ts
@@ -1,3 +1,4 @@
import { IBucket } from '@aws-cdk/aws-s3';
import { ArnFormat, IResource, Resource, Stack } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { CloudFormationTemplate } from './cloudformation-template';
Expand All @@ -23,6 +24,13 @@ export interface IProduct extends IResource {
*/
readonly productId: string;

/**
* The asset buckets of a product created via product stack.
* @attribute
* @default - Empty - no assets are used in this product
*/
readonly assetBuckets?: IBucket[];

/**
* Associate Tag Options.
* A TagOption is a key-value pair managed in AWS Service Catalog.
Expand Down Expand Up @@ -171,6 +179,11 @@ export abstract class Product extends ProductBase {
export class CloudFormationProduct extends Product {
public readonly productArn: string;
public readonly productId: string;
/**
* The asset bucket of a product created via product stack.
* @default - Empty - no assets are used in this product
*/
public readonly assetBuckets: IBucket[] = [];

constructor(scope: Construct, id: string, props: CloudFormationProductProps) {
super(scope, id);
Expand Down Expand Up @@ -206,6 +219,9 @@ export class CloudFormationProduct extends Product {
props: CloudFormationProductProps): CfnCloudFormationProduct.ProvisioningArtifactPropertiesProperty[] {
return props.productVersions.map(productVersion => {
const template = productVersion.cloudFormationTemplate.bind(this);
if (template.assetBucket) {
this.assetBuckets.push(template.assetBucket);
}
InputValidator.validateUrl(this.node.path, 'provisioning template url', template.httpUrl);
return {
name: productVersion.productVersionName,
Expand Down