From d09fdd735ed797d9c44f7cfb708fb46535ff9bed Mon Sep 17 00:00:00 2001 From: Bohdan Siryk Date: Fri, 23 Feb 2024 13:06:00 +0200 Subject: [PATCH] dcomprasion.StructsDiff was implemented --- .secrets.baseline | 12 +- apis/clusters/v1beta1/cadence_types.go | 2 +- apis/clusters/v1beta1/cassandra_types.go | 6 +- apis/clusters/v1beta1/kafka_types.go | 4 +- apis/clusters/v1beta1/kafkaconnect_types.go | 2 +- apis/clusters/v1beta1/opensearch_types.go | 4 +- apis/clusters/v1beta1/postgresql_types.go | 4 +- apis/clusters/v1beta1/redis_types.go | 4 +- apis/clusters/v1beta1/zookeeper_types.go | 3 - .../clusters.instaclustr.com_cadences.yaml | 3 +- .../clusters.instaclustr.com_cassandras.yaml | 2 - controllers/clusters/cassandra_controller.go | 7 - controllers/clusters/helpers.go | 74 +-- .../clusters/kafkaconnect_controller.go | 7 - controllers/clusters/opensearch_controller.go | 7 - controllers/clusters/postgresql_controller.go | 7 - controllers/clusters/redis_controller.go | 7 - controllers/clusters/zookeeper_controller.go | 7 - pkg/utils/dcomparison/map_diff.go | 7 +- pkg/utils/dcomparison/struct_diff.go | 229 +++++++++ pkg/utils/dcomparison/struct_diff_test.go | 445 ++++++++++++++++++ 21 files changed, 713 insertions(+), 130 deletions(-) create mode 100644 pkg/utils/dcomparison/struct_diff.go create mode 100644 pkg/utils/dcomparison/struct_diff_test.go diff --git a/.secrets.baseline b/.secrets.baseline index 70a51c546..0559fadd6 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -169,14 +169,14 @@ "filename": "apis/clusters/v1beta1/cadence_types.go", "hashed_secret": "a242f4a16b957f7ff99eb24e189e94d270d2348b", "is_verified": false, - "line_number": 291 + "line_number": 292 }, { "type": "Secret Keyword", "filename": "apis/clusters/v1beta1/cadence_types.go", "hashed_secret": "a57ce131bd944bdf8ba2f2f93e179dc416ed0315", "is_verified": false, - "line_number": 300 + "line_number": 301 } ], "apis/clusters/v1beta1/cassandra_types.go": [ @@ -319,21 +319,21 @@ "filename": "apis/clusters/v1beta1/postgresql_types.go", "hashed_secret": "5ffe533b830f08a0326348a9160afafc8ada44db", "is_verified": false, - "line_number": 355 + "line_number": 354 }, { "type": "Secret Keyword", "filename": "apis/clusters/v1beta1/postgresql_types.go", "hashed_secret": "a3d7d4a96d18c8fc5a1cf9c9c01c45b4690b4008", "is_verified": false, - "line_number": 361 + "line_number": 360 }, { "type": "Secret Keyword", "filename": "apis/clusters/v1beta1/postgresql_types.go", "hashed_secret": "a57ce131bd944bdf8ba2f2f93e179dc416ed0315", "is_verified": false, - "line_number": 481 + "line_number": 480 } ], "apis/clusters/v1beta1/redis_types.go": [ @@ -1146,5 +1146,5 @@ } ] }, - "generated_at": "2024-02-21T14:33:43Z" + "generated_at": "2024-02-26T10:23:28Z" } diff --git a/apis/clusters/v1beta1/cadence_types.go b/apis/clusters/v1beta1/cadence_types.go index 1ef9cf7df..2591faf80 100644 --- a/apis/clusters/v1beta1/cadence_types.go +++ b/apis/clusters/v1beta1/cadence_types.go @@ -86,7 +86,7 @@ type CadenceSpec struct { //+kubebuilder:validation:MaxItems:=1 TargetPrimaryCadence []*CadenceDependencyTarget `json:"targetPrimaryCadence,omitempty"` - ResizeSettings GenericResizeSettings `json:"resizeSettings,omitempty"` + ResizeSettings GenericResizeSettings `json:"resizeSettings,omitempty" dcomparisonSkip:"true"` UseCadenceWebAuth bool `json:"useCadenceWebAuth"` UseHTTPAPI bool `json:"useHttpApi,omitempty"` diff --git a/apis/clusters/v1beta1/cassandra_types.go b/apis/clusters/v1beta1/cassandra_types.go index ed52531f6..354595402 100644 --- a/apis/clusters/v1beta1/cassandra_types.go +++ b/apis/clusters/v1beta1/cassandra_types.go @@ -59,8 +59,8 @@ type CassandraSpec struct { PasswordAndUserAuth bool `json:"passwordAndUserAuth,omitempty"` BundledUseOnly bool `json:"bundledUseOnly,omitempty"` PCICompliance bool `json:"pciCompliance,omitempty"` - UserRefs References `json:"userRefs,omitempty"` - ResizeSettings GenericResizeSettings `json:"resizeSettings,omitempty"` + UserRefs References `json:"userRefs,omitempty" dcomparisonSkip:"true"` + ResizeSettings GenericResizeSettings `json:"resizeSettings,omitempty" dcomparisonSkip:"true"` } // CassandraStatus defines the observed state of Cassandra @@ -171,7 +171,7 @@ type DebeziumCassandraSpec struct { KafkaVPCType string `json:"kafkaVpcType"` KafkaTopicPrefix string `json:"kafkaTopicPrefix"` KafkaDataCentreID string `json:"kafkaCdcId,omitempty"` - ClusterRef *clusterresourcesv1beta1.ClusterRef `json:"clusterRef,omitempty"` + ClusterRef *clusterresourcesv1beta1.ClusterRef `json:"clusterRef,omitempty" dcomparisonSkip:"true"` Version string `json:"version"` } diff --git a/apis/clusters/v1beta1/kafka_types.go b/apis/clusters/v1beta1/kafka_types.go index c9a134e86..9b42c0b57 100644 --- a/apis/clusters/v1beta1/kafka_types.go +++ b/apis/clusters/v1beta1/kafka_types.go @@ -73,7 +73,7 @@ type KafkaSpec struct { ClientBrokerAuthWithMTLS bool `json:"clientBrokerAuthWithMtls,omitempty"` BundledUseOnly bool `json:"bundledUseOnly,omitempty"` PCICompliance bool `json:"pciCompliance,omitempty"` - UserRefs References `json:"userRefs,omitempty"` + UserRefs References `json:"userRefs,omitempty" dcomparisonSkip:"true"` // Provision additional dedicated nodes for Apache Zookeeper to run on. // Zookeeper nodes will be co-located with Kafka if this is not provided @@ -86,7 +86,7 @@ type KafkaSpec struct { KarapaceRestProxy []*KarapaceRestProxy `json:"karapaceRestProxy,omitempty"` KarapaceSchemaRegistry []*KarapaceSchemaRegistry `json:"karapaceSchemaRegistry,omitempty"` Kraft []*Kraft `json:"kraft,omitempty"` - ResizeSettings GenericResizeSettings `json:"resizeSettings,omitempty"` + ResizeSettings GenericResizeSettings `json:"resizeSettings,omitempty" dcomparisonSkip:"true"` } type Kraft struct { diff --git a/apis/clusters/v1beta1/kafkaconnect_types.go b/apis/clusters/v1beta1/kafkaconnect_types.go index 3a7b2b0a7..5610ce421 100644 --- a/apis/clusters/v1beta1/kafkaconnect_types.go +++ b/apis/clusters/v1beta1/kafkaconnect_types.go @@ -50,7 +50,7 @@ type ExternalCluster struct { type ManagedCluster struct { TargetKafkaClusterID string `json:"targetKafkaClusterId,omitempty"` - ClusterRef *clusterresource.ClusterRef `json:"clusterRef,omitempty"` + ClusterRef *clusterresource.ClusterRef `json:"clusterRef,omitempty" dcomparisonSkip:"true"` // Available options are KAFKA_VPC, VPC_PEERED, SEPARATE_VPC KafkaConnectVPCType string `json:"kafkaConnectVpcType"` diff --git a/apis/clusters/v1beta1/opensearch_types.go b/apis/clusters/v1beta1/opensearch_types.go index 4c482b047..97e83b317 100644 --- a/apis/clusters/v1beta1/opensearch_types.go +++ b/apis/clusters/v1beta1/opensearch_types.go @@ -50,9 +50,9 @@ type OpenSearchSpec struct { AlertingPlugin bool `json:"alertingPlugin,omitempty"` BundledUseOnly bool `json:"bundledUseOnly,omitempty"` PCICompliance bool `json:"pciCompliance,omitempty"` - UserRefs References `json:"userRefs,omitempty"` + UserRefs References `json:"userRefs,omitempty" dcomparisonSkip:"true"` //+kubuilder:validation:MaxItems:=1 - ResizeSettings []*ResizeSettings `json:"resizeSettings,omitempty"` + ResizeSettings []*ResizeSettings `json:"resizeSettings,omitempty" dcomparisonSkip:"true"` //+kubuilder:validation:MaxItems:=1 IngestNodes []*OpenSearchIngestNodes `json:"ingestNodes,omitempty"` } diff --git a/apis/clusters/v1beta1/postgresql_types.go b/apis/clusters/v1beta1/postgresql_types.go index 035f6df5b..6d67a628a 100644 --- a/apis/clusters/v1beta1/postgresql_types.go +++ b/apis/clusters/v1beta1/postgresql_types.go @@ -77,9 +77,9 @@ type PgSpec struct { DataCentres []*PgDataCentre `json:"dataCentres,omitempty"` ClusterConfigurations map[string]string `json:"clusterConfigurations,omitempty"` SynchronousModeStrict bool `json:"synchronousModeStrict,omitempty"` - UserRefs []*Reference `json:"userRefs,omitempty"` + UserRefs []*Reference `json:"userRefs,omitempty" dcomparisonSkip:"true"` //+kubebuilder:validate:MaxItems:=1 - ResizeSettings []*ResizeSettings `json:"resizeSettings,omitempty"` + ResizeSettings []*ResizeSettings `json:"resizeSettings,omitempty" dcomparisonSkip:"true"` // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="extensions cannot be changed after it is set" Extensions PgExtensions `json:"extensions,omitempty"` } diff --git a/apis/clusters/v1beta1/redis_types.go b/apis/clusters/v1beta1/redis_types.go index 15d2dd0ee..c3828e765 100644 --- a/apis/clusters/v1beta1/redis_types.go +++ b/apis/clusters/v1beta1/redis_types.go @@ -78,8 +78,8 @@ type RedisSpec struct { //+kubebuilder:validation:MaxItems:=2 DataCentres []*RedisDataCentre `json:"dataCentres"` - ResizeSettings GenericResizeSettings `json:"resizeSettings,omitempty"` - UserRefs References `json:"userRefs,omitempty"` + ResizeSettings GenericResizeSettings `json:"resizeSettings,omitempty" dcomparisonSkip:"true"` + UserRefs References `json:"userRefs,omitempty" dcomparisonSkip:"true"` } type RedisDataCentreStatus struct { diff --git a/apis/clusters/v1beta1/zookeeper_types.go b/apis/clusters/v1beta1/zookeeper_types.go index 95cc39d9b..2dece32a9 100644 --- a/apis/clusters/v1beta1/zookeeper_types.go +++ b/apis/clusters/v1beta1/zookeeper_types.go @@ -42,9 +42,6 @@ type ZookeeperSpec struct { // ZookeeperStatus defines the observed state of Zookeeper type ZookeeperStatus struct { - // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster - // Important: Run "make" to regenerate code after modifying this file - ClusterStatus `json:",inline"` DefaultUserSecretRef *Reference `json:"defaultUserSecretRef,omitempty"` } diff --git a/config/crd/bases/clusters.instaclustr.com_cadences.yaml b/config/crd/bases/clusters.instaclustr.com_cadences.yaml index a329f66ad..24f8c18ae 100644 --- a/config/crd/bases/clusters.instaclustr.com_cadences.yaml +++ b/config/crd/bases/clusters.instaclustr.com_cadences.yaml @@ -282,7 +282,7 @@ spec: type: object maxItems: 1 type: array - pciComplianceMode: + pciCompliance: type: boolean privateNetwork: type: boolean @@ -393,7 +393,6 @@ spec: type: string required: - dataCentres - - pciComplianceMode - useCadenceWebAuth type: object status: diff --git a/config/crd/bases/clusters.instaclustr.com_cassandras.yaml b/config/crd/bases/clusters.instaclustr.com_cassandras.yaml index e18d85ced..5ef899082 100644 --- a/config/crd/bases/clusters.instaclustr.com_cassandras.yaml +++ b/config/crd/bases/clusters.instaclustr.com_cassandras.yaml @@ -342,8 +342,6 @@ spec: type: array version: type: string - required: - - pciCompliance type: object status: description: CassandraStatus defines the observed state of Cassandra diff --git a/controllers/clusters/cassandra_controller.go b/controllers/clusters/cassandra_controller.go index 24d06faaf..cef870773 100644 --- a/controllers/clusters/cassandra_controller.go +++ b/controllers/clusters/cassandra_controller.go @@ -717,13 +717,6 @@ func (r *CassandraReconciler) newSyncJob(c *v1beta1.Cassandra) scheduler.Job { } else if c.Status.CurrentClusterOperationStatus == models.NoOperation && c.Annotations[models.ResourceStateAnnotation] != models.UpdatingEvent && !equals { - k8sData, err := removeRedundantFieldsFromSpec(c.Spec, "userRefs") - if err != nil { - l.Error(err, "Cannot remove redundant fields from k8s Spec") - return err - } - - l.Info(msgExternalChanges, "instaclustr data", iCassandra.Spec, "k8s resource spec", string(k8sData)) patch := c.NewPatch() c.Annotations[models.ExternalChangesAnnotation] = models.True diff --git a/controllers/clusters/helpers.go b/controllers/clusters/helpers.go index d093b990a..a6d88ac51 100644 --- a/controllers/clusters/helpers.go +++ b/controllers/clusters/helpers.go @@ -21,7 +21,6 @@ import ( "encoding/json" "fmt" "sort" - "strings" "github.com/go-logr/logr" "github.com/hashicorp/go-version" @@ -169,74 +168,33 @@ func getSortedAppVersions(versions []*models.AppVersions, appType string) []*ver return nil } -func removeRedundantFieldsFromSpec(k8sSpec any, ignoreFields ...string) ([]byte, error) { - k8sSpecJson, err := json.Marshal(k8sSpec) - if err != nil { - return nil, err - } - - if len(ignoreFields) == 0 { - return k8sSpecJson, nil - } - - k8sSpecMap := map[string]any{} - err = json.Unmarshal(k8sSpecJson, &k8sSpecMap) - - if err != nil { - return nil, err - } - - for _, field := range ignoreFields { - delete(k8sSpecMap, field) - } - - k8sSpecJson, err = json.Marshal(k8sSpecMap) - if err != nil { - return nil, err - } - return k8sSpecJson, nil +type objectDiff struct { + Field string `json:"field"` + K8sValue any `json:"k8sValue"` + InstaclustrValue any `json:"instaclustrValue"` } -func createSpecDifferenceMessage(k8sSpec, iSpec any) (string, error) { - k8sData, err := removeRedundantFieldsFromSpec(k8sSpec, "userRefs") +func createSpecDifferenceMessage[T any](k8sSpec, iSpec T) (string, error) { + diffs, err := dcomparison.StructsDiff(models.SpecPath, k8sSpec, iSpec) if err != nil { - return "", err + return "", fmt.Errorf("failed to create spec difference message, err: %w", err) } - iData, err := json.Marshal(iSpec) - if err != nil { - return "", err - } - - var k8sSpecMap map[string]any - err = json.Unmarshal(k8sData, &k8sSpecMap) - if err != nil { - return "", err + objectDiffs := make([]objectDiff, 0, len(diffs)) + for _, diff := range diffs { + objectDiffs = append(objectDiffs, objectDiff{ + Field: diff.Field, + K8sValue: diff.Value1, + InstaclustrValue: diff.Value2, + }) } - var iSpecMap map[string]any - err = json.Unmarshal(iData, &iSpecMap) + b, err := json.Marshal(objectDiffs) if err != nil { return "", err } - diffs := dcomparison.MapsDiff(models.SpecPath, k8sSpecMap, iSpecMap) - - return fmt.Sprintf("%s Diffs: %s", models.ExternalChangesBaseMessage, prepareDiffMessage(diffs)), nil -} - -func prepareDiffMessage(diffs dcomparison.ObjectDiffs) string { - var diffMessages []string - for _, diff := range diffs { - diffMessages = append(diffMessages, fmt.Sprintf( - "{field: %s, k8sValue: %v, instaclustrValue: %v}", - diff.Field, - diff.Value1, - diff.Value2, - )) - } - - return strings.Join(diffMessages, ", ") + return fmt.Sprintf("%s Diffs: %s", models.ExternalChangesBaseMessage, b), nil } var msgDeleteClusterWithTwoFactorDelete = "Please confirm cluster deletion via email or phone. " + diff --git a/controllers/clusters/kafkaconnect_controller.go b/controllers/clusters/kafkaconnect_controller.go index 4bbaa86ed..87985dd09 100644 --- a/controllers/clusters/kafkaconnect_controller.go +++ b/controllers/clusters/kafkaconnect_controller.go @@ -555,13 +555,6 @@ func (r *KafkaConnectReconciler) newSyncJob(kc *v1beta1.KafkaConnect) scheduler. } else if kc.Status.CurrentClusterOperationStatus == models.NoOperation && kc.Annotations[models.ResourceStateAnnotation] != models.UpdatingEvent && !equals { - k8sData, err := removeRedundantFieldsFromSpec(kc.Spec, "userRefs") - if err != nil { - l.Error(err, "Cannot remove redundant fields from k8s Spec") - return err - } - - l.Info(msgExternalChanges, "instaclustr data", iKC.Spec, "k8s resource spec", string(k8sData)) patch := kc.NewPatch() kc.Annotations[models.ExternalChangesAnnotation] = models.True diff --git a/controllers/clusters/opensearch_controller.go b/controllers/clusters/opensearch_controller.go index 87c09eaf7..696294cb9 100644 --- a/controllers/clusters/opensearch_controller.go +++ b/controllers/clusters/opensearch_controller.go @@ -638,13 +638,6 @@ func (r *OpenSearchReconciler) newSyncJob(o *v1beta1.OpenSearch) scheduler.Job { } else if o.Status.CurrentClusterOperationStatus == models.NoOperation && o.Annotations[models.ResourceStateAnnotation] != models.UpdatingEvent && !equals { - k8sData, err := removeRedundantFieldsFromSpec(o.Spec, "userRefs") - if err != nil { - l.Error(err, "Cannot remove redundant fields from k8s Spec") - return err - } - - l.Info(msgExternalChanges, "instaclustr data", iO.Spec, "k8s resource spec", string(k8sData)) patch := o.NewPatch() o.Annotations[models.ExternalChangesAnnotation] = models.True diff --git a/controllers/clusters/postgresql_controller.go b/controllers/clusters/postgresql_controller.go index b63956d01..6d2c50b73 100644 --- a/controllers/clusters/postgresql_controller.go +++ b/controllers/clusters/postgresql_controller.go @@ -840,13 +840,6 @@ func (r *PostgreSQLReconciler) newWatchStatusJob(pg *v1beta1.PostgreSQL) schedul } else if pg.Status.CurrentClusterOperationStatus == models.NoOperation && pg.Annotations[models.ResourceStateAnnotation] != models.UpdatingEvent && !equals { - k8sData, err := removeRedundantFieldsFromSpec(pg.Spec, "userRefs") - if err != nil { - l.Error(err, "Cannot remove redundant fields from k8s Spec") - return err - } - - l.Info(msgExternalChanges, "instaclustr data", iPg.Spec, "k8s resource spec", string(k8sData)) patch := pg.NewPatch() pg.Annotations[models.ExternalChangesAnnotation] = models.True diff --git a/controllers/clusters/redis_controller.go b/controllers/clusters/redis_controller.go index b70c051d2..89eda25db 100644 --- a/controllers/clusters/redis_controller.go +++ b/controllers/clusters/redis_controller.go @@ -712,13 +712,6 @@ func (r *RedisReconciler) newSyncJob(redis *v1beta1.Redis) scheduler.Job { } else if redis.Status.CurrentClusterOperationStatus == models.NoOperation && redis.Annotations[models.ResourceStateAnnotation] != models.UpdatingEvent && !equals { - k8sData, err := removeRedundantFieldsFromSpec(redis.Spec, "userRefs") - if err != nil { - l.Error(err, "Cannot remove redundant fields from k8s Spec") - return err - } - - l.Info(msgExternalChanges, "instaclustr data", iRedis.Spec, "k8s resource spec", string(k8sData)) patch := redis.NewPatch() redis.Annotations[models.ExternalChangesAnnotation] = models.True diff --git a/controllers/clusters/zookeeper_controller.go b/controllers/clusters/zookeeper_controller.go index 38a892a9a..258edc559 100644 --- a/controllers/clusters/zookeeper_controller.go +++ b/controllers/clusters/zookeeper_controller.go @@ -505,13 +505,6 @@ func (r *ZookeeperReconciler) newWatchStatusJob(zook *v1beta1.Zookeeper) schedul } else if zook.Status.CurrentClusterOperationStatus == models.NoOperation && zook.Annotations[models.ResourceStateAnnotation] != models.UpdatingEvent && !equals { - k8sData, err := removeRedundantFieldsFromSpec(zook.Spec, "userRefs") - if err != nil { - l.Error(err, "Cannot remove redundant fields from k8s Spec") - return err - } - - l.Info(msgExternalChanges, "instaclustr data", iZook.Spec, "k8s resource spec", string(k8sData)) patch := zook.NewPatch() zook.Annotations[models.ExternalChangesAnnotation] = models.True diff --git a/pkg/utils/dcomparison/map_diff.go b/pkg/utils/dcomparison/map_diff.go index 589e6cd91..61cd9c6ad 100644 --- a/pkg/utils/dcomparison/map_diff.go +++ b/pkg/utils/dcomparison/map_diff.go @@ -14,10 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package dcomparison provides a solution for deeply comparing two maps, -// including their nested maps and slices. It is designed to identify differences -// between two maps that can contain a variety of data types, such as strings, -// integers, other maps, and slices. +// Package dcomparison provides a solution for deeply comparing two objects (struct, maps). +// It is designed to identify differences between two objects that may contain a variety of +// data types, such as strings, integers, other maps, and slices. package dcomparison import ( diff --git a/pkg/utils/dcomparison/struct_diff.go b/pkg/utils/dcomparison/struct_diff.go new file mode 100644 index 000000000..7162db071 --- /dev/null +++ b/pkg/utils/dcomparison/struct_diff.go @@ -0,0 +1,229 @@ +/* +Copyright 2024. + +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 dcomparison + +import ( + "fmt" + "reflect" + "strings" +) + +func StructsDiff[T any](path string, obj1, obj2 T) (ObjectDiffs, error) { + val1 := reflect.ValueOf(obj1) + val2 := reflect.ValueOf(obj2) + + cmp := structsComparer{} + + if !cmp.isStruct(val1.Type()) { + return nil, fmt.Errorf("expected struct, got: %s", val1.Kind()) + } + + if val1.Kind() == reflect.Ptr { + val1 = val1.Elem() + val2 = val2.Elem() + } + + cmp.compare(val1, val2, path) + return cmp.diffs, nil +} + +const ( + skipTag = "dcomparisonSkip" + skipValue = "true" + + jsonTag = "json" + jsonInlineConstraint = ",inline" +) + +type structsComparer struct { + diffs ObjectDiffs +} + +func (s *structsComparer) compare(obj1, obj2 reflect.Value, path string) { + switch obj1.Kind() { + case reflect.Ptr: + s.comparePtrs(obj1, obj2, path) + case reflect.Struct: + s.compareStructs(obj1, obj2, path) + case reflect.Slice, reflect.Array: + s.compareSlicesOrArrays(obj1, obj2, path) + case reflect.Map: + s.compareMaps(obj1, obj2, path) + default: + val1 := s.getInterfaceValueIfValid(obj1) + val2 := s.getInterfaceValueIfValid(obj2) + + if val1 != val2 { + s.diffs.Append(ObjectDiff{ + Field: path, + Value1: val1, + Value2: val2, + }) + } + } +} + +func (s *structsComparer) isStruct(t reflect.Type) bool { + return t.Kind() == reflect.Struct || t.Kind() == reflect.Ptr && t.Elem().Kind() == reflect.Struct +} + +func (s *structsComparer) comparePtrs(obj1, obj2 reflect.Value, subpath string) { + switch { + case obj1.IsValid() && obj2.IsValid(): + s.compare(obj1.Elem(), obj2.Elem(), subpath) + + case obj1.IsZero() && obj2.IsZero(): + return + + default: + s.diffs.Append(ObjectDiff{ + Field: subpath, + Value1: s.getElemIfPtrOrInterface(obj1), + Value2: s.getElemIfPtrOrInterface(obj2), + }) + } +} + +func (s *structsComparer) compareStructs(obj1, obj2 reflect.Value, path string) { + n := obj1.NumField() + for i := 0; i < n; i++ { + field1 := obj1.Type().Field(i) + + if s.shouldSkip(field1) { + continue + } + + // If there is an embedded struct with the json `,inline` constraint: + // type s struct { + // EmbeddedStruct `json:",inline" + // } + // We should skip adding the name of the EmbeddedStruct to sub path + subPath := path + if !s.hasJSONInlineConstraint(field1) { + subPath += "." + s.getFieldName(field1) + } + + s.compare(obj1.Field(i), obj2.Field(i), subPath) + } +} + +// shouldSkip indicates should the field be skipped during comparing. +// It is skipped only when: +// 1. field is not exported +// 2. field has a skipTag +// 3. value of the skipTag == skipValue +func (s *structsComparer) shouldSkip(field reflect.StructField) bool { + if !field.IsExported() { + return true + } + + val, has := field.Tag.Lookup(skipTag) + return has && val == skipValue +} + +func (s *structsComparer) getJsonFieldName(tag reflect.StructTag) string { + val, has := tag.Lookup(jsonTag) + if !has { + return "" + } + + return strings.Split(val, ",")[0] +} + +func (s *structsComparer) getFieldName(field reflect.StructField) string { + fieldName := s.getJsonFieldName(field.Tag) + if fieldName == "" || fieldName == "-" { + // If there is no json tag use the name of field directly + fieldName = field.Name + } + + return fieldName +} + +func (s *structsComparer) compareSlicesOrArrays(slice1, slice2 reflect.Value, path string) { + maxLen := max(slice1.Len(), slice2.Len()) + for i := 0; i < maxLen; i++ { + val1, val2 := s.getSliceElement(slice1, i), s.getSliceElement(slice2, i) + subPath := fmt.Sprintf("%s[%d]", path, i) + s.compare(val1, val2, subPath) + } +} + +func (s *structsComparer) getSliceElement(slice reflect.Value, i int) reflect.Value { + if i < slice.Len() { + return slice.Index(i) + } + + return reflect.ValueOf(nil) +} + +func (s *structsComparer) compareMaps(map1, map2 reflect.Value, path string) { + for _, key := range map1.MapKeys() { + val1 := map1.MapIndex(key) + val2 := map2.MapIndex(key) + + subPath := fmt.Sprintf("%s[%v]", path, key.Interface()) + + if val2.IsValid() { + s.compare(val1, val2, subPath) + } else { + s.diffs.Append(ObjectDiff{ + Field: subPath, + Value1: val1.Interface(), + Value2: nil, + }) + } + } + + for _, key := range map2.MapKeys() { + subPath := fmt.Sprintf("%s[%v]", path, key.Interface()) + if !map1.MapIndex(key).IsValid() { + s.diffs.Append(ObjectDiff{ + Field: subPath, + Value1: nil, + Value2: map2.MapIndex(key).Interface(), + }) + } + } +} + +func (s *structsComparer) getInterfaceValueIfValid(value reflect.Value) any { + if value.IsValid() { + return value.Interface() + } + + return nil +} + +func (s *structsComparer) getElemIfPtrOrInterface(value reflect.Value) any { + if !value.IsValid() { + return nil + } + + switch value.Kind() { + case reflect.Pointer, reflect.Interface: + return value.Elem().Interface() + default: + return value.Interface() + } +} + +func (s *structsComparer) hasJSONInlineConstraint(field reflect.StructField) bool { + val, has := field.Tag.Lookup(jsonTag) + + return has && strings.Contains(val, jsonInlineConstraint) +} diff --git a/pkg/utils/dcomparison/struct_diff_test.go b/pkg/utils/dcomparison/struct_diff_test.go new file mode 100644 index 000000000..3abf8ed92 --- /dev/null +++ b/pkg/utils/dcomparison/struct_diff_test.go @@ -0,0 +1,445 @@ +package dcomparison + +import ( + "reflect" + "testing" +) + +type unexportedField struct { + unexported string +} + +type intFieldWithSkipTrue struct { + Int int `dcomparisonSkip:"true"` +} + +type intFieldWithSkipFalse struct { + Int int `dcomparisonSkip:"false"` +} + +type stringFieldWithJsonTag struct { + String string `json:"string"` +} + +type intSlice struct { + Slice []int `json:"slice"` +} + +type stringPointer struct { + StringPointer *string `json:"stringPointer"` +} + +type structPointer struct { + Ptr *stringFieldWithJsonTag `json:"ptr"` +} + +type structField struct { + Field stringFieldWithJsonTag `json:"field"` +} + +type sliceOfPointers struct { + Slice []*stringFieldWithJsonTag `json:"slice"` +} + +type mapStringToInt struct { + Map map[string]int `json:"map"` +} + +type mapStringToStructPtr struct { + Map map[string]*stringFieldWithJsonTag `json:"map"` +} + +type ExportedStruct struct { + Field string `json:"field"` +} + +type structWithEmbeddedStructWithJSONInlineConstraint struct { + ExportedStruct `json:",inline"` +} + +func ptrOf[T any](v T) *T { + return &v +} + +func TestStructDiff(t *testing.T) { + type args struct { + obj1 any + obj2 any + } + type testCase struct { + name string + args args + want ObjectDiffs + wantErr bool + } + + tests := []testCase{ + { + name: "non-struct type", + args: args{obj1: 1, obj2: 1}, + wantErr: true, + }, + { + name: "structs with unexported field, should be skipped", + args: args{ + obj1: unexportedField{unexported: "unexported"}, + obj2: nil, + }, + }, + { + name: "structs with exported int field, same values", + args: args{ + obj1: struct { + Exported int + }{1}, + obj2: struct { + Exported int + }{1}, + }, + want: nil, + }, + { + name: "structs with exported int field, different values", + args: args{ + obj1: struct { + Int int + }{1}, + obj2: struct { + Int int + }{2}, + }, + want: ObjectDiffs{ + {Field: "spec.Int", Value1: 1, Value2: 2}, + }, + }, + { + name: "structs with exported int field, same values, skip", + args: args{ + obj1: intFieldWithSkipTrue{1}, + obj2: intFieldWithSkipTrue{1}, + }, + }, + { + name: "structs with exported int field, different values, skip", + args: args{ + obj1: intFieldWithSkipTrue{1}, + obj2: intFieldWithSkipTrue{2}, + }, + }, + { + name: "structs with exported int field, same values, has skip tag but do not skip", + args: args{ + obj1: intFieldWithSkipFalse{1}, + obj2: intFieldWithSkipFalse{1}, + }, + }, + { + name: "structs with exported int field, different values, has skip tag but do not skip", + args: args{ + obj1: intFieldWithSkipFalse{2}, + obj2: intFieldWithSkipFalse{1}, + }, + want: ObjectDiffs{ + {Field: "spec.Int", Value1: 2, Value2: 1}, + }, + }, + { + name: "exported string field with json tag, same values, do not skip", + args: args{ + obj1: stringFieldWithJsonTag{"test"}, + obj2: stringFieldWithJsonTag{"test"}, + }, + }, + { + name: "exported string field with json tag, diff values, do not skip", + args: args{ + obj1: stringFieldWithJsonTag{"test1"}, + obj2: stringFieldWithJsonTag{"test2"}, + }, + want: ObjectDiffs{ + {Field: "spec.string", Value1: "test1", Value2: "test2"}, + }, + }, + { + name: "nil slices", + args: args{ + obj1: intSlice{Slice: nil}, + obj2: intSlice{Slice: nil}, + }, + }, + { + name: "empty slices", + args: args{ + obj1: intSlice{Slice: []int{}}, + obj2: intSlice{Slice: []int{}}, + }, + }, + { + name: "slices with different length", + args: args{ + obj1: intSlice{Slice: []int{1, 1, 1}}, + obj2: intSlice{Slice: []int{1, 1}}, + }, + want: ObjectDiffs{ + {Field: "spec.slice[2]", Value1: 1, Value2: nil}, + }, + }, + { + name: "slices same length, different values", + args: args{ + obj1: intSlice{Slice: []int{1, 2, 3}}, + obj2: intSlice{Slice: []int{3, 2, 1}}, + }, + want: ObjectDiffs{ + {Field: "spec.slice[0]", Value1: 1, Value2: 3}, + {Field: "spec.slice[2]", Value1: 3, Value2: 1}, + }, + }, + { + name: "structs with pointer to string, nil pointer, equal", + args: args{ + obj1: stringPointer{StringPointer: nil}, + obj2: stringPointer{StringPointer: nil}, + }, + }, + { + name: "structs with pointer to string, one of them is nil", + args: args{ + obj1: stringPointer{StringPointer: ptrOf("test")}, + obj2: stringPointer{StringPointer: nil}, + }, + want: ObjectDiffs{{ + Field: "spec.stringPointer", + Value1: "test", + Value2: nil, + }}, + }, + { + name: "structs with pointer to string, both are not nil, equal", + args: args{ + obj1: stringPointer{StringPointer: ptrOf("test")}, + obj2: stringPointer{StringPointer: ptrOf("test")}, + }, + }, + { + name: "structs with pointer to string, both are not nil, not equal", + args: args{ + obj1: stringPointer{StringPointer: ptrOf("test1")}, + obj2: stringPointer{StringPointer: ptrOf("test2")}, + }, + want: ObjectDiffs{{ + Field: "spec.stringPointer", + Value1: "test1", + Value2: "test2", + }}, + }, + { + name: "structs with pointer to other struct, nil, equal", + args: args{ + obj1: structPointer{Ptr: nil}, + obj2: structPointer{Ptr: nil}, + }, + }, + { + name: "structs with pointer to other struct, not nil, equal", + args: args{ + obj1: structPointer{Ptr: &stringFieldWithJsonTag{String: "string"}}, + obj2: structPointer{Ptr: &stringFieldWithJsonTag{String: "string"}}, + }, + }, + { + name: "structs with pointer to other struct, one of them is nil, not equal", + args: args{ + obj1: structPointer{Ptr: nil}, + obj2: structPointer{Ptr: &stringFieldWithJsonTag{String: "string"}}, + }, + want: ObjectDiffs{{ + Field: "spec.ptr", + Value1: nil, + Value2: stringFieldWithJsonTag{String: "string"}, + }}, + }, + { + name: "structs with pointer to other struct, not nil, not equal", + args: args{ + obj1: structPointer{Ptr: &stringFieldWithJsonTag{String: "string1"}}, + obj2: structPointer{Ptr: &stringFieldWithJsonTag{String: "string2"}}, + }, + want: ObjectDiffs{{ + Field: "spec.ptr.string", + Value1: "string1", + Value2: "string2", + }}, + }, + { + name: "structs with struct field, empty, equal", + args: args{ + obj1: structField{}, + obj2: structField{}, + }, + }, + { + name: "structs with struct field, one of them is empty, not equal", + args: args{ + obj1: structField{Field: stringFieldWithJsonTag{String: "test"}}, + obj2: structField{}, + }, + want: ObjectDiffs{{ + Field: "spec.field.string", + Value1: "test", + Value2: "", + }}, + }, + { + name: "structs with struct field, not empty, equal", + args: args{ + obj1: structField{Field: stringFieldWithJsonTag{String: "test"}}, + obj2: structField{Field: stringFieldWithJsonTag{String: "test"}}, + }, + }, + { + name: "structs with struct field, not empty, not equal", + args: args{ + obj1: structField{Field: stringFieldWithJsonTag{String: "test1"}}, + obj2: structField{Field: stringFieldWithJsonTag{String: "test2"}}, + }, + want: ObjectDiffs{{ + Field: "spec.field.string", + Value1: "test1", + Value2: "test2", + }}, + }, + { + name: "structs with slice of pointers to struct, empty, equal", + args: args{ + obj1: sliceOfPointers{}, + obj2: sliceOfPointers{}, + }, + }, + { + name: "structs with slice of pointers to struct, one of them is empty, not equal", + args: args{ + obj1: sliceOfPointers{Slice: []*stringFieldWithJsonTag{{String: "test"}}}, + obj2: sliceOfPointers{}, + }, + want: ObjectDiffs{{ + Field: "spec.slice[0]", + Value1: stringFieldWithJsonTag{String: "test"}, + Value2: nil, + }}, + }, + { + name: "structs with map string to int, both nil, equal", + args: args{ + obj1: mapStringToInt{Map: nil}, + obj2: mapStringToInt{Map: nil}, + }, + }, + { + name: "structs with map string to int, one of them is nil, not equal", + args: args{ + obj1: mapStringToInt{Map: map[string]int{"int": 1}}, + obj2: mapStringToInt{Map: nil}, + }, + want: ObjectDiffs{{ + Field: "spec.map[int]", + Value1: 1, + Value2: nil, + }}, + }, + { + name: "structs with map string to int, not empty, equal", + args: args{ + obj1: mapStringToInt{Map: map[string]int{"int": 1}}, + obj2: mapStringToInt{Map: map[string]int{"int": 1}}, + }, + }, + { + name: "structs with map string to int, not empty, both maps have extra pair", + args: args{ + obj1: mapStringToInt{Map: map[string]int{"int": 1, "extra1": 1}}, + obj2: mapStringToInt{Map: map[string]int{"int": 1, "extra2": 1}}, + }, + want: ObjectDiffs{ + { + Field: "spec.map[extra1]", + Value1: 1, + Value2: nil, + }, + { + Field: "spec.map[extra2]", + Value1: nil, + Value2: 1, + }, + }, + }, + { + name: "structs with map string to int, not empty, different value of the same key", + args: args{ + obj1: mapStringToInt{Map: map[string]int{"int": 1}}, + obj2: mapStringToInt{Map: map[string]int{"int": 2}}, + }, + want: ObjectDiffs{{ + Field: "spec.map[int]", + Value1: 1, + Value2: 2, + }}, + }, + { + name: "map string to struct ptr", + args: args{ + obj1: mapStringToStructPtr{Map: map[string]*stringFieldWithJsonTag{ + "ptr1": {String: "test"}, + }}, + obj2: mapStringToStructPtr{Map: map[string]*stringFieldWithJsonTag{ + "ptr1": {String: "test"}, + }}, + }, + }, + { + name: "map string to struct ptr, not equal", + args: args{ + obj1: mapStringToStructPtr{Map: map[string]*stringFieldWithJsonTag{ + "ptr1": {String: "test1"}, + }}, + obj2: mapStringToStructPtr{Map: map[string]*stringFieldWithJsonTag{ + "ptr1": {String: "test2"}, + }}, + }, + want: ObjectDiffs{{ + Field: "spec.map[ptr1].string", + Value1: "test1", + Value2: "test2", + }}, + }, + { + name: "struct with embedded struct with json inline constraint, not equal", + args: args{ + obj1: structWithEmbeddedStructWithJSONInlineConstraint{ + ExportedStruct: ExportedStruct{Field: "test1"}, + }, + obj2: structWithEmbeddedStructWithJSONInlineConstraint{ + ExportedStruct: ExportedStruct{Field: "test2"}, + }, + }, + want: ObjectDiffs{{ + Field: "spec.field", + Value1: "test1", + Value2: "test2", + }}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := StructsDiff("spec", tt.args.obj1, tt.args.obj2) + if (err != nil) != tt.wantErr { + t.Errorf("StructsDiff() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("StructsDiff() got = %v, want %v", got, tt.want) + } + }) + } +}