Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR removes two deprecated GitHub Actions workflows, converts the publish pipeline to a Docker-centric release flow (tag-triggered, buildx, GHCR push, release creation), adds a multi-stage Dockerfile to package the released server binary into a runtime image, and introduces a .dockerignore to limit the build context. (40 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (push tag)
participant GH as GitHub Actions (publish.yml)
participant Builder as Docker Buildx
participant Registry as GitHub Container Registry
participant Release as GitHub Release Action
Dev->>GH: push tag or trigger workflow
GH->>Builder: run buildx build using `release/` context
Builder->>Registry: push image (version, ref tags)
GH->>Release: create GitHub release with image metadata
Release-->>Dev: release published with image and metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/publish.yml:
- Around line 37-46: The shell uses unquoted GitHub Actions expressions which
can lead to injection; wrap the interpolations in double quotes. Update the if
condition to quote ${{ github.ref }} (use "${{ github.ref }}"), and quote
expansions/assignments such as VERSION="${GITHUB_REF#refs/tags/}" and any uses
of ${VERSION} when echoing to GITHUB_OUTPUT; keep references to symbols
github.ref, GITHUB_REF, VERSION, and GITHUB_OUTPUT so you change the comparison
and assignment sites accordingly.
- Around line 103-106: The BUILD_DATE build-arg is being passed as a literal
shell expression; add a prior step that computes the timestamp and exposes it as
an output (e.g., a step with id "date" that writes build_date to
$GITHUB_OUTPUT), then use that output in the docker/build-push-action build-args
(replace the literal $(date ...) with ${{ steps.date.outputs.build_date }}),
keeping the existing VERSION and VCS_REF references unchanged.
In `@Dockerfile`:
- Around line 24-42: Add a non-root runtime user for the final stage: create a
dedicated user/group (e.g., "exodus") in the final stage (the stage using
mcr.microsoft.com/dotnet/runtime:9.0), chown the application directory (/app) so
that the new user owns the files copied by COPY --from=build /src/server/ ., and
add a USER instruction before ENTRYPOINT to run Robust.Server as that non-root
user; ensure any required runtime permissions are granted to that user so the
HEALTHCHECK and server can run.
- Around line 39-40: The HEALTHCHECK uses curl but the final image (FROM
mcr.microsoft.com/dotnet/runtime:9.0) is minimal and doesn't include curl, so
every check will fail; either install curl in the final stage by adding package
installation steps in the final stage Dockerfile (apt-get update && apt-get
install -y curl and cleanup) or replace the HEALTHCHECK command with a tool
present in the final image (e.g., a dotnet-based probe or wget if available), or
remove the HEALTHCHECK and move it to docker-compose/orchestrator; update the
HEALTHCHECK line (the CMD that calls curl -f http://localhost:1212/status ||
exit 1) accordingly to match the chosen approach.
🧹 Nitpick comments (3)
Dockerfile (1)
10-14:ca-certificatesandcurlare unnecessary in the build stage.The build stage only needs
unzip. Thecurlandca-certificatespackages are not used here (no downloads happen in this stage). Removing them will reduce build time and layer size.Proposed fix
RUN apt-get update && apt-get install -y --no-install-recommends \ unzip \ - ca-certificates \ - curl \ && rm -rf /var/lib/apt/lists/*.github/workflows/publish.yml (2)
80-91: Semver tag patterns may produce no tags onworkflow_dispatch.When triggered via
workflow_dispatch(not a tag push),steps.version.outputs.versionisdev-<sha>, which is not valid semver. Thetype=semverpatterns will silently produce nothing. Onlytype=ref,event=branchand possiblytype=raw,value=latestwill generate tags. This is likely fine but worth being aware of — if dispatched from a non-default branch, the only tag will be the branch name.If a
devtag is always desired for non-tag runs, consider adding an explicit raw fallback:type=raw,value=dev,enable=${{ !startsWith(github.ref, 'refs/tags/') }}
108-118: Release step is well-guarded with thestartsWithcondition.The release is correctly gated to only run on tag pushes.
One minor note:
ncipollo/release-action@v1is a major-version tag — consider pinning to a specific minor/patch (e.g.,@v1.14.0) or using a commit SHA for reproducibility.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Dockerfile (1)
6-8: Remove unusedARGdeclarations from the build stage.
VERSION,BUILD_DATE, andVCS_REFare declared in the build stage but never referenced there. Besides being dead code, any change to these--build-argvalues will unnecessarily invalidate the build-stage cache layers.Proposed fix
FROM debian:trixie-slim AS build WORKDIR /src -ARG VERSION=dev -ARG BUILD_DATE -ARG VCS_REF - RUN apt-get update && apt-get install -y --no-install-recommends \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 6 - 8, Remove the unused ARG declarations (ARG VERSION, ARG BUILD_DATE, ARG VCS_REF) from the Dockerfile build stage; locate the build-stage ARG lines and delete those three ARG entries so build-stage cache isn't invalidated by irrelevant --build-arg changes, and ensure any place that actually needs those values (e.g., a later final stage or label) declares them there instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/publish.yml:
- Around line 108-118: The workflow hardcodes prerelease: false for the
ncipollo/release-action, causing pre-release tags like the v*.*.*-* pattern to
be published as stable releases; update the workflow to derive the prerelease
boolean from the version/tag produced by the version step (use
steps.version.outputs.version or steps.version.outputs.tag_with_v) — add a small
step that sets an output or env (e.g., is_prerelease) to true when the version
contains a hyphen (or matches the pre-release pattern) and then pass that
boolean into the release action's prerelease input so pre-release tags are
marked correctly.
---
Duplicate comments:
In @.github/workflows/publish.yml:
- Around line 103-106: The BUILD_DATE build-arg is being passed as the literal
string "$(date ...)" so it is not evaluated; add a prior step (e.g., name "Get
build date", id "date") that runs the date command and exposes it via
GITHUB_OUTPUT (e.g., set build_date) and then change the
docker/build-push-action build-args to use the computed output with ${{
steps.date.outputs.build_date }} for BUILD_DATE (leave VERSION as ${{
steps.version.outputs.version }} and VCS_REF as ${{ github.sha }}).
- Line 38: The conditional that tests refs is using an unquoted interpolation
`${{ github.ref }}` inside the `[[ ... ]]` test; update the `if [[ ${{
github.ref }} == refs/tags/* ]]; then` line to quote the variable like `if [[
"${{ github.ref }}" == refs/tags/* ]]; then` to prevent potential shell
injection and ensure safe string comparison of `github.ref`.
In `@Dockerfile`:
- Around line 24-42: Final image runs as root; add a dedicated non-root user and
switch to it in the final stage. In the Dockerfile final stage (WORKDIR /app,
ENTRYPOINT "./Robust.Server") create a user/group (or use an existing non-root
UID/GID), chown the application files in /app to that user, and add a USER
directive to run the container as that user; also ensure Robust.Server is
executable by that user. Reference the final stage symbols WORKDIR /app, COPY
--from=build /src/server/ ., ENTRYPOINT ["./Robust.Server"], and add USER
<username|uid> after ownership is set.
- Around line 39-40: The HEALTHCHECK uses curl but curl is only installed in the
build stage, so the final image's HEALTHCHECK will always fail; fix by
installing curl into the final runtime image
(mcr.microsoft.com/dotnet/runtime:9.0) before the HEALTHCHECK is declared —
e.g., add an apt-get update && apt-get install -y curl --no-install-recommends
and clean apt lists in the final stage, or alternatively change the HEALTHCHECK
to use a binary already present in the final image; update the HEALTHCHECK line
(the existing "HEALTHCHECK --interval=30s ... CMD curl -f
http://localhost:1212/status || exit 1") accordingly after ensuring curl is
available.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 6-8: Remove the unused ARG declarations (ARG VERSION, ARG
BUILD_DATE, ARG VCS_REF) from the Dockerfile build stage; locate the build-stage
ARG lines and delete those three ARG entries so build-stage cache isn't
invalidated by irrelevant --build-arg changes, and ensure any place that
actually needs those values (e.g., a later final stage or label) declares them
there instead.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
Dockerfile (5)
42-44: Consider re-adding aHEALTHCHECKnow that the final stage is established.The previous HEALTHCHECK was removed (correctly, since
curlwasn't present), but no replacement was added. Without a HEALTHCHECK, Docker/orchestrators cannot detect a hung or crashed server process.A minimal approach: install
curlin the final stage and restore the check, or use a lightweight probe binary.# Option A: install curl in the final stage RUN apt-get update && apt-get install -y --no-install-recommends curl \ && rm -rf /var/lib/apt/lists/* HEALTHCHECK --interval=30s --timeout=10s --start-period=10s --retries=3 \ CMD curl -f http://localhost:1212/status || exit 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 42 - 44, Re-add a Docker HEALTHCHECK for the final stage so orchestrators can detect a hung/crashed Robust.Server: install a lightweight probe (e.g., curl or a small static probe) in the final image (or ensure it’s copied in) and add a HEALTHCHECK instruction that probes the server endpoint (for example /status) at a sensible interval/timeout/retries; update the final-stage Dockerfile around the USER ss14 / ENTRYPOINT ["./Robust.Server"] block to include the install/copy step for the probe and a HEALTHCHECK line (name the probe or command as CURL or your probe binary in the comment to make the change obvious).
37-37:useraddis missing--no-log-init— add it to avoid large sparse files.Without
--no-log-init,useraddinitializes/var/log/lastlogand/var/log/faillog, which in containers can produce large sparse files that inflate image/layer sizes.♻️ Proposed fix
-RUN groupadd -r ss14 && useradd -r -g ss14 -d /app ss14 +RUN groupadd -r ss14 && useradd -r -g ss14 -d /app --no-log-init ss14🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` at line 37, The RUN command uses useradd (RUN groupadd -r ss14 && useradd -r -g ss14 -d /app ss14) which omits --no-log-init and causes /var/log/lastlog/faillog sparse files in the image; update the useradd invocation to include --no-log-init (i.e., useradd --no-log-init -r -g ss14 -d /app ss14) so those logs are not initialized in the container image.
39-41: UseCOPY --chownto eliminate the extraRUN chownlayer.The separate
RUN chown -Rcreates an additional layer over the copied files.COPY --chownsets ownership atomically at copy time, reducing layer count.♻️ Proposed fix
-COPY --from=build /src/server/ . - -RUN chown -R ss14:ss14 /app +COPY --chown=ss14:ss14 --from=build /src/server/ .🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 39 - 41, Replace the separate copy + chown steps by using COPY --chown to set ownership at copy time: update the COPY --from=build /src/server/ . instruction to include --chown=ss14:ss14 (so ownership is set when files are copied) and remove the subsequent RUN chown -R ss14:ss14 /app line; ensure the target path used in COPY matches the directory whose ownership was previously changed.
10-14:curlis installed but unused in the build stage.The zip is copied from the build context via
COPY, socurlis never invoked in this stage. Removing it trims the layer and marginally speeds upapt-get install.♻️ Proposed fix
RUN apt-get update && apt-get install -y --no-install-recommends \ unzip \ ca-certificates \ - curl \ && rm -rf /var/lib/apt/lists/*🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 10 - 14, The apt-get install RUN in the Dockerfile currently installs curl even though it isn't used in the build stage; remove "curl" from the package list in the RUN that installs unzip, ca-certificates, etc., so the layer is smaller and build is slightly faster—update the RUN installing packages (the line containing "apt-get update && apt-get install -y --no-install-recommends \ unzip \ ca-certificates \ curl \") to omit curl.
27-27:ARG VERSIONdefault differs between stages.The build stage defaults
VERSIONto"dev"(Line 6) while the final stage defaults it to"release"(Line 27). Since the workflow always supplies--build-arg VERSION=..., this is harmless in practice, but the inconsistency is confusing.♻️ Proposed fix — align defaults
-ARG VERSION=release +ARG VERSION=dev🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` at line 27, The Dockerfile has mismatched ARG VERSION defaults between the build stage (ARG VERSION=dev) and the final stage (ARG VERSION=release); update the final stage's ARG VERSION to match the build stage (or vice versa) so both stages use the same default value for ARG VERSION, keeping the workflow behavior unchanged while removing the confusing inconsistency..github/workflows/publish.yml (2)
25-27: Job outputsimage_tagandimage_digestare defined but never consumed.No downstream job in this file references these outputs. Either wire them up to a subsequent job (e.g., a deployment or attestation step) or remove them to avoid dead configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish.yml around lines 25 - 27, The workflow defines job outputs image_tag and image_digest (mapping to steps.meta.outputs.tags and steps.build.outputs.digest) that are never consumed; either remove the outputs block entirely or wire those outputs into a downstream job's needs.<job outputs> usage (for example, reference needs.<job_id>.outputs.image_tag and needs.<job_id>.outputs.image_digest in a deploy/attestation job) so they are consumed—update the downstream job to use needs.<producer_job>.outputs.image_tag and needs.<producer_job>.outputs.image_digest, or delete the image_tag/image_digest entries from the outputs block to eliminate dead configuration.
102-102:push: trueis unconditional — manual dispatches from feature branches push tagged images.On a
workflow_dispatchrun, the build pushes an image tagged with the branch name (e.g.,docker). If this is unintentional, consider guarding push on tag events only:♻️ Proposed fix
- push: true + push: ${{ startsWith(github.ref, 'refs/tags/') }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish.yml at line 102, The publish job currently uses an unconditional push setting (push: true) which causes workflow_dispatch runs from feature branches to push images; change the push behavior so it only pushes for tag releases by replacing the unconditional push in the Docker build/publish step (the push: true setting) with a conditional that evaluates true only for tag refs (use the github.ref or ref_type check) or by gating the step with an if that only runs on tag events; update the publish step configuration so pushes happen only when the ref is a tag (e.g., refs/tags/...) and leave workflow_dispatch builds from branches non-pushing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/publish.yml:
- Around line 89-94: Change the tags block so the "latest" raw tag is enabled
for stable (non-prerelease) tag pushes instead of checking is_default_branch;
replace enable={{is_default_branch}} with an expression that detects semantic
tag pushes and excludes prereleases, e.g. enable={{startsWith(github.ref,
'refs/tags/v') && !contains(steps.version.outputs.version, '-')}} so the
"latest" tag is emitted when the push is a normal semver tag (use the existing
steps.version.outputs.version in the check).
---
Nitpick comments:
In @.github/workflows/publish.yml:
- Around line 25-27: The workflow defines job outputs image_tag and image_digest
(mapping to steps.meta.outputs.tags and steps.build.outputs.digest) that are
never consumed; either remove the outputs block entirely or wire those outputs
into a downstream job's needs.<job outputs> usage (for example, reference
needs.<job_id>.outputs.image_tag and needs.<job_id>.outputs.image_digest in a
deploy/attestation job) so they are consumed—update the downstream job to use
needs.<producer_job>.outputs.image_tag and
needs.<producer_job>.outputs.image_digest, or delete the image_tag/image_digest
entries from the outputs block to eliminate dead configuration.
- Line 102: The publish job currently uses an unconditional push setting (push:
true) which causes workflow_dispatch runs from feature branches to push images;
change the push behavior so it only pushes for tag releases by replacing the
unconditional push in the Docker build/publish step (the push: true setting)
with a conditional that evaluates true only for tag refs (use the github.ref or
ref_type check) or by gating the step with an if that only runs on tag events;
update the publish step configuration so pushes happen only when the ref is a
tag (e.g., refs/tags/...) and leave workflow_dispatch builds from branches
non-pushing.
In `@Dockerfile`:
- Around line 42-44: Re-add a Docker HEALTHCHECK for the final stage so
orchestrators can detect a hung/crashed Robust.Server: install a lightweight
probe (e.g., curl or a small static probe) in the final image (or ensure it’s
copied in) and add a HEALTHCHECK instruction that probes the server endpoint
(for example /status) at a sensible interval/timeout/retries; update the
final-stage Dockerfile around the USER ss14 / ENTRYPOINT ["./Robust.Server"]
block to include the install/copy step for the probe and a HEALTHCHECK line
(name the probe or command as CURL or your probe binary in the comment to make
the change obvious).
- Line 37: The RUN command uses useradd (RUN groupadd -r ss14 && useradd -r -g
ss14 -d /app ss14) which omits --no-log-init and causes /var/log/lastlog/faillog
sparse files in the image; update the useradd invocation to include
--no-log-init (i.e., useradd --no-log-init -r -g ss14 -d /app ss14) so those
logs are not initialized in the container image.
- Around line 39-41: Replace the separate copy + chown steps by using COPY
--chown to set ownership at copy time: update the COPY --from=build /src/server/
. instruction to include --chown=ss14:ss14 (so ownership is set when files are
copied) and remove the subsequent RUN chown -R ss14:ss14 /app line; ensure the
target path used in COPY matches the directory whose ownership was previously
changed.
- Around line 10-14: The apt-get install RUN in the Dockerfile currently
installs curl even though it isn't used in the build stage; remove "curl" from
the package list in the RUN that installs unzip, ca-certificates, etc., so the
layer is smaller and build is slightly faster—update the RUN installing packages
(the line containing "apt-get update && apt-get install -y
--no-install-recommends \ unzip \ ca-certificates \ curl \") to omit curl.
- Line 27: The Dockerfile has mismatched ARG VERSION defaults between the build
stage (ARG VERSION=dev) and the final stage (ARG VERSION=release); update the
final stage's ARG VERSION to match the build stage (or vice versa) so both
stages use the same default value for ARG VERSION, keeping the workflow behavior
unchanged while removing the confusing inconsistency.
About the PR
Добавлен пайплайн упаковки сервера в контейнер.
Триггерится при установки тега "vX.Y.Z"
Summary by CodeRabbit