Further address review comments

tigera · Sep 15, 2023 · 5f8441f · 5f8441f
1 parent c8e145e
commit 5f8441f
Show file tree

Hide file tree

Showing 8 changed files with 136 additions and 237 deletions.
diff --git a/pkg/common/windows.go b/pkg/common/windows.go
@@ -21,6 +21,8 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	operatorv1 "github.com/tigera/operator/api/v1"
 )
 
 const (
@@ -36,3 +38,10 @@ func HasWindowsNodes(c client.Client) (bool, error) {
 
 	return len(nodes.Items) > 0, nil
 }
+
+// WindowsEnabled returns true if the given Installation enables Windows, false otherwise.
+func WindowsEnabled(installation operatorv1.InstallationSpec) bool {
+	return installation.CalicoNetwork != nil &&
+		installation.CalicoNetwork.WindowsDataplane != nil &&
+		*installation.CalicoNetwork.WindowsDataplane != operatorv1.WindowsDataplaneDisabled
+}
diff --git a/pkg/controller/installation/core_controller.go b/pkg/controller/installation/core_controller.go
@@ -1549,15 +1549,23 @@ func calicoDirectoryExists() bool {
 	return err == nil
 }
 
-// GetOrCreateTyphaNodeTLSConfig reads and validates the CA ConfigMap and Secrets for
+func GetOrCreateTyphaNodeTLSConfig(cli client.Client, certificateManager certificatemanager.CertificateManager) (*render.TyphaNodeTLS, error) {
+	return getOrCreateTyphaNodeTLSConfig(cli, certificateManager, certificateManager.GetOrCreateKeyPair)
+}
+
+func GetTyphaNodeTLSConfig(cli client.Client, certificateManager certificatemanager.CertificateManager) (*render.TyphaNodeTLS, error) {
+	return getOrCreateTyphaNodeTLSConfig(cli, certificateManager, certificateManager.GetKeyPair)
+}
+
+// getOrCreateTyphaNodeTLSConfig reads and validates the CA ConfigMap and Secrets for
 // Typha and Felix configuration. It returns the validated resources or error
 // if there was one.
-func GetOrCreateTyphaNodeTLSConfig(cli client.Client, certificateManager certificatemanager.CertificateManager) (*render.TyphaNodeTLS, error) {
+func getOrCreateTyphaNodeTLSConfig(cli client.Client, certificateManager certificatemanager.CertificateManager, createKeyPairFunc func(cli client.Client, secretName, secretNamespace string, dnsNames []string) (certificatemanagement.KeyPairInterface, error)) (*render.TyphaNodeTLS, error) {
 	// accumulate all the error messages so all problems with the certs
 	// and CA are reported.
 	var errMsgs []string
-	getKeyPair := func(secretName, commonName string) (keyPair certificatemanagement.KeyPairInterface, cn string, uriSAN string) {
-		keyPair, err := certificateManager.GetOrCreateKeyPair(cli, secretName, common.OperatorNamespace(), []string{commonName})
+	getOrCreateKeyPair := func(secretName, commonName string) (keyPair certificatemanagement.KeyPairInterface, cn string, uriSAN string) {
+		keyPair, err := createKeyPairFunc(cli, secretName, common.OperatorNamespace(), []string{commonName})
 		if err != nil {
 			errMsgs = append(errMsgs, err.Error())
 		} else {
@@ -1582,8 +1590,8 @@ func GetOrCreateTyphaNodeTLSConfig(cli client.Client, certificateManager certifi
 		}
 		return
 	}
-	node, nodeCommonName, nodeURISAN := getKeyPair(render.NodeTLSSecretName, render.FelixCommonName)
-	typha, typhaCommonName, typhaURISAN := getKeyPair(render.TyphaTLSSecretName, render.TyphaCommonName)
+	node, nodeCommonName, nodeURISAN := getOrCreateKeyPair(render.NodeTLSSecretName, render.FelixCommonName)
+	typha, typhaCommonName, typhaURISAN := getOrCreateKeyPair(render.TyphaTLSSecretName, render.TyphaCommonName)
 	var trustedBundle certificatemanagement.TrustedBundle
 	configMap, err := getConfigMap(cli, render.TyphaCAConfigMapName)
 	if err != nil {

diff --git a/pkg/controller/installation/defaults_test.go b/pkg/controller/installation/defaults_test.go
@@ -533,6 +533,7 @@ var _ = Describe("Defaulting logic tests", func() {
 			winDataplane := operator.WindowsDataplaneDisabled
 			if provider == operator.ProviderAKS {
 				winDataplane = operator.WindowsDataplaneHNS
+				instance.Spec.ServiceCIDRs = []string{"10.96.0.0/12"}
 			}
 			bgpDisabled := operator.BGPDisabled
 			Expect(instance.Spec.CalicoNetwork).To(Equal(&operator.CalicoNetworkSpec{

diff --git a/pkg/controller/installation/validation.go b/pkg/controller/installation/validation.go
@@ -21,6 +21,7 @@ import (
 	"strings"
 
 	operatorv1 "github.com/tigera/operator/api/v1"
+	"github.com/tigera/operator/pkg/common"
 	"github.com/tigera/operator/pkg/common/validation"
 	node "github.com/tigera/operator/pkg/common/validation/calico-node"
 	kubecontrollers "github.com/tigera/operator/pkg/common/validation/kube-controllers"
@@ -438,6 +439,17 @@ func validateCustomResource(instance *operatorv1.Installation) error {
 			return fmt.Errorf("Installation spec.Logging.cni is not valid and should not be provided when spec.cni.type is Not Calico")
 		}
 	}
+
+	if common.WindowsEnabled(instance.Spec) {
+		if len(instance.Spec.ServiceCIDRs) == 0 {
+			return fmt.Errorf("Installation spec.ServiceCIDRs must be provided when using Calico for Windows")
+		}
+	} else {
+		if instance.Spec.WindowsNodes != nil {
+			return fmt.Errorf("Installation spec.WindowsNodes is not valid and should not be provided when Calico for Windows is disabled")
+		}
+	}
+
 	return nil
 }
 

diff --git a/pkg/controller/installation/validation_test.go b/pkg/controller/installation/validation_test.go
@@ -736,6 +736,7 @@ var _ = Describe("Installation validation tests", func() {
 						},
 					},
 				},
+				ServiceCIDRs: []string{"10.96.0.0/12"},
 				CNI: &operator.CNISpec{
 					Type: operator.PluginCalico,
 					IPAM: &operator.IPAMSpec{Type: operator.IPAMPluginHostLocal},