Merge pull request #3263 from ziyue-101/add_diff_update

Improve update method to avoid permanent diff

pkg/controller/direct/gkehub/diffs.go

@@ -0,0 +1,249 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package gkehub
+
+import (
+	"fmt"
+	"reflect"
+
+	krm "github.com/GoogleCloudPlatform/k8s-config-connector/apis/gkehub/v1beta1"
+	featureapi "google.golang.org/api/gkehub/v1beta"
+)
+
+func diffFeatureMembership(left *krm.GKEHubFeatureMembershipSpec, right *featureapi.MembershipFeatureSpec) []string {
+	var diffs []string
+	if left.Configmanagement != nil {
+		if right.Configmanagement == nil {
+			diffs = append(diffs, "configmanagement")
+		} else {
+			diffs = append(diffs, diffConfigManagement(left.Configmanagement, right.Configmanagement)...)
+		}
+	}
+	if left.Policycontroller != nil {
+		if right.Policycontroller == nil {
+			diffs = append(diffs, "policycontroller")
+		} else {
+			diffs = append(diffs, diffPolicycontroller(left.Policycontroller, right.Policycontroller)...)
+		}
+	}
+	if left.Mesh != nil {
+		if right.Mesh == nil {
+			diffs = append(diffs, "mesh")
+		} else {
+			diffs = append(diffs, diffMesh(left.Mesh, right.Mesh)...)
+		}
+	}
+	return diffs
+}
+
+func diffConfigManagement(left *krm.FeaturemembershipConfigmanagement, right *featureapi.ConfigManagementMembershipSpec) []string {
+	var diffs []string
+	if left.Binauthz != nil {
+		if right.Binauthz == nil {
+			diffs = append(diffs, "binauthz")
+		} else {
+			diffs = append(diffs, diffBinauthz(left.Binauthz, right.Binauthz)...)
+		}
+	}
+	if left.ConfigSync != nil {
+		if right.ConfigSync == nil {
+			diffs = append(diffs, "configsync")
+		} else {
+			diffs = append(diffs, diffConfigSync(left.ConfigSync, right.ConfigSync)...)
+		}
+	}
+	if left.HierarchyController != nil {
+		if right.HierarchyController == nil {
+			diffs = append(diffs, "hierachycontroller")
+		} else {
+			diffs = append(diffs, diffHierarchyController(left.HierarchyController, right.HierarchyController)...)
+		}
+	}
+	if left.PolicyController != nil {
+		if right.PolicyController == nil {
+			diffs = append(diffs, "policyController")
+		} else {
+			diffs = append(diffs, diffPolicyController(left.PolicyController, right.PolicyController)...)
+		}
+	}
+	if left.Version != nil && !reflect.DeepEqual(left.Version, right.Version) {
+		diffs = append(diffs, "version")
+	}
+	return diffs
+}
+
+func diffBinauthz(left *krm.FeaturemembershipBinauthz, right *featureapi.ConfigManagementBinauthzConfig) []string {
+	var diffs []string
+	if left.Enabled != nil && !reflect.DeepEqual(left.Enabled, right.Enabled) {
+		diffs = append(diffs, "enabled")
+	}
+	return diffs
+}
+
+func diffConfigSync(left *krm.FeaturemembershipConfigSync, right *featureapi.ConfigManagementConfigSync) []string {
+	var diffs []string
+	if left.Git != nil {
+		if right.Git == nil {
+			diffs = append(diffs, "git")
+		} else {
+			diffs = append(diffs, diffGit(left.Git, right.Git)...)
+		}
+	}
+	if left.MetricsGcpServiceAccountRef != nil && !reflect.DeepEqual(left.MetricsGcpServiceAccountRef.External, right.MetricsGcpServiceAccountEmail) {
+		diffs = append(diffs, "metricsGcpServiceAccountEmail")
+	}
+	if left.Oci != nil {
+		if right.Oci == nil {
+			diffs = append(diffs, "oci")
+		} else {
+			diffs = append(diffs, diffOci(left.Oci, right.Oci)...)
+		}
+	}
+	if left.PreventDrift != nil && !reflect.DeepEqual(left.PreventDrift, right.PreventDrift) {
+		diffs = append(diffs, "preventDrift")
+	}
+	if left.SourceFormat != nil && !reflect.DeepEqual(left.SourceFormat, right.SourceFormat) {
+		diffs = append(diffs, "sourceFormat")
+	}
+	return diffs
+}
+
+func diffGit(left *krm.FeaturemembershipGit, right *featureapi.ConfigManagementGitConfig) []string {
+	var diffs []string
+	if left.GcpServiceAccountRef != nil && !reflect.DeepEqual(left.GcpServiceAccountRef.External, right.GcpServiceAccountEmail) {
+		diffs = append(diffs, "gcpServiceAccountEmail")
+	}
+	if left.HttpsProxy != nil && !reflect.DeepEqual(left.HttpsProxy, right.HttpsProxy) {
+		diffs = append(diffs, "httpsProxy")
+	}
+	if left.PolicyDir != nil && !reflect.DeepEqual(left.PolicyDir, right.PolicyDir) {
+		diffs = append(diffs, "policyDir")
+	}
+	if left.SecretType != nil && !reflect.DeepEqual(left.SecretType, right.SecretType) {
+		diffs = append(diffs, "secretType")
+	}
+	if left.SyncBranch != nil && !reflect.DeepEqual(left.SyncBranch, right.SyncBranch) {
+		diffs = append(diffs, "syncBranch")
+	}
+	if left.SyncRepo != nil && !reflect.DeepEqual(left.SyncRepo, right.SyncRepo) {
+		diffs = append(diffs, "syncRepo")
+	}
+	if left.SyncRev != nil && !reflect.DeepEqual(left.SyncRev, right.SyncRev) {
+		diffs = append(diffs, "syncRev")
+	}
+	if left.SyncWaitSecs != nil && !reflect.DeepEqual(left.SyncWaitSecs, fmt.Sprintf("%d", right.SyncWaitSecs)) {
+		diffs = append(diffs, "syncWaitSecss")
+	}
+	return diffs
+}
+
+func diffOci(left *krm.FeaturemembershipOci, right *featureapi.ConfigManagementOciConfig) []string {
+	var diffs []string
+	if left.GcpServiceAccountRef != nil && !reflect.DeepEqual(left.GcpServiceAccountRef.External, right.GcpServiceAccountEmail) {
+		diffs = append(diffs, "gcpServiceAccountEmail")
+	}
+	if left.PolicyDir != nil && !reflect.DeepEqual(left.PolicyDir, right.PolicyDir) {
+		diffs = append(diffs, "policyDir")
+	}
+	if left.SecretType != nil && !reflect.DeepEqual(left.SecretType, right.SecretType) {
+		diffs = append(diffs, "secretType")
+	}
+	if left.SyncRepo != nil && !reflect.DeepEqual(left.SyncRepo, right.SyncRepo) {
+		diffs = append(diffs, "syncRepo")
+	}
+	if left.SyncWaitSecs != nil && !reflect.DeepEqual(left.SyncWaitSecs, fmt.Sprintf("%d", right.SyncWaitSecs)) {
+		diffs = append(diffs, "syncWaitSecs")
+	}
+	return diffs
+}
+
+func diffHierarchyController(left *krm.FeaturemembershipHierarchyController, right *featureapi.ConfigManagementHierarchyControllerConfig) []string {
+	var diffs []string
+	if left.EnableHierarchicalResourceQuota != nil && !reflect.DeepEqual(left.EnableHierarchicalResourceQuota, right.EnableHierarchicalResourceQuota) {
+		diffs = append(diffs, "enableHierarchicalResourceQuota")
+	}
+	if left.EnablePodTreeLabels != nil && !reflect.DeepEqual(left.EnablePodTreeLabels, right.EnablePodTreeLabels) {
+		diffs = append(diffs, "enablePodTreeLabels")
+	}
+	if left.Enabled != nil && !reflect.DeepEqual(left.Enabled, right.Enabled) {
+		diffs = append(diffs, "enabled")
+	}
+	return diffs
+}
+
+func diffPolicyController(left *krm.FeaturemembershipPolicyController, right *featureapi.ConfigManagementPolicyController) []string {
+	var diffs []string
+	if left.AuditIntervalSeconds != nil && !reflect.DeepEqual(left.AuditIntervalSeconds, fmt.Sprintf("%d", right.AuditIntervalSeconds)) {
+		diffs = append(diffs, "auditIntervalSeconds")
+	}
+	if left.Enabled != nil && !reflect.DeepEqual(left.Enabled, right.Enabled) {
+		diffs = append(diffs, "enabled")
+	}
+	if left.ExemptableNamespaces != nil && !reflect.DeepEqual(left.ExemptableNamespaces, right.ExemptableNamespaces) {
+		diffs = append(diffs, "exemptableNamespaces")
+	}
+	if left.LogDeniesEnabled != nil && !reflect.DeepEqual(left.LogDeniesEnabled, right.LogDeniesEnabled) {
+		diffs = append(diffs, "logDeniesEnabled")
+	}
+	if left.Monitoring != nil {
+		if right.Monitoring == nil {
+			diffs = append(diffs, "monitoring")
+		} else {
+			diffs = append(diffs, diffMonitoring(left.Monitoring, right.Monitoring)...)
+		}
+	}
+	if left.MutationEnabled != nil && !reflect.DeepEqual(left.MutationEnabled, right.MutationEnabled) {
+		diffs = append(diffs, "mutationEnabled")
+	}
+	if left.ReferentialRulesEnabled != nil && !reflect.DeepEqual(left.ReferentialRulesEnabled, right.ReferentialRulesEnabled) {
+		diffs = append(diffs, "referentialRulesEnabled")
+	}
+	if left.TemplateLibraryInstalled != nil && !reflect.DeepEqual(left.TemplateLibraryInstalled, right.TemplateLibraryInstalled) {
+		diffs = append(diffs, "templateLibraryInstalled")
+	}
+	return diffs
+}
+
+func diffMonitoring(left *krm.FeaturemembershipMonitoring, right *featureapi.ConfigManagementPolicyControllerMonitoring) []string {
+	var diffs []string
+	if left.Backends != nil && !reflect.DeepEqual(left.Backends, right.Backends) {
+		diffs = append(diffs, "backends")
+	}
+	return diffs
+}
+
+func diffMesh(left *krm.FeaturemembershipMesh, right *featureapi.ServiceMeshMembershipSpec) []string {
+	var diffs []string
+	if left.ControlPlane != nil && !reflect.DeepEqual(left.ControlPlane, right.ControlPlane) {
+		diffs = append(diffs, "controlPlane")
+	}
+	if left.Management != nil && !reflect.DeepEqual(left.Management, right.Management) {
+		diffs = append(diffs, "management")
+	}
+	return diffs
+}
+
+func diffPolicycontroller(left *krm.FeaturemembershipPolicycontroller, right *featureapi.PolicyControllerMembershipSpec) []string {
+	var diffs []string
+	if right.PolicyControllerHubConfig == nil {
+		diffs = append(diffs, "policyControllerHubConfig")
+	} else if !reflect.DeepEqual(left.PolicyControllerHubConfig, right.PolicyControllerHubConfig) {
+		diffs = append(diffs, "policyControllerHubConfig")
+	}
+	if left.Version != nil && !reflect.DeepEqual(left.Version, right.Version) {
+		diffs = append(diffs, "version")
+	}
+	return diffs
+}

