Fixes #39429 - Add KEA DHCP provider by archanaserver · Pull Request #949 · theforeman/smart-proxy

archanaserver · 2026-06-15T09:20:31Z

Ref: theforeman/foremanctl#531 (comment)

KEA DHCP integration for Smart Proxy as we're transitioning away from ISC DHCP (which reached EOL in 2022) to KEA now.

Adds Smart Proxy DHCP provider for ISC KEA DHCP server using the KEA Control Agent API, providing a migration path for users currently using the deprecated dhcp_isc provider.

Note: Based on the smart_proxy_dhcp_kea_api gem by Sam McCarthy, to integrate into Smart Proxy core. https://gitlab.surrey.ac.uk/sm0049/smart-proxy-dhcp-kea-api/-/issues/3

Fixes: https://redhat.atlassian.net/browse/SAT-27739

ekohl · 2026-06-15T11:44:21Z

+      network, prefix = cidr.split('/')
+      prefix = prefix.to_i
+
+      mask = (0xffffffff << (32 - prefix)) & 0xffffffff
+
+      [mask].pack('N').unpack('C4').join('.')


IPAddr has this built in:

Suggested change

network, prefix = cidr.split('/')

prefix = prefix.to_i

mask = (0xffffffff << (32 - prefix)) & 0xffffffff

[mask].pack('N').unpack('C4').join('.')

cidr.netmask

Sadly, this is Ruby 3.1+ only :/

