From 91b640c7ca69b17c7e1da1827be11b11eefc8ff2 Mon Sep 17 00:00:00 2001 From: Dennis Marttinen Date: Mon, 4 Jul 2022 16:51:49 +0300 Subject: [PATCH] feat: dhcpv4: send current hostname, fix spec compliance of renewals This adds support for automatically registering node hostnames in DNS by sending the current hostname to DHCP via option 12. If the current hostname is updated, issue a new DISCOVER to propagate the update to DHCP (updating the hostname on lease renewals is not universally supported by DHCP servers). This addition maintains the previous functionality where the node can also request its hostname from the DHCP server. The received hostname will be processed and prioritized as usual by the `network.HostnameSpecController`. This change set also contains fixes to make DHCP renewals compliant with RFC 2131, specifically avoiding sending the server identifier and requested IP address when issuing renewals using a previous offer. This also uncovered an issue in the upstream `insomniacslk/dhcp` library, for which a fix is now pending at https://github.com/insomniacslk/dhcp/pull/469. As upstream contributions seem to have stalled, I have temporarily added a replacement for the library to my own fork that carries the fix. Sending hostname updates have been tested against `dnsmasq` and the built-in DHCP + DNS services in Windows Server. Hostname retrieval from DHCP and edge cases with overridden hostnames from different configuration layers have been extensively tested against `dnsmasq`. Signed-off-by: Dennis Marttinen --- .golangci.yml | 1 + go.mod | 4 + go.sum | 4 +- go.work.sum | 12 +- .../pkg/controllers/network/operator/dhcp4.go | 332 ++++++++++++++---- .../network/operator/utils/utils.go | 22 ++ .../pkg/controllers/network/operator_spec.go | 2 +- pkg/machinery/resources/network/condition.go | 2 +- .../resources/network/hostname_spec.go | 2 +- 9 files changed, 305 insertions(+), 76 deletions(-) create mode 100644 internal/app/machined/pkg/controllers/network/operator/utils/utils.go diff --git a/.golangci.yml b/.golangci.yml index 28e4237aff4..934407540e8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -132,6 +132,7 @@ linters-settings: replace-allow-list: - gopkg.in/yaml.v3 - inet.af/tcpproxy + - github.com/insomniacslk/dhcp - github.com/vmware-tanzu/sonobuoy retract-allow-no-explanation: false exclude-forbidden: true diff --git a/go.mod b/go.mod index e0be72d35dc..f9ae13ea103 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,10 @@ module github.com/talos-systems/talos go 1.19 replace ( + // Forked insomniacslk/dhcp with DHCP spec compliancy fixes + // Upstream PR: https://github.com/insomniacslk/dhcp/pull/469 + github.com/insomniacslk/dhcp => github.com/twelho/dhcp v0.0.0-20220911105712-2eae1fc34ef5 + // Use nested module. github.com/talos-systems/talos/pkg/machinery => ./pkg/machinery diff --git a/go.sum b/go.sum index 3456496be4c..2dfc268ced6 100644 --- a/go.sum +++ b/go.sum @@ -702,8 +702,6 @@ github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU= github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= -github.com/insomniacslk/dhcp v0.0.0-20220822114210-de18a9d48e84 h1:MJTy6H+EpXLeAn0P5WAWeLk6dJA3V0ik6S3VJfUyQuI= -github.com/insomniacslk/dhcp v0.0.0-20220822114210-de18a9d48e84/go.mod h1:h+MxyHxRg9NH3terB1nfRIUaQEcI0XOVkdR9LNBlp8E= github.com/j-keck/arping v0.0.0-20160618110441-2cf9dc699c56/go.mod h1:ymszkNOg6tORTn+6F6j+Jc8TOr5osrynvN6ivFWZ2GA= github.com/jmespath/go-jmespath v0.0.0-20160202185014-0b12d6b521d8/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/jmespath/go-jmespath v0.0.0-20160803190731-bd40a432e4c7/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= @@ -1159,6 +1157,8 @@ github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/tv42/httpunix v0.0.0-20191220191345-2ba4b9c3382c/go.mod h1:hzIxponao9Kjc7aWznkXaL4U4TWaDSs8zcsY4Ka08nM= +github.com/twelho/dhcp v0.0.0-20220911105712-2eae1fc34ef5 h1:gAb8gdXrmWjQz/y6anQukHmbYtNgl9bZ3xRudOmVQkU= +github.com/twelho/dhcp v0.0.0-20220911105712-2eae1fc34ef5/go.mod h1:h+MxyHxRg9NH3terB1nfRIUaQEcI0XOVkdR9LNBlp8E= github.com/u-root/u-root v0.9.0 h1:1dpUzrE0FyKrNEjxpKFOkyveuV1f3T0Ko5CQg4gTkCg= github.com/u-root/u-root v0.9.0/go.mod h1:ewc9w6JF1ayZCVC9Y5wsrUiCBw3nMmPC3QItvrEwmew= github.com/u-root/uio v0.0.0-20210528114334-82958018845c/go.mod h1:LpEX5FO/cB+WF4TYGY1V5qktpaZLkKkSegbr0V4eYXA= diff --git a/go.work.sum b/go.work.sum index f927404c1d0..79dbaa421b7 100644 --- a/go.work.sum +++ b/go.work.sum @@ -126,8 +126,6 @@ github.com/google/go-containerregistry v0.5.1 h1:/+mFTs4AlwsJ/mJe8NDtKb7BxLtbZFp github.com/google/go-tpm v0.3.3 h1:P/ZFNBZYXRxc+z7i5uyd8VP7MaDteuLZInzrH2idRGo= github.com/google/goexpect v0.0.0-20191001010744-5b6988669ffa h1:PMkmJA8ju9DjqAJjIzrBdrmhuuPsoNnNLYgKQBopWL0= github.com/google/goterm v0.0.0-20200907032337-555d40f16ae2 h1:CVuJwN34x4xM2aT4sIKhmeib40NeBPhRihNjQmpJsA4= -github.com/google/nftables v0.0.0-20220904180503-6cd15ed86387 h1:f7O0IANfB30CjnazLz27UpC4PV+hvx7+4YWxBBErNX0= -github.com/google/nftables v0.0.0-20220904180503-6cd15ed86387/go.mod h1:b97ulCCFipUC+kSin+zygkvUVpx0vyIAwxXFdY3PlNc= github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 h1:K6RDEckDVWvDI9JAJYCmNdQXq6neHJOYx3V6jnqNEec= github.com/google/renameio v0.1.0 h1:GOZbcHa3HfsPKPlmyPyN2KEohoMXOhdMbHrvbpl2QaA= github.com/googleapis/gnostic v0.4.1 h1:DLJCy1n/vrD4HPjOvYcT8aYQXpPIzoRZONaYwyycI+I= @@ -153,6 +151,7 @@ github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI= github.com/iancoleman/strcase v0.2.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho= github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639 h1:mV02weKRL81bEnm8A0HT1/CAelMQDBuQIfLw8n+d6xI= github.com/imdario/mergo v0.3.6/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA= +github.com/insomniacslk/dhcp v0.0.0-20220822114210-de18a9d48e84/go.mod h1:h+MxyHxRg9NH3terB1nfRIUaQEcI0XOVkdR9LNBlp8E= github.com/intel-go/cpuid v0.0.0-20200819041909-2aa72927c3e2 h1:h+RKaNPjka7LRJGoeub/IQBdXSoEaJjfADkBq02hvjw= github.com/intel/goresctrl v0.2.0 h1:JyZjdMQu9Kl/wLXe9xA6s1X+tF6BWsQPFGJMEeCfWzE= github.com/j-keck/arping v0.0.0-20160618110441-2cf9dc699c56 h1:742eGXur0715JMq73aD95/FU0XpVKXqNuTnEfXsLOYQ= @@ -196,8 +195,6 @@ github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N github.com/opencontainers/runtime-tools v0.0.0-20181011054405-1d69bd0f9c39 h1:H7DMc6FAjgwZZi8BRqjrAAHWoqEr5e5L6pS4V0ezet4= github.com/opentracing/opentracing-go v1.1.0 h1:pWlfV3Bxv7k65HYwkikxat0+s3pV4bsqf19k25Ur8rU= github.com/orangecms/go-framebuffer v0.0.0-20200613202404-a0700d90c330 h1:zJBTzBuTR7EdFzmCGx0xp0pbOOb82sAh0+YUK4JTDEI= -github.com/packethost/packngo v0.26.0 h1:jbPHPEd+KpoM2OBQ3EgbgEYiAUK7vpZ9XhqOMEbnk78= -github.com/packethost/packngo v0.26.0/go.mod h1:/UHguFdPs6Lf6FOkkSEPnRY5tgS0fsVM+Zv/bvBrmt0= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c h1:Lgl0gzECD8GnQ5QCWA8o6BtfL6mDH5rQgM4/fX3avOs= github.com/pborman/getopt/v2 v2.1.0 h1:eNfR+r+dWLdWmV8g5OlpyrTYHkhVNxHBdN2cCrJmOEA= github.com/pierrec/lz4/v4 v4.1.14 h1:+fL8AQEZtz/ijeNnpduH0bROTu0O3NZAlPjQxGn8LwE= @@ -210,8 +207,6 @@ github.com/prometheus/tsdb v0.7.1 h1:YZcsG11NqnK4czYLrWd9mpEuAJIHVQLwdrleYfszMAA github.com/rasky/go-xdr v0.0.0-20170217172119-4930550ba2e2 h1:lbe6PJ3nOQAUvpx9P3GtsQ/jyNBOHLV+cj2++uZrpa4= github.com/rck/unit v0.0.3 h1:q3/Ui9gcrFKpEneZXw2gNmNEbzv5jLrZnH6qhX1ypZ0= github.com/rekby/gpt v0.0.0-20200219180433-a930afbc6edc h1:goZGTwEEn8mWLcY012VouWZWkJ8GrXm9tS3VORMxT90= -github.com/rivo/tview v0.0.0-20220903125348-532bb46474ec h1:hmV2IC9ucKTG5BLUHQC1WI5EhC0BRpU5wuiDs/3yxWA= -github.com/rivo/tview v0.0.0-20220903125348-532bb46474ec/go.mod h1:qNg8EdRsWg0Bxcj7npd4dSgxVbYB7kL7Mhk1YSOaVtw= github.com/rogpeppe/fastuuid v1.2.0 h1:Ppwyp6VYCF1nvBTXL3trRso7mXMlRrw9ooo375wvi2s= github.com/sagikazarmark/crypt v0.3.0 h1:TV5DVog+pihN4Rr0rN1IClv4ePpkzdg9sPrw7WDofZ8= github.com/sclevine/spec v1.2.0 h1:1Jwdf9jSfDl9NVmt8ndHqbTZ7XCCPbh1jI3hkDBHVYA= @@ -234,6 +229,7 @@ github.com/talos-systems/siderolink v0.1.1/go.mod h1:1PLRyKRx+MAkz1vWJXIP19p5wCh github.com/tchap/go-patricia v2.2.6+incompatible h1:JvoDL7JSoIP2HDE8AbDH3zC8QBPxmzYe32HHy5yQ+Ck= github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802 h1:uruHq4dN7GR16kFc5fp3d1RIYzJW5onx8Ybykw2YQFA= github.com/tv42/httpunix v0.0.0-20191220191345-2ba4b9c3382c h1:u6SKchux2yDvFQnDHS3lPnIRmfVJ5Sxy3ao2SIdysLQ= +github.com/twelho/dhcp v0.0.0-20220911105712-2eae1fc34ef5/go.mod h1:h+MxyHxRg9NH3terB1nfRIUaQEcI0XOVkdR9LNBlp8E= github.com/u-root/gobusybox/src v0.0.0-20220728145311-85dc1fd1bc75 h1:pCQLzZyKqQnYn/lTpROHlQJO8LrBQqrHE0gar1TnGOA= github.com/u-root/iscsinl v0.1.1-0.20210528121423-84c32645822a h1:A0sK7WEodak7eVd21MOEatnh2pfAAwZaEPSIEEsjctQ= github.com/ugorji/go v1.1.4 h1:j4s+tAvLfL3bZyefP2SEWmhBzmuIlH/eqNuPdFPgngw= @@ -287,8 +283,6 @@ golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 h1:0A+M6Uqn+Eje4kHMK80dtF3JC golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220829200755-d48e67d00261 h1:v6hYoSR9T5oet+pMXwUWkbiVqx/63mlHjefrHmxwfeY= -golang.org/x/sys v0.0.0-20220829200755-d48e67d00261/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/time v0.0.0-20220210224613-90d013bbcef8/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E= golang.zx2c4.com/go118/netip v0.0.0-20211106132939-9d41d90554dd h1:gUHae7sCd+tFJLcCximWeBFD2b6Jg3O7UaNaPvjIJHc= @@ -335,8 +329,6 @@ k8s.io/cri-api v0.24.2 h1:8i65YM7mC/VkeH0EFavTI0oXnxRZaQ4gPgOLJrj8LJs= k8s.io/cri-api v0.24.2/go.mod h1:t3tImFtGeStN+ES69bQUX9sFg67ek38BM9YIJhMmuig= k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c h1:GohjlNKauSai7gN4wsJkeZ3WAJx4Sh+oT/b5IYn5suA= k8s.io/gengo v0.0.0-20211129171323-c02415ce4185 h1:TT1WdmqqXareKxZ/oNXEUSwKlLiHzPMyB0t8BaFeBYI= -k8s.io/klog/v2 v2.80.0 h1:lyJt0TWMPaGoODa8B8bUuxgHS3W/m/bNr2cca3brA/g= -k8s.io/klog/v2 v2.80.0/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= k8s.io/kubectl v0.24.2 h1:+RfQVhth8akUmIc2Ge8krMl/pt66V7210ka3RE/p0J4= k8s.io/kubectl v0.24.2/go.mod h1:+HIFJc0bA6Tzu5O/YcuUt45APAxnNL8LeMuXwoiGsPg= k8s.io/kubelet v0.24.2 h1:VAvULig8RiylCtyxudgHV7nhKsLnNIrdVBCRD4bXQ3Y= diff --git a/internal/app/machined/pkg/controllers/network/operator/dhcp4.go b/internal/app/machined/pkg/controllers/network/operator/dhcp4.go index 6731163c922..af932c678d0 100644 --- a/internal/app/machined/pkg/controllers/network/operator/dhcp4.go +++ b/internal/app/machined/pkg/controllers/network/operator/dhcp4.go @@ -14,11 +14,14 @@ import ( "sync" "time" + "github.com/cosi-project/runtime/pkg/resource" + "github.com/cosi-project/runtime/pkg/state" "github.com/insomniacslk/dhcp/dhcpv4" "github.com/insomniacslk/dhcp/dhcpv4/nclient4" "go.uber.org/zap" "go4.org/netipx" + "github.com/talos-systems/talos/internal/app/machined/pkg/controllers/network/operator/utils" "github.com/talos-systems/talos/internal/app/machined/pkg/runtime" "github.com/talos-systems/talos/pkg/machinery/generic/slices" "github.com/talos-systems/talos/pkg/machinery/nethelpers" @@ -28,12 +31,13 @@ import ( // DHCP4 implements the DHCPv4 network operator. type DHCP4 struct { logger *zap.Logger + state state.State linkName string routeMetric uint32 requestMTU bool - offer *dhcpv4.DHCPv4 + lease *nclient4.Lease mu sync.Mutex addresses []network.AddressSpecSpec @@ -45,9 +49,10 @@ type DHCP4 struct { } // NewDHCP4 creates DHCPv4 operator. -func NewDHCP4(logger *zap.Logger, linkName string, routeMetric uint32, platform runtime.Platform) *DHCP4 { +func NewDHCP4(logger *zap.Logger, linkName string, routeMetric uint32, platform runtime.Platform, state state.State) *DHCP4 { return &DHCP4{ logger: logger, + state: state, linkName: linkName, routeMetric: routeMetric, // <3 azure @@ -62,25 +67,134 @@ func (d *DHCP4) Prefix() string { return fmt.Sprintf("dhcp4/%s", d.linkName) } +// extractHostname extracts a hostname from the given resource if it is a valid network.HostnameStatus. +func extractHostname(res resource.Resource) string { + if res, ok := res.(*network.HostnameStatus); ok { + return res.TypedSpec().Hostname + } + + return "" +} + +// setupHostnameWatch returns the initial hostname and a channel that outputs all events related to hostname changes. +func (d *DHCP4) setupHostnameWatch(ctx context.Context) (string, <-chan state.Event, error) { + hostnameWatchCh := make(chan state.Event) + if err := d.state.Watch(ctx, resource.NewMetadata( + network.NamespaceName, + network.HostnameStatusType, + network.HostnameID, + resource.VersionUndefined, + ), hostnameWatchCh); err != nil { + return "", nil, err + } + + return extractHostname((<-hostnameWatchCh).Resource), hostnameWatchCh, nil +} + +// knownHostname checks if the given hostname has been defined by this operator. +func (d *DHCP4) knownHostname(hostname string) bool { + for i := range d.hostname { + if d.hostname[i].Hostname == hostname { + return true + } + } + + return false +} + +// waitForNetworkReady waits for the network to be ready and the leased address to +// be assigned to the associated so that unicast operations can bind successfully. +func (d *DHCP4) waitForNetworkReady(ctx context.Context) error { + // if an IP address has been registered, wait for the address association to be ready + if len(d.addresses) > 0 { + _, err := d.state.WatchFor(ctx, + resource.NewMetadata( + network.NamespaceName, + network.AddressStatusType, + network.AddressID(d.linkName, d.addresses[0].Address), + resource.VersionUndefined, + ), + state.WithPhases(resource.PhaseRunning), + ) + if err != nil { + return fmt.Errorf("failed to wait for the address association to be ready: %w", err) + } + } + + // wait for the network (address and connectivity) to be ready + if err := network.NewReadyCondition(d.state, network.AddressReady, network.ConnectivityReady).Wait(ctx); err != nil { + return fmt.Errorf("failed to wait for the network address and connectivity to be ready: %w", err) + } + + return nil +} + // Run the operator loop. // -//nolint:gocyclo,dupl +//nolint:gocyclo,cyclop,dupl func (d *DHCP4) Run(ctx context.Context, notifyCh chan<- struct{}) { const minRenewDuration = 5 * time.Second // protect from renewing too often renewInterval := minRenewDuration + hostname, hostnameWatchCh, err := d.setupHostnameWatch(ctx) + if err != nil && !errors.Is(err, context.Canceled) { + d.logger.Warn("failed to watch for hostname changes", zap.Error(err)) + } + for { - leaseTime, err := d.renew(ctx) + // track if we need to acquire a new lease + newLease := d.lease == nil + + // perform a lease request or renewal + leaseTime, err := d.requestRenew(ctx, hostname) if err != nil && !errors.Is(err, context.Canceled) { - d.logger.Warn("renew failed", zap.Error(err), zap.String("link", d.linkName)) + d.logger.Warn("request/renew failed", zap.Error(err), zap.String("link", d.linkName)) } + if newLease { + // notify the underlying controller about the new lease + if err == nil { + select { + case notifyCh <- struct{}{}: + case <-ctx.Done(): + return + } + } + + // wait for networking to be established before transitioning to unicast operations + if err = d.waitForNetworkReady(ctx); err != nil && !errors.Is(err, context.Canceled) { + d.logger.Warn("failed to wait for networking to become ready", zap.Error(err)) + } + } + + // DHCP hostname parroting protection: if e.g. `dnsmasq` receives a request that both sends + // a hostname and requests one, it will "parrot" the sent hostname back if no other name has + // been defined for the requesting host. That causes update anomalies, since removing a + // hostname defined previously by e.g. the configuration layer causes a copy of that + // hostname to live on in a spec defined by this operator, even though it isn't sourced from + // DHCP. + // + // To avoid this issue, never send and request a hostname in the same operation. When + // negotiating a new lease, send the current hostname when acquiring the lease, and follow + // up with a dedicated INFORM request to ask the server for a DHCP-defined hostname. When + // renewing a lease, we're free to always request a hostname (to detect server-side + // changes), since any changes to the node hostname will cause a lease invalidation and + // re-start the negotiation process. More details below. if err == nil { - select { - case notifyCh <- struct{}{}: - case <-ctx.Done(): - return + // request the node hostname from the DHCP server + err = d.requestHostname(ctx) + if err != nil && !errors.Is(err, context.Canceled) { + d.logger.Warn("hostname request failed", zap.Error(err), zap.String("link", d.linkName)) + } + + // notify the underlying controller about the received hostname and/or renewed lease + if err == nil { + select { + case notifyCh <- struct{}{}: + case <-ctx.Done(): + return + } } } @@ -94,10 +208,39 @@ func (d *DHCP4) Run(ctx context.Context, notifyCh chan<- struct{}) { renewInterval = minRenewDuration } - select { - case <-ctx.Done(): - return - case <-time.After(renewInterval): + for { + select { + case <-ctx.Done(): + return + case <-time.After(renewInterval): + case event := <-hostnameWatchCh: + // If the hostname resource was deleted entirely, we must still inform the DHCP + // server that the node has no hostname anymore. `extractHostname` will return a + // blank hostname for a Tombstone resource generated by a deletion event. + hostname = extractHostname(event.Resource) + + // If, on first invocation, the DHCP server has given a new hostname for the node, + // and the network.HostnameSpecController decides to apply it as a preferred + // hostname, this operator would unnecessarily drop the lease and restart DHCP + // discovery. Thus, if the selected hostname has been sourced from this operator, + // we don't need to do anything. + if d.knownHostname(hostname) { + continue + } + + // While updating the hostname together with a RENEW request works for dnsmasq, it + // doesn't work with the Windows Server DHCP + DNS. A hostname update via an + // INIT-REBOOT request also gets ignored. Thus, the only reliable way to update the + // hostname seems to be to forget the old release and initiate a new DISCOVER flow + // with the new hostname. RFC 2131 doesn't define any better way to do this, and + // since according to the spec the DISCOVER cannot be targeted at the previous + // lessor, the node may switch DHCP servers on hostname change. This is not that + // big of a concern though, since a single network should not have multiple + // competing DHCP servers in the first place. + d.lease = nil + } + + break } } } @@ -150,8 +293,33 @@ func (d *DHCP4) TimeServerSpecs() []network.TimeServerSpecSpec { return d.timeservers } +func (d *DHCP4) parseHostnameFromAck(ack *dhcpv4.DHCPv4) { + d.mu.Lock() + defer d.mu.Unlock() + + if ack.HostName() != "" { + spec := network.HostnameSpecSpec{ + ConfigLayer: network.ConfigOperator, + } + + if err := spec.ParseFQDN(ack.HostName()); err == nil { + if ack.DomainName() != "" { + spec.Domainname = ack.DomainName() + } + + d.hostname = []network.HostnameSpecSpec{ + spec, + } + } else { + d.hostname = nil + } + } else { + d.hostname = nil + } +} + //nolint:gocyclo -func (d *DHCP4) parseAck(ack *dhcpv4.DHCPv4) { +func (d *DHCP4) parseNetworkConfigFromAck(ack *dhcpv4.DHCPv4) { d.mu.Lock() defer d.mu.Unlock() @@ -264,26 +432,6 @@ func (d *DHCP4) parseAck(ack *dhcpv4.DHCPv4) { d.resolvers = nil } - if ack.HostName() != "" { - spec := network.HostnameSpecSpec{ - ConfigLayer: network.ConfigOperator, - } - - if err = spec.ParseFQDN(ack.HostName()); err == nil { - if ack.DomainName() != "" { - spec.Domainname = ack.DomainName() - } - - d.hostname = []network.HostnameSpecSpec{ - spec, - } - } else { - d.hostname = nil - } - } else { - d.hostname = nil - } - if len(ack.NTPServers()) > 0 { convertIP := func(ip net.IP) string { result, _ := netipx.FromStdIP(ip) @@ -302,12 +450,83 @@ func (d *DHCP4) parseAck(ack *dhcpv4.DHCPv4) { } } -func (d *DHCP4) renew(ctx context.Context) (time.Duration, error) { +func (d *DHCP4) newClient() (*nclient4.Client, error) { + var clientOpts []nclient4.ClientOpt + + // we have an existing lease, target the server with unicast + if d.lease != nil { + serverAddr, err := utils.ToUDPAddr(d.lease.Offer.ServerIPAddr, nclient4.ServerPort) + if err != nil { + return nil, err + } + + clientAddr, err := utils.ToUDPAddr(d.lease.Offer.YourIPAddr, nclient4.ClientPort) + if err != nil { + return nil, err + } + + // RFC 2131, section 4.3.2: + // DHCPREQUEST generated during RENEWING state: + // ... This message will be unicast, so no relay + // agents will be involved in its transmission. + clientOpts = append(clientOpts, + nclient4.WithServerAddr(serverAddr), + // WithUnicast must be specified manually, WithServerAddr is not enough + nclient4.WithUnicast(clientAddr), + ) + } + + // create a new client, the caller is responsible for closing it + return nclient4.New(d.linkName, clientOpts...) +} + +// requestHostname uses an INFORM request to request a hostname from DHCP as requesting +// it during a DISCOVER is not reliable when simultaneously sending the local hostname. +func (d *DHCP4) requestHostname(ctx context.Context) error { + opts := []dhcpv4.OptionCode{ + dhcpv4.OptionHostName, + dhcpv4.OptionDomainName, + } + + client, err := d.newClient() + if err != nil { + return err + } + + //nolint:errcheck + defer client.Close() + + d.logger.Debug("DHCP INFORM", zap.String("link", d.linkName)) + + // create an INFORM request for querying a hostname from DHCP + request, err := dhcpv4.NewInform(client.InterfaceAddr(), d.lease.Offer.YourIPAddr, + dhcpv4.WithOptionCopied(d.lease.Offer, dhcpv4.OptionServerIdentifier), + dhcpv4.WithRequestedOptions(opts...), + ) + if err != nil { + return err + } + + // send the request and wait for a response + ack, err := client.TransactAckNak(ctx, request) + if err != nil { + return err + } + + d.logger.Debug("DHCP ACK", zap.String("link", d.linkName), zap.String("dhcp", collapseSummary(ack.Summary()))) + + // parse the hostname from the response + d.parseHostnameFromAck(ack) + + return nil +} + +func (d *DHCP4) requestRenew(ctx context.Context, hostname string) (time.Duration, error) { opts := []dhcpv4.OptionCode{ dhcpv4.OptionClasslessStaticRoute, dhcpv4.OptionDomainNameServer, + // TODO(twelho): This is unused until network.ResolverSpec supports search domains dhcpv4.OptionDNSDomainSearchList, - dhcpv4.OptionHostName, dhcpv4.OptionNTPServers, dhcpv4.OptionDomainName, } @@ -317,50 +536,41 @@ func (d *DHCP4) renew(ctx context.Context) (time.Duration, error) { } mods := []dhcpv4.Modifier{dhcpv4.WithRequestedOptions(opts...)} - clientOpts := []nclient4.ClientOpt{} - if d.offer != nil { - // do not use broadcast, but send the packet to DHCP server directly - addr, err := net.ResolveUDPAddr("udp", d.offer.ServerIPAddr.String()+":67") - if err != nil { - return 0, err - } - - // by default it's set to 0.0.0.0 which actually breaks lease renew - d.offer.ClientIPAddr = d.offer.YourIPAddr - - clientOpts = append(clientOpts, nclient4.WithServerAddr(addr)) + // if the node has a hostname, always send it to the DHCP + // server with option 12 during lease acquisition and renewal + if len(hostname) > 0 { + mods = append(mods, dhcpv4.WithOption(dhcpv4.OptHostName(hostname))) } - cli, err := nclient4.New(d.linkName, clientOpts...) + client, err := d.newClient() if err != nil { return 0, err } //nolint:errcheck - defer cli.Close() - - var lease *nclient4.Lease + defer client.Close() - if d.offer != nil { - lease, err = cli.RequestFromOffer(ctx, d.offer, mods...) + if d.lease != nil { + d.logger.Debug("DHCP RENEW", zap.String("link", d.linkName)) + err = client.Renew(ctx, d.lease, mods...) } else { - lease, err = cli.Request(ctx, mods...) + d.logger.Debug("DHCP REQUEST", zap.String("link", d.linkName)) + d.lease, err = client.Request(ctx, mods...) } if err != nil { - // clear offer if request fails to start with discover sequence next time - d.offer = nil + // explicitly clear the lease on failure to start with the discovery sequence next time + d.lease = nil return 0, err } - d.logger.Debug("DHCP ACK", zap.String("link", d.linkName), zap.String("dhcp", collapseSummary(lease.ACK.Summary()))) + d.logger.Debug("DHCP ACK", zap.String("link", d.linkName), zap.String("dhcp", collapseSummary(d.lease.ACK.Summary()))) - d.offer = lease.Offer - d.parseAck(lease.ACK) + d.parseNetworkConfigFromAck(d.lease.ACK) - return lease.ACK.IPAddressLeaseTime(time.Minute * 30), nil + return d.lease.ACK.IPAddressLeaseTime(time.Minute * 30), nil } func collapseSummary(summary string) string { diff --git a/internal/app/machined/pkg/controllers/network/operator/utils/utils.go b/internal/app/machined/pkg/controllers/network/operator/utils/utils.go new file mode 100644 index 00000000000..29e00f45ad2 --- /dev/null +++ b/internal/app/machined/pkg/controllers/network/operator/utils/utils.go @@ -0,0 +1,22 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +// Package utils provides common methods for operators. +package utils + +import ( + "fmt" + "net" + "net/netip" +) + +// ToUDPAddr combines the given net.IP and port to form a net.UDPAddr. +func ToUDPAddr(ip net.IP, port uint16) (*net.UDPAddr, error) { + addr, ok := netip.AddrFromSlice(ip) + if !ok { + return nil, fmt.Errorf("failed to parse %q as an IP address", ip) + } + + return net.UDPAddrFromAddrPort(netip.AddrPortFrom(addr, port)), nil +} diff --git a/internal/app/machined/pkg/controllers/network/operator_spec.go b/internal/app/machined/pkg/controllers/network/operator_spec.go index f7bbd151018..eda3bdf787c 100644 --- a/internal/app/machined/pkg/controllers/network/operator_spec.go +++ b/internal/app/machined/pkg/controllers/network/operator_spec.go @@ -421,7 +421,7 @@ func (ctrl *OperatorSpecController) newOperator(logger *zap.Logger, spec *networ case network.OperatorDHCP4: logger = logger.With(zap.String("operator", "dhcp4")) - return operator.NewDHCP4(logger, spec.LinkName, spec.DHCP4.RouteMetric, ctrl.V1alpha1Platform) + return operator.NewDHCP4(logger, spec.LinkName, spec.DHCP4.RouteMetric, ctrl.V1alpha1Platform, ctrl.State) case network.OperatorDHCP6: logger = logger.With(zap.String("operator", "dhcp6")) diff --git a/pkg/machinery/resources/network/condition.go b/pkg/machinery/resources/network/condition.go index 61903a6211e..f1b4dfb027b 100644 --- a/pkg/machinery/resources/network/condition.go +++ b/pkg/machinery/resources/network/condition.go @@ -19,7 +19,7 @@ type ReadyCondition struct { checks []StatusCheck } -// NewReadyCondition builds a coondition which waits for the network to be ready. +// NewReadyCondition builds a condition which waits for the network to be ready. func NewReadyCondition(state state.State, checks ...StatusCheck) *ReadyCondition { return &ReadyCondition{ state: state, diff --git a/pkg/machinery/resources/network/hostname_spec.go b/pkg/machinery/resources/network/hostname_spec.go index 28e015b1bd0..08ce6d236aa 100644 --- a/pkg/machinery/resources/network/hostname_spec.go +++ b/pkg/machinery/resources/network/hostname_spec.go @@ -25,7 +25,7 @@ type HostnameSpec = typed.Resource[HostnameSpecSpec, HostnameSpecRD] // HostnameID is the ID of the singleton instance. const HostnameID resource.ID = "hostname" -// HostnameSpecSpec describes node nostname. +// HostnameSpecSpec describes node hostname. // //gotagsrewrite:gen type HostnameSpecSpec struct {