Conversation
📝 WalkthroughWalkthroughAdds CI and packaging to produce multi-architecture OCI images: a reusable container workflow, two Dockerfiles (CI/local), main build pipeline integration (artifact upload and container job), and README installation instructions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Build as build-code job
participant Artifact as Artifact Store
participant ContainerWF as container workflow
participant Buildx as Buildx/QEMU
participant Registry as Container Registry
participant Attest as Attestation Service
Dev->>GH: push / open PR
GH->>Build: run build-code job
Build->>Artifact: upload metaschema-cli ZIP (build_zip)
GH->>ContainerWF: invoke build-container (needs: build-code, push: true)
ContainerWF->>Buildx: setup QEMU & Buildx (multi-arch)
ContainerWF->>Artifact: download ZIP into build context
ContainerWF->>Buildx: build image for linux/amd64 & linux/arm64/v8
alt push == true
ContainerWF->>Registry: authenticate & push image + tags
Registry-->>ContainerWF: return image digest
ContainerWF->>Attest: optionally push attestations with digest
else push == false
Buildx-->>ContainerWF: produce local image and metadata only
end
ContainerWF->>GH: attach annotations / metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
aj-stein
force-pushed
the
561-publish-oci-image
branch
from
d7dab9e to
60e8baa
Compare
![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
♻️ Duplicate comments (1)
Dockerfile.local (1)
40-43: Apply the same user naming and /app ownership improvements as Dockerfile.ci.This runner stage has the same user naming inconsistency and
/appownership issue as flagged inDockerfile.ci(lines 14-17). Please apply the same refactor here to ensure consistency between both Dockerfiles.
🧹 Nitpick comments (4)
Dockerfile.ci (1)
14-17: Consider renaming the user to align with the CLI name and ensure proper /app ownership.The user and group are named
oscalcli, but this container is formetaschema-cli. While not breaking, consider renaming tometaschemaormetaschema-clifor clarity.Additionally, the
/appdirectory is created by root but the container runs asoscalcli. If users need to write files to the working directory at runtime, they'll encounter permission errors. Consider addingchown -R oscalcli:oscalcli /appafter creating the directory.🔎 Proposed refactor
-RUN groupadd -r oscalcli && \ - useradd -r -g oscalcli -s /bin/false oscalcli && \ - chown -R oscalcli:oscalcli /opt/metaschema-cli && \ - mkdir -p /app +RUN groupadd -r metaschema && \ + useradd -r -g metaschema -s /bin/false metaschema && \ + chown -R metaschema:metaschema /opt/metaschema-cli && \ + mkdir -p /app && \ + chown -R metaschema:metaschema /app WORKDIR /app -USER oscalcli +USER metaschemaDockerfile.local (2)
9-9: Prefer COPY over ADD for local source files.Docker best practice recommends using
COPYinstead ofADDfor local files.ADDhas implicit behavior (URL fetching, tar extraction) that isn't needed here and can lead to unexpected results.🔎 Proposed refactor
-ADD . /usr/local/src +COPY . /usr/local/src
31-36: Consider simplifying the zip extraction logic.The current approach uses
findwith-execto copy the zip, then unzips it in a separate step. This could be streamlined for better readability.🔎 Proposed refactor
Option 1 - Direct unzip from Maven target:
-RUN find /usr/local/src/target \ - -iname '*metaschema-cli.zip' \ - -exec cp {} /tmp/metaschema-cli.zip \; RUN mkdir -p /opt/metaschema-cli -RUN unzip /tmp/metaschema-cli.zip -d /opt/metaschema-cli +RUN unzip /usr/local/src/target/*metaschema-cli.zip -d /opt/metaschema-cli RUN chmod +x /opt/metaschema-cli/bin/metaschema-cliOption 2 - Combine into single RUN to reduce layers:
-RUN find /usr/local/src/target \ - -iname '*metaschema-cli.zip' \ - -exec cp {} /tmp/metaschema-cli.zip \; -RUN mkdir -p /opt/metaschema-cli -RUN unzip /tmp/metaschema-cli.zip -d /opt/metaschema-cli -RUN chmod +x /opt/metaschema-cli/bin/metaschema-cli +RUN mkdir -p /opt/metaschema-cli && \ + unzip /usr/local/src/target/*metaschema-cli.zip -d /opt/metaschema-cli && \ + chmod +x /opt/metaschema-cli/bin/metaschema-cli.github/workflows/container.yml (1)
73-85: Add a non-push build step to validate Dockerfile in PRs.Currently, when
inputs.pushisfalse, no image is built at all. This means Dockerfile errors won't be caught during PR validation. Consider adding an unconditional build step that validates the Dockerfile without pushing, followed by the conditional push step.🔎 Proposed refactor
+ - name: Container image build (validation) + id: image_build_validation + if: ${{ !inputs.push }} + uses: docker/build-push-action@f2a1d5e99d037542a71f64918e516c093c6f3fc4 + with: + context: . + file: ./Dockerfile.ci + push: false + tags: ${{ steps.image_metadata.outputs.tags }} + labels: ${{ steps.image_metadata.outputs.labels }} + platforms: linux/amd64,linux/arm64/v8 + cache-from: type=gha + cache-to: type=gha,mode=max - if: inputs.push name: Container image registry push id: image_registry_push
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/container.yml(1 hunks)Dockerfile.ci(1 hunks)Dockerfile.local(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: david-waltermire
Repo: metaschema-framework/liboscal-java PR: 222
File: .github/workflows/build.yml:30-32
Timestamp: 2025-12-13T15:29:03.112Z
Learning: In the liboscal-java repository, nightly builds are managed independently using GitHub Actions schedule triggers (cron), not via repository_dispatch from metaschema-java. Each repo in the metaschema-framework manages its own nightly builds.
🪛 Hadolint (2.14.0)
Dockerfile.local
[error] 30-30: unexpected 'R'
expecting a new line followed by the next instruction
(DL1000)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Website
- GitHub Check: Code
🔇 Additional comments (4)
Dockerfile.local (1)
30-30: Static analysis hint can be safely ignored.Hadolint reports an error at this line, but the
RUN mvncommand is syntactically correct. This appears to be a false positive, likely caused by the heredoc syntax in the precedingRUNstatement (lines 12-29). The build should execute correctly..github/workflows/container.yml (3)
57-63: Note: Latest tag only applies to main branch.The workflow correctly configures
latestto only tag images from themainbranch. Since this PR targetsdevelop, images built from this branch will not receive thelatesttag. This is appropriate for the standard Git flow model.
27-92: Excellent security practice: All actions pinned to commit SHAs.The workflow correctly pins all GitHub Actions to specific commit SHA hashes rather than version tags. This prevents supply chain attacks where tags could be moved to malicious code and ensures reproducible builds.
31-35: Verify that the build_zip artifact is produced by the calling workflow.This workflow depends on a pre-built artifact named
build_zipfrom an upstream workflow. Ensure that the workflow triggering this step produces this artifact and that the download step properly references it with arun-idif downloading from a different workflow run.
|
The latest here looks great! Very clean. Would it be useful to include instructions for using the container in the README.md? |
aj-stein
force-pushed
the
561-publish-oci-image
branch
from
3c32db9 to
b593f1a
Compare
Alright, @david-waltermire, docs added and ready for review. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
README.md (1)
50-53: Optional: Vary sentence structure in the list.Three consecutive list items begin with "Download," which is stylistically repetitive. Consider rewording one for variety.
🔎 Suggested rewording
1. Build the project locally with Maven. 1. Download a developer snapshot from the Metaschema Framework repository for pre-release builds. 1. Download the official releases from the official Maven Central. -1. Download an OCI-conformant container image from the [GitHub Container Registry (ghcr.io)](https://ghcr.io) for use with `docker`, `podman`, Kubernetes, or other container orchestration platforms. +1. Pull an OCI-conformant container image from the [GitHub Container Registry (ghcr.io)](https://ghcr.io) for use with `docker`, `podman`, Kubernetes, or other container orchestration platforms.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/build.yml.github/workflows/container.ymlDockerfile.ciDockerfile.localREADME.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/container.yml
- Dockerfile.ci
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: david-waltermire
Repo: metaschema-framework/liboscal-java PR: 222
File: .github/workflows/build.yml:30-32
Timestamp: 2025-12-13T15:29:03.112Z
Learning: In the liboscal-java repository, nightly builds are managed independently using GitHub Actions schedule triggers (cron), not via repository_dispatch from metaschema-java. Each repo in the metaschema-framework manages its own nightly builds.
🪛 Hadolint (2.14.0)
Dockerfile.local
[error] 30-30: unexpected 'R'
expecting a new line followed by the next instruction
(DL1000)
🪛 LanguageTool
README.md
[style] ~53-~53: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ses from the official Maven Central. 1. Download an OCI-conformant container image from ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Website
- GitHub Check: Code
🔇 Additional comments (6)
README.md (1)
96-103: LGTM! Clear and practical container usage examples.The container installation documentation is concise and provides both pull and run examples, making it easy for users to get started with the OCI image.
Dockerfile.local (4)
1-4: LGTM! Good use of ARGs for flexibility.The Dockerfile uses build arguments effectively, allowing users to customize base images while providing sensible defaults.
30-30: Hadolint error is a false positive.The static analysis tool flagged this line with error DL1000 ("unexpected 'R'"), but the
RUNcommand syntax is correct. This is a false positive from Hadolint and can be safely ignored.
5-37: LGTM! Builder stage is well-structured.The builder stage effectively:
- Configures Maven toolchains for reproducible builds
- Builds the project with the release profile
- Locates and extracts the CLI artifact using case-insensitive search (good for robustness)
- Sets proper permissions
The multi-stage approach keeps the final image lean by separating build dependencies from runtime.
38-48: LGTM! Excellent security practices in runner stage.The runner stage demonstrates good security practices:
- Non-root user execution with dedicated group
- Disabled shell (
/bin/false) for the user- Verification step ensures the CLI works before the image is finalized
- Clean minimal runtime image
.github/workflows/build.yml (1)
108-112: LGTM! Artifact upload step is correctly configured.The build zip artifact upload step properly captures the metaschema-cli archive for downstream consumption by the container build job.
david-waltermire
left a comment
david-waltermire
left a comment
There was a problem hiding this comment.
Please address the coderabbit comments.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Dockerfile.local (1)
11-11: Consider cleaning apt cache to reduce image size.The builder stage doesn't clean up the apt cache after installing unzip, which increases the layer size unnecessarily.
🔎 Proposed fix
-RUN apt-get update && apt-get install -y unzip +RUN apt-get update && apt-get install -y unzip && rm -rf /var/lib/apt/lists/*
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build.yml.github/workflows/container.ymlDockerfile.ciDockerfile.local
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/container.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: david-waltermire
Repo: metaschema-framework/liboscal-java PR: 222
File: .github/workflows/build.yml:30-32
Timestamp: 2025-12-13T15:29:03.112Z
Learning: In the liboscal-java repository, nightly builds are managed independently using GitHub Actions schedule triggers (cron), not via repository_dispatch from metaschema-java. Each repo in the metaschema-framework manages its own nightly builds.
🪛 Hadolint (2.14.0)
Dockerfile.local
[error] 30-30: unexpected 'R'
expecting a new line followed by the next instruction
(DL1000)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Website
- GitHub Check: Code
🔇 Additional comments (11)
Dockerfile.local (7)
1-3: LGTM!The syntax directive and base image ARGs are well-defined with reasonable defaults.
12-29: LGTM!The Maven toolchains configuration using heredoc is well-structured and correctly substitutes the JDK parameters.
30-30: Dismiss the static analysis warning.The Hadolint error (DL1000) flagging "unexpected 'R'" appears to be a parser false positive. The Maven command syntax is valid.
31-36: Artifact extraction looks good.The find-and-copy pattern for the CLI zip followed by extraction and permission setup is appropriate. Based on past review discussions, the glob pattern requires minimal maintenance.
38-43: LGTM!The runner stage correctly sets up a non-root user ("metaschema") with appropriate security settings and ownership. The naming inconsistency from earlier reviews has been resolved.
44-45: LGTM!Switching to the non-root user before defining the entrypoint follows security best practices.
46-47: LGTM!Verifying the CLI installation with
--versionduring the build is excellent practice, and using exec-form ENTRYPOINT is the recommended approach.Dockerfile.ci (2)
1-10: Builder stage implementation is sound.The multi-stage build pattern is appropriate, and the builder stage correctly installs dependencies, extracts the CLI archive, and sets executable permissions. Error handling is implicit—both
unzipandchmodwill fail the build if the archive is malformed or paths are incorrect.The wildcard COPY pattern at line 7 has already been thoroughly reviewed in past comments.
18-21: LGTM: Working directory, user context, and entrypoint are properly configured.The smoke test at line 20 validates that the CLI binary is functional before the image is finalized, which is excellent defensive practice. Running it as the
metaschemauser ensures permission issues with the binary itself would be caught during the build..github/workflows/build.yml (2)
108-112: LGTM: Artifact upload correctly captures the CLI archive.The upload step properly captures the version-stamped CLI archive using a wildcard pattern. Unlike Docker's
COPY, theactions/upload-artifactaction will fail if no files match the pattern, providing explicit error feedback.
119-122: LGTM: Job dependency and workflow delegation are properly configured.The
needs: build-codedependency ensures the CLI artifact is available before container build, and the reusable workflow call withpush: truecorrectly enables registry publishing when appropriate.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (2)
92-92: Align glob pattern with snapshot installation method.Line 92 uses a quoted glob
"/tmp/*metaschema-cli.zip"which differs from the approach used in line 72 for the snapshot method (/tmp/metaschema-cli-*-metaschema-cli.zip). For consistency and to match the more specific pattern that reflects the actual artifact naming convention, consider using the unquoted, specific glob pattern here as well.🔎 Proposed fix
# Extract zip archive to /opt/metaschema-cli # You might need sudo for permission to write files to this path -sudo unzip "/tmp/*metaschema-cli.zip" -d /opt/metaschema-cli +sudo unzip /tmp/metaschema-cli-*-metaschema-cli.zip -d /opt/metaschema-cli # Now add this installation directory to the path export PATH="${PATH}:/opt/metaschema-cli"
50-53: Optional: Vary sentence structure in the list.Lines 51-53 each begin with "Download," which creates repetitive phrasing. While acceptable in a list format, you could improve readability by varying the structure.
🔎 Proposed alternative phrasing
1. Build the project locally with Maven. -1. Download a developer snapshot from the Metaschema Framework repository for pre-release builds. -1. Download the official releases from the official Maven Central. -1. Download an OCI-conformant container image from the [GitHub Container Registry (ghcr.io)](https://ghcr.io) for use with `docker`, `podman`, Kubernetes, or other container orchestration platforms. +1. Obtain a developer snapshot from the Metaschema Framework repository for pre-release builds. +1. Install an official release from Maven Central. +1. Pull an OCI-conformant container image from the [GitHub Container Registry (ghcr.io)](https://ghcr.io) for use with `docker`, `podman`, Kubernetes, or other container orchestration platforms.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/build.yml.github/workflows/container.ymlDockerfile.ciDockerfile.localREADME.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/build.yml
- .github/workflows/container.yml
🧰 Additional context used
🪛 Hadolint (2.14.0)
Dockerfile.local
[error] 30-30: unexpected 'R'
expecting a new line followed by the next instruction
(DL1000)
🪛 LanguageTool
README.md
[style] ~53-~53: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ses from the official Maven Central. 1. Download an OCI-conformant container image from ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Website
- GitHub Check: Code
🔇 Additional comments (3)
Dockerfile.ci (1)
1-22: LGTM! Past feedback addressed.The two-stage build is well-structured:
- Builder stage efficiently extracts the pre-built CLI artifact
- Runner stage properly creates a non-root user and assigns directory ownership
- The
/appdirectory ownership issue from past reviews has been addressed (lines 17-18)- The wildcard in
COPYat line 7 is necessary and acceptable given the versioned artifact naming- Non-root execution follows container security best practices
Dockerfile.local (2)
12-30: Hadolint false positive - Maven build configuration is correct.The Hadolint error flagging line 30 is a false positive. The heredoc syntax (lines 12-29) and subsequent
RUN mvncommand are valid Docker syntax. The toolchains.xml configuration properly specifies JDK vendor and version for Maven builds, and the build command correctly uses the-Preleaseprofile.
31-49: LGTM! Consistent with Dockerfile.ci approach.The artifact handling and runner stage are well-implemented:
- The
findcommand robustly locates the built artifact- Runner stage properly creates non-root user and assigns directory ownership
- Past feedback regarding user/group naming has been addressed ("metaschema" instead of "oscalcli")
- Implementation is consistent with Dockerfile.ci
Minor note: Line 44 appears to have trailing whitespace after
/app, which you may want to trim for consistency.
aj-stein
force-pushed
the
561-publish-oci-image
branch
from
c4d8f2c to
f14270f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
51-53: Consider varying the sentence structure for better readability.Three consecutive list items begin with "Download," which slightly impacts readability per the static analysis hint.
🔎 Suggested rewording
1. Build the project locally with Maven. -1. Download a developer snapshot from the Metaschema Framework repository for pre-release builds. -1. Download the official releases from the official Maven Central. -1. Download an OCI-conformant container image from the [GitHub Container Registry (ghcr.io)](https://ghcr.io) for use with `docker`, `podman`, Kubernetes, or other container orchestration platforms. +1. Obtain a developer snapshot from the Metaschema Framework repository for pre-release builds. +1. Retrieve the official releases from Maven Central. +1. Pull an OCI-conformant container image from the [GitHub Container Registry (ghcr.io)](https://ghcr.io) for use with `docker`, `podman`, Kubernetes, or other container orchestration platforms.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/build.yml.github/workflows/container.ymlDockerfile.ciDockerfile.localREADME.md
🚧 Files skipped from review as they are similar to previous changes (1)
- Dockerfile.ci
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: david-waltermire
Repo: metaschema-framework/liboscal-java PR: 222
File: .github/workflows/build.yml:30-32
Timestamp: 2025-12-13T15:29:03.112Z
Learning: In the liboscal-java repository, nightly builds are managed independently using GitHub Actions schedule triggers (cron), not via repository_dispatch from metaschema-java. Each repo in the metaschema-framework manages its own nightly builds.
🪛 Hadolint (2.14.0)
Dockerfile.local
[error] 30-30: unexpected 'R'
expecting a new line followed by the next instruction
(DL1000)
🪛 LanguageTool
README.md
[style] ~53-~53: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ses from the official Maven Central. 1. Download an OCI-conformant container image from ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Code
- GitHub Check: Website
🔇 Additional comments (7)
README.md (1)
99-107: LGTM!The container installation instructions are clear, accurate, and align well with the container workflow that publishes to
ghcr.io.Dockerfile.local (1)
38-49: LGTM!The runner stage is well-structured with appropriate security practices: minimal base image, non-root user (metaschema), proper ownership, and verification of the CLI before setting the entrypoint.
.github/workflows/container.yml (3)
1-16: LGTM!The workflow configuration is well-designed with a clear reusable pattern, appropriate environment variables for the container registry and metadata behavior, and a sensible
pushinput for controlling publish behavior.
51-72: LGTM!The metadata configuration properly generates image tags for different Git events (SHA, branch, tag, PR) with the latest tag correctly limited to the main branch. The OCI annotations provide comprehensive metadata per the spec.
73-92: LGTM!The conditional push logic is well-implemented. When
push: true, the workflow builds multi-architecture images (amd64, arm64), leverages GitHub Actions cache, and generates build provenance attestations—all following current best practices..github/workflows/build.yml (2)
108-112: LGTM!The artifact upload step correctly packages the metaschema-cli ZIP from the build output with an appropriate name (
build_zip) that the container workflow can consume.
113-123: LGTM!The container job is properly configured with correct permissions (including
id-token: writefor attestations as discussed), appropriate dependencies onbuild-code, and invokes the reusable container workflow withpush: trueto publish the image.
|
@david-waltermire I cleaned up all recommendations, even the nitpicky ones. Let me know what you think. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Dockerfile.local (1)
31-36: Consider combining RUN statements to reduce image layers.The five separate
RUNcommands create multiple image layers. Combining them would reduce layer count and potentially improve build efficiency:Proposed consolidation
-RUN find /usr/local/src/metaschema-cli/target \ - -iname 'metaschema-cli-*metaschema-cli.zip' \ - -exec cp {} /tmp/metaschema-cli.zip \; -RUN mkdir -p /opt/metaschema-cli -RUN unzip /tmp/metaschema-cli.zip -d /opt/metaschema-cli -RUN chmod +x /opt/metaschema-cli/bin/metaschema-cli +RUN find /usr/local/src/metaschema-cli/target \ + -iname 'metaschema-cli-*metaschema-cli.zip' \ + -exec cp {} /tmp/metaschema-cli.zip \; && \ + mkdir -p /opt/metaschema-cli && \ + unzip /tmp/metaschema-cli.zip -d /opt/metaschema-cli && \ + chmod +x /opt/metaschema-cli/bin/metaschema-cli && \ + rm /tmp/metaschema-cli.zipNote: This also cleans up the temporary ZIP file.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Dockerfile.ciDockerfile.local
🚧 Files skipped from review as they are similar to previous changes (1)
- Dockerfile.ci
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-11-14T17:07:03.586Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IIPv4AddressItem.java:66-73
Timestamp: 2024-11-14T17:07:03.586Z
Learning: In the Metaschema Java codebase, differences in casting patterns across atomic type implementations are intentional and required; any differences in approach are significant and necessary.
Applied to files:
Dockerfile.local
📚 Learning: 2025-12-24T21:21:52.756Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:52.756Z
Learning: In metaschema-testing, generated binding classes under gov.nist.secauto.metaschema.model.testing.testsuite are produced by metaschema-maven-plugin from YAML metaschema definitions. Javadoc issues in these generated classes should not be flagged for manual fixes; improvements are tracked and handled through code generator enhancements rather than manual edits to the generated source.
Applied to files:
Dockerfile.local
📚 Learning: 2025-12-24T21:21:35.530Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/csrc/ns/metaschema/test_suite/_1_0/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:35.530Z
Learning: In metaschema-framework/metaschema-java, generated binding classes in package gov.nist.csrc.ns.metaschema.test_suite._1_0 (and similar generated binding packages) are pre-generated by metaschema-maven-plugin and checked into source control. Javadoc coverage issues in these generated classes should be tracked as code generator improvements rather than file-level issues, and improvements are deferred to generator enhancements.
Applied to files:
Dockerfile.local
📚 Learning: 2025-12-24T21:21:59.692Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/GenerationCase.java:74-80
Timestamp: 2025-12-24T21:21:59.692Z
Learning: Files in the package gov.nist.secauto.metaschema.model.testing.testsuite in metaschema-testing are generated binding classes created from Metaschema definitions. Documentation and style improvements for these files should be made at the code generator level (metaschema-maven-plugin) rather than by manually editing the generated code.
Applied to files:
Dockerfile.local
🪛 Hadolint (2.14.0)
Dockerfile.local
[error] 30-30: unexpected 'R'
expecting a new line followed by the next instruction
(DL1000)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Code
- GitHub Check: Website
🔇 Additional comments (3)
Dockerfile.local (3)
12-30: Hadolint warning is a false positive.Hadolint 2.14.0 reports
DL1000: unexpected 'R'on line 30, but the heredoc syntax used in lines 12-29 is valid Docker syntax (requires# syntax=docker/dockerfile:1on line 1, which is present). The Maven command on line 30 is correctly formatted. This warning can be safely ignored.
38-48: LGTM: Runner stage follows security best practices.The runner stage correctly:
- Copies the built artifact from the builder stage
- Creates a non-root
metaschemauser and group (good security practice)- Sets appropriate ownership and permissions
- Includes a smoke test (
--version) to verify the CLI works- Configures the entrypoint to run the CLI
The implementation is clean and follows Docker best practices for multi-stage builds and security.
31-36: The path structure concern is not valid. The Maven assembly descriptor (assembly.xml) explicitly sets<includeBaseDirectory>false</includeBaseDirectory>, which prevents a top-level versioned directory from being created in the ZIP. Thecomponent.xmlincludes files directly to the root of the archive (fromappassemblerandgenerated-distro), so the unzipped structure will be:/opt/metaschema-cli/bin/metaschema-cli /opt/metaschema-cli/lib/ /opt/metaschema-cli/README.mdThe paths in lines 35–36 and the
ENTRYPOINTin line 48 are correct as written.
We do not really need id-token, so I will remove given feedback and research
…rms" This reverts commit 0e19a6c.
aj-stein
force-pushed
the
561-publish-oci-image
branch
from
54e9216 to
bad2c00
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
README.md (1)
92-92: Verify the glob pattern works correctly in the unzip command.Line 92 uses
sudo unzip "/tmp/*metaschema-cli.zip"with the glob pattern inside quotes. When the glob is quoted, the shell will not expand it, andunzipwill receive the literal string/tmp/*metaschema-cli.ziprather than the actual filename. This will cause the command to fail.If you intend to use a glob pattern, remove the quotes so the shell expands it before passing to unzip. However, note that if multiple matching files exist in
/tmp, the command could fail or behave unexpectedly.🔎 Proposed fix
Option 1: Remove quotes to allow shell expansion (your current approach elsewhere):
-sudo unzip "/tmp/*metaschema-cli.zip" -d /opt/metaschema-cli +sudo unzip /tmp/*metaschema-cli.zip -d /opt/metaschema-cliOption 2: Use a more explicit pattern that captures the downloaded filename:
# Download the zip archive of the latest release to /tmp mvn \ org.apache.maven.plugins:maven-dependency-plugin:LATEST:copy \ -DoutputDirectory=/tmp \ -DremoteRepositories=https://repo1.maven.org/maven2 \ -Dartifact=dev.metaschema.java:metaschema-cli:LATEST:zip:metaschema-cli +# Find the downloaded file +CLI_ZIP=$(find /tmp -name 'metaschema-cli-*-metaschema-cli.zip' -type f -print -quit) # Extract zip archive to /opt/metaschema-cli # You might need sudo for permission to write files to this path -sudo unzip "/tmp/*metaschema-cli.zip" -d /opt/metaschema-cli +sudo unzip "${CLI_ZIP}" -d /opt/metaschema-cli
🧹 Nitpick comments (2)
README.md (1)
50-53: Consider varying the sentence structure in the list.Three consecutive list items (lines 51-53) begin with "Download," which creates a repetitive pattern. While this is clear and functional, varying the wording slightly could improve readability.
🔎 Suggested alternative wording
1. Build the project locally with Maven. -1. Download a developer snapshot from the Metaschema Framework repository for pre-release builds. -1. Download the official releases from the official Maven Central. -1. Download an OCI-conformant container image from the [GitHub Container Registry (ghcr.io)](https://ghcr.io) for use with `docker`, `podman`, Kubernetes, or other container orchestration platforms. +1. Obtain a developer snapshot from the Metaschema Framework repository for pre-release builds. +1. Retrieve the official releases from Maven Central. +1. Pull an OCI-conformant container image from the [GitHub Container Registry (ghcr.io)](https://ghcr.io) for use with `docker`, `podman`, Kubernetes, or other container orchestration platforms.Based on static analysis findings.
Dockerfile.local (1)
29-30: Consider adding a blank line after the heredoc.Hadolint flags line 30 with a parsing issue. While the syntax is valid, adding a blank line between the heredoc EOF (line 29) and the next RUN instruction (line 30) may improve readability and eliminate the static analysis warning.
🔎 Proposed fix
XMLEOF EOF + RUN mvn -B -e -Prelease packageBased on static analysis findings.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/build.yml.github/workflows/container.ymlDockerfile.ciDockerfile.localREADME.md
🚧 Files skipped from review as they are similar to previous changes (2)
- Dockerfile.ci
- .github/workflows/container.yml
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-11-14T17:07:03.586Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IIPv4AddressItem.java:66-73
Timestamp: 2024-11-14T17:07:03.586Z
Learning: In the Metaschema Java codebase, differences in casting patterns across atomic type implementations are intentional and required; any differences in approach are significant and necessary.
Applied to files:
Dockerfile.local
📚 Learning: 2025-12-24T21:21:52.756Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:52.756Z
Learning: In metaschema-testing, generated binding classes under gov.nist.secauto.metaschema.model.testing.testsuite are produced by metaschema-maven-plugin from YAML metaschema definitions. Javadoc issues in these generated classes should not be flagged for manual fixes; improvements are tracked and handled through code generator enhancements rather than manual edits to the generated source.
Applied to files:
Dockerfile.local
📚 Learning: 2025-12-24T21:21:35.530Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/csrc/ns/metaschema/test_suite/_1_0/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:35.530Z
Learning: In metaschema-framework/metaschema-java, generated binding classes in package gov.nist.csrc.ns.metaschema.test_suite._1_0 (and similar generated binding packages) are pre-generated by metaschema-maven-plugin and checked into source control. Javadoc coverage issues in these generated classes should be tracked as code generator improvements rather than file-level issues, and improvements are deferred to generator enhancements.
Applied to files:
Dockerfile.local
📚 Learning: 2025-12-24T21:21:59.692Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/GenerationCase.java:74-80
Timestamp: 2025-12-24T21:21:59.692Z
Learning: Files in the package gov.nist.secauto.metaschema.model.testing.testsuite in metaschema-testing are generated binding classes created from Metaschema definitions. Documentation and style improvements for these files should be made at the code generator level (metaschema-maven-plugin) rather than by manually editing the generated code.
Applied to files:
Dockerfile.local
🪛 Hadolint (2.14.0)
Dockerfile.local
[error] 30-30: unexpected 'R'
expecting a new line followed by the next instruction
(DL1000)
🪛 LanguageTool
README.md
[style] ~53-~53: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ses from the official Maven Central. 1. Download an OCI-conformant container image from ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (5)
README.md (1)
99-107: LGTM! Clear container installation instructions.The container installation section is well-written and provides a straightforward example of pulling and running the CLI image. The version check demonstrates proper usage.
Dockerfile.local (2)
31-36: LGTM! Filename consistency issue resolved.The file handling logic correctly locates the built ZIP archive, copies it to a fixed filename
/tmp/metaschema-cli.zip, and then extracts from that same fixed filename. This addresses the critical issue raised in the previous review.
38-49: LGTM! Secure and well-configured runner stage.The runner stage properly:
- Copies the built CLI from the builder stage
- Creates a dedicated
metaschemauser/group (addressing previous naming concerns)- Runs as a non-root user for security
- Validates the CLI installation with a version check
- Configures the entrypoint correctly
.github/workflows/build.yml (2)
108-112: LGTM! Artifact upload properly configured.The upload step correctly captures the built CLI ZIP archive using an appropriate glob pattern and a clear artifact name that can be referenced by downstream workflows.
113-123: LGTM! Container job properly configured with correct permissions.The build-container job is well-structured:
- Correctly depends on
build-codeto ensure artifacts are available- Includes all necessary permissions (contents, packages, attestations, id-token) as confirmed in previous review discussions
- Properly calls the reusable container workflow with
push: trueto publish the image
2 participants
Committer Notes
Closes #561.
All Submissions:
By submitting a pull request, you are agreeing to provide this contribution under the CC0 1.0 Universal public domain dedication.
Changes to Core Features:
Have you written new tests for your core changes, as applicable?Have you included examples of how to use your new feature(s)?Have you updated all website and readme documentation affected by the changes you made?Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.