cmd/k8s-operator: remove conffile hashing mechanism (#16335)annotations

Proxies know how to reload configfile on changes since 1.80, which
is going to be the earliest supported proxy version with 1.84 operator,
so remove the mechanism that was updating configfile hash to force
proxy Pod restarts on config changes.

Updates #13032

Signed-off-by: Irbe Krumina <irbe@tailscale.com>

1 files changed, 16 insertions, 69 deletions

diff --git a/cmd/k8s-operator/sts.go b/cmd/k8s-operator/sts.go
index 70b25f2d2..4c7c3ac67 100644
--- a/cmd/k8s-operator/sts.go
+++ b/cmd/k8s-operator/sts.go
@@ -7,7 +7,6 @@ package main
 
 import (
 	"context"
-	"crypto/sha256"
 	_ "embed"
 	"encoding/json"
 	"errors"
@@ -91,8 +90,6 @@ const (
 	podAnnotationLastSetClusterDNSName    = "tailscale.com/operator-last-set-cluster-dns-name"
 	podAnnotationLastSetTailnetTargetIP   = "tailscale.com/operator-last-set-ts-tailnet-target-ip"
 	podAnnotationLastSetTailnetTargetFQDN = "tailscale.com/operator-last-set-ts-tailnet-target-fqdn"
-	// podAnnotationLastSetConfigFileHash is sha256 hash of the current tailscaled configuration contents.
-	podAnnotationLastSetConfigFileHash = "tailscale.com/operator-last-set-config-file-hash"
 
 	proxyTypeEgress          = "egress_service"
 	proxyTypeIngressService  = "ingress_service"
@@ -110,7 +107,7 @@ var (
 	// tailscaleManagedLabels are label keys that tailscale operator sets on StatefulSets and Pods.
 	tailscaleManagedLabels = []string{kubetypes.LabelManaged, LabelParentType, LabelParentName, LabelParentNamespace, "app"}
 	// tailscaleManagedAnnotations are annotation keys that tailscale operator sets on StatefulSets and Pods.
-	tailscaleManagedAnnotations = []string{podAnnotationLastSetClusterIP, podAnnotationLastSetTailnetTargetIP, podAnnotationLastSetTailnetTargetFQDN, podAnnotationLastSetConfigFileHash}
+	tailscaleManagedAnnotations = []string{podAnnotationLastSetClusterIP, podAnnotationLastSetTailnetTargetIP, podAnnotationLastSetTailnetTargetFQDN}
 )
 
 type tailscaleSTSConfig struct {
@@ -201,11 +198,11 @@ func (a *tailscaleSTSReconciler) Provision(ctx context.Context, logger *zap.Suga
 	}
 	sts.ProxyClass = proxyClass
 
-	secretName, tsConfigHash, _, err := a.createOrGetSecret(ctx, logger, sts, hsvc)
+	secretName, _, err := a.createOrGetSecret(ctx, logger, sts, hsvc)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create or get API key secret: %w", err)
 	}
-	_, err = a.reconcileSTS(ctx, logger, sts, hsvc, secretName, tsConfigHash)
+	_, err = a.reconcileSTS(ctx, logger, sts, hsvc, secretName)
 	if err != nil {
 		return nil, fmt.Errorf("failed to reconcile statefulset: %w", err)
 	}
@@ -335,7 +332,7 @@ func (a *tailscaleSTSReconciler) reconcileHeadlessService(ctx context.Context, l
 	return createOrUpdate(ctx, a.Client, a.operatorNamespace, hsvc, func(svc *corev1.Service) { svc.Spec = hsvc.Spec })
 }
 
-func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger *zap.SugaredLogger, stsC *tailscaleSTSConfig, hsvc *corev1.Service) (secretName, hash string, configs tailscaledConfigs, _ error) {
+func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger *zap.SugaredLogger, stsC *tailscaleSTSConfig, hsvc *corev1.Service) (secretName string, configs tailscaledConfigs, _ error) {
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			// Hardcode a -0 suffix so that in future, if we support
@@ -351,7 +348,7 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger *
 		logger.Debugf("secret %s/%s already exists", secret.GetNamespace(), secret.GetName())
 		orig = secret.DeepCopy()
 	} else if !apierrors.IsNotFound(err) {
-		return "", "", nil, err
+		return "", nil, err
 	}
 
 	var authKey string
@@ -361,13 +358,13 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger *
 		// ACME account key.
 		sts, err := getSingleObject[appsv1.StatefulSet](ctx, a.Client, a.operatorNamespace, stsC.ChildResourceLabels)
 		if err != nil {
-			return "", "", nil, err
+			return "", nil, err
 		}
 		if sts != nil {
 			// StatefulSet exists, so we have already created the secret.
 			// If the secret is missing, they should delete the StatefulSet.
 			logger.Errorf("Tailscale proxy secret doesn't exist, but the corresponding StatefulSet %s/%s already does. Something is wrong, please delete the StatefulSet.", sts.GetNamespace(), sts.GetName())
-			return "", "", nil, nil
+			return "", nil, nil
 		}
 		// Create API Key secret which is going to be used by the statefulset
 		// to authenticate with Tailscale.
@@ -378,25 +375,20 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger *
 		}
 		authKey, err = newAuthKey(ctx, a.tsClient, tags)
 		if err != nil {
-			return "", "", nil, err
+			return "", nil, err
 		}
 	}
 	configs, err := tailscaledConfig(stsC, authKey, orig)
 	if err != nil {
-		return "", "", nil, fmt.Errorf("error creating tailscaled config: %w", err)
+		return "", nil, fmt.Errorf("error creating tailscaled config: %w", err)
 	}
