diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ddde9f894..e6b1a30b6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,11 +15,11 @@ on: - prep-v[1-9].* env: - BUILDTIME_BASE: "golang:1.22.3-alpine3.18" + BUILDTIME_BASE: "golang:1.23.4-alpine3.21" # Do not bump past Alpine 3.18 until upstream netfilter problems in iptables v1.8.10 are resolved. See: # https://github.com/cloudnativelabs/kube-router/issues/1676 - RUNTIME_BASE: "alpine:3.18" - GO_VERSION: "~1.22.3" + RUNTIME_BASE: "alpine:3.21" + GO_VERSION: "~1.23.4" GO_CACHE: "/home/runner/.cache/go-build" GO_MOD_CACHE: "/home/runner/go/pkg/mod" diff --git a/.golangci.yml b/.golangci.yml index f94b26302..cd01a42c6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,21 +3,21 @@ run: linters: enable: - bodyclose + - copyloopvar - dogsled - dupl - durationcheck - - exportloopref - exhaustive - gochecknoinits - goconst - gocritic - gofmt - goimports - - gomnd - gosec - govet - lll - misspell + - mnd - nakedret - noctx - nolintlint @@ -35,24 +35,24 @@ issues: # Excluding single digits from magic number detector because it produces too many obvious results (like klog) - text: "Magic number: [0-9]{1}," linters: - - gomnd + - mnd # Exclude file masks from magic number detector because these numbers are obvious - text: "Magic number: 0[0-7]{3}," linters: - - gomnd + - mnd # Exlude IP masks netmasks as substituting them for constants only makes these less obvious - text: "Magic number: 255," linters: - - gomnd + - mnd path: pkg/controllers/proxy/network_services_controller.go # Exclude IP netmasks from magic number detector because these numbers are obvious - text: "Magic number: 32," linters: - - gomnd + - mnd # Exclude decimal bases from magic number detector because these numbers are obvious - text: "Magic number: 10," linters: - - gomnd + - mnd # Exclude file mask security findings as we are always intentional about the file masks we use - text: "G306:" linters: @@ -70,6 +70,7 @@ issues: # always show all issues of a type rather than showing 3 max-same-issues: 0 output: - format: tab + formats: + - format: tab print-issued-lines: true print-linter-name: true diff --git a/.goreleaser.yml b/.goreleaser.yml index 93ff96052..a0ab77c75 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -41,4 +41,4 @@ archives: - Documentation* snapshot: - name_template: SNAPSHOT-{{ .Commit }} + version_template: SNAPSHOT-{{ .Commit }} diff --git a/Dockerfile b/Dockerfile index 971a1a6d8..942059d69 100644 --- a/Dockerfile +++ b/Dockerfile @@ -25,7 +25,7 @@ FROM ${RUNTIME_BASE} RUN apk add --no-cache \ iptables \ - ip6tables \ + iptables-legacy \ ipset \ iproute2 \ ipvsadm \ @@ -37,6 +37,11 @@ RUN apk add --no-cache \ curl -L -o /usr/local/share/bash-completion/bash-completion \ https://raw.githubusercontent.com/scop/bash-completion/master/bash_completion +COPY ./build/apks /apks + +RUN apk add --allow-untrusted /apks/*.apk && \ + rm -rf /apks + COPY build/image-assets/bashrc /root/.bashrc COPY build/image-assets/profile /root/.profile COPY build/image-assets/vimrc /root/.vimrc diff --git a/Makefile b/Makefile index 2d6676138..8ac67ea5b 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ DOCKER=$(if $(or $(IN_DOCKER_GROUP),$(IS_ROOT),$(OSX)),docker,sudo docker) MAKEFILE_DIR=$(dir $(realpath $(firstword $(MAKEFILE_LIST)))) UPSTREAM_IMPORT_PATH=$(GOPATH)/src/github.com/cloudnativelabs/kube-router/ BUILD_IN_DOCKER?=true -DOCKER_BUILD_IMAGE?=golang:1.22.3-alpine3.18 +DOCKER_BUILD_IMAGE?=golang:1.23.4-alpine3.21 ## These variables are used by the Dockerfile as the bases for building and creating the runtime container ## During CI these come from .github/workflows/ci.yaml below we define for local builds as well GO_CACHE?=$(shell go env GOCACHE) @@ -25,14 +25,14 @@ GO_MOD_CACHE?=$(shell go env GOMODCACHE) BUILDTIME_BASE?=$(DOCKER_BUILD_IMAGE) # Do not bump past Alpine 3.18 until upstream netfilter problems in iptables v1.8.10 are resolved. See: # https://github.com/cloudnativelabs/kube-router/issues/1676 -RUNTIME_BASE?=alpine:3.18 -DOCKER_LINT_IMAGE?=golangci/golangci-lint:v1.56.2 +RUNTIME_BASE?=alpine:3.21 +DOCKER_LINT_IMAGE?=golangci/golangci-lint:v1.62.2 DOCKER_MARKDOWNLINT_IMAGE?=tmknom/markdownlint:0.39.0 GOBGP_VERSION=v3.29.0 QEMU_IMAGE?=multiarch/qemu-user-static -GORELEASER_VERSION=v1.24.0 -MOQ_VERSION=v0.3.4 -CNI_VERSION=v1.4.0 +GORELEASER_VERSION=v2.5.0 +MOQ_VERSION=v0.5.1 +CNI_VERSION=v1.6.1 UID?=$(shell id -u) ifeq ($(GOARCH), arm) ARCH_TAG_PREFIX=$(GOARCH) diff --git a/build/apks/APKINDEX.tar.gz b/build/apks/APKINDEX.tar.gz new file mode 100644 index 000000000..ec204b67e Binary files /dev/null and b/build/apks/APKINDEX.tar.gz differ diff --git a/build/apks/iptables-1.8.11-r1.apk b/build/apks/iptables-1.8.11-r1.apk new file mode 100644 index 000000000..70aa76fc7 Binary files /dev/null and b/build/apks/iptables-1.8.11-r1.apk differ diff --git a/build/apks/iptables-dev-1.8.11-r1.apk b/build/apks/iptables-dev-1.8.11-r1.apk new file mode 100644 index 000000000..9ac582640 Binary files /dev/null and b/build/apks/iptables-dev-1.8.11-r1.apk differ diff --git a/build/apks/iptables-doc-1.8.11-r1.apk b/build/apks/iptables-doc-1.8.11-r1.apk new file mode 100644 index 000000000..a2939815d Binary files /dev/null and b/build/apks/iptables-doc-1.8.11-r1.apk differ diff --git a/build/apks/iptables-legacy-1.8.11-r1.apk b/build/apks/iptables-legacy-1.8.11-r1.apk new file mode 100644 index 000000000..1a92f79f4 Binary files /dev/null and b/build/apks/iptables-legacy-1.8.11-r1.apk differ diff --git a/build/apks/iptables-openrc-1.8.11-r1.apk b/build/apks/iptables-openrc-1.8.11-r1.apk new file mode 100644 index 000000000..1b481f867 Binary files /dev/null and b/build/apks/iptables-openrc-1.8.11-r1.apk differ diff --git a/build/apks/libip4tc-1.8.11-r1.apk b/build/apks/libip4tc-1.8.11-r1.apk new file mode 100644 index 000000000..5e70c0966 Binary files /dev/null and b/build/apks/libip4tc-1.8.11-r1.apk differ diff --git a/build/apks/libip6tc-1.8.11-r1.apk b/build/apks/libip6tc-1.8.11-r1.apk new file mode 100644 index 000000000..5bc9c821b Binary files /dev/null and b/build/apks/libip6tc-1.8.11-r1.apk differ diff --git a/build/apks/libipq-1.8.11-r1.apk b/build/apks/libipq-1.8.11-r1.apk new file mode 100644 index 000000000..ae854f62e Binary files /dev/null and b/build/apks/libipq-1.8.11-r1.apk differ diff --git a/build/apks/libxtables-1.8.11-r1.apk b/build/apks/libxtables-1.8.11-r1.apk new file mode 100644 index 000000000..cea9b17b5 Binary files /dev/null and b/build/apks/libxtables-1.8.11-r1.apk differ diff --git a/pkg/controllers/lballoc/lballoc.go b/pkg/controllers/lballoc/lballoc.go index 131bc4330..19dadb27d 100644 --- a/pkg/controllers/lballoc/lballoc.go +++ b/pkg/controllers/lballoc/lballoc.go @@ -155,7 +155,7 @@ func (lbc *LoadBalancerController) runLeaderElection(ctx context.Context, isLead leaderelection.RunOrDie(ctx, leaderelection.LeaderElectionConfig{ Lock: lbc.lock, ReleaseOnCancel: true, - LeaseDuration: 15 * time.Second, //nolint:gomnd // No reason for a 15 second constant + LeaseDuration: 15 * time.Second, //nolint:mnd // No reason for a 15 second constant RenewDeadline: 10 * time.Second, RetryPeriod: 2 * time.Second, Callbacks: leaderelection.LeaderCallbacks{ diff --git a/pkg/controllers/netpol/network_policy_controller.go b/pkg/controllers/netpol/network_policy_controller.go index a93788662..8ff36db07 100644 --- a/pkg/controllers/netpol/network_policy_controller.go +++ b/pkg/controllers/netpol/network_policy_controller.go @@ -308,7 +308,6 @@ func (npc *NetworkPolicyController) fullPolicySync() { } for ipFamily, iptablesSaveRestore := range npc.iptablesSaveRestore { - ipFamily := ipFamily restoreStart := time.Now() err := iptablesSaveRestore.Restore("filter", npc.filterTableRules[ipFamily].Bytes()) restoreEndTime := time.Since(restoreStart) @@ -855,7 +854,6 @@ func NewNetworkPolicyController(clientset kubernetes.Interface, // Validate that ClusterIP service range type matches the configuration if config.EnableIPv4 && !config.EnableIPv6 { if !netutils.IsIPv4CIDR(&npc.serviceClusterIPRanges[0]) { - //nolint:goconst // we don't care about abstracting an error message return nil, fmt.Errorf("failed to get parse --service-cluster-ip-range parameter: " + "IPv4 is enabled but only IPv6 address is provided") } diff --git a/pkg/controllers/netpol/network_policy_controller_test.go b/pkg/controllers/netpol/network_policy_controller_test.go index 614f6d223..0b1139b06 100644 --- a/pkg/controllers/netpol/network_policy_controller_test.go +++ b/pkg/controllers/netpol/network_policy_controller_test.go @@ -574,7 +574,7 @@ func TestNetworkPolicyBuilder(t *testing.T) { } for ipFamily, filterTableRules := range krNetPol.filterTableRules { for _, np := range netpols { - fmt.Printf(np.policyType) + fmt.Print(np.policyType) if np.policyType == kubeEgressPolicyType || np.policyType == kubeBothPolicyType { err = krNetPol.processEgressRules(np, "", nil, "1", ipFamily) if err != nil { diff --git a/pkg/controllers/netpol/pod.go b/pkg/controllers/netpol/pod.go index b42175906..5d2791653 100644 --- a/pkg/controllers/netpol/pod.go +++ b/pkg/controllers/netpol/pod.go @@ -82,7 +82,6 @@ func (npc *NetworkPolicyController) syncPodFirewallChains(networkPoliciesInfo [] for ipFamily, filterTableRules := range npc.filterTableRules { _, err := getPodIPForFamily(pod, ipFamily) if err != nil { - //nolint:goconst // don't need to make error messages a constant klog.V(2).Infof("unable to get address for pod: %s -- skipping drop rules for pod "+ "(this is normal for pods that are not dual-stack)", err.Error()) continue diff --git a/pkg/controllers/netpol/policy.go b/pkg/controllers/netpol/policy.go index b6237bf66..d7de17e34 100644 --- a/pkg/controllers/netpol/policy.go +++ b/pkg/controllers/netpol/policy.go @@ -146,7 +146,6 @@ func (npc *NetworkPolicyController) syncNetworkPolicyChains(networkPoliciesInfo } for ipFamily, ipset := range npc.ipSetHandlers { - ipFamily := ipFamily restoreStart := time.Now() err := ipset.Restore() restoreEndTime := time.Since(restoreStart) @@ -223,7 +222,6 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo } npc.ipSetHandlers[ipFamily].RefreshSet(namedPortIPSetName, setEntries, utils.TypeHashIP) - //nolint:goconst // don't need to make error messages a constant comment := "rule to ACCEPT traffic from source pods to dest pods selected by policy name " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, srcPodIPSetName, namedPortIPSetName, @@ -250,7 +248,6 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo // with specified port (if any) and protocol if ingressRule.matchAllSource && !ingressRule.matchAllPorts { for _, portProtocol := range ingressRule.ports { - //nolint:goconst // don't need to make error messages a constant comment := "rule to ACCEPT traffic from all sources to dest pods selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, "", targetDestPodIPSetName, @@ -296,7 +293,6 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo if !ingressRule.matchAllPorts { for _, portProtocol := range ingressRule.ports { - //nolint:goconst // don't need to make error messages a constant comment := "rule to ACCEPT traffic from specified ipBlocks to dest pods selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, srcIPBlockIPSetName, @@ -407,7 +403,6 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo, // with specified port (if any) and protocol if egressRule.matchAllDestinations && !egressRule.matchAllPorts { for _, portProtocol := range egressRule.ports { - //nolint:goconst // don't need to make error messages a constant comment := "rule to ACCEPT traffic from source pods to all destinations selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, targetSourcePodIPSetName, diff --git a/pkg/controllers/proxy/linux_networking.go b/pkg/controllers/proxy/linux_networking.go index 2fe37b3b3..2a9563984 100644 --- a/pkg/controllers/proxy/linux_networking.go +++ b/pkg/controllers/proxy/linux_networking.go @@ -265,10 +265,18 @@ func (ln *linuxNetworking) ipvsAddService(svcs []*ipvs.Service, vip net.IP, prot vip, svc.Address, protocol, svc.Protocol, port, svc.Port) if vip.Equal(svc.Address) && protocol == svc.Protocol && port == svc.Port { klog.V(2).Info("Service matched VIP") + ptim, err := utils.Int32ToUInt32(persistentTimeout) + if err != nil { + return svcs, nil, fmt.Errorf("failed to convert persistent timeout to uint32: %v", err) + } if (persistent && (svc.Flags&ipvsPersistentFlagHex) == 0) || (!persistent && (svc.Flags&ipvsPersistentFlagHex) != 0) || - svc.Timeout != uint32(persistentTimeout) { - ipvsSetPersistence(svc, persistent, persistentTimeout) + svc.Timeout != ptim { + err = ipvsSetPersistence(svc, persistent, persistentTimeout) + if err != nil { + return svcs, nil, fmt.Errorf("failed to set persistence for service %s due to: %v", + ipvsServiceString(svc), err) + } err = ln.ipvsUpdateService(svc) if err != nil { @@ -323,7 +331,11 @@ func (ln *linuxNetworking) ipvsAddService(svcs []*ipvs.Service, vip net.IP, prot Netmask: ipMask, } - ipvsSetPersistence(&svc, persistent, persistentTimeout) + err = ipvsSetPersistence(&svc, persistent, persistentTimeout) + if err != nil { + return svcs, nil, fmt.Errorf("failed to set persistence for service %s due to: %v", + ipvsServiceString(&svc), err) + } ipvsSetSchedFlags(&svc, flags) klog.V(1).Infof("%s didn't match any existing IPVS services, creating a new IPVS service", @@ -356,13 +368,17 @@ func (ln *linuxNetworking) ipvsAddFWMarkService(svcs []*ipvs.Service, fwMark uin if fwMark == svc.FWMark { if (persistent && (svc.Flags&ipvsPersistentFlagHex) == 0) || (!persistent && (svc.Flags&ipvsPersistentFlagHex) != 0) { - ipvsSetPersistence(svc, persistent, persistentTimeout) + err := ipvsSetPersistence(svc, persistent, persistentTimeout) + if err != nil { + return nil, fmt.Errorf("failed to set persistence for service %s due to: %v", + ipvsServiceString(svc), err) + } if changedIpvsSchedFlags(svc, flags) { ipvsSetSchedFlags(svc, flags) } - err := ln.ipvsUpdateService(svc) + err = ln.ipvsUpdateService(svc) if err != nil { return nil, fmt.Errorf("failed to update persistence flags for service %s due to %v", ipvsServiceString(svc), err) @@ -419,10 +435,13 @@ func (ln *linuxNetworking) ipvsAddFWMarkService(svcs []*ipvs.Service, fwMark uin SchedName: ipvs.RoundRobin, } - ipvsSetPersistence(&svc, persistent, persistentTimeout) + err := ipvsSetPersistence(&svc, persistent, persistentTimeout) + if err != nil { + return nil, fmt.Errorf("failed to set persistence for service %s due to: %v", ipvsServiceString(&svc), err) + } ipvsSetSchedFlags(&svc, flags) - err := ln.ipvsNewService(&svc) + err = ln.ipvsNewService(&svc) if err != nil { return nil, err } diff --git a/pkg/controllers/proxy/network_services_controller.go b/pkg/controllers/proxy/network_services_controller.go index aaf8015fc..33afacc91 100644 --- a/pkg/controllers/proxy/network_services_controller.go +++ b/pkg/controllers/proxy/network_services_controller.go @@ -303,7 +303,7 @@ func (nsc *NetworkServicesController) Run(healthChan chan<- *healthcheck.Control // https://github.com/cloudnativelabs/kube-router/issues/282 err = nsc.setupIpvsFirewall() if err != nil { - klog.Fatalf("error setting up ipvs firewall: %s" + err.Error()) + klog.Fatalf("error setting up ipvs firewall: %v", err.Error()) } nsc.ProxyFirewallSetup.Broadcast() @@ -346,7 +346,7 @@ func (nsc *NetworkServicesController) Run(healthChan chan<- *healthcheck.Control klog.V(1).Info("Performing requested full sync of services") err = nsc.doSync() if err != nil { - klog.Errorf("Error during full sync in network service controller. Error: " + err.Error()) + klog.Errorf("error during full sync in network service controller. Error: %v", err) } case synctypeIpvs: // We call the component pieces of doSync() here because for methods that send this on the channel they @@ -356,11 +356,11 @@ func (nsc *NetworkServicesController) Run(healthChan chan<- *healthcheck.Control nsc.mu.Lock() err = nsc.syncIpvsServices(nsc.serviceMap, nsc.endpointsMap) if err != nil { - klog.Errorf("Error during ipvs sync in network service controller. Error: " + err.Error()) + klog.Errorf("error during ipvs sync in network service controller. Error: %v", err) } err = nsc.syncHairpinIptablesRules() if err != nil { - klog.Errorf("Error syncing hairpin iptables rules: %s", err.Error()) + klog.Errorf("error syncing hairpin iptables rules: %v", err) } nsc.mu.Unlock() } @@ -373,7 +373,7 @@ func (nsc *NetworkServicesController) Run(healthChan chan<- *healthcheck.Control healthcheck.SendHeartBeat(healthChan, healthcheck.NetworkServicesController) err := nsc.doSync() if err != nil { - klog.Errorf("Error during periodic ipvs sync in network service controller. Error: " + err.Error()) + klog.Errorf("error during periodic ipvs sync in network service controller. Error: %v", err.Error()) klog.Errorf("Skipping sending heartbeat from network service controller as periodic sync failed.") } else { healthcheck.SendHeartBeat(healthChan, healthcheck.NetworkServicesController) @@ -581,7 +581,6 @@ func (nsc *NetworkServicesController) cleanupIpvsFirewall() { } for _, ipSetName := range []string{localIPsIPSetName, serviceIPsIPSetName, serviceIPPortsSetName} { - ipSetName := ipSetName if _, ok := ipSetHandler.Sets()[ipSetName]; ok { err = ipSetHandler.Destroy(ipSetName) if err != nil { @@ -731,16 +730,20 @@ func (nsc *NetworkServicesController) publishMetrics(serviceInfoMap serviceInfoM protocol = convertSvcProtoToSysCallProto(svc.protocol) for _, ipvsSvc := range ipvsSvcs { + uPort, err := utils.IntToUInt16(svc.port) + if err != nil { + klog.Errorf("failed to convert port %d to uint16: %v", svc.port, err) + } switch svcAddress := ipvsSvc.Address.String(); svcAddress { case svc.clusterIP.String(): - if protocol == ipvsSvc.Protocol && uint16(svc.port) == ipvsSvc.Port { + if protocol == ipvsSvc.Protocol && uPort == ipvsSvc.Port { pushMetric = true svcVip = svc.clusterIP.String() } else { pushMetric = false } case nsc.krNode.GetPrimaryNodeIP().String(): - if protocol == ipvsSvc.Protocol && uint16(svc.port) == ipvsSvc.Port { + if protocol == ipvsSvc.Protocol && uPort == ipvsSvc.Port { pushMetric = true svcVip = nsc.krNode.GetPrimaryNodeIP().String() } else { @@ -1541,14 +1544,21 @@ func ipvsDestinationString(d *ipvs.Destination) string { return fmt.Sprintf("%s:%v (Family: %s, Weight: %v)", d.Address, d.Port, family, d.Weight) } -func ipvsSetPersistence(svc *ipvs.Service, p bool, timeout int32) { +func ipvsSetPersistence(svc *ipvs.Service, p bool, timeout int32) error { if p { + uTimeout, err := utils.Int32ToUInt32(timeout) + if err != nil { + return fmt.Errorf("failed to convert timeout to uint32: %v", err) + } + svc.Flags |= ipvsPersistentFlagHex - svc.Timeout = uint32(timeout) + svc.Timeout = uTimeout } else { svc.Flags &^= ipvsPersistentFlagHex svc.Timeout = 0 } + + return nil } func ipvsSetSchedFlags(svc *ipvs.Service, s schedFlags) { @@ -1811,7 +1821,7 @@ func (nsc *NetworkServicesController) Cleanup() { } else { err = netlink.LinkDel(dummyVipInterface) if err != nil { - klog.Errorf("Could not delete dummy interface " + KubeDummyIf + " due to " + err.Error()) + klog.Errorf("could not delete dummy interface %s due to: %v", KubeDummyIf, err.Error()) return } } diff --git a/pkg/controllers/proxy/service_endpoints_sync.go b/pkg/controllers/proxy/service_endpoints_sync.go index 107be86c9..c887d6587 100644 --- a/pkg/controllers/proxy/service_endpoints_sync.go +++ b/pkg/controllers/proxy/service_endpoints_sync.go @@ -135,6 +135,10 @@ func (nsc *NetworkServicesController) setupClusterIPServices(serviceInfoMap serv if err != nil { return fmt.Errorf("failed creating dummy interface: %v", err) } + sPort, err := utils.IntToUInt16(svc.port) + if err != nil { + return fmt.Errorf("failed to convert service port to uint16: %v", err) + } for family, famClusIPs := range clusterIPs { var nodeIP string @@ -159,7 +163,7 @@ func (nsc *NetworkServicesController) setupClusterIPServices(serviceInfoMap serv // create IPVS service for the service to be exposed through the cluster ip ipvsSvcs, svcID, ipvsSvc = nsc.addIPVSService(ipvsSvcs, activeServiceEndpointMap, svc, clusterIP, - protocol, uint16(svc.port)) + protocol, sPort) // We weren't able to create the IPVS service, so we won't be able to add endpoints to it if svcID == "" { // not logging an error here because it was already logged in the addIPVSService function @@ -226,7 +230,6 @@ func (nsc *NetworkServicesController) addEndpointsToIPVSService(endpoints []endp switch family { case v1.IPv4Protocol: if endpoint.isIPv6 { - //nolint:goconst // don't need to make error messages a constant klog.V(3).Infof("not adding endpoint %s to service %s with VIP %s because families don't "+ "match", endpoint.ip, svc.name, vip) continue @@ -241,13 +244,19 @@ func (nsc *NetworkServicesController) addEndpointsToIPVSService(endpoints []endp syscallINET = syscall.AF_INET6 } + ePort, err := utils.IntToUInt16(endpoint.port) + if err != nil { + klog.Errorf("failed to convert endpoint port to uint16: %v", err) + continue + } + dst := ipvs.Destination{ Address: eIP, AddressFamily: syscallINET, - Port: uint16(endpoint.port), + Port: ePort, Weight: 1, } - err := nsc.ln.ipvsAddServer(ipvsSvc, &dst) + err = nsc.ln.ipvsAddServer(ipvsSvc, &dst) if err != nil { klog.Errorf("encountered error adding endpoint to service: %v", err) continue @@ -282,6 +291,11 @@ func (nsc *NetworkServicesController) setupNodePortServices(serviceInfoMap servi continue } + nPort, err := utils.IntToUInt16(svc.nodePort) + if err != nil { + return fmt.Errorf("failed to convert node port to uint16: %v", err) + } + var svcID string var ipvsSvc *ipvs.Service if nsc.nodeportBindOnAllIP { @@ -312,7 +326,7 @@ func (nsc *NetworkServicesController) setupNodePortServices(serviceInfoMap servi for _, addr := range addrs { ipvsSvcs, svcID, ipvsSvc = nsc.addIPVSService(ipvsSvcs, activeServiceEndpointMap, svc, addr, - protocol, uint16(svc.nodePort)) + protocol, nPort) // We weren't able to create the IPVS service, so we won't be able to add endpoints to it if svcID == "" { continue @@ -322,7 +336,7 @@ func (nsc *NetworkServicesController) setupNodePortServices(serviceInfoMap servi } } else { ipvsSvcs, svcID, ipvsSvc = nsc.addIPVSService(ipvsSvcs, activeServiceEndpointMap, svc, - nsc.krNode.GetPrimaryNodeIP(), protocol, uint16(svc.nodePort)) + nsc.krNode.GetPrimaryNodeIP(), protocol, nPort) // We weren't able to create the IPVS service, so we won't be able to add endpoints to it if svcID == "" { continue @@ -416,6 +430,11 @@ func (nsc *NetworkServicesController) setupExternalIPForService(svc *serviceInfo return fmt.Errorf("failed get list of IPVS services due to: %v", err) } + sPort, err := utils.IntToUInt16(svc.port) + if err != nil { + return fmt.Errorf("failed to convert service port to uint16: %v", err) + } + // ensure director with vip assigned err = nsc.ln.ipAddrAdd(dummyVipInterface, externalIP.String(), nodeIP.String(), true) if err != nil && err.Error() != IfaceHasAddr { @@ -424,8 +443,7 @@ func (nsc *NetworkServicesController) setupExternalIPForService(svc *serviceInfo } // create IPVS service for the service to be exposed through the external ip - _, svcID, ipvsExternalIPSvc = nsc.addIPVSService(ipvsSvcs, svcEndpointMap, svc, externalIP, protocol, - uint16(svc.port)) + _, svcID, ipvsExternalIPSvc = nsc.addIPVSService(ipvsSvcs, svcEndpointMap, svc, externalIP, protocol, sPort) if svcID == "" { return fmt.Errorf("failed to create ipvs service for external ip: %s", externalIP) } @@ -510,7 +528,12 @@ func (nsc *NetworkServicesController) setupExternalIPForDSRService(svcIn *servic return fmt.Errorf("failed to generate FW mark") } - ipvsExternalIPSvc, err := nsc.ln.ipvsAddFWMarkService(ipvsSvcs, fwMark, sysFamily, protocol, uint16(svcIn.port), + sInPort, err := utils.IntToUInt16(svcIn.port) + if err != nil { + return fmt.Errorf("failed to convert serviceIn port to uint16: %v", err) + } + + ipvsExternalIPSvc, err := nsc.ln.ipvsAddFWMarkService(ipvsSvcs, fwMark, sysFamily, protocol, sInPort, svcIn.sessionAffinity, svcIn.sessionAffinityTimeoutSeconds, svcIn.scheduler, svcIn.flags) if err != nil { return fmt.Errorf("failed to create IPVS service for FWMark service: %d (external IP: %s) due to: %s", @@ -570,12 +593,17 @@ func (nsc *NetworkServicesController) setupExternalIPForDSRService(svcIn *servic syscallINET = syscall.AF_INET6 } + ePort, err := utils.IntToUInt16(endpoint.port) + if err != nil { + return fmt.Errorf("failed to convert endpoint port to uint16: %v", err) + } + // create the basic IPVS destination record dst := ipvs.Destination{ Address: eIP, AddressFamily: syscallINET, ConnectionFlags: ipvs.ConnectionFlagTunnel, - Port: uint16(endpoint.port), + Port: ePort, Weight: 1, } @@ -794,7 +822,7 @@ func (nsc *NetworkServicesController) cleanupDSRService(fwMark uint32) error { mangleTableRulesDump := bytes.Buffer{} var mangleTableRules []string if err := utils.SaveInto(iptablesBinary, "mangle", &mangleTableRulesDump); err != nil { - klog.Errorf("Failed to run iptables-save: %s" + err.Error()) + klog.Errorf("failed to run iptables-save: %v", err) } else { mangleTableRules = strings.Split(mangleTableRulesDump.String(), "\n") } diff --git a/pkg/controllers/routing/aws.go b/pkg/controllers/routing/aws.go index 7402a3c75..61b248d48 100644 --- a/pkg/controllers/routing/aws.go +++ b/pkg/controllers/routing/aws.go @@ -35,7 +35,7 @@ func (nrc *NetworkRoutingController) disableSourceDestinationCheck() { providerID := strings.Replace(node.Spec.ProviderID, "///", "//", 1) URL, err := url.Parse(providerID) if err != nil { - klog.Errorf("Failed to parse URL for providerID " + providerID + " : " + err.Error()) + klog.Errorf("failed to parse URL for providerID %s: %v", providerID, err) return } instanceID := URL.Path @@ -45,7 +45,7 @@ func (nrc *NetworkRoutingController) disableSourceDestinationCheck() { metadataClient := ec2metadata.New(sess) region, err := metadataClient.Region() if err != nil { - klog.Errorf("Failed to disable source destination check due to: " + err.Error()) + klog.Errorf("failed to disable source destination check due to: %v", err) return } sess.Config.Region = aws.String(region) @@ -66,9 +66,9 @@ func (nrc *NetworkRoutingController) disableSourceDestinationCheck() { "disabling src-dst check.") return } - klog.Errorf("Failed to disable source destination check due to: %v", err.Error()) + klog.Errorf("failed to disable source destination check due to: %v", err) } else { - klog.Infof("Disabled source destination check for the instance: " + instanceID) + klog.Infof("disabled source destination check for the instance: %s", instanceID) } // to prevent EC2 rejecting API call due to API throttling give a delay between the calls diff --git a/pkg/controllers/routing/bgp_policies.go b/pkg/controllers/routing/bgp_policies.go index 890b42940..5e1dd147f 100644 --- a/pkg/controllers/routing/bgp_policies.go +++ b/pkg/controllers/routing/bgp_policies.go @@ -135,10 +135,14 @@ func (nrc *NetworkRoutingController) addPodCidrDefinedSet() error { if cidrLen < 0 || cidrLen > cidrMax { return fmt.Errorf("the pod CIDR IP given is not a proper mask: %d", cidrLen) } + uCIDRLen, err := utils.IntToUInt32(cidrLen) + if err != nil { + return fmt.Errorf("failed to convert CIDR length to uint32: %v", err) + } prefixes = append(prefixes, &gobgpapi.Prefix{ IpPrefix: cidr, - MaskLengthMin: uint32(cidrLen), - MaskLengthMax: uint32(cidrLen), + MaskLengthMin: uCIDRLen, + MaskLengthMax: uCIDRLen, }) } podCidrDefinedSet := &gobgpapi.DefinedSet{ @@ -318,7 +322,13 @@ func (nrc *NetworkRoutingController) addCustomImportRejectDefinedSet() error { prefix := new(gobgpapi.Prefix) prefix.IpPrefix = ipNet.String() mask, _ := ipNet.Mask.Size() - prefix.MaskLengthMin = uint32(mask) + + uIntMask, err := utils.IntToUInt32(mask) + if err != nil { + return fmt.Errorf("failed to convert mask to uint32: %v", err) + } + + prefix.MaskLengthMin = uIntMask prefix.MaskLengthMax = uint32(ipv4MaskMinBits) prefixes = append(prefixes, prefix) } diff --git a/pkg/controllers/routing/network_routes_controller.go b/pkg/controllers/routing/network_routes_controller.go index f9a4a6240..4d9ea06b1 100644 --- a/pkg/controllers/routing/network_routes_controller.go +++ b/pkg/controllers/routing/network_routes_controller.go @@ -393,8 +393,8 @@ func (nrc *NetworkRoutingController) Run(healthChan chan<- *healthcheck.Controll if err == nil { healthcheck.SendHeartBeat(healthChan, healthcheck.NetworkRoutesController) } else { - klog.Errorf("Error during periodic sync in network routing controller. Error: " + err.Error()) - klog.Errorf("Skipping sending heartbeat from network routing controller as periodic sync failed.") + klog.Errorf("error during periodic sync in network routing controller. Error: %v", err) + klog.Errorf("skipping sending heartbeat from network routing controller as periodic sync failed.") } select { @@ -460,7 +460,7 @@ func (nrc *NetworkRoutingController) watchBgpUpdates() { } klog.V(2).Infof("Processing bgp route advertisement from peer: %s", path.NeighborIp) if err := nrc.injectRoute(path); err != nil { - klog.Errorf("Failed to inject routes due to: " + err.Error()) + klog.Errorf("failed to inject routes due to: %v", err) } } } @@ -476,7 +476,7 @@ func (nrc *NetworkRoutingController) watchBgpUpdates() { }, }, pathWatch) if err != nil { - klog.Errorf("failed to register monitor global routing table callback due to : " + err.Error()) + klog.Errorf("failed to register monitor global routing table callback due to: %v", err) } } @@ -520,7 +520,7 @@ func (nrc *NetworkRoutingController) advertisePodRoute() error { }, }) if err != nil { - return fmt.Errorf(err.Error()) + return err } klog.V(1).Infof("Response from adding path: %s", response) } @@ -566,7 +566,7 @@ func (nrc *NetworkRoutingController) advertisePodRoute() error { }, }) if err != nil { - return fmt.Errorf(err.Error()) + return err } klog.V(1).Infof("Response from adding path: %s", response) } @@ -777,11 +777,11 @@ func (nrc *NetworkRoutingController) Cleanup() { for _, ipset := range nrc.ipSetHandlers { err = ipset.Save() if err != nil { - klog.Errorf("Failed to clean up ipsets: " + err.Error()) + klog.Errorf("failed to clean up ipsets: %v", err) } err = ipset.DestroyAllWithin() if err != nil { - klog.Warningf("Error deleting ipset: %s", err.Error()) + klog.Warningf("error deleting ipset: %v", err) } } @@ -1060,15 +1060,20 @@ func (nrc *NetworkRoutingController) startBgpServer(grpcServer bool) error { localAddressList = append(localAddressList, addr) } + intBGPPort, err := utils.UInt32ToInt32(nrc.bgpPort) + if err != nil { + return fmt.Errorf("failed to convert BGP port to int32: %v", err) + } + global := &gobgpapi.Global{ Asn: nodeAsnNumber, RouterId: nrc.routerID, ListenAddresses: localAddressList, - ListenPort: int32(nrc.bgpPort), + ListenPort: intBGPPort, } if err := nrc.bgpServer.StartBgp(context.Background(), &gobgpapi.StartBgpRequest{Global: global}); err != nil { - return errors.New("failed to start BGP server due to : " + err.Error()) + return fmt.Errorf("failed to start BGP server due to: %v", err) } go nrc.watchBgpUpdates() @@ -1402,13 +1407,23 @@ func NewNetworkRoutingController(clientset kubernetes.Interface, // Convert ints to uint32s peerASNs := make([]uint32, 0) for _, i := range kubeRouterConfig.PeerASNs { - peerASNs = append(peerASNs, uint32(i)) + ui, err := utils.UIntToUInt32(i) + if err != nil { + return nil, fmt.Errorf("failed to convert Peer ASNs to uint32: %s", err) + } + + peerASNs = append(peerASNs, ui) } // Convert uints to uint16s peerPorts := make([]uint32, 0) for _, i := range kubeRouterConfig.PeerPorts { - peerPorts = append(peerPorts, uint32(i)) + ui, err := utils.UIntToUInt32(i) + if err != nil { + return nil, fmt.Errorf("failed to convert Peer Port to uint32: %s", err) + } + + peerPorts = append(peerPorts, ui) } // PeerPasswords as cli params take precedence over password file diff --git a/pkg/options/options.go b/pkg/options/options.go index 5bd81d18c..98322437b 100644 --- a/pkg/options/options.go +++ b/pkg/options/options.go @@ -89,7 +89,7 @@ type KubeRouterConfig struct { } func NewKubeRouterConfig() *KubeRouterConfig { - //nolint:gomnd // Here we are specifying the names of the literals which is very similar to constant behavior + //nolint:mnd // Here we are specifying the names of the literals which is very similar to constant behavior return &KubeRouterConfig{ BGPGracefulRestartDeferralTime: 360 * time.Second, BGPGracefulRestartTime: 90 * time.Second, diff --git a/pkg/tunnels/linux_tunnels.go b/pkg/tunnels/linux_tunnels.go index 1da4f3d58..2c8a59e96 100644 --- a/pkg/tunnels/linux_tunnels.go +++ b/pkg/tunnels/linux_tunnels.go @@ -192,7 +192,6 @@ func (o *OverlayTunnel) SetupOverlayTunnel(tunnelName string, nextHop net.IP, fouArgs = append(fouArgs, "fou", "add", "port", strFormattedEncapPort, "gue") out, err := exec.Command("ip", fouArgs...).CombinedOutput() if err != nil { - //nolint:goconst // don't need to make error messages a constant return nil, fmt.Errorf("route not injected for the route advertised by the node %s "+ "Failed to set FoU tunnel port - error: %s, output: %s", tunnelName, err, string(out)) } @@ -256,7 +255,7 @@ func CleanupTunnel(destinationSubnet *net.IPNet, tunnelName string) { klog.V(1).Infof("Cleaning up any lingering tunnel interfaces named: %s", tunnelName) if link, err := netlink.LinkByName(tunnelName); err == nil { if err = netlink.LinkDel(link); err != nil { - klog.Errorf("Failed to delete tunnel link for the node due to " + err.Error()) + klog.Errorf("failed to delete tunnel link for the node due to %v", err) } } } diff --git a/pkg/utils/linux_routingtest.go b/pkg/utils/linux_routingtest.go index 1c83ce657..cef803e17 100644 --- a/pkg/utils/linux_routingtest.go +++ b/pkg/utils/linux_routingtest.go @@ -32,10 +32,10 @@ func NewFakeLocalLinkQuerier(addrStrings []string, mtus []int) *FakeLocalLinkQue ip := net.ParseIP(addr) var netMask net.IPMask if ip.To4() != nil { - //nolint:gomnd // Hardcoded value is used for testing purposes + //nolint:mnd // Hardcoded value is used for testing purposes netMask = net.CIDRMask(24, 32) } else { - //nolint:gomnd // Hardcoded value is used for testing purposes + //nolint:mnd // Hardcoded value is used for testing purposes netMask = net.CIDRMask(64, 128) } ipNet := &net.IPNet{ diff --git a/pkg/utils/number.go b/pkg/utils/number.go new file mode 100644 index 000000000..1365e317d --- /dev/null +++ b/pkg/utils/number.go @@ -0,0 +1,70 @@ +package utils + +import ( + "fmt" + "math" +) + +// File is used to handle numerical conversions safely and ensure that there are no overflows + +// IntToUInt64 converts an int to a uint16, returns an error if the int is negative or too large +func IntToUInt16(i int) (uint16, error) { + if i > math.MaxUint16 { + return 0, fmt.Errorf("value %d is too large to be converted to uint16", i) + } + + if i < 0 { + return 0, fmt.Errorf("value %d is negative, cannot be converted to uint16", i) + } + + return uint16(i), nil +} + +// IntToUInt64 converts an int to a uint64, returns an error if the int is negative or too large +func IntToUInt32(i int) (uint32, error) { + if i > math.MaxUint32 { + return 0, fmt.Errorf("value %d is too large to be converted to uint32", i) + } + + if i < 0 { + return 0, fmt.Errorf("value %d is negative, cannot be converted to uint32", i) + } + + return uint32(i), nil +} + +// Int32ToUInt32 converts an int32 to a uint32, returns an error if the int is negative +func Int32ToUInt32(i int32) (uint32, error) { + if i < 0 { + return 0, fmt.Errorf("value %d is negative, cannot be converted to uint32", i) + } + + return uint32(i), nil +} + +// UIntToInt converts a uint to an int, returns an error if the uint is too large +func UIntToInt(i uint) (int, error) { + if i > math.MaxInt { + return 0, fmt.Errorf("value %d is too large to be converted to int", i) + } + + return int(i), nil +} + +// UIntToUInt32 converts a uint to a uint32, returns an error if the uint is too large +func UIntToUInt32(i uint) (uint32, error) { + if i > math.MaxUint32 { + return 0, fmt.Errorf("value %d is too large to be converted to uint32", i) + } + + return uint32(i), nil +} + +// UInt32ToInt32 converts a uint32 to an int32, returns an error if the uint is too large +func UInt32ToInt32(i uint32) (int32, error) { + if i > math.MaxInt32 { + return 0, fmt.Errorf("value %d is too large to be converted to int32", i) + } + + return int32(i), nil +}