multiple cluster ips

internal/controller/impersonatorconfig/impersonator_config.go

@@ -193,7 +193,7 @@ type certNameInfo struct {
 
 	// The IP address or hostname which was selected to be used as the name in the cert.
 	// Either selectedIP or selectedHostname will be set, but not both.
-	selectedIP       net.IP
+	selectedIPs      []net.IP
 	selectedHostname string
 
 	// The name of the endpoint to which a client should connect to talk to the impersonator.
@@ -636,14 +636,14 @@ func (c *impersonatorConfigController) deleteTLSSecretWhenCertificateDoesNotMatc
 	actualIPs := actualCertFromSecret.IPAddresses
 	actualHostnames := actualCertFromSecret.DNSNames
 	plog.Info("Checking TLS certificate names",
-		"desiredIP", nameInfo.selectedIP,
+		"desiredIPs", nameInfo.selectedIPs,
 		"desiredHostname", nameInfo.selectedHostname,
 		"actualIPs", actualIPs,
 		"actualHostnames", actualHostnames,
 		"secret", c.tlsSecretName,
 		"namespace", c.namespace)
 
-	if certHostnameAndIPMatchDesiredState(nameInfo.selectedIP, actualIPs, nameInfo.selectedHostname, actualHostnames) {
+	if certHostnameAndIPMatchDesiredState(nameInfo.selectedIPs, actualIPs, nameInfo.selectedHostname, actualHostnames) {
 		// The cert already matches the desired state, so there is no need to delete/recreate it.
 		return false, nil
 	}
@@ -654,8 +654,13 @@ func (c *impersonatorConfigController) deleteTLSSecretWhenCertificateDoesNotMatc
 	return true, nil
 }
 
-func certHostnameAndIPMatchDesiredState(desiredIP net.IP, actualIPs []net.IP, desiredHostname string, actualHostnames []string) bool {
+func certHostnameAndIPMatchDesiredState(desiredIPs []net.IP, actualIPs []net.IP, desiredHostname string, actualHostnames []string) bool {
-	if desiredIP != nil && len(actualIPs) == 1 && desiredIP.Equal(actualIPs[0]) && len(actualHostnames) == 0 {
+	if len(desiredIPs) > 0 && len(actualIPs) > 0 && len(actualIPs) == len(desiredIPs) && len(actualHostnames) == 0 {
+		for i := range desiredIPs {
+			if !actualIPs[i].Equal(desiredIPs[i]) {
+				return false
+			}
+		}
 		return true
 	}
 	if desiredHostname != "" && len(actualHostnames) == 1 && desiredHostname == actualHostnames[0] && len(actualIPs) == 0 {
@@ -677,7 +682,7 @@ func (c *impersonatorConfigController) ensureTLSSecretIsCreatedAndLoaded(ctx con
 		return nil
 	}
 
-	newTLSSecret, err := c.createNewTLSSecret(ctx, ca, nameInfo.selectedIP, nameInfo.selectedHostname)
+	newTLSSecret, err := c.createNewTLSSecret(ctx, ca, nameInfo.selectedIPs, nameInfo.selectedHostname)
 	if err != nil {
 		return err
 	}
@@ -746,12 +751,6 @@ func (c *impersonatorConfigController) createCASecret(ctx context.Context) (*cer
 }
 
 func (c *impersonatorConfigController) findDesiredTLSCertificateName(config *v1alpha1.ImpersonationProxySpec) (*certNameInfo, error) {
-	// possible valid options:
-	// - you have a loadbalancer and are autoconfiguring the endpoint -> get cert info based on load balancer ip/hostnome
-	// - you have a loadbalancer AND an external endpoint -> either should work since they should be the same
-	// - external endpoint no loadbalancer or other service -> use the endpoint config
-	// - external endpoint and ClusterIP -> use external endpoint?
-	// - clusterip and no external endpoint
 	if config.ExternalEndpoint != "" {
 		return c.findTLSCertificateNameFromEndpointConfig(config), nil
 	} else if config.Service.Type == v1alpha1.ImpersonationProxyServiceTypeClusterIP {
@@ -765,7 +764,7 @@ func (c *impersonatorConfigController) findTLSCertificateNameFromEndpointConfig(
 	endpointWithoutPort := strings.Split(endpointMaybeWithPort, ":")[0]
 	parsedAsIP := net.ParseIP(endpointWithoutPort)
 	if parsedAsIP != nil {
-		return &certNameInfo{ready: true, selectedIP: parsedAsIP, clientEndpoint: endpointMaybeWithPort}
+		return &certNameInfo{ready: true, selectedIPs: []net.IP{parsedAsIP}, clientEndpoint: endpointMaybeWithPort}
 	}
 	return &certNameInfo{ready: true, selectedHostname: endpointWithoutPort, clientEndpoint: endpointMaybeWithPort}
 }
@@ -797,7 +796,7 @@ func (c *impersonatorConfigController) findTLSCertificateNameFromLoadBalancer()
 		ip := ingress.IP
 		parsedIP := net.ParseIP(ip)
 		if parsedIP != nil {
-			return &certNameInfo{ready: true, selectedIP: parsedIP, clientEndpoint: ip}, nil
+			return &certNameInfo{ready: true, selectedIPs: []net.IP{parsedIP}, clientEndpoint: ip}, nil
 		}
 	}
 
@@ -815,22 +814,27 @@ func (c *impersonatorConfigController) findTLSCertificateNameFromClusterIPServic
 		return nil, err
 	}
 	ip := clusterIP.Spec.ClusterIP
+	ips := clusterIP.Spec.ClusterIPs
 	if ip != "" {
-		parsedIP := net.ParseIP(ip)
+		// clusterIP will always exist when clusterIPs does, but not vice versa
-		return &certNameInfo{ready: true, selectedIP: parsedIP, clientEndpoint: ip}, nil
+		var parsedIPs []net.IP
+		if len(ips) > 0 {
+			for _, ipFromIPs := range ips {
+				parsedIPs = append(parsedIPs, net.ParseIP(ipFromIPs))
+			}
+		} else {
+			parsedIPs = []net.IP{net.ParseIP(ip)}
+		}
+		return &certNameInfo{ready: true, selectedIPs: parsedIPs, clientEndpoint: ip}, nil
 	}
 	return &certNameInfo{ready: false}, nil
 }
 
-func (c *impersonatorConfigController) createNewTLSSecret(ctx context.Context, ca *certauthority.CA, ip net.IP, hostname string) (*v1.Secret, error) {
+func (c *impersonatorConfigController) createNewTLSSecret(ctx context.Context, ca *certauthority.CA, ips []net.IP, hostname string) (*v1.Secret, error) {
 	var hostnames []string
-	var ips []net.IP
 	if hostname != "" {
 		hostnames = []string{hostname}
 	}
-	if ip != nil {
-		ips = []net.IP{ip}
-	}
 
 	impersonationCert, err := ca.IssueServerCert(hostnames, ips, approximatelyOneHundredYears)
 	if err != nil {

internal/controller/impersonatorconfig/impersonator_config_test.go

@@ -427,7 +427,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 					t.Logf("DialContext replacing addr %s with %s", addr, replacementAddr)
 					addr = replacementAddr
 				} else if dnsOverrides != nil {
-					t.Fatal("dnsOverrides was provided but not used, which was probably a mistake")
+					t.Fatalf("dnsOverrides was provided but not used, which was probably a mistake. addr %s", addr)
 				}
 				return realDialer.DialContext(ctx, network, addr)
 			}
@@ -747,6 +747,15 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			r.NoError(client.Tracker().Add(clusterIPService))
 		}
 
+		var addDualStackClusterIPServiceToTracker = func(resourceName string, clusterIP0 string, clusterIP1 string, client *kubernetesfake.Clientset) {
+			clusterIPService := newClusterIPService(resourceName, corev1.ServiceStatus{}, corev1.ServiceSpec{
+				Type:       corev1.ServiceTypeClusterIP,
+				ClusterIP:  clusterIP0,
+				ClusterIPs: []string{clusterIP0, clusterIP1},
+			})
+			r.NoError(client.Tracker().Add(clusterIPService))
+		}
+
 		var addSecretToTrackers = func(secret *corev1.Secret, clients ...*kubernetesfake.Clientset) {
 			for _, client := range clients {
 				secretCopy := secret.DeepCopy()
@@ -1541,6 +1550,39 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 				})
 			})
 
+			when("a clusterip service exists with dual stack ips", func() {
+				const fakeIP1 = "127.0.0.123"
+				const fakeIP2 = "127.0.0.234" // TODO test that this works for an IPv6 address, which is the actual use case for multiple clusterips
+				it.Before(func() {
+					addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
+						ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName},
+						Spec: v1alpha1.CredentialIssuerSpec{
+							ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{
+								Mode: v1alpha1.ImpersonationProxyModeEnabled,
+								Service: v1alpha1.ImpersonationProxyServiceSpec{
+									Type: v1alpha1.ImpersonationProxyServiceTypeClusterIP,
+								},
+							},
+						},
+					}, pinnipedInformerClient, pinnipedAPIClient)
+					addNodeWithRoleToTracker("worker", kubeAPIClient)
+					addDualStackClusterIPServiceToTracker(clusterIPServiceName, fakeIP1, fakeIP2, kubeInformerClient)
+					addDualStackClusterIPServiceToTracker(clusterIPServiceName, fakeIP1, fakeIP2, kubeAPIClient)
+				})
+
+				it("certs are valid for both ip addresses", func() {
+					startInformersAndController()
+					r.NoError(runControllerSync())
+					r.Len(kubeAPIClient.Actions(), 3)
+					requireNodesListed(kubeAPIClient.Actions()[0])
+					ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1])
+					requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca)
+					requireTLSServerIsRunning(ca, fakeIP2, map[string]string{fakeIP2 + ":443": testServerAddr()})
+					requireTLSServerIsRunning(ca, fakeIP1, map[string]string{fakeIP1 + ":443": testServerAddr()})
+					requireCredentialIssuer(newSuccessStrategy(fakeIP1, ca))
+				})
+			})
+
 			when("a load balancer and a secret already exists", func() {
 				var caCrt []byte
 				it.Before(func() {