-
Notifications
You must be signed in to change notification settings - Fork 284
Harden build actions #656
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?
Harden build actions #656
Conversation
| with: | ||
| # https://github.com/docker/setup-qemu-action/issues/188#issuecomment-2604322104 | ||
| image: tonistiigi/binfmt:qemu-v8.1.5 | ||
| image: tonistiigi/binfmt:qemu-v8.1.5@sha256:2d2918e86e5327d0661f7083d67a95280b0f7be8f77ed79a8418f81d7d90ce6f |
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.
can we also add a renovate annotation here? also the issue in the comment seem to be resolved via qemu-v9.2.0 -> can be updated when renovate proposes the container update
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.
Excellent, the newest QEMU image seems to work. In that case, we can definitely add a renovate rule to this and the buildkit image.
| with: | ||
| persist-credentials: false |
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.
while this can be done, it achieves not much: The default GITHUB_TOKEN only has read access to public repos, is anyways only valid until end of the workflow run, all the other action steps anyways have access to that token no matter if the checkout step persists it or not.... not sure if its worth to have those two extra line in every workflow
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.
That's true for the default token, but I don't see any harm in not persisting it. If you think we should leave the default as is, I can revert this.
renovate.json
Outdated
| }, | ||
| { | ||
| "customType": "regex", | ||
| "fileMatch": [ | ||
| "^\\.github/workflows/[^/]+\\.ya?ml$" | ||
| ], | ||
| "matchStrings": [ | ||
| "#\\s*renovate:\\s*datasource=(?<datasource>.*?)\\s+depName=(?<depName>.*?)\\s+version:\\s*(?<currentValue>.*?)\\s" | ||
| ] | ||
| }, | ||
| { | ||
| "customType": "regex", | ||
| "fileMatch": [ | ||
| "^\\.github/workflows/[^/]+\\.ya?ml$" | ||
| ], | ||
| "matchStrings": [ | ||
| "image[:|=]\\s*(?<depName>.*?):(?<currentValue>.*?)@(?<currentDigest>.*?)" | ||
| ], | ||
| "datasourceTemplate": "docker" | ||
| } | ||
| ], | ||
| "packageRules": [ | ||
| { | ||
| "matchDatasources": ["docker"], | ||
| "matchPackageNames": ["tonistiigi/binfmt"], | ||
| "versioning": "regex:^qemu-v(?<major>\\d+)\\.(?<minor>\\d+)\\.(?<patch>\\d+)$" |
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 find reviewing and maintaining regex code always a bit risky... Just thinking a bit more here:
- There is https://docs.renovatebot.com/presets-customManagers/#custommanagersgithubactionsversions
- We have that activated here https://github.com/anaconda/renovate-config/blob/main/default.json#L11
so we can have this on job/workflow level without further configuring / creating new regexes:
env:
# renovate: datasource=github-releases depName=docker/buildx
BUILDX_VERSION: 'v0.24.0'
and use that via:
with:
version: ${{ env.BUILDX_VERSION }}
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 did this for the Docker images, too, so that we do not need any additional regex managers.
jezdez
left a comment
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 am not sure why we put those versions in env vars, can't we just keep this inline?
See this thread: #656 (comment) Using environment variables this way makes for much simpler renovate integration since it supports the |
Harden the GitHub Actions that build the Docker images by