🐛 Stop writing zero values for spec.controlPlaneEndpoint to ControlPlane objects

internal/contract/controlplane.go

@@ -54,6 +54,33 @@ func (c *ControlPlaneContract) MachineTemplate() *ControlPlaneMachineTemplate {
 	return &ControlPlaneMachineTemplate{}
 }
 
+// IgnorePaths returns a list of paths to be ignored when reconciling an ControlPlane.
+// NOTE: The controlPlaneEndpoint struct currently contains two mandatory fields (host and port).
+// As the host and port fields are not using omitempty, they are automatically set to their zero values
+// if they are not set by the user. We don't want to reconcile the zero values as we would then overwrite
+// changes applied by the infrastructure provider controller.
+func (c *ControlPlaneContract) IgnorePaths(controlPlane *unstructured.Unstructured) ([]Path, error) {
+	var ignorePaths []Path
+
+	host, ok, err := unstructured.NestedString(controlPlane.UnstructuredContent(), ControlPlane().ControlPlaneEndpoint().host().Path()...)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to retrieve %s", ControlPlane().ControlPlaneEndpoint().host().Path().String())
+	}
+	if ok && host == "" {
+		ignorePaths = append(ignorePaths, ControlPlane().ControlPlaneEndpoint().host().Path())
+	}
+
+	port, ok, err := unstructured.NestedInt64(controlPlane.UnstructuredContent(), ControlPlane().ControlPlaneEndpoint().port().Path()...)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to retrieve %s", ControlPlane().ControlPlaneEndpoint().port().Path().String())
+	}
+	if ok && port == 0 {
+		ignorePaths = append(ignorePaths, ControlPlane().ControlPlaneEndpoint().port().Path())
+	}
+
+	return ignorePaths, nil
+}
+
 // Version provide access to version field in a ControlPlane object, if any.
 // NOTE: When working with unstructured there is no way to understand if the ControlPlane provider
 // do support a field in the type definition from the fact that a field is not set in a given instance.

internal/contract/controlplane_test.go

@@ -487,6 +487,169 @@ func TestControlPlane(t *testing.T) {
 	})
 }
 
+func TestControlPlaneEndpoints(t *testing.T) {
+	tests := []struct {
+		name         string
+		controlPlane *unstructured.Unstructured
+		want         []Path
+		expectErr    bool
+	}{
+		{
+			name: "No ignore paths when controlPlaneEndpoint is not set",
+			controlPlane: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"server": "1.2.3.4",
+					},
+				},
+			},
+			want: nil,
+		},
+		{
+			name: "No ignore paths when controlPlaneEndpoint is nil",
+			controlPlane: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"controlPlaneEndpoint": nil,
+					},
+				},
+			},
+
+			want: nil,
+		},
+		{
+			name: "No ignore paths when controlPlaneEndpoint is an empty object",
+			controlPlane: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"controlPlaneEndpoint": map[string]interface{}{},
+					},
+				},
+			},
+
+			want: nil,
+		},
+		{
+			name: "Don't ignore host when controlPlaneEndpoint.host is set",
+			controlPlane: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"controlPlaneEndpoint": map[string]interface{}{
+							"host": "example.com",
+						},
+					},
+				},
+			},
+			want: nil,
+		},
+		{
+			name: "Ignore host when controlPlaneEndpoint.host is set to its zero value",
+			controlPlane: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"controlPlaneEndpoint": map[string]interface{}{
+							"host": "",
+						},
+					},
+				},
+			},
+			want: []Path{
+				{"spec", "controlPlaneEndpoint", "host"},
+			},
+		},
+		{
+			name: "Don't ignore port when controlPlaneEndpoint.port is set",
+			controlPlane: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"controlPlaneEndpoint": map[string]interface{}{
+							"port": int64(6443),
+						},
+					},
+				},
+			},
+
+			want: nil,
+		},
+		{
+			name: "Ignore port when controlPlaneEndpoint.port is set to its zero value",
+			controlPlane: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"controlPlaneEndpoint": map[string]interface{}{
+							"port": int64(0),
+						},
+					},
+				},
+			},
+			want: []Path{
+				{"spec", "controlPlaneEndpoint", "port"},
+			},
+		},
+		{
+			name: "Ignore host and port when controlPlaneEndpoint host and port are set to their zero values",
+			controlPlane: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"controlPlaneEndpoint": map[string]interface{}{
+							"host": "",
+							"port": int64(0),
+						},
+					},
+				},
+			},
+			want: []Path{
+				{"spec", "controlPlaneEndpoint", "host"},
+				{"spec", "controlPlaneEndpoint", "port"},
+			},
+		},
+		{
+			name: "Ignore host when controlPlaneEndpoint host is to its zero values, even if port is set",
+			controlPlane: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"controlPlaneEndpoint": map[string]interface{}{
+							"host": "",
+							"port": int64(6443),
+						},
+					},
+				},
+			},
+			want: []Path{
+				{"spec", "controlPlaneEndpoint", "host"},
+			},
+		},
+		{
+			name: "Ignore port when controlPlaneEndpoint port is to its zero values, even if host is set",
+			controlPlane: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"controlPlaneEndpoint": map[string]interface{}{
+							"host": "example.com",
+							"port": int64(0),
+						},
+					},
+				},
+			},
+			want: []Path{
+				{"spec", "controlPlaneEndpoint", "port"},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			got, err := InfrastructureCluster().IgnorePaths(tt.controlPlane)
+			if tt.expectErr {
+				g.Expect(err).To(HaveOccurred())
+				return
+			}
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(got).To(Equal(tt.want))
+		})
+	}
+}
+
 func TestControlPlaneIsUpgrading(t *testing.T) {
 	tests := []struct {
 		name          string

internal/controllers/topology/cluster/reconcile_state.go

@@ -385,11 +385,17 @@ func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope)
 	// Create or update the ControlPlaneObject for the ControlPlaneState.
 	log := ctrl.LoggerFrom(ctx).WithValues(s.Desired.ControlPlane.Object.GetKind(), klog.KObj(s.Desired.ControlPlane.Object))
 	ctx = ctrl.LoggerInto(ctx, log)
+
+	ignorePaths, err := contract.ControlPlane().IgnorePaths(s.Desired.ControlPlane.Object)
+	if err != nil {
+		return false, errors.Wrap(err, "failed to calculate ignore paths")
+	}
 	created, err := r.reconcileReferencedObject(ctx, reconcileReferencedObjectInput{
 		cluster:       s.Current.Cluster,
 		current:       s.Current.ControlPlane.Object,
 		desired:       s.Desired.ControlPlane.Object,
 		versionGetter: contract.ControlPlane().Version().Get,
+		ignorePaths:   ignorePaths,
 	})
 	if err != nil {
 		// Best effort cleanup of the InfrastructureMachineTemplate (only on creation).