Filter resources by label to catch aggregated api mistakes

.golangci.yml

@@ -12,4 +12,7 @@ linters-settings:
     template-path: code-header-template.txt
 issues:
   max-issues-per-linter: 0
-  max-same-issues: 0
+  max-same-issues: 0
+  exclude:
+    # exclude stuttering linting error
+    - ".*and that stutters;.*"

go.mod

@@ -23,6 +23,7 @@ require (
 	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
 	github.com/spf13/cobra v1.1.1
 	github.com/spf13/pflag v1.0.5
+	github.com/stretchr/testify v1.7.0
 	golang.org/x/net v0.0.0-20190620200207-3b0461eec859
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.2.8

go.sum

@@ -232,10 +232,13 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/spf13/viper v1.7.0/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5qpdg=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
+github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
+github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/vito/go-interact v0.0.0-20171111012221-fa338ed9e9ec h1:Klu98tQ9Z1t23gvC7p7sCmvxkZxLhBHLNyrUPsWsYFg=
@@ -400,6 +403,8 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
 gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

pkg/kapp/cmd/app/factory.go

@@ -38,7 +38,7 @@ func FactoryClients(depsFactory cmdcore.DepsFactory, nsFlags cmdcore.NamespaceFl
 		CanIgnoreFailingAPIService: resTypesFlags.CanIgnoreFailingAPIService,
 	})
 
-	resources := ctlres.NewResources(resTypes, coreClient, dynamicClient, fallbackAllowedNss, logger)
+	resources := ctlres.NewResourcesImpl(resTypes, coreClient, dynamicClient, fallbackAllowedNss, logger)
 
 	identifiedResources := ctlres.NewIdentifiedResources(
 		coreClient, resTypes, resources, fallbackAllowedNss, logger)

pkg/kapp/cmd/tools/list_labels.go

@@ -128,7 +128,7 @@ func (o *ListLabelsOptions) listResources() ([]ctlres.Resource, error) {
 	}
 
 	resTypes := ctlres.NewResourceTypesImpl(coreClient, ctlres.ResourceTypesImplOpts{})
-	resources := ctlres.NewResources(resTypes, coreClient, dynamicClient, nil, o.logger)
+	resources := ctlres.NewResourcesImpl(resTypes, coreClient, dynamicClient, nil, o.logger)
 	identifiedResources := ctlres.NewIdentifiedResources(coreClient, resTypes, resources, nil, o.logger)
 
 	labelSelector, err := labels.Parse("!kapp")

pkg/kapp/resources/identified_resources.go

