Skip to content

support for building inside docker #5630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ondras wants to merge 3 commits into
base: main
Choose a base branch
from
+33 −0
Open

support for building inside docker #5630

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee8ba6e
support for building inside docker
ondras May 19, 2026
ec35731
fixes suggested by the llm review
ondras May 19, 2026
ab00feb
extra env
ondras May 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .dockerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.git
.github
.vscode
.devcontainer
*.log
*.tmp
16 changes: 16 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
FROM ubuntu:latest
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pin the base image to a specific version.

Using ubuntu:latest leads to non-reproducible builds because the underlying image can change at any time. For a containerized build environment, reproducibility is essential.

🔒 Proposed fix to pin Ubuntu version 
-FROM ubuntu:latest
+FROM ubuntu:24.04
📝 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.

Suggested change
FROM ubuntu:latest
FROM ubuntu:24.04
🧰 Tools
🪛 Trivy (0.69.3)

[error] 1-1: Image user should not be 'root'

Specify at least 1 USER command in Dockerfile with non-root user as argument

Rule: DS-0002

Learn more

(IaC/Dockerfile)

[error] 1-1: Image user should not be 'root'

Specify at least 1 USER command in Dockerfile with non-root user as argument

Rule: DS-0002

Learn more

(IaC/Dockerfile)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Dockerfile` at line 1, The Dockerfile currently uses an unpinned base image
"FROM ubuntu:latest"; change this to a specific, immutable image tag or digest
(for example a release tag like ubuntu:22.04 or an image@sha256:<digest>) to
ensure reproducible builds—update the FROM line in the Dockerfile to use the
chosen pinned tag or digest and document the selected Ubuntu version.


RUN apt-get update \
&& apt-get install -y --no-install-recommends nodejs npm git ca-certificates python3-pip \
&& rm -rf /var/lib/apt/lists/*
Comment on lines +3 to +5
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify Dockerfile uses venv-based Python deps and no system-break flags.
rg -n 'break-system-packages|ignore-installed|python3-venv|python3 -m venv|ENV PATH="/opt/venv/bin:\$PATH"' Dockerfile

Repository: wled/WLED

Length of output: 135

Avoid system-level pip override flags; install Python deps in a venv instead.

--break-system-packages and --ignore-installed reduce determinism and can hide dependency conflicts. Use an explicit virtual environment in the image.

Proposed fix 
 RUN apt-get update \
-    && apt-get install -y --no-install-recommends nodejs npm git ca-certificates python3-pip \
+    && apt-get install -y --no-install-recommends nodejs npm git ca-certificates python3-pip python3-venv \
     && rm -rf /var/lib/apt/lists/*
@@
 RUN npm ci
-RUN pip install --break-system-packages --ignore-installed -r requirements.txt
+RUN python3 -m venv /opt/venv \
+    && /opt/venv/bin/pip install --no-cache-dir -r requirements.txt
+ENV PATH="/opt/venv/bin:$PATH"
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Dockerfile` around lines 3 - 5, The RUN layer currently installs python3-pip
and may encourage use of pip flags like --break-system-packages; instead create
and use an isolated virtualenv and install Python deps from there: in the
Dockerfile replace direct pip usage with steps to install python3-venv (if
needed), run python3 -m venv /opt/venv, upgrade pip via /opt/venv/bin/pip,
install your requirements via /opt/venv/bin/pip install -r requirements.txt
(without --break-system-packages or --ignore-installed), and update PATH (e.g.
ENV PATH="/opt/venv/bin:$PATH") so the rest of the image uses the venv-managed
Python; remove any global pip flags and avoid altering system packages.


WORKDIR /workdir

COPY . .

RUN npm ci || npm install
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
RUN pip install --break-system-packages --ignore-installed -r requirements.txt

ENV WLED_RELEASE=True
Copy link
Copy Markdown
Member

@softhack007 softhack007 May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is WLED_RELEASE needed for?

Its not checked by platformio or pio-scripts, neither referenced anywhere in our nightly CI jobs. SO maybe just remove it, to avoid future problems.

Copy link
Copy Markdown
Author

@ondras ondras May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will remove it then. This was suggested in the intially drafted Dockerfile in the Discord conversation, I just copypasted it.


CMD ["bash"]
7 changes: 7 additions & 0 deletions compose.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
services:
wled-build:
image: wled-build
build: .
volumes:
- ./build_output:/workdir/build_output
command: ["bash", "-c", "npm run build && pio run -e $PIO_ENV"]
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Loading