Skip to content
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

Jump to bottom

Add Kconfig option for Supervisor channel #3618

Merged
merged 4 commits into from
Nov 18, 2024
Merged

Add Kconfig option for Supervisor channel #3618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e1f757c
Add Makefile variable for Supervisor channel
agners Oct 10, 2024
cf8446c
Use choice instead of string variable
agners Nov 18, 2024
8cfa907
Fix channel in Supervisor updater.json config
agners Nov 18, 2024
c21cca3
Add newlines
agners Nov 18, 2024
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
23 changes: 23 additions & 0 deletions buildroot-external/package/hassio/Config.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,27 @@ config BR2_PACKAGE_HASSIO_MACHINE
help
Machine to pull containers for (used for landing page).

choice
prompt "Default Channel"
default BR2_PACKAGE_HASSIO_CHANNEL_STABLE
help
Channel to use by default.

config BR2_PACKAGE_HASSIO_CHANNEL_STABLE
agners marked this conversation as resolved.
Show resolved Hide resolved
bool "Stable"
help
Stable channel.

config BR2_PACKAGE_HASSIO_CHANNEL_BETA
bool "Beta"
help
Beta channel.

config BR2_PACKAGE_HASSIO_CHANNEL_DEV
bool "Dev"
help
Dev channel.

endchoice

Comment on lines +42 to +43
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing string config for channel value mapping

The choice configuration needs to be followed by a string config that maps the selected choice to actual channel values. This string config will be used by other scripts to determine the selected channel.

Add this configuration after the choice block:

 endchoice
+
+config BR2_PACKAGE_HASSIO_CHANNEL
+	string
+	default "stable" if BR2_PACKAGE_HASSIO_CHANNEL_STABLE
+	default "beta" if BR2_PACKAGE_HASSIO_CHANNEL_BETA
+	default "dev" if BR2_PACKAGE_HASSIO_CHANNEL_DEV
📝 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
endchoice
endchoice
config BR2_PACKAGE_HASSIO_CHANNEL
string
default "stable" if BR2_PACKAGE_HASSIO_CHANNEL_STABLE
default "beta" if BR2_PACKAGE_HASSIO_CHANNEL_BETA
default "dev" if BR2_PACKAGE_HASSIO_CHANNEL_DEV

endif
3 changes: 2 additions & 1 deletion buildroot-external/package/hassio/create-data-partition.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ set -e

build_dir=$1
dst_dir=$2
channel=$3
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add parameter validation for the channel argument

The script should validate that the required channel parameter is provided and contains a valid value.

Add this validation after the parameter declaration:

 channel=$3
+
+# Validate channel parameter
+if [ -z "${channel}" ]; then
+    echo "Error: Channel parameter is required"
+    echo "Usage: $0 <build_dir> <dst_dir> <channel>"
+    exit 1
+fi
+
+# Validate channel value
+case "${channel}" in
+    stable|beta|dev) ;;
+    *)
+        echo "Error: Invalid channel '${channel}'. Must be one of: stable, beta, dev"
+        exit 1
+        ;;
+esac
📝 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
channel=$3
channel=$3
# Validate channel parameter
if [ -z "${channel}" ]; then
echo "Error: Channel parameter is required"
echo "Usage: $0 <build_dir> <dst_dir> <channel>"
exit 1
fi
# Validate channel value
case "${channel}" in
stable|beta|dev) ;;
*)
echo "Error: Invalid channel '${channel}'. Must be one of: stable, beta, dev"
exit 1
;;
esac


data_img="${dst_dir}/data.ext4"

Expand All @@ -24,7 +25,7 @@ container=$(docker run --privileged -e DOCKER_TLS_CERTDIR="" \
-v "${build_dir}":/build \
-d docker:27.2-dind --storage-driver overlay2)

docker exec "${container}" sh /build/dind-import-containers.sh
docker exec "${container}" sh /build/dind-import-containers.sh "${channel}"

docker stop "${container}"

Expand Down
4 changes: 4 additions & 0 deletions buildroot-external/package/hassio/dind-import-containers.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#!/bin/sh
set -e

channel=$1

Comment on lines +4 to +5
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add parameter validation and documentation.

The script now accepts a channel parameter but lacks validation and documentation. Consider these improvements:

 #!/bin/sh
 set -e
 
+# Usage: dind-import-containers.sh <channel>
+# <channel>: The release channel to use (e.g., "stable", "beta", "dev")
+
+if [ -z "$1" ]; then
+    echo "Error: Channel parameter is required"
+    echo "Usage: $0 <channel>"
+    exit 1
+fi
+
 channel=$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.

Suggested change
channel=$1
#!/bin/sh
set -e
# Usage: dind-import-containers.sh <channel>
# <channel>: The release channel to use (e.g., "stable", "beta", "dev")
if [ -z "$1" ]; then
echo "Error: Channel parameter is required"
echo "Usage: $0 <channel>"
exit 1
fi
channel=$1

