Skip to content

Conversation

lllamnyp
Copy link
Member

@lllamnyp lllamnyp commented Aug 21, 2025

This patch adds the lldpd extension to Cozystack's Talos build. Additionally it changes the profile generation scripts to use
Siderolabs' recommended way to get appropriate extension image versions to include with Talos.

Release note

[talos] Add LLDP support in default Talos build.

Signed-off-by: Timofei Larkin lllamnyp@gmail.com

@lllamnyp lllamnyp requested review from kvaps and klinch0 as code owners August 21, 2025 07:41
Copy link
Contributor

coderabbitai bot commented Aug 21, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds LLDPD as a system extension image and as a kernel module in Talos profiles and the e2e cluster preparation. Also converts many installer/profile image references from tag-based to digest-pinned imageRefs and switches gen-profiles to produce _IMAGE variables driven by image-digests.

Changes

Cohort / File(s) Summary
E2E cluster patch
hack/e2e-prepare-cluster.bats
Inserts "lldpd" into machine.kernel.modules in the generated Talos cluster configuration patch (placed after "spl").
Profile generator script
packages/core/installer/hack/gen-profiles.sh
Adds lldpd to EXTENSIONS; switches from tag-based _VERSION variables to image-digests-driven _IMAGE variables (e.g., LLDPD_IMAGE), and emits extension imageRefs using the new _IMAGE variables.
Talos installer/initramfs/iso/kernel/metal/nocloud profiles
packages/core/installer/images/talos/profiles/*.yaml
Bumps profile versions v1.10.5 → v1.10.6; updates input.baseInstaller.imageRef to v1.10.6; replaces many tag-only imageRefs with digest-pinned imageRef: ...@sha256:...; adds new systemExtensions entries (notably amdgpu, i915, and lldpd); adds outFormat/`outFormat: raw

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant Gen as gen-profiles.sh
  participant Registry as Image Registry (ghcr.io)
  participant Profiles as Talos Profiles (YAML)
  participant E2E as e2e-prepare-cluster.bats
  participant Cluster as Talos Cluster

  Dev->>Gen: run profile generation
  Gen->>Registry: fetch image digests (crane) → image-digests
  Registry-->>Gen: digest pins
  Gen->>Profiles: emit _IMAGE vars and systemExtensions (incl. LLDPD_IMAGE)

  Dev->>E2E: run e2e prepare
  E2E->>Cluster: apply cluster config patch (machine.kernel.modules includes lldpd)
  Cluster->>Cluster: kernel modules list includes lldpd (load on boot)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • klinch0
  • kvaps

Poem

I twitch my ears at lldpd’s name,
New carrot in the patchwork game.
Profiles baked with digests tight—
Modules load at morning light.
Hop, test, and nibble — code looks spry. 🥕🐇

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/lldpd-extension-talos

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @lllamnyp, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request integrates the lldpd extension into the Talos Linux image. This enhancement allows Talos systems to utilize the Link Layer Discovery Protocol, improving network discoverability and management capabilities within the environment.

Highlights

  • Enable lldpd extension: The lldpd extension has been added to the list of machine extensions in the e2e cluster preparation script, ensuring it's available for testing and deployment.
  • Include lldpd in image profiles: The lldpd extension is now explicitly included in the EXTENSIONS variable used by the profile generation script, ensuring it's bundled into the various Talos image profiles.
  • Reference lldpd image: The necessary image reference for lldpd has been added to the input section of the image profile generation, allowing the build system to pull and integrate the lldpd component.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly modifies the scripts to include the lldpd extension. However, a critical part of the change is missing: the generated profile files under packages/core/installer/images/talos/profiles/ have not been updated. Please run packages/core/installer/hack/gen-profiles.sh and commit the resulting changes to these YAML files to complete the PR. I've also left a comment regarding a potential improvement in the shell script to avoid code duplication and improve maintainability.

@@ -4,7 +4,7 @@ set -u

PROFILES="initramfs kernel iso installer nocloud metal"
FIRMWARES="amd-ucode amdgpu-firmware bnx2-bnx2x i915-ucode intel-ice-firmware intel-ucode qlogic-firmware"
EXTENSIONS="drbd zfs"
EXTENSIONS="drbd zfs lldpd"

Choose a reason for hiding this comment

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

medium

While adding lldpd here is correct, it highlights a maintainability issue. The list of extensions is defined in this EXTENSIONS variable, but it's also manually maintained in the systemExtensions list within the YAML template on lines 86-88. This duplication is error-prone, as one might be updated without the other.

To improve this, consider generating the systemExtensions list dynamically from this EXTENSIONS variable. This would make the script more robust and easier to maintain.

Copy link
Member

@kvaps kvaps left a comment

Choose a reason for hiding this comment

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

Could you please also run make generate to update all the profiles÷

@lllamnyp
Copy link
Member Author

Could you please also run make generate to update all the profiles÷

The gen-profiles script doesn't work for lldpd. The image doesn't contain the Talos version in the tag. Sidero recommends this command:

$ crane export ghcr.io/siderolabs/extensions:v1.10.6 | tar x -O image-digests | grep lldpd
ghcr.io/siderolabs/lldpd:1.0.19@sha256:73caa3c3a6c325970d0f527963f982698154d5f39c8c045b0fc2eb51d7da7b85

perhaps the scripts should be updated to use crane as well?

@lllamnyp lllamnyp force-pushed the feat/lldpd-extension-talos branch from d14197b to c9e9dd2 Compare August 26, 2025 08:05
This patch adds the lldpd extension to Cozystack's Talos build.
Additionally it changes the profile generation scripts to use
Siderolabs' recommended way to get appropriate extension image versions
to include with Talos.

Release note:
```release-note
[talos] Add LLDP support in default Talos build.
```

Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
@lllamnyp lllamnyp force-pushed the feat/lldpd-extension-talos branch from c9e9dd2 to 8681487 Compare August 26, 2025 08:10
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/core/installer/hack/gen-profiles.sh (1)

13-13: Regex bug: tag filter can select wrong versions or fail on non-GNU grep

The pattern uses BRE with \+ and unescaped dots. On BusyBox/Dash, \+ may be literal, and . matches any char. Use ERE and escape dots.

- talos_version=${1:-$(skopeo --override-os linux --override-arch amd64 list-tags docker://ghcr.io/siderolabs/imager | jq -r '.Tags[]' | grep '^v[0-9]\+.[0-9]\+.[0-9]\+$' | sort -V | tail -n 1)}
+ talos_version=${1:-$(skopeo --override-os linux --override-arch amd64 list-tags docker://ghcr.io/siderolabs/imager \
+   | jq -r '.Tags[]' \
+   | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' \
+   | sort -V | tail -n 1)}
♻️ Duplicate comments (1)
packages/core/installer/hack/gen-profiles.sh (1)

83-93: Avoid duplicated, hand-maintained lists for systemExtensions

This block must be kept in sync with FIRMWARES/EXTENSIONS. Generate it from those arrays to prevent drift. Similar feedback was shared previously.

-  systemExtensions:
-    - imageRef: ${AMD_UCODE_IMAGE}
-    - imageRef: ${AMDGPU_IMAGE}
-    - imageRef: ${BNX2_BNX2X_IMAGE}
-    - imageRef: ${INTEL_ICE_FIRMWARE_IMAGE}
-    - imageRef: ${I915_IMAGE}
-    - imageRef: ${INTEL_UCODE_IMAGE}
-    - imageRef: ${QLOGIC_FIRMWARE_IMAGE}
-    - imageRef: ${DRBD_IMAGE}
-    - imageRef: ${ZFS_IMAGE}
-    - imageRef: ${LLDPD_IMAGE}
+  systemExtensions:
+$(for name in $FIRMWARES $EXTENSIONS; do
+  var=$(echo "$name" | tr '[:lower:]' '[:upper:]' | tr - _)_IMAGE
+  printf "    - imageRef: %s\n" "$(eval "printf '%s' \"\$$var\"")"
+done)
🧹 Nitpick comments (2)
packages/core/installer/hack/gen-profiles.sh (1)

1-4: Preflight tool checks for better UX

Fail early with clear messages if required tools are missing: skopeo, jq, crane, tar.

 #!/bin/sh
 set -e
 set -u
+for tool in skopeo jq crane tar; do
+  command -v "$tool" >/dev/null 2>&1 || { echo "error: '$tool' is required" >&2; exit 1; }
+done
packages/core/installer/images/talos/profiles/metal.yaml (1)

13-13: Consider digest-pinning the base installer image for full reproducibility

All systemExtensions are pinned to digests (great), but the base installer still references a tag. To keep builds hermetic and auditable, consider pinning this as well (and ideally have hack/gen-profiles.sh emit it).

Apply after you obtain the digest (via crane or skopeo):

-    imageRef: "ghcr.io/siderolabs/installer:v1.10.6"
+    imageRef: "ghcr.io/siderolabs/installer:v1.10.6@sha256:<installer-digest>"

I can wire up gen-profiles.sh to resolve and inject the digest automatically if you want.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d14197b and 8681487.

📒 Files selected for processing (8)
  • hack/e2e-prepare-cluster.bats (1 hunks)
  • packages/core/installer/hack/gen-profiles.sh (3 hunks)
  • packages/core/installer/images/talos/profiles/initramfs.yaml (1 hunks)
  • packages/core/installer/images/talos/profiles/installer.yaml (1 hunks)
  • packages/core/installer/images/talos/profiles/iso.yaml (1 hunks)
  • packages/core/installer/images/talos/profiles/kernel.yaml (1 hunks)
  • packages/core/installer/images/talos/profiles/metal.yaml (1 hunks)
  • packages/core/installer/images/talos/profiles/nocloud.yaml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/core/installer/images/talos/profiles/installer.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • hack/e2e-prepare-cluster.bats
⏰ 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: Build
🔇 Additional comments (7)
packages/core/installer/images/talos/profiles/kernel.yaml (2)

15-24: LGTM: digest-pinned systemExtensions incl. lldpd

All extensions are pinned with sha256 digests; lldpd 1.0.19 is present. This improves reproducibility.


6-14: Manual Digest Verification Required

I attempted to run the provided script but crane isn’t available in this environment, so I couldn’t extract the image-digests automatically. Please manually verify that the lldpd digest in packages/core/installer/images/talos/profiles/kernel.yaml matches the one in the ghcr.io/siderolabs/extensions:v1.10.6 image. For example:

  • Pull and extract the digests from the container:
    docker pull ghcr.io/siderolabs/extensions:v1.10.6
    container=$(docker create ghcr.io/siderolabs/extensions:v1.10.6)
    docker cp "$container":/image-digests /tmp/ext-digests
    docker rm "$container"
  • Compare the lldpd entries:
    grep -F '/lldpd:' /tmp/ext-digests
    rg -nP 'lldpd:.*@sha256:[a-f0-9]+' packages/core/installer/images/talos/profiles/kernel.yaml

Ensure the SHA256 digest for /lldpd: in both outputs is identical.

packages/core/installer/images/talos/profiles/nocloud.yaml (1)

15-24: LGTM: nocloud profile updated with pinned digests and lldpd; outFormat .xz set

Looks consistent with other profiles; no issues noticed.

packages/core/installer/images/talos/profiles/iso.yaml (1)

15-24: LGTM: iso profile adopts digest pins and includes lldpd

Consistent with kernel/initramfs; reproducible and aligned with v1.10.6.

packages/core/installer/images/talos/profiles/initramfs.yaml (1)

15-24: LGTM: initramfs profile includes lldpd and digest pins

No functional concerns here.

packages/core/installer/images/talos/profiles/metal.yaml (2)

6-6: Version bump to v1.10.6: confirm parity across all Talos profiles and release notes entry

Looks good here. Please confirm that every generated Talos profile (initramfs/installer/iso/kernel/nocloud/metal) is on the same version and that the release notes include this version bump and the new lldpd extension.

Would you like me to open a checklist for cross-profile parity and release-note updates?


15-24: Ensure Crane Is Installed Before Running Digest Verification

The digest pins look consistent, and the addition of lldpd 1.0.19 with its SHA-256 is great. To guard against registry drift, please install crane in your environment (or CI) and rerun the following verification script against packages/core/installer/images/talos/profiles/metal.yaml. If installing crane is not feasible, manually verify that each @sha256: matches the registry’s canonical digest.

#!/usr/bin/env bash
set -euo pipefail

FILE="packages/core/installer/images/talos/profiles/metal.yaml"

# Ensure crane is available
if ! command -v crane &>/dev/null; then
  echo "Error: 'crane' is required. Install it from https://github.com/google/go-containerregistry."
  exit 1
fi

# Extract all imageRefs
mapfile -t IMAGES < <(
  awk -F': ' '/^\s*- imageRef:/{print $2}' "$FILE" | tr -d '"'
)

FAIL=0
for ref in "${IMAGES[@]}"; do
  want="${ref##*@}"
  repo="${ref%@*}"
  got="$(crane digest "$repo" | sed 's/^sha256://')"
  if [[ "$got" != "$want" ]]; then
    echo "Mismatch: $repo → got sha256:$got, want sha256:$want"
    FAIL=1
  fi
done

if [[ $FAIL -eq 0 ]]; then
  echo "✓ All digests validated."
else
  echo "✗ One or more digests mismatched." >&2
  exit 2
fi

• If you cannot install crane, please manually query each <repo>:<tag> via your preferred container registry tool to confirm its SHA-256 matches the pinned digest.
• Optionally, extend this check to the baseInstaller profile once it is likewise digest-pinned.

@@ -2,9 +2,10 @@
set -e
set -u

TMPDIR=$(mktemp -d)
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

No cleanup for mktemp directory

Add a trap to remove the temporary directory on exit.

 TMPDIR=$(mktemp -d)
+trap 'rm -rf "$TMPDIR"' EXIT
📝 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
TMPDIR=$(mktemp -d)
TMPDIR=$(mktemp -d)
trap 'rm -rf "$TMPDIR"' EXIT
🤖 Prompt for AI Agents
In packages/core/installer/hack/gen-profiles.sh around line 5, the temporary
directory created with TMPDIR=$(mktemp -d) has no cleanup; add a trap to remove
the directory on script exit and on common signals by registering something
like: trap 'rm -rf -- "$TMPDIR"' EXIT INT TERM; ensure TMPDIR is quoted and use
rm -rf with -- to safely remove the temp directory, and set TMPDIR only after
mktemp succeeds so the trap always has a valid path.

@@ -14,20 +15,22 @@ echo "$talos_version"

export "TALOS_VERSION=$talos_version"

crane export ghcr.io/siderolabs/extensions:v1.10.6 | tar x -O image-digests > $TMPDIR/image-digests
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Hard-coded extensions version causes profile/image mismatch

Extensions are pinned from v1.10.6 regardless of computed TALOS_VERSION, risking inconsistent images (e.g., base installer vX.Y.Z but extensions from v1.10.6).

-crane export ghcr.io/siderolabs/extensions:v1.10.6 | tar x -O image-digests > $TMPDIR/image-digests
+crane export "ghcr.io/siderolabs/extensions:${TALOS_VERSION}" \
+  | tar x -O image-digests > "$TMPDIR/image-digests"
📝 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
crane export ghcr.io/siderolabs/extensions:v1.10.6 | tar x -O image-digests > $TMPDIR/image-digests
crane export "ghcr.io/siderolabs/extensions:${TALOS_VERSION}" \
| tar x -O image-digests > "$TMPDIR/image-digests"
🤖 Prompt for AI Agents
In packages/core/installer/hack/gen-profiles.sh around line 18, the script
currently hard-codes the extensions tag to v1.10.6 which can mismatch the
computed TALOS_VERSION; change the command to use a variable (e.g.
$TALOS_VERSION or an explicitly computed EXTENSIONS_TAG derived from
TALOS_VERSION) instead of the literal v1.10.6 so the exported extensions image
matches the installer version, ensure that the variable is set/validated before
use (exit with an error if unset) and update any callers or defaults so the
variable is populated when the script runs.

Comment on lines 20 to 26
for firmware in $FIRMWARES; do
printf "fetching %s version: " "$firmware"
firmware_var=$(echo "$firmware" | tr '[:lower:]' '[:upper:]' | tr - _)_VERSION
version=$(skopeo list-tags docker://ghcr.io/siderolabs/$firmware | jq -r '.Tags[]|select(length == 8)|select(startswith("20"))' | sort -V | tail -n 1)
echo "$version"
export "$firmware_var=$version"
firmware_var=$(echo "$firmware" | tr '[:lower:]' '[:upper:]' | tr - _)_IMAGE
image=$(grep $firmware $TMPDIR/image-digests)
echo "$image"
export "$firmware_var=$image"
done
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fragile grep; prefer exact, quoted, fixed-string match

grep $firmware $TMPDIR/image-digests is unquoted and regex-based. Use fixed-string search scoped to /${name}: to avoid partial matches and quoting issues.

-  firmware_var=$(echo "$firmware" | tr '[:lower:]' '[:upper:]' | tr - _)_IMAGE
-  image=$(grep $firmware $TMPDIR/image-digests)
+  firmware_var=$(echo "$firmware" | tr '[:lower:]' '[:upper:]' | tr - _)_IMAGE
+  image=$(grep -F "/${firmware}:" "$TMPDIR/image-digests")
   echo "$image"
   export "$firmware_var=$image"
📝 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
for firmware in $FIRMWARES; do
printf "fetching %s version: " "$firmware"
firmware_var=$(echo "$firmware" | tr '[:lower:]' '[:upper:]' | tr - _)_VERSION
version=$(skopeo list-tags docker://ghcr.io/siderolabs/$firmware | jq -r '.Tags[]|select(length == 8)|select(startswith("20"))' | sort -V | tail -n 1)
echo "$version"
export "$firmware_var=$version"
firmware_var=$(echo "$firmware" | tr '[:lower:]' '[:upper:]' | tr - _)_IMAGE
image=$(grep $firmware $TMPDIR/image-digests)
echo "$image"
export "$firmware_var=$image"
done
for firmware in $FIRMWARES; do
printf "fetching %s version: " "$firmware"
firmware_var=$(echo "$firmware" | tr '[:lower:]' '[:upper:]' | tr - _)_IMAGE
image=$(grep -F "/${firmware}:" "$TMPDIR/image-digests")
echo "$image"
export "$firmware_var=$image"
done
🤖 Prompt for AI Agents
In packages/core/installer/hack/gen-profiles.sh around lines 20 to 26, the line
using grep is fragile because it is unquoted and performs a regex match which
can produce partial matches; change it to quote variables and perform a
fixed-string search scoped to the name plus colon (e.g. search for
"${firmware}:") and pass the filename as a separate, quoted argument (or use
awk/while-read to match the start of line) so you get exact, quoted matches and
avoid regex/word-splitting issues.

Comment on lines 28 to 34
for extension in $EXTENSIONS; do
printf "fetching %s version: " "$extension"
extension_var=$(echo "$extension" | tr '[:lower:]' '[:upper:]' | tr - _)_VERSION
version=$(skopeo --override-os linux --override-arch amd64 list-tags docker://ghcr.io/siderolabs/$extension | jq -r '.Tags[]' | grep "\-${talos_version}$" | sort -V | tail -n1)
echo "$version"
export "$extension_var=$version"
extension_var=$(echo "$extension" | tr '[:lower:]' '[:upper:]' | tr - _)_IMAGE
image=$(grep $extension $TMPDIR/image-digests)
echo "$image"
export "$extension_var=$image"
done
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Same grep robustness applies to extensions loop

Mirror the fixed-string match and quoting for extensions.

-  extension_var=$(echo "$extension" | tr '[:lower:]' '[:upper:]' | tr - _)_IMAGE
-  image=$(grep $extension $TMPDIR/image-digests)
+  extension_var=$(echo "$extension" | tr '[:lower:]' '[:upper:]' | tr - _)_IMAGE
+  image=$(grep -F "/${extension}:" "$TMPDIR/image-digests")
   echo "$image"
   export "$extension_var=$image"
📝 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
for extension in $EXTENSIONS; do
printf "fetching %s version: " "$extension"
extension_var=$(echo "$extension" | tr '[:lower:]' '[:upper:]' | tr - _)_VERSION
version=$(skopeo --override-os linux --override-arch amd64 list-tags docker://ghcr.io/siderolabs/$extension | jq -r '.Tags[]' | grep "\-${talos_version}$" | sort -V | tail -n1)
echo "$version"
export "$extension_var=$version"
extension_var=$(echo "$extension" | tr '[:lower:]' '[:upper:]' | tr - _)_IMAGE
image=$(grep $extension $TMPDIR/image-digests)
echo "$image"
export "$extension_var=$image"
done
for extension in $EXTENSIONS; do
printf "fetching %s version: " "$extension"
extension_var=$(echo "$extension" | tr '[:lower:]' '[:upper:]' | tr - _)_IMAGE
image=$(grep -F "/${extension}:" "$TMPDIR/image-digests")
echo "$image"
export "$extension_var=$image"
done
🤖 Prompt for AI Agents
In packages/core/installer/hack/gen-profiles.sh around lines 28-34, the grep
call inside the extensions loop should use fixed-string matching and proper
quoting to avoid treating extension names as regexes or word-splitting; replace
the grep invocation with a fixed-string, end-of-options form (e.g. grep -F --
"$extension" "$TMPDIR/image-digests"), and ensure all variable expansions are
quoted when used (echo "$image" and export using the existing export
"$extension_var=$image" pattern) so matches are exact and values aren’t subject
to word-splitting.

@kvaps kvaps merged commit 32a857f into main Aug 26, 2025
20 checks passed
@kvaps kvaps deleted the feat/lldpd-extension-talos branch August 26, 2025 12:36
lllamnyp added a commit that referenced this pull request Aug 26, 2025
Some version strings were accidentally hardcoded instead of retrieving
them dynamically in the profile generator for the Talos build. This
follows up #1351 and fixes these issues.

```release-note
[talos] Add LLDP support and improve profile generation logic.
```

Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
kvaps added a commit that referenced this pull request Aug 26, 2025
## What this PR does

Some version strings were accidentally hardcoded instead of retrieving
them dynamically in the profile generator for the Talos build. This
follows up #1351 and fixes these issues.

### Release note

```release-note
[talos] Add LLDP support and improve profile generation logic.
```

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- Bug Fixes
- Anchored matching for firmware and extension digests to avoid false
positives and incorrect selections during export, improving reliability
of installs.

- Chores
- Switched to dynamic image tagging based on the detected Talos version,
ensuring the correct extension images are exported for each release and
reducing version mismatch issues for more consistent builds.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants