Various TLS & Sync related fixes

Author: ewoutp

pkg/apis/deployment/v1alpha/sync_external_access_spec.go

@@ -35,7 +35,7 @@ import (
 type SyncExternalAccessSpec struct {
 	ExternalAccessSpec
 	MasterEndpoint           []string `json:"masterEndpoint,omitempty"`
-	AccessPackageSecretNames []string `json:accessPackageSecretNames,omitempty"`
+	AccessPackageSecretNames []string `json:"accessPackageSecretNames,omitempty"`
 }
 
 // GetMasterEndpoint returns the value of masterEndpoint.

pkg/deployment/context_impl.go

@@ -25,6 +25,8 @@ package deployment
 import (
 	"context"
 	"fmt"
+	"net"
+	"strconv"
 
 	"github.com/arangodb/arangosync/client"
 	"github.com/arangodb/arangosync/tasks"
@@ -174,7 +176,11 @@ func (d *Deployment) GetSyncServerClient(ctx context.Context, group api.ServerGr
 	dnsName := k8sutil.CreatePodDNSName(d.apiObject, group.AsRole(), id)
 
 	// Build client
-	source := client.Endpoint{dnsName}
+	port := k8sutil.ArangoSyncMasterPort
+	if group == api.ServerGroupSyncWorkers {
+		port = k8sutil.ArangoSyncWorkerPort
+	}
+	source := client.Endpoint{"https://" + net.JoinHostPort(dnsName, strconv.Itoa(port))}
 	tlsAuth := tasks.TLSAuthentication{
 		TLSClientAuthentication: tasks.TLSClientAuthentication{
 			ClientToken: monitoringToken,

pkg/deployment/reconcile/action_wait_for_member_up.go

@@ -154,7 +154,7 @@ func (a *actionWaitForMemberUp) checkProgressCluster(ctx context.Context) (bool,
 // of a sync master / worker.
 func (a *actionWaitForMemberUp) checkProgressArangoSync(ctx context.Context) (bool, bool, error) {
 	log := a.log
-	c, err := a.actionCtx.GetSyncServerClient(ctx, a.action.Group, a.action.ID)
+	c, err := a.actionCtx.GetSyncServerClient(ctx, a.action.Group, a.action.MemberID)
 	if err != nil {
 		log.Debug().Err(err).Msg("Failed to create arangosync client")
 		return false, false, maskAny(err)

pkg/deployment/reconcile/plan_builder_tls.go

@@ -62,10 +62,14 @@ func createRotateTLSServerCertificatePlan(log zerolog.Logger, spec api.Deploymen
 					Msg("Failed to get TLS secret")
 				continue
 			}
-			renewalNeeded := tlsKeyfileNeedsRenewal(log, keyfile)
+			tlsSpec := spec.TLS
+			if group.IsArangosync() {
+				tlsSpec = spec.Sync.TLS
+			}
+			renewalNeeded, reason := tlsKeyfileNeedsRenewal(log, keyfile, tlsSpec)
 			if renewalNeeded {
 				plan = append(append(plan,
-					api.NewAction(api.ActionTypeRenewTLSCertificate, group, m.ID)),
+					api.NewAction(api.ActionTypeRenewTLSCertificate, group, m.ID, reason)),
 					createRotateMemberPlan(log, m, group, "TLS certificate renewal")...,
 				)
 			}
@@ -124,8 +128,32 @@ func createRotateTLSCAPlan(log zerolog.Logger, spec api.DeploymentSpec, status a
 
 // tlsKeyfileNeedsRenewal decides if the certificate in the given keyfile
 // should be renewed.
-func tlsKeyfileNeedsRenewal(log zerolog.Logger, keyfile string) bool {
+func tlsKeyfileNeedsRenewal(log zerolog.Logger, keyfile string, spec api.TLSSpec) (bool, string) {
 	raw := []byte(keyfile)
+	// containsAll returns true when all elements in the expected list
+	// are in the actual list.
+	containsAll := func(actual []string, expected []string) bool {
+		for _, x := range expected {
+			found := false
+			for _, y := range actual {
+				if x == y {
+					found = true
+					break
+				}
+			}
+			if !found {
+				return false
+			}
+		}
+		return true
+	}
+	ipsToStringSlice := func(list []net.IP) []string {
+		result := make([]string, len(list))
+		for i, x := range list {
+			result[i] = x.String()
+		}
+		return result
+	}
 	for {
 		var derBlock *pem.Block
 		derBlock, raw = pem.Decode(raw)
@@ -137,7 +165,7 @@ func tlsKeyfileNeedsRenewal(log zerolog.Logger, keyfile string) bool {
 			if err != nil {
 				// We do not understand the certificate, let's renew it
 				log.Warn().Err(err).Msg("Failed to parse x509 certificate. Renewing it")
-				return true
+				return true, "Cannot parse x509 certificate: " + err.Error()
 			}
 			if cert.IsCA {
 				// Only look at the server certificate, not CA or intermediate
@@ -153,42 +181,31 @@ func tlsKeyfileNeedsRenewal(log zerolog.Logger, keyfile string) bool {
 					Str("not-after", cert.NotAfter.String()).
 					Str("expiration-date", expirationDate.String()).
 					Msg("TLS certificate renewal needed")
-				return true
+				return true, "Server certificate about to expire"
+			}
+			// Check alternate names against spec
+			dnsNames, ipAddresses, emailAddress, err := spec.GetParsedAltNames()
+			if err == nil {
+				if !containsAll(cert.DNSNames, dnsNames) {
+					return true, "Some alternate DNS names are missing"
+				}
+				if !containsAll(ipsToStringSlice(cert.IPAddresses), ipAddresses) {
+					return true, "Some alternate IP addresses are missing"
+				}
+				if !containsAll(cert.EmailAddresses, emailAddress) {
+					return true, "Some alternate email addresses are missing"
+				}
 			}
 		}
 	}
-	return false
+	return false, ""
 }
 
 // tlsCANeedsRenewal decides if the given CA certificate
 // should be renewed.
 // Returns: shouldRenew, reason
 func tlsCANeedsRenewal(log zerolog.Logger, cert string, spec api.TLSSpec) (bool, string) {
 	raw := []byte(cert)
-	// containsAll returns true when all elements in the expected list
-	// are in the actual list.
-	containsAll := func(actual []string, expected []string) bool {
-		for _, x := range expected {
-			found := false
-			for _, y := range actual {
-				if x == y {
-					found = true
-					break
-				}
-			}
-			if !found {
-				return false
-			}
-		}
-		return true
-	}
-	ipsToStringSlice := func(list []net.IP) []string {
-		result := make([]string, len(list))
-		for i, x := range list {
-			result[i] = x.String()
-		}
-		return result
-	}
 	for {
 		var derBlock *pem.Block
 		derBlock, raw = pem.Decode(raw)
@@ -218,19 +235,6 @@ func tlsCANeedsRenewal(log zerolog.Logger, cert string, spec api.TLSSpec) (bool,
 					Msg("TLS CA certificate renewal needed")
 				return true, "CA Certificate about to expire"
 			}
-			// Check alternate names against spec
-			dnsNames, ipAddresses, emailAddress, err := spec.GetParsedAltNames()
-			if err == nil {
-				if !containsAll(cert.DNSNames, dnsNames) {
-					return true, "Some alternate DNS names are missing"
-				}
-				if !containsAll(ipsToStringSlice(cert.IPAddresses), ipAddresses) {
-					return true, "Some alternate IP addresses are missing"
-				}
-				if !containsAll(cert.EmailAddresses, emailAddress) {
-					return true, "Some alternate email addresses are missing"
-				}
-			}
 		}
 	}
 	return false, ""

pkg/deployment/resources/certificates_tls.go

@@ -45,20 +45,13 @@ const (
 // specified in the given spec.
 func createTLSCACertificate(log zerolog.Logger, cli v1.CoreV1Interface, spec api.TLSSpec, deploymentName, namespace string, ownerRef *metav1.OwnerReference) error {
 	log = log.With().Str("secret", spec.GetCASecretName()).Logger()
-	dnsNames, ipAddresses, emailAddress, err := spec.GetParsedAltNames()
-	if err != nil {
-		log.Debug().Err(err).Msg("Failed to get alternate names")
-		return maskAny(err)
-	}
 
 	options := certificates.CreateCertificateOptions{
-		CommonName:     fmt.Sprintf("%s Root Certificate", deploymentName),
-		Hosts:          append(dnsNames, ipAddresses...),
-		EmailAddresses: emailAddress,
-		ValidFrom:      time.Now(),
-		ValidFor:       caTTL,
-		IsCA:           true,
-		ECDSACurve:     tlsECDSACurve,
+		CommonName: fmt.Sprintf("%s Root Certificate", deploymentName),
+		ValidFrom:  time.Now(),
+		ValidFor:   caTTL,
+		IsCA:       true,
+		ECDSACurve: tlsECDSACurve,
 	}
 	cert, priv, err := certificates.CreateCertificate(options, nil)
 	if err != nil {