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

(servicecatalog): ProductStack does not support Assets #20690

Closed
1 of 2 tasks
mackalex opened this issue Jun 9, 2022 · 14 comments · Fixed by #22857
Closed
1 of 2 tasks

(servicecatalog): ProductStack does not support Assets #20690

mackalex opened this issue Jun 9, 2022 · 14 comments · Fixed by #22857
Labels
@aws-cdk/aws-servicecatalog Related to AWS Service Catalog effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. in-progress This issue is being actively worked on. p1

Comments

@mackalex
Copy link
Contributor

mackalex commented Jun 9, 2022

Describe the feature

The feature is an improvement to the existing ProductStack construct to add support for the use of asset files.

Use Case

I'm always frustrated as a Service Catalog administrator when I try to add a Lambda function to my ProductStack in CDK because I want to reference my Lambda code from an asset file, and CDK throws an error when I attempt to synthesize this. This limitation means that I'm unable to make use of ProductStack when I want to create a Service Catalog product consisting of Lambas that run large amounts of code. This is an example of a product which I would like to deploy to Service Catalog and share with end users across AWS accounts:

class ServerlessProduct extends sc.ProductStack {
  constructor(scope: cdk.Construct, id: string) {
    super(scope, id);

    // Defines an AWS Lambda resource
    const myHandler = new lambda.Function(this, 'Handler', {
      runtime: lambda.Runtime.NODEJS_14_X,
      code: lambda.Code.fromAsset(path.join(__dirname, 'handler')),
      handler: 'index.handler'
    });

    // Defines an API Gateway REST API resource backed by the handler function
    new apigw.LambdaRestApi(this, 'Endpoint', {
      handler: myHandler
    });
  }
}

cdk synth

Error thrown: Service Catalog Product Stacks cannot use Assets

Proposed Solution

Design as of 6/9/22 by @wanjacki, @mackalex

Currently, CDK vends an asset bucket during bootstrap-time to the customer's AWS account. This bucket can be used successfully for enabling file asset support in ProductStack with CFN outputs from the parent stack for both the S3 bucket name and object key. The major issue with this approach is that when sharing a Service Catalog portfolio across accounts, a product that makes use of file assets cannot be provisioned since the parent stack with the aforementioned outputs does not exist in the end-user account.

To solve this, we could implement the usage of a bespoke S3 Bucket to contain asset files from assets used in a Service Catalog ProductStack. The bucket could exist at the Service Catalog Portfolio level which is instantiated in a ProductStack's parent stack. A bespoke bucket for this use case allows us to control the naming of the bucket as well as its permissions. Controlling the bucket name is important, especially at synth-time, since this will be referenced by resources that use assets, such as a Lambda function which references Python code stored in an asset file in the S3 bucket. Controlling permissions on a bucket which contains assets is important within the framework of Service Catalog since the administrator of a Service Catalog portfolio shares this portfolio across AWS accounts with end users who make use of products which reference asset files.

Other Information

Additional design considerations:
We have considered making use of the bootstrap bucket which CDK vends to customers to hold assets used by resources in a ProductStack. This presents issues with cross-account sharing of Service Catalog portfolios since the assets bucket would require permissions for the end-user account to access an asset file used by a Provisioned Product, and appending permissions to an existing S3 bucket policy is nearly infeasible without overwriting the bucket policy. This is not a desirable experience, especially when the bucket policy being overwritten would be the bootstrap bucket used by many components of CDK.

Relates issues:
#20361

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.24.1 (build 585f9ca)

Environment details (OS name and version, etc.)

macOS Big Sur Version 11.6.5 (20G527)

@mackalex mackalex added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jun 9, 2022
@github-actions github-actions bot added the @aws-cdk/aws-servicecatalog Related to AWS Service Catalog label Jun 9, 2022
@peterwoodworth peterwoodworth added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jun 10, 2022
@peterwoodworth
Copy link
Contributor

This would be a great addition to make ProductStack more complete. Are you willing to make a PR for this and discuss based on that?

@mackalex
Copy link
Contributor Author

Yes, a PR will be made for this issue for a more in depth review of the implementation.

@padaszewski
Copy link

Hi @mackalex,
Thx for taking care of this. Our team currently overrides the bucket policy and uses an own managed key for cross-account sharing from the assets bucket. This a really awful workaround, which makes the integration and production pipelines to be dependent on the update of the bucket policy and key policy (happens in the dev pipeline).
Do You have any orientation timeline for the PR?

@bendudz
Copy link

bendudz commented Jul 12, 2022

Hi

Has there been any movement on this feature please? I am trying to produce a Service Catalog product for our org to use a common EKS stack. I didn’t want to implement a workaround if this feature will be ready soon.

Many thanks for your effort on this.

@wanjacki
Copy link
Contributor

wanjacki commented Jul 12, 2022

@bendudz
We are looking at having a PR out by end of July and merged by mid August.

