Allow istioctl analyze to suppress messages via cmd line (istio#19673)

* Allow istioctl analyze to suppress messages via cmd line

* Break early if suppression match is found

* Use backticks instead of double quotes

* Support wildcards in suppression

Basic globbing is supported (similar to filepath globbing). Also embedded message filtering logic into for-loop.

* Use S as shorhand, better docs

* Add basic wildcard test

* Document wildcard syntax

* Extract out filter method to aid in readability

* Describe wildcards on AnalysisSuppression.ResourceName

* Use go-glob instead of filepath matcher

* Example shows multiple suppression rules at once

* Update code to work after merge conflict

* Fix lint errors

* Warn if analyzer code is unknown

* Clear messaging

* goimports

* Added further explanation in message output

* Fixes after merge

galley/pkg/config/analysis/local/analyze.go

@@ -74,6 +74,9 @@ type SourceAnalyzer struct {
 	namespace            resource.Namespace
 	istioNamespace       resource.Namespace
 
+	// List of code and resource suppressions to exclude messages on
+	suppressions []snapshotter.AnalysisSuppression
+
 	// Mesh config for this analyzer. This can come from multiple sources, and the last added version will take precedence.
 	meshCfg *v1alpha1.MeshConfig
 
@@ -176,6 +179,7 @@ func (sa *SourceAnalyzer) Analyze(cancel chan struct{}) (AnalysisResult, error)
 		TriggerSnapshot:    snapshots.LocalAnalysis,
 		CollectionReporter: sa.collectionReporter,
 		AnalysisNamespaces: namespaces,
+		Suppressions:       sa.suppressions,
 	}
 	distributor := snapshotter.NewAnalyzingDistributor(distributorSettings)
 
@@ -204,6 +208,13 @@ func (sa *SourceAnalyzer) Analyze(cancel chan struct{}) (AnalysisResult, error)
 	return result, nil
 }
 
