Skip to content

Commit

Permalink
Merge pull request #198 from vmware-tanzu/labels
Browse files Browse the repository at this point in the history
Filter resources by label to catch aggregated api mistakes
  • Loading branch information
cppforlife authored Mar 5, 2021
2 parents fc35325 + 7168dfc commit 7a4f630
Show file tree
Hide file tree
Showing 58 changed files with 22,551 additions and 23 deletions.
5 changes: 4 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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;.*"
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
2 changes: 1 addition & 1 deletion pkg/kapp/cmd/app/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/kapp/cmd/tools/list_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions pkg/kapp/resources/identified_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")}
Expand Down
12 changes: 12 additions & 0 deletions pkg/kapp/resources/identified_resources_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
74 changes: 74 additions & 0 deletions pkg/kapp/resources/identified_resources_list_test.go
Original file line number Diff line number Diff line change
@@ -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 }
46 changes: 28 additions & 18 deletions pkg/kapp/resources/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)) }()
Expand All @@ -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)) }()
Expand All @@ -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)) }()
Expand All @@ -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)) }()
Expand Down Expand Up @@ -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)) }()
Expand All @@ -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)) }()
Expand Down Expand Up @@ -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" {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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()

Expand Down
15 changes: 15 additions & 0 deletions vendor/github.com/davecgh/go-spew/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 7a4f630

Please sign in to comment.