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

Action fails on first deployment if secrets are used #162

Open
1000hz opened this issue Aug 30, 2023 · 13 comments · Fixed by cloudflare/workers-sdk#4032
Open

Action fails on first deployment if secrets are used #162

1000hz opened this issue Aug 30, 2023 · 13 comments · Fixed by cloudflare/workers-sdk#4032

Comments

@1000hz
Copy link
Contributor

1000hz commented Aug 30, 2023

We currently upload secrets before deploying the script, but this understandably will fail if the script has never been deployed yet. We should think about how to prevent this, e.g. "touch" the script if we get a 404 response and retry.

1000hz added a commit that referenced this issue Aug 30, 2023
The action fails to upload secrets during one of the testing steps since the Worker doesn't exist yet.
@penalosa
Copy link
Contributor

Wrangler should do this by default (it uploads an empty script). Is this failing somehow?

@1000hz
Copy link
Contributor Author

1000hz commented Aug 30, 2023

Yeah, I was observing API errors. Perhaps that fallback behavior wasn’t implemented for wrangler secret:bulk.

@JacobMGEvans
Copy link
Contributor

Yeah might not be implemented on secret:bulk

@madhusudhand
Copy link

I ran into the same issue. Without a secret, workers script gets deployed successfully.

image

I believe I have the right configuration.

- name: Deploy
  uses: cloudflare/wrangler-action@v3
  with:
    accountId: ${{ secrets.CF_ACCOUNT_ID }}
    apiToken: ${{ secrets.CF_API_TOKEN }}
    workingDirectory: my-app
    command: deploy --env dev
    secrets: DB_SECRET
  env:
    DB_SECRET: ${{ secrets.DB_SECRET }}

I expected some detailed error message, so that I know what's wrong. Looks like in the exception catch block the error message is not printed.

@theetherGit
Copy link

I think we can use vars also. Some basic variable are not needed to be secrets.

- name: Deploy
  uses: cloudflare/wrangler-action@v3
  with:
    accountId: ${{ secrets.CF_ACCOUNT_ID }}
    apiToken: ${{ secrets.CF_API_TOKEN }}
    workingDirectory: my-app
    command: deploy --env dev
    vars: DB_SECRET
  env:
    DB_SECRET: ${{ secrets.DB_SECRET }}

Worked for me as of now

@hubertott
Copy link

I ran into the same issue. Without a secret, workers script gets deployed successfully.

image I believe I have the right configuration.
- name: Deploy
  uses: cloudflare/wrangler-action@v3
  with:
    accountId: ${{ secrets.CF_ACCOUNT_ID }}
    apiToken: ${{ secrets.CF_API_TOKEN }}
    workingDirectory: my-app
    command: deploy --env dev
    secrets: DB_SECRET
  env:
    DB_SECRET: ${{ secrets.DB_SECRET }}

I expected some detailed error message, so that I know what's wrong. Looks like in the exception catch block the error message is not printed.

I'd agree this is also a chicken and egg problem.

However, I also think the issue is the fact that we are now being asked to pass the --env to the command. eg. deploy --env dev But as mentioned the secret is uploaded prior to that. At that point Wrangler does not know what environment you are in.

As per the ambiguous warning which states:

Since you have specified an environment you need to make sure to pass in '--env dev' to your command.

You need to be including the environment parameter to the action as well as the command
environment: dev
ie.

- name: Deploy
  uses: cloudflare/wrangler-action@v3
  with:
    accountId: ${{ secrets.CF_ACCOUNT_ID }}
    apiToken: ${{ secrets.CF_API_TOKEN }}
    environment: dev
    workingDirectory: my-app
    command: deploy --env dev
    secrets: DB_SECRET
  env:
    DB_SECRET: ${{ secrets.DB_SECRET }}

@timothymiller
Copy link

timothymiller commented Sep 17, 2023

Experiencing this issue too

Edit:
Oh wow, specifying environment: preview fixed it.

@JacobMGEvans
Copy link
Contributor

This should be resolved with the next release of Wrangler.

@1000hz
Copy link
Contributor Author

1000hz commented Sep 26, 2023

FWIW we'll still need a PR to bump the default version of wrangler

@aroman
Copy link

aroman commented Sep 30, 2023

@JacobMGEvans i don't think your PR quite fixes this.

  1. while your PR does create a draft worker, it preserves the return false, so the wrangler command will fail even though the secrets upload successfully. furthermore, the log "succesfully created secret for key" is suppressed in the draft-worker codepath (source)

  2. as a result, while the PR does cause create a draft worker to be created and the secrets succesfully uploaded, the wrangler command exits uncleanly (code 1), and logs:

✨ 0 secrets successfully uploaded

✘ [ERROR] 🚨 7 secrets failed to upload
  1. also, perhaps not a big problem, but i also noticed that the draft worker creation occurs within a Promise.all() call, so it might happen multiple times as part of the promise race. since there is no real "bulk secret" upload (just a faked version with a bunch of concurrent promises to upload individual secrets), an API call to create a draft worker is created for each secret in the bulk secret list. i think this should be fairly harmless though. i guess the real solution here is for there to be an actual bulk secret upload API though.

@vladinator1000
Copy link

vladinator1000 commented Mar 4, 2024

I just migrated to wrangler-action v3 and this started happening to me. It fails indefinitely though, doesn't pass on retry.

image

You need to be including the environment parameter to the action as well as the command environment: dev

This feels like it would mess with my env vars and how the wrangler.toml is read. I'm deploying to production, I'm not changing the environment to dev or preview.

This should be resolved with the next release of Wrangler.

@JacobMGEvans I'm on wrangler-action v3, do you see anything wrong with my config? I'm not sure what to do here.

Here's my workflow file

name: Deploy

on:
  workflow_dispatch:
  push:
    branches: [main]

jobs:
  build_and_deploy:
    name: Build and deploy
    runs-on: ubuntu-latest

    strategy:
      matrix:
        node-version: [21.x]

    env:
      ENVIRONMENT: production
      APP_SECRET: ${{ secrets.APP_SECRET }}
      DATABASE_URL: ${{ secrets.DATABASE_URL }}
      CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}

    steps:
      - uses: actions/checkout@v4
      - name: Use Node.js ${{ matrix.node-version }}
        uses: actions/setup-node@v4
        with:
          node-version: ${{ matrix.node-version }}

      - uses: c-hive/gha-yarn-cache@v2

      - name: Get version
        id: version
        run: echo "::set-output name=version::$(date +'%Y-%m-%dT%H:%M:%S')-${{ github.sha }}"

      - name: Install dependencies
        run: yarn --frozen-lockfile

      - name: 🔨📦 Build and deploy
        uses: cloudflare/wrangler-action@v3
        with:
          apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
          environment: 'production'
          secrets: |
            APP_SECRET
            DATABASE_URL

@JacobMGEvans
Copy link
Contributor

JacobMGEvans commented Mar 4, 2024

@vladinator1000
I am no longer at Cloudflare, however I am sure that @1000hz can pick this up 😄 I would suggest making your own separate issue at a glance though.

@vladinator1000
Copy link

Whoops, sorry for the ping @JacobMGEvans 😅 New issue here #240

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

Successfully merging a pull request may close this issue.

9 participants