Edit/Update:
We are shooting for end of September now (We are getting alot of request for this so we are definitely prioritizing this as much as we can)

@philiphorrocks
Copy link

Any update on this? Would be great to get this feature as we have products which require lamba functions

@peterwoodworth peterwoodworth added the in-progress This issue is being actively worked on. label Sep 7, 2022
@wanjacki
Copy link
Contributor

wanjacki commented Sep 7, 2022

@philiphorrocks We are working on it, I provided an update:

"We are shooting for end of September now (We are getting alot of request for this so we are definitely prioritizing this as much as we can)"

As a work around if your lambda is small enough you might be able to use the fromInline feature for now

@dennisfleischmann
Copy link

Hi is there already an update on that? Is it meanwhile supported? I receive the same issue on the following

`
class SelfserviceVpc(servicecatalog.ProductStack):
def init(self, scope: Construct, id: str, regional_ipam_pools: List[object],):
super().init(scope, id)

[...]
## Get CIDR
self.getcidr = cr.AwsCustomResource(self, "GetCidr",
on_create=cr.AwsSdkCall(
service='EC2',
action='getIpamPoolAllocations',
parameters = {
'IpamPoolAllocationId': cidr_allocation.attr_ipam_pool_allocation_id,
'IpamPoolId': ipam_pool.ref
},
physical_resource_id=cr.PhysicalResourceId.of("GetCidr")
),
policy=cr.AwsCustomResourcePolicy.from_sdk_calls(
resources=cr.AwsCustomResourcePolicy.ANY_RESOURCE
)
)
`

@marcus-j
Copy link

Hi there, is there a (new) timeline for this feature?

@wanjacki
Copy link
Contributor

Hi Marcus as of now there's another PR that needs to be merged blocking #22857 . There's no new timeline for this feature as of now, we will try to get it in as soon as possible.

@padaszewski
Copy link

Hi @wanjacki, do You think that there might be a chance to merge this till the end of January? We are waiting for this for almost a year now and therefore constantly moving some of our releases. We want of course provide a clean implementation and not our temporary workarounds. It is hard to me from the GitHub conversations find a conclusion or orientation for us, thus the question.

@paulmowat
Copy link

I've also just hit this and having to try find workarounds to get it to work. If there is no ETA on when it will be ready. Do you have a recommended/suggested workaround for how to do this at the minute?

@wanjacki
Copy link
Contributor

Well we got an approval from CDK team so hopefully we can get that merged and close this issue soon. End of year should be possible.

@mergify mergify bot closed this as completed in #22857 Dec 28, 2022
mergify bot pushed a commit that referenced this issue Dec 28, 2022
Currently Assets are not supported in Product Stacks. Service Catalog has an unique use case where assets need to be shared cross account and sharing the entire CDK asset bucket is not ideal. Users can either create their own ProductStackAssetBucket or have one automatically generated for them based on their account Id and region. By using S3 Deployments we able to copy the assets to that bucket and share it when a portfolio is shared in Service Catalog.

More details can be found here: #20690. Closes #20690

RFC: aws/aws-cdk-rfcs#458
----

### All Submissions:

* [X ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [X] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [X] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [X ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
----
Co-authored-by: Theron Mansilla[[imanolympic](https://github.com/imanolympic)]
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

brennanho pushed a commit to brennanho/aws-cdk that referenced this issue Jan 20, 2023
Currently Assets are not supported in Product Stacks. Service Catalog has an unique use case where assets need to be shared cross account and sharing the entire CDK asset bucket is not ideal. Users can either create their own ProductStackAssetBucket or have one automatically generated for them based on their account Id and region. By using S3 Deployments we able to copy the assets to that bucket and share it when a portfolio is shared in Service Catalog.

More details can be found here: aws#20690. Closes aws#20690

RFC: aws/aws-cdk-rfcs#458
----

### All Submissions:

* [X ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [X] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [X] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [X ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
----
Co-authored-by: Theron Mansilla[[imanolympic](https://github.com/imanolympic)]
brennanho pushed a commit to brennanho/aws-cdk that referenced this issue Feb 22, 2023
Currently Assets are not supported in Product Stacks. Service Catalog has an unique use case where assets need to be shared cross account and sharing the entire CDK asset bucket is not ideal. Users can either create their own ProductStackAssetBucket or have one automatically generated for them based on their account Id and region. By using S3 Deployments we able to copy the assets to that bucket and share it when a portfolio is shared in Service Catalog.

More details can be found here: aws#20690. Closes aws#20690

RFC: aws/aws-cdk-rfcs#458
----

### All Submissions:

* [X ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [X] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [X] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [X ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
----
Co-authored-by: Theron Mansilla[[imanolympic](https://github.com/imanolympic)]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-servicecatalog Related to AWS Service Catalog effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. in-progress This issue is being actively worked on. p1
Projects
None yet
9 participants