-
Notifications
You must be signed in to change notification settings - Fork 247
chore(ci): use github app for tokens #6534
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,24 +10,22 @@ jobs: | |
| name: Bump packages | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - name: Create Github App Token | ||
| uses: mongodb-js/devtools-shared/actions/setup-bot-token@ni/github-app-action | ||
| id: app-token | ||
| with: | ||
| app-id: ${{ vars.DEVTOOLS_BOT_APP_ID }} | ||
| private-key: ${{ secrets.DEVTOOLS_BOT_PRIVATE_KEY }} | ||
|
|
||
| - uses: actions/checkout@v4 | ||
| with: | ||
| # don't checkout a detatched HEAD | ||
| ref: ${{ github.head_ref }} | ||
|
|
||
| # this is important so git log can pick up on | ||
| # the whole history to generate the list of AUTHORS | ||
| fetch-depth: '0' | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're not updating AUTHORS in this workflow, so this seems unnecessary |
||
|
|
||
| - name: Setup git | ||
| run: | | ||
| git config --local user.email "41898282+github-actions[bot]@users.noreply.github.com" | ||
| git config --local user.name "github-actions[bot]" | ||
|
|
||
| - uses: actions/setup-node@v3 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20.16.0 | ||
| cache: 'npm' | ||
| cache: "npm" | ||
|
|
||
| - name: Install [email protected] | ||
| run: | | ||
|
|
@@ -40,21 +38,20 @@ jobs: | |
|
|
||
| - name: Bump packages | ||
| env: | ||
| LAST_BUMP_COMMIT_MESSAGE: 'chore(release): bump package versions' | ||
| SKIP_BUMP_PACKAGES: 'mongodb-compass' | ||
| LAST_BUMP_COMMIT_MESSAGE: "chore(release): bump package versions" | ||
| SKIP_BUMP_PACKAGES: "mongodb-compass" | ||
| run: | | ||
| npm run bump-packages | ||
| git add . | ||
| git commit --no-allow-empty -m "$LAST_BUMP_COMMIT_MESSAGE" || true | ||
|
|
||
| - name: Create Pull Request | ||
| id: cpr | ||
| uses: peter-evans/create-pull-request@v6 | ||
| uses: peter-evans/create-pull-request@5e914681df9dc83aa4e4905692ca88beb2f9e91f # 7.0.5 | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For 3rd party packages, we should use git commit shas instead of version tags as those can be force pushed.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a documentation for this that we can point to somewhere around this code? It's not obvious and would be very easy to revert back over time without noting why we are doing this
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's part of the general security hardening recommendations for actions: https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-third-party-actions While I don't have reasons to mistrust Peter, a rule of thumb I tend to follow is to use versions for actions by corporations such as microsoft, github, amazon, where there's strong expectation that they have security practices in place that would prevent access to their code. And for everything else, even if it's a popular action, I pin the commit sha.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, so can we add this link somewhere around this line (or any other place you think is appropriate) so that when someone is going over this code in the future and is unsure why exactly this is set to a hash instead of a version the rule is still followed?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should be adding comments as the commit sha usage should be the default for any 3rd party action. I did drop a message on slack though to bring more attention to this. I've also updated CONTRIBUTING.md with a section highlighting a few of the security best practices that also links to the full doc. |
||
| with: | ||
| token: ${{ secrets.SVC_DEVTOOLSBOT_TOKEN }} | ||
| commit-message: 'chore(release): bump package versions' | ||
| token: ${{ steps.app-token.outputs.token }} | ||
| commit-message: "chore(release): bump package versions" | ||
| branch: ci/bump-packages | ||
| title: 'chore(release): bump package versions' | ||
| title: "chore(release): bump package versions" | ||
| labels: no-title-validation | ||
| body: | | ||
| - Bump package versions | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,18 +3,23 @@ on: | |
| workflow_dispatch: | ||
| schedule: | ||
| # Each Tuesday at 5 AM UTC | ||
| - cron: '0 5 * * 2' | ||
| - cron: "0 5 * * 2" | ||
|
|
||
| jobs: | ||
| merge_bump_packages_pr: | ||
| name: Merge bump packages PR | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checkout seems unnecessary here as we don't use anything from the repo |
||
| - name: Create Github App Token | ||
| uses: actions/create-github-app-token@v1 | ||
nirinchev marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| id: app-token | ||
| with: | ||
| app-id: ${{ vars.DEVTOOLS_BOT_APP_ID }} | ||
| private-key: ${{ secrets.DEVTOOLS_BOT_PRIVATE_KEY }} | ||
|
|
||
| - name: Merge PR | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.SVC_DEVTOOLSBOT_TOKEN }} | ||
| GITHUB_TOKEN: ${{ steps.app-token.outputs.token }} | ||
| run: | | ||
| set -e | ||
| PR_NUMBER=$(gh pr list -s open --head=ci/bump-packages --limit=1 --json number | jq '.[0].number') | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,44 +11,40 @@ jobs: | |
| name: Update Electron | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - name: Create Github App Token | ||
| uses: mongodb-js/devtools-shared/actions/setup-bot-token@ni/github-app-action | ||
| id: app-token | ||
| with: | ||
| app-id: ${{ vars.DEVTOOLS_BOT_APP_ID }} | ||
| private-key: ${{ secrets.DEVTOOLS_BOT_PRIVATE_KEY }} | ||
|
|
||
| - uses: actions/checkout@v4 | ||
| with: | ||
| # don't checkout a detatched HEAD | ||
| ref: ${{ github.head_ref }} | ||
|
|
||
| # this is important so git log can pick up on | ||
| # the whole history to generate the list of AUTHORS | ||
| fetch-depth: '0' | ||
|
|
||
| - name: Setup git | ||
| run: | | ||
| git config --local user.email "41898282+github-actions[bot]@users.noreply.github.com" | ||
| git config --local user.name "github-actions[bot]" | ||
| - uses: actions/setup-node@v3 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20.16.0 | ||
| cache: 'npm' | ||
| cache: "npm" | ||
|
|
||
| - name: Install [email protected] | ||
| run: | | ||
| npm install -g [email protected] | ||
|
|
||
| - name: Install Dependencies | ||
| run: | | ||
| npm -v | ||
| npm ci | ||
| run: npm ci | ||
|
|
||
| - name: Bump packages | ||
| run: | | ||
| node scripts/update-electron.js | ||
| git add . | ||
| git commit --no-allow-empty -m "chore(deps): update electron" || true | ||
|
Comment on lines
-42
to
-43
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The create-pull-request action will automatically add and commit, so no need to do it here |
||
| run: node scripts/update-electron.js | ||
|
|
||
| - name: Create Pull Request | ||
| id: cpr | ||
| uses: peter-evans/create-pull-request@v6 | ||
| uses: peter-evans/create-pull-request@5e914681df9dc83aa4e4905692ca88beb2f9e91f # 7.0.5 | ||
| with: | ||
| token: ${{ secrets.SVC_DEVTOOLSBOT_TOKEN }} | ||
| commit-message: 'chore(deps): update electron' | ||
| token: ${{ steps.app-token.outputs.token }} | ||
| commit-message: "chore(deps): update electron" | ||
| branch: ci/update-electron | ||
| title: 'chore(deps): update electron' | ||
| title: "chore(deps): update electron" | ||
| labels: no-title-validation | ||
| body: | | ||
| - Update electron | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,7 +97,7 @@ In particular each change to the `main` branch is analyzed to calculate a new ve | |
|
|
||
| Merging that PR will trigger another CI job that will publish to NPM any package which version is not yet present on the registry. | ||
|
|
||
| The version of packages is calculated following conventional bumps: See https://github.com/mongodb-js/devtools-shared/tree/main/packages/bump-monorepo-packages for details. | ||
| The version of packages is calculated following conventional bumps: See https://github.com/mongodb-js/devtools-shared/tree/main/packages/monorepo-tools for details. | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a drive-by - noticed the link is outdated |
||
|
|
||
| ## Add / Update / Remove Dependencies in Packages | ||
|
|
||
|
|
||
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 removed steps in this file have been moved to our
setup-bot-tokenaction.