Skip to content

Commit

Permalink
Merge pull request #143 from lack/revert_strictMatch
Browse files Browse the repository at this point in the history
Revert "Introduce strict-match correlation flag"
  • Loading branch information
openshift-merge-bot[bot] authored Nov 22, 2024
2 parents 28c5766 + 98da6aa commit c77f189
Show file tree
Hide file tree
Showing 15 changed files with 24 additions and 334 deletions.
55 changes: 18 additions & 37 deletions docs/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,49 +52,30 @@ resources and resource templates. The matches can be added to the config as pair

##### Correlation by group of fields (apiVersion, kind, namespace and name)

When there is no manual match for a CR the command will try to match a template
for the resource by looking at the 4-tuple: apiVersion, kind, namespace and
name.
When there is no manual match for a CR the command will try to match a template for the resource by looking at the
4-tuple: apiVersion, kind, namespace and name . The Correlation is based on which fields in the templates that are not
user-variable. Templates get matched to resources based on all the features from the 4-tuple that are declared fixed (
not user-variable) in the templates.

For each resource the group correlation will be finding a template which
matches the largest number of fields from this group: apiVersion, kind,
namespace, name. This is done in the following order, with the first match
taken:
For example a template with a fixed namespace, kind, name and templated (user-variable) apiVersion will only be a
potential match by the kind-namespace-name criterion.

For each resource the group correlation will be done by the next logic:

1. Exact match of apiVersion-kind-namespace-name
1. Exact Match in 3/4 fields from apiVersion, kind, namespace, name. (meaning
one of: kind-namespace-name or apiVersion-kind-name or
apiVersion-kind-namespace)
1. Exact Match in 2/4 fields from apiVersion, kind, namespace, name. (meaning
one of: kind-namespace or kind-name or apiVersion-kind)
1. If there is a result in the reference, comparison will be done
1. Exact Match in 3/4 fields from apiVersion, kind, namespace, name. ( meaning exact match in: kind-namespace-name or
apiVersion-kind-name or apiVersion-kind-namespace)
1. If there is a result in the reference, comparison will be done
1. Exact Match in 2/4 fields from apiVersion, kind, namespace, name. ( meaning exact match in: kind-namespace or
kind-name or apiVersion-kind)
1. If there is a result in the reference, comparison will be done
1. Match kind
1. If there is a result in the reference, comparison will be done
1. No match – comparison cannot be made and the file is flagged as unmatched.

In the event that a single CR matches multiple templates, the diff logic
assumes that the template with the smallest number of diffs from the given CR
is the intended template. This can be overridden by either specifying manual
matches (see the prior section), or be enabling strict match mode (see the
following section).

###### Strict match mode

Strict match correlation is based on which fields in the templates that are not
user-variable. In this mode, templates get matched to resources based on all
the features from the 4-tuple that are declared fixed (not user-variable) in
the templates.

For example a template with a fixed namespace, kind, name and templated
(user-variable) apiVersion will only be a potential match by the
kind-namespace-name criterion, but would not match a CR with the same kind and
namespace if the name is different.

To enable strict matching, add the following user config in your diff-config
(`-c` flag):

```yaml
correlationSettings:
strictMatch: true
```
We can phrase this logic in a more general form. Each CR will be correlated to a template with an exact match in the
largest number of fields from this group: apiVersion, kind, namespace, name.

### How it works

Expand Down
5 changes: 2 additions & 3 deletions pkg/compare/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func (o *Options) setupCorrelators() error {
correlators = append(correlators, manualCorrelator)
}

groupCorrelator, err := NewGroupCorrelator(defaultFieldGroups, o.templates, o.userConfig.CorrelationSettings.StrictMatch)
groupCorrelator, err := NewGroupCorrelator(defaultFieldGroups, o.templates)
if err != nil {
return err
}
Expand Down Expand Up @@ -387,8 +387,7 @@ func (o *Options) setupOverrideCorrelators() error {
correlators = append(correlators, manualOverrideCorrelator)
}

