Merge pull request #291 from derailed/popeye/release_v0.20.5

Popeye release v0.20.5

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

change_logs/release_v0.20.5.md

@@ -0,0 +1,27 @@
+<img src="https://raw.githubusercontent.com/derailed/popeye/master/assets/popeye_logo.png" align="right" width="200" height="auto"/>
+
+# 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
+
+---
+
+<img src="https://raw.githubusercontent.com/derailed/popeye/master/assets/imhotep_logo.png" width="32" height="auto"/>&nbsp; © 2024 Imhotep Software LLC. All materials licensed under [Apache v2.0](http://www.apache.org/licenses/LICENSE-2.0)

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() {

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

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=

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

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)
 }

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 {

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
 	}
 

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