Escape double-colons (::)

[karakter98]

Escape double-colons (::) from AWS custom resource naming convention, so the resource URNs are compatible with Pulumi's format.

Allows users to define their own dynamic providers for custom resources, which partially solves #60.

With these minimal changes, I was able to implement a generic dynamic provider for custom resources, which seems to work fine for S3BucketDeployment and S3AutoDeleteObjects resources. I'll brush it up a bit and will open another PR.

[github-actions]

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[iwahbe]

Hi @karakter98. Thanks for the PR! It would be really helpful if you could post an example of what works with this change, but didn't work without it. A quick test would be even better.

[iwahbe]

/run-acceptance-tests

[github-actions]

Please view the PR build - https://github.com/pulumi/pulumi-cdk/actions/runs/6005610699

[karakter98]

@iwahbe I tried to write a test, but it seems the pulumi runtime isn't actually called to register the resource, so the URN error never shows up in the test file. I believe it's because of the mocks, but I'm not familiar enough with the codebase to figure it out further.

What we can do now and couldn't do before: register custom resources from CDK that contain :: in their names, like Custom::S3AutoDeleteObjects from the cdk Bucket class.

[karakter98]

A quick example:

const bucket = new Bucket(this, "Bucket", {
    enforceSSL: true,
    removalPolicy: RemovalPolicy.DESTROY,
    autoDeleteObjects: true,
});

new BucketDeployment(this, "Deployment", {
    sources: [Source.asset(path.join("..", "build", "web"))],
    destinationBucket: bucket,
    retainOnDelete: false,
});

Both the S3AutoDeleteObjects and S3BucketDeployment custom resources created by the above code can't be registered by Pulumi, because of double-colons in the resource names (of the form Custom::S3AutoDeleteObjects or similar). This clashes with Pulumi's URN format when it tries to register the resources.

[iwahbe]

Thanks for the example @karakter98. I have asked someone familiar with pulumi-ckd to take a look at your PR.

[lukehoban]

@karakter98 Thanks for this PR!

A quick example

Do you have this example running end-to-end after these changes? I'd love to include a test that locks in the correct functionality for these cases.

It would also be great to pull this replacement logic out into a function - perhaps urnEncode.

I do also slightly wonder if we want to do a different URN encoding that (a) is reversible and (b) is more certain to avoid ::. The current replacement isn't reversile because you can't be sure : should be : or ::. And also doesn't strictly speaking avoid ::, as :::: would still turn into ::. It's uglier, but perhaps sending : to %3A might be more reliable.

Technically, this is also a breaking change, but I think given that it would always have errored previously, there was no working code that this can break. In the future, if we want to change this encoding pattern, we can likely use aliases to support the current pattern - so the decision on the replacement is not a one-way door.

[karakter98]

@lukehoban Yes, I'm currently using this to deploy the frontend for a new project, so I can confirm it allows the use of custom resources.

The actual complete example is a bit complicated, because it uses:

• remapCloudControlResource to handle each custom resource separately
• A dynamic provider that calls the Lambdas behind the custom resources according to the CloudFormation contract (and provides them with S3 presigned URLs so they can upload the execution status, just like with CloudFormation)
• Changes to the default remapCloudControlResource for Lambda and IAM Role resources, because the generated names when nested into custom constructs get longer than 64 characters, so I had to also limit this length to get custom resources to work

I think it makes sense to split this into multiple PRs, because so much setup is needed on the user side that the test code would be pretty huge.

I'll test out your suggestion with %3A encoding and see if Pulumi accepts that format, thanks for the idea!

[lukehoban]

I think it makes sense to split this into multiple PRs, because so much setup is needed on the user side that the test code would be pretty huge.

Got it. Yeah - I tried out the original example and ran into all those same problems as well. I’ll open some issues to track the remaining things we’ll need to address here. If you already have PRs in progress, great!

[github-actions]

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[karakter98]

@lukehoban I changed the encoding according to your feedback, can confirm Pulumi registers the names correctly with the %3A (ran pulumi preview on the project I'm working on without errors)

[iwahbe]

/run-acceptance-tests

[github-actions]

Please view the PR build - https://github.com/pulumi/pulumi-cdk/actions/runs/6051638879

[karakter98]

Is this fine to be merged? I already opened other PRs and am ready to start working on the dynamic provider PR for custom resources, but this needs to be merged first

[karakter98]

@lukehoban could you take a look when you get some time?

[github-actions]

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[corymhall]

@karakter98 we have added support for custom resources in #190