ADR 0070: Support for zstd:chunked image compression - #357
Conversation
ReviewFindingsHigh
Medium
Low
Previous runReviewFindingsHigh
Medium
Low
Previous run (2)ReviewFindingsMedium
Low
Labels: New ADR proposing a feature capability (dual-compression for container builds). Previous run (3)ReviewFindingsMedium
Low
Info
Previous run (4)ReviewFindingsMedium
Low
Info
Previous run (5)ReviewFindingsCritical
High
Medium
Low
Info
Previous run (6)ReviewFindingsMedium
Low
Previous run (7)ReviewFindingsLow
Info
Previous run (8)ReviewFindingsCritical
Low
Previous run (9)ReviewFindingsLow
Info
Previous run (10)ReviewFindingsInfo
Previous run (11)ReviewNo findings. Previous run (12)ReviewFindingsInfo
No blocking findings. The ADR is well-structured, follows existing conventions, and thoroughly covers the decision rationale, implementation details, alternatives considered, and consequences. Previous run (13)ReviewFindingsLow
Info
Previous run (14)Review: #357Head SHA: e9b9472 SummaryThis PR adds ADR 0067 proposing dual-compression (gzip + zstd:chunked) for container image builds and updates the ADR index in AGENTS.md. The ADR is well-structured, technically coherent, and follows the repository's conventions. The AGENTS.md line/count updates are arithmetically correct (64→65 ADRs, 10430→10509 lines). No security, injection, or correctness concerns were found. Two minor style observations are noted below but do not warrant blocking. FindingsMedium / Low / Info
FooterOutcome: approve Previous run (15)Review: #357Head SHA: 55018a9 SummaryThis PR adds ADR 0067 proposing dual-compression (gzip + zstd:chunked) for Konflux container builds, along with the corresponding AGENTS.md index update. The ADR is well-structured, technically detailed, and follows the established ADR conventions. The AGENTS.md arithmetic is correct (line count 10430→10509 matches the 79-line ADR; entry count 64→65). No correctness, security, or injection concerns were found. One minor style note below. FindingsInfo
FooterOutcome: approve Previous run (16)Review: #357Head SHA: 0ac8063 SummaryThis PR adds ADR 0067 proposing dual-compression (gzip + zstd:chunked) for container builds and updates the ADR index in AGENTS.md. The ADR is well-structured, technically sound, and follows the repository's conventions. The AGENTS.md line/count updates are arithmetically correct. No security, correctness, or intent-alignment concerns were found. FindingsCriticalNone. HighNone. MediumNone. Low
Info
FooterOutcome: approve Previous run (17)Review: #357Head SHA: 7ac20ef SummaryThis PR adds ADR 0067, a well-structured proposal for dual-compression (gzip + zstd:chunked) in container builds. The ADR follows the repository's established template and conventions, provides clear context with references to upstream specs and issues, documents implementation details with rollout gating, and considers two alternatives with sound rationale for rejecting them. No correctness, security, or injection concerns apply — this is a documentation-only change. The content is technically coherent and appropriately scoped for an architecture decision record. FindingsInfo
FooterOutcome: approve Previous run (18)Review: #357Head SHA: e4778ed SummaryThis PR adds a new ADR (0067) proposing dual-compression (gzip + zstd:chunked) for container builds. The ADR is well-structured, technically detailed, and follows existing conventions. No critical or high findings. A few minor style and consistency observations are noted below. FindingsCriticalNone. HighNone. MediumNone. Low
Info
FooterOutcome: comment-only Previous run (19)Review: #357Head SHA: 7016222 SummaryThis PR adds a new ADR (0067) proposing dual-compression (gzip + zstd:chunked) for container image builds. The document is well-structured, follows the established ADR template and conventions, correctly identifies backward-compatibility risks with older Docker versions, and thoroughly covers the implementation plan including release pipeline impacts. No blocking findings were identified across any review dimension. FindingsInfo
FooterOutcome: approve Previous run (20)Review: #357Head SHA: c542bce SummaryThis PR adds ADR 0067, proposing dual-compression (gzip + zstd:chunked) for container builds. The ADR is well-structured, follows the repository's ADR template and numbering conventions (sequential after 0066), and presents a technically sound proposal. The context section clearly motivates the change with backward-compatibility concerns, the decision section provides concrete implementation details including new task parameters and release pipeline impacts, and the alternatives section explains why other approaches were rejected. No security, correctness, or style issues were found. FindingsInfo
FooterOutcome: approve |
c542bce to
7016222
Compare
|
How will all of this be represented in the SBOM? Currently, each architecture gets an SBOM where the arch-specific manifest is the main component, and the index gets an SBOM pointing to each arch-specific manifest. Example: # index SBOM
$ cosign download sbom quay.io/konflux-ci/task-runner:1.6.0
# x86_64 manifest SBOM
$ cosign download sbom quay.io/konflux-ci/task-runner@sha256:b4b46f35d697dd7cbcd77b051ceac41d324164edcf85d39379b7b53f492d8394If we push two different images for each arch, which one will be the main component in the SBOM? How will the other one be represented? Will we need to attach the SBOM to both of them? If we push a single index with two manifests for each arch, can that be the main component? Is it OK if the SBOM doesn't reference the individual manifests? Or can we represent them as sub-components of the main component (in both CycloneDX and SPDX)? Will we need to attach the SBOM to the index and both the manifests, or just the index? |
e4778ed to
7ac20ef
Compare
7ac20ef to
0ac8063
Compare
55018a9 to
e9b9472
Compare
e9b9472 to
6646549
Compare
6646549 to
efefc14
Compare
|
@Amanyd when making changes that address some open threads, would you be able to respond in those threads? |
As for the per-arch index now, the SBOM will just attach directly to the index digest, because both variants only differ in compression, one SBOM on the index is all we need, we don't have to generate separate SBOMs for each variant. |
|
🤖 Finished Review · ✅ Success · Started 3:11 PM UTC · Completed 3:27 PM UTC |
chmeliik
left a comment
There was a problem hiding this comment.
LGTM from my side, good job!
b228ca2 to
8c489e9
Compare
|
🤖 Finished Review · ✅ Success · Started 11:25 PM UTC · Completed 11:42 PM UTC |
|
Oh, small thing. Needs to run |
|
🤖 Review · |
|
🤖 Finished Review · ✅ Success · Started 4:02 PM UTC · Completed 4:17 PM UTC |
|
🤖 Finished Retro · ✅ Success · Started 5:11 PM UTC · Completed 5:18 PM UTC |
Retro: PR #357 — ADR 0070: Support for zstd:chunked image compressionThis was a human-authored PR (branch Timeline highlights
Review quality assessmentBot strengths: ADR number collision detection (caught twice, genuinely valuable and blocking). Line count validation. Style/grammar nits. Bot gaps vs. human review: Human reviewers drove ALL substantive architectural discourse:
The review agent could not reason about cross-service trust models, pipeline data flow, or Tekton runtime constraints — the domain knowledge gap is fundamental for this type of architectural document. Token cost9 review runs total (7 completed, 1 cancelled, 1 retro). Commit Existing issue coverageThe issues observed on this PR are comprehensively covered by existing open issues in
No new proposals are warranted — the improvement areas are well-tracked. Resolving #963, #1013, and #1659 would have the highest impact on reducing the waste patterns observed on this PR. |
Draft ADR 0070 proposing dual-compression for container builds.