From f7509b48faedac7dc8db27b71d878504003bb578 Mon Sep 17 00:00:00 2001 From: Joe Selman Date: Thu, 9 Jan 2020 22:44:47 -0800 Subject: [PATCH] Allow istioctl analyze to suppress messages via cmd line (#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 | 11 ++ .../snapshotter/analyzingdistributor.go | 76 +++++++++---- .../snapshotter/analyzingdistributor_test.go | 107 ++++++++++++++++++ go.mod | 2 +- istioctl/cmd/analyze.go | 42 +++++++ 5 files changed, 218 insertions(+), 20 deletions(-) diff --git a/galley/pkg/config/analysis/local/analyze.go b/galley/pkg/config/analysis/local/analyze.go index 0d3820412951..431cb22f94c1 100644 --- a/galley/pkg/config/analysis/local/analyze.go +++ b/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) diff --git a/galley/pkg/config/processing/snapshotter/analyzingdistributor.go b/galley/pkg/config/processing/snapshotter/analyzingdistributor.go index 5c49138bd87a..f5cdaeb8d926 100644 --- a/galley/pkg/config/processing/snapshotter/analyzingdistributor.go +++ b/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{} diff --git a/galley/pkg/config/processing/snapshotter/analyzingdistributor_test.go b/galley/pkg/config/processing/snapshotter/analyzingdistributor_test.go index 1b3fce53d68b..454dbcca409e 100644 --- a/galley/pkg/config/processing/snapshotter/analyzingdistributor_test.go +++ b/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 { diff --git a/go.mod b/go.mod index 9a92aedf3ae3..8fdf4fb10a77 100644 --- a/go.mod +++ b/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 diff --git a/istioctl/cmd/analyze.go b/istioctl/cmd/analyze.go index cfc2ca7059b3..b64b620b733c 100644 --- a/istioctl/cmd/analyze.go +++ b/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 "+ + `= (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