From f9a13d57ec33c3d261400e431a2dd1872d4605b6 Mon Sep 17 00:00:00 2001 From: derailed Date: Tue, 27 Feb 2024 13:38:02 -0700 Subject: [PATCH 1/5] [Bug] Fix #286 --- internal/db/db.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/db/db.go b/internal/db/db.go index f71b5c1f..cc623b07 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -190,7 +190,7 @@ func (db *DB) FindJobs(fqn string) ([]*batchv1.Job, error) { continue } for _, o := range jo.OwnerReferences { - if o.Controller == nil && !*o.Controller { + if o.Controller == nil || !*o.Controller { continue } if o.Name == cn { From 53e8d27531462dbc73d76be1abe869c53cef7ec6 Mon Sep 17 00:00:00 2001 From: derailed Date: Tue, 27 Feb 2024 13:40:45 -0700 Subject: [PATCH 2/5] rel v0.20.4 --- Makefile | 2 +- change_logs/release_v0.20.4.md | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 change_logs/release_v0.20.4.md diff --git a/Makefile b/Makefile index 456d0afa..5d5e2ce2 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ NAME := popeye PACKAGE := github.com/derailed/$(NAME) -VERSION := v0.20.3 +VERSION := v0.20.4 GIT := $(shell git rev-parse --short HEAD) DATE := $(shell date +%FT%T%Z) IMG_NAME := derailed/popeye diff --git a/change_logs/release_v0.20.4.md b/change_logs/release_v0.20.4.md new file mode 100644 index 00000000..3adfc70c --- /dev/null +++ b/change_logs/release_v0.20.4.md @@ -0,0 +1,25 @@ + + +# Release v0.20.4 + +## Notes + +Thank you to all that contributed with flushing out issues and enhancements for Popeye! I'll try to mark some of these issues as fixed. But if you don't mind grab the latest rev and see if we're happier with some of the fixes! If you've filed an issue please help me verify and close. Your support, kindness and awesome suggestions to make Popeye better is as ever very much noticed and appreciated! + +This project offers a GitHub Sponsor button (over here 👆). As you well know this is not pimped out by big corps with deep pockets. If you feel `Popeye` is saving you cycles diagnosing potential cluster issues please consider sponsoring this project!! It does go a long way in keeping our servers lights on and beers in our fridge. + +Also if you dig this tool, please make some noise on social! [@kitesurfer](https://twitter.com/kitesurfer) + +--- + +## Maintenance Release + +--- + +## Resolved Issues + +. [#286](https://github.com/derailed/popeye/issues/286) Boom! runtime error: invalid memory address or nil pointer dereference (see logs) + +--- + +  © 2024 Imhotep Software LLC. All materials licensed under [Apache v2.0](http://www.apache.org/licenses/LICENSE-2.0) From 5167013b567b35d6d6b1df9a2d162a8bd958a5cb Mon Sep 17 00:00:00 2001 From: derailed Date: Tue, 5 Mar 2024 11:41:54 -0700 Subject: [PATCH 3/5] [Maint] semver pkg change --- cmd/root.go | 2 +- go.mod | 2 +- go.sum | 4 ++-- internal/cache/cluster.go | 2 +- internal/cache/cluster_test.go | 8 ++++---- internal/dag/cluster.go | 2 +- internal/dag/helper_test.go | 16 ++++++++-------- internal/dag/helpers.go | 6 +++--- internal/lint/cluster.go | 4 ++-- internal/lint/cluster_test.go | 2 +- 10 files changed, 24 insertions(+), 24 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 14b2c668..57fd9b74 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -84,7 +84,7 @@ func bomb(err error) { if err == nil { return } - panic(fmt.Sprintf("💥 %s\n", report.Colorize(err.Error(), report.ColorRed))) + panic(fmt.Errorf("💥 %s\n", report.Colorize(err.Error(), report.ColorRed))) } func initPopeyeFlags() { diff --git a/go.mod b/go.mod index a12e3c93..47eac6e3 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module github.com/derailed/popeye go 1.21.1 require ( - github.com/Masterminds/semver v1.5.0 github.com/aws/aws-sdk-go v1.35.21 + github.com/blang/semver/v4 v4.0.0 github.com/fvbommel/sortorder v1.0.1 github.com/hashicorp/go-memdb v1.3.4 github.com/prometheus/client_golang v1.17.0 diff --git a/go.sum b/go.sum index 84a64373..49f56512 100644 --- a/go.sum +++ b/go.sum @@ -2,12 +2,12 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMT github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOElx5B5HZ4hJQsoJ/PvUvKRhJHDQXO8P8= github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= -github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= -github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= github.com/aws/aws-sdk-go v1.35.21 h1:6cMeHzcca+0uweOpUonDYv4DsPp9Qa9PTMYxH+VqDkY= github.com/aws/aws-sdk-go v1.35.21/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= +github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= diff --git a/internal/cache/cluster.go b/internal/cache/cluster.go index fc75801c..83478f47 100644 --- a/internal/cache/cluster.go +++ b/internal/cache/cluster.go @@ -6,7 +6,7 @@ package cache import ( "errors" - "github.com/Masterminds/semver" + "github.com/blang/semver/v4" ) // ClusterKey tracks Cluster resource references diff --git a/internal/cache/cluster_test.go b/internal/cache/cluster_test.go index 4d9f95c7..baa84190 100644 --- a/internal/cache/cluster_test.go +++ b/internal/cache/cluster_test.go @@ -6,7 +6,7 @@ package cache_test import ( "testing" - "github.com/Masterminds/semver" + "github.com/blang/semver/v4" "github.com/derailed/popeye/internal/cache" "github.com/rs/zerolog" "github.com/stretchr/testify/assert" @@ -17,11 +17,11 @@ func init() { } func TestCluster(t *testing.T) { - v, err := semver.NewVersion("1.9") + v, err := semver.ParseTolerant("1.9") assert.NoError(t, err) - c := cache.NewCluster(v) + c := cache.NewCluster(&v) v1, err := c.ListVersion() assert.NoError(t, err) - assert.Equal(t, v, v1) + assert.Equal(t, &v, v1) } diff --git a/internal/dag/cluster.go b/internal/dag/cluster.go index 5d57a705..8abfd598 100644 --- a/internal/dag/cluster.go +++ b/internal/dag/cluster.go @@ -6,7 +6,7 @@ package dag import ( "context" - "github.com/Masterminds/semver" + "github.com/blang/semver/v4" ) // ListVersion return server api version. diff --git a/internal/dag/helper_test.go b/internal/dag/helper_test.go index c96ca36a..cee4cd61 100644 --- a/internal/dag/helper_test.go +++ b/internal/dag/helper_test.go @@ -5,25 +5,24 @@ package dag import ( "errors" - "fmt" "testing" - "github.com/Masterminds/semver" + "github.com/blang/semver/v4" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/version" ) func TestParseVers(t *testing.T) { - v, _ := semver.NewVersion("1.28") + v, _ := semver.ParseTolerant("1.28") uu := map[string]struct { info version.Info err error - ver *semver.Version + ver semver.Version }{ "empty": { - err: fmt.Errorf(`semver parse failed for "." (""|""): %w`, errors.New("Invalid Semantic Version")), + err: errors.New(`semver parse failed for "." (""|""): strconv.ParseUint: parsing "": invalid syntax`), }, "happy": { info: version.Info{Major: "1", Minor: "28"}, @@ -39,9 +38,10 @@ func TestParseVers(t *testing.T) { u := uu[k] t.Run(k, func(t *testing.T) { v, err := ParseVersion(&u.info) - assert.Equal(t, u.err, err) - if err == nil { - assert.Equal(t, u.ver, v) + if err != nil { + assert.Equal(t, u.err.Error(), err.Error()) + } else { + assert.Equal(t, &u.ver, v) } }) } diff --git a/internal/dag/helpers.go b/internal/dag/helpers.go index d18b7022..f1b2e67a 100644 --- a/internal/dag/helpers.go +++ b/internal/dag/helpers.go @@ -8,7 +8,7 @@ import ( "fmt" "strings" - "github.com/Masterminds/semver" + "github.com/blang/semver/v4" "github.com/derailed/popeye/internal" "github.com/derailed/popeye/types" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -21,12 +21,12 @@ func ParseVersion(info *version.Info) (*semver.Version, error) { return nil, fmt.Errorf("no cluster version available") } v := strings.TrimSuffix(info.Major+"."+info.Minor, "+") - rev, err := semver.NewVersion(v) + rev, err := semver.ParseTolerant(v) if err != nil { err = fmt.Errorf("semver parse failed for %q (%q|%q): %w", v, info.Major, info.Minor, err) } - return rev, err + return &rev, err } func mustExtractFactory(ctx context.Context) types.Factory { diff --git a/internal/lint/cluster.go b/internal/lint/cluster.go index d77ad9c8..087a6d05 100644 --- a/internal/lint/cluster.go +++ b/internal/lint/cluster.go @@ -6,7 +6,7 @@ package lint import ( "context" - "github.com/Masterminds/semver" + "github.com/blang/semver/v4" "github.com/derailed/popeye/internal" "github.com/derailed/popeye/internal/issues" ) @@ -50,7 +50,7 @@ func (c *Cluster) checkVersion(ctx context.Context) error { } ctx = internal.WithSpec(ctx, specFor("Version", nil)) - if rev.Major() != tolerableMajor || rev.Minor() < tolerableMinor { + if rev.Major != tolerableMajor || rev.Minor < tolerableMinor { c.AddCode(ctx, 405) } else { c.AddCode(ctx, 406) diff --git a/internal/lint/cluster_test.go b/internal/lint/cluster_test.go index 84ffafb8..b35528c3 100644 --- a/internal/lint/cluster_test.go +++ b/internal/lint/cluster_test.go @@ -6,7 +6,7 @@ package lint import ( "testing" - "github.com/Masterminds/semver" + "github.com/blang/semver/v4" "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/version" From 696889d93b18833adfe014cbb85d2394af531b38 Mon Sep 17 00:00:00 2001 From: derailed Date: Tue, 5 Mar 2024 11:43:40 -0700 Subject: [PATCH 4/5] [Bug] Fix #290, #289 --- internal/client/client.go | 52 ++++++++++++------- internal/client/config.go | 2 +- internal/client/factory.go | 86 ++++++++++++++++++------------- internal/client/helpers.go | 6 +-- internal/db/db.go | 29 +++++------ internal/db/loader.go | 14 ++--- internal/issues/assets/codes.yaml | 4 +- internal/lint/svc.go | 11 ++-- internal/lint/svc_test.go | 6 +-- pkg/config/config.go | 2 +- pkg/popeye.go | 5 +- types/types.go | 4 +- 12 files changed, 117 insertions(+), 104 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index a8f61d7d..9047cc9a 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -64,14 +64,18 @@ func InitConnectionOrDie(config types.Config) (*APIClient, error) { config: config, cache: cache.NewLRUExpireCache(cacheSize), } + _, err := a.serverGroups() + if err != nil { + return nil, err + } if err := a.supportsMetricsResources(); err != nil { - log.Warn().Msgf("no metrics server detected %s", err.Error()) + log.Warn().Err(err).Msgf("no metrics server detected") } return &a, nil } -func makeSAR(ns string, gvr types.GVR) *authorizationv1.SelfSubjectAccessReview { +func makeSAR(ns string, gvr types.GVR, n string) *authorizationv1.SelfSubjectAccessReview { if ns == "-" { ns = "" } @@ -83,13 +87,14 @@ func makeSAR(ns string, gvr types.GVR) *authorizationv1.SelfSubjectAccessReview Group: res.Group, Resource: res.Resource, Subresource: gvr.SubResource(), + Name: n, }, }, } } -func makeCacheKey(ns, gvr string, vv []string) string { - return ns + ":" + gvr + "::" + strings.Join(vv, ",") +func makeCacheKey(ns string, gvr types.GVR, n string, vv []string) string { + return ns + ":" + gvr.String() + ":" + n + "::" + strings.Join(vv, ",") } // ActiveContext returns the current context name. @@ -146,11 +151,11 @@ func (a *APIClient) ConnectionOK() bool { } // CanI checks if user has access to a certain resource. -func (a *APIClient) CanI(ns string, gvr types.GVR, verbs ...string) (auth bool, err error) { +func (a *APIClient) CanI(ns string, gvr types.GVR, n string, verbs []string) (auth bool, err error) { if IsClusterWide(ns) { - ns = AllNamespaces + ns = BlankNamespace } - key := makeCacheKey(ns, gvr.String(), verbs) + key := makeCacheKey(ns, gvr, n, verbs) if v, ok := a.cache.Get(key); ok { if auth, ok = v.(bool); ok { return auth, nil @@ -161,7 +166,7 @@ func (a *APIClient) CanI(ns string, gvr types.GVR, verbs ...string) (auth bool, if err != nil { return false, err } - dial, sar := c.AuthorizationV1().SelfSubjectAccessReviews(), makeSAR(ns, gvr) + dial, sar := c.AuthorizationV1().SelfSubjectAccessReviews(), makeSAR(ns, gvr, n) ctx, cancel := context.WithTimeout(context.Background(), CallTimeout) defer cancel() for _, v := range verbs { @@ -357,30 +362,38 @@ func (a *APIClient) checkCacheBool(key string) (state bool, ok bool) { return } +func (a *APIClient) serverGroups() (*metav1.APIGroupList, error) { + dial, err := a.CachedDiscovery() + if err != nil { + log.Warn().Err(err).Msgf("Unable to dial discovery API") + return nil, err + } + apiGroups, err := dial.ServerGroups() + if err != nil { + log.Warn().Err(err).Msgf("Unable to retrieve server groups") + return nil, fmt.Errorf("unable to fetch server groups: %w", err) + } + + return apiGroups, nil +} + func (a *APIClient) supportsMetricsResources() error { supported, ok := a.checkCacheBool(cacheMXAPIKey) if ok { if supported { return nil } - return errors.New("No metrics-server detected") + return errors.New("no metrics-server detected") } - defer func() { a.cache.Add(cacheMXAPIKey, supported, cacheExpiry) }() - dial, err := a.CachedDiscovery() - if err != nil { - log.Warn().Err(err).Msgf("Unable to dial discovery API") - return err - } - apiGroups, err := dial.ServerGroups() + gg, err := a.serverGroups() if err != nil { - log.Warn().Err(err).Msgf("Unable to retrieve server groups") return err } - for _, grp := range apiGroups.Groups { + for _, grp := range gg.Groups { if grp.Name != metricsapi.GroupName { continue } @@ -390,8 +403,9 @@ func (a *APIClient) supportsMetricsResources() error { } } - return errors.New("No metrics-server detected") + return errors.New("no metrics-server detected") } + func checkMetricsVersion(grp metav1.APIGroup) bool { for _, version := range grp.Versions { for _, supportedVersion := range supportedMetricsAPIVersions { diff --git a/internal/client/config.go b/internal/client/config.go index 85cd1f3f..a4c2f40d 100644 --- a/internal/client/config.go +++ b/internal/client/config.go @@ -234,7 +234,7 @@ func (c *Config) CurrentUserName() (string, error) { // CurrentNamespaceName retrieves the active namespace. func (c *Config) CurrentNamespaceName() (string, error) { - if c.flags.Namespace != nil { + if isSet(c.flags.Namespace) { return *c.flags.Namespace, nil } diff --git a/internal/client/factory.go b/internal/client/factory.go index 99d40804..5942f20d 100644 --- a/internal/client/factory.go +++ b/internal/client/factory.go @@ -50,7 +50,7 @@ func (f *Factory) Start(ns string) { } } -// Terminate stops the factory. +// Terminate terminates all watchers and forwards. func (f *Factory) Terminate() { f.mx.Lock() defer f.mx.Unlock() @@ -66,48 +66,68 @@ func (f *Factory) Terminate() { // List returns a resource collection. func (f *Factory) List(gvr types.GVR, ns string, wait bool, labels labels.Selector) ([]runtime.Object, error) { - inf, err := f.CanForResource(ns, gvr, types.MonitorAccess...) + inf, err := f.CanForResource(ns, gvr, types.ListAccess) if err != nil { return nil, err } - if wait { - f.waitForCacheSync(ns) + if IsAllNamespace(ns) { + ns = BlankNamespace } + + var oo []runtime.Object if IsClusterScoped(ns) { - return inf.Lister().List(labels) + oo, err = inf.Lister().List(labels) + } else { + oo, err = inf.Lister().ByNamespace(ns).List(labels) + } + if !wait || (wait && inf.Informer().HasSynced()) { + return oo, err } - if IsAllNamespace(ns) { - ns = AllNamespaces + f.waitForCacheSync(ns) + if IsClusterScoped(ns) { + return inf.Lister().List(labels) } return inf.Lister().ByNamespace(ns).List(labels) } +// HasSynced checks if given informer is up to date. +func (f *Factory) HasSynced(gvr types.GVR, ns string) (bool, error) { + inf, err := f.CanForResource(ns, gvr, types.ListAccess) + if err != nil { + return false, err + } + + return inf.Informer().HasSynced(), nil +} + // Get retrieves a given resource. -func (f *Factory) Get(gvr types.GVR, path string, wait bool, sel labels.Selector) (runtime.Object, error) { - ns, n := Namespaced(path) - inf, err := f.CanForResource(ns, gvr, types.GetVerb) +func (f *Factory) Get(gvr types.GVR, fqn string, wait bool, sel labels.Selector) (runtime.Object, error) { + ns, n := Namespaced(fqn) + inf, err := f.CanForResource(ns, gvr, []string{types.GetVerb}) if err != nil { return nil, err } - - if wait { - f.waitForCacheSync(ns) + var o runtime.Object + if IsClusterScoped(ns) { + o, err = inf.Lister().Get(n) + } else { + o, err = inf.Lister().ByNamespace(ns).Get(n) + } + if !wait || (wait && inf.Informer().HasSynced()) { + return o, err } + + f.waitForCacheSync(ns) if IsClusterScoped(ns) { return inf.Lister().Get(n) } - return inf.Lister().ByNamespace(ns).Get(n) } func (f *Factory) waitForCacheSync(ns string) { if IsClusterWide(ns) { - ns = AllNamespaces - } - - if f.isClusterWide() { - ns = AllNamespaces + ns = BlankNamespace } f.mx.RLock() @@ -131,7 +151,7 @@ func (f *Factory) WaitForCacheSync() { for ns, fac := range f.factories { m := fac.WaitForCacheSync(f.stopChan) for k, v := range m { - log.Debug().Msgf("CACHE %q synched %t:%s", ns, v, k) + log.Debug().Msgf("CACHE `%q Loaded %t:%s", ns, v, k) } } } @@ -148,33 +168,24 @@ func (f *Factory) FactoryFor(ns string) di.DynamicSharedInformerFactory { // SetActiveNS sets the active namespace. func (f *Factory) SetActiveNS(ns string) error { - if !f.isClusterWide() { - if _, err := f.ensureFactory(ns); err != nil { - return err - } + if f.isClusterWide() { + return nil } - - return nil + _, err := f.ensureFactory(ns) + return err } func (f *Factory) isClusterWide() bool { f.mx.RLock() defer f.mx.RUnlock() + _, ok := f.factories[BlankNamespace] - _, ok := f.factories[AllNamespaces] return ok } // CanForResource return an informer is user has access. -func (f *Factory) CanForResource(ns string, gvr types.GVR, verbs ...string) (informers.GenericInformer, error) { - // If user can access resource cluster wide, prefer cluster wide factory. - if !IsClusterWide(ns) { - auth, err := f.Client().CanI(AllNamespaces, gvr, verbs...) - if auth && err == nil { - return f.ForResource(AllNamespaces, gvr) - } - } - auth, err := f.Client().CanI(ns, gvr, verbs...) +func (f *Factory) CanForResource(ns string, gvr types.GVR, verbs []string) (informers.GenericInformer, error) { + auth, err := f.Client().CanI(ns, gvr, "", verbs) if err != nil { return nil, err } @@ -206,13 +217,14 @@ func (f *Factory) ForResource(ns string, gvr types.GVR) (informers.GenericInform func (f *Factory) ensureFactory(ns string) (di.DynamicSharedInformerFactory, error) { if IsClusterWide(ns) { - ns = AllNamespaces + ns = BlankNamespace } f.mx.Lock() defer f.mx.Unlock() if fac, ok := f.factories[ns]; ok { return fac, nil } + dial, err := f.client.DynDial() if err != nil { return nil, err diff --git a/internal/client/helpers.go b/internal/client/helpers.go index f150576d..7e1892ed 100644 --- a/internal/client/helpers.go +++ b/internal/client/helpers.go @@ -17,13 +17,13 @@ var toFileName = regexp.MustCompile(`[^(\w/\.)]`) // IsClusterWide returns true if ns designates cluster scope, false otherwise. func IsClusterWide(ns string) bool { - return ns == NamespaceAll || ns == AllNamespaces || ns == ClusterScope + return ns == NamespaceAll || ns == BlankNamespace || ns == ClusterScope } // CleanseNamespace ensures all ns maps to blank. func CleanseNamespace(ns string) string { if IsAllNamespace(ns) { - return AllNamespaces + return BlankNamespace } return ns @@ -36,7 +36,7 @@ func IsAllNamespace(ns string) bool { // IsAllNamespaces returns true if all namespaces, false otherwise. func IsAllNamespaces(ns string) bool { - return ns == NamespaceAll || ns == AllNamespaces + return ns == NamespaceAll || ns == BlankNamespace } // IsNamespaced returns true if a specific ns is given. diff --git a/internal/db/db.go b/internal/db/db.go index cc623b07..067451bf 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -10,6 +10,7 @@ import ( "github.com/derailed/popeye/internal" "github.com/derailed/popeye/internal/client" + "github.com/derailed/popeye/internal/db/schema" "github.com/derailed/popeye/types" "github.com/hashicorp/go-memdb" batchv1 "k8s.io/api/batch/v1" @@ -146,12 +147,25 @@ func (db *DB) Find(kind types.GVR, fqn string) (any, error) { defer txn.Abort() o, err := txn.First(kind.String(), "id", fqn) if err != nil || o == nil { + log.Error().Err(err).Msgf("db.find unable to find object: [%s]%s", kind, fqn) return nil, fmt.Errorf("object not found: %q", fqn) } return o, nil } +func (db *DB) Dump(gvr types.GVR) { + txn, it := db.MustITFor(gvr) + defer txn.Abort() + + log.Debug().Msgf("> Dumping %q", gvr) + for o := it.Next(); o != nil; o = it.Next() { + m := o.(schema.MetaAccessor) + log.Debug().Msgf(" o %s/%s", m.GetNamespace(), m.GetName()) + } + log.Debug().Msg("< Done") +} + func (db *DB) FindPod(ns string, sel map[string]string) (*v1.Pod, error) { txn := db.Txn(false) defer txn.Abort() @@ -273,21 +287,6 @@ func (db *DB) FindNSBySel(sel *metav1.LabelSelector) ([]*v1.Namespace, error) { return nss, nil } -func (db *DB) DumpNS() error { - txn := db.Txn(false) - defer txn.Abort() - it, err := txn.Get(internal.Glossary[internal.NS].String(), "id") - if err != nil { - return err - } - for o := it.Next(); o != nil; o = it.Next() { - ns, _ := o.(*v1.Namespace) - log.Debug().Msgf("NS %q", ns.Name) - } - - return nil -} - func (db *DB) FindNS(ns string) (*v1.Namespace, error) { txn := db.Txn(false) defer txn.Abort() diff --git a/internal/db/loader.go b/internal/db/loader.go index 7aa47124..5a25e50e 100644 --- a/internal/db/loader.go +++ b/internal/db/loader.go @@ -172,18 +172,10 @@ func (l *Loader) fetchNodesMetrics(c types.Connection) (*mv1beta1.NodeMetricsLis func loadResource(ctx context.Context, gvr types.GVR) ([]runtime.Object, error) { f := mustExtractFactory(ctx) - if strings.Contains(gvr.String(), "metrics") { - if !f.Client().HasMetrics() { - return nil, nil - } - } - if gvr.IsMetricsRes() { - var res dao.Generic - res.Init(f, gvr) - return res.List(ctx) + if strings.Contains(gvr.String(), "metrics") && !f.Client().HasMetrics() { + return nil, nil } - - var res dao.Resource + var res dao.Generic res.Init(f, gvr) return res.List(ctx) diff --git a/internal/issues/assets/codes.yaml b/internal/issues/assets/codes.yaml index cfd6917b..fc16368d 100644 --- a/internal/issues/assets/codes.yaml +++ b/internal/issues/assets/codes.yaml @@ -258,7 +258,7 @@ codes: message: Do you mean it? Type NodePort detected severity: 1 1105: - message: No associated endpoints + message: No associated endpoints found severity: 3 1106: message: No target ports match service port %s @@ -270,7 +270,7 @@ codes: message: NodePort detected but service sets externalTrafficPolicy to "Local" severity: 1 1109: - message: Only one Pod associated with this endpoint + message: Single endpoint is associated with this service severity: 2 1110: message: Match EP has no subsets diff --git a/internal/lint/svc.go b/internal/lint/svc.go index 1712b8c3..5d81825a 100644 --- a/internal/lint/svc.go +++ b/internal/lint/svc.go @@ -106,7 +106,7 @@ func (s *Service) checkEndpoints(ctx context.Context, fqn string, kind v1.Servic } o, err := s.db.Find(internal.Glossary[internal.EP], fqn) - if err != nil || o == nil { + if err != nil { s.AddCode(ctx, 1105) return } @@ -115,14 +115,9 @@ func (s *Service) checkEndpoints(ctx context.Context, fqn string, kind v1.Servic s.AddCode(ctx, 1110) return } - numEndpointAddresses := 0 - for _, s := range ep.Subsets { - numEndpointAddresses += len(s.Addresses) - if numEndpointAddresses > 1 { - return - } + if len(ep.Subsets) == 1 { + s.AddCode(ctx, 1109) } - s.AddCode(ctx, 1109) } // ---------------------------------------------------------------------------- diff --git a/internal/lint/svc_test.go b/internal/lint/svc_test.go index 76e66cdb..ebbef2be 100644 --- a/internal/lint/svc_test.go +++ b/internal/lint/svc_test.go @@ -47,7 +47,7 @@ func Test_svcCheckEndpoints(t *testing.T) { Group: "__root__", GVR: "v1/services", Level: rules.ErrorLevel, - Message: "[POP-1105] No associated endpoints", + Message: "[POP-1105] No associated endpoints found", }, }, }, @@ -62,7 +62,7 @@ func Test_svcCheckEndpoints(t *testing.T) { Group: "__root__", GVR: "v1/services", Level: rules.ErrorLevel, - Message: "[POP-1105] No associated endpoints", + Message: "[POP-1105] No associated endpoints found", }, }, }, @@ -74,7 +74,7 @@ func Test_svcCheckEndpoints(t *testing.T) { Group: "__root__", GVR: "v1/services", Level: rules.WarnLevel, - Message: "[POP-1109] Only one Pod associated with this endpoint", + Message: "[POP-1109] Single endpoint is associated with this service", }, }, }, diff --git a/pkg/config/config.go b/pkg/config/config.go index 3bd9dde8..bb5b6a52 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -47,7 +47,7 @@ func NewConfig(flags *Flags) (*Config, error) { flags.Namespace = nil } if flags.AllNamespaces != nil && *flags.AllNamespaces { - all := client.AllNamespaces + all := client.NamespaceAll flags.Namespace = &all } cfg.LintLevel = int(rules.ToIssueLevel(flags.LintLevel)) diff --git a/pkg/popeye.go b/pkg/popeye.go index 072ad2ce..54c0adf9 100644 --- a/pkg/popeye.go +++ b/pkg/popeye.go @@ -97,6 +97,7 @@ func (p *Popeye) Init() error { return err } } + if err := p.aliases.Init(p.client()); err != nil { return err } @@ -144,7 +145,7 @@ func (p *Popeye) initFactory() error { log.Debug().Msgf("Skipping linter %q", k) continue } - ok, err := clt.CanI(client.AllNamespaces, gvr, types.ReadAllAccess...) + ok, err := clt.CanI(client.AllNamespaces, gvr, "", types.ReadAllAccess) if !ok || err != nil { return fmt.Errorf("current user does not have read access for resource %q -- %w", gvr, err) } @@ -198,7 +199,7 @@ func (p *Popeye) buildCtx(ctx context.Context) context.Context { } ns, err := p.client().Config().CurrentNamespaceName() if err != nil { - ns = client.AllNamespaces + ns = client.DefaultNamespace } ctx = context.WithValue(ctx, internal.KeyNamespace, ns) diff --git a/types/types.go b/types/types.go index d8cb98e7..bb2f7fd4 100644 --- a/types/types.go +++ b/types/types.go @@ -58,7 +58,7 @@ type NamespaceNames map[string]struct{} // Authorizer checks what a user can or cannot do to a resource. type Authorizer interface { // CanI returns true if the user can use these actions for a given resource. - CanI(string, GVR, ...string) (bool, error) + CanI(string, GVR, string, []string) (bool, error) } // Config represents an api server configuration. @@ -144,7 +144,7 @@ type Factory interface { ForResource(string, GVR) (informers.GenericInformer, error) // CanForResource fetch an informer for a given resource if authorized - CanForResource(string, GVR, ...string) (informers.GenericInformer, error) + CanForResource(string, GVR, []string) (informers.GenericInformer, error) // WaitForCacheSync synchronize the cache. WaitForCacheSync() From fc96b29755616dc2356b0312ad52872d1f7de51c Mon Sep 17 00:00:00 2001 From: derailed Date: Tue, 5 Mar 2024 11:47:47 -0700 Subject: [PATCH 5/5] release v0.20.5 --- Makefile | 2 +- change_logs/release_v0.20.5.md | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 change_logs/release_v0.20.5.md diff --git a/Makefile b/Makefile index 5d5e2ce2..a61a6443 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ NAME := popeye PACKAGE := github.com/derailed/$(NAME) -VERSION := v0.20.4 +VERSION := v0.20.5 GIT := $(shell git rev-parse --short HEAD) DATE := $(shell date +%FT%T%Z) IMG_NAME := derailed/popeye diff --git a/change_logs/release_v0.20.5.md b/change_logs/release_v0.20.5.md new file mode 100644 index 00000000..28d230eb --- /dev/null +++ b/change_logs/release_v0.20.5.md @@ -0,0 +1,27 @@ + + +# Release v0.20.5 + +## Notes + +Thank you to all that contributed with flushing out issues and enhancements for Popeye! I'll try to mark some of these issues as fixed. But if you don't mind grab the latest rev and see if we're happier with some of the fixes! If you've filed an issue please help me verify and close. Your support, kindness and awesome suggestions to make Popeye better is as ever very much noticed and appreciated! + +This project offers a GitHub Sponsor button (over here 👆). As you well know this is not pimped out by big corps with deep pockets. If you feel `Popeye` is saving you cycles diagnosing potential cluster issues please consider sponsoring this project!! It does go a long way in keeping our servers lights on and beers in our fridge. + +Also if you dig this tool, please make some noise on social! [@kitesurfer](https://twitter.com/kitesurfer) + +--- + +## Maintenance Release + +--- + +## Resolved Issues + +. [#290](https://github.com/derailed/popeye/issues/290) Secrets is not scanned on Openshift 4.12 bug +. [#289](https://github.com/derailed/popeye/issues/289) "No associated endpoint" (POP-1105) reported for service question +. [#288](https://github.com/derailed/popeye/issues/288) A Score granted despite no resources being scanned + +--- + +  © 2024 Imhotep Software LLC. All materials licensed under [Apache v2.0](http://www.apache.org/licenses/LICENSE-2.0)