-
-
Notifications
You must be signed in to change notification settings - Fork 101
use same devcontainer as home assistant core #1677
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.
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
Conversation
WalkthroughTwo new files have been introduced: a development container configuration for Home Assistant and a comprehensive JSON schema for validating integration manifests. The devcontainer file sets up a tailored VS Code environment with specific extensions, settings, and features, replacing the previously deleted generic devcontainer configuration. The schema file defines strict validation rules for Home Assistant integration manifest files. Additionally, new Dockerfiles for building the Home Assistant Core container image and a development environment have been added. A bootstrap script was introduced for dependency installation, and the setup script was updated to use it. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
scripts/json_schemas/manifest_schema.json (1)
1-448
: Reduce duplication with$defs
+$ref
domain
,name
, andintegration_type
blocks are duplicated across five locations.
Moving them to$defs
and referencing via$ref
will:
- eliminate drift between copies
- shrink the schema
- improve maintainability
No functional change—pure refactor.
.devcontainer/devcontainer.json (1)
22-24
: Disabling SELinux labels weakens container isolationThe run-arg
--security-opt label=disable
turns off SELinux confinement. Only keep this if absolutely required for Home Assistant; otherwise drop it to preserve default Docker security.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.devcontainer/devcontainer.json
(1 hunks)scripts/json_schemas/manifest_schema.json
(1 hunks)
⏰ 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). (1)
- GitHub Check: Run tests
🔇 Additional comments (1)
.devcontainer/devcontainer.json (1)
10-13
: Verify availability of custom dev-container feature
ghcr.io/anthropics/devcontainer-features/claude-code:1.0
is not an official feature and may not exist in GHCR, causing the container build to fail.Please confirm the image/tag is published or replace it with a stable source.
"domain": { | ||
"description": "The domain identifier of the integration.\nhttps://developers.home-assistant.io/docs/creating_integration_manifest/#domain", | ||
"examples": [ | ||
"mobile_app" | ||
], | ||
"type": "string", | ||
"pattern": "[0-9a-z_]+" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anchor regex patterns to avoid false-positives
pattern
is used three times to validate domain
, but without ^
/ $
anchors it will accept any string containing a valid domain instead of requiring the whole value to match.
Example: "foo_bar"
is valid, but "xxxfoo_barzzz"
would also pass.
- "pattern": "[0-9a-z_]+"
+ "pattern": "^[0-9a-z_]+$"
Apply the same change to Lines 66 & 151 (and any other repeated occurrences).
Also applies to: 60-67, 93-100
🤖 Prompt for AI Agents
In scripts/json_schemas/manifest_schema.json around lines 20 to 27, the regex
pattern for the "domain" property is missing start (^) and end ($) anchors,
causing it to match substrings instead of the entire string. Update the pattern
to include ^ at the beginning and $ at the end to ensure full string matching.
Apply the same fix to the other occurrences of the "pattern" property at lines
60-67, 93-100, 66, and 151 to maintain consistency and correctness.
"codeowners": { | ||
"description": "A list of GitHub usernames or GitHub team names of the integration owners.\nhttps://developers.home-assistant.io/docs/creating_integration_manifest/#code-owners", | ||
"type": "array", | ||
"minItems": 0, | ||
"items": { | ||
"type": "string", | ||
"pattern": "^@.+$" | ||
}, | ||
"uniqueItems": true | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
codeowners
can be required yet empty
codeowners
is declared required
(Line 429) but its schema allows an empty array (minItems: 0
).
That makes the “required” check meaningless.
- "minItems": 0,
+ "minItems": 1,
This guarantees at least one owner is specified.
Also applies to: 426-431
🤖 Prompt for AI Agents
In scripts/json_schemas/manifest_schema.json around lines 385 to 394, the
"codeowners" property is marked as required but allows an empty array due to
"minItems" being 0, which defeats the purpose of requiring it. To fix this,
change "minItems" from 0 to 1 to ensure that when "codeowners" is present, it
contains at least one item, guaranteeing at least one owner is specified.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1677 +/- ##
==========================================
+ Coverage 92.40% 92.64% +0.24%
==========================================
Files 12 12
Lines 2053 2053
==========================================
+ Hits 1897 1902 +5
+ Misses 156 151 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (3)
Dockerfile (2)
34-34
: Consider pinninguv
by commit / hash instead of PyPI mutable version tag
pip3 install uv==0.7.1
still resolves from PyPI at build time; if 0.7.1 is yanked or replaced you get non-reproducible images. Using the GitHub release tarball with a digest (oruv
’s OCI image as done in Dockerfile.dev) would keep builds deterministic.
19-23
:ARG BUILD_ARCH
is declared afterFROM
– redclare before if reused in other stagesDocker only preserves ARGs defined before the first
FROM
for subsequent stages.
If a multi-stage build is introduced later, this ARG will silently be undefined. Move the declaration above the firstFROM
to avoid future breakage.Dockerfile.dev (1)
5-8
: Yarn key added but package never installedThe repository key is configured yet
yarn
itself is not in theapt-get install
list, producing unnecessary network traffic and maintenance surface. Remove the key setup or addyarn
to the package list.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Dockerfile
(1 hunks)Dockerfile.dev
(1 hunks)
🔇 Additional comments (1)
Dockerfile.dev (1)
38-39
: Confirm Python 3.13.2 availability
uv python install 3.13.2
assumes 3.13 has been officially released and mirrored byuv
. If the build runs earlier it will fail. Please verify the version exists in CI or switch to the current stable (3.12.x) until 3.13 GA.
# Get go2rtc binary | ||
RUN \ | ||
case "${BUILD_ARCH}" in \ | ||
"aarch64") go2rtc_suffix='arm64' ;; \ | ||
"armhf") go2rtc_suffix='armv6' ;; \ | ||
"armv7") go2rtc_suffix='arm' ;; \ | ||
*) go2rtc_suffix=${BUILD_ARCH} ;; \ | ||
esac \ | ||
&& curl -L https://github.com/AlexxIT/go2rtc/releases/download/v1.9.9/go2rtc_linux_${go2rtc_suffix} --output /bin/go2rtc \ | ||
&& chmod +x /bin/go2rtc \ | ||
# Verify go2rtc can be executed | ||
&& go2rtc --version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add checksum / signature verification for downloaded go2rtc
binary
The image blindly downloads an executable from GitHub and runs it.
Without verifying a checksum / GPG signature this is a supply-chain risk: the URL can be hijacked or the asset replaced, and the malicious binary would run during build time with root privileges.
At minimum, fetch the release’s SHA-256 from the GitHub API (or bundle a known hash in the repo) and validate it before chmod +x
.
- && curl -L https://github.com/AlexxIT/go2rtc/releases/download/v1.9.9/go2rtc_linux_${go2rtc_suffix} --output /bin/go2rtc \
- && chmod +x /bin/go2rtc \
+ && curl -L -f https://github.com/AlexxIT/go2rtc/releases/download/v1.9.9/go2rtc_linux_${go2rtc_suffix} -o /tmp/go2rtc \
+ && echo "<expected-sha256> /tmp/go2rtc" | sha256sum -c - \
+ && install -m 0755 /tmp/go2rtc /bin/go2rtc \
📝 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.
# Get go2rtc binary | |
RUN \ | |
case "${BUILD_ARCH}" in \ | |
"aarch64") go2rtc_suffix='arm64' ;; \ | |
"armhf") go2rtc_suffix='armv6' ;; \ | |
"armv7") go2rtc_suffix='arm' ;; \ | |
*) go2rtc_suffix=${BUILD_ARCH} ;; \ | |
esac \ | |
&& curl -L https://github.com/AlexxIT/go2rtc/releases/download/v1.9.9/go2rtc_linux_${go2rtc_suffix} --output /bin/go2rtc \ | |
&& chmod +x /bin/go2rtc \ | |
# Verify go2rtc can be executed | |
&& go2rtc --version | |
# Get go2rtc binary | |
RUN \ | |
case "${BUILD_ARCH}" in \ | |
"aarch64") go2rtc_suffix='arm64' ;; \ | |
"armhf") go2rtc_suffix='armv6' ;; \ | |
"armv7") go2rtc_suffix='arm' ;; \ | |
*) go2rtc_suffix=${BUILD_ARCH} ;; \ | |
esac \ | |
&& curl -L -f https://github.com/AlexxIT/go2rtc/releases/download/v1.9.9/go2rtc_linux_${go2rtc_suffix} -o /tmp/go2rtc \ | |
&& echo "<expected-sha256> /tmp/go2rtc" | sha256sum -c - \ | |
&& install -m 0755 /tmp/go2rtc /bin/go2rtc \ | |
# Verify go2rtc can be executed | |
&& go2rtc --version |
🤖 Prompt for AI Agents
In Dockerfile lines 20 to 31, the downloaded go2rtc binary is not verified for
integrity, posing a supply-chain risk. To fix this, add a step to fetch the
SHA-256 checksum for the specific go2rtc release from the GitHub API or use a
pre-stored known hash, then verify the downloaded binary's checksum matches
before running chmod +x. If the checksum does not match, the build should fail
to prevent using a potentially tampered binary.
Dockerfile
Outdated
uv pip install \ | ||
--no-build \ | ||
-r homeassistant/requirements.txt | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Package constraints file copied but not applied
homeassistant/package_constraints.txt
is present yet the install line omits --constraint
. Skipping it can pull in versions that violate HA’s tested matrix and break runtime.
- uv pip install \
- --no-build \
- -r homeassistant/requirements.txt
+ uv pip install \
+ --no-build \
+ -r homeassistant/requirements.txt \
+ --constraint homeassistant/homeassistant/package_constraints.txt
📝 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.
uv pip install \ | |
--no-build \ | |
-r homeassistant/requirements.txt | |
uv pip install \ | |
--no-build \ | |
-r homeassistant/requirements.txt \ | |
--constraint homeassistant/homeassistant/package_constraints.txt |
🤖 Prompt for AI Agents
In Dockerfile lines 42 to 45, the pip install command is missing the
--constraint option to apply the package constraints from
homeassistant/package_constraints.txt. Modify the pip install command to include
--constraint homeassistant/package_constraints.txt before the -r option to
ensure package versions adhere to Home Assistant's tested matrix and prevent
runtime issues.
RUN \ | ||
curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - \ | ||
&& apt-get update \ | ||
&& DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
apt-key
is deprecated; migrate to keyring file
apt-key add -
will be removed in Debian 13. Use the recommended keyring approach to avoid future build failures.
-curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add -
+curl -fsSL https://dl.yarnpkg.com/debian/pubkey.gpg | \
+ gpg --dearmor -o /etc/apt/keyrings/yarn.gpg && \
+echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/yarn.gpg] \
+ https://dl.yarnpkg.com/debian/ stable main" > /etc/apt/sources.list.d/yarn.list
📝 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.
RUN \ | |
curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - \ | |
&& apt-get update \ | |
&& DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ | |
RUN \ | |
curl -fsSL https://dl.yarnpkg.com/debian/pubkey.gpg | \ | |
gpg --dearmor -o /etc/apt/keyrings/yarn.gpg && \ | |
echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/yarn.gpg] \ | |
https://dl.yarnpkg.com/debian/ stable main" > /etc/apt/sources.list.d/yarn.list \ | |
&& apt-get update \ | |
&& DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ |
🤖 Prompt for AI Agents
In Dockerfile.dev around lines 5 to 8, the use of `apt-key add -` is deprecated
and will be removed in Debian 13. Replace this with the recommended approach by
downloading the GPG key and saving it to a keyring file under
/usr/share/keyrings/, then reference this keyring in the apt sources list entry.
This avoids using `apt-key` and ensures compatibility with future Debian
releases.
# Add go2rtc binary | ||
COPY --from=ghcr.io/alexxit/go2rtc:latest /usr/local/bin/go2rtc /bin/go2rtc | ||
|
||
WORKDIR /usr/src | ||
|
||
COPY --from=ghcr.io/astral-sh/uv:latest /uv /usr/local/bin/uv | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Images pulled with latest
tag make the build non-reproducible
Both go2rtc
and uv
are copied from :latest
images, which drift over time. Pin to a digest or exact version tag to guarantee deterministic builds and simplify SBOM / provenance audits.
-COPY --from=ghcr.io/alexxit/go2rtc:latest /usr/local/bin/go2rtc /bin/go2rtc
+COPY --from=ghcr.io/alexxit/go2rtc:v1.9.9@sha256:<digest> /usr/local/bin/go2rtc /bin/go2rtc
Do the same for the uv
stage.
🤖 Prompt for AI Agents
In Dockerfile.dev around lines 31 to 37, the COPY commands pull images with the
:latest tag for go2rtc and uv, causing non-reproducible builds. Replace the
:latest tags with specific version tags or image digests for both go2rtc and uv
to ensure deterministic builds and improve SBOM and provenance tracking.
Dockerfile.dev
Outdated
COPY requirements.txt ./ | ||
COPY homeassistant/package_constraints.txt homeassistant/package_constraints.txt | ||
RUN uv pip install -r requirements.txt | ||
COPY requirements_test.txt requirements_test_pre_commit.txt ./ | ||
RUN uv pip install -r requirements_test.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Constraints not respected during dev dependency install
As in the production Dockerfile, the install lines omit --constraint
for package_constraints.txt
, so local dev containers may end up with different dependency versions than CI.
-uv pip install -r requirements.txt
+uv pip install -r requirements.txt --constraint homeassistant/package_constraints.txt
Apply the same for requirements_test*.txt
.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Dockerfile.dev around lines 52 to 56, the pip install commands for both
requirements.txt and requirements_test.txt do not use the --constraint option
with package_constraints.txt, which can cause inconsistent dependency versions
between local dev and CI environments. Update the RUN commands to include
--constraint=homeassistant/package_constraints.txt when installing both
requirements.txt and requirements_test.txt to ensure consistent dependency
resolution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/bootstrap (1)
9-9
: Consider adding dependency checks for robustness.The script assumes
python3
andpip
are available. Consider adding checks to provide clearer error messages if these dependencies are missing.+# Check if python3 is available +if ! command -v python3 >/dev/null 2>&1; then + echo "Error: python3 is not installed or not in PATH" >&2 + exit 1 +fi + python3 -m pip install -r requirements.txtscripts/setup (1)
1-1
: Consider shell interpreter consistency.The
scripts/setup
uses#!/usr/bin/env bash
whilescripts/bootstrap
uses#!/bin/sh
. While this works fine, consider standardizing on one interpreter for consistency across the project's shell scripts.Also applies to: 7-7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.devcontainer/devcontainer.json
(1 hunks)scripts/bootstrap
(1 hunks)scripts/setup
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .devcontainer/devcontainer.json
🔇 Additional comments (5)
scripts/bootstrap (3)
1-1
: LGTM: Good choice of shell interpreter.Using
#!/bin/sh
ensures POSIX compliance and broad compatibility across different systems.
4-5
: LGTM: Proper error handling.The
set -e
directive ensures the script exits immediately if any command fails, which is essential for reliable automation scripts.
7-7
: LGTM: Correct directory resolution.The script properly changes to the project root directory using the script's location as a reference point.
scripts/setup (2)
7-7
: LGTM: Good separation of concerns.Delegating dependency installation to the dedicated
scripts/bootstrap
script improves maintainability and follows the single responsibility principle.
7-7
: scripts/bootstrap is executable
Confirmed thatscripts/bootstrap
has-rwxr-xr-x
permissions. No further action required.
Summary by CodeRabbit