[pd] allow dynamic upstream interface in _dhcpcd

[yangsong-cnyn]

This UPSTREAM_INTERFACE variable is updated to dynamically use the INFRA_IF_NAME environment variable. This fix the error where configuration of the upstream network interface isn't applied to PD.

It also embeds dhcpcd hooks into _dhcpcd to include the dynamic UPSTREAM_INTERFACE

[gemini-code-assist]

Summary of Changes

Hello @yangsong-cnyn, I'm Gemini Code Assist¹! 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 refactors the DHCPv6 Prefix Delegation (PD) configuration by introducing dynamic handling of the upstream network interface and embedding dhcpcd hook scripts directly into the main configuration script. This change addresses an issue where upstream interface settings were not correctly applied to PD, ensuring that radvd configurations are properly updated and reloaded based on dynamic network conditions and delegated IPv6 prefixes.

Highlights

• Dynamic Upstream Interface: The UPSTREAM_INTERFACE variable now dynamically uses the INFRA_IF_NAME environment variable, falling back to eth0 if not set. This ensures the correct upstream interface is used for DHCPv6 Prefix Delegation (PD) configurations.
• Embedded DHCPCD Hooks: The dhcpcd.enter-hook and dhcpcd.exit-hook scripts are no longer static files but are now dynamically generated and embedded within the _dhcpv6_pd_ref script, centralizing their management.
• Improved Prefix Delegation Handling: The newly embedded hooks correctly handle IPv6 prefix delegation events (e.g., DELEGATED6, EXPIRE6), updating radvd.conf to deprecate old prefixes and publish new ones, and reloading the radvd service as needed.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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. ↩

[yangsong-cnyn]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the dhcpcd hook script creation by embedding them into the _dhcpv6_pd_ref script. This is a good change that allows for dynamic configuration of the upstream interface. However, there are several issues that need to be addressed.

Most importantly, the new functions create_dhcpcd_enter_hook and create_dhcpcd_exit_hook are defined but are not called within the dhcpv6_pd_ref_install function. The dhcpv6_pd_ref_install function still attempts to install the hook scripts from their old location (script/reference-device/), but these files are deleted in this pull request. This will cause the setup script to fail. You need to modify dhcpv6_pd_ref_install to call these new functions.

Additionally, there are several issues in the generated hook scripts, including a critical error in a sudo command, a security vulnerability related to logging environment variables, and multiple instances of unquoted variables which can lead to bugs. Please see the detailed comments for specifics.

[gemini-code-assist]

Code Review

This pull request refactors the DHCPv6 PD setup by dynamically generating the dhcpcd hook scripts instead of using static files. This allows for a dynamic UPSTREAM_INTERFACE. The implementation looks mostly correct, but I've found a few critical issues that will prevent the script from working as intended, including a typo in a sudo command and uncalled functions. There are also a couple of logic and robustness improvements I've suggested.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.53%. Comparing base (2b41187) to head (a36b698).
⚠️ Report is 1382 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3125      +/-   ##
==========================================
- Coverage   55.77%   46.53%   -9.24%     
==========================================
  Files          87      145      +58     
  Lines        6890    17732   +10842     
  Branches        0     1472    +1472     
==========================================
+ Hits         3843     8252    +4409     
- Misses       3047     8900    +5853     
- Partials        0      580     +580     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[yangsong-cnyn]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates the _dhcpv6_pd_ref script to allow for a dynamic upstream interface by using the INFRA_IF_NAME environment variable. This is accomplished by dynamically generating the dhcpcd hook scripts, which previously used a hardcoded interface name. This change effectively resolves the issue of applying the correct upstream network interface configuration. The implementation is sound, but I've identified a couple of areas for improvement in the generated shell scripts related to variable quoting and the redundant use of sudo, which I've detailed in the comments.

[yangsong-cnyn]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the dhcpcd hook scripts to be dynamically generated, allowing the use of a configurable upstream interface via the INFRA_IF_NAME environment variable. This is a good improvement over the previous hardcoded value. The logic from the static hook files has been moved into _dhcpv6_pd_ref and is now embedded into the hooks at installation time. My review has identified a critical issue where sudo has been omitted from commands that write to /etc/radvd.conf within the generated hook scripts. This could lead to permission errors and prevent the router advertisement configuration from being updated.

[yangsong-cnyn]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the dhcpcd script to allow for a dynamic upstream interface and embeds the dhcpcd hook scripts directly, which improves configurability and maintainability. The overall approach is good, but I've found a couple of critical issues in the implementation of the generated hook scripts that would cause them to fail. Please see the detailed comments for fixes.

[suveshpratapa]

Please rebase after changes in #3122

[yangsong-cnyn]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the dhcpcd configuration to dynamically determine the upstream interface from the INFRA_IF_NAME environment variable, which is a good improvement over the previous hardcoded value. The change also embeds the dhcpcd hook scripts directly into the _dhcpcd setup script, which centralizes the logic. My review focuses on improving the robustness and correctness of the generated hook scripts. I've pointed out unnecessary sudo calls, which can be problematic, and a missing variable quote. Overall, the changes are in the right direction.

[yangsong-cnyn]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the _dhcpcd script to dynamically determine the upstream interface and embeds the dhcpcd hook scripts, which is a great improvement. The logic for generating the hook scripts is sound. My review includes a few suggestions to enhance the shell scripting parts for better robustness and maintainability, such as using modern bash constructs like [[...]] for tests and ensuring consistent error handling.

[yangsong-cnyn]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a nice improvement. It makes the upstream interface configurable dynamically, which is a great enhancement for flexibility. Embedding the dhcpcd hook scripts directly into the main _dhcpcd script is a good simplification that makes the setup more self-contained and easier to manage. I have a couple of suggestions to improve the robustness and readability of the generated hook scripts.