-
Notifications
You must be signed in to change notification settings - Fork 25
feat: add boto3 and botocore automated dependency update github worklflows #94
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
…mments to structs in build.rs
eb3100b to
262d7c3
Compare
| - cron: "0 0 * * *" | ||
| workflow_dispatch: | ||
| permissions: | ||
| contents: write |
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.
we don't need such permissions for the entire action right?
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.
yeah you're right, i'll revise!
| ref: release | ||
| token: ${{ secrets.CUSTOM_GITHUB_ACTION_PAT }} | ||
|
|
||
| - name: Get autopilot head commit |
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.
why we are not using --version -d for this, Get boto3 version used by release HEAD and Get botocore version used by release HEAD?
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.
my thinking is that ipa --version -d should be run using the ipa commit not at the HEAD of release, but rather using the ipa commit at the most recent release with a tag. Thus, I made Get boto3 version used by release HEAD and Get botocore version used by release HEAD only retrieve the current submodule commit IDs of boto3/botocore at the HEAD of release; ipa --version -d is not run here at the HEAD of release, in case the HEAD of release has not yet been tagged with an actual official release yet. Instead, the retrieved submodule commit IDs at the HEAD of release are used by the later job checkout_release_repos_with_autopilot_head_submodule to checkout the submodules to those retrieved commit IDs, checkout IPA to its most recent tagged release (not the HEAD of the release branch), and run ipa --version -d only then.
| - name: update submodules to current versions used at head of release branch | ||
| id: checkout-submodules-to-autopilot-head-commit | ||
| env: | ||
| AUTOPILOT_RELEASE_HEAD_BOTO3_COMMIT: ${{ needs.checkout_autopilot_release_head.outputs.autopilot_release_head_boto3_commit }} |
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.
these steps seem to be more complex than I expected. Can we not do something like:
git submodule --init --update
cargo build --workspace
ipa --version -d
... // logic for comparing diff
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.
My thinking is that IPA should be built with the latest tagged release of IPA, not what is currently at the HEAD of release. Thus, because we are checking out to the IPA's latest tagged release (rather than the HEAD of release branch), we need to manually update the submodules to the versions that are actually at the HEAD of release branch. I get the tags for those submodules, just for logging purposes in the PR (since it seems important to know whether the submodule update is just bumping the patch version, vs. a major/minor version, in the tag).
| run: | | ||
| cd iam-policy-autopilot-policy-generation/resources/config/sdks/boto3 && git submodule update --init --recursive && git checkout $LATEST_RELEASE_BOTO3_COMMIT && cd ../../../../.. | ||
|
|
||
| - name: Trim commit hashes to 7 chars |
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.
why?
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.
in line 276, the PR must be submitted, such that the newer boto3 version we are updating to, is staged locally.
in line 278, it's just for making the PR message cleaner :)
| with: | ||
| commit-message: | | ||
| chore: update boto3 version from ${{ steps.trim-commit-hashes.outputs.autopilot_release_head_boto3_commit_display }} (tag: ${{ needs.checkout_release_repos_with_autopilot_head_submodule.outputs.boto3_autopilot_release_head_git_tag }}) to ${{ steps.trim-commit-hashes.outputs.latest_release_boto3_commit_display }} (tag: ${{ needs.checkout_release_repos_with_latest_submodule.outputs.boto3_latest_release_git_tag }}) | ||
| branch: dependencies/boto3 |
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.
shall we create new branch or reuse the same branch?
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.
the PR must be staged somewhere, so I said to create+re-use the dependencies/boto3 branch to stage the PR. Not sure if it would result in merge conflicts when repeatedly overriding that branch though, but I think it's a force update.
| run: | | ||
| cd iam-policy-autopilot-policy-generation/resources/config/sdks/botocore-data && git submodule update --init --recursive && git checkout $LATEST_RELEASE_BOTOCORE_COMMIT && cd ../../../../.. | ||
|
|
||
| - name: Trim commit hashes to 7 chars |
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.
why?
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.
in line 276, the PR must be submitted, such that the newer botocore version we are updating to, is staged locally.
in line 278, it's just for making the PR message cleaner :)
| chore: update botocore version from ${{ steps.trim-commit-hashes.outputs.autopilot_release_head_botocore_commit_display }} (tag: ${{ needs.checkout_release_repos_with_autopilot_head_submodule.outputs.botocore_autopilot_release_head_git_tag }} ) to ${{ steps.trim-commit-hashes.outputs.latest_release_botocore_commit_display }} (tag: ${{ needs.checkout_release_repos_with_latest_submodule.outputs.botocore_latest_release_git_tag }}) | ||
| body: | | ||
| botocore version info at current head: ```${{ needs.checkout_release_repos_with_autopilot_head_submodule.outputs.botocore_autopilot_release_head_version_info }}``` | ||
| botocore version info to be updated to: ```${{ needs.checkout_release_repos_with_latest_submodule.outputs.botocore_latest_release_version_info }}``` |
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'd not imagine this action to be 353 lines long...
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.
😳
| branches: | ||
| - release | ||
| workflow_dispatch: | ||
| permissions: |
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.
lets keep the entire action's branch minimal and only grant these permissions at step level when needed
| uses: hmarr/auto-approve-action@v4 | ||
| with: | ||
| pull-request-number: ${{ needs.sync-branches.outputs.pull-request-number }} | ||
| github-token: ${{ secrets.CUSTOM_GITHUB_ACTION_PAT }} No newline at end of file |
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.
why this action needs to be separate from submodule_update_pr.yml?
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's so I can use the condition:
if: needs.sync-branches.outputs.pull-request-number != ''
I didn't see a way to skip substeps of a job, only entire jobs.
Issue #, if available:
#63
Description of changes:
There are two workflows being added here:
submodule_update_pr.yml:This does the following, on a manual trigger or a cron basis:
iam-policy-autopilot --version --debugto get the corresponding data checksum.iam-policy-autopilot --version --debugto get the corresponding data checksum.sync_release_to_main.yml:The previous action only submits PR updates to the release branch. Once those submitted PRs are merged, we want another workflow to run, which merges that dependency update in the release branch, back to the main branch. This workflow does this by:
CUSTOM_GITHUB_ACTION_PATrepository secret in https://github.com/awslabs/iam-policy-autopilot/settings/secrets/actions.Two notes:
--version --debugfunctionality needed for this action.CUSTOM_GITHUB_ACTION_PATin https://github.com/awslabs/iam-policy-autopilot/settings/secrets/actions. This is needed for auto-approval action to work successfully.Example of successful PR runs:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.