APPARMOR_URL="https://version.home-assistant.io/apparmor.txt"

# Make sure we can talk to the Docker daemon
Expand All @@ -27,3 +29,5 @@ docker tag "${supervisor}" "ghcr.io/home-assistant/${arch}-hassio-supervisor:lat
# Setup AppArmor
mkdir -p "/data/supervisor/apparmor"
wget -O "/data/supervisor/apparmor/hassio-supervisor" "${APPARMOR_URL}"

echo "{ \"channel\": \"${channel}\" }" > /data/supervisor/updater.json
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve JSON file creation robustness and security.

The current implementation has several potential issues:

  1. No directory existence check
  2. No error handling
  3. Potential JSON injection vulnerability
  4. Hard-coded path

Consider this more robust implementation:

-echo "{ \"channel\": \"${channel}\" }" > /data/supervisor/updater.json
+UPDATER_DIR="/data/supervisor"
+UPDATER_FILE="${UPDATER_DIR}/updater.json"
+
+# Ensure directory exists
+mkdir -p "${UPDATER_DIR}" || {
+    echo "Error: Failed to create directory ${UPDATER_DIR}"
+    exit 1
+}
+
+# Create JSON with proper escaping
+printf '{"channel":"%s"}\n' "$(echo "${channel}" | sed 's/[\"]/\\&/g')" > "${UPDATER_FILE}" || {
+    echo "Error: Failed to write to ${UPDATER_FILE}"
+    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.

Suggested change
echo "{ \"channel\": \"${channel}\" }" > /data/supervisor/updater.json
UPDATER_DIR="/data/supervisor"
UPDATER_FILE="${UPDATER_DIR}/updater.json"
# Ensure directory exists
mkdir -p "${UPDATER_DIR}" || {
echo "Error: Failed to create directory ${UPDATER_DIR}"
exit 1
}
# Create JSON with proper escaping
printf '{"channel":"%s"}\n' "$(echo "${channel}" | sed 's/[\"]/\\&/g')" > "${UPDATER_FILE}" || {
echo "Error: Failed to write to ${UPDATER_FILE}"
exit 1
}

15 changes: 11 additions & 4 deletions buildroot-external/package/hassio/hassio.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,35 @@ HASSIO_LICENSE = Apache License 2.0
# HASSIO_LICENSE_FILES = $(BR2_EXTERNAL_HASSOS_PATH)/../LICENSE
HASSIO_SITE = $(BR2_EXTERNAL_HASSOS_PATH)/package/hassio
HASSIO_SITE_METHOD = local
HASSIO_VERSION_URL = "https://version.home-assistant.io/stable.json"
HASSIO_VERSION_URL = "https://version.home-assistant.io/"
ifeq ($(BR2_PACKAGE_HASSIO_CHANNEL_STABLE),y)
HASSIO_VERSION_CHANNEL = "stable"
else ifeq ($(BR2_PACKAGE_HASSIO_CHANNEL_BETA),y)
HASSIO_VERSION_CHANNEL = "beta"
else ifeq ($(BR2_PACKAGE_HASSIO_CHANNEL_DEV),y)
HASSIO_VERSION_CHANNEL = "dev"
endif

HASSIO_CONTAINER_IMAGES_ARCH = supervisor dns audio cli multicast observer core

define HASSIO_CONFIGURE_CMDS
# Deploy only landing page for "core" by setting version to "landingpage"
curl -s $(HASSIO_VERSION_URL) | jq '.core = "landingpage"' > $(@D)/stable.json
curl -s $(HASSIO_VERSION_URL)$(HASSIO_VERSION_CHANNEL)".json" | jq '.core = "landingpage"' > $(@D)/version.json
endef

define HASSIO_BUILD_CMDS
$(Q)mkdir -p $(@D)/images
$(Q)mkdir -p $(HASSIO_DL_DIR)
$(foreach image,$(HASSIO_CONTAINER_IMAGES_ARCH),\
$(BR2_EXTERNAL_HASSOS_PATH)/package/hassio/fetch-container-image.sh \
$(BR2_PACKAGE_HASSIO_ARCH) $(BR2_PACKAGE_HASSIO_MACHINE) $(@D)/stable.json $(image) "$(HASSIO_DL_DIR)" "$(@D)/images"
$(BR2_PACKAGE_HASSIO_ARCH) $(BR2_PACKAGE_HASSIO_MACHINE) $(@D)/version.json $(image) "$(HASSIO_DL_DIR)" "$(@D)/images"
)
endef

HASSIO_INSTALL_IMAGES = YES

define HASSIO_INSTALL_IMAGES_CMDS
$(BR2_EXTERNAL_HASSOS_PATH)/package/hassio/create-data-partition.sh "$(@D)" "$(BINARIES_DIR)"
$(BR2_EXTERNAL_HASSOS_PATH)/package/hassio/create-data-partition.sh "$(@D)" "$(BINARIES_DIR)" "$(HASSIO_VERSION_CHANNEL)"
endef

$(eval $(generic-package))