pkg/controller/direct/gkehub/featuremembership_controller.go

@@ -17,7 +17,6 @@ package gkehub
 import (
 	"context"
 	"fmt"
-	"reflect"
 	"strings"
 	"time"
 
@@ -62,7 +61,7 @@ type gkeHubAdapter struct {
 	projectID    string
 	location     string
 
-	desired *featureapi.MembershipFeatureSpec
+	desired *krm.GKEHubFeatureMembership
 	actual  *featureapi.Feature
 
 	hubClient *gkeHubClient
@@ -108,16 +107,12 @@ func (m *gkeHubModel) AdapterForObject(ctx context.Context, reader client.Reader
 	if err := resolveIAMReferences(ctx, reader, obj); err != nil {
 		return nil, err
 	}
-	apiObj, err := featureMembershipSpecKRMtoMembershipFeatureSpecAPI(&obj.Spec)
-	if err != nil {
-		return nil, err
-	}
 	return &gkeHubAdapter{
 		membershipID: membership.id,
 		featureID:    feature.id,
 		projectID:    projectID,
 		location:     obj.Spec.Location,
-		desired:      apiObj,
+		desired:      obj,
 		hubClient:    hubClient,
 	}, nil
 }
@@ -179,7 +174,7 @@ func (a *gkeHubAdapter) Delete(ctx context.Context, deleteOp *directbase.DeleteO
 		return false, nil
 	}
 	// emptying the membershipspec is sufficient
-	a.desired = &featureapi.MembershipFeatureSpec{}
+	a.desired = &krm.GKEHubFeatureMembership{}
 	if _, err := a.patchMembershipSpec(ctx); err != nil {
 		return false, fmt.Errorf("deleting membershipspec for %s: %w", a.membershipID, err)
 	}
@@ -193,7 +188,11 @@ func (a *gkeHubAdapter) patchMembershipSpec(ctx context.Context) ([]byte, error)
 		mSpecs = make(map[string]featureapi.MembershipFeatureSpec)
 	}
 	// only change the feature configuration for the associated membership
-	mSpecs[a.membershipID] = *a.desired
+	desiredApiObj, err := featureMembershipSpecKRMtoMembershipFeatureSpecAPI(&a.desired.Spec)
+	if err != nil {
+		return nil, err
+	}
+	mSpecs[a.membershipID] = *desiredApiObj
 	feature.MembershipSpecs = mSpecs
 	op, err := a.hubClient.featureClient.Patch(a.featureID, feature).UpdateMask("membershipSpecs").Context(ctx).Do()
 	if err != nil {
@@ -249,7 +248,7 @@ func (a *gkeHubAdapter) Update(ctx context.Context, updateOp *directbase.UpdateO
 	log.V(2).Info("updating object", "u", u)
 	actual := a.actual.MembershipSpecs[a.membershipID]
 	//  There are no output fields in the api Object, so we can compare the desired and the actaul directly.
-	if !reflect.DeepEqual(a.desired.Configmanagement, actual.Configmanagement) || !reflect.DeepEqual(a.desired.Policycontroller, actual.Policycontroller) || !reflect.DeepEqual(a.desired.Mesh, actual.Mesh) {
+	if len(diffFeatureMembership(&a.desired.Spec, &actual)) != 0 {
 		log.V(2).Info("diff detected, patching gkehubfeaturemembership")
 		if _, err := a.patchMembershipSpec(ctx); err != nil {
 			return fmt.Errorf("patching gkehubfeaturemembership failed: %w", err)