Refactor Dockerfile: optimize build stages, reduce image size, and im… #1721
Conversation
…prove security - Consolidated RUN commands to reduce layer count - Added virtual dependencies for build and removed them after use - Simplified architecture logic for downloading grpc-health-probe - Reordered COPY and build commands for better cache efficiency
| ENV RAILS_ENV="production" \ | ||
| BUNDLE_DEPLOYMENT="1" \ | ||
| BUNDLE_PATH="/usr/local/bundle" \ | ||
| BUNDLE_WITHOUT="development test" \ | ||
| BUNDLE_FORCE_RUBY_PLATFORM="1" | ||
| BUNDLE_FORCE_RUBY_PLATFORM="1" \ | ||
| RUBY_YJIT_ENABLE="true" |
There was a problem hiding this comment.
This is required in the final build stage so that YJIT is enabled when the application is running.
Does this persist to the final build stage?
There was a problem hiding this comment.
In a multi-stage Dockerfile, environment variables defined in one stage do not automatically persist to later stages. So, even though you set BUNDLE_FORCE_RUBY_PLATFORM="1" and RUBY_YJIT_ENABLE="true" in the base stage, these will not carry over to the build or final stages unless you set them explicitly in each stage that requires them.
| RUN apk add --no-cache wget curl jq | ||
|
|
||
| # Download and configure the gRPC health probe | ||
| RUN ARCH=$(uname -m) && \ |
There was a problem hiding this comment.
You changed arch to uname -m. I can't remember why I used arch but I think it does the same as uname -m Also small thing but the static analysis here: https://app.codacy.com/gh/somleng/somleng/pull-requests/1721/issues recommends to quote the output. I know here it's redundant because that command shouldn't output newlines anyway.
There was a problem hiding this comment.
This item may be rolled back.
| RUN bundle install && \ | ||
| rm -rf ~/.bundle/ "${BUNDLE_PATH}"/ruby/*/cache "${BUNDLE_PATH}"/ruby/*/bundler/gems/*/.git && \ | ||
| find "${BUNDLE_PATH}" -name "*.o" -delete && find "${BUNDLE_PATH}" -name "*.c" -delete && \ | ||
| mkdir -p tmp/pids && \ |
There was a problem hiding this comment.
I think these mkdir -p are here because it will fail later in the build if those directories don't exist
There was a problem hiding this comment.
This item may be rolled back too.
| apk upgrade --no-cache && \ | ||
| apk add --update --no-cache build-base gcompat postgresql-dev vips-dev ffmpeg | ||
| # Install only runtime dependencies | ||
| RUN apk add --no-cache postgresql-libs vips-dev ffmpeg |
There was a problem hiding this comment.
I'm can't remember why, and I should have left a better description but this commit: https://github.com/somleng/somleng/pull/1721/checks?check_run_id=32366076143 adds gcompat. I recall it had something to do with alpine arm64 builds. Is there a reason why you removed this?
There was a problem hiding this comment.
This item may be rolled back, I will review this item as well.
…prove security