merge upstream 0.0.34

.gitignore

@@ -5,3 +5,10 @@
 /easy-rsa-master/
 /easy-rsa.tar.gz
 /easy-rsa
+
+# editor and IDE paraphernalia
+.idea
+.vscode
+
+# macOS paraphernalia
+.DS_Store

Makefile

@@ -24,7 +24,7 @@ endif
 GOOS ?= $(shell go env GOOS)
 GOARCH ?= $(shell go env GOARCH)
 INSTALL_LOCATION:=$(shell go env GOPATH)/bin
-GOLANGCI_LINT_VERSION ?= 1.45.2
+GOLANGCI_LINT_VERSION ?= 1.50.1
 GOSEC_VERSION ?= 2.13.1
 
 REGISTRY ?= gcr.io/$(shell gcloud config get-value project)
@@ -56,7 +56,8 @@ mock_gen:
 
 .PHONY: test
 test:
-	GO111MODULE=on go test -race sigs.k8s.io/apiserver-network-proxy/...
+	go test -race ./...
+	cd konnectivity-client && go test -race ./...
 
 ## --------------------------------------
 ## Binaries
@@ -90,7 +91,7 @@ bin/proxy-server-static: proto/agent/agent.pb.go konnectivity-client/proto/clien
 .PHONY: lint
 lint:
 	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(INSTALL_LOCATION) v$(GOLANGCI_LINT_VERSION)
-	$(INSTALL_LOCATION)/golangci-lint run --no-config --disable-all --enable=gofmt,golint,gosec,govet,unused --fix --verbose --timeout 3m
+	$(INSTALL_LOCATION)/golangci-lint run --no-config --disable-all --enable=gofmt,revive,gosec,govet,unused --fix --verbose --timeout 3m
 
 ## --------------------------------------
 ## Go

OWNERS

@@ -1,15 +1,15 @@
 # See the OWNERS docs at https://go.k8s.io/owners
 
 reviewers:
-  - caesarxuchao
   - dberkov
   - jefftree
-  - jkh52
+  - tallclair
 approvers:
-  - cheftako
-  - mcrute
   - anfernee
+  - cheftako
+  - jkh52
+emeritus_approvers:
   - andrewsykim
   - caesarxuchao
-emeritus_approvers:
+  - mcrute
   - Sh4d1