Sadly the Ruby's API docs didn't mention that :(

evgeni · 2026-06-16T11:56:08Z

Given this is an import of an previously external provider, I'd vote for "make CI green and merge it", with any further cleanup in a separate PR.

evgeni · 2026-06-17T10:36:37Z

+# NOTE: ISC DHCP reached end-of-life in October 2022. ISC recommends
+# migrating to KEA DHCP. Use `dhcp_kea` for new deployments.


Suggested change

# NOTE: ISC DHCP reached end-of-life in October 2022. ISC recommends

# migrating to KEA DHCP. Use `dhcp_kea` for new deployments.

stejskalleos · 2026-06-17T12:07:14Z

+require 'dhcp_kea/plugin_configuration'
+require 'dhcp_kea/dhcp_kea_plugin'


Is it a plugin? IMO it's not.

Suggested change

require 'dhcp_kea/plugin_configuration'

require 'dhcp_kea/dhcp_kea_plugin'

require 'dhcp_kea/configuration'

require 'dhcp_kea/dhcp_kea'

I kept dhcp_kea_plugin because we already have dhcp_kea as the entry point. Renaming the plugin file to dhcp_kea.rb would create confusion, so let's stay with this?

stejskalleos · 2026-06-17T12:08:58Z

+#:dhcp_kea_verify_ssl: true
+
+# Timeout for lease operations in seconds
+#:dhcp_kea_lease_timeout: 60


Discussion: 60 seconds is quite generous, and I don't know how it will work with all those queue steps and JS actions in the create host UI form.

IMO, we should lower this to <30 seconds.

Where is that used anyway?

currently it is only passed into the KEA provider and stored as @lease_timeout, but it is not actually used in the lease/reservation operation calls yet.

soeither wire this into the KEA API client timeout handling, or remove the config option for now if we don’t think we need it here. that's my thought on this

ekohl · 2026-06-17T13:19:57Z

+    load_classes ::Proxy::DHCP::Kea::PluginConfiguration
+    load_programmable_settings ::Proxy::DHCP::Kea::PluginConfiguration
+    load_dependency_injection_wirings ::Proxy::DHCP::Kea::PluginConfiguration


Note that all of these are optional. If they're empty, you can leave them out. You can also use a block. For example:

smart-proxy/modules/httpboot/httpboot_plugin.rb

Lines 7 to 11 in 3e249dd

load_programmable_settings do |settings|

settings[:http_port] = ::Proxy::Settings::Plugin.http_enabled?(settings[:enabled]) ? Proxy::SETTINGS.http_port : nil

settings[:https_port] = ::Proxy::Settings::Plugin.https_enabled?(settings[:enabled]) ? Proxy::SETTINGS.https_port : nil

settings

end

I'm fine either way, but I want to point out the option since it may be easier to understand to have fewer files.

The trade off is that it's harder to test. For load_classes that's not a concern since we don't test that anyway.

Yeah that make sense, Thanks for pointing this out!

archanaserver · 2026-06-22T15:34:03Z

One test changes and now it's failing completely, FYI this is in-progress right now.

archanaserver · 2026-06-23T07:15:11Z

@evgeni @stejskalleos can you help how it keep failing here workflow? i'm not sure what i'm missing

evgeni · 2026-06-24T05:53:17Z

+      'test.example.com',
+      '172.16.0.50',
+      'aa:bb:cc:dd:ee:ff',
+      Proxy::DHCP::Subnet.new('172.16.0.0/24', '255.255.255.0')


Suggested change

Proxy::DHCP::Subnet.new('172.16.0.0/24', '255.255.255.0')

Proxy::DHCP::Subnet.new('172.16.0.0', '255.255.255.0')

Subnet.new wants an IP and a netmask, separately

evgeni · 2026-06-24T06:02:33Z

+      end
+
+      if record.is_a?(::Proxy::DHCP::Reservation)
+        service.delete_host(subnet.network, record)


delete_host only expects one parameter: a record. you're passing two, and thus it fails.

evgeni · 2026-06-24T06:02:51Z

+      if record.is_a?(::Proxy::DHCP::Reservation)
+        service.delete_host(subnet.network, record)
+      elsif record.is_a?(::Proxy::DHCP::Lease)
+        service.delete_lease(subnet.network, record)


delete_lease also expects just one parameter, seems there is no test for this case.

also, for this to work, we should have issued lease4-del, not reservation-del as done by delete_reservation_by_ip, right?

evgeni · 2026-06-24T06:37:43Z

Which version (commit) of https://gitlab.surrey.ac.uk/sm0049/smart-proxy-dhcp-kea-api/ was this based on? I have a hard time comparing the code and the issues :(

ekohl · 2026-06-24T07:21:06Z

Which version (commit) of https://gitlab.surrey.ac.uk/sm0049/smart-proxy-dhcp-kea-api/ was this based on? I have a hard time comparing the code and the issues :(

Perhaps you can write 2 commits: 1 doing a verbatim import with proper ownership (git commit --author '...') and then another with the changes that you made to it. I strongly feel that proper attribution of the original owner is very important in open source. There may be some legal reasons (since there still is a license), but IMHO also ethical. Someone took the effort to write it and we should acknowledge that.

You can use Refs #39429 - ... on the first commit so make the Redmine checker happy.

Imported from smart-proxy-dhcp-kea-api. This is a verbatim import of the original KEA DHCP provider code.

Adapt the imported smart_proxy_dhcp_kea_api code for Smart Proxy core integration. - Rename classes to match Smart Proxy conventions - Update test structure and fix test issues - Add configuration template - Integrate with Smart Proxy module system

pr-processor Bot added Not yet reviewed Waiting on contributor labels Jun 15, 2026

ekohl reviewed Jun 15, 2026

View reviewed changes

pr-processor Bot removed the Not yet reviewed label Jun 15, 2026

evgeni reviewed Jun 16, 2026

View reviewed changes

Comment thread bundler.d/dhcp_kea.rb Outdated

evgeni reviewed Jun 16, 2026

View reviewed changes

Comment thread config/settings.d/dhcp.yml.example Outdated

evgeni reviewed Jun 16, 2026

View reviewed changes

Comment thread config/settings.d/dhcp_kea.yml.example Outdated

archanaserver force-pushed the dhcp-kea branch from a402d7f to 73ede59 Compare June 16, 2026 20:32

pr-processor Bot added Needs re-review and removed Needs re-review labels Jun 16, 2026

archanaserver force-pushed the dhcp-kea branch from 73ede59 to da4046e Compare June 16, 2026 20:57

pr-processor Bot added the Needs re-review label Jun 16, 2026

archanaserver marked this pull request as ready for review June 16, 2026 20:57

archanaserver force-pushed the dhcp-kea branch 2 times, most recently from 1c98bff to 5e48025 Compare June 16, 2026 21:03

archanaserver changed the title ~~Add KEA DHCP provider~~ Fixes #39429 - Add KEA DHCP provider Jun 16, 2026

pr-processor Bot removed the Needs re-review label Jun 17, 2026

evgeni reviewed Jun 17, 2026

View reviewed changes

stejskalleos previously approved these changes Jun 17, 2026

View reviewed changes

ekohl reviewed Jun 17, 2026

View reviewed changes

archanaserver dismissed stejskalleos’s stale review via ba15277 June 22, 2026 11:01

archanaserver force-pushed the dhcp-kea branch from 5e48025 to ba15277 Compare June 22, 2026 11:01

pr-processor Bot added Needs re-review and removed Needs re-review labels Jun 22, 2026

archanaserver force-pushed the dhcp-kea branch from ab137f9 to 99699f4 Compare June 22, 2026 14:35

pr-processor Bot removed the Needs re-review label Jun 22, 2026

archanaserver force-pushed the dhcp-kea branch from 99699f4 to 31c4447 Compare June 22, 2026 14:47

pr-processor Bot added the Needs re-review label Jun 22, 2026

archanaserver force-pushed the dhcp-kea branch from 31c4447 to f319ebd Compare June 22, 2026 15:00

pr-processor Bot removed the Waiting on contributor label Jun 22, 2026

archanaserver force-pushed the dhcp-kea branch 3 times, most recently from 3555f47 to 5bd9443 Compare June 22, 2026 15:30

archanaserver marked this pull request as draft June 22, 2026 15:34

archanaserver force-pushed the dhcp-kea branch from 5bd9443 to 5e9377e Compare June 23, 2026 06:38

archanaserver marked this pull request as ready for review June 23, 2026 10:13

archanaserver requested review from evgeni and stejskalleos June 23, 2026 10:58

evgeni reviewed Jun 24, 2026

View reviewed changes

pr-processor Bot removed the Needs re-review label Jun 24, 2026

evgeni reviewed Jun 24, 2026

View reviewed changes

Refs #39429 - Import smart_proxy_dhcp_kea_api provider

67d60e7

Imported from smart-proxy-dhcp-kea-api. This is a verbatim import of the original KEA DHCP provider code.

archanaserver force-pushed the dhcp-kea branch from 5e9377e to e5128d2 Compare June 25, 2026 07:58

Fixes #39429 - Add Kea DHCP provider

cb5e99a

Adapt the imported smart_proxy_dhcp_kea_api code for Smart Proxy core integration. - Rename classes to match Smart Proxy conventions - Update test structure and fix test issues - Add configuration template - Integrate with Smart Proxy module system

archanaserver force-pushed the dhcp-kea branch from e5128d2 to cb5e99a Compare June 25, 2026 08:15

		# NOTE: ISC DHCP reached end-of-life in October 2022. ISC recommends
		# migrating to KEA DHCP. Use `dhcp_kea` for new deployments.

		require 'dhcp_kea/plugin_configuration'
		require 'dhcp_kea/dhcp_kea_plugin'

	load_programmable_settings do \|settings\|
	settings[:http_port] = ::Proxy::Settings::Plugin.http_enabled?(settings[:enabled]) ? Proxy::SETTINGS.http_port : nil
	settings[:https_port] = ::Proxy::Settings::Plugin.https_enabled?(settings[:enabled]) ? Proxy::SETTINGS.https_port : nil
	settings
	end

	Proxy::DHCP::Subnet.new('172.16.0.0/24', '255.255.255.0')
	Proxy::DHCP::Subnet.new('172.16.0.0', '255.255.255.0')

Uh oh!

Conversation

archanaserver commented Jun 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekohl Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evgeni commented Jun 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

archanaserver Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

archanaserver commented Jun 22, 2026

Uh oh!

archanaserver commented Jun 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

evgeni commented Jun 24, 2026

Uh oh!

ekohl commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ekohl Jun 17, 2026 •

edited

Loading

archanaserver Jun 22, 2026 •

edited

Loading