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.

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

dhcp: T3316: Support hostname, DUID and MAC address in reservation #2650

Merged
merged 3 commits into from
Dec 28, 2023

Conversation

indrajitr
Copy link
Contributor

@indrajitr indrajitr commented Dec 18, 2023

Change Summary

Reinstate support for hostname in DHCP reservation. Having hostname in allows for server-side assignment of hostname. This is useful for static lookup of hostname.

Additionally, support using either of DUID or MAC address for reservation. While MAC address is typically used for IPv4, and DUID is typically used for IPv6, both can be used for both cases in Kea.

For details, see:

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

Component(s) name

dhcp-server, dhcpv6-server

Proposed changes

How to test

Smoketest result

vyos@v15-1221:~$ /usr/libexec/vyos/tests/smoke/cli/test_service_dhcp-server.py 
test_dhcp_exclude_in_range (__main__.TestServiceDHCPServer.test_dhcp_exclude_in_range) ... ok
test_dhcp_exclude_not_in_range (__main__.TestServiceDHCPServer.test_dhcp_exclude_not_in_range) ... ok
test_dhcp_failover (__main__.TestServiceDHCPServer.test_dhcp_failover) ... 
No DHCP address range or active static-mapping configured within shared-
network "FAILOVER, 192.0.2.0/25"!

ok
test_dhcp_multiple_pools (__main__.TestServiceDHCPServer.test_dhcp_multiple_pools) ... ok
test_dhcp_relay_server (__main__.TestServiceDHCPServer.test_dhcp_relay_server) ... ok
test_dhcp_single_pool_options (__main__.TestServiceDHCPServer.test_dhcp_single_pool_options) ... 
No DHCP address range or active static-mapping configured within shared-
network "SMOKE-0815, 192.0.2.0/25"!

ok
test_dhcp_single_pool_range (__main__.TestServiceDHCPServer.test_dhcp_single_pool_range) ... 
No DHCP address range or active static-mapping configured within shared-
network "SMOKE-1, 192.0.2.0/25"!

ok
test_dhcp_single_pool_static_mapping (__main__.TestServiceDHCPServer.test_dhcp_single_pool_static_mapping) ... 
No DHCP address range or active static-mapping configured within shared-
network "SMOKE-2, 192.0.2.0/25"!


Either MAC address or Client identifier (DUID) is required for static
mapping "client1" within shared-network "SMOKE-2, 192.0.2.0/25"!

ok

----------------------------------------------------------------------
Ran 8 tests in 198.886s

OK
vyos@v15-1221:~$ /usr/libexec/vyos/tests/smoke/cli/test_service_dhcpv6-server.py 
test_global_nameserver (__main__.TestServiceDHCPv6Server.test_global_nameserver) ... ok
test_prefix_delegation (__main__.TestServiceDHCPv6Server.test_prefix_delegation) ... ok
test_single_pool (__main__.TestServiceDHCPv6Server.test_single_pool) ... 
Either MAC address or Client identifier (DUID) is required for static
mapping "client1" within shared-network "SMOKE-1, 2001:db8:f00::/64"!

ok

----------------------------------------------------------------------
Ran 3 tests in 85.947s

OK

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro, sever-sever and c-po and removed request for a team December 18, 2023 07:50
interface-definitions/dhcpv6-server.xml.in Outdated Show resolved Hide resolved
@@ -316,6 +316,19 @@
</constraint>
</properties>
</leafNode>
<leafNode name="identifier">
Copy link
Member

Choose a reason for hiding this comment

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

We already have this:

    <leafNode name="duid">
      <properties>
        <help>DHCP unique identifier (DUID) to be sent by dhcpv6 client</help>
        <valueHelp>
          <format>duid</format>
          <description>DHCP unique identifier (DUID)</description>
        </valueHelp>
        <constraint>
          <validator name="ipv6-duid"/>
        </constraint>
      </properties>
    </leafNode>

in interface/dhcpv6-options.xml.i - maybe rename this node here and make it a common XML include block?

Copy link
Contributor Author

@indrajitr indrajitr Dec 18, 2023

Choose a reason for hiding this comment

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

I have a follow-up PR lined up for refactoring the XML blocks that include this one.
Couple of reasons I held back is because:

  1. the regex pattern for ipv6-duid validator is slightly different. It won't match aa:bb:c:d, but current identifier node honors that. I need to have migration take care of that.
  2. ipv6-duid is a full Python script. I think we can convert that into a simple shell script having ${vyos_libexec_dir}/validate-value --regex ....

I plan to tackle them together (including migration).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm adding migration from identifier to duid in this PR instead of a followup PR. Converting this to draft till migration scripts are ready.

@indrajitr indrajitr force-pushed the kea-reservation-fix branch 2 times, most recently from 71ca7bf to 012fe97 Compare December 20, 2023 20:29
@indrajitr indrajitr marked this pull request as draft December 21, 2023 03:30
@indrajitr
Copy link
Contributor Author

This needs to be applied after #2664.

@indrajitr indrajitr marked this pull request as ready for review December 21, 2023 06:44
@vyosbot vyosbot requested a review from a team December 21, 2023 06:44
@indrajitr indrajitr marked this pull request as draft December 21, 2023 08:21
Reinstate support for hostname in DHCP reservation. Having `hostname` in
allows for server-side assignment of hostname. This is useful for static
lookup of hostname.

Ensure that hostname is a valid FQDN (doesn't have underscore, etc.)

Additionally, support using either of DUID or MAC address for
reservation. While MAC address is typically used for IPv4, and DUID is
typically used for IPv6, either of them can be used in IPv4 and IPv6
reservations in Kea.
Also, have the smoketest start with a clean CLI instance.
@indrajitr indrajitr marked this pull request as ready for review December 22, 2023 05:50
@c-po c-po merged commit b0fc250 into vyos:current Dec 28, 2023
9 checks passed
@indrajitr indrajitr deleted the kea-reservation-fix branch December 28, 2023 13:45
sarthurdev added a commit to sarthurdev/vyos-documentation that referenced this pull request Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants