Skip to content

Commit

Permalink
feat: introduce new config for excluding fields on history-less resou…
Browse files Browse the repository at this point in the history
…rces

Signed-off-by: Tim Heurich <[email protected]>
  • Loading branch information
theurichde committed Jun 29, 2023
1 parent 5664503 commit f910030
Show file tree
Hide file tree
Showing 13 changed files with 64 additions and 105 deletions.
2 changes: 1 addition & 1 deletion pkg/kapp/cmd/app/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (o *DeleteOptions) calculateAndPresentChanges(existingResources []ctlres.Re
)

{ // Figure out changes for X existing resources -> 0 new resources
changeFactory := ctldiff.NewChangeFactory(nil, nil)
changeFactory := ctldiff.NewChangeFactory(nil, nil, nil)
changeSetFactory := ctldiff.NewChangeSetFactory(o.DiffFlags.ChangeSetOpts, changeFactory)

changes, err := changeSetFactory.New(existingResources, nil).Calculate()
Expand Down
2 changes: 1 addition & 1 deletion pkg/kapp/cmd/app/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ func (o *DeployOptions) calculateAndPresentChanges(existingResources,
var clusterChangeSet ctlcap.ClusterChangeSet

{ // Figure out changes for X existing resources -> X new resources
changeFactory := ctldiff.NewChangeFactory(conf.RebaseMods(), conf.DiffAgainstLastAppliedFieldExclusionMods())
changeFactory := ctldiff.NewChangeFactory(conf.RebaseMods(), conf.DiffAgainstLastAppliedFieldExclusionMods(), conf.DiffAgainstExistingFieldExclusionMods())
changeSetFactory := ctldiff.NewChangeSetFactory(o.DiffFlags.ChangeSetOpts, changeFactory)

err := ctldiff.NewRenewableResources(existingResources, newResources).Prepare()
Expand Down
2 changes: 1 addition & 1 deletion pkg/kapp/cmd/app/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (o *InspectOptions) Run() error {
switch {
case o.Raw:
for _, res := range resources {
historylessRes, err := ctldiff.NewResourceWithHistory(res, nil, nil).HistorylessResource()
historylessRes, err := ctldiff.NewResourceWithoutHistory(res, nil).Resource()
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kapp/cmd/tools/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (o *DiffOptions) Run() error {
return err
}

changeFactory := ctldiff.NewChangeFactory(nil, nil)
changeFactory := ctldiff.NewChangeFactory(nil, nil, nil)

changes, err := ctldiff.NewChangeSet(existingResources, newResources, o.DiffFlags.ChangeSetOpts, changeFactory).Calculate()
if err != nil {
Expand Down
10 changes: 10 additions & 0 deletions pkg/kapp/config/conf.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,16 @@ func (c Conf) DiffAgainstLastAppliedFieldExclusionMods() []ctlres.FieldRemoveMod
return mods
}

func (c Conf) DiffAgainstExistingFieldExclusionMods() []ctlres.FieldRemoveMod {
var mods []ctlres.FieldRemoveMod
for _, config := range c.configs {
for _, rule := range config.DiffAgainstExistingFieldExclusionRules {
mods = append(mods, rule.AsMod())
}
}
return mods
}

func (c Conf) OwnershipLabelMods() func(kvs map[string]string) []ctlres.StringMapAppendMod {
return func(kvs map[string]string) []ctlres.StringMapAppendMod {
var mods []ctlres.StringMapAppendMod
Expand Down
14 changes: 14 additions & 0 deletions pkg/kapp/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type Config struct {

AdditionalLabels map[string]string
DiffAgainstLastAppliedFieldExclusionRules []DiffAgainstLastAppliedFieldExclusionRule
DiffAgainstExistingFieldExclusionRules []DiffAgainstExistingFieldExclusionRule

// TODO additional?
// TODO validations
Expand Down Expand Up @@ -96,6 +97,11 @@ type DiffAgainstLastAppliedFieldExclusionRule struct {
Path ctlres.Path
}

type DiffAgainstExistingFieldExclusionRule struct {
ResourceMatchers []ResourceMatcher
Path ctlres.Path
}

type OwnershipLabelRule struct {
ResourceMatchers []ResourceMatcher
Path ctlres.Path
Expand Down Expand Up @@ -290,6 +296,14 @@ func (r DiffAgainstLastAppliedFieldExclusionRule) AsMod() ctlres.FieldRemoveMod
Path: r.Path,
}
}
func (r DiffAgainstExistingFieldExclusionRule) AsMod() ctlres.FieldRemoveMod {
return ctlres.FieldRemoveMod{
ResourceMatcher: ctlres.AnyMatcher{
Matchers: ResourceMatchers(r.ResourceMatchers).AsResourceMatchers(),
},
Path: r.Path,
}
}

func (r OwnershipLabelRule) AsMod(kvs map[string]string) ctlres.StringMapAppendMod {
return ctlres.StringMapAppendMod{
Expand Down
1 change: 1 addition & 0 deletions pkg/kapp/config/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ diffAgainstLastAppliedFieldExclusionRules:
- path: [metadata, annotations, "deployment.kubernetes.io/revision"]
resourceMatchers: *appsV1DeploymentWithRevAnnKey
diffAgainstExistingFieldExclusionRules:
- path: [status]
resourceMatchers:
- allMatcher: {}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kapp/config/default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
func TestDefaultTemplateRules(t *testing.T) {
_, defaultConfig, err := config.NewConfFromResourcesWithDefaults([]ctlres.Resource{})
require.NoError(t, err)
changeFactory := ctldiff.NewChangeFactory(defaultConfig.RebaseMods(), defaultConfig.DiffAgainstLastAppliedFieldExclusionMods())
changeFactory := ctldiff.NewChangeFactory(defaultConfig.RebaseMods(), defaultConfig.DiffAgainstLastAppliedFieldExclusionMods(), defaultConfig.DiffAgainstExistingFieldExclusionMods())

testCases := []struct {
description string
Expand Down
17 changes: 11 additions & 6 deletions pkg/kapp/diff/change_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import (
type ChangeFactory struct {
rebaseMods []ctlres.ResourceModWithMultiple
diffAgainstLastAppliedFieldExclusionMods []ctlres.FieldRemoveMod
diffAgainstExistingFieldExclusionRules []ctlres.FieldRemoveMod
}

func NewChangeFactory(rebaseMods []ctlres.ResourceModWithMultiple,
diffAgainstLastAppliedFieldExclusionMods []ctlres.FieldRemoveMod) ChangeFactory {
diffAgainstLastAppliedFieldExclusionMods []ctlres.FieldRemoveMod, diffAgainstExistingFieldExclusionRules []ctlres.FieldRemoveMod) ChangeFactory {

return ChangeFactory{rebaseMods, diffAgainstLastAppliedFieldExclusionMods}
return ChangeFactory{rebaseMods, diffAgainstLastAppliedFieldExclusionMods, diffAgainstExistingFieldExclusionRules}
}

func (f ChangeFactory) NewChangeAgainstLastApplied(existingRes, newRes ctlres.Resource) (Change, error) {
Expand All @@ -36,7 +37,7 @@ func (f ChangeFactory) NewChangeAgainstLastApplied(existingRes, newRes ctlres.Re
existingRes = rebasedLastAppliedRes
}

historylessExistingRes, err := f.NewResourceWithHistory(existingRes).HistorylessResource()
historylessExistingRes, err := f.newResourceWithoutHistory(existingRes).Resource()
if err != nil {
return nil, err
}
Expand All @@ -45,7 +46,7 @@ func (f ChangeFactory) NewChangeAgainstLastApplied(existingRes, newRes ctlres.Re
}

if newRes != nil {
historylessNewRes, err := f.NewResourceWithHistory(newRes).HistorylessResource()
historylessNewRes, err := f.newResourceWithoutHistory(newRes).Resource()
if err != nil {
return nil, err
}
Expand All @@ -63,7 +64,7 @@ func (f ChangeFactory) NewChangeAgainstLastApplied(existingRes, newRes ctlres.Re

func (f ChangeFactory) NewExactChange(existingRes, newRes ctlres.Resource) (Change, error) {
if existingRes != nil {
historylessExistingRes, err := f.NewResourceWithHistory(existingRes).HistorylessResource()
historylessExistingRes, err := f.newResourceWithoutHistory(existingRes).Resource()
if err != nil {
return nil, err
}
Expand All @@ -72,7 +73,7 @@ func (f ChangeFactory) NewExactChange(existingRes, newRes ctlres.Resource) (Chan
}

if newRes != nil {
historylessNewRes, err := f.NewResourceWithHistory(newRes).HistorylessResource()
historylessNewRes, err := f.newResourceWithoutHistory(newRes).Resource()
if err != nil {
return nil, err
}
Expand All @@ -91,3 +92,7 @@ func (f ChangeFactory) NewExactChange(existingRes, newRes ctlres.Resource) (Chan
func (f ChangeFactory) NewResourceWithHistory(resource ctlres.Resource) ResourceWithHistory {
return NewResourceWithHistory(resource, &f, f.diffAgainstLastAppliedFieldExclusionMods)
}

func (f ChangeFactory) newResourceWithoutHistory(resource ctlres.Resource) ResourceWithoutHistory {
return NewResourceWithoutHistory(resource, f.diffAgainstExistingFieldExclusionRules)
}
10 changes: 5 additions & 5 deletions pkg/kapp/diff/change_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ metadata:
},
}

changeFactory := ctldiff.NewChangeFactory(mods, nil)
changeFactory := ctldiff.NewChangeFactory(mods, nil, nil)
changeSet := ctldiff.NewChangeSet([]ctlres.Resource{existingRes}, []ctlres.Resource{newRes},
ctldiff.ChangeSetOpts{}, changeFactory)

Expand Down Expand Up @@ -106,7 +106,7 @@ metadata:
},
}

changeFactory := ctldiff.NewChangeFactory(mods, nil)
changeFactory := ctldiff.NewChangeFactory(mods, nil, nil)
changeSet := ctldiff.NewChangeSet([]ctlres.Resource{existingRes}, []ctlres.Resource{newRes},
ctldiff.ChangeSetOpts{}, changeFactory)

Expand Down Expand Up @@ -174,7 +174,7 @@ metadata:
},
}

changeFactory := ctldiff.NewChangeFactory(rebaseMods, ignoreFieldsMods)
changeFactory := ctldiff.NewChangeFactory(rebaseMods, ignoreFieldsMods, nil)
changeSet := ctldiff.NewChangeSet([]ctlres.Resource{existingRes}, []ctlres.Resource{newRes},
ctldiff.ChangeSetOpts{AgainstLastApplied: true}, changeFactory)

Expand Down Expand Up @@ -246,7 +246,7 @@ metadata:
},
}

changeFactory := ctldiff.NewChangeFactory(rebaseMods, ignoreFieldsMods)
changeFactory := ctldiff.NewChangeFactory(rebaseMods, ignoreFieldsMods, nil)
changeSet := ctldiff.NewChangeSet([]ctlres.Resource{existingRes}, []ctlres.Resource{newRes},
ctldiff.ChangeSetOpts{AgainstLastApplied: true}, changeFactory)

Expand Down Expand Up @@ -304,7 +304,7 @@ metadata:
},
}

changeFactory := ctldiff.NewChangeFactory(mods, nil)
changeFactory := ctldiff.NewChangeFactory(mods, nil, nil)
changeSet := ctldiff.NewChangeSet([]ctlres.Resource{existingRes}, []ctlres.Resource{newRes},
ctldiff.ChangeSetOpts{}, changeFactory)

Expand Down
25 changes: 16 additions & 9 deletions pkg/kapp/diff/resource_with_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ func NewResourceWithHistory(resource ctlres.Resource,

return ResourceWithHistory{resource.DeepCopy(), changeFactory, diffAgainstLastAppliedFieldExclusionMods}
}

func (r ResourceWithHistory) HistorylessResource() (ctlres.Resource, error) {
return resourceWithoutHistory{r.resource}.Resource()
func NewResourceWithoutHistory(resource ctlres.Resource, fieldExclusionMods []ctlres.FieldRemoveMod) ResourceWithoutHistory {
return ResourceWithoutHistory{res: resource, fieldExclusionMods: fieldExclusionMods}
}

// LastAppliedResource will return "last applied" resource that was saved
Expand Down Expand Up @@ -156,24 +155,25 @@ func (r ResourceWithHistory) recalculateLastAppliedChange() ([]Change, string, s

func (r ResourceWithHistory) newExactHistorylessChange(existingRes, newRes ctlres.Resource) (Change, error) {
// If annotations are not removed line numbers will be mismatched
existingRes, err := resourceWithoutHistory{existingRes}.Resource()
existingRes, err := ResourceWithoutHistory{existingRes, nil}.Resource()
if err != nil {
return nil, err
}

newRes, err = resourceWithoutHistory{newRes}.Resource()
newRes, err = ResourceWithoutHistory{newRes, nil}.Resource()
if err != nil {
return nil, err
}

return r.changeFactory.NewExactChange(existingRes, newRes)
}

type resourceWithoutHistory struct {
res ctlres.Resource
type ResourceWithoutHistory struct {
res ctlres.Resource
fieldExclusionMods []ctlres.FieldRemoveMod
}

func (r resourceWithoutHistory) Resource() (ctlres.Resource, error) {
func (r ResourceWithoutHistory) Resource() (ctlres.Resource, error) {
res := r.res.DeepCopy()

for _, t := range r.removeAppliedResAnnKeysMods() {
Expand All @@ -183,10 +183,17 @@ func (r resourceWithoutHistory) Resource() (ctlres.Resource, error) {
}
}

for _, t := range r.fieldExclusionMods {
err := t.Apply(res)
if err != nil {
return nil, err
}
}

return res, nil
}

func (resourceWithoutHistory) removeAppliedResAnnKeysMods() []ctlres.ResourceMod {
func (ResourceWithoutHistory) removeAppliedResAnnKeysMods() []ctlres.ResourceMod {
return []ctlres.ResourceMod{
ctlres.FieldRemoveMod{
ResourceMatcher: ctlres.AllMatcher{},
Expand Down
80 changes: 1 addition & 79 deletions test/e2e/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -591,38 +591,6 @@ status:
conditions: []
storedVersions: []
`

updatedStatusYaml := `
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: tests.kapp.example
spec:
group: kapp.example
names:
kind: Test
plural: tests
scope: Namespaced
versions:
- name: v1alpha1
schema:
openAPIV3Schema:
type: object
served: true
storage: true
status:
acceptedNames:
kind: ""
plural: ""
conditions:
- message: some conflicts found
reason: NoConflicts
status: "True"
type: NamesAccepted
storedVersions: []
`

updatedYaml := `
---
apiVersion: apiextensions.k8s.io/v1
Expand Down Expand Up @@ -668,20 +636,6 @@ status:
-linesss- schema:
-linesss- openAPIV3Schema:
-linesss- type: object
-linesss- status:
-linesss- acceptedNames:`

expectedChangesForStatusUpdate := `
@@ update customresourcedefinition/tests.kapp.example (apiextensions.k8s.io/v1) cluster @@
...
-linesss- plural: ""
-linesss- conditions: []
-linesss- conditions:
-linesss- - message: some conflicts found
-linesss- reason: NoConflicts
-linesss- status: "True"
-linesss- type: NamesAccepted
-linesss- storedVersions: []
-linesss- `

expectedChangesForExternalUpdate := `
Expand All @@ -703,31 +657,7 @@ status:
-linesss- plural: tests
-linesss- singular: test
-linesss- scope: Namespaced
-linesss- versions:
...
-linesss- acceptedNames:
-linesss- kind: Test
-linesss- listKind: TestList
-linesss- plural: tests
-linesss- singular: test
-linesss- conditions:
-linesss- - lastTransitionTime: "2006-01-02T15:04:05Z07:00"
-linesss- message: no conflicts found
-linesss- reason: NoConflicts
-linesss- status: "True"
-linesss- type: NamesAccepted
-linesss- - lastTransitionTime: "2006-01-02T15:04:05Z07:00"
-linesss- message: the initial names have been accepted
-linesss- reason: InitialNamesAccepted
-linesss- status: "True"
-linesss- type: Established
-linesss- storedVersions:
-linesss- - v1alpha1
-linesss- kind: ""
-linesss- plural: ""
-linesss- conditions: []
-linesss- storedVersions: []
-linesss- `
-linesss- versions:`

name := "test-default-config-exclude-diff-against-last-applied-status"
cleanUp := func() {
Expand Down Expand Up @@ -767,14 +697,6 @@ status:
checkChangesOutput(t, replaceTimestampWithDfaultValue(out), expectedChangesForUpdate)
})

logger.Section("deploy with some changes to status field", func() {
out, _ := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "-c"},
RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedStatusYaml)})

// if status is updated, diffing is done against last applied status
checkChangesOutput(t, replaceTimestampWithDfaultValue(out), expectedChangesForStatusUpdate)
})

logger.Section("deploy after some changes are made by external controller", func() {
patchCRD := `[{ "op": "add", "path": "/metadata/annotations/test", "value": "test-ann-val"}]`

Expand Down
2 changes: 1 addition & 1 deletion test/e2e/order_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ metadata:
break
}
}
patch := `[{ "op": "add", "path": "/status/conditions/-", "value": {type: "Available", status: "True"}}]`
patch := `[{ "op": "add", "path": "/status", "value": {"conditions": [{"type": "Available", "status": "True"}]}}]`
PatchClusterResource("CronTab", "my-new-cron-object-1", env.Namespace, patch, kubectl)
}()

Expand Down

0 comments on commit f910030

Please sign in to comment.