From 62e58a66aa71bfbdfd6721b2225a29d83869cd88 Mon Sep 17 00:00:00 2001 From: Brad Hoekstra Date: Thu, 9 May 2019 11:34:56 -0400 Subject: [PATCH] Fix some lint errors in pkg/proxy --- cmd/kube-proxy/app/server_others.go | 6 ++--- hack/.golint_failures | 2 -- pkg/proxy/iptables/proxier.go | 34 ++++++++++++++++++++++------- pkg/proxy/util/endpoints.go | 8 +++---- pkg/proxy/util/utils.go | 13 ++++++++++- 5 files changed, 45 insertions(+), 18 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index 1182c001668..1b31497db02 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -237,7 +237,7 @@ func newProxyServer( }, nil } -func getProxyMode(proxyMode string, iptver iptables.IPTablesVersioner, khandle ipvs.KernelHandler, ipsetver ipvs.IPSetVersioner, kcompat iptables.KernelCompatTester) string { +func getProxyMode(proxyMode string, iptver iptables.Versioner, khandle ipvs.KernelHandler, ipsetver ipvs.IPSetVersioner, kcompat iptables.KernelCompatTester) string { switch proxyMode { case proxyModeUserspace: return proxyModeUserspace @@ -250,7 +250,7 @@ func getProxyMode(proxyMode string, iptver iptables.IPTablesVersioner, khandle i return tryIPTablesProxy(iptver, kcompat) } -func tryIPVSProxy(iptver iptables.IPTablesVersioner, khandle ipvs.KernelHandler, ipsetver ipvs.IPSetVersioner, kcompat iptables.KernelCompatTester) string { +func tryIPVSProxy(iptver iptables.Versioner, khandle ipvs.KernelHandler, ipsetver ipvs.IPSetVersioner, kcompat iptables.KernelCompatTester) string { // guaranteed false on error, error only necessary for debugging // IPVS Proxier relies on ip_vs_* kernel modules and ipset useIPVSProxy, err := ipvs.CanUseIPVSProxier(khandle, ipsetver) @@ -267,7 +267,7 @@ func tryIPVSProxy(iptver iptables.IPTablesVersioner, khandle ipvs.KernelHandler, return tryIPTablesProxy(iptver, kcompat) } -func tryIPTablesProxy(iptver iptables.IPTablesVersioner, kcompat iptables.KernelCompatTester) string { +func tryIPTablesProxy(iptver iptables.Versioner, kcompat iptables.KernelCompatTester) string { // guaranteed false on error, error only necessary for debugging useIPTablesProxy, err := iptables.CanUseIPTablesProxier(iptver, kcompat) if err != nil { diff --git a/hack/.golint_failures b/hack/.golint_failures index 9ef59605506..75ce617ee0a 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -194,9 +194,7 @@ pkg/master/tunneler pkg/proxy pkg/proxy/apis/config pkg/proxy/apis/config/v1alpha1 -pkg/proxy/iptables pkg/proxy/userspace -pkg/proxy/util pkg/proxy/winkernel pkg/proxy/winuserspace pkg/quota/v1/evaluator/core diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 5911f0e623e..4dc84e2866e 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -73,18 +73,18 @@ const ( // the kubernetes postrouting chain kubePostroutingChain utiliptables.Chain = "KUBE-POSTROUTING" - // the mark-for-masquerade chain + // KubeMarkMasqChain is the mark-for-masquerade chain KubeMarkMasqChain utiliptables.Chain = "KUBE-MARK-MASQ" - // the mark-for-drop chain + // KubeMarkDropChain is the mark-for-drop chain KubeMarkDropChain utiliptables.Chain = "KUBE-MARK-DROP" // the kubernetes forward chain kubeForwardChain utiliptables.Chain = "KUBE-FORWARD" ) -// IPTablesVersioner can query the current iptables version. -type IPTablesVersioner interface { +// Versioner can query the current iptables version. +type Versioner interface { // returns "X.Y.Z" GetVersion() (string, error) } @@ -100,7 +100,7 @@ type KernelCompatTester interface { // the iptables version and for the existence of kernel features. It may return // an error if it fails to get the iptables version without error, in which // case it will also return false. -func CanUseIPTablesProxier(iptver IPTablesVersioner, kcompat KernelCompatTester) (bool, error) { +func CanUseIPTablesProxier(iptver Versioner, kcompat KernelCompatTester) (bool, error) { minVersion, err := utilversion.ParseGeneric(iptablesMinVersion) if err != nil { return false, err @@ -124,12 +124,14 @@ func CanUseIPTablesProxier(iptver IPTablesVersioner, kcompat KernelCompatTester) return true, nil } +// LinuxKernelCompatTester is the Linux implementation of KernelCompatTester type LinuxKernelCompatTester struct{} +// IsCompatible checks for the required sysctls. We don't care about the value, just +// that it exists. If this Proxier is chosen, we'll initialize it as we +// need. func (lkct LinuxKernelCompatTester) IsCompatible() error { - // Check for the required sysctls. We don't care about the value, just - // that it exists. If this Proxier is chosen, we'll initialize it as we - // need. + _, err := utilsysctl.New().GetSysctl(sysctlRouteLocalnet) return err } @@ -507,21 +509,29 @@ func (proxier *Proxier) isInitialized() bool { return atomic.LoadInt32(&proxier.initialized) > 0 } +// OnServiceAdd is called whenever creation of new service object +// is observed. func (proxier *Proxier) OnServiceAdd(service *v1.Service) { proxier.OnServiceUpdate(nil, service) } +// OnServiceUpdate is called whenever modification of an existing +// service object is observed. func (proxier *Proxier) OnServiceUpdate(oldService, service *v1.Service) { if proxier.serviceChanges.Update(oldService, service) && proxier.isInitialized() { proxier.syncRunner.Run() } } +// OnServiceDelete is called whenever deletion of an existing service +// object is observed. func (proxier *Proxier) OnServiceDelete(service *v1.Service) { proxier.OnServiceUpdate(service, nil) } +// OnServiceSynced is called once all the initial even handlers were +// called and the state is fully propagated to local cache. func (proxier *Proxier) OnServiceSynced() { proxier.mu.Lock() proxier.servicesSynced = true @@ -532,20 +542,28 @@ func (proxier *Proxier) OnServiceSynced() { proxier.syncProxyRules() } +// OnEndpointsAdd is called whenever creation of new endpoints object +// is observed. func (proxier *Proxier) OnEndpointsAdd(endpoints *v1.Endpoints) { proxier.OnEndpointsUpdate(nil, endpoints) } +// OnEndpointsUpdate is called whenever modification of an existing +// endpoints object is observed. func (proxier *Proxier) OnEndpointsUpdate(oldEndpoints, endpoints *v1.Endpoints) { if proxier.endpointsChanges.Update(oldEndpoints, endpoints) && proxier.isInitialized() { proxier.syncRunner.Run() } } +// OnEndpointsDelete is called whever deletion of an existing endpoints +// object is observed. func (proxier *Proxier) OnEndpointsDelete(endpoints *v1.Endpoints) { proxier.OnEndpointsUpdate(endpoints, nil) } +// OnEndpointsSynced is called once all the initial event handlers were +// called and the state is fully propagated to local cache. func (proxier *Proxier) OnEndpointsSynced() { proxier.mu.Lock() proxier.endpointsSynced = true diff --git a/pkg/proxy/util/endpoints.go b/pkg/proxy/util/endpoints.go index 716491cd25c..0a65f43955e 100644 --- a/pkg/proxy/util/endpoints.go +++ b/pkg/proxy/util/endpoints.go @@ -39,12 +39,12 @@ func IPPart(s string) string { return "" } // Check if host string is a valid IP address - if ip := net.ParseIP(host); ip != nil { - return ip.String() - } else { + ip := net.ParseIP(host) + if ip == nil { klog.Errorf("invalid IP part '%s'", host) + return "" } - return "" + return ip.String() } // PortPart returns just the port part of an endpoint string. diff --git a/pkg/proxy/util/utils.go b/pkg/proxy/util/utils.go index 822da8534d3..d390c7f5b0a 100644 --- a/pkg/proxy/util/utils.go +++ b/pkg/proxy/util/utils.go @@ -33,15 +33,23 @@ import ( ) const ( + // IPv4ZeroCIDR is the CIDR block for the whole IPv4 address space IPv4ZeroCIDR = "0.0.0.0/0" + + // IPv6ZeroCIDR is the CIDR block for the whole IPv6 address space IPv6ZeroCIDR = "::/0" ) var ( + // ErrAddressNotAllowed indicates the address is not allowed ErrAddressNotAllowed = errors.New("address not allowed") - ErrNoAddresses = errors.New("No addresses for hostname") + + // ErrNoAddresses indicates there are no addresses for the hostname + ErrNoAddresses = errors.New("No addresses for hostname") ) +// IsZeroCIDR checks whether the input CIDR string is either +// the IPv4 or IPv6 zero CIDR func IsZeroCIDR(cidr string) bool { if cidr == IPv4ZeroCIDR || cidr == IPv6ZeroCIDR { return true @@ -89,6 +97,8 @@ func IsProxyableHostname(ctx context.Context, resolv Resolver, hostname string) return nil } +// IsLocalIP checks if a given IP address is bound to an interface +// on the local system func IsLocalIP(ip string) (bool, error) { addrs, err := net.InterfaceAddrs() if err != nil { @@ -106,6 +116,7 @@ func IsLocalIP(ip string) (bool, error) { return false, nil } +// ShouldSkipService checks if a given service should skip proxying func ShouldSkipService(svcName types.NamespacedName, service *v1.Service) bool { // if ClusterIP is "None" or empty, skip proxying if !helper.IsServiceIPSet(service) {