Repository: vmware-tanzu/tanzu-framework (language: go)

Issue:
Always populate kubernetesSpec.{*}.imageRepository fields in TKR_DATA values
<!-- This is used to request new product features -->

**Describe the feature request**

`imageRepository` fields in `kubernetesSpec.{etcd,pause,coredns,kube-vip}` can be empty. In this case, the clients should be defaulting to `kubernetesSpec.imageRepository`, BUT: it is not as easy as just having the data already in `kubernetesSpec.{*}.imageRepository`.

We would allow for simpler ClusterClass JSON patches, if we populate `kubernetesSpec.{*}.imageRepository` fields in TKR_DATA values.

**Describe alternative(s) you've considered**

Current situation: ClusterClass authors need to check for empty value and get the value from `kubernetesSpec.imageRepository`. Sometime they'd forget, and it'd cause of a bug.

**Affected product area (please put an X in all that apply)**

- (X) APIs
- ( ) Addons
- ( ) CLI
- ( ) Docs
- ( ) IAM
- ( ) Installation
- ( ) Plugin
- ( ) Security
- ( ) Test and Release
- ( ) User Experience
- (X) Developer Experience

**Additional context**



Discussion / hints:

Fix patch:
diff --git a/tkr/webhook/cluster/tkr-resolver/cluster/cluster.go b/tkr/webhook/cluster/tkr-resolver/cluster/cluster.go
index decebdb127..2e28e93e93 100644
--- a/tkr/webhook/cluster/tkr-resolver/cluster/cluster.go
+++ b/tkr/webhook/cluster/tkr-resolver/cluster/cluster.go
@@ -324,7 +324,7 @@ func tkrDataValue(customImageRepository string, tkr *runv1.TanzuKubernetesReleas
 	osimage.SetRefLabels(ls, osImage.Spec.Image.Type, osImage.Spec.Image.Ref)
 
 	return &TKRDataValue{
-		KubernetesSpec: *withCustomImageRepository(customImageRepository, &tkr.Spec.Kubernetes),
+		KubernetesSpec: *withCustomImageRepository(customImageRepository, defaultImageRepository(&tkr.Spec.Kubernetes)),
 		OSImageRef:     osImage.Spec.Image.Ref,
 		Labels:         ls,
 	}
@@ -348,6 +348,23 @@ func withCustomImageRepository(customImageRepository string, k8sSpec *runv1.Kube
 	return k8sSpec
 }
 
+func defaultImageRepository(k8sSpec *runv1.KubernetesSpec) *runv1.KubernetesSpec {
+	if k8sSpec.ImageRepository != "" {
+		k8sSpec = k8sSpec.DeepCopy()
+		for _, imageInfo := range []*runv1.ContainerImageInfo{
+			k8sSpec.CoreDNS,
+			k8sSpec.Etcd,
+			k8sSpec.Pause,
+			k8sSpec.KubeVIP,
+		} {
+			if imageInfo != nil && imageInfo.ImageRepository == "" {
+				imageInfo.ImageRepository = k8sSpec.ImageRepository
+			}
+		}
+	}
+	return k8sSpec
+}
+
 // getMap returns the map (creates it first if the map is nil). mp has to be a pointer to the variable holding the map,
 // so that we could save the newly created map.
 // Pre-reqs: mp != nil


Test patch:
diff --git a/tkr/webhook/cluster/tkr-resolver/cluster/cluster_test.go b/tkr/webhook/cluster/tkr-resolver/cluster/cluster_test.go
index 7f52c26aff..ebbe4c5159 100644
--- a/tkr/webhook/cluster/tkr-resolver/cluster/cluster_test.go
+++ b/tkr/webhook/cluster/tkr-resolver/cluster/cluster_test.go
@@ -418,6 +418,7 @@ var _ = Describe("cluster.Webhook", func() {
 					cluster.Spec.Topology = &clusterv1.Topology{}
 					cluster.Spec.Topology.Version = k8sVersionPrefix
 
+					customImageRepository = ""
 					if rand.Intn(2) != 0 {
 						customImageRepository = rand.String(10)
 					}
@@ -480,8 +481,19 @@ var _ = Describe("cluster.Webhook", func() {
 							Expect(topology.GetVariable(cluster, VarTKRData, &tkrData)).To(Succeed())
 							Expect(tkrData).ToNot(BeNil())
 							Expect(tkrData).To(HaveKey(tkr.Spec.Kubernetes.Version))
-							Expect(tkrData[tkr.Spec.Kubernetes.Version].Labels[runv1.LabelTKR]).To(Equal(tkr.Name))
-							Expect(tkrData[tkr.Spec.Kubernetes.Version].KubernetesSpec).To(Equal(*withCustomImageRepository(customImageRepository, &tkr.Spec.Kubernetes)))
+							dataValue := tkrData[tkr.Spec.Kubernetes.Version]
+							Expect(dataValue.Labels[runv1.LabelTKR]).To(Equal(tkr.Name))
+							Expect(dataValue.KubernetesSpec).To(Equal(*withCustomImageRepository(customImageRepository, defaultImageRepository(&tkr.Spec.Kubernetes))))
+							for _, imageInfo := range []*runv1.ContainerImageInfo{
+								dataValue.KubernetesSpec.Etcd,
+								dataValue.KubernetesSpec.Pause,
+								dataValue.KubernetesSpec.CoreDNS,
+								dataValue.KubernetesSpec.KubeVIP,
+							} {
+								if imageInfo != nil {
+									Expect(imageInfo.ImageRepository).ToNot(BeEmpty())
+								}
+							}
 						})
 					})
 				})