Skip to content

Comments

chore(ci): address lint findings in build-docker.yml#15245

Merged
zanieb merged 1 commit intomainfrom
ww/build-docker-lint
Aug 12, 2025
Merged

chore(ci): address lint findings in build-docker.yml#15245
zanieb merged 1 commit intomainfrom
ww/build-docker-lint

Conversation

@woodruffw
Copy link
Member

@woodruffw woodruffw commented Aug 12, 2025

Summary

This re-creates #15145, with fixes following the revert in #15174.

The overall approach is the same, except that I've added an explicit permissions block to docker-annotate-base that should cover the needed permissions in that job.

(One confusion is around how that wasn't failing before -- FWICT it was receiving the default GITHUB_TOKEN, which doesn't include id-token: write or packages: write. So it should have been failing even before I explicitly did permissions: {}...)

Edit: Oh, I see why -- the actual release process does a workflow_call, so this inherits its GITHUB_TOKEN from release.yml:custom-build-docker, which in turn has the right permissions granted to it.

Test Plan

See what happens in CI. Plus maybe we could do a release dry-run?

@woodruffw woodruffw requested a review from zanieb August 12, 2025 18:10
@woodruffw woodruffw self-assigned this Aug 12, 2025
@woodruffw woodruffw added the internal A refactor or improvement that is not user-facing label Aug 12, 2025
@woodruffw woodruffw temporarily deployed to uv-test-registries August 12, 2025 18:11 — with GitHub Actions Inactive
@zanieb
Copy link
Member

zanieb commented Aug 12, 2025

Just fyi we don't really have a release dry-run option here. We'll run the Docker builds in the pull request, but we can't test annotating publishes without publishing.

@woodruffw
Copy link
Member Author

Just fyi we don't really have a release dry-run option here. We'll run the Docker builds in the pull request, but we can't test annotating publishes without publishing.

Gotcha, OK -- in that case I don't have a great way to test my reusable workflow permissions theory 😅. However, I cross-checked that the permissions are consistent here:

permissions:
"attestations": "write"
"contents": "read"
"id-token": "write"
"packages": "write"

So I think this is good to go.

@zanieb zanieb merged commit 65b4dbc into main Aug 12, 2025
122 checks passed
@zanieb zanieb deleted the ww/build-docker-lint branch August 12, 2025 18:59
zanieb added a commit that referenced this pull request Aug 13, 2025
The change in #15245 added a
variable to the step scope that's shadowed by the inner loop.

Closes #15262
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal A refactor or improvement that is not user-facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants