[ci][publisher] Fix docker image push#600
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe pipeline's publish job was updated to use registry-image-style parameters when putting to github-container-registry-...-stemcell: it now supplies image: stemcell-image/image, additional_tags: candidate-.../.resource/version, and bump_aliases. The corresponding resource declaration was changed from type: docker-image to type: registry-image. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci/pipelines/publisher.yml (1)
839-844: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider adding explicit
tagin resource source.The resource source lacks a
tagfield. Without it,registry-imagedefaults tolatest. If version-specific tags are needed (as implied by the oldtag_fileusage), this needs to be addressed in conjunction with the put step params.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/pipelines/publisher.yml` around lines 839 - 844, The resource definition github-container-registry-(@= data.values.stemcell_details.os_name @)-stemcell currently omits an explicit tag and therefore defaults to latest; update its source to include a deterministic tag (e.g., tag: ((stemcell_tag)) or derive from the previous tag_file mechanism) and ensure the corresponding put step that pushes images uses the same tag via params (or tag_file) so the pipeline consistently references versioned images rather than latest; modify the repository/source block to add the tag key and wire a variable or file-based value used by the put step for publishing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/pipelines/publisher.yml`:
- Around line 484-487: The pipeline is incorrectly passing a file path as the
registry-image `version` tag (in the `put: github-container-registry-(@=
data.values.stemcell_details.os_name @)-stemcell` params), so change the
`params` to read the tag file into `additional_tags` (whitespace-separated)
instead of using `version: candidate-.../.resource/version`, and enable
`bump_aliases: true` (or ensure `latest` is included in the tags file) so
aliases are applied; update `params.image` and remove the file path from
`version` to ensure the image tag(s) are applied correctly.
---
Outside diff comments:
In `@ci/pipelines/publisher.yml`:
- Around line 839-844: The resource definition github-container-registry-(@=
data.values.stemcell_details.os_name @)-stemcell currently omits an explicit tag
and therefore defaults to latest; update its source to include a deterministic
tag (e.g., tag: ((stemcell_tag)) or derive from the previous tag_file mechanism)
and ensure the corresponding put step that pushes images uses the same tag via
params (or tag_file) so the pipeline consistently references versioned images
rather than latest; modify the repository/source block to add the tag key and
wire a variable or file-based value used by the put step for publishing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 31b3766b-459e-4fd1-aa20-0b5b5aa239d4
📒 Files selected for processing (1)
ci/pipelines/publisher.yml
|
Actionable comments posted: 0 |
benjaminguttmann-avtq
changed the title
|
Cannot test if it works as expected though |
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
cat ci/pipelines/publisher.yml 2>/dev/null || fd -e yml publisher ci/pipelines/ | xargs catLength of output: 35757
1. Where does
|
| Tag | Source |
|---|---|
latest |
Default behavior (no tag in source) |
<version> (e.g. 1.406) |
additional_tags → .resource/version file |
1 (and other semver aliases) |
bump_aliases: true |
Minor thing to double-check
The build-stemcell-image task uses CONTEXT: stemcell-image/image but the extract-warden-image task extracts into stemcell-image/ with dir: stemcell-image. Make sure the extracted content puts a valid Dockerfile (or that oci-build-task is configured correctly for a context without one — it may need DOCKERFILE to be set or the directory to contain a Dockerfile). That's the most likely runtime failure point.
The pipeline logic otherwise looks correct to me. Since you can't test it live, you could do a dry run by temporarily pointing the resource at a test GHCR repo with less restrictive credentials.
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/pipelines/publisher.yml`:
- Around line 500-504: The publisher step for the resource
github-container-registry-(@= data.values.stemcell_details.os_name @)-stemcell
is missing an explicit tag and a version param, so `latest` won't be pushed and
`bump_aliases` is ineffective; update the resource `source` for
github-container-registry-(@= data.values.stemcell_details.os_name @)-stemcell
to include `tag: latest` (or ensure `latest` is present in the produced tags
file) and modify the `put` params (alongside `additional_tags` and
`bump_aliases: true`) to supply a concrete `version:` value (derived from your
.resource/version content or computed semver) so alias bumping works and
`latest` is published.
- Around line 484-499: The build step is using an invalid file as the OCI build
context and aliasing is broken: modify the extract step (e.g., in the task that
runs under extract-warden-image) to create a directory context containing a
Dockerfile (e.g., write "FROM scratch\nADD image /" after extracting the tarball
so you have a directory like stemcell-image/ with Dockerfile and the extracted
"image" file), then update the build-stemcell-image task to set CONTEXT to that
directory (CONTEXT: stemcell-image) instead of the single file; also fix the
github-container-registry-...-stemcell resource/task by providing a concrete
version/source.tag (or remove bump_aliases: true) so bump_aliases has a version
to derive aliases from, and confirm additional_tags are complemented by the
version/tag to produce the desired latest/alias tags.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ce7b1d8-8253-45e7-9384-01b886a45418
📒 Files selected for processing (1)
ci/pipelines/publisher.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/pipelines/publisher.yml`:
- Around line 484-487: The pipeline is passing a raw rootfs tar
(stemcell-image/image) directly to the registry-image `put:
github-container-registry-(@= data.values.stemcell_details.os_name @)-stemcell`
which expects a Docker/OCI image tar or layout; add an explicit conversion/build
task between the extract step and this `put` so `image: stemcell-image/image`
becomes a valid Docker/OCI artifact (examples: run a task that uses `docker
import` then `docker save` to produce an image tar, or use an `oci-build-task`
to convert the rootfs into an OCI image layout), and update the `put` to point
to the resulting image artifact instead (refer to the `image` param in the `put:
github-container-registry-(@= data.values.stemcell_details.os_name @)-stemcell`
resource and the extracted `stemcell-image/image` source).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e63e7cf9-8bf8-4276-b31f-0c492fde0403
📒 Files selected for processing (1)
ci/pipelines/publisher.yml
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ci/pipelines/publisher.yml (2)
839-845: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
tag: latestaddition correctly enableslatesttag, but semantic version aliases require additional configuration.Adding
tag: latestto the resource source ensures the primary tag will belatest. Combined withadditional_tagsin the put step (line 487), the image will be tagged with bothlatestand the explicit version from.resource/version.However, if you need automatic semantic version aliases (e.g.,
1.x,1.2.xin addition to1.2.3), you must also addbump_aliases: trueand provide aversion:parameter in the put step's params (lines 485-487). Thebump_aliasesfeature compares the provided version against existing registry tags to determine which aliases to update.Example: Enable semantic version aliasing
If semantic version aliases are desired, add to the put params:
- put: github-container-registry-(@= data.values.stemcell_details.os_name @)-stemcell params: image: stemcell-image/image additional_tags: candidate-(@= data.values.stemcell_details.os_name @)-stemcell-(@= str(data.values.stemcell_details.major_version) @)/.resource/version + version: ((read the version from .resource/version file or compute)) + bump_aliases: trueNote: You'll need to read the version value from the file to supply as the
versionparameter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/pipelines/publisher.yml` around lines 839 - 845, The resource declaration now sets tag: latest for github-container-registry-(@= data.values.stemcell_details.os_name @)-stemcell, but to enable semantic version aliases you must update the corresponding put step (the job that uses this resource and already sets additional_tags) to include bump_aliases: true and pass a version: param (read from your version file) in the put params so the pipeline can compute aliases like 1.x and 1.2.x; locate the put step that references this resource and add bump_aliases: true under params and set version: to the file-derived value.
484-487:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftFix
registry-imageput:image: stemcell-image/imageis likely a raw rootfs tarballThe pipeline extracts an
imagefile from the warden-boshlite stemcell tarball and passes it directly to the nextputasparams.image: stemcell-image/image, with no intermediate Docker/OCI conversion step (nodocker save/import/load,skopeo,oci-build, etc. present between the extract and the put). This will typically fail ifregistry-imageexpects a Docker/OCI image tarball or layout rather than a bare root filesystem archive.Add a conversion/build step to produce a valid Docker image tarball, OCI image tarball, or OCI image layout before the
put.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/pipelines/publisher.yml` around lines 484 - 487, The pipeline currently passes the extracted rootfs file directly to the registry `put` (put: github-container-registry-(@= data.values.stemcell_details.os_name @)-stemcell with params.image: stemcell-image/image), but registry-image expects a Docker/OCI image tarball or layout; add an intermediate conversion/build step that takes the extracted stemcell rootfs (stemcell-image/image) and produces a valid Docker/OCI image tarball or OCI layout (for example using docker import + docker save, buildah bud/save, or skopeo copy to an OCI layout) and then point the `put`'s params.image to that produced artifact instead of the raw rootfs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ci/pipelines/publisher.yml`:
- Around line 839-845: The resource declaration now sets tag: latest for
github-container-registry-(@= data.values.stemcell_details.os_name @)-stemcell,
but to enable semantic version aliases you must update the corresponding put
step (the job that uses this resource and already sets additional_tags) to
include bump_aliases: true and pass a version: param (read from your version
file) in the put params so the pipeline can compute aliases like 1.x and 1.2.x;
locate the put step that references this resource and add bump_aliases: true
under params and set version: to the file-derived value.
- Around line 484-487: The pipeline currently passes the extracted rootfs file
directly to the registry `put` (put: github-container-registry-(@=
data.values.stemcell_details.os_name @)-stemcell with params.image:
stemcell-image/image), but registry-image expects a Docker/OCI image tarball or
layout; add an intermediate conversion/build step that takes the extracted
stemcell rootfs (stemcell-image/image) and produces a valid Docker/OCI image
tarball or OCI layout (for example using docker import + docker save, buildah
bud/save, or skopeo copy to an OCI layout) and then point the `put`'s
params.image to that produced artifact instead of the raw rootfs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c752fe61-808d-46df-b947-a53ae33a94fb
📒 Files selected for processing (1)
ci/pipelines/publisher.yml
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
aramprice
dismissed stale reviews from coderabbitai[bot] and themself
via
da0abf8
aramprice
force-pushed
the
fix-docker-image-push-publisher
branch
from
5529bc3 to
da0abf8
Compare
There was a problem hiding this comment.
Pull request overview
Migrates the publisher pipeline from the deprecated docker-image resource type to registry-image for pushing stemcell container images to GHCR. Because registry-image cannot ingest a raw rootfs tarball (the way docker-image's import_file did), the pipeline now extends the warden-image extraction step to also generate a Dockerfile, adds a new task that uses concourse/oci-build-task to produce an OCI image tarball, and pushes that artifact via the new resource.
Changes:
- Switch
github-container-registry-*-stemcellresource fromdocker-imagetoregistry-image(withtag: latest). - Generate a
Dockerfilealongside the extracted rootfs inextract-warden-image. - Add
build-stemcell-oci-imagetask and update theputto useimage/additional_tagsinstead ofimport_file/tag_file/tag_as_latest.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aramprice
force-pushed
the
fix-docker-image-push-publisher
branch
from
da0abf8 to
90ccd41
Compare
- make all stemcell `put` actions parallel
aramprice
force-pushed
the
fix-docker-image-push-publisher
branch
from
90ccd41 to
c4d28cc
Compare
|
reworked, commits squashed, flown and ran successfully: ^ does not have "parallel put" change |
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Foundational Infrastructure Working Group
NOTE: this repository uses a "Merge Forward" strategy
Changes should be made in the earliest applicable branch, and
merged forward through subsequent branches.
ubuntu-<short_name>)merge-to-<next_short_name>branchubuntu-<short_name>intomerge-to-<next_short_name>merge-to-<next_short_name>intoubuntu-<next_short_name>Not sure yet where to get the version from, as registry-image does not expect a file to provide it + how to ensure it gets tagged with latest