cloudflare / pages-action

MIT License
473 stars 97 forks source link

Race condition between publishing and retrieving the deployment metadata #29

Closed fubhy closed 1 month ago

fubhy commented 1 year ago

This code here can cause (albeit relatively unlinkely) a race condition where the values returned for the deployment url, id, etc. are not actually those produced by the previous npx wrangler pages publish command.

We ran into this with our production deployment and a pull request that seemingly happened at the exact same time. Since we use that output to call an external API to register the deployment URL and metadata, this caused a temporary service degradation on our end.

So even if this is indeed a rather rare event, in a large team with a lot of parallel development and github workflows running all the time it's definitely bound to happen every now and then.

I'd suggest to extend wrangler to support a --json flag that outputs machine-readable metadata for all its commands and then use that here instead of the REST api call.

fubhy commented 1 year ago

Sorry, meant to add a link to the code in question: https://github.com/cloudflare/pages-action/blob/main/src/index.ts#L68-L74

GregBrimble commented 1 year ago

Hi, thanks for the report! We're slowly but surely building out APIs in wrangler, so once we have a import { pagesPublish } from 'wrangler' (or similar) surface, we'll be able to properly address this problem.

fubhy commented 1 year ago

Great. That's a good solution for this github action indeed! I'd still argue that a --json option could be helpful for the wrangler CLI in general :-)

jasikpark commented 1 year ago

Is there any progress on this? sorry to poke :p I suppose this is still blocked on wrangler having a JS api?

https://github.com/cloudflare/wrangler2/blob/main/packages/wrangler/src/pages/publish.tsx

my main interest is getting consistent alias output so that I can generate useful links to the deploy previews

jasikpark commented 1 year ago

Sorry to poke again, but is there any work I can do to help move this along? I have a script that creates a facimile address by just appending

`${branchName}${cfpages-preview-url}`

but this doesn't work due to DNS encoding values and truncation of the url. I suppose I could just reverse engineer what the rest of the algo is 🤷

mcoetzee commented 6 months ago

@jasikpark This might interest you. I managed to get the alias with the below setup.

After deploying I make a request to the Cloudflare API to get the alias. You just need to replace YOUR_POJECT_NAME in the endpoint and it should work. You can then use ${{ steps.get-alias.outputs.alias }} as you see fit:

      - name: Publish
        id: cloudflare
        uses: cloudflare/pages-action@1
        with:
          ... All your settings

      - name: Get Deployment Alias
        if: github.event_name == 'pull_request'
        id: get-alias
        run: |
          sleep 20s
          deployment_response=$(curl --request GET --url https://api.cloudflare.com/client/v4/accounts/${{ secrets.CLOUDFLARE_ACCOUNT_ID }}/pages/projects/YOUR_POJECT_NAME/deployments/${{ steps.cloudflare.outputs.id }} --header 'Content-Type: application/json' -H "Authorization: Bearer ${{ secrets.CLOUDFLARE_API_TOKEN }}")
          alias=$(jq -r '.result.aliases[0]' <<< "$deployment_response")
          echo "alias=$alias" >> "$GITHUB_OUTPUT"

      - name: Echo outputs
        run: |
          echo ${{ steps.cloudflare.outputs.id }}
          echo ${{ steps.cloudflare.outputs.url }}
          echo ${{ steps.cloudflare.outputs.environment }}
          echo ${{ steps.get-alias.outputs.alias }}

Edit 1: Getting some mixed results when deploying the PR for the first time. Sometimes the alias is not yet available.

Edit 2: Adding the sleep 20s before making the request seems to work well enough.

jasikpark commented 6 months ago

My current solution is to just manually generate the deploy url from the branch name:

/** Discovered from generated urls in the CloudFlare Dashboard */
const MAX_LENGTH_SUBDOMAIN = 28;

/**
 * {@link genDeployURL} tries to take the branch for a deploy preview and generate the same subdomain that CloudFlare does.
 * @param props {{ branchName: string }}
 */
export default function genDeployURL({ branchName }) {
  let formattedSubdomain = branchName
    .toLowerCase()
    .replace(/[_|\/]/g, '-')
    .slice(0, MAX_LENGTH_SUBDOMAIN);

  // Do not end the subdomain with an `-`.
  if (formattedSubdomain.endsWith('-')) {
    formattedSubdomain = formattedSubdomain.slice(0, formattedSubdomain.length - 1);
  }

  return formattedSubdomain;
}
- name: Generate CF Deploy Preview URL
        id: deploy-preview-url-gen
        uses: actions/github-script@v6
        with:
          result-encoding: string
          script: |
            const { default: script } = await import('${{ github.workspace }}/.github/scripts/gen-deploy-url.mjs')
            return script({ branchName: '${{ steps.branch-name.outputs.current_branch }}' })

This may yet be incomplete, but it's been good enough for me for now. I use a GA to grab the true branch name and another GA to comment on the PR with the deploy details.

nprogers commented 1 month ago

I agree that the alias output from the publish step seems to always be the deployment URL. We are not making any improvements to this repo moving forward. The workarounds above will also work with wrangler-action.