Merge pull request #710 from openshift-cherrypick-robot/cherry-pick-6…

…53-to-release-4.3

Bug 1789421: [release-4.3] If the cluster is single-stack IPv6, bind to :: rather than 0.0.0.0.

bindata/bootkube/config/bootstrap-config-overrides.yaml

@@ -14,6 +14,8 @@ kubeletClientInfo:
 serviceAccountPublicKeyFiles:
 - /etc/kubernetes/secrets/service-account.pub
 servingInfo:
+  bindAddress: "{{.BindAddress}}"
+  bindNetwork: {{.BindNetwork}}
   certFile: /etc/kubernetes/secrets/kube-apiserver-service-network-server.crt
   clientCA: /etc/kubernetes/secrets/kube-apiserver-complete-client-ca-bundle.crt
   keyFile: /etc/kubernetes/secrets/kube-apiserver-service-network-server.key

pkg/cmd/render/render.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"net"
 	"path/filepath"
 
 	"github.com/ghodss/yaml"
@@ -130,6 +131,12 @@ type TemplateData struct {
 
 	// ServiceClusterIPRange is the IP range for service IPs.
 	ServiceCIDR []string
+
+	// BindAddress is the IP address and port to bind to
+	BindAddress string
+
+	// BindNetwork is the network (tcp4 or tcp6) to bind to
+	BindNetwork string
 }
 
 // Run contains the logic of the render command.
@@ -138,6 +145,8 @@ func (r *renderOpts) Run() error {
 		LockHostPath:   r.lockHostPath,
 		EtcdServerURLs: r.etcdServerURLs,
 		EtcdServingCA:  r.etcdServingCA,
+		BindAddress:    "0.0.0.0:6443",
+		BindNetwork:    "tcp4",
 	}
 	if len(r.clusterConfigFile) > 0 {
 		clusterConfigFileData, err := ioutil.ReadFile(r.clusterConfigFile)
@@ -148,6 +157,24 @@ func (r *renderOpts) Run() error {
 			return fmt.Errorf("unable to parse restricted CIDRs from config %q: %v", r.clusterConfigFile, err)
 		}
 	}
+	if len(renderConfig.ClusterCIDR) > 0 {
+		anyIPv4 := false
+		for _, cidr := range renderConfig.ClusterCIDR {
+			cidrBaseIP, _, err := net.ParseCIDR(cidr)
+			if err != nil {
+				return fmt.Errorf("invalid cluster CIDR %q: %v", cidr, err)
+			}
+			if cidrBaseIP.To4() != nil {
+				anyIPv4 = true
+				break
+			}
+		}
+		if !anyIPv4 {
+			// Single-stack IPv6 cluster, so listen on IPv6 not IPv4.
+			renderConfig.BindAddress = "[::]:6443"
+			renderConfig.BindNetwork = "tcp6"
+		}
+	}
 	if err := r.manifest.ApplyTo(&renderConfig.ManifestConfig); err != nil {
 		return err
 	}

pkg/cmd/render/render_test.go

@@ -52,6 +52,38 @@ spec:
   serviceNetwork:
     - 172.30.0.0/16
 status: {}
