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

Bug/Question: Why is stepsToDownloadAssembly() called with cdkoutDir? #706

Open
diranged opened this issue Sep 2, 2023 · 4 comments
Open

Comments

@diranged
Copy link

diranged commented Sep 2, 2023

Problem

We're trying to leverage this library to run a few pipelines in our CDK project, and we were running into problems where each time the npx projen build was run, we got a modified deploy.yml file. Each and every time we ran it, the diff changes:

% npx projen build && git diff .github/workflows/deploy.yml
...
diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml
index f8eb654..2a6ed0e 100644
--- a/.github/workflows/deploy.yml
+++ b/.github/workflows/deploy.yml
@@ -49,7 +49,7 @@ jobs:
         uses: actions/download-artifact@v3
         with:
           name: cdk.out
-          path: /private/var/folders/dm/b5by_qw91nd0ctdjbvggzgr40000gq/T/cdk.outz8GheM
+          path: /private/var/folders/dm/b5by_qw91nd0ctdjbvggzgr40000gq/T/cdk.outiCdOwR
       - name: Install
         run: npm install --no-save cdk-assets
       - name: Authenticate Via OIDC Role
% npx projen build && git diff .github/workflows/deploy.yml
...
diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml
index f8eb654..2a6ed0e 100644
--- a/.github/workflows/deploy.yml
+++ b/.github/workflows/deploy.yml
@@ -49,7 +49,7 @@ jobs:
         uses: actions/download-artifact@v3
         with:
           name: cdk.out
-          path: /private/var/folders/dm/b5by_qw91nd0ctdjbvggzgr40000gq/T/cdk.outz8GheM
+          path: /private/var/folders/dm/b5by_qw91nd0ctdjbvggzgr40000gq/T/cdk.outBt0txL
       - name: Install
         run: npm install --no-save cdk-assets
       - name: Authenticate Via OIDC Role

Digging through the code, I found that stepsToDownloadAssembly() is being called with targetDir: cdkoutDir. Tracing the cdkoutDir back, we find that it's basically set to app.outdir at

const cdkoutDir = app.outdir;
.

What does this cause?

Each and every time the build.yml's self-mutation step runs, it detects a change to .github/workflows/deploy.yml and commits that change, causing a build loop (presuming you have granted your Projen Github App with the right credentials so that it can mutate the workflow files).

Our fix: Set CDK_OUTDIR

If we explicitly set the CDK_OUTDIR environment variable to /tmp, we can convince the system to generate a consistent deploy,yml file:

% npx projen build && git diff .github/workflows/deploy.yml
...
diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml
index f8eb654..2a6ed0e 100644
--- a/.github/workflows/deploy.yml
+++ b/.github/workflows/deploy.yml
@@ -49,7 +49,7 @@ jobs:
         uses: actions/download-artifact@v3
         with:
           name: cdk.out
-          path: /private/var/folders/dm/b5by_qw91nd0ctdjbvggzgr40000gq/T/cdk.outz8GheM
+          path: /tmp
       - name: Install
         run: npm install --no-save cdk-assets
       - name: Authenticate Via OIDC Role

The trick was, how to do this programmatically? So we added a few hacks to our .projenrc.ts file:

# .projenrc.ts
project.github?.tryFindWorkflow('build')?.file?.addOverride('env', { CDK_OUTDIR: '/tmp' });
# main.ts
const pipeline = new GitHubWorkflow(app, 'StagingPipeline', {
...
  synth: new ShellStep('Build', {
    commands: ['yarn install', 'yarn build'],
    env: {
      CDK_OUTDIR: '/tmp',
    },
  }),

Question: Is this an intentional design decision?

My main question here is whether or not it is an intentional decision that the deploy.yml file would be mutated each and every time that npx projen build is run? That seems pretty strange .. yet I can't find any evidence of other users complaining about this.

@kaizencc
Copy link
Contributor

kaizencc commented Feb 9, 2024

Hi @diranged, thanks for the ticket + the investigation, and I'm sorry I'm only now getting to this. This is not intentional and synthing should be deterministic. I plan on looking at some common failures this weekend, and will keep this in mind. If you have any additional thoughts let me know!

@kaizencc
Copy link
Contributor

kaizencc commented Feb 9, 2024

@diranged there is one thing here though. I think you are doing extra work to get your outDir: '/tmp'. You should be able to specify the outdir directly in the App, and then you are good to go.

app = new App({
    outdir: '/tmp',
  });

@diranged
Copy link
Author

@kaizencc Hey ... thanks for the response. So we're not really using this library now, we're instead using https://github.com/Nextdoor/cdk-pipelines-github which does this in a more simple fashion I believe.. but I think my big question was - how are other people using this? Are they? This seemed like a pretty big issue at least from our perspective..

@kaizencc
Copy link
Contributor

I would wager that others are almost certainly setting outdir in their App. The random temp directory is not being generated by this library, it is being generated by CDK. So the best we can do here is clearly call out the need for outdir to be specified in the README of this module. That change is incoming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants