Skip to content

Commit

Permalink
Merge pull request #115 from nocturnalastro/defaultRefList
Browse files Browse the repository at this point in the history
CNF-14746: Add includes to fields to omit
  • Loading branch information
openshift-merge-bot[bot] authored Oct 16, 2024
2 parents c163696 + fe5049e commit 7594d62
Show file tree
Hide file tree
Showing 27 changed files with 751 additions and 13 deletions.
21 changes: 21 additions & 0 deletions docs/reference-config-guide-v2.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,27 @@ fieldsToOmit:

The default value of `defaultOmitRef` is a built-in list `cluster-compare-built-in` and can still be referenced even if the `defaultOmitRef` is set.

#### Referencing field omission groups

A group of field omissions may reference other groups of field omission items to allow less duplication in group creation. For example:

```yaml
fieldsToOmit:
defaultOmitRef: default
items:
common:
- pathToKey: metadata.annotations."kubernetes.io/metadata.name"
- pathToKey: metadata.annotations."kubernetes.io/metadata.name"
- pathToKey: metadata.annotations."kubectl.kubernetes.io/last-applied-configuration"
- pathToKey: metadata.creationTimestamp
- pathToKey: metadata.generation
- pathToKey: spec.ownerReferences
- pathToKey: metadata.ownerReferences
default:
- include: common
- pathToKey: status
```

#### pathToKey syntax

The syntax for `pathToKey` is a dot seperated path.
Expand Down
21 changes: 21 additions & 0 deletions pkg/compare/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,13 @@ func (test Test) withBadAPIResources() Test {
return newTest
}

func (test Test) withSubTestWithMetadata(subName string) Test {
squashed := strings.ReplaceAll(subName, " ", "_")
return test.withSubTestSuffix(subName).
withMetadataFile(fmt.Sprintf("metadata_%s.yaml", squashed)).
withChecks(test.checks.withPrefixedSuffix("_" + squashed + "_"))
}

func (test *Test) subTestName(mode Mode) string {
name := test.name
if test.subTestSuffix != "" {
Expand Down Expand Up @@ -493,6 +500,20 @@ func TestCompareRun(t *testing.T) {
withBadAPIResources().
withModes([]Mode{{Live, LocalRef}}).
withChecks(defaultChecks.withPrefixedSuffix("badAPI")),

defaultTest("Reference V2 Diff in Custom Omitted Fields Isnt Shown").
withSubTestWithMetadata("basic"),
defaultTest("Reference V2 Diff in Custom Omitted Fields Isnt Shown").
withSubTestWithMetadata("quoted"),
defaultTest("Reference V2 Diff in Custom Omitted Fields Isnt Shown").
withSubTestWithMetadata("leading dot"),
defaultTest("Reference V2 Diff in Custom Omitted Fields Isnt Shown").
withSubTestWithMetadata("non default"),
defaultTest("Reference V2 Diff in Custom Omitted Fields Isnt Shown").
withSubTestWithMetadata("basic include"),
defaultTest("Reference V2 Diff in Custom Omitted Fields Isnt Shown").
withSubTestWithMetadata("circular include"),
defaultTest("Reference V2 Diff in Custom Omitted Fields Isnt Shown Prefix"),
}

tf := cmdtesting.NewTestFactory()
Expand Down
23 changes: 12 additions & 11 deletions pkg/compare/referenceV1.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (toOmit *FieldsToOmitV1) process() error {
klog.Warningf(fieldsToOmitBuiltInOverwritten, builtInPathsKey)
}

toOmit.Items[builtInPathsKey] = builtInPaths
toOmit.Items[builtInPathsKey] = builtInPathsV1

if toOmit.DefaultOmitRef == "" {
toOmit.DefaultOmitRef = builtInPathsKey
Expand Down Expand Up @@ -207,14 +207,15 @@ type ReferenceTemplateV1 struct {
func (rf ReferenceTemplateV1) GetFieldsToOmit(fieldsToOmit FieldsToOmit) []*ManifestPathV1 {
result := make([]*ManifestPathV1, 0)
// ValidateFieldsToOmit should check the ok
fieldsToOmitv1 := fieldsToOmit.(*FieldsToOmitV1)

items := fieldsToOmit.GetItems()
if len(rf.Config.FieldsToOmitRefs) == 0 {
return fieldsToOmitv1.Items[fieldsToOmitv1.DefaultOmitRef]
result = append(result, items[fieldsToOmit.GetDefault()]...)
return result
}

for _, feildsRef := range rf.Config.FieldsToOmitRefs {
result = append(result, fieldsToOmitv1.Items[feildsRef]...)
result = append(result, items[feildsRef]...)
}
return result
}
Expand All @@ -224,14 +225,10 @@ const (
)

func (rf ReferenceTemplateV1) ValidateFieldsToOmit(fieldsToOmit FieldsToOmit) error {
fieldsToOmitv1, ok := fieldsToOmit.(*FieldsToOmitV1)
if !ok {
return fmt.Errorf("unable to cast %T into %T to parse for V1", fieldsToOmit, fieldsToOmitv1)
}

errs := make([]error, 0)
items := fieldsToOmit.GetItems()
for _, feildsRef := range rf.Config.FieldsToOmitRefs {
if _, ok := fieldsToOmitv1.Items[feildsRef]; !ok {
if _, ok := items[feildsRef]; !ok {
errs = append(errs, fmt.Errorf(fieldsToOmitRefsNotFound, feildsRef))
}
}
Expand Down Expand Up @@ -280,7 +277,7 @@ func (rf ReferenceTemplateV1) GetTemplateTree() *parse.Tree {

const builtInPathsKey = "cluster-compare-built-in"

var builtInPaths = []*ManifestPathV1{
var builtInPathsV1 = []*ManifestPathV1{
{PathToKey: "metadata.resourceVersion"},
{PathToKey: "metadata.generation"},
{PathToKey: "metadata.uid"},
Expand All @@ -299,6 +296,10 @@ type ManifestPathV1 struct {
}

func (p *ManifestPathV1) Process() error {
if len(p.parts) > 0 {
return nil
}

pathToKey, _ := strings.CutPrefix(p.PathToKey, ".")
r := csv.NewReader(strings.NewReader(pathToKey))
r.Comma = '.'
Expand Down
137 changes: 135 additions & 2 deletions pkg/compare/referenceV2.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ import (
"io/fs"
"path"
"reflect"
"slices"
"strings"
"text/template"

"k8s.io/klog/v2"
)

const ReferenceVersionV2 string = "v2"
Expand All @@ -21,7 +24,7 @@ type ReferenceV2 struct {

Parts []*PartV2 `json:"parts"`
TemplateFunctionFiles []string `json:"templateFunctionFiles,omitempty"`
FieldsToOmit *FieldsToOmitV1 `json:"fieldsToOmit,omitempty"`
FieldsToOmit *FieldsToOmitV2 `json:"fieldsToOmit,omitempty"`
}

func (r *ReferenceV2) GetAPIVersion() string {
Expand Down Expand Up @@ -81,6 +84,136 @@ func (r *ReferenceV2) GetValidationIssues(matchedTemplates map[string]int) (map[
return crs, count
}

func getbuiltInPathsV2() []*FieldsToOmitV2Entry {
res := make([]*FieldsToOmitV2Entry, 0)
for _, p := range builtInPathsV1 {
res = append(res, &FieldsToOmitV2Entry{ManifestPathV1: p})
}
return res
}

type FieldsToOmitV2 struct {
DefaultOmitRef string `json:"defaultOmitRef,omitempty"`
Items map[string][]*FieldsToOmitV2Entry `json:"items,omitempty"`
items map[string][]*ManifestPathV1
}

func (toOmit *FieldsToOmitV2) GetDefault() string {
return toOmit.DefaultOmitRef
}

func (toOmit *FieldsToOmitV2) GetItems() map[string][]*ManifestPathV1 {
return toOmit.items
}

// Setup FieldsToOmit to be used by setting defaults
// and processing the item strings into paths
func (toOmit *FieldsToOmitV2) process() error {
if toOmit.items == nil {
toOmit.items = make(map[string][]*ManifestPathV1)
}

if toOmit.Items == nil {
toOmit.Items = make(map[string][]*FieldsToOmitV2Entry)
}

if _, ok := toOmit.Items[builtInPathsKey]; ok {
klog.Warningf(fieldsToOmitBuiltInOverwritten, builtInPathsKey)
}

errs := make([]error, 0)

toOmit.Items[builtInPathsKey] = getbuiltInPathsV2()

if len(errs) > 0 {
return errors.Join(errs...)
}

if toOmit.DefaultOmitRef == "" {
toOmit.DefaultOmitRef = builtInPathsKey
}

for key := range toOmit.Items {
paths, err := processFieldsToOmitEntries(key, toOmit, []string{})
if err != nil {
errs = append(errs, err)
} else {
// TODO: we should look into dedupe the paths
toOmit.items[key] = append(toOmit.items[key], paths...)
}

}
return errors.Join(errs...)
}

func processFieldsToOmitEntries(key string, toOmit *FieldsToOmitV2, previousKeys []string) ([]*ManifestPathV1, error) {
currentKeys := make([]string, 0)
currentKeys = append(currentKeys, previousKeys...)
currentKeys = append(currentKeys, key)

errs := make([]error, 0)
paths := make([]*ManifestPathV1, 0)
for _, entry := range toOmit.Items[key] {
entryPaths, err := entry.process(currentKeys, toOmit)
if err != nil {
errs = append(errs, err)
continue
}
paths = append(paths, entryPaths...)

}
return paths, errors.Join(errs...)
}

type FieldsToOmitV2Entry struct {
*ManifestPathV1
Include string `json:"include,omitempty"`
paths []*ManifestPathV1
processingError error
}

func (entry *FieldsToOmitV2Entry) process(previousKeys []string, toOmit *FieldsToOmitV2) ([]*ManifestPathV1, error) {
if len(entry.paths) != 0 {
return entry.paths, entry.processingError
}

paths := make([]*ManifestPathV1, 0)
if entry.Include == "" && (entry.ManifestPathV1 == nil || entry.PathToKey == "") {
return paths, fmt.Errorf("must have either include or pathToKey")
}

errs := make([]error, 0)
if entry.ManifestPathV1 != nil && entry.PathToKey != "" {
err := entry.ManifestPathV1.Process()
if err != nil {
errs = append(errs, err)
} else {
paths = append(paths, entry.ManifestPathV1)
}
}

if entry.Include != "" {
foundCircle := slices.Contains(previousKeys, entry.Include)
if foundCircle {
circularKeys := make([]string, 0)
circularKeys = append(circularKeys, previousKeys...)
circularKeys = append(circularKeys, entry.Include)
return paths, fmt.Errorf("circular import found %s", strings.Join(circularKeys, " -> "))
}

entryPaths, err := processFieldsToOmitEntries(entry.Include, toOmit, previousKeys)
if err != nil {
errs = append(errs, err)
} else {
paths = append(paths, entryPaths...)
}
}

entry.paths = append(entry.paths, paths...)
entry.processingError = errors.Join(errs...)
return paths, entry.processingError
}

type PartV2 struct {
Name string `json:"name"`
Components []*ComponentV2 `json:"components"`
Expand Down Expand Up @@ -349,7 +482,7 @@ func getReferenceV2(fsys fs.FS, referenceFileName string) (*ReferenceV2, error)
return result, err
}
if result.FieldsToOmit == nil {
result.FieldsToOmit = &FieldsToOmitV1{}
result.FieldsToOmit = &FieldsToOmitV2{}
}
err = result.FieldsToOmit.process()
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Summary
CRs with diffs: 0/3
No validation issues with the cluster
No CRs are unmatched to reference CRs
Metadata Hash: d1b43073d9263292a98639abf0845c1f68a53f21a210429b1a71d3b8b47b4d14
No patched CRs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Summary
CRs with diffs: 0/3
No validation issues with the cluster
No CRs are unmatched to reference CRs
Metadata Hash: aec723b5a3f391084f76a8815354719badc5306910b6025df025ed535b6ecead
No patched CRs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
error: circular import found deployment -> deployment
circular import found includeMe -> includeWithDepth -> includeMe
circular import found includeWithDepth -> includeMe -> includeWithDepth
error code:2
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Summary
CRs with diffs: 0/3
No validation issues with the cluster
No CRs are unmatched to reference CRs
Metadata Hash: 8f628171c03544439e58089ab3c89e3adbe1534287e505fde2add79db82cca96
No patched CRs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Summary
CRs with diffs: 0/3
No validation issues with the cluster
No CRs are unmatched to reference CRs
Metadata Hash: aefa16fbc84fb41ae77b36a53637b0a4e5147ddcbb892d8bfa877080714b4c0e
No patched CRs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Summary
CRs with diffs: 0/3
No validation issues with the cluster
No CRs are unmatched to reference CRs
Metadata Hash: b276bc9ca9b157818bea3e7a6747c0c6acaa2dfbeb9bfe8dab6ec9b00d8a843d
No patched CRs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
kind: Deployment
apiVersion: apps/v1
metadata:
labels:
k8s-app: dashboard-metrics-scraper
name: {{ .metadata.name }}
namespace: kubernetes-dashboard
spec:
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
k8s-app: dashboard-metrics-scraper
template:
metadata:
labels:
k8s-app: dashboard-metrics-scraper
spec:
{{ if .spec.template.spec }}{{ .spec.template.spec | toYaml | indent 6 }}{{ end }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
apiVersion: v2
parts:
- name: ExamplePart
components:
- name: Dashboard
allOf:
- path: deploymentMetrics.yaml

fieldsToOmit:
defaultOmitRef: deployment
items:
deployment:
- pathToKey: spec.selector.matchLabels.k8s-app
- pathToKey: metadata.labels.k8s-app
- pathToKey: spec.template.metadata.labels.k8s-app
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
apiVersion: v2
parts:
- name: ExamplePart
components:
- name: Dashboard
allOf:
- path: deploymentMetrics.yaml

fieldsToOmit:
defaultOmitRef: deployment
items:
deployment:
- include: includeMe
includeMe:
- pathToKey: spec.selector.matchLabels.k8s-app
- pathToKey: metadata.labels.k8s-app
- pathToKey: spec.template.metadata.labels.k8s-app
Loading

0 comments on commit 7594d62

Please sign in to comment.