@@ -14,13 +14,13 @@ import (
 type IdentifiedResources struct {
 	coreClient                kubernetes.Interface
 	resourceTypes             ResourceTypes
-	resources                 *Resources
+	resources                 Resources
 	fallbackAllowedNamespaces []string
 	logger                    logger.Logger
 }
 
 func NewIdentifiedResources(coreClient kubernetes.Interface, resourceTypes ResourceTypes,
-	resources *Resources, fallbackAllowedNamespaces []string, logger logger.Logger) IdentifiedResources {
+	resources Resources, fallbackAllowedNamespaces []string, logger logger.Logger) IdentifiedResources {
 
 	return IdentifiedResources{coreClient, resourceTypes, resources,
 		fallbackAllowedNamespaces, logger.NewPrefixed("IdentifiedResources")}

pkg/kapp/resources/identified_resources_list.go

@@ -47,6 +47,18 @@ func (r IdentifiedResources) List(labelSelector labels.Selector, resRefs []Resou
 		return nil, err
 	}
 
+	// Check returned resources against label selector
+	// in case of Kubernetes APIs returned resources that do not match.
+	// This can happen if custom aggregated APIs did not implement label selector filtering.
+	// (https://github.com/vmware-tanzu/carvel-kapp/issues/160)
+	var filteredResources []Resource
+	for _, res := range resources {
+		if labelSelector.Matches(labels.Set(res.Labels())) {
+			filteredResources = append(filteredResources, res)
+		}
+	}
+	resources = filteredResources
+
 	// Mark resources that were not created by kapp as transient
 	for i, res := range resources {
 		if !NewIdentityAnnotation(res).Valid() {

pkg/kapp/resources/identified_resources_list_test.go

@@ -0,0 +1,74 @@
+// Copyright 2021 VMware, Inc.
+// SPDX-License-Identifier: Apache-2.0
+package resources_test
+
+import (
+	"testing"
+
+	"github.com/cppforlife/go-cli-ui/ui"
+	"github.com/k14s/kapp/pkg/kapp/logger"
+	ctlres "github.com/k14s/kapp/pkg/kapp/resources"
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
+)
+
+func TestIdentifiedResourcesListReturnsLabeledResources(t *testing.T) {
+	fakeResourceTypes := &FakeResourceTypes{}
+	fakeResources := &FakeResources{t}
+
+	identifiedResources := ctlres.NewIdentifiedResources(nil, fakeResourceTypes, fakeResources, []string{}, logger.NewUILogger(ui.NewNoopUI()))
+	sel := labels.Set(map[string]string{"some-label": "value"}).AsSelector()
+
+	resources, err := identifiedResources.List(sel, nil)
+	require.Nil(t, err)
+	require.NotNil(t, resources)
+
+	require.Equal(t, 1, len(resources))
+	require.Contains(t, resources[0].Labels(), "some-label")
+	require.Equal(t, resources[0].Labels()["some-label"], "value")
+}
+
+type FakeResources struct {
+	t *testing.T
+}
+
+func (r *FakeResources) All([]ctlres.ResourceType, ctlres.AllOpts) ([]ctlres.Resource, error) {
+	antreaBs := `---
+apiVersion: clusterinformation.antrea.tanzu.vmware.com/v1beta1
+kind: AntreaControllerInfo
+metadata:
+  name: antrea-controller
+version: v0.10.1
+`
+
+	deploymentBs := `---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  labels:
+    some-label: "value"
+`
+
+	antreaRes := ctlres.MustNewResourceFromBytes([]byte(antreaBs))
+	deploymentRes := ctlres.MustNewResourceFromBytes([]byte(deploymentBs))
+
+	return []ctlres.Resource{antreaRes, deploymentRes}, nil
+}
+func (r *FakeResources) Delete(ctlres.Resource) error                 { return nil }
+func (r *FakeResources) Exists(ctlres.Resource) (bool, error)         { return true, nil }
+func (r *FakeResources) Get(ctlres.Resource) (ctlres.Resource, error) { return nil, nil }
+func (r *FakeResources) Patch(ctlres.Resource, types.PatchType, []byte) (ctlres.Resource, error) {
+	return nil, nil
+}
+func (r *FakeResources) Update(ctlres.Resource) (ctlres.Resource, error) { return nil, nil }
+func (r *FakeResources) Create(ctlres.Resource) (ctlres.Resource, error) { return nil, nil }
+
+type FakeResourceTypes struct{}
+
+func (r *FakeResourceTypes) All() ([]ctlres.ResourceType, error) { return nil, nil }
+func (r *FakeResourceTypes) Find(ctlres.Resource) (ctlres.ResourceType, error) {
+	return ctlres.ResourceType{}, nil
+}
+func (r *FakeResourceTypes) CanIgnoreFailingGroupVersion(schema.GroupVersion) bool { return true }

pkg/kapp/resources/resources.go

@@ -37,7 +37,17 @@ const (
 	resourcesDebug = false
 )
 
-type Resources struct {
+type Resources interface {
+	All([]ResourceType, AllOpts) ([]Resource, error)
+	Delete(Resource) error
+	Exists(Resource) (bool, error)
+	Get(Resource) (Resource, error)
+	Patch(Resource, types.PatchType, []byte) (Resource, error)
+	Update(Resource) (Resource, error)
+	Create(resource Resource) (Resource, error)
+}
+
+type ResourcesImpl struct {
 	resourceTypes             ResourceTypes
 	coreClient                kubernetes.Interface
 	dynamicClient             dynamic.Interface
@@ -49,10 +59,10 @@ type Resources struct {
 	logger logger.Logger
 }
 
-func NewResources(resourceTypes ResourceTypes, coreClient kubernetes.Interface,
-	dynamicClient dynamic.Interface, fallbackAllowedNamespaces []string, logger logger.Logger) *Resources {
+func NewResourcesImpl(resourceTypes ResourceTypes, coreClient kubernetes.Interface,
+	dynamicClient dynamic.Interface, fallbackAllowedNamespaces []string, logger logger.Logger) *ResourcesImpl {
 
-	return &Resources{
+	return &ResourcesImpl{
 		resourceTypes:             resourceTypes,
 		coreClient:                coreClient,
 		dynamicClient:             dynamicClient,
@@ -66,7 +76,7 @@ type unstructItems struct {
 	Items   []unstructured.Unstructured
 }
 
-func (c *Resources) All(resTypes []ResourceType, opts AllOpts) ([]Resource, error) {
+func (c *ResourcesImpl) All(resTypes []ResourceType, opts AllOpts) ([]Resource, error) {
 	defer c.logger.DebugFunc("All").Finish()
 
 	if opts.ListOpts == nil {
@@ -154,7 +164,7 @@ func (c *Resources) All(resTypes []ResourceType, opts AllOpts) ([]Resource, erro
 	return resources, nil
 }
 
-func (c *Resources) allForNamespaces(client dynamic.NamespaceableResourceInterface, listOpts *metav1.ListOptions) (*unstructured.UnstructuredList, error) {
+func (c *ResourcesImpl) allForNamespaces(client dynamic.NamespaceableResourceInterface, listOpts *metav1.ListOptions) (*unstructured.UnstructuredList, error) {
 	defer c.logger.DebugFunc("allForNamespaces").Finish()
 
 	allowedNs, err := c.assumedAllowedNamespaces()
@@ -202,7 +212,7 @@ func (c *Resources) allForNamespaces(client dynamic.NamespaceableResourceInterfa
 	return list, nil
 }
 
-func (c *Resources) Create(resource Resource) (Resource, error) {
+func (c *ResourcesImpl) Create(resource Resource) (Resource, error) {
 	if resourcesDebug {
 		t1 := time.Now().UTC()
 		defer func() { c.logger.Debug("create %s", time.Now().UTC().Sub(t1)) }()
@@ -229,7 +239,7 @@ func (c *Resources) Create(resource Resource) (Resource, error) {
 	return NewResourceUnstructured(*createdUn, resType), nil
 }
 
-func (c *Resources) Update(resource Resource) (Resource, error) {
+func (c *ResourcesImpl) Update(resource Resource) (Resource, error) {
 	if resourcesDebug {
 		t1 := time.Now().UTC()
 		defer func() { c.logger.Debug("update %s", time.Now().UTC().Sub(t1)) }()
@@ -256,7 +266,7 @@ func (c *Resources) Update(resource Resource) (Resource, error) {
 	return NewResourceUnstructured(*updatedUn, resType), nil
 }
 
-func (c *Resources) Patch(resource Resource, patchType types.PatchType, data []byte) (Resource, error) {
+func (c *ResourcesImpl) Patch(resource Resource, patchType types.PatchType, data []byte) (Resource, error) {
 	if resourcesDebug {
 		t1 := time.Now().UTC()
 		defer func() { c.logger.Debug("patch %s", time.Now().UTC().Sub(t1)) }()
@@ -280,7 +290,7 @@ func (c *Resources) Patch(resource Resource, patchType types.PatchType, data []b
 	return NewResourceUnstructured(*patchedUn, resType), nil
 }
 
-func (c *Resources) Delete(resource Resource) error {
+func (c *ResourcesImpl) Delete(resource Resource) error {
 	if resourcesDebug {
 		t1 := time.Now().UTC()
 		defer func() { c.logger.Debug("delete %s", time.Now().UTC().Sub(t1)) }()
@@ -326,7 +336,7 @@ func (c *Resources) Delete(resource Resource) error {
 	return nil
 }
 
-func (c *Resources) Get(resource Resource) (Resource, error) {
+func (c *ResourcesImpl) Get(resource Resource) (Resource, error) {
 	if resourcesDebug {
 		t1 := time.Now().UTC()
 		defer func() { c.logger.Debug("get %s", time.Now().UTC().Sub(t1)) }()
@@ -351,7 +361,7 @@ func (c *Resources) Get(resource Resource) (Resource, error) {
 	return NewResourceUnstructured(*item, resType), nil
 }
 
-func (c *Resources) Exists(resource Resource) (bool, error) {
+func (c *ResourcesImpl) Exists(resource Resource) (bool, error) {
 	if resourcesDebug {
 		t1 := time.Now().UTC()
 		defer func() { c.logger.Debug("exists %s", time.Now().UTC().Sub(t1)) }()
@@ -406,7 +416,7 @@ var (
 	podMetricsNotFoundErrCheck = regexp.MustCompile("Error while getting pod (.+) not found \\(reason: \\)")
 )
 
-func (c *Resources) isPodMetrics(resource Resource, err error) bool {
+func (c *ResourcesImpl) isPodMetrics(resource Resource, err error) bool {
 	// Abnormal error case. Get/Delete on PodMetrics may fail
 	// without NotFound reason due to its dependence on Pod existance
 	if resource.Kind() == "PodMetrics" && resource.APIGroup() == "metrics.k8s.io" {
@@ -417,11 +427,11 @@ func (c *Resources) isPodMetrics(resource Resource, err error) bool {
 	return false
 }
 
-func (c *Resources) isGeneralRetryableErr(err error) bool {
+func (c *ResourcesImpl) isGeneralRetryableErr(err error) bool {
 	return IsResourceChangeBlockedErr(err) || c.isServerRescaleErr(err)
 }
 
-func (*Resources) isServerRescaleErr(err error) bool {
+func (*ResourcesImpl) isServerRescaleErr(err error) bool {
 	switch err := err.(type) {
 	case *http2.GoAwayError:
 		return true
@@ -433,14 +443,14 @@ func (*Resources) isServerRescaleErr(err error) bool {
 	return false
 }
 
-func (c *Resources) resourceErr(err error, action string, resource Resource) error {
+func (c *ResourcesImpl) resourceErr(err error, action string, resource Resource) error {
 	if typedErr, ok := err.(errors.APIStatus); ok {
 		return resourceStatusErr{resourcePlainErr{err, action, resource}, typedErr.Status()}
 	}
 	return resourcePlainErr{err, action, resource}
 }
 
-func (c *Resources) resourceClient(resource Resource) (dynamic.ResourceInterface, ResourceType, error) {
+func (c *ResourcesImpl) resourceClient(resource Resource) (dynamic.ResourceInterface, ResourceType, error) {
 	resType, err := c.resourceTypes.Find(resource)
 	if err != nil {
 		return nil, ResourceType{}, err
@@ -449,7 +459,7 @@ func (c *Resources) resourceClient(resource Resource) (dynamic.ResourceInterface
 	return c.dynamicClient.Resource(resType.GroupVersionResource).Namespace(resource.Namespace()), resType, nil
 }
 
-func (c *Resources) assumedAllowedNamespaces() ([]string, error) {
+func (c *ResourcesImpl) assumedAllowedNamespaces() ([]string, error) {
 	c.assumedAllowedNamespacesMemoLock.Lock()
 	defer c.assumedAllowedNamespacesMemoLock.Unlock()