Feat/k8s no clustername (#402)

* k8s1.25

* interface

* GetAnnotations() replaces ClusterName field in ClusterResourceId interface

* actually support k8s v1.25 (with controller-runtime v0.13.1)

* update go templates; changelog; dep updates

* put changelog in folder

* update from generated code

* add annotations field to ClusterObjectRef proto to match ClusterResourceId interface

* remove annotations field from ClusterObjectRef proto

* Adding changelog file to new location

* Deleting changelog file from old location

* add function to allow ClusterObjectRef struct to satisfy the ClusterResourceId interface

* handle possibility of clusterName being set by annotation or field

* pr feedback during working session with eitan

* Update contrib/pkg/sets/sets.go

Co-authored-by: J. Conrad Hanson <[email protected]>

* contrib/reconciler: Update to converting to new interface. Not stored directly as its not exported.

* Adding changelog file to new location

* Deleting changelog file from old location

* remove extraneous separators

* mod: Revert the comingled k8s1.25 changes

* changelog: Update to reference what is being done correctly

* add back

* codegen

* fix input reconciler

* register: Update to ensure that sa secret exists. Note: Not handled on SA creation but seems to be the only place where we need to ensure secrets exist

* register: Make sure to have a namespace

* register: Make sure wait for token population

* reigster: Longer delay now that extra controller layer is needed for secrets

* mod: skv2 tocreate on the correct cluster

* secret-type change

* templ/input: Set clusters in a novel way

* templ/input: Set clusters consistently given new novel approach

* kubeconfig: Update comment

* ezkube: Method for converting deprecated objects will no longer blow away other annotations

* register: harden service account getting

* test/registration: Change expectations given serviceaccount changes

* template/inputs/hybrid: Clusternaming

Co-authored-by: Eitan Yarmush <[email protected]>
Co-authored-by: Conrad Hanson <[email protected]>
Co-authored-by: changelog-bot <changelog-bot>
Co-authored-by: Jenny Shu <[email protected]>

changelog/v0.26.0/clustername-via-annotations.yaml

@@ -0,0 +1,7 @@
+changelog:
+    - type: BREAKING_CHANGE
+      issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/5158
+      description: >
+        In the ClusterResourceId interface, ClusterName field is replaced with GetAnnotations method,
+        since k8s v1.24+ removes the ClusterName field from resources.
+        Now, a resource annotation of "cluster.solo.io/cluster" is expected to hold a string of the cluster name.

contrib/codegen/templates/input/hybrid_input_reconciler.gotmpl

@@ -159,7 +159,7 @@ type remote{{ $snapshotName }}InputReconciler struct {
 {{- $kindLowerCamelPlural := pluralize $kindLowerCamel }}
 
 func (r *remote{{ $snapshotName }}InputReconciler) Reconcile{{ $resource.Kind }}(clusterName string, obj *{{ $type_import_prefix }}.{{ $resource.Kind }}) (reconcile.Result, error) {
-    obj.ClusterName = clusterName
+    ezkube.SetClusterName(obj, clusterName)
     return r.base.ReconcileRemoteGeneric(obj)
 }
 

contrib/codegen/templates/input/input_reconciler.gotmpl

@@ -26,6 +26,7 @@ import (
     multicluster_v2 "github.com/solo-io/skv2/pkg/multicluster/v2"
     reconcile "github.com/solo-io/skv2/pkg/reconcile"
     reconcile_v2 "github.com/solo-io/skv2/pkg/reconcile/v2"
+    "github.com/solo-io/skv2/pkg/ezkube"
 
     "sigs.k8s.io/controller-runtime/pkg/manager"
     "sigs.k8s.io/controller-runtime/pkg/predicate"
@@ -88,7 +89,7 @@ func RegisterMultiCluster{{ $snapshotName }}Reconciler(
     multicluster_reconcile_v2.NewLoop("das", clusters, &{{ $types_import_prefix }}.{{ $resource.Kind }}{}, reconcile_v2.Options{}).
         AddReconciler(ctx, &multicluster_v2.ReconcilerFuncs[*{{ $types_import_prefix }}.{{ $resource.Kind }}]{
             ReconcileFunc: func(ctx context.Context, clusterName string, obj *{{ $types_import_prefix }}.{{ $resource.Kind }}) (reconcile.Result, error) {
-                obj.ClusterName = clusterName
+                ezkube.SetClusterName(obj, clusterName)
                 return r.base.ReconcileRemoteGeneric(obj)
             } ,
             ReconcileDeletionFunc: func(ctx context.Context, clusterName string, obj reconcile_v2.Request) error {
@@ -97,7 +98,7 @@ func RegisterMultiCluster{{ $snapshotName }}Reconciler(
                     Namespace:            obj.Namespace,
                     ClusterName:          clusterName,
                 }
-                _, err := r.base.ReconcileRemoteGeneric(ref)
+                _, err := r.base.ReconcileRemoteGeneric(ezkube.ConvertRefToId(ref))
             return err
         },
     }, predicates...)

contrib/codegen/templates/input/input_snapshot.gotmpl

@@ -26,6 +26,7 @@ import (
 
     "k8s.io/apimachinery/pkg/labels"
     "k8s.io/apimachinery/pkg/runtime/schema"
+    "github.com/solo-io/skv2/pkg/ezkube"
     "github.com/solo-io/skv2/pkg/resource"
     "github.com/solo-io/skv2/pkg/verifier"
     "github.com/solo-io/go-utils/contextutils"
@@ -335,7 +336,7 @@ func (s *snapshot{{ $snapshotName }}) ForEachObject(handleObject func(cluster st
     {{- $kindLowerCamel := lower_camel $resource.Kind }}
     {{- $kindLowerCamelPlural := pluralize $kindLowerCamel }}
     for _, obj := range s.{{ $kindLowerCamelPlural }}.List() {
-        cluster := obj.GetClusterName()
+        cluster := ezkube.GetClusterName(obj)
         gvk := schema.GroupVersionKind{
             Group: "{{ $resource.Group.Group }}",
             Version: "{{ $resource.Version }}",
@@ -487,7 +488,8 @@ func (b *multiCluster{{ $snapshotName }}Builder) insert{{ $kindPlural }}FromClus
 
     for _, item := range {{ $kindLowerCamel }}List.Items {
         item := item.DeepCopy()             // pike + own
-        item.ClusterName = cluster // set cluster for in-memory processing
+
+        ezkube.SetClusterName(item, cluster) // set cluster for what was in-memory processing
         {{ $kindLowerCamelPlural }}.Insert(item)
     }
 
@@ -601,7 +603,7 @@ func (b *singleCluster{{ $snapshotName }}Builder) insert{{ $kindPlural }}(ctx co
 
     for _, item := range {{ $kindLowerCamel }}List.Items {
         item := item.DeepCopy()               // pike + own the item.
-        item.ClusterName = b.clusterName
+        ezkube.SetClusterName(item, b.clusterName) // set cluster for what was in-memory processing
         {{ $kindLowerCamelPlural }}.Insert(item)
     }
 

contrib/codegen/templates/output/output_snapshot.gotmpl

@@ -269,7 +269,7 @@ func (s *snapshot) ForEachObject(handleObject func(cluster string, gvk schema.Gr
     {{- $kindLowerCamelPlural := pluralize $kindLowerCamel }}
     for _, set := range s.{{ $kindLowerCamelPlural }} {
         for _, obj := range set.Set().List() {
-            cluster := obj.GetClusterName()
+            cluster := ezkube.GetClusterName(obj)
             gvk := schema.GroupVersionKind{
                 Group: "{{ $resource.Group.Group }}",
                 Version: "{{ $resource.Version }}",
@@ -668,7 +668,7 @@ func (b *builder) Generic() resource.ClusterSnapshot {
 {{- $kindLowerCamel := lower_camel $resource.Kind }}
 {{- $kindLowerCamelPlural := pluralize $kindLowerCamel }}
     for _, obj := range b.Get{{ $kindPlural }}().List() {
-        cluster := obj.GetClusterName()
+        cluster := ezkube.GetClusterName(obj)
         gvk := schema.GroupVersionKind{
             Group: "{{ $resource.Group.Group }}",
             Version: "{{ $resource.Version }}",
@@ -697,7 +697,7 @@ func (b *builder) ForEachObject(handleObject func(cluster string, gvk schema.Gro
 {{- $kindLowerCamel := lower_camel $resource.Kind }}
 {{- $kindLowerCamelPlural := pluralize $kindLowerCamel }}
     for _, obj := range b.Get{{ $kindPlural }}().List() {
-        cluster := obj.GetClusterName()
+        cluster := ezkube.GetClusterName(obj)
         gvk := schema.GroupVersionKind{
             Group: "{{ $resource.Group.Group }}",
             Version: "{{ $resource.Version }}",

contrib/pkg/input/input_reconciler.go

@@ -121,7 +121,7 @@ func (r *inputReconciler) processNextWorkItem() bool {
 	// determine whether the resource has been read from a remote cluster
 	// based on whether its ClusterName field is set
 	if clusterResource, ok := key.(ezkube.ClusterResourceId); ok {
-		if clusterResource.GetClusterName() != "" {
+		if ezkube.GetClusterName(clusterResource) != "" {
 			isRemoteCluster = true
 		}
 	}

contrib/pkg/output/snapshot.go

@@ -180,7 +180,7 @@ type ResourceList struct {
 func (l ResourceList) SplitByClusterName() map[string][]ezkube.Object {
 	listsByCluster := map[string][]ezkube.Object{}
 	for _, resource := range l.Resources {
-		clusterName := resource.GetClusterName()
+		clusterName := ezkube.GetClusterName(resource)
 		listForCluster := listsByCluster[clusterName]
 
 		// list func (i.e. garbage collection labels) shared across clusters
@@ -354,7 +354,7 @@ func (s Snapshot) syncList(
 func (s Snapshot) upsert(ctx context.Context, cli client.Client, obj ezkube.Object, statusUpdate bool) error {
 	// add cluster label to object
 	obj.SetLabels(
-		AddClusterLabels(obj.GetClusterName(), obj.GetLabels()),
+		AddClusterLabels(ezkube.GetClusterName(obj), obj.GetLabels()),
 	)
 
 	var (

contrib/pkg/sets/sets.go

@@ -34,18 +34,22 @@ func Key(id ezkube.ResourceId) string {
 	if id == nil {
 		return "<unknown>"
 	}
-	if clusterId, ok := id.(ezkube.ClusterResourceId); ok {
-		b.WriteString(clusterId.GetName())
-		b.WriteString(separator)
-		b.WriteString(clusterId.GetNamespace())
-		b.WriteString(separator)
-		b.WriteString(clusterId.GetClusterName())
-		return b.String()
-	}
 	b.WriteString(id.GetName())
 	b.WriteString(separator)
 	b.WriteString(id.GetNamespace())
 	b.WriteString(separator)
+	// handle the possibility that clusterName could be set either as an annotation (new way)
+	// or as a field (old way pre-k8s 1.25)
+	if clusterId, ok := id.(ezkube.ClusterResourceId); ok {
+		clusterNameByAnnotation := ezkube.GetClusterName(clusterId)
+		if clusterNameByAnnotation != "" {
+			b.WriteString(clusterNameByAnnotation)
+			return b.String()
+		}
+	}
+	if deprecatedClusterId, ok := id.(interface{ GetClusterName() string }); ok {
+		b.WriteString(deprecatedClusterId.GetClusterName())
+	}
 	return b.String()
 }
 

pkg/ezkube/object_ref.go

@@ -22,7 +22,7 @@ func MakeClusterObjectRef(resource ClusterResourceId) *v1.ClusterObjectRef {
 	return &v1.ClusterObjectRef{
 		Name:        resource.GetName(),
 		Namespace:   resource.GetNamespace(),
-		ClusterName: resource.GetClusterName(),
+		ClusterName: GetClusterName(resource),
 	}
 }
 
@@ -42,7 +42,7 @@ func ClusterRefsMatch(ref1, ref2 ClusterResourceId) bool {
 	}
 	return ref1.GetNamespace() == ref2.GetNamespace() &&
 		ref1.GetName() == ref2.GetName() &&
-		ref1.GetClusterName() == ref2.GetClusterName()
+		GetClusterName(ref1) == GetClusterName(ref2)
 }
 
 func MakeClientObjectKey(ref ResourceId) client.ObjectKey {

pkg/ezkube/resource_id.go

@@ -1,14 +1,86 @@
 package ezkube
 
+import (
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+const ClusterAnnotation = "cluster.solo.io/cluster"
+
 // ResourceId represents a global identifier for a k8s resource.
 type ResourceId interface {
 	GetName() string
 	GetNamespace() string
 }
 
-// ResourceId represents a global identifier for a k8s resource.
+// ClusterResourceId represents a global identifier for a k8s resource.
 type ClusterResourceId interface {
+	GetName() string
+	GetNamespace() string
+	GetAnnotations() map[string]string
+}
+
+// internal struct needed to create helper func that converts ref to struct that satisfies ClusterResourceId interface
+type clusterResourceId struct {
+	name, namespace string
+	annotations     map[string]string
+}
+
+func (c clusterResourceId) GetName() string {
+	return c.name
+}
+
+func (c clusterResourceId) GetNamespace() string {
+	return c.namespace
+}
+
+func (c clusterResourceId) GetAnnotations() map[string]string {
+	return c.annotations
+}
+
+type deprecatedClusterResourceId interface {
 	GetName() string
 	GetNamespace() string
 	GetClusterName() string
 }
+
+// ConvertRefToId converts a ClusterObjectRef to a struct that implements the ClusterResourceId interface
+// Will not set an empty cluster name over an existing cluster name
+func ConvertRefToId(ref deprecatedClusterResourceId) ClusterResourceId {
+	// if ref is already stores annotations then we need to store the updates
+	anno := map[string]string{}
+
+	if cri, ok := ref.(ClusterResourceId); ok {
+		anno = cri.GetAnnotations()
+	}
+	cn := ref.GetClusterName()
+	if cn != "" {
+		anno[ClusterAnnotation] = cn
+	}
+
+	return clusterResourceId{
+		name:        ref.GetName(),
+		namespace:   ref.GetNamespace(),
+		annotations: anno,
+	}
+}
+
+func GetDeprecatedClusterName(id ResourceId) string {
+	if id, ok := id.(deprecatedClusterResourceId); ok {
+		return id.GetClusterName()
+	}
+	return ""
+}
+
+func GetClusterName(id ClusterResourceId) string {
+	if id.GetAnnotations() == nil {
+		return ""
+	}
+	return id.GetAnnotations()[ClusterAnnotation]
+}
+
+func SetClusterName(obj client.Object, cluster string) {
+	if obj.GetAnnotations() == nil {
+		obj.SetAnnotations(map[string]string{})
+	}
+	obj.GetAnnotations()[ClusterAnnotation] = cluster
+}

pkg/multicluster/kubeconfig/kubeconfig_secret.go

@@ -26,7 +26,8 @@ var (
 
 // TODO settle on how to canonicalize cluster names: https://github.com/solo-io/skv2/issues/15
 
-// ToSecret converts a kubernetes api.Config to a secret with the provided name and namespace.
+// ToSecret converts a kubernetes api.Config to a secret with the provided
+// cluster as the name and the provided namespace.
 func ToSecret(namespace string, cluster string, resourceLabels map[string]string, kc api.Config) (*kubev1.Secret, error) {
 	err := convertCertFilesToInline(kc)
 	if err != nil {

pkg/multicluster/register/registrant.go

@@ -42,8 +42,8 @@ var (
 	// exponential backoff retry with an initial period of 0.1s for 7 iterations, which will mean a cumulative retry period of ~6s
 	// visible for testing
 	SecretLookupOpts = []retry.Option{
-		retry.Delay(time.Millisecond * 100),
-		retry.Attempts(7),
+		retry.Delay(time.Millisecond * 200),
+		retry.Attempts(10),
 		retry.DelayType(retry.BackOffDelay),
 	}
 )
@@ -594,26 +594,64 @@ func (c *clusterRegistrant) getTokenForSa(
 	if err != nil {
 		return "", err
 	}
-	var foundSecret *corev1.Secret
+
+	var sa *corev1.ServiceAccount
+
 	if err = retry.Do(func() error {
-		sa, err := saClient.GetServiceAccount(ctx, saRef)
+		sa, err = saClient.GetServiceAccount(ctx, saRef)
 		if err != nil {
 			return err
 		}
-		if len(sa.Secrets) == 0 {
+		return nil
+	}, SecretLookupOpts...); err != nil {
+		return "", err
+	}
+
+	// create a secret name format. This is similar to what was previously done
+	// but after the k8s upgrade we no longer have knowledge of what was previously
+	// autogenerated by k8s.
+	// TODO(nfuden): move away from these long lived connections as they are
+	// likely to be deprecated even more in the future.
+	secretName := fmt.Sprintf("%s-secret", sa.Name)
+
+	// see if there is a secret
+	_, err = remoteSecretClient.GetSecret(ctx, client.ObjectKey{Name: secretName, Namespace: saRef.Namespace})
+	if err != nil && k8s_errs.IsNotFound(err) {
+
+		saSecret := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:        secretName,
+				Namespace:   saRef.Namespace,
+				Annotations: map[string]string{"kubernetes.io/service-account.name": saRef.Name},
+			},
+			Type: "kubernetes.io/service-account-token",
+		}
+
+		// make secret
+		if err = remoteSecretClient.UpsertSecret(ctx, saSecret); err != nil {
+			return "", err
+		}
+
+	}
+
+	var foundSecret *corev1.Secret
+	if err = retry.Do(func() error {
+		secret, err := remoteSecretClient.GetSecret(ctx, client.ObjectKey{Name: secretName, Namespace: saRef.Namespace})
+		if err != nil {
 			return eris.Errorf(
 				"service account %s.%s does not have a token secret associated with it",
 				saRef.Name,
 				saRef.Namespace,
 			)
 		}
-		secretName := sa.Secrets[0].Name
-		secret, err := remoteSecretClient.GetSecret(ctx, client.ObjectKey{Name: secretName, Namespace: saRef.Namespace})
-		if err != nil {
-			return err
-		}
-
 		foundSecret = secret
+		if _, ok := foundSecret.Data[SecretTokenKey]; !ok {
+			return eris.Errorf(
+				"service account %s.%s has not populated its token secret",
+				saRef.Name,
+				saRef.Namespace,
+			)
+		}
 		return nil
 	}, SecretLookupOpts...); err != nil {
 		return "", SecretNotReady(err)

pkg/multicluster/register/registrant_test.go

@@ -283,7 +283,7 @@ var _ = Describe("Registrant", func() {
 			token := "hello"
 			saSecret := &k8s_core_types.Secret{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "sa-secret",
+					Name:      "sa-name-secret",
 					Namespace: namespace,
 				},
 				Data: map[string][]byte{
@@ -295,18 +295,12 @@ var _ = Describe("Registrant", func() {
 					Name:      sa.Name,
 					Namespace: sa.Namespace,
 				},
-				Secrets: []k8s_core_types.ObjectReference{
-					{
-						Namespace: namespace,
-						Name:      saSecret.GetName(),
-					},
-				},
 			}
 
 			saClient.EXPECT().
 				GetServiceAccount(ctx, sa).
 				Return(expectedSa, nil).
-				Times(2)
+				Times(1)
 
 			secretClient.EXPECT().
 				GetSecret(ctx, client.ObjectKey{