cmd/agent/app/options/options.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package options
 
 import (
@@ -30,10 +46,14 @@ type GrpcProxyAgentOptions struct {
 	ProxyServerPort int
 	AlpnProtos      []string
 
-	// Ports for the health and admin server
+	// Bind address for the health connections.
 	HealthServerHost string
+	// Port we listen for health connections on.
 	HealthServerPort int
-	AdminServerPort  int
+	// Bind address for the admin connections.
+	AdminBindAddress string
+	// Port we listen for admin connections on.
+	AdminServerPort int
 	// Enables pprof at host:adminPort/debug/pprof.
 	EnableProfiling bool
 	// If EnableProfiling is true, this enables the lock contention
@@ -59,9 +79,9 @@ type GrpcProxyAgentOptions struct {
 	// The check is an "unlocked" read but is still use at your own peril.
 	WarnOnChannelLimit bool
 
-	SyncForever bool
-	BindAddress        string
-	ApiServerMapping   portMapping
+	SyncForever      bool
+	BindAddress      string
+	APIServerMapping portMapping
 }
 
 var _ pflag.Value = &portMapping{}
@@ -108,6 +128,7 @@ func (o *GrpcProxyAgentOptions) Flags() *pflag.FlagSet {
 	flags.StringVar(&o.HealthServerHost, "health-server-host", o.HealthServerHost, "The host address to listen on, without port.")
 	flags.IntVar(&o.HealthServerPort, "health-server-port", o.HealthServerPort, "The port the health server is listening on.")
 	flags.IntVar(&o.AdminServerPort, "admin-server-port", o.AdminServerPort, "The port the admin server is listening on.")
+	flags.StringVar(&o.AdminBindAddress, "admin-bind-address", o.AdminBindAddress, "Bind address for admin connections. If empty, we will bind to all interfaces.")
 	flags.BoolVar(&o.EnableProfiling, "enable-profiling", o.EnableProfiling, "enable pprof at host:admin-port/debug/pprof")
 	flags.BoolVar(&o.EnableContentionProfiling, "enable-contention-profiling", o.EnableContentionProfiling, "enable contention profiling at host:admin-port/debug/pprof/block. \"--enable-profiling\" must also be set.")
 	flags.StringVar(&o.AgentID, "agent-id", o.AgentID, "The unique ID of this agent. Can also be set by the 'PROXY_AGENT_ID' environment variable. Default to a generated uuid if not set.")
@@ -118,7 +139,7 @@ func (o *GrpcProxyAgentOptions) Flags() *pflag.FlagSet {
 	flags.StringVar(&o.ServiceAccountTokenPath, "service-account-token-path", o.ServiceAccountTokenPath, "If non-empty proxy agent uses this token to prove its identity to the proxy server.")
 	flags.StringVar(&o.AgentIdentifiers, "agent-identifiers", o.AgentIdentifiers, "Identifiers of the agent that will be used by the server when choosing agent. N.B. the list of identifiers must be in URL encoded format. e.g.,host=localhost&host=node1.mydomain.com&cidr=127.0.0.1/16&ipv4=1.2.3.4&ipv4=5.6.7.8&ipv6=:::::&default-route=true")
 	flags.BoolVar(&o.WarnOnChannelLimit, "warn-on-channel-limit", o.WarnOnChannelLimit, "Turns on a warning if the system is going to push to a full channel. The check involves an unsafe read.")
-	flags.Var(&o.ApiServerMapping, "apiserver-port-mapping", "Mapping between a local port and the host:port used to reach the Kubernetes API Server")
+	flags.Var(&o.APIServerMapping, "apiserver-port-mapping", "Mapping between a local port and the host:port used to reach the Kubernetes API Server")
 	flags.StringVar(&o.BindAddress, "bind-address", o.BindAddress, "Address used to listen for traffic generated on cluster network")
 	// add feature gates flag
 	features.DefaultMutableFeatureGate.AddFlag(flags)
@@ -135,6 +156,7 @@ func (o *GrpcProxyAgentOptions) Print() {
 	klog.V(1).Infof("ALPNProtos set to %+s.\n", o.AlpnProtos)
 	klog.V(1).Infof("HealthServerHost set to %s\n", o.HealthServerHost)
 	klog.V(1).Infof("HealthServerPort set to %d.\n", o.HealthServerPort)
+	klog.V(1).Infof("Admin bind address set to %q.\n", o.AdminBindAddress)
 	klog.V(1).Infof("AdminServerPort set to %d.\n", o.AdminServerPort)
 	klog.V(1).Infof("EnableProfiling set to %v.\n", o.EnableProfiling)
 	klog.V(1).Infof("EnableContentionProfiling set to %v.\n", o.EnableContentionProfiling)
@@ -148,7 +170,7 @@ func (o *GrpcProxyAgentOptions) Print() {
 	klog.V(1).Infof("WarnOnChannelLimit set to %t.\n", o.WarnOnChannelLimit)
 	if features.DefaultMutableFeatureGate.Enabled(features.NodeToMasterTraffic) {
 		klog.V(1).Infof("AgentBindAddress set to %s.\n", o.BindAddress)
-		klog.V(1).Infof("Apiserver port mapping set to %s.\n", o.ApiServerMapping.String())
+		klog.V(1).Infof("Apiserver port mapping set to %s.\n", o.APIServerMapping.String())
 	}
 	klog.V(1).Infof("SyncForever set to %v.\n", o.SyncForever)
 }
@@ -201,20 +223,20 @@ func (o *GrpcProxyAgentOptions) Validate() error {
 	if err := validateHostnameOrIP(o.BindAddress); err != nil {
 		return fmt.Errorf("agent bind address is invalid: %v", err)
 	}
-	if err := validateHostnameOrIP(o.ApiServerMapping.RemoteHost); err != nil {
+	if err := validateHostnameOrIP(o.APIServerMapping.RemoteHost); err != nil {
 		return fmt.Errorf("apiserver address is invalid: %v", err)
 	}
-	if o.ApiServerMapping.LocalPort > 49151 {
-		return fmt.Errorf("please do not try to use ephemeral port %d for the apiserver local port", o.ApiServerMapping.LocalPort)
+	if o.APIServerMapping.LocalPort > 49151 {
+		return fmt.Errorf("please do not try to use ephemeral port %d for the apiserver local port", o.APIServerMapping.LocalPort)
 	}
-	if o.ApiServerMapping.LocalPort < 1024 {
-		return fmt.Errorf("please do not try to use reserved port %d for the apiserver local port", o.ApiServerMapping.LocalPort)
+	if o.APIServerMapping.LocalPort < 1024 {
+		return fmt.Errorf("please do not try to use reserved port %d for the apiserver local port", o.APIServerMapping.LocalPort)
 	}
-	if o.ApiServerMapping.RemotePort > 49151 {
-		return fmt.Errorf("please do not try to use ephemeral port %d for the apiserver remote port", o.ApiServerMapping.LocalPort)
+	if o.APIServerMapping.RemotePort > 49151 {
+		return fmt.Errorf("please do not try to use ephemeral port %d for the apiserver remote port", o.APIServerMapping.LocalPort)
 	}
-	if o.ApiServerMapping.RemotePort < 1 {
-		return fmt.Errorf("invalid port %d for the apiserver remote port", o.ApiServerMapping.RemotePort)
+	if o.APIServerMapping.RemotePort < 1 {
+		return fmt.Errorf("invalid port %d for the apiserver remote port", o.APIServerMapping.RemotePort)
 	}
 	return nil
 }
@@ -259,6 +281,7 @@ func NewGrpcProxyAgentOptions() *GrpcProxyAgentOptions {
 		ProxyServerPort:           8091,
 		HealthServerHost:          "",
 		HealthServerPort:          8093,
+		AdminBindAddress:          "127.0.0.1",
 		AdminServerPort:           8094,
 		EnableProfiling:           false,
 		EnableContentionProfiling: false,
@@ -271,7 +294,7 @@ func NewGrpcProxyAgentOptions() *GrpcProxyAgentOptions {
 		ServiceAccountTokenPath:   "",
 		WarnOnChannelLimit:        false,
 		SyncForever:               false,
-		ApiServerMapping:          portMapping{LocalPort: 6443, RemoteHost: "localhost", RemotePort: 6443},
+		APIServerMapping:          portMapping{LocalPort: 6443, RemoteHost: "localhost", RemotePort: 6443},
 		BindAddress:               "127.0.0.1",
 	}
 	return &o

cmd/agent/app/options/options_test.go

@@ -0,0 +1,172 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package options
+
+import (
+	"fmt"
+	"github.com/stretchr/testify/assert"
+	"reflect"
+	"testing"
+	"time"
+)
+
+/*
+ * TestDefaultServerOptions is intended to ensure we do not make a backward incompatible
+ * change to the default flag values for the ANP agent.
+ */
+func TestDefaultServerOptions(t *testing.T) {
+	defaultAgentOptions := NewGrpcProxyAgentOptions()
+	assertDefaultValue(t, "AgentCert", defaultAgentOptions.AgentCert, "")
+	assertDefaultValue(t, "AgentKey", defaultAgentOptions.AgentKey, "")
+	assertDefaultValue(t, "CaCert", defaultAgentOptions.CaCert, "")
+	assertDefaultValue(t, "ProxyServerHost", defaultAgentOptions.ProxyServerHost, "127.0.0.1")
+	assertDefaultValue(t, "ProxyServerPort", defaultAgentOptions.ProxyServerPort, 8091)
+	assertDefaultValue(t, "HealthServerHost", defaultAgentOptions.HealthServerHost, "")
+	assertDefaultValue(t, "HealthServerPort", defaultAgentOptions.HealthServerPort, 8093)
+	assertDefaultValue(t, "AdminBindAddress", defaultAgentOptions.AdminBindAddress, "127.0.0.1")
+	assertDefaultValue(t, "AdminServerPort", defaultAgentOptions.AdminServerPort, 8094)
+	assertDefaultValue(t, "EnableProfiling", defaultAgentOptions.EnableProfiling, false)
+	assertDefaultValue(t, "EnableContentionProfiling", defaultAgentOptions.EnableContentionProfiling, false)
+	assertDefaultValue(t, "AgentIdentifiers", defaultAgentOptions.AgentIdentifiers, "")
+	assertDefaultValue(t, "SyncInterval", defaultAgentOptions.SyncInterval, 1*time.Second)
+	assertDefaultValue(t, "ProbeInterval", defaultAgentOptions.ProbeInterval, 1*time.Second)
+	assertDefaultValue(t, "SyncIntervalCap", defaultAgentOptions.SyncIntervalCap, 10*time.Second)
+	assertDefaultValue(t, "KeepaliveTime", defaultAgentOptions.KeepaliveTime, 1*time.Hour)
+	assertDefaultValue(t, "ServiceAccountTokenPath", defaultAgentOptions.ServiceAccountTokenPath, "")
+	assertDefaultValue(t, "WarnOnChannelLimit", defaultAgentOptions.WarnOnChannelLimit, false)
+	assertDefaultValue(t, "SyncForever", defaultAgentOptions.SyncForever, false)
+}
+
+func assertDefaultValue(t *testing.T, fieldName string, actual, expected interface{}) {
+	t.Helper()
+	assert.IsType(t, expected, actual, "For field %s, got the wrong type.", fieldName)
+	assert.Equal(t, expected, actual, "For field %s, got the wrong value.", fieldName)
+}
+
+func TestValidate(t *testing.T) {
+	for desc, tc := range map[string]struct {
+		fieldMap map[string]interface{}
+		expected error
+	}{
+		"default": {
+			fieldMap: map[string]interface{}{},
+			expected: nil,
+		},
+		"ZeroProxyServerPort": {
+			fieldMap: map[string]interface{}{"ProxyServerPort": 0},
+			expected: fmt.Errorf("proxy server port 0 must be greater than 0"),
+		},
+		"NegativeProxyServerPort": {
+			fieldMap: map[string]interface{}{"ProxyServerPort": -1},
+			expected: fmt.Errorf("proxy server port -1 must be greater than 0"),
+		},
+		"ReservedProxyServerPort": {
+			fieldMap: map[string]interface{}{"ProxyServerPort": 1023},
+			expected: nil, //TODO: fmt.Errorf("please do not try to use reserved port 1023 for the proxy server port"),
+		},
+		"StartValidProxyServerPort": {
+			fieldMap: map[string]interface{}{"ProxyServerPort": 1024},
+			expected: nil,
+		},
+		"EndValidProxyServerPort": {
+			fieldMap: map[string]interface{}{"ProxyServerPort": 49151},
+			expected: nil,
+		},
+		"StartEphemeralProxyServerPort": {
+			fieldMap: map[string]interface{}{"ProxyServerPort": 49152},
+			expected: nil, //TODO: fmt.Errorf("please do not try to use ephemeral port 49152 for the proxy server port"),
+		},
+		"ZeroHealthServerPort": {
+			fieldMap: map[string]interface{}{"HealthServerPort": 0},
+			expected: fmt.Errorf("health server port 0 must be greater than 0"),
+		},
+		"NegativeHealthServerPort": {
+			fieldMap: map[string]interface{}{"HealthServerPort": -1},
+			expected: fmt.Errorf("health server port -1 must be greater than 0"),
+		},
+		"ReservedHealthServerPort": {
+			fieldMap: map[string]interface{}{"HealthServerPort": 1023},
+			expected: nil, //TODO: fmt.Errorf("please do not try to use reserved port 1023 for the health server port"),
+		},
+		"StartValidHealthServerPort": {
+			fieldMap: map[string]interface{}{"HealthServerPort": 1024},
+			expected: nil,
+		},
+		"EndValidHealthServerPort": {
+			fieldMap: map[string]interface{}{"HealthServerPort": 49151},
+			expected: nil,
+		},
+		"StartEphemeralHealthServerPort": {
+			fieldMap: map[string]interface{}{"HealthServerPort": 49152},
+			expected: nil, //TODO: fmt.Errorf("please do not try to use ephemeral port 49152 for the health server port"),
+		},
+		"ZeroAdminServerPort": {
+			fieldMap: map[string]interface{}{"AdminServerPort": 0},
+			expected: fmt.Errorf("admin server port 0 must be greater than 0"),
+		},
+		"NegativeAdminServerPort": {
+			fieldMap: map[string]interface{}{"AdminServerPort": -1},
+			expected: fmt.Errorf("admin server port -1 must be greater than 0"),
+		},
+		"ReservedAdminServerPort": {
+			fieldMap: map[string]interface{}{"AdminServerPort": 1023},
+			expected: nil, //TODO: fmt.Errorf("please do not try to use reserved port 1023 for the health port"),
+		},
+		"StartValidAdminServerPort": {
+			fieldMap: map[string]interface{}{"AdminServerPort": 1024},
+			expected: nil,
+		},
+		"EndValidAdminServerPort": {
+			fieldMap: map[string]interface{}{"AdminServerPort": 49151},
+			expected: nil,
+		},
+		"StartEphemeralAdminServerPort": {
+			fieldMap: map[string]interface{}{"AdminServerPort": 49152},
+			expected: nil, //TODO: fmt.Errorf("please do not try to use ephemeral port 49152 for the health port"),
+		},
+		"ContentionProfilingRequiresProfiling": {
+			fieldMap: map[string]interface{}{
+				"EnableContentionProfiling": true,
+				"EnableProfiling":           false,
+			},
+			expected: fmt.Errorf("if --enable-contention-profiling is set, --enable-profiling must also be set"),
+		},
+	} {
+		t.Run(desc, func(t *testing.T) {
+			testAgentOptions := NewGrpcProxyAgentOptions()
+			for field, value := range tc.fieldMap {
+				rv := reflect.ValueOf(testAgentOptions)
+				rv = rv.Elem()
+				fv := rv.FieldByName(field)
+				switch reflect.TypeOf(value).Kind() {
+				case reflect.String:
+					svalue := value.(string)
+					fv.SetString(svalue)
+				case reflect.Int:
+					ivalue := value.(int)
+					fv.SetInt(int64(ivalue))
+				case reflect.Bool:
+					bvalue := value.(bool)
+					fv.SetBool(bvalue)
+				}
+			}
+			actual := testAgentOptions.Validate()
+			assert.IsType(t, tc.expected, actual, "Validation for case %s, got the wrong type.", desc)
+			assert.Equal(t, tc.expected, actual, "Validation for case %s, got the wrong value.", desc)
+		})
+	}
+}