Update and Improve the CDN Publishing Workflow#80
Open
cabutlermit wants to merge 8 commits into
Open
Conversation
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
Why these changes are being made: This started as just a simple "update the version of third party actions" and then turned into a refactor. The refactor cleans up the logic and DRYs the code a bit and requires NO changes from any caller workflow. How this addresses that need: * Update actions/checkout and aws-actions/configure-aws-credentials to the most recent versions * Clean up usage of environment variables in the various bash commands throughout the workflow * Add a "polling" step (using the `aws cloudfront wait` command) to ensure that the cache invalidation completes before the workflow finishes
Why these changes are being introduced: The previous commit was a simple cleanup of the shared CDN publishing workflow. This commit extends that work with a full refactor of the workflow that remains compatible with our existing caller workflows and sets the stage for future repositories that might need to publish content to the CDN (in particular, the future work to publish the NDE-tacos content to the CDN). How this addresses that need: * Create additional inputs to allow for more options for the the source and target of the `aws s3 sync` command * Update existing inputs with corrected "required" versus "optional" and default values * Update all the inputs with descriptions * Add a "validation" step to ensure that some of the freeform inputs are valid and fail the job quickly if they are not * Refine the "environment" and "domain" verifications and set environment variables with the correct combination of environment, domain, S3 bucket name, and CloudFront domain name (this also allows future caller workflows to not have to pass the bucket name when calling this workflow) * Using the new envrionment variables, simplify both the `aws s3 sync` command and the cache invalidation command Side effects of this change: None Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/NDE-96
Why these changes are being made: Eventually, we will update the caller workflows, but for now we want to ensure that the old caller workflows still work even with all the changes to this shared workflow. How these changes are implemented: * Add a block to the validation step to parse the deprecated S3_URI input to set "legacy" environment variables for later steps and set a boolean to indicate if this workflow has been called by a legacy caller workflow * Update later steps with a conditional for the SOURCE_PATH variable to set it correctly for legacy and non-legacy workflows Side effects: None.
cabutlermit
force-pushed
the
update-cdn-publish
branch
from
adb854d to
a14748c
Compare
Why these changes are being introduced: First, we want to ensure that the only additional parameters supplied by the caller workflow are `--include` and `--exclude` parameters. Second, since we pass the parameters via an `env:` block, we have to be extra careful with double-quotes (which are required by the `aws s3 sync` command, but typically get stripped when expanded in a bash script). How these changes are implemented: * Add a block to the validation step to throw an error if there is any other parameter in the SYNC_PARAMS input outside of `--include` and `--exclude` * If the SYNC_PARAMS is valid, merge it together with the other stock `--exclude` parameters and set one string in the GITHUB_ENV with the full list of inclues and excludes * Add the `eval` command to properly expand the `aws s3 sync` command and preserve the double-quotes where they are required Side effects: None.
cabutlermit
force-pushed
the
update-cdn-publish
branch
from
a14748c to
2a4e2ba
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the shared CDN publishing workflow to be safer and more flexible for both existing “static site” callers and future repositories by adding input validation, reworking sync source/target handling, and improving cache invalidation behavior.
Changes:
- Expands workflow inputs to support
SOURCE_PATH+TARGET_PATHwhile keeping legacyS3URIsupport. - Adds validation and refactors variable expansion toward
env:to reduce injection risk and fail fast on bad inputs. - Updates the workflow to wait for CloudFront invalidation completion and improves Action summary output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| README.md | Updates documentation for the shared CDN publishing workflow inputs and legacy/deprecation guidance. |
| .github/workflows/cdn-shared-publish.yml | Refactors the shared publish workflow: new inputs, validation, unified AWS credential setup, S3 sync source/target changes, and CloudFront invalidation waiting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Why these changes are being introduced: GitHub Copilot reviewed the pull request and noted a few issues in the shared workflow code. After reviewing the comments by Copilot, it was appropriate to address all of the concerns. How this addresses that need: * Update the Validate step to ensure that the SOURCE_PATH does not start with `..` (preventing a directory outside of the repository itself from being the source) * Update the Validate step to fail quickly if there are any newline characters in the SYNC_PARAMS input * Update the Validate step to add validation of the value passed in to the GHA_ROLE input to ensure that it is formmatted correctly as an IAM Role name (using a grep filter provided by AWS) * Update the Sync to CDN S3 Bucket step to prevent command injection; the previous version used `eval` to handle the command and preserve the double-quotes. The new version uses a bash array and the `read` command to parse the VALID_SYNC_PARAMS and then just executes the combined array (this techinque was suggested by GitHub Copilot) * Update the Invalidate Cache step to correct the mistake in the --paths argument (the "*" was missing previously) * Update the README with more details on the workflow inputs and a clearer explanation of how this shared workflow actually works Side effects of this change: None.
Why these changes are being introduced:= GitHub Copilot noted additional problems that needed to be fixed; a combination of typos in the README and mistakes in the workflow. How this addresses that need: * Update the validation of the GHA_ROLE using a simpler regex pattern and the "=~" bash operator * Rebuild the sync command, shifting the construction of the command line parameters from the validation step to the synchronization step. We still use a bash array to build the command, but we handle the SYNC_PARAMS input with a combination of printf and xargs to ensure that the input string is split correctly without any glob expansion * Fix the typo in the cache invalidation step (the "$" was in the wrong place) * Update the README
Contributor
Author
|
@JPrevost let's try another GitHub Copilot review... 🤞 |
Why these changes are being introduced: The second review by GitHub Copilot raised a few more concerns regarding validation steps. Those are addressed here. How this addresses that need: * Fix the SOURCE_PATH validation, leveraging the `realpath` bash tool to ensure that the SOURCE_PATH does not stray outside of a subdirectory of the repository * Fix the "legacy check" to prevent a "custom" domain workflow for trying to synchronize content to the root of the CDN bucket * Minor updates to the README for clarity Side effects of this change: None.
Comment on lines
+223
to
+227
| echo "DISTRIBUTION_ID=$(aws ssm get-parameter \ | ||
| --name "/tfvars/libraries-website/standard-cdn-id" \ | ||
| --query 'Parameter.Value' \ | ||
| --output text)" >> $GITHUB_ENV | ||
| echo "S3URI=s3://$BUCKET/cdn$TARGET_PATH/" >> $GITHUB_ENV |
* Address additional conerns regarding validation checks and building the correct S3 target path. Fix the README typo.
cabutlermit
force-pushed
the
update-cdn-publish
branch
from
a32b7f7 to
e97398d
Compare
JPrevost
reviewed
Jul 2, 2026
JPrevost
left a comment
JPrevost
left a comment
Member
There was a problem hiding this comment.
I think this might be easier to read and thus maintain if we split the two publication paths into separate workflows.
While both are copying files to S3, the logic diverges quite a few times to the point of this requiring a lot of cognitive load to follow even if you understand what is happening.
checkout, env setup, AWS auth would be duplicated but could be extracted into composite actions each publication workflow calls if we wanted to.
I feel like this tried to avoid duplication but in so doing increased cognitive load enough that I think making changes will be difficult to do without causing inadvertent errors as is.
I'd be happy to chat in zoom or slack about this, and if you agree but don't want to deal with that split at this time I can plod through the rest of this and get to an approval state I'm sure :)
| echo "Invalid. The SOURCE_PATH must begin with a '.'" | ||
| exit 1 | ||
| fi | ||
| REPO_PATH=$(realpath -e "${WORKSPACE_PATH}" 2>/dev/null) |
Member
There was a problem hiding this comment.
Non-blocking: I'm not sure I see the value in swallowing error messages here. Was it really noisy?
| There are a number of inputs to the shared workflow, some optional and some required. Here's a summary of the inputs. | ||
|
|
||
| - `AWS_REGION` (**optional**, *string*, default = `us-east-1`): the region where the S3 bucket lives. | ||
| - `DOMAIN` (**optional**, *string*, default = `standard`): the default value of `standard` refers to the standard CDN. If the content in question is associated with the custom domain CDN, then the caller workflow must pass the value `custom` instead of relying on the default. |
Member
There was a problem hiding this comment.
I'm finding myself wanting examples of repos using standard vs default. Not as an exhaustive permanent list, but so I could possible go look at how they are configured in the future if I wanted to set something up to work like it does. i.e. See web-images-staticfor a standard workflow andgrandchallenges-static` for custom. Note: using custom requires additional config outside this workflow (I assume)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose and background context
Overview
The end result is a shared workflow that is backward compatible with the existing caller workflows from our "static site" repositories AND is ready to use for any future repositories that need to publish objects to paths in our CDN. It fails fast on invalid input values, has flexibility for both source paths (in the caller repository) and target paths (in the S3 bucket backing the CDN), waits until the cache invalidation is complete before finishing the workflow run, and has cleaner output to the Action summary page.
For more details about the CloudFront Distributions, see the Reference: CDN, CloudFront, Web Content document in Confluence.
Details
The
actions/checkoutandaws-actions/configure-aws-credentialsare updated to the latest versions and their parameters are updated for clarity and security (e.g., no persistinggitcredentials after the initial checkout).The whole workflow received a number of updates, from cleaning up and expanding the caller workflow inputs, to a new step to run through validation on a number of the inputs to ensure that the values are usable and if they are not, the workflow will fail fast. At the same time, the logic of the workflow was completely revamped to DRY out the code and move input & variable expansion from the
run:blocks intoenv:blocks (this prevents injection attacks). Polling was added to the cache invalidation step to only let the workflow finish after the cache invalidation step is complete.Knowing that this workflow will be used by future repositories to push content into the CDN, the whole concept of source & target for the
aws s3 sync ...command was reworked from the old assumption that the source was always the root of the repository. Moving forward, this shared workflow expectsSOURCE_PATHandTARGET_PATHinput variables instead.Finally, extra bits were added to the workflow to ensure that our existing caller workflows that still pass just the
S3_URIvalue will still work as expected. Those caller workflows will eventually need to be updated, but for now, they will still work properly.How can a reviewer manually see the effects of these changes?
During the development of these changes, the caller workflows from future-of-libraries-static and web-images-static were run to verify compatibility. Each of those repositories has a feature branch named
test-publish-workflowthat updates thedev-cdn-publish.ymlworkflow to point to this feature branch for the shared workflow.A review can visit one of those two repositories, select the Actions tab, select the DEV Publish workflow, and manually run that workflow from the
test-publish-workflowbranch. That will call the shared workflow from this branch.Includes new or updated dependencies?
YES: New versions of third-party Actions
Changes expectations for external applications?
NO: This is 100% backwards compatible for existing caller workflows
Code review