// UserOverrides are never lenient; must match as exactly as possible
groupCorrelator, err := NewGroupCorrelator(defaultFieldGroups, o.userOverrides, true)
groupCorrelator, err := NewGroupCorrelator(defaultFieldGroups, o.userOverrides)
if err != nil {
return err
}
Expand Down
5 changes: 0 additions & 5 deletions pkg/compare/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,11 +422,6 @@ func TestCompareRun(t *testing.T) {
defaultTest("SomeDiffs").
withVerboseOutput().
withChecks(defaultChecks.withPrefixedSuffix("withVebosityFlag")),
defaultTest("Strict Match"),
defaultTest("Strict Match").
withUserConfig(userConfigFileName).
withSubTestSuffix("Strict Mode On").
withChecks(defaultChecks.withPrefixedSuffix("strict_mode_on")),
defaultTest("Invalid Resources Are Skipped"),
defaultTest("Ref Contains Templates With Function Templates In Same File"),
defaultTest("User Override").
Expand Down
20 changes: 4 additions & 16 deletions pkg/compare/correlator.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,11 @@ type GroupCorrelator[T CorrelationEntry] struct {
// For fieldsGroups = {{{"metadata", "namespace"}, {"kind"}}, {{"kind"}}} and the following templates: [fixedKindTemplate, fixedNamespaceKindTemplate]
// the fixedNamespaceKindTemplate will be added to a mapping where the keys are in the format of `namespace_kind`. The fixedKindTemplate
// will be added to a mapping where the keys are in the format of `kind`.
func NewGroupCorrelator[T CorrelationEntry](fieldGroups [][][]string, objects []T, strict bool) (*GroupCorrelator[T], error) {
func NewGroupCorrelator[T CorrelationEntry](fieldGroups [][][]string, objects []T) (*GroupCorrelator[T], error) {
sort.Slice(fieldGroups, func(i, j int) bool {
return len(fieldGroups[i]) >= len(fieldGroups[j])
})
core := GroupCorrelator[T]{}
first := true
for _, group := range fieldGroups {
fc := FieldCorrelator[T]{Fields: group, hashFunc: createGroupHashFunc(group)}
newObjects := fc.ClaimTemplates(objects)
Expand All @@ -130,22 +129,16 @@ func NewGroupCorrelator[T CorrelationEntry](fieldGroups [][][]string, objects []
continue
}

// Only warn if we find duplicate templates matching on the most-specific FieldCorrelator, or in strict mode
fc.warnDupe = first || strict
first = false
objects = newObjects
core.fieldCorrelators = append(core.fieldCorrelators, &fc)

err := fc.ValidateTemplates()
if err != nil {
klog.Warning(err)
}

if strict {
// In strict mode, only continue to process the objects that we haven't yet matched to a FieldCorrelator
objects = newObjects
if len(objects) == 0 {
break
}
if len(objects) == 0 {
break
}
}

Expand Down Expand Up @@ -266,7 +259,6 @@ type FieldCorrelator[T CorrelationEntry] struct {
Fields [][]string
hashFunc templateHashFunc
objects map[string][]T
warnDupe bool
}

func (f *FieldCorrelator[T]) ClaimTemplates(templates []T) []T {
Expand All @@ -289,10 +281,6 @@ func (f *FieldCorrelator[T]) ClaimTemplates(templates []T) []T {
}

func (f *FieldCorrelator[T]) ValidateTemplates() error {
if !f.warnDupe {
return nil
}

errs := make([]error, 0)
for _, values := range f.objects {
if len(values) > 1 {
Expand Down
1 change: 0 additions & 1 deletion pkg/compare/parsing.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ type UserConfig struct {

type CorrelationSettings struct {
ManualCorrelation ManualCorrelation `json:"manualCorrelation"`
StrictMatch bool `json:"strictMatch,omitempty"`
}

type ManualCorrelation struct {
Expand Down
2 changes: 0 additions & 2 deletions pkg/compare/testdata/StrictMatch/localerr.golden

This file was deleted.

44 changes: 0 additions & 44 deletions pkg/compare/testdata/StrictMatch/localout.golden

This file was deleted.

This file was deleted.

11 changes: 0 additions & 11 deletions pkg/compare/testdata/StrictMatch/localstrict_mode_onout.golden

This file was deleted.

This file was deleted.

19 changes: 0 additions & 19 deletions pkg/compare/testdata/StrictMatch/reference/deploymentMetrics.yaml

This file was deleted.

8 changes: 0 additions & 8 deletions pkg/compare/testdata/StrictMatch/reference/metadata.yaml

This file was deleted.

52 changes: 0 additions & 52 deletions pkg/compare/testdata/StrictMatch/resources/d2.yaml

This file was deleted.

Loading

0 comments on commit c77f189

Please sign in to comment.