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

Unexpected behavior for pre/post commands using wrangler #178

Open
zcei opened this issue Oct 2, 2023 · 2 comments
Open

Unexpected behavior for pre/post commands using wrangler #178

zcei opened this issue Oct 2, 2023 · 2 comments

Comments

@zcei
Copy link

zcei commented Oct 2, 2023

Hej there,

what we do is using the preCommands hook to execute wrangler secret:bulk.

(Just for some context: in our CI step we need to sync some secrets from an external system towards Cloudflare. Thus we can/do not want to use the very static approach of using the secrets/env dance in the action definition, because this would mean using GitHub as an intermediary secrets store and having one more place that needs to be kept in sync manually.)

Thing's I've found:

  1. Despite the claim in the README, wrangler is not available right away in that context, but needs to be invoked via your package manager of choice (kudos & hat tip for recent work on supporting multiple ones 👌)
  2. By not differentiating the wrangler script from any other binary, it makes sense that all configuration is lost. I would have loved to inherit the environment, so that I don't need to re-state it again.
 - name: Deploy
   uses: cloudflare/wrangler-action@v3
   with:
     environment: ${{ inputs.environment }}
     packageManager: pnpm
     apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
     preCommands: |
       echo "Sync secrets to Cloudflare Workers Secrets"
-      fetch_from_external_system | pnpm wrangler secret:bulk --env ${{ inputs.environment }}
+      fetch_from_external_system | wrangler secret:bulk

Let me know what you think about this. Definitely not urgent as there are easy workaround, but took me a couple attempts to get it right 😇

Thanks for all your work on the action 🙏

@1000hz
Copy link
Contributor

1000hz commented Oct 30, 2023

Because preCommands/postCommands allow you to execute in the context of a shell, I don't think we'd want to attempt to append any wrangler flags to these commands as there's a high likelihood we'd get it wrong or put the flag in the wrong place.

As for needing to execute wrangler via your package manager, this is because we don't install wrangler globally. I'll have a think on whether it would make sense for us to do so, but also I want us to get away from installing wrangler unless we really need to (see #199). This means in most cases it would not end up installed globally (it would probably be listed as a devDependency and installed to your project's node_modules during your workflow's install step).

@zcei
Copy link
Author

zcei commented Oct 31, 2023

I don't think we'd want to attempt to append any wrangler flags to these commands as there's a high likelihood we'd get it wrong or put the flag in the wrong place.

Agreed, I was thinking about something like a CLOUDFLARE_CI_CONFIGURED_ENVIRONMENT or so environment variable that is set in the shell context. It's ignored if any explicit environment is passed, otherwise basically used to "inherit" the configuration that has been explicitly stated in the GitHub Actions config.

it would probably be listed as a devDependency

Yep, that's already the case for us, be we couldn't stop the action from attempting to install it 😛 I'm indecisive whether I would expect in a wrangler-action to have wrangler command available, I get the point about a "clean" shell context, but also get my past-me that it did expect it to be available 🤣
For now, let's just adjust the README then to make it more explicit that you need to invoke wrangler with your package manager of choice?

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