diff --git a/hack/.gitignore b/hack/.gitignore new file mode 100644 index 000000000..92d88f5c3 --- /dev/null +++ b/hack/.gitignore @@ -0,0 +1,2 @@ +.bash_history +.dlv diff --git a/hack/build-podman.sh b/hack/build-podman.sh new file mode 100755 index 000000000..6a244a1d2 --- /dev/null +++ b/hack/build-podman.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +cd $(dirname ${BASH_SOURCE[0]}) + +./podman.sh build diff --git a/hack/podman.sh b/hack/podman.sh new file mode 100755 index 000000000..5120de7ce --- /dev/null +++ b/hack/podman.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash + +cd $(dirname ${BASH_SOURCE[0]}) + +action=$1 + +if [ "$action" ]; then + [ -x "$action.sh" ] && action=hack/$action.sh +else + histfile=.bash_history + [ -f $histfile ] || >$histfile + + interactive="-it -v $PWD/$histfile:/root/$histfile" + + dlv_conf=.dlv + [ -d $dlv_conf ] || mkdir $dlv_conf + + interactive="$interactive -v $PWD/$dlv_conf:/root/.config/dlv" +fi + +# SYS_PTRACE for dlv +# host network for minikube +podman run --rm \ + -v gopath:/go \ + -v gobuild:/root/.cache/go-build \ + -v $HOME/.kube:/root/.kube \ + -v $HOME/.minikube:$HOME/.minikube \ + -v $(realpath $PWD/..):/mnt -w /mnt \ + $interactive \ + --cap-add SYS_PTRACE \ + --network host \ + docker.io/golang $action diff --git a/hack/test-podman.sh b/hack/test-podman.sh new file mode 100755 index 000000000..86da65e9e --- /dev/null +++ b/hack/test-podman.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +cd $(dirname ${BASH_SOURCE[0]}) + +./podman.sh test diff --git a/pkg/kapp/cmd/app/deploy.go b/pkg/kapp/cmd/app/deploy.go index f57405502..7187615c6 100644 --- a/pkg/kapp/cmd/app/deploy.go +++ b/pkg/kapp/cmd/app/deploy.go @@ -416,7 +416,7 @@ func (o *DeployOptions) calculateAndPresentChanges(existingResources, changes, err := ctldiff.NewChangeSetWithVersionedRs( existingResources, newResources, conf.TemplateRules(), - o.DiffFlags.ChangeSetOpts, changeFactory).Calculate() + o.DiffFlags.ChangeSetOpts, changeFactory, conf.StripNameHashSuffixConfigs()).Calculate() if err != nil { return clusterChangeSet, nil, false, "", err } diff --git a/pkg/kapp/config/conf.go b/pkg/kapp/config/conf.go index 7b0c0fdb8..61fe54e35 100644 --- a/pkg/kapp/config/conf.go +++ b/pkg/kapp/config/conf.go @@ -178,3 +178,11 @@ func (c Conf) ChangeRuleBindings() []ChangeRuleBinding { } return result } + +func (c Conf) StripNameHashSuffixConfigs() []StripNameHashSuffixConfig { + var result []StripNameHashSuffixConfig + for _, config := range c.configs { + result = append(result, config.StripNameHashSuffixConfigs...) + } + return result +} diff --git a/pkg/kapp/config/config.go b/pkg/kapp/config/config.go index 175ed5fed..44da2564e 100644 --- a/pkg/kapp/config/config.go +++ b/pkg/kapp/config/config.go @@ -38,6 +38,8 @@ type Config struct { // TODO validations ChangeGroupBindings []ChangeGroupBinding ChangeRuleBindings []ChangeRuleBinding + + StripNameHashSuffixConfigs []StripNameHashSuffixConfig } type WaitRule struct { @@ -139,6 +141,11 @@ type ChangeRuleBinding struct { ResourceMatchers []ResourceMatcher } +type StripNameHashSuffixConfig struct { + Includes []ResourceMatcher + Excludes []ResourceMatcher +} + func NewConfigFromResource(res ctlres.Resource) (Config, error) { if res.APIVersion() != configAPIVersion { return Config{}, fmt.Errorf( @@ -314,3 +321,11 @@ func (r WaitRule) ResourceMatcher() ctlres.ResourceMatcher { Matchers: ResourceMatchers(r.ResourceMatchers).AsResourceMatchers(), } } + +func (r StripNameHashSuffixConfig) IncludeMatchers() []ctlres.ResourceMatcher { + return ResourceMatchers(r.Includes).AsResourceMatchers() +} + +func (r StripNameHashSuffixConfig) ExcludeMatchers() []ctlres.ResourceMatcher { + return ResourceMatchers(r.Excludes).AsResourceMatchers() +} diff --git a/pkg/kapp/diff/change_set_with_versioned_rs.go b/pkg/kapp/diff/change_set_with_versioned_rs.go index 4e12315de..e08c262f2 100644 --- a/pkg/kapp/diff/change_set_with_versioned_rs.go +++ b/pkg/kapp/diff/change_set_with_versioned_rs.go @@ -19,23 +19,24 @@ const ( ) type ChangeSetWithVersionedRs struct { - existingRs, newRs []ctlres.Resource - rules []ctlconf.TemplateRule - opts ChangeSetOpts - changeFactory ChangeFactory + existingRs, newRs []ctlres.Resource + rules []ctlconf.TemplateRule + opts ChangeSetOpts + changeFactory ChangeFactory + stripNameHashSuffixConfig stripNameHashSuffixConfig } func NewChangeSetWithVersionedRs(existingRs, newRs []ctlres.Resource, - rules []ctlconf.TemplateRule, opts ChangeSetOpts, changeFactory ChangeFactory) *ChangeSetWithVersionedRs { + rules []ctlconf.TemplateRule, opts ChangeSetOpts, changeFactory ChangeFactory, stripNameHashSuffixConfigs []ctlconf.StripNameHashSuffixConfig) *ChangeSetWithVersionedRs { - return &ChangeSetWithVersionedRs{existingRs, newRs, rules, opts, changeFactory} + return &ChangeSetWithVersionedRs{existingRs, newRs, rules, opts, changeFactory, newStripNameHashSuffixConfigFromConf(stripNameHashSuffixConfigs)} } func (d ChangeSetWithVersionedRs) Calculate() ([]Change, error) { - existingRs := existingVersionedResources(d.existingRs) + existingRs := d.existingVersionedResources() existingRsGrouped := d.groupResources(existingRs.Versioned) - newRs := newVersionedResources(d.newRs) + newRs := d.newVersionedResources() allChanges := []Change{} d.assignNewNames(newRs, existingRsGrouped) @@ -76,37 +77,33 @@ func (d ChangeSetWithVersionedRs) Calculate() ([]Change, error) { return allChanges, nil } -func (d ChangeSetWithVersionedRs) groupResources(rs []ctlres.Resource) map[string][]ctlres.Resource { - result := map[string][]ctlres.Resource{} +func (d ChangeSetWithVersionedRs) groupResources(vrs []VersionedResource) map[string][]VersionedResource { + result := map[string][]VersionedResource{} - groupByFunc := func(res ctlres.Resource) string { - if _, found := res.Annotations()[versionedResAnnKey]; found { - return VersionedResource{res, nil}.UniqVersionedKey().String() - } - panic("Expected to find versioned annotation on resource") + groupByFunc := func(ver VersionedResource) string { + return ver.UniqVersionedKey().String() } - for resKey, subRs := range (GroupResources{rs, groupByFunc}).Resources() { - sort.Slice(subRs, func(i, j int) bool { - return VersionedResource{subRs[i], nil}.Version() < VersionedResource{subRs[j], nil}.Version() + for resKey, subVRs := range (GroupVersionedResources{vrs, groupByFunc}).Resources() { + sort.Slice(subVRs, func(i, j int) bool { + return subVRs[i].Version() < subVRs[j].Version() }) - result[resKey] = subRs + result[resKey] = subVRs } return result } func (d ChangeSetWithVersionedRs) assignNewNames( - newRs versionedResources, existingRsGrouped map[string][]ctlres.Resource) { + newVRs versionedResources, existingVRsGrouped map[string][]VersionedResource) { // TODO name isnt used during diffing, should it? - for _, newRes := range newRs.Versioned { - newVerRes := VersionedResource{newRes, nil} + for _, newVerRes := range newVRs.Versioned { newResKey := newVerRes.UniqVersionedKey().String() - if existingRs, found := existingRsGrouped[newResKey]; found { - existingRes := existingRs[len(existingRs)-1] - newVerRes.SetBaseName(VersionedResource{existingRes, nil}.Version() + 1) + if existingVRs, found := existingVRsGrouped[newResKey]; found { + existingVerRes := existingVRs[len(existingVRs)-1] + newVerRes.SetBaseName(existingVerRes.Version() + 1) } else { newVerRes.SetBaseName(1) } @@ -114,38 +111,38 @@ func (d ChangeSetWithVersionedRs) assignNewNames( } func (d ChangeSetWithVersionedRs) addAndKeepChanges( - newRs versionedResources, existingRsGrouped map[string][]ctlres.Resource) ( - []Change, map[string]ctlres.Resource, error) { + newVRs versionedResources, existingVRsGrouped map[string][]VersionedResource) ( + []Change, map[string]VersionedResource, error) { changes := []Change{} - alreadyAdded := map[string]ctlres.Resource{} + alreadyAdded := map[string]VersionedResource{} - for _, newRes := range newRs.Versioned { - newResKey := VersionedResource{newRes, nil}.UniqVersionedKey().String() - usedRes := newRes + for _, newVerRes := range newVRs.Versioned { + newResKey := newVerRes.UniqVersionedKey().String() + usedVerRes := newVerRes - if existingRs, found := existingRsGrouped[newResKey]; found { - existingRes := existingRs[len(existingRs)-1] + if existingVRs, found := existingVRsGrouped[newResKey]; found { + existingVerRes := existingVRs[len(existingVRs)-1] // Calculate update change to determine if anything changed - updateChange, err := d.newChange(existingRes, newRes) + updateChange, err := d.newChange(existingVerRes.Res(), newVerRes.Res()) if err != nil { return nil, nil, err } switch updateChange.Op() { case ChangeOpUpdate: - changes = append(changes, d.newAddChangeFromUpdateChange(newRes, updateChange)) + changes = append(changes, d.newAddChangeFromUpdateChange(newVerRes.Res(), updateChange)) case ChangeOpKeep: // Use latest copy of resource to update affected resources - usedRes = existingRes - changes = append(changes, d.newKeepChange(existingRes)) + usedVerRes = existingVerRes + changes = append(changes, d.newKeepChange(existingVerRes.Res())) default: panic(fmt.Sprintf("Unexpected change op %s", updateChange.Op())) } } else { // Since there no existing resource, create change for new resource - addChange, err := d.newChange(nil, newRes) + addChange, err := d.newChange(nil, newVerRes.Res()) if err != nil { return nil, nil, err } @@ -153,19 +150,18 @@ func (d ChangeSetWithVersionedRs) addAndKeepChanges( } // Update both versioned and non-versioned - verRes := VersionedResource{usedRes, d.rules} - err := verRes.UpdateAffected(newRs.NonVersioned) + err := usedVerRes.UpdateAffected(newVRs.NonVersionedRs()) if err != nil { return nil, nil, err } - err = verRes.UpdateAffected(newRs.Versioned) + err = usedVerRes.UpdateAffected(newVRs.VersionedRs()) if err != nil { return nil, nil, err } - alreadyAdded[newResKey] = newRes + alreadyAdded[newResKey] = newVerRes } return changes, alreadyAdded, nil @@ -179,29 +175,41 @@ func (d ChangeSetWithVersionedRs) newAddChangeFromUpdateChange( } func (d ChangeSetWithVersionedRs) noopAndDeleteChanges( - existingRsGrouped map[string][]ctlres.Resource, - alreadyAdded map[string]ctlres.Resource) ([]Change, error) { + existingVRsGrouped map[string][]VersionedResource, + alreadyAdded map[string]VersionedResource) ([]Change, error) { changes := []Change{} // Find existing resources that were not already diffed (not in new set of resources) - for existingResKey, existingRs := range existingRsGrouped { + for existingResKey, existingVRs := range existingVRsGrouped { numToKeep := 0 - if newRes, found := alreadyAdded[existingResKey]; found { + newVRes, found := alreadyAdded[existingResKey] + if found { var err error - numToKeep, err = d.numOfResourcesToKeep(newRes) + numToKeep, err = d.numOfResourcesToKeep(newVRes) if err != nil { return nil, err } } - if numToKeep > len(existingRs) { - numToKeep = len(existingRs) + if numToKeep > len(existingVRs) { + numToKeep = len(existingVRs) } // Create changes to delete all or extra resources - for _, existingRes := range existingRs[0 : len(existingRs)-numToKeep] { - change, err := d.newChange(existingRes, nil) + for _, existingVRes := range existingVRs[0 : len(existingVRs)-numToKeep] { + + existingRes := existingVRes.Res() + + var newRes ctlres.Resource + if numToKeep == 0 && found && existingRes.Name() == newVRes.Res().Name() { + // versioned resources without "last X" semantics would also + // delete the not-actually-old existing resource when it is + // reapplied without changes. + newRes = newVRes.Res() + } + + change, err := d.newChange(existingRes, newRes) if err != nil { return nil, err } @@ -209,8 +217,8 @@ func (d ChangeSetWithVersionedRs) noopAndDeleteChanges( } // Create changes that "noop" resources - for _, existingRes := range existingRs[len(existingRs)-numToKeep:] { - changes = append(changes, d.newNoopChange(existingRes)) + for _, existingVRes := range existingVRs[len(existingVRs)-numToKeep:] { + changes = append(changes, d.newNoopChange(existingVRes.Res())) } } @@ -225,9 +233,17 @@ func (d ChangeSetWithVersionedRs) newNoopChange(existingRes ctlres.Resource) Cha return NewChangePrecalculated(existingRes, nil, nil, ChangeOpNoop, nil, OpsDiff{}) } -func (ChangeSetWithVersionedRs) numOfResourcesToKeep(res ctlres.Resource) (int, error) { +func (ChangeSetWithVersionedRs) numOfResourcesToKeep(vres VersionedResource) (int, error) { + switch vres.(type) { + case HashSuffixResource: + // there is no meaningful way to order hash-suffixed resources and as such there is no "last X" resources semantic. + // thus simply delete all old resources. + return 0, nil + } + // TODO get rid of arbitrary cut off numToKeep := 5 + res := vres.Res() if numToKeepAnn, found := res.Annotations()[versionedResNumVersAnnKey]; found { var err error @@ -254,44 +270,86 @@ func (d ChangeSetWithVersionedRs) newChange(existingRes, newRes ctlres.Resource) } type versionedResources struct { - Versioned []ctlres.Resource + Versioned []VersionedResource NonVersioned []ctlres.Resource } -func newVersionedResources(rs []ctlres.Resource) versionedResources { +func (d *versionedResources) AddVersionedRes(vr VersionedResource) { + d.Versioned = append(d.Versioned, vr) +} + +func (d *versionedResources) AddNonVersionedRes(res ctlres.Resource) { + d.NonVersioned = append(d.NonVersioned, res) +} + +func (d versionedResources) VersionedRs() []ctlres.Resource { + var rs []ctlres.Resource + for _, vres := range d.Versioned { + rs = append(rs, vres.Res()) + } + return rs +} + +func (d versionedResources) NonVersionedRs() []ctlres.Resource { + return d.NonVersioned +} + +func (d ChangeSetWithVersionedRs) newVersionedResources() versionedResources { var result versionedResources - for _, res := range rs { + + for _, res := range d.newRs { + _, hasVersionedAnn := res.Annotations()[versionedResAnnKey] _, hasVersionedOrigAnn := res.Annotations()[versionedResOrigAnnKey] if hasVersionedAnn { - result.Versioned = append(result.Versioned, res) + result.AddVersionedRes(VersionedResourceImpl{res, d.rules}) if hasVersionedOrigAnn { - result.NonVersioned = append(result.NonVersioned, res.DeepCopy()) + result.AddNonVersionedRes(res.DeepCopy()) } - } else { - result.NonVersioned = append(result.NonVersioned, res) + continue } + + if d.stripNameHashSuffixConfig.EnabledFor(res) { + result.AddVersionedRes(HashSuffixResource{res}) + continue + } + + result.AddNonVersionedRes(res) } + return result } -func existingVersionedResources(rs []ctlres.Resource) versionedResources { +func (d ChangeSetWithVersionedRs) existingVersionedResources() versionedResources { var result versionedResources - for _, res := range rs { + + for _, res := range d.existingRs { + // Expect that versioned resources should not be transient // (Annotations may have been copied from versioned resources // onto transient resources for non-versioning related purposes). - _, hasVersionedAnn := res.Annotations()[versionedResAnnKey] + if !res.Transient() { + + _, hasVersionedAnn := res.Annotations()[versionedResAnnKey] + if hasVersionedAnn { + versionedRes := VersionedResourceImpl{res, d.rules} + _, version := versionedRes.BaseNameAndVersion() + if version != "" { + result.AddVersionedRes(versionedRes) + continue + } + } - versionedRs := VersionedResource{res: res} - _, version := versionedRs.BaseNameAndVersion() + if d.stripNameHashSuffixConfig.EnabledFor(res) { + result.AddVersionedRes(HashSuffixResource{res}) + continue + } - if hasVersionedAnn && !res.Transient() && version != "" { - result.Versioned = append(result.Versioned, res) - } else { - result.NonVersioned = append(result.NonVersioned, res) } + + result.AddNonVersionedRes(res) } + return result } diff --git a/pkg/kapp/diff/change_set_with_versioned_rs_test.go b/pkg/kapp/diff/change_set_with_versioned_rs_test.go index 66db1d906..41bf9887f 100644 --- a/pkg/kapp/diff/change_set_with_versioned_rs_test.go +++ b/pkg/kapp/diff/change_set_with_versioned_rs_test.go @@ -27,9 +27,7 @@ metadata: kapp.k14s.io/versioned: "" `)) - changeSetWithVerRes := NewChangeSetWithVersionedRs([]ctlres.Resource{existingRes}, []ctlres.Resource{newRs}, nil, - ChangeSetOpts{}, ChangeFactory{}) - + changeSetWithVerRes := newChangeSet(existingRes, newRs) changes, err := changeSetWithVerRes.Calculate() require.NoError(t, err) @@ -75,8 +73,7 @@ metadata: name: secret `)) - changeSetWithVerRes := NewChangeSetWithVersionedRs([]ctlres.Resource{existingRes}, []ctlres.Resource{newRs}, nil, - ChangeSetOpts{}, ChangeFactory{}) + changeSetWithVerRes := newChangeSet(existingRes, newRs) changes, err := changeSetWithVerRes.Calculate() require.NoError(t, err) @@ -124,8 +121,7 @@ metadata: name: secret `)) - changeSetWithVerRes := NewChangeSetWithVersionedRs([]ctlres.Resource{existingRes}, []ctlres.Resource{newRs}, nil, - ChangeSetOpts{}, ChangeFactory{}) + changeSetWithVerRes := newChangeSet(existingRes, newRs) changes, err := changeSetWithVerRes.Calculate() require.NoError(t, err) @@ -157,6 +153,53 @@ metadata: 4, 7 ` checkChangeDiff(t, changes[1], expectedDiff2) + +} + +func TestChangeSet_StripKustomizeSuffix(t *testing.T) { + + existingRs := ctlres.MustNewResourceFromBytes([]byte(` +apiVersion: v1 +kind: ConfigMap +metadata: + name: configmap-abc +data: + foo: foo +`)) + + newRs := ctlres.MustNewResourceFromBytes([]byte(` +apiVersion: v1 +kind: ConfigMap +metadata: + name: configmap-cdf +data: + foo: bar +`)) + + stripNameHashSuffix := stripNameHashSuffixConfig{ResourceMatcher: ctlres.AllMatcher{}} + changeSetWithVerRes := ChangeSetWithVersionedRs{[]ctlres.Resource{existingRs}, []ctlres.Resource{newRs}, nil, ChangeSetOpts{}, ChangeFactory{}, stripNameHashSuffix} + + changes, err := changeSetWithVerRes.Calculate() + require.NoError(t, err) + + expectedDiff := ` 0, 0 apiVersion: v1 + 1, 1 data: + 2, 2 - foo: foo + 3, 2 + foo: bar + 3, 3 kind: ConfigMap + 4, 4 metadata: + 5, 5 - name: configmap-abc + 6, 5 + name: configmap-cdf + 6, 6 +` + + checkChangeDiff(t, changes[0], expectedDiff) + +} + +func newChangeSet(existingRes, newRes ctlres.Resource) *ChangeSetWithVersionedRs { + stripNameHashSuffix := newStripNameHashSuffixConfigEmpty() + return &ChangeSetWithVersionedRs{[]ctlres.Resource{existingRes}, []ctlres.Resource{newRes}, nil, ChangeSetOpts{}, ChangeFactory{}, stripNameHashSuffix} } func checkChangeDiff(t *testing.T, change Change, expectedDiff string) { diff --git a/pkg/kapp/diff/group_resources.go b/pkg/kapp/diff/group_resources.go index 8d54d256b..b5ada61ed 100644 --- a/pkg/kapp/diff/group_resources.go +++ b/pkg/kapp/diff/group_resources.go @@ -3,17 +3,13 @@ package diff -import ( - ctlres "github.com/vmware-tanzu/carvel-kapp/pkg/kapp/resources" -) - -type GroupResources struct { - resources []ctlres.Resource - groupFunc func(ctlres.Resource) string +type GroupVersionedResources struct { + resources []VersionedResource + groupFunc func(VersionedResource) string } -func (r GroupResources) Resources() map[string][]ctlres.Resource { - result := map[string][]ctlres.Resource{} +func (r GroupVersionedResources) Resources() map[string][]VersionedResource { + result := map[string][]VersionedResource{} for _, res := range r.resources { id := r.groupFunc(res) diff --git a/pkg/kapp/diff/stripnamehashsuffix.go b/pkg/kapp/diff/stripnamehashsuffix.go new file mode 100644 index 000000000..75615ab84 --- /dev/null +++ b/pkg/kapp/diff/stripnamehashsuffix.go @@ -0,0 +1,76 @@ +// Copyright 2020 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package diff + +import ( + "strings" + + ctlconf "github.com/vmware-tanzu/carvel-kapp/pkg/kapp/config" + ctlres "github.com/vmware-tanzu/carvel-kapp/pkg/kapp/resources" +) + +type stripNameHashSuffixConfig struct { + ResourceMatcher ctlres.ResourceMatcher +} + +func (d stripNameHashSuffixConfig) EnabledFor(res ctlres.Resource) (stripEnabled bool) { + return d.ResourceMatcher.Matches(res) +} + +func newStripNameHashSuffixConfigFromConf(confs []ctlconf.StripNameHashSuffixConfig) stripNameHashSuffixConfig { + includeMatchers := []ctlres.ResourceMatcher{} + excludeMatchers := []ctlres.ResourceMatcher{} + for _, conf := range confs { + includeMatchers = append(includeMatchers, conf.IncludeMatchers()...) + excludeMatchers = append(excludeMatchers, conf.ExcludeMatchers()...) + } + return stripNameHashSuffixConfig{ + ResourceMatcher: ctlres.AndMatcher{ + Matchers: []ctlres.ResourceMatcher{ + ctlres.AnyMatcher{Matchers: includeMatchers}, + ctlres.NotMatcher{ + Matcher: ctlres.AnyMatcher{Matchers: excludeMatchers}, + }, + }, + }, + } +} + +func newStripNameHashSuffixConfigEmpty() stripNameHashSuffixConfig { + return newStripNameHashSuffixConfigFromConf([]ctlconf.StripNameHashSuffixConfig{}) +} + +type HashSuffixResource struct { + res ctlres.Resource +} + +func (d HashSuffixResource) Res() ctlres.Resource { + return d.res +} + +func (d HashSuffixResource) SetBaseName(ver int) { + // not necessary +} + +func (d HashSuffixResource) BaseNameAndVersion() (string, string) { + pieces := strings.Split(d.res.Name(), "-") + if len(pieces) > 1 { + return strings.Join(pieces[0:len(pieces)-1], "-"), "" + } + panic("expected suffix!") +} + +func (d HashSuffixResource) Version() int { + return -1 +} + +func (d HashSuffixResource) UniqVersionedKey() ctlres.UniqueResourceKey { + baseName, _ := d.BaseNameAndVersion() + return ctlres.NewUniqueResourceKeyWithCustomName(d.res, baseName) +} + +func (d HashSuffixResource) UpdateAffected(rs []ctlres.Resource) error { + // no updates required + return nil +} diff --git a/pkg/kapp/diff/stripnamehashsuffix_test.go b/pkg/kapp/diff/stripnamehashsuffix_test.go new file mode 100644 index 000000000..ff8303496 --- /dev/null +++ b/pkg/kapp/diff/stripnamehashsuffix_test.go @@ -0,0 +1,162 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package diff + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + ctlconf "github.com/vmware-tanzu/carvel-kapp/pkg/kapp/config" + ctlres "github.com/vmware-tanzu/carvel-kapp/pkg/kapp/resources" +) + +func TestStripNameHashSuffix_TestConfigIncludeExclude(t *testing.T) { + + includedCM := ctlres.MustNewResourceFromBytes([]byte(` +apiVersion: v1 +kind: ConfigMap +metadata: + name: configmap-abc +data: + foo: foo +`)) + + excludedCM := ctlres.MustNewResourceFromBytes([]byte(` +apiVersion: v1 +kind: ConfigMap +metadata: + name: foo +data: + foo: foo +`)) + + includedKind := ctlres.MustNewResourceFromBytes([]byte(` +apiVersion: v2 +kind: MyKind +metadata: + name: my-res +spec: + key: val +`)) + + excludedKind := ctlres.MustNewResourceFromBytes([]byte(` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: bar +spec: + replicas: 1 +`)) + + includes := []ctlconf.ResourceMatcher{ + ctlconf.ResourceMatcher{ + APIVersionKindMatcher: &ctlconf.APIVersionKindMatcher{APIVersion: includedCM.APIVersion(), Kind: includedCM.Kind()}, + }, + ctlconf.ResourceMatcher{ + APIVersionKindMatcher: &ctlconf.APIVersionKindMatcher{APIVersion: includedKind.APIVersion(), Kind: includedKind.Kind()}, + }, + } + + excludes := []ctlconf.ResourceMatcher{ + ctlconf.ResourceMatcher{ + KindNamespaceNameMatcher: &ctlconf.KindNamespaceNameMatcher{Kind: excludedCM.Kind(), Name: excludedCM.Name()}, + }, + } + + configCases := []struct { + desc string + confs []ctlconf.StripNameHashSuffixConfig + }{ + { + "SingleConfig", + []ctlconf.StripNameHashSuffixConfig{ + ctlconf.StripNameHashSuffixConfig{ + Includes: includes, + Excludes: excludes, + }, + }, + }, + { + "AccrossConfigs", + []ctlconf.StripNameHashSuffixConfig{ + ctlconf.StripNameHashSuffixConfig{ + Includes: includes, + }, + ctlconf.StripNameHashSuffixConfig{ + Excludes: excludes, + }, + }, + }, + } + + for _, configTc := range configCases { + t.Run(configTc.desc, func(t *testing.T) { + + config := newStripNameHashSuffixConfigFromConf(configTc.confs) + + resCases := []struct { + kind string + res ctlres.Resource + expected bool + }{ + {"ConfigMap", includedCM, true}, + {"ConfigMap", excludedCM, false}, + {"Kind", includedKind, true}, + {"Kind", excludedKind, false}, + } + + for _, resTc := range resCases { + var inEx, not string + if resTc.expected { + inEx = "included" + not = "" + } else { + inEx = "excluded" + not = " not" + } + desc := fmt.Sprintf("%s_%s", resTc.kind, inEx) + t.Run(desc, func(t *testing.T) { + require.Equalf(t, resTc.expected, config.EnabledFor(resTc.res), "expected %s %s to%s match!", inEx, resTc.kind, not) + }) + } + }) + } +} + +func TestStripNameHashSuffix_TestConfig_DefaultNone(t *testing.T) { + excludedCM := ctlres.MustNewResourceFromBytes([]byte(` +apiVersion: v1 +kind: ConfigMap +metadata: + name: foo +data: + foo: foo +`)) + + excludedKind := ctlres.MustNewResourceFromBytes([]byte(` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: bar +spec: + replicas: 1 +`)) + + config := newStripNameHashSuffixConfigFromConf([]ctlconf.StripNameHashSuffixConfig{}) + + cases := []struct { + kind string + res ctlres.Resource + }{ + {"ConfigMap", excludedCM}, + {"Deployment", excludedKind}, + } + + for _, tc := range cases { + t.Run(tc.kind, func(t *testing.T) { + require.Falsef(t, config.EnabledFor(excludedCM), "expected %s to not match by default!") + }) + } +} diff --git a/pkg/kapp/diff/versioned_resource.go b/pkg/kapp/diff/versioned_resource.go index 1240907f1..206a36f66 100644 --- a/pkg/kapp/diff/versioned_resource.go +++ b/pkg/kapp/diff/versioned_resource.go @@ -18,17 +18,30 @@ const ( nameSuffixSep = "-ver-" ) -type VersionedResource struct { +type VersionedResource interface { + Res() ctlres.Resource + SetBaseName(ver int) + BaseNameAndVersion() (string, string) + Version() int + UniqVersionedKey() ctlres.UniqueResourceKey + UpdateAffected(rs []ctlres.Resource) error +} + +type VersionedResourceImpl struct { res ctlres.Resource allRules []ctlconf.TemplateRule } -func (d VersionedResource) SetBaseName(ver int) { +func (d VersionedResourceImpl) Res() ctlres.Resource { + return d.res +} + +func (d VersionedResourceImpl) SetBaseName(ver int) { name := fmt.Sprintf("%s%s%d", d.res.Name(), nameSuffixSep, ver) d.res.SetName(name) } -func (d VersionedResource) BaseNameAndVersion() (string, string) { +func (d VersionedResourceImpl) BaseNameAndVersion() (string, string) { name := d.res.Name() pieces := strings.Split(name, nameSuffixSep) if len(pieces) > 1 { @@ -37,7 +50,7 @@ func (d VersionedResource) BaseNameAndVersion() (string, string) { return name, "" } -func (d VersionedResource) Version() int { +func (d VersionedResourceImpl) Version() int { _, ver := d.BaseNameAndVersion() if len(ver) == 0 { panic(fmt.Sprintf("Missing version in versioned resource '%s'", d.res.Description())) @@ -51,12 +64,12 @@ func (d VersionedResource) Version() int { return verInt } -func (d VersionedResource) UniqVersionedKey() ctlres.UniqueResourceKey { +func (d VersionedResourceImpl) UniqVersionedKey() ctlres.UniqueResourceKey { baseName, _ := d.BaseNameAndVersion() return ctlres.NewUniqueResourceKeyWithCustomName(d.res, baseName) } -func (d VersionedResource) UpdateAffected(rs []ctlres.Resource) error { +func (d VersionedResourceImpl) UpdateAffected(rs []ctlres.Resource) error { rules, err := d.matchingRules() if err != nil { return err @@ -73,7 +86,7 @@ func (d VersionedResource) UpdateAffected(rs []ctlres.Resource) error { return nil } -func (d VersionedResource) updateAffected(rule ctlconf.TemplateRule, rs []ctlres.Resource) error { +func (d VersionedResourceImpl) updateAffected(rule ctlconf.TemplateRule, rs []ctlres.Resource) error { for _, affectedObjRef := range rule.AffectedResources.ObjectReferences { matchers := ctlconf.ResourceMatchers(affectedObjRef.ResourceMatchers).AsResourceMatchers() @@ -122,7 +135,7 @@ func (d VersionedResource) updateAffected(rule ctlconf.TemplateRule, rs []ctlres return nil } -func (d VersionedResource) buildObjRefReplacementFunc( +func (d VersionedResourceImpl) buildObjRefReplacementFunc( affectedObjRef ctlconf.TemplateAffectedObjRef) func(map[string]interface{}) error { baseName, _ := d.BaseNameAndVersion() @@ -171,7 +184,7 @@ func (d VersionedResource) buildObjRefReplacementFunc( } } -func (d VersionedResource) matchingRules() ([]ctlconf.TemplateRule, error) { +func (d VersionedResourceImpl) matchingRules() ([]ctlconf.TemplateRule, error) { var result []ctlconf.TemplateRule for _, rule := range d.allRules { diff --git a/pkg/kapp/resources/matchers.go b/pkg/kapp/resources/matchers.go index 28b31124e..9f8049ab2 100644 --- a/pkg/kapp/resources/matchers.go +++ b/pkg/kapp/resources/matchers.go @@ -77,6 +77,9 @@ type AndMatcher struct { var _ ResourceMatcher = AndMatcher{} func (m AndMatcher) Matches(res Resource) bool { + if m.Matchers == nil { + return false + } for _, m := range m.Matchers { if !m.Matches(res) { return false diff --git a/test/e2e/res.go b/test/e2e/res.go new file mode 100644 index 000000000..a26c5ca2b --- /dev/null +++ b/test/e2e/res.go @@ -0,0 +1,25 @@ +// Copyright 2022 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "bytes" + "embed" + "io" +) + +//go:embed res +var testResFs embed.FS + +func TestResReader(path string) (io.Reader, error) { + b, err := TestResBytes(path) + if err != nil { + return nil, err + } + return bytes.NewReader(b), nil +} + +func TestResBytes(path string) ([]byte, error) { + return testResFs.ReadFile(path) +} diff --git a/test/e2e/res/kustomize/README.md b/test/e2e/res/kustomize/README.md new file mode 100644 index 000000000..986411347 --- /dev/null +++ b/test/e2e/res/kustomize/README.md @@ -0,0 +1,3 @@ +Test Resources for kustomize name-hash-suffix stripping. + +The files used by kapp are `overlay/*/kapp.yml`. (Re-)generate them using `./gen.sh`. diff --git a/test/e2e/res/kustomize/components/kapp-config/kapp-cm.yml b/test/e2e/res/kustomize/components/kapp-config/kapp-cm.yml new file mode 100644 index 000000000..d30041ec8 --- /dev/null +++ b/test/e2e/res/kustomize/components/kapp-config/kapp-cm.yml @@ -0,0 +1,8 @@ +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config +metadata: + name: irrelevant + +stripNameHashSuffixConfigs: +- includes: + - apiVersionKindMatcher: {apiVersion: v1, kind: ConfigMap} diff --git a/test/e2e/res/kustomize/components/kapp-config/kustomization.yaml b/test/e2e/res/kustomize/components/kapp-config/kustomization.yaml new file mode 100644 index 000000000..559cef01f --- /dev/null +++ b/test/e2e/res/kustomize/components/kapp-config/kustomization.yaml @@ -0,0 +1,5 @@ +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component + +resources: + - kapp-cm.yml diff --git a/test/e2e/res/kustomize/gen.sh b/test/e2e/res/kustomize/gen.sh new file mode 100755 index 000000000..1ba4306d3 --- /dev/null +++ b/test/e2e/res/kustomize/gen.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash +cd $(dirname ${BASH_SOURCE}) +for overlay in overlays/*/ +do + kustomize build $overlay -o $overlay/kapp.yml +done diff --git a/test/e2e/res/kustomize/overlays/versioned1/kapp.yml b/test/e2e/res/kustomize/overlays/versioned1/kapp.yml new file mode 100644 index 000000000..b13525143 --- /dev/null +++ b/test/e2e/res/kustomize/overlays/versioned1/kapp.yml @@ -0,0 +1,16 @@ +apiVersion: v1 +data: + foo: foo +kind: ConfigMap +metadata: + name: config-map-7tgk8db28b +--- +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config +metadata: + name: irrelevant +stripNameHashSuffixConfigs: +- includes: + - apiVersionKindMatcher: + apiVersion: v1 + kind: ConfigMap diff --git a/test/e2e/res/kustomize/overlays/versioned1/kustomization.yaml b/test/e2e/res/kustomize/overlays/versioned1/kustomization.yaml new file mode 100644 index 000000000..6a8afcd23 --- /dev/null +++ b/test/e2e/res/kustomize/overlays/versioned1/kustomization.yaml @@ -0,0 +1,10 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +components: + - ../../components/kapp-config + +configMapGenerator: + - name: config-map + literals: + - foo=foo diff --git a/test/e2e/res/kustomize/overlays/versioned2/kapp.yml b/test/e2e/res/kustomize/overlays/versioned2/kapp.yml new file mode 100644 index 000000000..d351ae197 --- /dev/null +++ b/test/e2e/res/kustomize/overlays/versioned2/kapp.yml @@ -0,0 +1,16 @@ +apiVersion: v1 +data: + foo: bar +kind: ConfigMap +metadata: + name: config-map-798k5k7g9f +--- +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config +metadata: + name: irrelevant +stripNameHashSuffixConfigs: +- includes: + - apiVersionKindMatcher: + apiVersion: v1 + kind: ConfigMap diff --git a/test/e2e/res/kustomize/overlays/versioned2/kustomization.yaml b/test/e2e/res/kustomize/overlays/versioned2/kustomization.yaml new file mode 100644 index 000000000..aa5a0bfa8 --- /dev/null +++ b/test/e2e/res/kustomize/overlays/versioned2/kustomization.yaml @@ -0,0 +1,10 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +components: + - ../../components/kapp-config + +configMapGenerator: + - name: config-map + literals: + - foo=bar diff --git a/test/e2e/res/kustomize/overlays/versioned3/kapp.yml b/test/e2e/res/kustomize/overlays/versioned3/kapp.yml new file mode 100644 index 000000000..b6e9e87fd --- /dev/null +++ b/test/e2e/res/kustomize/overlays/versioned3/kapp.yml @@ -0,0 +1,16 @@ +apiVersion: v1 +data: + foo: bal +kind: ConfigMap +metadata: + name: config-map-5bghbfbdc8 +--- +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config +metadata: + name: irrelevant +stripNameHashSuffixConfigs: +- includes: + - apiVersionKindMatcher: + apiVersion: v1 + kind: ConfigMap diff --git a/test/e2e/res/kustomize/overlays/versioned3/kustomization.yaml b/test/e2e/res/kustomize/overlays/versioned3/kustomization.yaml new file mode 100644 index 000000000..6fd6655e1 --- /dev/null +++ b/test/e2e/res/kustomize/overlays/versioned3/kustomization.yaml @@ -0,0 +1,10 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +components: + - ../../components/kapp-config + +configMapGenerator: + - name: config-map + literals: + - foo=bal diff --git a/test/e2e/versioned_stripnamesuffix_test.go b/test/e2e/versioned_stripnamesuffix_test.go new file mode 100644 index 000000000..3f7597ed3 --- /dev/null +++ b/test/e2e/versioned_stripnamesuffix_test.go @@ -0,0 +1,99 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "fmt" + "regexp" + "testing" + + ui "github.com/cppforlife/go-cli-ui/ui" + uitest "github.com/cppforlife/go-cli-ui/ui/test" + "github.com/stretchr/testify/require" +) + +func run(t *testing.T, overlay1, overlay2 string) (resp ui.JSONUIResp) { + env := BuildEnv(t) + logger := Logger{} + kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger} + + appName := "test-versioned-stripnamesuffix" + cleanUp := func() { + kapp.Run([]string{"delete", "-a", appName}) + } + + cleanUp() + + if _, err := kappDeployOverlay(kapp, overlay1, appName); err != nil { + t.Errorf("Failed to deploy initial overlay!") + } + + stdout, err := kappDeployOverlay(kapp, overlay2, appName) + + defer cleanUp() + + if err != nil { + t.Errorf("Failed to deploy next overlay!") + } + + return uitest.JSONUIFromBytes(t, []byte(stdout)) +} + +func testResOverlayPath(name string) string { + return fmt.Sprintf("res/kustomize/overlays/%s/kapp.yml", name) +} + +func kappDeployOverlay(kapp Kapp, name string, app string) (string, error) { + testResReader, err := TestResReader(testResOverlayPath(name)) + if err != nil { + return "", fmt.Errorf("Could not load overlay for %s! %w", name, err) + } + return kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", app, "--json", "-c"}, RunOpts{IntoNs: true, StdinReader: testResReader}) +} + +func TestStripNameSuffix(t *testing.T) { + t.Run("Basic", testStripNameSuffixBasic) + t.Run("NoOp", testStripNameSuffixNoop) +} + +func testStripNameSuffixBasic(t *testing.T) { + + resp := run(t, "versioned1", "versioned2") + + for _, tc := range []struct { + name string + regex *regexp.Regexp + }{ + // comparing the whole diff is unreliable; depending on the k8s version + // managedFields will (not) be set and as such (not) included in the diff. + // in that regard it also seems unreliable to assume "data" diff will + // always be in the first lines. + {"new", regexp.MustCompile(`(?m)^ \d+ \+ foo: bar$`)}, + {"old", regexp.MustCompile(`(?m)^ \d+ \- foo: foo$`)}, + } { + t.Run("Diff_"+tc.name, func(t *testing.T) { + diffBlock := resp.Blocks[0] + require.Regexpf(t, tc.regex, diffBlock, "Expected to see %s line in diff", tc.name) + }) + } + + t.Run("DeleteOld", func(t *testing.T) { + expectedNote := "Op: 1 create, 1 delete, 0 update, 0 noop, 0 exists" + actualNote := resp.Tables[0].Notes[0] + // Ensure old ConfigMap is deleted + require.Exactlyf(t, expectedNote, actualNote, "Expected one delete and one create Op") + }) +} + +func testStripNameSuffixNoop(t *testing.T) { + + resp := run(t, "versioned1", "versioned1") + + expectedNote := "Op: 0 create, 0 delete, 0 update, 0 noop, 0 exists" + + actualNote := resp.Tables[0].Notes[0] + + // Ensure current ConfigMap is not deleted + require.Exactlyf(t, expectedNote, actualNote, "Expected to see no Op's") +}