From c6192902275b73e69db8bca7797ae3afe98b755d Mon Sep 17 00:00:00 2001 From: Andreas Fritzler Date: Tue, 24 Sep 2024 14:39:41 +0200 Subject: [PATCH 1/4] Add `breaking` labeling to release drafter configuration --- .github/release-drafter.yml | 6 ++++++ .github/workflows/{golangci-lint.yml => lint.yml} | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) rename .github/workflows/{golangci-lint.yml => lint.yml} (77%) diff --git a/.github/release-drafter.yml b/.github/release-drafter.yml index 7e48f44..3dab5a5 100644 --- a/.github/release-drafter.yml +++ b/.github/release-drafter.yml @@ -1,6 +1,9 @@ name-template: 'v$RESOLVED_VERSION' tag-template: 'v$RESOLVED_VERSION' categories: + - title: '⚠️ Breaking' + labels: + - 'breaking' - title: '🚀 Features' labels: - 'feature' @@ -51,6 +54,9 @@ autolabeler: - label: 'enhancement' branch: - '/enh\/.+/' + - label: 'chore' + branch: + - '/chore\/.+/' template: | ## Changes $CHANGES \ No newline at end of file diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/lint.yml similarity index 77% rename from .github/workflows/golangci-lint.yml rename to .github/workflows/lint.yml index 760cc37..5635e0c 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/lint.yml @@ -1,4 +1,4 @@ -name: Lint Golang Codebase +name: Lint on: pull_request: @@ -14,6 +14,7 @@ jobs: - uses: actions/setup-go@v5 with: go-version-file: 'go.mod' - - uses: golangci/golangci-lint-action@v6 + - name: golangci-lint + uses: golangci/golangci-lint-action@v6 with: version: v1.61 From 7c72bf5205b3915f08afe48e97370526c627a693 Mon Sep 17 00:00:00 2001 From: Andreas Fritzler Date: Tue, 24 Sep 2024 14:42:51 +0200 Subject: [PATCH 2/4] Add golangci-lint configuration --- .golangci.yml | 41 +++++++++++++++++++++++++++++++++++++++++ .reuse/dep5 | 2 +- 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..a09b41d --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,41 @@ +run: + timeout: 10m + allow-parallel-runners: true + +issues: + # don't skip warning about doc comments + # don't exclude the default set of lint + exclude-use-default: false + # restore some of the defaults + # (fill in the rest as needed) + exclude-rules: + - path: "api/*" + linters: + - lll + - path: "internal/*" + linters: + - dupl + - lll +linters: + disable-all: true + enable: + - copyloopvar + - dupl + - errcheck + - goconst + - gocyclo + - gofmt + - goimports + - gosimple + - govet + - ineffassign + - ginkgolinter + - lll + - misspell + - nakedret + - prealloc + - staticcheck + - typecheck + - unconvert + - unparam + - unused diff --git a/.reuse/dep5 b/.reuse/dep5 index 25e9696..cfc181f 100644 --- a/.reuse/dep5 +++ b/.reuse/dep5 @@ -9,7 +9,7 @@ Source: https://github.com/ironcore-dev/FeDHCP Files: .github/* .gitignore - .golangci.yaml + .golangci.yml Dockerfile Makefile config/* From ce7568859486dc2571fe90976a9b7a7ee5242ece Mon Sep 17 00:00:00 2001 From: Andreas Fritzler Date: Wed, 25 Sep 2024 16:30:52 +0200 Subject: [PATCH 3/4] Fix linting issues --- internal/kubernetes/client.go | 2 +- main.go | 2 +- plugins/httpboot/plugin.go | 4 +- plugins/httpboot/plugin_test.go | 25 +++++++---- plugins/ipam/k8s.go | 22 ++++++---- plugins/ipam/plugin.go | 3 +- plugins/metal/plugin_test.go | 73 +++++++++++++++++---------------- plugins/onmetal/plugin_test.go | 3 +- plugins/oob/k8s.go | 30 ++++++++++---- plugins/oob/plugin.go | 3 +- plugins/pxeboot/plugin_test.go | 30 +++++++++++--- 11 files changed, 127 insertions(+), 70 deletions(-) diff --git a/internal/kubernetes/client.go b/internal/kubernetes/client.go index 72f14fa..54a40fe 100644 --- a/internal/kubernetes/client.go +++ b/internal/kubernetes/client.go @@ -5,12 +5,12 @@ package kubernetes import ( "fmt" - "k8s.io/client-go/rest" ipamv1alpha1 "github.com/ironcore-dev/ipam/api/ipam/v1alpha1" metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/config" ) diff --git a/main.go b/main.go index 01a1bd0..e376c68 100644 --- a/main.go +++ b/main.go @@ -6,7 +6,6 @@ package main import ( "flag" "fmt" - "k8s.io/apimachinery/pkg/util/sets" "os" "github.com/coredhcp/coredhcp/config" @@ -35,6 +34,7 @@ import ( "github.com/ironcore-dev/fedhcp/plugins/onmetal" "github.com/ironcore-dev/fedhcp/plugins/oob" "github.com/ironcore-dev/fedhcp/plugins/pxeboot" + "k8s.io/apimachinery/pkg/util/sets" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/log/zap" ) diff --git a/plugins/httpboot/plugin.go b/plugins/httpboot/plugin.go index ec3e762..bf621b1 100644 --- a/plugins/httpboot/plugin.go +++ b/plugins/httpboot/plugin.go @@ -206,7 +206,9 @@ func fetchUKIURL(url string, clientIPs []string) (string, error) { log.Errorf("HTTP request failed: %v", err) return "", err } - defer resp.Body.Close() + defer func() { + _ = resp.Body.Close() + }() body, err := io.ReadAll(resp.Body) if err != nil { diff --git a/plugins/httpboot/plugin_test.go b/plugins/httpboot/plugin_test.go index 0b15df8..1a5aed6 100644 --- a/plugins/httpboot/plugin_test.go +++ b/plugins/httpboot/plugin_test.go @@ -13,11 +13,9 @@ import ( "testing" "time" - "github.com/insomniacslk/dhcp/iana" - "github.com/insomniacslk/dhcp/dhcpv4" - "github.com/insomniacslk/dhcp/dhcpv6" + "github.com/insomniacslk/dhcp/iana" ) const ( @@ -285,7 +283,10 @@ func TestHTTPBootNotRelayedMsg6(t *testing.T) { func TestGenericHTTPBootRequested4(t *testing.T) { Init4(expectedGenericBootURL) - req, err := dhcpv4.NewDiscovery(net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, dhcpv4.WithRequestedOptions(dhcpv4.OptionClassIdentifier)) + req, err := dhcpv4.NewDiscovery(net.HardwareAddr{ + 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, + dhcpv4.WithRequestedOptions(dhcpv4.OptionClassIdentifier), + ) if err != nil { t.Fatal(err) } @@ -320,7 +321,10 @@ func TestGenericHTTPBootRequested4(t *testing.T) { func TestMalformedHTTPBootRequested4(t *testing.T) { Init4(expectedGenericBootURL) - req, err := dhcpv4.NewDiscovery(net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, dhcpv4.WithRequestedOptions(dhcpv4.OptionClassIdentifier)) + req, err := dhcpv4.NewDiscovery(net.HardwareAddr{ + 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, + dhcpv4.WithRequestedOptions(dhcpv4.OptionClassIdentifier), + ) if err != nil { t.Fatal(err) } @@ -385,7 +389,10 @@ func TestMalformedHTTPBootRequested4(t *testing.T) { func TestHTTPBootNotRequested4(t *testing.T) { Init4(expectedGenericBootURL) - req, err := dhcpv4.NewDiscovery(net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, dhcpv4.WithRequestedOptions(dhcpv4.OptionClassIdentifier)) + req, err := dhcpv4.NewDiscovery(net.HardwareAddr{ + 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, + dhcpv4.WithRequestedOptions(dhcpv4.OptionClassIdentifier), + ) if err != nil { t.Fatal(err) } @@ -506,7 +513,8 @@ func TestCustomHTTPBootRequestedKnownMAC(t *testing.T) { req.UpdateOption(&optVendorClass) // not known LinkAddr - relayedRequest, err := dhcpv6.EncapsulateRelay(req, dhcpv6.MessageTypeRelayForward, net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}, net.IPv6loopback) + relayedRequest, err := dhcpv6.EncapsulateRelay(req, dhcpv6.MessageTypeRelayForward, + net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}, net.IPv6loopback) if err != nil { t.Fatal(err) } @@ -574,7 +582,8 @@ func TestCustomHTTPBootRequestedUnknownClient(t *testing.T) { req.UpdateOption(&optVendorClass) // not known LinkAddr - relayedRequest, err := dhcpv6.EncapsulateRelay(req, dhcpv6.MessageTypeRelayForward, net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}, net.IPv6loopback) + relayedRequest, err := dhcpv6.EncapsulateRelay(req, dhcpv6.MessageTypeRelayForward, + net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}, net.IPv6loopback) if err != nil { t.Fatal(err) } diff --git a/plugins/ipam/k8s.go b/plugins/ipam/k8s.go index 7907cbc..51091c6 100644 --- a/plugins/ipam/k8s.go +++ b/plugins/ipam/k8s.go @@ -8,12 +8,12 @@ import ( "encoding/hex" "encoding/json" "fmt" - "github.com/ironcore-dev/fedhcp/internal/kubernetes" "net" "os" "reflect" "strings" + "github.com/ironcore-dev/fedhcp/internal/kubernetes" ipamv1alpha1 "github.com/ironcore-dev/ipam/api/ipam/v1alpha1" ipam "github.com/ironcore-dev/ipam/clientgo/ipam" "github.com/pkg/errors" @@ -135,7 +135,10 @@ func (k K8sClient) getMatchingSubnet(subnetName string, ipaddr net.IP) (*ipamv1a return subnet, nil } -func (k K8sClient) prepareCreateIpamIP(subnetName string, ipaddr net.IP, mac net.HardwareAddr) (*ipamv1alpha1.IP, error) { +func (k K8sClient) prepareCreateIpamIP( + subnetName string, + ipaddr net.IP, + mac net.HardwareAddr) (*ipamv1alpha1.IP, error) { ip, err := ipamv1alpha1.IPAddrFromString(ipaddr.String()) if err != nil { return nil, fmt.Errorf("failed to parse IP %s: %w", ipaddr, err) @@ -176,23 +179,28 @@ func (k K8sClient) prepareCreateIpamIP(subnetName string, ipaddr net.IP, mac net noop() } else { if !reflect.DeepEqual(ipamIP.Spec, existingIpamIP.Spec) { - log.Debugf("IP mismatch:\nold IP: %v,\nnew IP: %v", prettyFormat(existingIpamIP.Spec), prettyFormat(ipamIP.Spec)) + log.Debugf("IP mismatch:\nold IP: %v,\nnew IP: %v", prettyFormat(existingIpamIP.Spec), + prettyFormat(ipamIP.Spec)) log.Infof("Deleting old IP %s/%s", existingIpamIP.Namespace, existingIpamIP.Name) // delete old IP object err = k.Client.Delete(k.Ctx, existingIpamIP) if err != nil { - return nil, fmt.Errorf("failed to delete IP %s/%s: %w", existingIpamIP.Namespace, existingIpamIP.Name, err) + return nil, fmt.Errorf("failed to delete IP %s/%s: %w", existingIpamIP.Namespace, + existingIpamIP.Name, err) } err = k.waitForDeletion(existingIpamIP) if err != nil { - return nil, fmt.Errorf("failed to delete IP %s/%s: %w", existingIpamIP.Namespace, existingIpamIP.Name, err) + return nil, fmt.Errorf("failed to delete IP %s/%s: %w", existingIpamIP.Namespace, + existingIpamIP.Name, err) } k.EventRecorder.Eventf(existingIpamIP, corev1.EventTypeNormal, "Deleted", "Deleted old IPAM IP") - log.Infof("Old IP %s/%s deleted from subnet %s", existingIpamIP.Namespace, existingIpamIP.Name, existingIpamIP.Spec.Subnet.Name) + log.Infof("Old IP %s/%s deleted from subnet %s", existingIpamIP.Namespace, existingIpamIP.Name, + existingIpamIP.Spec.Subnet.Name) } else { - log.Infof("IP %s/%s already exists in subnet %s, nothing to do", existingIpamIP.Namespace, existingIpamIP.Name, existingIpamIP.Spec.Subnet.Name) + log.Infof("IP %s/%s already exists in subnet %s, nothing to do", existingIpamIP.Namespace, + existingIpamIP.Name, existingIpamIP.Spec.Subnet.Name) return nil, nil } } diff --git a/plugins/ipam/plugin.go b/plugins/ipam/plugin.go index a2bb277..3dcdd32 100644 --- a/plugins/ipam/plugin.go +++ b/plugins/ipam/plugin.go @@ -29,7 +29,8 @@ var ( func parseArgs(args ...string) (string, []string, error) { if len(args) < 2 { - return "", []string{""}, fmt.Errorf("at least two arguments must be passed to ipam plugin, a namespace and a comma-separated subnet names list, got %d", len(args)) + return "", []string{""}, fmt.Errorf("at least two arguments must be passed to ipam plugin, a namespace "+ + "and a comma-separated subnet names list, got %d", len(args)) } namespace := args[0] diff --git a/plugins/metal/plugin_test.go b/plugins/metal/plugin_test.go index 49fa01d..bbbe65d 100644 --- a/plugins/metal/plugin_test.go +++ b/plugins/metal/plugin_test.go @@ -89,32 +89,32 @@ var _ = Describe("Endpoint", func() { It("Setup6 should return error if less arguments are provided", func() { _, err := setup6() - Expect(err).NotTo(BeNil()) + Expect(err).To(HaveOccurred()) }) It("Setup6 should return error if more arguments are provided", func() { _, err := setup6("foo", "bar") - Expect(err).NotTo(BeNil()) + Expect(err).To(HaveOccurred()) }) It("Setup6 should return error if config file does not exist", func() { _, err := setup6("does-not-exist.yaml") - Expect(err).NotTo(BeNil()) + Expect(err).To(HaveOccurred()) }) It("Setup4 should return error if less arguments are provided", func() { _, err := setup4() - Expect(err).NotTo(BeNil()) + Expect(err).To(HaveOccurred()) }) It("Setup4 should return error if more arguments are provided", func() { _, err := setup4("foo", "bar") - Expect(err).NotTo(BeNil()) + Expect(err).To(HaveOccurred()) }) It("Setup4 should return error if config file does not exist", func() { _, err := setup4("does-not-exist.yaml") - Expect(err).NotTo(BeNil()) + Expect(err).To(HaveOccurred()) }) It("Should return empty inventory list if the config file is malformed", func() { @@ -196,26 +196,28 @@ var _ = Describe("Endpoint", func() { Eventually(ip).Should(BeNil()) }) - It("Should not create an endpoint for IPv6 DHCP request from a known machine without IP address", func(ctx SpecContext) { - mac, _ := net.ParseMAC(machineWithoutIPAddressMACAddress) - ip := net.ParseIP(linkLocalIPV6Prefix) - linkLocalIPV6Addr, _ := eui64.ParseMAC(ip, mac) + It("Should not create an endpoint for IPv6 DHCP request from a known machine without IP address", + func(ctx SpecContext) { + mac, _ := net.ParseMAC(machineWithoutIPAddressMACAddress) + ip := net.ParseIP(linkLocalIPV6Prefix) + linkLocalIPV6Addr, _ := eui64.ParseMAC(ip, mac) - req, _ := dhcpv6.NewMessage() - req.MessageType = dhcpv6.MessageTypeRequest - relayedRequest, _ := dhcpv6.EncapsulateRelay(req, dhcpv6.MessageTypeRelayForward, net.IPv6loopback, linkLocalIPV6Addr) + req, _ := dhcpv6.NewMessage() + req.MessageType = dhcpv6.MessageTypeRequest + relayedRequest, _ := dhcpv6.EncapsulateRelay(req, dhcpv6.MessageTypeRelayForward, + net.IPv6loopback, linkLocalIPV6Addr) - stub, _ := dhcpv6.NewMessage() - stub.MessageType = dhcpv6.MessageTypeReply - _, _ = handler6(relayedRequest, stub) + stub, _ := dhcpv6.NewMessage() + stub.MessageType = dhcpv6.MessageTypeReply + _, _ = handler6(relayedRequest, stub) - endpoint := &metalv1alpha1.Endpoint{ - ObjectMeta: metav1.ObjectMeta{ - Name: machineWithoutIPAddressName, - }, - } - Eventually(Get(endpoint)).Should(Satisfy(apierrors.IsNotFound)) - }) + endpoint := &metalv1alpha1.Endpoint{ + ObjectMeta: metav1.ObjectMeta{ + Name: machineWithoutIPAddressName, + }, + } + Eventually(Get(endpoint)).Should(Satisfy(apierrors.IsNotFound)) + }) It("Should not create an endpoint for IPv6 DHCP request from a unknown machine", func(ctx SpecContext) { mac, _ := net.ParseMAC(unknownMachineMACAddress) @@ -271,21 +273,22 @@ var _ = Describe("Endpoint", func() { DeferCleanup(k8sClient.Delete, endpoint) }) - It("Should not create an endpoint for IPv4 DHCP request from a known machine without IP address", func(ctx SpecContext) { - mac, _ := net.ParseMAC(machineWithoutIPAddressMACAddress) + It("Should not create an endpoint for IPv4 DHCP request from a known machine without IP address", + func(ctx SpecContext) { + mac, _ := net.ParseMAC(machineWithoutIPAddressMACAddress) - req, _ := dhcpv4.NewDiscovery(mac) - stub, _ := dhcpv4.NewReplyFromRequest(req) + req, _ := dhcpv4.NewDiscovery(mac) + stub, _ := dhcpv4.NewReplyFromRequest(req) - _, _ = handler4(req, stub) + _, _ = handler4(req, stub) - endpoint := &metalv1alpha1.Endpoint{ - ObjectMeta: metav1.ObjectMeta{ - Name: machineWithoutIPAddressName, - }, - } - Eventually(Get(endpoint)).Should(Satisfy(apierrors.IsNotFound)) - }) + endpoint := &metalv1alpha1.Endpoint{ + ObjectMeta: metav1.ObjectMeta{ + Name: machineWithoutIPAddressName, + }, + } + Eventually(Get(endpoint)).Should(Satisfy(apierrors.IsNotFound)) + }) It("Should not create an endpoint for IPv6 DHCP request from a unknown machine", func(ctx SpecContext) { mac, _ := net.ParseMAC(unknownMachineMACAddress) diff --git a/plugins/onmetal/plugin_test.go b/plugins/onmetal/plugin_test.go index c2807d4..40f6fe6 100644 --- a/plugins/onmetal/plugin_test.go +++ b/plugins/onmetal/plugin_test.go @@ -175,7 +175,8 @@ func TestPrefixDelegationRequested6(t *testing.T) { Options: dhcpv6.PDOptions{}, }) - relayedRequest, err := dhcpv6.EncapsulateRelay(req, dhcpv6.MessageTypeRelayForward, net.ParseIP("2001:db8:1111:2222:3333:4444:5555:6666"), net.IPv6loopback) + relayedRequest, err := dhcpv6.EncapsulateRelay(req, dhcpv6.MessageTypeRelayForward, + net.ParseIP("2001:db8:1111:2222:3333:4444:5555:6666"), net.IPv6loopback) if err != nil { t.Fatal(err) } diff --git a/plugins/oob/k8s.go b/plugins/oob/k8s.go index 5cb5daa..6401c75 100644 --- a/plugins/oob/k8s.go +++ b/plugins/oob/k8s.go @@ -84,7 +84,11 @@ func NewK8sClient(namespace string, oobLabel string) (*K8sClient, error) { return &k8sClient, nil } -func (k K8sClient) getIp(ipaddr net.IP, mac net.HardwareAddr, exactIP bool, subnetType ipamv1alpha1.SubnetAddressType) (net.IP, error) { +func (k K8sClient) getIp( + ipaddr net.IP, + mac net.HardwareAddr, + exactIP bool, + subnetType ipamv1alpha1.SubnetAddressType) (net.IP, error) { var ipamIP *ipamv1alpha1.IP macKey := strings.ReplaceAll(mac.String(), ":", "") @@ -115,7 +119,8 @@ func (k K8sClient) getIp(ipaddr net.IP, mac net.HardwareAddr, exactIP bool, subn return nil, err } } else { - log.Infof("Reserved IP %s (%s/%s) already exists in subnet %s", ipamIP.Status.Reserved.String(), ipamIP.Namespace, ipamIP.Name, ipamIP.Spec.Subnet.Name) + log.Infof("Reserved IP %s (%s/%s) already exists in subnet %s", ipamIP.Status.Reserved.String(), + ipamIP.Namespace, ipamIP.Name, ipamIP.Spec.Subnet.Name) k.applySubnetLabel(ipamIP) } // break at first subnet match, there can be only one @@ -156,11 +161,14 @@ func (k K8sClient) prepareCreateIpamIP(subnetName string, macKey string) (*ipamv for _, existingIpamIP := range ipList.Items { if existingIpamIP.Spec.Subnet.Name != subnetName { // IP with that MAC is assigned to a different subnet (v4 vs v6?) - log.Debugf("IPAM IP with MAC %v and wrong subnet %s/%s found, ignoring", macKey, existingIpamIP.Namespace, existingIpamIP.Spec.Subnet.Name) + log.Debugf("IPAM IP with MAC %v and wrong subnet %s/%s found, ignoring", macKey, + existingIpamIP.Namespace, existingIpamIP.Spec.Subnet.Name) continue } else if existingIpamIP.Status.State == ipamv1alpha1.CFailedIPState { - log.Infof("Failed IP %s/%s in subnet %s found, deleting", existingIpamIP.Namespace, existingIpamIP.Name, existingIpamIP.Spec.Subnet.Name) - log.Debugf("Deleting old IP %s/%s:\n%v", existingIpamIP.Namespace, existingIpamIP.Name, prettyFormat(existingIpamIP.Status)) + log.Infof("Failed IP %s/%s in subnet %s found, deleting", existingIpamIP.Namespace, + existingIpamIP.Name, existingIpamIP.Spec.Subnet.Name) + log.Debugf("Deleting old IP %s/%s:\n%v", existingIpamIP.Namespace, existingIpamIP.Name, + prettyFormat(existingIpamIP.Status)) err = k.Client.Delete(k.Ctx, &existingIpamIP) if err != nil { return nil, fmt.Errorf("failed to delete IP %s/%s: %w", existingIpamIP.Namespace, existingIpamIP.Name, err) @@ -172,7 +180,8 @@ func (k K8sClient) prepareCreateIpamIP(subnetName string, macKey string) (*ipamv } k.EventRecorder.Eventf(&existingIpamIP, corev1.EventTypeNormal, "Deleted", "Deleted old IPAM IP") - log.Debugf("Old IP %s/%s deleted from subnet %s", existingIpamIP.Namespace, existingIpamIP.Name, existingIpamIP.Spec.Subnet.Name) + log.Debugf("Old IP %s/%s deleted from subnet %s", existingIpamIP.Namespace, + existingIpamIP.Name, existingIpamIP.Spec.Subnet.Name) } else { // IP already exists return &existingIpamIP, nil @@ -183,7 +192,11 @@ func (k K8sClient) prepareCreateIpamIP(subnetName string, macKey string) (*ipamv return nil, nil } -func (k K8sClient) doCreateIpamIP(subnetName string, macKey string, ipaddr net.IP, exactIP bool) (*ipamv1alpha1.IP, error) { +func (k K8sClient) doCreateIpamIP( + subnetName string, + macKey string, + ipaddr net.IP, + exactIP bool) (*ipamv1alpha1.IP, error) { oobLabelKey := strings.Split(k.OobLabel, "=")[0] oobLabelValue := strings.Split(k.OobLabel, "=")[1] var ipamIP *ipamv1alpha1.IP @@ -236,7 +249,8 @@ func (k K8sClient) doCreateIpamIP(subnetName string, macKey string, ipaddr net.I if err != nil { return nil, fmt.Errorf("failed to create IP %s/%s: %w", ipamIP.Namespace, ipamIP.Name, err) } else { - log.Infof("New IP %s (%s/%s) created in subnet %s", ipamIP.Status.Reserved.String(), ipamIP.Namespace, ipamIP.Name, ipamIP.Spec.Subnet.Name) + log.Infof("New IP %s (%s/%s) created in subnet %s", ipamIP.Status.Reserved.String(), + ipamIP.Namespace, ipamIP.Name, ipamIP.Spec.Subnet.Name) k.EventRecorder.Eventf(ipamIP, corev1.EventTypeNormal, "Created", "Created IPAM IP") // update IP attributes diff --git a/plugins/oob/plugin.go b/plugins/oob/plugin.go index d9457de..dae2b5f 100644 --- a/plugins/oob/plugin.go +++ b/plugins/oob/plugin.go @@ -37,7 +37,8 @@ const ( func parseArgs(args ...string) (string, string, error) { if len(args) < 2 { - return "", "", fmt.Errorf("at least two arguments must be passed to ipam plugin, a namespace and a OOB subnet label, got %d", len(args)) + return "", "", fmt.Errorf("at least two arguments must be passed to ipam plugin, a namespace and a "+ + "OOB subnet label, got %d", len(args)) } namespace := args[0] diff --git a/plugins/pxeboot/plugin_test.go b/plugins/pxeboot/plugin_test.go index 4bc399a..714085e 100644 --- a/plugins/pxeboot/plugin_test.go +++ b/plugins/pxeboot/plugin_test.go @@ -327,7 +327,10 @@ func TestTFTPNotRequested6(t *testing.T) { func TestPXERequested4(t *testing.T) { Init4() - req, err := dhcpv4.NewDiscovery(net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName)) + req, err := dhcpv4.NewDiscovery(net.HardwareAddr{ + 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, + dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName), + ) if err != nil { t.Fatal(err) } @@ -358,7 +361,10 @@ func TestPXERequested4(t *testing.T) { func TestTFTPRequested4(t *testing.T) { Init4() - req, err := dhcpv4.NewDiscovery(net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName)) + req, err := dhcpv4.NewDiscovery(net.HardwareAddr{ + 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, + dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName), + ) if err != nil { t.Fatal(err) } @@ -396,7 +402,10 @@ func TestTFTPRequested4(t *testing.T) { func TestPXENotRequested4(t *testing.T) { Init4() - req, err := dhcpv4.NewDiscovery(net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName)) + req, err := dhcpv4.NewDiscovery(net.HardwareAddr{ + 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, + dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName), + ) if err != nil { t.Fatal(err) } @@ -424,7 +433,10 @@ func TestPXENotRequested4(t *testing.T) { func TestTFTPNotRequested4(t *testing.T) { Init4() - req, err := dhcpv4.NewDiscovery(net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName)) + req, err := dhcpv4.NewDiscovery(net.HardwareAddr{ + 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, + dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName), + ) if err != nil { t.Fatal(err) } @@ -456,7 +468,10 @@ func TestTFTPNotRequested4(t *testing.T) { func TestWrongPXERequested4(t *testing.T) { Init4() - req, err := dhcpv4.NewDiscovery(net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName)) + req, err := dhcpv4.NewDiscovery(net.HardwareAddr{ + 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, + dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName), + ) if err != nil { t.Fatal(err) } @@ -487,7 +502,10 @@ func TestWrongPXERequested4(t *testing.T) { func TestWrongTFTPRequested4(t *testing.T) { Init4() - req, err := dhcpv4.NewDiscovery(net.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName)) + req, err := dhcpv4.NewDiscovery(net.HardwareAddr{ + 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}, + dhcpv4.WithRequestedOptions(dhcpv4.OptionBootfileName), + ) if err != nil { t.Fatal(err) } From 7a747a4d2b8f1504a6a914d0b46f6d1023ab960f Mon Sep 17 00:00:00 2001 From: Andreas Fritzler Date: Wed, 25 Sep 2024 16:52:39 +0200 Subject: [PATCH 4/4] Fix linting issues --- .golangci.yml | 3 +- plugins/httpboot/plugin_test.go | 96 ++++++++++----------------------- 2 files changed, 31 insertions(+), 68 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a09b41d..c04c147 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,8 +9,9 @@ issues: # restore some of the defaults # (fill in the rest as needed) exclude-rules: - - path: "api/*" + - path: "plugins/*" linters: + - dupl - lll - path: "internal/*" linters: diff --git a/plugins/httpboot/plugin_test.go b/plugins/httpboot/plugin_test.go index 1a5aed6..53cee9b 100644 --- a/plugins/httpboot/plugin_test.go +++ b/plugins/httpboot/plugin_test.go @@ -219,13 +219,19 @@ func TestHTTPBootNotRequested6(t *testing.T) { req.MessageType = dhcpv6.MessageTypeRequest req.AddOption(dhcpv6.OptRequestedOption(dhcpv6.OptionBootfileURL)) + // known LinkAddr + relayedRequest, err := dhcpv6.EncapsulateRelay(req, dhcpv6.MessageTypeRelayForward, net.IPv6loopback, net.IPv6loopback) + if err != nil { + t.Fatal(err) + } + stub, err := dhcpv6.NewMessage() if err != nil { t.Fatal(err) } stub.MessageType = dhcpv6.MessageTypeReply - resp, stop := handler6(req, stub) + resp, stop := handler6(relayedRequest, stub) if resp == nil { t.Fatal("plugin did not return a message") } @@ -496,31 +502,18 @@ func TestCustomHTTPBootRequestedKnownIP(t *testing.T) { } func TestCustomHTTPBootRequestedKnownMAC(t *testing.T) { - Init6(fmt.Sprintf(bootServiceEndpoint, bootServicePort)) - req, err := dhcpv6.NewMessage() - if err != nil { - t.Fatal(err) - } - req.MessageType = dhcpv6.MessageTypeRequest - req.AddOption(dhcpv6.OptRequestedOption(dhcpv6.OptionBootfileURL)) - optVendorClass := dhcpv6.OptVendorClass{} - buf := []byte{ - 0, 0, 5, 57, // nice "random" enterprise number, can be ignored - 0, 10, // length ot vendor class - 'H', 'T', 'T', 'P', 'C', 'l', 'i', 'e', 'n', 't', // vendor class - } - _ = optVendorClass.FromBytes(buf) - req.UpdateOption(&optVendorClass) + // known LinkLayerAddress + macAddress, _ := net.ParseMAC("aa:bb:cc:dd:ee:ff") - // not known LinkAddr - relayedRequest, err := dhcpv6.EncapsulateRelay(req, dhcpv6.MessageTypeRelayForward, - net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}, net.IPv6loopback) + err, relayedRequest := createHTTPBootRequest(t) if err != nil { t.Fatal(err) } - // known LinkLayerAddress - macAddress, _ := net.ParseMAC("aa:bb:cc:dd:ee:ff") + ensureBootURL(t, macAddress, relayedRequest, expectedCustomBootURL) +} + +func ensureBootURL(t *testing.T, macAddress net.HardwareAddr, relayedRequest *dhcpv6.RelayMessage, expectedBootURL string) { opt := dhcpv6.OptClientLinkLayerAddress(iana.HWTypeEthernet, macAddress) relayedRequest.AddOption(opt) @@ -544,8 +537,8 @@ func TestCustomHTTPBootRequestedKnownMAC(t *testing.T) { } bootFileURL := resp.(*dhcpv6.Message).Options.BootFileURL() - if bootFileURL != expectedCustomBootURL { - t.Errorf("Found BootFileURL %s, expected %s", bootFileURL, expectedCustomBootURL) + if bootFileURL != expectedBootURL { + t.Errorf("Found BootFileURL %s, expected %s", bootFileURL, expectedBootURL) } opts = resp.GetOption(dhcpv6.OptionVendorClass) @@ -565,6 +558,18 @@ func TestCustomHTTPBootRequestedKnownMAC(t *testing.T) { } func TestCustomHTTPBootRequestedUnknownClient(t *testing.T) { + // not known LinkLayerAddress + macAddress, _ := net.ParseMAC("11:22:33:44:55:66") + + err, relayedRequest := createHTTPBootRequest(t) + if err != nil { + t.Fatal(err) + } + + ensureBootURL(t, macAddress, relayedRequest, expectedDefaultCustomBootURL) +} + +func createHTTPBootRequest(t *testing.T) (error, *dhcpv6.RelayMessage) { Init6(fmt.Sprintf(bootServiceEndpoint, bootServicePort)) req, err := dhcpv6.NewMessage() if err != nil { @@ -587,50 +592,7 @@ func TestCustomHTTPBootRequestedUnknownClient(t *testing.T) { if err != nil { t.Fatal(err) } - - // not known LinkLayerAddress - macAddress, _ := net.ParseMAC("11:22:33:44:55:66") - opt := dhcpv6.OptClientLinkLayerAddress(iana.HWTypeEthernet, macAddress) - relayedRequest.AddOption(opt) - - stub, err := dhcpv6.NewMessage() - if err != nil { - t.Fatal(err) - } - stub.MessageType = dhcpv6.MessageTypeReply - - resp, stop := handler6(relayedRequest, stub) - if resp == nil { - t.Fatal("plugin did not return a message") - } - if stop { - t.Error("plugin interrupted processing, but it shouldn't have") - } - - opts := resp.GetOption(dhcpv6.OptionBootfileURL) - if len(opts) != optionEnabled { - t.Fatalf("Expected %d BootFileUrl option, got %d: %v", optionEnabled, len(opts), opts) - } - - bootFileURL := resp.(*dhcpv6.Message).Options.BootFileURL() - if bootFileURL != expectedDefaultCustomBootURL { - t.Errorf("Found BootFileURL %s, expected %s", bootFileURL, expectedDefaultCustomBootURL) - } - - opts = resp.GetOption(dhcpv6.OptionVendorClass) - if len(opts) != optionEnabled { - t.Fatalf("Expected %d VendorClass option, got %d: %v", optionEnabled, len(opts), opts) - } - - vc := resp.(*dhcpv6.Message).Options.VendorClasses()[0] - if vc.EnterpriseNumber != expectedEnterpriseNumber { - t.Errorf("Found EnterpriseNumber %d, expected %d", vc.EnterpriseNumber, expectedEnterpriseNumber) - } - - vcData := resp.(*dhcpv6.Message).Options.VendorClass(vc.EnterpriseNumber) - if !bytes.Equal(vcData[0], expectedHTTPClient) { - t.Errorf("Found VendorClass %x, expected %x", vcData[0], expectedHTTPClient) - } + return err, relayedRequest } func TestNoRelayCustomHTTPBootRequested(t *testing.T) {