-
Notifications
You must be signed in to change notification settings - Fork 0
Workflow update #5
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
base: main
Are you sure you want to change the base?
Conversation
Why these changes are being made: The first attempt at the automated deployment workflows failed when it got to the point of deploying in stage & prod environments (the script failed to create the matrix list). The changes here do two things. First, there are updates to the GHA workflows to handle the delivery of the ZIP file to the S3 bucket in the correct AWS Account. Second, there are updates to the shell script that generates the matrix JSON object to determine which application should be updated. How these changes are implemented: * Update all three GHA workflows to the latest actions/checkout and aws-actions/configure-aws-credentials * Update all three GHA workflows to pull the last 10 commits when running actions/checkout (to ensure that we have enough commit history to handle the git diff) * Remove the workflow_dispatch trigger (since that trigger does not play nice with any sort of git diff approach) * Add a conditional so that the workflows will not run if triggered by dependabot * Add environment variables that are pulled from the github context so that the shell script has the right git history information to generate the correct git diff * Update the prod-deploy GHA workflow to match the deployment process used in other repositories: copy the ZIP from stage to prod * Update the script with additional documentation * Update the script to account for the significantly different GHA context for a published release tag workflow * Update the script to allow for better local testing WIP: exclude dependabot
Significant updates to the README documentation in this repo. * Add files to the `tests/` folder that contain snippets of the GitHub Actions context * Update the README in `tests/` to provide detailed instructions for testing the script * Update the main README for clarity
Why these changes are being introduced: There were open dependabot PRs for some dependency updates. How this addresses that need: * Ran `make update-all` to update npm dependencies Side effects of this change: None.
| The [dev-deploy.yml](.github/workflows/dev-deploy.yml) workflow uses the `pull_request` trigger. The reference file for the GitHub Actions context for this trigger is either [pr_open.json](tests/pr_open.json) for a new PR. | ||
|
|
||
|
|
||
| ```bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to guess and add some SHAs that were probably close enough to be useful to see this working.
It would probably be good to clarify how to decide which SHAs from git log --pretty=oneline --no-abbrev-commit should be used to set the various config.
I set GITHUB_SHA, HEAD_SHA to the current head SHA and BASE_SHA to one commit back in the history. If that makes sense, we can just say that. If it doesn't, we should clarify what does.
Additionally, I'm confused as I thought as I thought for PR builds not all of these values are set by Actions so it's unclear why we set all of these values for all of the testing types rather than just passing in what Actions would provide to better simulate that environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It indeed works the same if I leave out GITHUB_SHA so I think this example (and the other similar ones) should be update to reflect what Actions actually sets to eliminate this potential confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider DEFAULT_BRANCH=main BASE_SHA=$(git rev-parse --short HEAD^) HEAD_SHA=$(git rev-parse --short HEAD) ACTION="opened" ./.github/scripts/generate-matrix.sh as a one liner to set values possibly usefully to test this locally. Similar approach could be applied to other examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oof! I used to have a check in the script for the existence of GITHUB_SHA in the env that would fail out immediately. After I removed that from the script, I forgot to remove that line from the test blocks. 🤦♂️
more to follow...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(not sure how deep into the weeds we want to go here...)
First, here is the current pretty git log
% git log --pretty=oneline --no-abbrev-commit workflow-update
500d3feedd4fa954c2a7f7038da350f7c4957b04 (HEAD -> workflow-update, origin/workflow-update) Dependency Updates
3452e4b46d4f1bfd4c73e749b647fc0d4d42e73f Documentation Updates
e216b8c9e38fd86c63ff8575581da00f9c956923 Update Workflows
8fe0da84c8ed9881e921102bca844c7b30fcd7ec (tag: v0.1.0, origin/main, origin/HEAD, main) Merge pull request #2 from MITLibraries/workflow-updates
a609d0217c08d1394c2d97fdae63cad5a8e2d34c (origin/workflow-updates) Address PR Comments
5200b2203daa5c3c65474117f60c741ffdb9f32c Documentation Updates
a2d38548fdb8303352f00b9e3212a701dacc19e9 Create Stage & Prod GHA Stubs
70d60446e41c18ee10dfac621e56b4ffeb1b2f8c Add Dev Deployment Workflow
1d821faf259575d5d6f0271e383c48eb6387fb8d Reorganize For Upcoming GHA Workflows
e2f2b79d93a55360f1e67caf99231a0b51d1f70b (tag: v0.0.2) Merge pull request #1 from MITLibraries/lambda-url-checker
8c0009a85ebc19f8724a96d4d2d20fefa3e8d651 (origin/lambda-url-checker) Test Suite for Lambda Function URL Checker
24efb46a9b03231454adf59505aea334f0e66e4c (tag: v0.0.1) First Iteration of Lambda URL Checker
ed15e5b15deef6fea1607c811e5d847731dad6c0 Repository Cleanup and Documentation
b10336c8fd5487429efcf2d2555e9535c6c5451c Initial File Structure
54d2746a0566188054b2becf371633eacd1fa842 Initial commit
To imitate the "open a new PR" workflow, the HEAD_SHA would be e216b8c9... while the BASE_SHA would be 8fe0da84... (corresponding to the first commit on the feature branch and the last commit on the main branch, respectively). So, your suggested oneliner would probably be better like this:
DEFAULT_BRANCH=main BASE_SHA=$(git rev-parse --short main) HEAD_SHA=$(git rev-parse --short HEAD) ACTION="opened" ./.github/scripts/generate-matrix.sh
The same goes for the "additional commit on an open PR" workflow. The BASE_SHA should be the last commit on main before the commits on the feature branch. You can see this in the Confluence document in the two context snippets here. That is, we don't use the before and after SHAs, we still use the base and head SHAs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah... right, this all assumes you are in a branch when testing locally so your one-liner (adding --short to the main call to normalize) is ideal. If someone is on a branch with no commits or poking at this in main then I think that would always result in no changes... which is probably expected but we may want to warn about that.
This addresses the questions/concerns raised during code review for the README in the tests folder that describes how to test the script that does the git diff work and generates the matrix for the rest of the GitHub Action.
|
@JPrevost I finally found my way back to this and made some updates to the README per our conversation above. |
Purpose and background context
This fixes the automated deployment workflows to get the CD part of our CI/CD pipelines working properly.
There are significant updates to the main README and to the README in the
tests/folder. Additionally, there is a new document in Confluence giving an overview of how to address continuous delivery for a monorepo with GitHub Actions:How can a reviewer manually see the effects of these changes?
Hmmm... not 100% possible! But, it is possible to run some local tests to verify that the updates to the script and to the workflows will hopefully work as expected.
See the notes in the
tests/README.mdfile for details on how to run some local tests to verify that the workflows will behave as expected.Includes new or updated dependencies?
YES: There are a few
npmdependency updates included in this PR which should force a redeployment of thelambda_function_url_checkerapplication.Changes expectations for external applications?
NO