+// SetSuppressions will set the list of suppressions for the analyzer. Any
+// resource that matches the provided suppression will not be included in the
+// final message output.
+func (sa *SourceAnalyzer) SetSuppressions(suppressions []snapshotter.AnalysisSuppression) {
+	sa.suppressions = suppressions
+}
+
 // AddReaderKubeSource adds a source based on the specified k8s yaml files to the current SourceAnalyzer
 func (sa *SourceAnalyzer) AddReaderKubeSource(readers []io.Reader) error {
 	src := inmemory.NewKubeSource(sa.kubeResources)

galley/pkg/config/processing/snapshotter/analyzingdistributor.go

@@ -17,6 +17,8 @@ package snapshotter
 import (
 	"sync"
 
+	"github.com/ryanuber/go-glob"
+
 	"istio.io/istio/galley/pkg/config/analysis"
 	"istio.io/istio/galley/pkg/config/analysis/diag"
 	coll "istio.io/istio/galley/pkg/config/collection"
@@ -66,6 +68,23 @@ type AnalyzingDistributorSettings struct {
 
 	// Namespaces that should be analyzed
 	AnalysisNamespaces []resource.Namespace
+
+	// Suppressions that suppress a set of matching messages.
+	Suppressions []AnalysisSuppression
+}
+
+// AnalysisSuppression describes a resource and analysis code to be suppressed
+// (e.g. ignored) during analysis. Used when a particular message code is to be
+// ignored for a specific resource.
+type AnalysisSuppression struct {
+	// Code is the analysis code to suppress (e.g. "IST0104").
+	Code string
+
+	// ResourceName is the name of the resource to suppress the message for. For
+	// K8s resources it has the same form as used by istioctl (e.g.
+	// "DestinationRule default.istio-system"). Note that globbing wildcards are
+	// supported (e.g. "DestinationRule *.istio-system").
+	ResourceName string
 }
 
 // NewAnalyzingDistributor returns a new instance of AnalyzingDistributor.
@@ -140,25 +159,7 @@ func (d *AnalyzingDistributor) analyzeAndDistribute(cancelCh chan struct{}, name
 	d.s.Analyzer.Analyze(ctx)
 	scope.Analysis.Debugf("Finished analyzing the current snapshot, found messages: %v", ctx.messages)
 
-	// Only keep messages for resources in namespaces we want to analyze if the
-	// message doesn't have an origin (meaning we can't determine the
-	// namespace). Also kept are cluster-level resources where the namespace is
-	// the empty string. If no such limit is specified, keep them all.
-	var msgs diag.Messages
-	if len(namespaces) == 0 {
-		msgs = ctx.messages
-	} else {
-		for _, m := range ctx.messages {
-			if m.Resource != nil && m.Resource.Origin.Namespace() != "" {
-				if _, ok := namespaces[m.Resource.Origin.Namespace()]; !ok {
-					continue
-				}
-			}
-
-			msgs = append(msgs, m)
-		}
-	}
-
+	msgs := filterMessages(ctx.messages, namespaces, d.s.Suppressions)
 	if !ctx.Canceled() {
 		d.s.StatusUpdater.Update(msgs.SortedDedupedCopy())
 	}
@@ -186,6 +187,43 @@ func (d *AnalyzingDistributor) getCombinedSnapshot() *Snapshot {
 	return &Snapshot{set: coll.NewSetFromCollections(collections)}
 }
 
+func filterMessages(messages diag.Messages, namespaces map[resource.Namespace]struct{}, suppressions []AnalysisSuppression) diag.Messages {
+	nsNames := make(map[string]struct{})
+	for k := range namespaces {
+		nsNames[k.String()] = struct{}{}
+	}
+
+	var msgs diag.Messages
+FilterMessages:
+	for _, m := range messages {
+		// Only keep messages for resources in namespaces we want to analyze if the
+		// message doesn't have an origin (meaning we can't determine the
+		// namespace). Also kept are cluster-level resources where the namespace is
+		// the empty string. If no such limit is specified, keep them all.
+		if len(namespaces) > 0 && m.Resource != nil && m.Resource.Origin.Namespace() != "" {
+			if _, ok := nsNames[m.Resource.Origin.Namespace().String()]; !ok {
+				continue FilterMessages
+			}
+		}
+
+		// Filter out any messages that match our suppressions.
+		for _, s := range suppressions {
+			if m.Resource == nil || s.Code != m.Type.Code() {
+				continue
+			}
+
+			if !glob.Glob(s.ResourceName, m.Resource.Origin.FriendlyName()) {
+				continue
+			}
+			scope.Analysis.Debugf("Suppressing code %s on resource %s due to suppressions list", m.Type.Code(), m.Resource.Origin.FriendlyName())
+			continue FilterMessages
+		}
+
+		msgs = append(msgs, m)
+	}
+	return msgs
+}
+
 type context struct {
 	sn                 *Snapshot
 	cancelCh           chan struct{}

galley/pkg/config/processing/snapshotter/analyzingdistributor_test.go

@@ -249,6 +249,113 @@ func TestAnalyzeSortsMessages(t *testing.T) {
 	g.Expect(u.messages[1].Resource).To(Equal(r1))
 }
 
+func TestAnalyzeSuppressesMessages(t *testing.T) {
+	g := NewGomegaWithT(t)
+
+	u := &updaterMock{}
+	r1 := &resource.Instance{
+		Origin: &rt.Origin{
+			Collection: basicmeta.K8SCollection1.Name(),
+			FullName:   resource.NewFullName("includedNamespace", "r2"),
+			Kind:       "Kind1",
+		},
+	}
+	r2 := &resource.Instance{
+		Origin: &rt.Origin{
+			Collection: basicmeta.K8SCollection1.Name(),
+			FullName:   resource.NewFullName("includedNamespace", "r1"),
+			Kind:       "Kind1",
+		},
+	}
+
+	a := &analyzerMock{
+		collectionToAccess: basicmeta.K8SCollection1.Name(),
+		resourcesToReport:  []*resource.Instance{r1, r2},
+	}
+	s := AnalysisSuppression{
+		Code:         "IST0001", // InternalError, reported by analyzerMock
+		ResourceName: "Kind1 r2.includedNamespace",
+	}
+	d := NewInMemoryDistributor()
+
+	settings := AnalyzingDistributorSettings{
+		StatusUpdater:      u,
+		Analyzer:           analysis.Combine("testCombined", a),
+		Distributor:        d,
+		AnalysisSnapshots:  []string{snapshots.Default},
+		TriggerSnapshot:    snapshots.Default,
+		CollectionReporter: nil,
+		AnalysisNamespaces: []resource.Namespace{"includedNamespace"},
+		Suppressions:       []AnalysisSuppression{s},
+	}
+	ad := NewAnalyzingDistributor(settings)
+
+	sDefault := getTestSnapshot()
+
+	ad.Distribute(snapshots.Default, sDefault)
+
+	g.Eventually(func() []*Snapshot { return a.analyzeCalls }).Should(ConsistOf(sDefault))
+	g.Expect(u.messages).To(HaveLen(1))
+	g.Expect(u.messages[0].Resource).To(Equal(r2))
+}
+
+func TestAnalyzeSuppressesMessagesWithWildcards(t *testing.T) {
+	g := NewGomegaWithT(t)
+
+	u := &updaterMock{}
+	// r1 and r2 have the same prefix, but r3 does not
+	r1 := &resource.Instance{
+		Origin: &rt.Origin{
+			Collection: basicmeta.K8SCollection1.Name(),
+			FullName:   resource.NewFullName("includedNamespace", "r2"),
+			Kind:       "Kind1",
+		},
+	}
+	r2 := &resource.Instance{
+		Origin: &rt.Origin{
+			Collection: basicmeta.K8SCollection1.Name(),
+			FullName:   resource.NewFullName("includedNamespace", "r1"),
+			Kind:       "Kind1",
+		},
+	}
+	r3 := &resource.Instance{
+		Origin: &rt.Origin{
+			Collection: basicmeta.K8SCollection1.Name(),
+			FullName:   resource.NewFullName("includedNamespace", "x1"),
+			Kind:       "Kind1",
+		},
+	}
+	a := &analyzerMock{
+		collectionToAccess: basicmeta.K8SCollection1.Name(),
+		resourcesToReport:  []*resource.Instance{r1, r2, r3},
+	}
+	s := AnalysisSuppression{
+		Code:         "IST0001",                    // InternalError, reported by analyzerMock
+		ResourceName: "Kind1 r*.includedNamespace", // should catch r1/r2 but not x1
+	}
+	d := NewInMemoryDistributor()
+
+	settings := AnalyzingDistributorSettings{
+		StatusUpdater:      u,
+		Analyzer:           analysis.Combine("testCombined", a),
+		Distributor:        d,
+		AnalysisSnapshots:  []string{snapshots.Default},
+		TriggerSnapshot:    snapshots.Default,
+		CollectionReporter: nil,
+		AnalysisNamespaces: []resource.Namespace{"includedNamespace"},
+		Suppressions:       []AnalysisSuppression{s},
+	}
+	ad := NewAnalyzingDistributor(settings)
+
+	sDefault := getTestSnapshot()
+
+	ad.Distribute(snapshots.Default, sDefault)
+
+	g.Eventually(func() []*Snapshot { return a.analyzeCalls }).Should(ConsistOf(sDefault))
+	g.Expect(u.messages).To(HaveLen(1))
+	g.Expect(u.messages[0].Resource).To(Equal(r3))
+}
+
 func getTestSnapshot(schemas ...collection.Schema) *Snapshot {
 	c := make([]*coll.Instance, 0)
 	for _, s := range schemas {

go.mod

@@ -121,7 +121,7 @@ require (
 	github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4
 	github.com/prometheus/common v0.6.0
 	github.com/prometheus/prom2json v1.2.1
-	github.com/ryanuber/go-glob v0.0.0-20160226084822-572520ed46db // indirect
+	github.com/ryanuber/go-glob v0.0.0-20160226084822-572520ed46db
 	github.com/satori/go.uuid v1.2.0
 	github.com/sethgrid/pester v0.0.0-20180227223404-ed9870dad317 // indirect
 	github.com/spf13/cobra v0.0.5

istioctl/cmd/analyze.go

@@ -24,6 +24,9 @@ import (
 	"strings"
 	"time"
 
+	"istio.io/istio/galley/pkg/config/analysis/msg"
+	"istio.io/istio/galley/pkg/config/processing/snapshotter"
+
 	"github.com/ghodss/yaml"
 	"github.com/mattn/go-isatty"
 	"github.com/spf13/cobra"
@@ -70,6 +73,7 @@ var (
 	msgOutputFormat string
 	meshCfgFile     string
 	allNamespaces   bool
+	suppress        []string
 	analysisTimeout time.Duration
 
 	termEnvVar = env.RegisterStringVar("TERM", "", "Specifies terminal type.  Use 'dumb' to suppress color output")
@@ -104,6 +108,13 @@ istioctl analyze a.yaml b.yaml
 # Analyze yaml files without connecting to a live cluster
 istioctl analyze --use-kube=false a.yaml b.yaml
 
+# Analyze the current live cluster and suppress PodMissingProxy for pod mypod in namespace 'testing'.
+istioctl analyze -S "IST0103=Pod mypod.testing"
+
+# Analyze the current live cluster and suppress PodMissingProxy for all pods in namespace 'testing',
+# and suppress MisplacedAnnotation on deployment foobar in namespace default.
+istioctl analyze -S "IST0103=Pod *.testing" -S "IST0107=Deployment foobar.default"
+
 # List available analyzers
 istioctl analyze -L
 `,
@@ -139,6 +150,33 @@ istioctl analyze -L
 			sa := local.NewSourceAnalyzer(schema.MustGet(), analyzers.AllCombined(),
 				resource.Namespace(selectedNamespace), resource.Namespace(istioNamespace), nil, true, analysisTimeout)
 
+			// Check for suppressions and add them to our SourceAnalyzer
+			var suppressions []snapshotter.AnalysisSuppression
+			for _, s := range suppress {
+				parts := strings.Split(s, "=")
+				if len(parts) != 2 {
+					return fmt.Errorf("%s is not a valid suppression value. See istioctl analyze --help", s)
+				}
+				// Check to see if the supplied code is valid. If not, emit a
+				// warning but continue.
+				codeIsValid := false
+				for _, at := range msg.All() {
+					if at.Code() == parts[0] {
+						codeIsValid = true
+						break
+					}
+				}
+
+				if !codeIsValid {
+					fmt.Fprintf(cmd.ErrOrStderr(), "Warning: Supplied message code '%s' is an unknown message code and will not have any effect.\n", parts[0])
+				}
+				suppressions = append(suppressions, snapshotter.AnalysisSuppression{
+					Code:         parts[0],
+					ResourceName: parts[1],
+				})
+			}
+			sa.SetSuppressions(suppressions)
+
 			// If we're using kube, use that as a base source.
 			if useKube {
 				// Set up the kube client
@@ -279,6 +317,10 @@ istioctl analyze -L
 		"Overrides the mesh config values to use for analysis.")
 	analysisCmd.PersistentFlags().BoolVarP(&allNamespaces, "all-namespaces", "A", false,
 		"Analyze all namespaces")
+	analysisCmd.PersistentFlags().StringArrayVarP(&suppress, "suppress", "S", []string{},
+		"Suppress reporting a message code on a specific resource. Values are supplied in the form "+
+			`<code>=<resource> (e.g. '--suppress "IST0102=DestinationRule primary-dr.default"'). Can be repeated. `+
+			`You can include the wildcard character '*' to support a partial match (e.g. '--suppress "IST0102=DestinationRule *.default" ).`)
 	analysisCmd.PersistentFlags().DurationVar(&analysisTimeout, "timeout", 30*time.Second,
 		"the duration to wait before failing")
 	return analysisCmd