jogold / cloudstructs

High-level constructs for AWS CDK
Apache License 2.0
167 stars 14 forks source link

Docker requirement necessary for ToolkitCleaner? #269

Closed perpil closed 5 hours ago

perpil commented 6 months ago

Thank you for creating this! It's super helpful.

I was using ToolkitCleaner and noticed it required Docker running, which I believe is due to this.

I don't use ECR currently and I'm wondering whether it would be possible to have a property in ToolkitCleanerProps to conditionally deploy the image cleanup code/step function branch so Docker isn't necessary. Does DOCKER_IMAGE_ASSET_HASH need to be set from dockerImageAsset or could it be set from fileAsset?

If this is possible and makes sense, let me know, I'd be happy to attempt submitting a PR.

jogold commented 6 months ago

Hi @perpil,

Maybe we can introduce a prop cleanEcrAssets?: boolean that defaults to true?

Does DOCKER_IMAGE_ASSET_HASH need to be set from dockerImageAsset or could it be set from fileAsset?

From dockerImageAsset, but if we don't deploy the ECR part we don't need this env var in the runtime code to search for asset hashes.

perpil commented 6 months ago

A cleanEcrAssets property makes sense. Thanks for the clarification on dockerImageAsset. I'll try to implement this when I get some time.

perpil commented 3 days ago

I started work on this, but I'm having trouble with building. When I run yarn install and then yarn build I get this on the build compile step:

👾 build » compile | jsii --silence-warnings=reserved-word
[2024-11-19T16:10:05.183] [ERROR] jsii/compiler - Compilation errors prevented the JSII assembly from being created
error TS2688: Cannot find type definition file for 'cookie'.
  The file is in the program because:
    Entry point for implicit type library 'cookie'
error TS2688: Cannot find type definition file for 'debug'.
  The file is in the program because:
    Entry point for implicit type library 'debug'
error TS2688: Cannot find type definition file for 'hast'.
  The file is in the program because:
    Entry point for implicit type library 'hast'
error TS2688: Cannot find type definition file for 'mdast'.
  The file is in the program because:
    Entry point for implicit type library 'mdast'
error TS2688: Cannot find type definition file for 'ms'.
  The file is in the program because:
    Entry point for implicit type library 'ms'
error TS2688: Cannot find type definition file for 'nlcst'.
  The file is in the program because:
    Entry point for implicit type library 'nlcst'
error TS2688: Cannot find type definition file for 'unist'.
  The file is in the program because:
    Entry point for implicit type library 'unist'
👾 Task "build » compile" failed when executing "jsii --silence-warnings=reserved-word" (cwd: /Users/david/Documents/GitHub/jogold/cloudstructs)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I stashed my changes and still see the same, is there a step I'm missing to build the package?

perpil commented 3 days ago

I got passed that error by adding the following devDeps in the .projenrc.js

--- a/.projenrc.js
+++ b/.projenrc.js
@@ -30,6 +30,13 @@ const project = new awscdk.AwsCdkConstructLibrary({
     '@types/aws-lambda',
     '@types/mjml',
     '@types/tsscmp',
+    '@types/cookie',
+    '@types/debug',
+    '@types/hast',
+    '@types/mdast',
+    '@types/ms',
+    '@types/nlcst',
+    '@types/unist',

Is this expected?

jogold commented 3 days ago

Is this expected?

No, it isn't. You should run yarn install and then npx projen build.

perpil commented 2 days ago

I tried this in a GitHub codespace and that above error didn't occur so it must have been an issue with my local dev box. Disregard. I'll proceed with testing my changes.

perpil commented 5 hours ago

I'm not sure whether something changed with the CDK, but running cdk synth/cdk deploy on a new cdk app that uses ToolkitCleaner does not require Docker to be present and running. This change is no longer necessary.