Conversation
|
|
github-actions
bot
added
S: Untriaged
No C#
YML
S: Needs Review
size/M
and removed
S: Untriaged
labels
📝 WalkthroughWalkthroughThis PR containerizes the application by introducing Docker-based CI/CD infrastructure. It removes two deprecated GitHub Actions workflows, significantly overhauls the main publish workflow to use Docker container registry publishing with automated version tagging and release creation, and adds a multi-stage Dockerfile and .dockerignore for optimized builds. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 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.
| run: | | ||
| if [[ ${{ github.ref }} == refs/tags/* ]]; then | ||
| VERSION=${GITHUB_REF#refs/tags/} | ||
| VERSION=${VERSION#v} | ||
| echo "version=${VERSION}" >> $GITHUB_OUTPUT | ||
| echo "tag_with_v=v${VERSION}" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "version=dev-${{ github.sha }}" >> $GITHUB_OUTPUT | ||
| echo "tag_with_v=dev" >> $GITHUB_OUTPUT | ||
| fi |
There was a problem hiding this comment.
Quote the expression to prevent potential shell injection.
${{ github.ref }} is interpolated directly into the shell script. While tag refs are unlikely to contain malicious characters, it's best practice to quote all GitHub Actions expressions to prevent injection.
Proposed fix
- if [[ ${{ github.ref }} == refs/tags/* ]]; then
+ if [[ "${{ github.ref }}" == refs/tags/* ]]; then🤖 Prompt for AI Agents
In @.github/workflows/publish.yml around lines 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.
| build-args: | | ||
| VERSION=${{ steps.version.outputs.version }} | ||
| BUILD_DATE=$(date -u +'%Y-%m-%dT%H:%M:%SZ') | ||
| VCS_REF=${{ github.sha }} |
There was a problem hiding this comment.
BUILD_DATE build-arg passes a literal string, not the evaluated date.
In docker/build-push-action, the build-args input is not processed through a shell. The value $(date -u +'%Y-%m-%dT%H:%M:%SZ') will be passed as-is to Docker, so BUILD_DATE in the image label will be the literal string $(date -u +'%Y-%m-%dT%H:%M:%SZ') rather than an actual timestamp.
Compute the date in a prior step or use a GitHub Actions expression.
Proposed fix: compute the date in a prior step
Add a step before the Docker build:
- name: Get build date
id: date
run: echo "build_date=$(date -u +'%Y-%m-%dT%H:%M:%SZ')" >> $GITHUB_OUTPUTThen reference it in build-args:
build-args: |
VERSION=${{ steps.version.outputs.version }}
- BUILD_DATE=$(date -u +'%Y-%m-%dT%H:%M:%SZ')
+ BUILD_DATE=${{ steps.date.outputs.build_date }}
VCS_REF=${{ github.sha }}🤖 Prompt for AI Agents
In @.github/workflows/publish.yml around lines 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.
| FROM mcr.microsoft.com/dotnet/runtime:9.0 AS final | ||
| WORKDIR /app | ||
|
|
||
| ARG VERSION=dev | ||
| ARG BUILD_DATE | ||
| ARG VCS_REF | ||
|
|
||
| LABEL org.opencontainers.image.version="${VERSION}" \ | ||
| org.opencontainers.image.created="${BUILD_DATE}" \ | ||
| org.opencontainers.image.revision="${VCS_REF}" \ | ||
| org.opencontainers.image.title="Exodus Monolith Server" \ | ||
| org.opencontainers.image.description="SS14 Exodus Monolith Server" | ||
|
|
||
| COPY --from=build /src/server/ . | ||
|
|
||
| HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ | ||
| CMD curl -f http://localhost:1212/status || exit 1 | ||
|
|
||
| ENTRYPOINT [ "./Robust.Server" ] |
There was a problem hiding this comment.
Container runs as root — add a non-root USER.
Trivy flagged DS-0002: the image runs everything as root. If the server is compromised, the attacker has full root privileges inside the container. Add a dedicated user for the runtime stage.
Proposed fix
FROM mcr.microsoft.com/dotnet/runtime:9.0 AS final
WORKDIR /app
+RUN groupadd -r ss14 && useradd -r -g ss14 -d /app ss14
+
ARG VERSION=dev
ARG BUILD_DATE
ARG VCS_REF
LABEL org.opencontainers.image.version="${VERSION}" \
org.opencontainers.image.created="${BUILD_DATE}" \
org.opencontainers.image.revision="${VCS_REF}" \
org.opencontainers.image.title="Exodus Monolith Server" \
org.opencontainers.image.description="SS14 Exodus Monolith Server"
COPY --from=build /src/server/ .
+RUN chown -R ss14:ss14 /app
+USER ss14
HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \
CMD curl -f http://localhost:1212/status || exit 1
ENTRYPOINT [ "./Robust.Server" ]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FROM mcr.microsoft.com/dotnet/runtime:9.0 AS final | |
| WORKDIR /app | |
| ARG VERSION=dev | |
| ARG BUILD_DATE | |
| ARG VCS_REF | |
| LABEL org.opencontainers.image.version="${VERSION}" \ | |
| org.opencontainers.image.created="${BUILD_DATE}" \ | |
| org.opencontainers.image.revision="${VCS_REF}" \ | |
| org.opencontainers.image.title="Exodus Monolith Server" \ | |
| org.opencontainers.image.description="SS14 Exodus Monolith Server" | |
| COPY --from=build /src/server/ . | |
| HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ | |
| CMD curl -f http://localhost:1212/status || exit 1 | |
| ENTRYPOINT [ "./Robust.Server" ] | |
| FROM mcr.microsoft.com/dotnet/runtime:9.0 AS final | |
| WORKDIR /app | |
| RUN groupadd -r ss14 && useradd -r -g ss14 -d /app ss14 | |
| ARG VERSION=dev | |
| ARG BUILD_DATE | |
| ARG VCS_REF | |
| LABEL org.opencontainers.image.version="${VERSION}" \ | |
| org.opencontainers.image.created="${BUILD_DATE}" \ | |
| org.opencontainers.image.revision="${VCS_REF}" \ | |
| org.opencontainers.image.title="Exodus Monolith Server" \ | |
| org.opencontainers.image.description="SS14 Exodus Monolith Server" | |
| COPY --from=build /src/server/ . | |
| RUN chown -R ss14:ss14 /app | |
| USER ss14 | |
| HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ | |
| CMD curl -f http://localhost:1212/status || exit 1 | |
| ENTRYPOINT [ "./Robust.Server" ] |
🤖 Prompt for AI Agents
In `@Dockerfile` around lines 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.
| HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ | ||
| CMD curl -f http://localhost:1212/status || exit 1 |
There was a problem hiding this comment.
curl is not installed in the final stage — HEALTHCHECK will always fail.
The HEALTHCHECK uses curl, but curl was only installed in the build stage (debian). The final stage (mcr.microsoft.com/dotnet/runtime:9.0) is a minimal image that does not include curl. Every health check invocation will fail with "command not found," causing Docker/orchestrators to perpetually mark the container as unhealthy.
Either install curl in the final stage, or use a tool already available (e.g., a dotnet-based check or wget if present).
Option A: Install curl in the final stage
FROM mcr.microsoft.com/dotnet/runtime:9.0 AS final
WORKDIR /app
+RUN apt-get update && apt-get install -y --no-install-recommends curl \
+ && rm -rf /var/lib/apt/lists/*
+
ARG VERSION=devOption B: Remove the HEALTHCHECK from the Dockerfile and define it in docker-compose or the orchestrator instead
-HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \
- CMD curl -f http://localhost:1212/status || exit 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ | |
| CMD curl -f http://localhost:1212/status || exit 1 |
🤖 Prompt for AI Agents
In `@Dockerfile` around lines 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.
2 participants
About the PR
Добавлен пайплайн упаковки сервера в контейнер.
Триггерится при установки тега "vX.Y.Z"
Summary by CodeRabbit