-	hash, err = tailscaledConfigHash(configs)
-	if err != nil {
-		return "", "", nil, fmt.Errorf("error calculating hash of tailscaled configs: %w", err)
-	}
-
 	latest := tailcfg.CapabilityVersion(-1)
 	var latestConfig ipn.ConfigVAlpha
 	for key, val := range configs {
 		fn := tsoperator.TailscaledConfigFileName(key)
 		b, err := json.Marshal(val)
 		if err != nil {
-			return "", "", nil, fmt.Errorf("error marshalling tailscaled config: %w", err)
+			return "", nil, fmt.Errorf("error marshalling tailscaled config: %w", err)
 		}
 		mak.Set(&secret.StringData, fn, string(b))
 		if key > latest {
@@ -408,7 +400,7 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger *
 	if stsC.ServeConfig != nil {
 		j, err := json.Marshal(stsC.ServeConfig)
 		if err != nil {
-			return "", "", nil, err
+			return "", nil, err
 		}
 		mak.Set(&secret.StringData, "serve-config", string(j))
 	}
@@ -416,15 +408,15 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger *
 	if orig != nil {
 		logger.Debugf("patching the existing proxy Secret with tailscaled config %s", sanitizeConfigBytes(latestConfig))
 		if err := a.Patch(ctx, secret, client.MergeFrom(orig)); err != nil {
-			return "", "", nil, err
+			return "", nil, err
 		}
 	} else {
 		logger.Debugf("creating a new Secret for the proxy with tailscaled config %s", sanitizeConfigBytes(latestConfig))
 		if err := a.Create(ctx, secret); err != nil {
-			return "", "", nil, err
+			return "", nil, err
 		}
 	}
-	return secret.Name, hash, configs, nil
+	return secret.Name, configs, nil
 }
 
 // sanitizeConfigBytes returns ipn.ConfigVAlpha in string form with redacted
@@ -535,7 +527,7 @@ var proxyYaml []byte
 //go:embed deploy/manifests/userspace-proxy.yaml
 var userspaceProxyYaml []byte
 
-func (a *tailscaleSTSReconciler) reconcileSTS(ctx context.Context, logger *zap.SugaredLogger, sts *tailscaleSTSConfig, headlessSvc *corev1.Service, proxySecret, tsConfigHash string) (*appsv1.StatefulSet, error) {
+func (a *tailscaleSTSReconciler) reconcileSTS(ctx context.Context, logger *zap.SugaredLogger, sts *tailscaleSTSConfig, headlessSvc *corev1.Service, proxySecret string) (*appsv1.StatefulSet, error) {
 	ss := new(appsv1.StatefulSet)
 	if sts.ServeConfig != nil && sts.ForwardClusterTrafficViaL7IngressProxy != true { // If forwarding cluster traffic via is required we need non-userspace + NET_ADMIN + forwarding
 		if err := yaml.Unmarshal(userspaceProxyYaml, &ss); err != nil {
@@ -662,11 +654,6 @@ func (a *tailscaleSTSReconciler) reconcileSTS(ctx context.Context, logger *zap.S
 		})
 	}
 
-	dev, err := a.DeviceInfo(ctx, sts.ChildResourceLabels, logger)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get device info: %w", err)
-	}
-
 	app, err := appInfoForProxy(sts)
 	if err != nil {
 		// No need to error out if now or in future we end up in a
@@ -685,25 +672,7 @@ func (a *tailscaleSTSReconciler) reconcileSTS(ctx context.Context, logger *zap.S
 		ss = applyProxyClassToStatefulSet(sts.ProxyClass, ss, sts, logger)
 	}
 	updateSS := func(s *appsv1.StatefulSet) {
-		// This is a temporary workaround to ensure that proxies with capver older than 110
-		// are restarted when tailscaled configfile contents have changed.
-		// This workaround ensures that:
-		// 1. The hash mechanism is used to trigger pod restarts for proxies below capver 110.
-		// 2. Proxies above capver are not unnecessarily restarted when the configfile contents change.
-		// 3. If the hash has alreay been set, but the capver is above 110, the old hash is preserved to avoid
-		// unnecessary pod restarts that could result in an update loop where capver cannot be determined for a
-		// restarting Pod and the hash is re-added again.
-		// Note that the hash annotation is only set on updates not creation, because if the StatefulSet is
-		// being created, there is no need for a restart.
-		// TODO(irbekrm): remove this in 1.84.
-		hash := tsConfigHash
-		if dev == nil || dev.capver >= 110 {
-			hash = s.Spec.Template.GetAnnotations()[podAnnotationLastSetConfigFileHash]
-		}
 		s.Spec = ss.Spec
-		if hash != "" {
-			mak.Set(&s.Spec.Template.Annotations, podAnnotationLastSetConfigFileHash, hash)
-		}
 		s.ObjectMeta.Labels = ss.Labels
 		s.ObjectMeta.Annotations = ss.Annotations
 	}
@@ -937,8 +906,7 @@ func readAuthKey(secret *corev1.Secret, key string) (*string, error) {
 }
 
 // tailscaledConfig takes a proxy config, a newly generated auth key if generated and a Secret with the previous proxy
-// state and auth key and returns tailscaled config files for currently supported proxy versions and a hash of that
-// configuration.
+// state and auth key and returns tailscaled config files for currently supported proxy versions.
 func tailscaledConfig(stsC *tailscaleSTSConfig, newAuthkey string, oldSecret *corev1.Secret) (tailscaledConfigs, error) {
 	conf := &ipn.ConfigVAlpha{
 		Version:             "alpha0",
@@ -1031,27 +999,6 @@ type ptrObject[T any] interface {
 
 type tailscaledConfigs map[tailcfg.CapabilityVersion]ipn.ConfigVAlpha
 
-// hashBytes produces a hash for the provided tailscaled config that is the same across
-// different invocations of this code. We do not use the
-// tailscale.com/deephash.Hash here because that produces a different hash for
-// the same value in different tailscale builds. The hash we are producing here
-// is used to determine if the container running the Connector Tailscale node
-// needs to be restarted. The container does not need restarting when the only
-// thing that changed is operator version (the hash is also exposed to users via
-// an annotation and might be confusing if it changes without the config having
-// changed).
-func tailscaledConfigHash(c tailscaledConfigs) (string, error) {
-	b, err := json.Marshal(c)
-	if err != nil {
-		return "", fmt.Errorf("error marshalling tailscaled configs: %w", err)
-	}
-	h := sha256.New()
-	if _, err = h.Write(b); err != nil {
-		return "", fmt.Errorf("error calculating hash: %w", err)
-	}
-	return fmt.Sprintf("%x", h.Sum(nil)), nil
-}
-
 // createOrMaybeUpdate adds obj to the k8s cluster, unless the object already exists,
 // in which case update is called to make changes to it. If update is nil or returns
 // an error, the object is returned unmodified.