+`
+	networkConfigV6 = `
+apiVersion: config.openshift.io/v1
+kind: Network
+metadata:
+  creationTimestamp: null
+  name: cluster
+spec:
+  clusterNetwork:
+    - cidr: fd01::/48
+      hostPrefix: 64
+  networkType: OpenShiftSDN
+  serviceNetwork:
+    - fd02::/112
+status: {}
+`
+	networkConfigDual = `
+apiVersion: config.openshift.io/v1
+kind: Network
+metadata:
+  creationTimestamp: null
+  name: cluster
+spec:
+  clusterNetwork:
+    - cidr: fd01::/48
+      hostPrefix: 64
+    - cidr: 10.128.0.0/14
+      hostPrefix: 23
+  networkType: OpenShiftSDN
+  serviceNetwork:
+    - 172.30.0.0/16
+status: {}
 `
 )
 
@@ -130,9 +162,10 @@ func TestRenderCommand(t *testing.T) {
 
 	tests := []struct {
 		// note the name is used as a name for a temporary directory
-		name         string
-		args         []string
-		testFunction func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error
+		name          string
+		args          []string
+		setupFunction func() error
+		testFunction  func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error
 	}{
 		{
 			name: "scenario 1 checks feature gates",
@@ -177,6 +210,50 @@ func TestRenderCommand(t *testing.T) {
 				return nil
 			},
 		},
+		{
+			name: "scenario 2 checks BindAddress under IPv6",
+			args: []string{
+				"--asset-input-dir=" + assetsInputDir,
+				"--templates-input-dir=" + templateDir,
+				"--cluster-config-file=" + filepath.Join(assetsInputDir, "config-v6.yaml"),
+				"--asset-output-dir=",
+				"--config-output-file=",
+			},
+			setupFunction: func() error {
+				return ioutil.WriteFile(filepath.Join(assetsInputDir, "config-v6.yaml"), []byte(networkConfigV6), 0644)
+			},
+			testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
+				if cfg.ServingInfo.BindAddress != "[::]:6443" {
+					return fmt.Errorf("incorrect IPv6 BindAddress: %s", cfg.ServingInfo.BindAddress)
+				}
+				if cfg.ServingInfo.BindNetwork != "tcp6" {
+					return fmt.Errorf("incorrect IPv6 BindNetwork: %s", cfg.ServingInfo.BindNetwork)
+				}
+				return nil
+			},
+		},
+		{
+			name: "scenario 3 checks BindAddress under dual IPv4-IPv6",
+			args: []string{
+				"--asset-input-dir=" + assetsInputDir,
+				"--templates-input-dir=" + templateDir,
+				"--cluster-config-file=" + filepath.Join(assetsInputDir, "config-v4.yaml"),
+				"--asset-output-dir=",
+				"--config-output-file=",
+			},
+			setupFunction: func() error {
+				return ioutil.WriteFile(filepath.Join(assetsInputDir, "config-v4.yaml"), []byte(networkConfigDual), 0644)
+			},
+			testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
+				if cfg.ServingInfo.BindAddress != "0.0.0.0:6443" {
+					return fmt.Errorf("incorrect IPv4 BindAddress: %s", cfg.ServingInfo.BindAddress)
+				}
+				if cfg.ServingInfo.BindNetwork != "tcp4" {
+					return fmt.Errorf("incorrect IPv4 BindNetwork: %s", cfg.ServingInfo.BindNetwork)
+				}
+				return nil
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -187,6 +264,12 @@ func TestRenderCommand(t *testing.T) {
 		}
 		defer teardown()
 
+		if test.setupFunction != nil {
+			if err := test.setupFunction(); err != nil {
+				t.Fatalf("%q failed to set up, error: %v", test.name, err)
+			}
+		}
+
 		test.args = setOutputFlags(test.args, outputDir)
 		err = runRender(test.args...)
 		if err != nil {

pkg/operator/configobservation/network/observe_network.go

@@ -1,6 +1,8 @@
 package network
 
 import (
+	"net"
+
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	"github.com/openshift/library-go/pkg/operator/configobserver"
@@ -73,30 +75,42 @@ func ObserveRestrictedCIDRs(genericListers configobserver.Listers, recorder even
 }
 
 // ObserveServicesSubnet watches the network configuration and generates the
-// servicesSubnet
+// servicesSubnet (and bindAddress)
 func ObserveServicesSubnet(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) {
 	listers := genericListers.(configobservation.Listers)
 
 	out := map[string]interface{}{}
-	configPath := []string{"servicesSubnet"}
+	servicesSubnetConfigPath := []string{"servicesSubnet"}
+	bindAddressConfigPath := []string{"servingInfo", "bindAddress"}
+	bindNetworkConfigPath := []string{"servingInfo", "bindNetwork"}
 
-	prev, ok, err := unstructured.NestedString(existingConfig, configPath...)
-	if err != nil {
-		return out, []error{err}
-	}
-	if ok {
-		if err := unstructured.SetNestedField(out, prev, configPath...); err != nil {
-			return out, []error{err}
-		}
-	}
+	previouslyObservedConfig, errs := extractPreviouslyObservedConfig(existingConfig, servicesSubnetConfigPath, bindAddressConfigPath, bindNetworkConfigPath)
 
-	errs := []error{}
 	serviceCIDR, err := network.GetServiceCIDR(listers.NetworkLister, recorder)
 	if err != nil {
 		errs = append(errs, err)
+		return previouslyObservedConfig, errs
 	}
 
-	if err := unstructured.SetNestedField(out, serviceCIDR, configPath...); err != nil {
+	if err := unstructured.SetNestedField(out, serviceCIDR, servicesSubnetConfigPath...); err != nil {
+		errs = append(errs, err)
+	}
+	bindAddress := "0.0.0.0:6443"
+	bindNetwork := "tcp4"
+	// TODO: only do this in the single-stack IPv6 case once network.GetServiceCIDR()
+	// supports dual-stack
+	if serviceCIDR != "" {
+		if ip, _, err := net.ParseCIDR(serviceCIDR); err != nil {
+			errs = append(errs, err)
+		} else if ip.To4() == nil {
+			bindAddress = "[::]:6443"
+			bindNetwork = "tcp6"
+		}
+	}
+	if err := unstructured.SetNestedField(out, bindAddress, bindAddressConfigPath...); err != nil {
+		errs = append(errs, err)
+	}
+	if err := unstructured.SetNestedField(out, bindNetwork, bindNetworkConfigPath...); err != nil {
 		errs = append(errs, err)
 	}
 
@@ -123,7 +137,8 @@ func ObserveExternalIPPolicy(genericListers configobserver.Listers, recorder eve
 	// 2. Null externalip policy: enable the externalip admission controller with deny-all
 	// 3. Non-null policy: enable the externalip admission controller, pass through configuration.
 
-	errs := []error{}
+	previouslyObservedConfig, errs := extractPreviouslyObservedConfig(existingConfig, configPath)
+
 	externalIPPolicy, err := network.GetExternalIPPolicy(listers.NetworkLister, recorder)
 	if err != nil {
 		errs = append(errs, err)
@@ -138,14 +153,6 @@ func ObserveExternalIPPolicy(genericListers configobserver.Listers, recorder eve
 	// Case 1: retrieval error. Pass through our existing configuration path,
 	// if it exists.
 	if len(errs) > 0 {
-		previouslyObservedConfig := map[string]interface{}{}
-
-		// an error here is very unlikely
-		prevConfig, ok, err2 := unstructured.NestedMap(existingConfig, configPath...)
-		if ok && err2 != nil {
-			unstructured.SetNestedMap(previouslyObservedConfig, prevConfig, configPath...)
-		}
-
 		return previouslyObservedConfig, errs
 	}
 
@@ -182,3 +189,23 @@ func ObserveExternalIPPolicy(genericListers configobserver.Listers, recorder eve
 
 	return observedConfig, errs
 }
+
+// extractPreviouslyObservedConfig extracts the previously observed config from the existing config.
+func extractPreviouslyObservedConfig(existing map[string]interface{}, paths ...[]string) (map[string]interface{}, []error) {
+	var errs []error
+	previous := map[string]interface{}{}
+	for _, fields := range paths {
+		value, found, err := unstructured.NestedFieldCopy(existing, fields...)
+		if !found {
+			continue
+		}
+		if err != nil {
+			errs = append(errs, err)
+		}
+		err = unstructured.SetNestedField(previous, value, fields...)
+		if err != nil {
+			errs = append(errs, err)
+		}
+	}
+	return previous, errs
+}

pkg/operator/configobservation/network/observe_network_test.go

@@ -126,7 +126,7 @@ func TestObserveServicesSubnet(t *testing.T) {
 	// With no network configured, check that a rump configuration is returned
 	result, errors := ObserveServicesSubnet(listers, events.NewInMemoryRecorder("network"), map[string]interface{}{})
 	if len(errors) > 0 {
-		t.Error("expected len(errors) == 0")
+		t.Errorf("expected len(errors) == 0: %v", errors)
 	}
 	if result == nil {
 		t.Errorf("expected result != nil")
@@ -144,39 +144,73 @@ func TestObserveServicesSubnet(t *testing.T) {
 	if err := indexer.Add(&configv1.Network{
 		ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
 		Status: configv1.NetworkStatus{
-			ClusterNetwork: []configv1.ClusterNetworkEntry{{CIDR: "podCIDR"}},
-			ServiceNetwork: []string{"serviceCIDR"},
+			ClusterNetwork: []configv1.ClusterNetworkEntry{{CIDR: "10.128.0.0/14"}},
+			ServiceNetwork: []string{"172.30.0.0/16"},
 		},
 	}); err != nil {
 		t.Fatal(err.Error())
 	}
 
 	result, errors = ObserveServicesSubnet(listers, events.NewInMemoryRecorder("network"), map[string]interface{}{})
+	if len(errors) > 0 {
+		t.Errorf("expected len(errors) == 0: %v", errors)
+	}
 	conf, ok, err = unstructured.NestedString(result, "servicesSubnet")
 	if err != nil || !ok {
 		t.Errorf("Unexpected configuration returned: %v", result)
 	}
-	if conf != "serviceCIDR" {
+	if conf != "172.30.0.0/16" {
+		t.Errorf("Unexpected value: %v", conf)
+	}
+	conf, ok, err = unstructured.NestedString(result, "servingInfo", "bindAddress")
+	if err != nil || !ok {
+		t.Errorf("Unexpected configuration returned: %v", result)
+	}
+	if conf != "0.0.0.0:6443" {
+		t.Errorf("Unexpected value: %v", conf)
+	}
+	conf, ok, err = unstructured.NestedString(result, "servingInfo", "bindNetwork")
+	if err != nil || !ok {
+		t.Errorf("Unexpected configuration returned: %v", result)
+	}
+	if conf != "tcp4" {
 		t.Errorf("Unexpected value: %v", conf)
 	}
 
 	// Change the config and see that it is updated.
 	if err := indexer.Update(&configv1.Network{
 		ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
 		Status: configv1.NetworkStatus{
-			ClusterNetwork: []configv1.ClusterNetworkEntry{{CIDR: "podCIDR1"}},
-			ServiceNetwork: []string{"serviceCIDR1"},
+			ClusterNetwork: []configv1.ClusterNetworkEntry{{CIDR: "10.128.0.0/14"}},
+			ServiceNetwork: []string{"fd02::/112"},
 		},
 	}); err != nil {
 		t.Fatal(err.Error())
 	}
 
 	result, errors = ObserveServicesSubnet(listers, events.NewInMemoryRecorder("network"), result)
+	if len(errors) > 0 {
+		t.Errorf("expected len(errors) == 0: %v", errors)
+	}
 	conf, ok, err = unstructured.NestedString(result, "servicesSubnet")
 	if err != nil || !ok {
 		t.Errorf("Unexpected configuration returned: %v", result)
 	}
-	if conf != "serviceCIDR1" {
+	if conf != "fd02::/112" {
+		t.Errorf("Unexpected value: %v", conf)
+	}
+	conf, ok, err = unstructured.NestedString(result, "servingInfo", "bindAddress")
+	if err != nil || !ok {
+		t.Errorf("Unexpected configuration returned: %v", result)
+	}
+	if conf != "[::]:6443" {
+		t.Errorf("Unexpected value: %v", conf)
+	}
+	conf, ok, err = unstructured.NestedString(result, "servingInfo", "bindNetwork")
+	if err != nil || !ok {
+		t.Errorf("Unexpected configuration returned: %v", result)
+	}
+	if conf != "tcp6" {
 		t.Errorf("Unexpected value: %v", conf)
 	}
 }