kms/health: fieldManager static or uid

Author: ibihim

pkg/operator/encryption/kms/health/cmd.go

@@ -26,9 +26,11 @@ const (
 var kmsSocketPattern = regexp.MustCompile(`^unix:///var/run/kmsplugin/kms-(\d+)\.sock$`)
 
 // NewEncryptionStatusWriterFunc builds the EncryptionStatusWriter for a target
-// apiserver operator status CR. fieldManager sets the owner in the
-// managedFields when doing SSA.
-type NewEncryptionStatusWriterFunc func(restConfig *rest.Config, fieldManager string) (EncryptionStatusWriter, error)
+// apiserver operator status CR. The writer derives its own SSA fieldManager
+// from nodeName, because only it knows whether its status collects many
+// per-node reporters (kube-apiserver: fieldManager from the node UID) or a
+// single one (a constant fieldManager).
+type NewEncryptionStatusWriterFunc func(restConfig *rest.Config, nodeName string) (EncryptionStatusWriter, error)
 
 // EncryptionStatusWriter is capable of applying the
 // KMSEncryptionStatusApplyConfiguration at the correct place in the operator's

pkg/operator/encryption/kms/health/options.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/spf13/pflag"
 
-	metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/clientcmd"
 )
@@ -61,9 +60,6 @@ func (o *options) validate() error {
 	if o.NodeName == "" {
 		return fmt.Errorf("--node-name is required")
 	}
-	if fieldManager := fmt.Sprintf("%s-%s", Subcommand, o.NodeName); len(fieldManager) > metav1validation.FieldManagerMaxLength {
-		return fmt.Errorf("--node-name too long: reporter identity %q is %d chars, exceeds the %d-char fieldManager limit", fieldManager, len(fieldManager), metav1validation.FieldManagerMaxLength)
-	}
 
 	return nil
 }
@@ -76,9 +72,10 @@ func (o *options) Config(ctx context.Context) (*Config, error) {
 		return nil, fmt.Errorf("build rest config: %w", err)
 	}
 
-	// fieldManager is the per-node ownership-identity.
-	fieldManager := fmt.Sprintf("%s-%s", Subcommand, o.NodeName)
-	writeStatus, err := o.newWriter(restCfg, fieldManager)
+	// The writer owns its fieldManager: it turns the node name into whatever
+	// ownership identity its status needs (a per-node UID for the kube-apiserver,
+	// a constant for the single-reporter operators).
+	writeStatus, err := o.newWriter(restCfg, o.NodeName)
 	if err != nil {
 		return nil, fmt.Errorf("build encryption status writer: %w", err)
 	}

pkg/operator/encryption/kms/health/options_test.go

@@ -1,7 +1,6 @@
 package health
 
 import (
-	"strings"
 	"testing"
 	"time"
 
@@ -104,15 +103,6 @@ func TestValidate(t *testing.T) {
 			mutate:  func(o *options) { o.NodeName = "" },
 			wantErr: true,
 		},
-		{
-			name:   "node name at fieldManager limit",
-			mutate: func(o *options) { o.NodeName = strings.Repeat("n", 108) }, // 20-char prefix + 108 == 128
-		},
-		{
-			name:    "node name over fieldManager limit",
-			mutate:  func(o *options) { o.NodeName = strings.Repeat("n", 109) }, // 20-char prefix + 109 > 128
-			wantErr: true,
-		},
 	}
 
 	for _, tc := range tests {

pkg/operator/encryption/kms/health/writers/writers.go

@@ -9,8 +9,10 @@ package writers
 
 import (
 	"context"
+	"fmt"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 
 	applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
@@ -22,7 +24,7 @@ import (
 // NewAuthenticationWriter writes into Authentication/cluster at
 // .status.oauthAPIServer.encryptionStatus. The auth operator manages the
 // oauth-apiserver. There is an extra hop the other two operators don't have.
-func NewAuthenticationWriter(restConfig *rest.Config, fieldManager string) (health.EncryptionStatusWriter, error) {
+func NewAuthenticationWriter(restConfig *rest.Config, _ string) (health.EncryptionStatusWriter, error) {
 	client, err := operatorclient.NewForConfig(restConfig)
 	if err != nil {
 		return nil, err
@@ -37,21 +39,42 @@ func NewAuthenticationWriter(restConfig *rest.Config, fieldManager string) (heal
 				),
 			),
 
-			metav1.ApplyOptions{FieldManager: fieldManager, Force: true},
+			// A single reporter writes this status, so a constant fieldManager
+			// is enough and always fits the apiserver's length limit.
+			metav1.ApplyOptions{FieldManager: health.Subcommand, Force: true},
 		)
 		return err
 	}, nil
 }
 
 // NewKubeAPIServerWriter writes into KubeAPIServer/cluster at
 // .status.encryptionStatus.
-func NewKubeAPIServerWriter(restConfig *rest.Config, fieldManager string) (health.EncryptionStatusWriter, error) {
+func NewKubeAPIServerWriter(restConfig *rest.Config, nodeName string) (health.EncryptionStatusWriter, error) {
 	client, err := operatorclient.NewForConfig(restConfig)
 	if err != nil {
 		return nil, err
 	}
+	kubeClient, err := kubernetes.NewForConfig(restConfig)
+	if err != nil {
+		return nil, err
+	}
 
+	// One reporter runs per control-plane node and each applies its own row, so
+	// the fieldManager must be per-node. The node UID is a fixed-length,
+	// instance-stable identity that fits the fieldManager length limit, unlike
+	// the node name (a DNS subdomain up to 253 chars).
+	//
+	// Lazy evaluation on first call as constructor isn't concerned with any context.
+	var fieldManager string
 	return func(ctx context.Context, status *applyoperatorv1.KMSEncryptionStatusApplyConfiguration) error {
+		if fieldManager == "" {
+			node, err := kubeClient.CoreV1().Nodes().Get(ctx, nodeName, metav1.GetOptions{})
+			if err != nil {
+				return fmt.Errorf("get node %q: %w", nodeName, err)
+			}
+			fieldManager = fmt.Sprintf("%s-%s", health.Subcommand, node.UID)
+		}
+
 		_, err := client.OperatorV1().KubeAPIServers().ApplyStatus(
 			ctx,
 			applyoperatorv1.KubeAPIServer("cluster").WithStatus(
@@ -65,7 +88,7 @@ func NewKubeAPIServerWriter(restConfig *rest.Config, fieldManager string) (healt
 
 // NewOpenShiftAPIServerWriter writes into OpenShiftAPIServer/cluster at
 // .status.encryptionStatus.
-func NewOpenShiftAPIServerWriter(restConfig *rest.Config, fieldManager string) (health.EncryptionStatusWriter, error) {
+func NewOpenShiftAPIServerWriter(restConfig *rest.Config, _ string) (health.EncryptionStatusWriter, error) {
 	client, err := operatorclient.NewForConfig(restConfig)
 	if err != nil {
 		return nil, err
@@ -78,7 +101,9 @@ func NewOpenShiftAPIServerWriter(restConfig *rest.Config, fieldManager string) (
 				applyoperatorv1.OpenShiftAPIServerStatus().WithEncryptionStatus(status),
 			),
 
-			metav1.ApplyOptions{FieldManager: fieldManager, Force: true},
+			// A single reporter writes this status, so a constant fieldManager
+			// is enough and always fits the apiserver's length limit.
+			metav1.ApplyOptions{FieldManager: health.Subcommand, Force: true},
 		)
 		return err
 	}, nil