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

Commands piped to Wrangler doesn't work in GitHub Actions #210

Closed
vipulgupta2048 opened this issue Nov 21, 2023 · 4 comments
Closed

Commands piped to Wrangler doesn't work in GitHub Actions #210

vipulgupta2048 opened this issue Nov 21, 2023 · 4 comments

Comments

@vipulgupta2048
Copy link

Here's our GitHub action step using Wrangler Action.

    - name: Deploy to Cloudflare Pages
        uses: cloudflare/wrangler-action@5e8484995321734668f14981c316aa9188d76ed1 # v3.3.2
        with:
          apiToken: ${{ secrets.CF_API_TOKEN }}
          accountId: ${{ secrets.CF_ACCOUNT_ID }}
          wranglerVersion: "3.5.1" 
          preCommands: wrangler --version
          postCommands: wrangler pages deploy --branch ${{ env.CF_BRANCH }} --project-name=${{ inputs.cloudflare_website }} build/ | tee -a $GITHUB_STEP_SUMMARY

This command used to work, but is now failing when trying out the new wrangler-action v3.3.2 with the following error. We used the pipe to get command output into GitHub Step Summary for a nice bit of result and get the Cloudflare pages draft URL.

Here's the error: The wrangle command tries to check what these arguments are as well.

🚀 Running Wrangler Commands
  /home/runner/_work/_tool/node/18.18.2/x64/bin/npx wrangler pages deploy --branch renovate/cloudflare-wrangler-action-3.3.x --project-name=flowzone build/ | tee -a $GITHUB_STEP_SUMMARY`


  ✘ [ERROR] Unknown arguments: a, |, tee

I recommend that we define proper outputs for this action in the action.yml file. These standardized outputs can be used by folks anywhere? Can add that myself if people agree?

Link of the action run: https://github.com/product-os/flowzone/actions/runs/6905675937/job/18789164936

@1000hz
Copy link
Contributor

1000hz commented Nov 22, 2023

Only preCommands/postCommands are executed within a shell (and thus allow piping between processes). Because we need to augment command with extra arguments, we don't support piping and execute it via child_process.spawn().

Adding some outputs to the action is a great suggestion that I think we definitely ought to implement. I've opened #211 to capture this.

While far from ideal, in the meantime you could work around this with the following changes to your workflow config:

      - name: Deploy to Cloudflare Pages
        uses: cloudflare/wrangler-action@5e8484995321734668f14981c316aa9188d76ed1
        with:
          apiToken: ${{ secrets.CF_API_TOKEN }}
          accountId: ${{ secrets.CF_ACCOUNT_ID }}
          wranglerVersion: 3.5.1
-         preCommands: wrangler --version
-         command: pages deploy --branch ${{ env.CF_BRANCH }} --project-name=${{ inputs.cloudflare_website }} build/ | tee -a $GITHUB_STEP_SUMMARY
+         command: --version
+         postCommands: npx pages deploy --branch ${{ env.CF_BRANCH }} --project-name=${{ inputs.cloudflare_website }} build/ | tee -a $GITHUB_STEP_SUMMARY

@1000hz 1000hz closed this as completed Nov 22, 2023
@github-project-automation github-project-automation bot moved this from Untriaged to Done in workers-sdk Nov 22, 2023
@1000hz 1000hz closed this as not planned Won't fix, can't repro, duplicate, stale Nov 22, 2023
@vipulgupta2048
Copy link
Author

Heyyo @1000hz From my testing, I understood as much about preCommands/postCommands. I was thinking about this suggestion that you made as the only way out but wanted to confirm if the removal of piping in command is expected. Will make the change as you suggested, thanks!

@1000hz
Copy link
Contributor

1000hz commented Nov 22, 2023

Yes, removing support for shell piping in command was intentional since it probably should have never been allowed given how we treat the input. In hindsight, we should have done a major version bump when we made this change because it effectively broke anyone relying on this behavior.

@vipulgupta2048
Copy link
Author

Yeah no worries @1000hz These things happen. Happy to help improve this bit.

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