From a8d2a9ae2ba1f31e339ad69307bb8970cb0d65ef Mon Sep 17 00:00:00 2001 From: Giuseppe Guerra Date: Thu, 6 Jul 2023 16:34:27 +0200 Subject: [PATCH] Plugins: Angular detector: Add database cache store for remote patterns (#70693) * Plugins: Angular detector: Remote patterns fetching * Renamed PatternType to GCOMPatternType * Renamed files * Renamed more files * Moved files again * Add type checks, unexport GCOM structs * Cache failures, update log messages, fix GCOM URL * Fail silently for unknown pattern types, update docstrings * Fix tests * Rename gcomPattern.Value to gcomPattern.Pattern * Refactoring * Add FlagPluginsRemoteAngularDetectionPatterns feature flag * Fix tests * Re-generate feature flags * Add TestProvideInspector, renamed TestDefaultStaticDetectorsInspector * Add TestProvideInspector * Add TestContainsBytesDetector and TestRegexDetector * Renamed getter to provider * More tests * TestStaticDetectorsProvider, TestSequenceDetectorsProvider * GCOM tests * Lint * Made detector.detect unexported, updated docstrings * Allow changing grafana.com URL * Fix API path, add more logs * Update tryUpdateRemoteDetectors docstring * Use angulardetector http client * Return false, nil if module.js does not exist * Chore: Split angualrdetector into angularinspector and angulardetector packages Moved files around, changed references and fixed tests: - Split the old angulardetector package into angular/angulardetector and angular/angularinspector - angulardetector provides the detection structs/interfaces (Detector, DetectorsProvider...) - angularinspector provides the actual angular detection service used directly in pluginsintegration - Exported most of the stuff that was private and now put into angulardetector, as it is not required by angularinspector * Renamed detector.go -> angulardetector.go and inspector.go -> angularinspector.go Forgot to rename those two files to match the package's names * Renamed angularinspector.ProvideInspector to angularinspector.ProvideService * Renamed "harcoded" to "static" and "remote" to "dynamic" from PR review, matches the same naming schema used for signing keys fetching * WIP: Angular: cache patterns in db, moved gcom into pluginsintegration More similar to signing keys fetching * Rename package, refactoring * try to solve circular import * Fix merge conflict on updated angular patterns * Fix circular imports * Fix wire gen * Add docstrings, refactoring * Removed angualrdetectorsprovider dependency into angularpatternsstore * Moved GCOM test files * Removed GCOM cache * Renamed Detect to DetectAngular and Detector to AngularDetector * Fix call to NewGCOMDetectorsProvider in newDynamicInspector * Removed unused test function newError500GCOMScenario * Added angularinspector service definition in pluginsintegration * refactoring * lint * Fix angularinspector TestProvideService * cleanup * Await initial restore * Register dynamicAngularDetector background service * Removed static detectors provider from pluginsintegration * Add tests for kvstore * Add more tests * order imports in dynamic_test.go * Fix potential panic in dynamic_test * Add "runs the job periodically" test * lint * add timeout to test * refactoring * Removed context.Context from DetectorsProvider * Refactoring, ensure angular dynamic background service is not started if feature flag is off * Fix deadlock on startup * Fix angulardetectorsprovider tests * Revert "Removed context.Context from DetectorsProvider" This reverts commit 4e8c6dded70844709400fa0ce4ce45e66c8458ca. * Fix wrong argument number in dynamic_teset * Standardize gcom http client * Reduce context timeout for angular inspector in plugins loader * Simplify initial restore logic * Fix dynamic detectors provider tests * Chore: removed angulardetector/provider.go * Add more tests * Removed backgroundJob interface, PR review feedback * Update tests * PR review feedback: remove ErrNoCachedValue from kv store Get * Update tests * PR review feedback: add IsDisabled and remove nop background srevice * Update tests * Remove initialRestore channel, use mux instead * Removed backgroundJobInterval, use package-level variable instead * Add TestDynamicAngularDetectorsProviderBackgroundService * Removed timeouts * pr review feedback: restore from store before returning the service * Update tests * Log duration on startup restore and cron run * Switch cron job start log to debug level * Do not attempt to restore if disabled --- .../angularinspector/angularinspector.go | 2 +- pkg/plugins/manager/loader/loader.go | 4 +- .../backgroundsvcs/background_services.go | 3 + .../angulardetector/gcom.go | 157 ------ .../angulardetector/gcom_test.go | 144 ----- .../angulardetectorsprovider/dynamic.go | 261 +++++++++ .../angulardetectorsprovider/dynamic_test.go | 505 ++++++++++++++++++ .../angulardetectorsprovider/gcom.go | 54 ++ .../angulardetectorsprovider/gcom_test.go | 55 ++ .../angularinspector/angularinspector.go | 31 +- .../angularinspector/angularinspector_test.go | 30 +- .../angularpatternsstore/store.go | 75 +++ .../angularpatternsstore/store_test.go | 70 +++ .../pluginsintegration/pluginsintegration.go | 7 +- 14 files changed, 1061 insertions(+), 337 deletions(-) delete mode 100644 pkg/services/pluginsintegration/angulardetector/gcom.go delete mode 100644 pkg/services/pluginsintegration/angulardetector/gcom_test.go create mode 100644 pkg/services/pluginsintegration/angulardetectorsprovider/dynamic.go create mode 100644 pkg/services/pluginsintegration/angulardetectorsprovider/dynamic_test.go create mode 100644 pkg/services/pluginsintegration/angulardetectorsprovider/gcom.go create mode 100644 pkg/services/pluginsintegration/angulardetectorsprovider/gcom_test.go create mode 100644 pkg/services/pluginsintegration/angularpatternsstore/store.go create mode 100644 pkg/services/pluginsintegration/angularpatternsstore/store_test.go diff --git a/pkg/plugins/manager/loader/angular/angularinspector/angularinspector.go b/pkg/plugins/manager/loader/angular/angularinspector/angularinspector.go index dd28a5d8076..2f9299c9d3a 100644 --- a/pkg/plugins/manager/loader/angular/angularinspector/angularinspector.go +++ b/pkg/plugins/manager/loader/angular/angularinspector/angularinspector.go @@ -51,7 +51,7 @@ func (i *PatternsListInspector) Inspect(ctx context.Context, p *plugins.Plugin) return } -// defaultDetectors contains all the detectors to DetectAngular Angular plugins. +// defaultDetectors contains all the detectors to detect Angular plugins. // They are executed in the specified order. var defaultDetectors = []angulardetector.AngularDetector{ &angulardetector.ContainsBytesDetector{Pattern: []byte("PanelCtrl")}, diff --git a/pkg/plugins/manager/loader/loader.go b/pkg/plugins/manager/loader/loader.go index d94a556817a..9852efcf9e7 100644 --- a/pkg/plugins/manager/loader/loader.go +++ b/pkg/plugins/manager/loader/loader.go @@ -191,12 +191,12 @@ func (l *Loader) loadPlugins(ctx context.Context, src plugins.PluginSource, foun if p.IsExternalPlugin() { var err error - cctx, canc := context.WithTimeout(ctx, time.Minute*1) + cctx, canc := context.WithTimeout(ctx, time.Second*10) p.AngularDetected, err = l.angularInspector.Inspect(cctx, p) canc() if err != nil { - l.log.Warn("could not inspect plugin for angular", "pluginID", p.ID, "err", err) + l.log.Warn("Could not inspect plugin for angular", "pluginID", p.ID, "err", err) } // Do not initialize plugins if they're using Angular and Angular support is disabled diff --git a/pkg/server/backgroundsvcs/background_services.go b/pkg/server/backgroundsvcs/background_services.go index 802b5521290..d7254e22be2 100644 --- a/pkg/server/backgroundsvcs/background_services.go +++ b/pkg/server/backgroundsvcs/background_services.go @@ -23,6 +23,7 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert" "github.com/grafana/grafana/pkg/services/notifications" plugindashboardsservice "github.com/grafana/grafana/pkg/services/plugindashboards/service" + "github.com/grafana/grafana/pkg/services/pluginsintegration/angulardetectorsprovider" "github.com/grafana/grafana/pkg/services/pluginsintegration/keyretriever/dynamic" "github.com/grafana/grafana/pkg/services/provisioning" publicdashboardsmetric "github.com/grafana/grafana/pkg/services/publicdashboards/metric" @@ -52,6 +53,7 @@ func ProvideBackgroundServiceRegistry( bundleService *supportbundlesimpl.Service, publicDashboardsMetric *publicdashboardsmetric.Service, keyRetriever *dynamic.KeyRetriever, + dynamicAngularDetectorsProvider *angulardetectorsprovider.Dynamic, // Need to make sure these are initialized, is there a better place to put them? _ dashboardsnapshots.Service, _ *alerting.AlertNotificationService, _ serviceaccounts.Service, _ *guardian.Provider, @@ -89,6 +91,7 @@ func ProvideBackgroundServiceRegistry( bundleService, publicDashboardsMetric, keyRetriever, + dynamicAngularDetectorsProvider, ) } diff --git a/pkg/services/pluginsintegration/angulardetector/gcom.go b/pkg/services/pluginsintegration/angulardetector/gcom.go deleted file mode 100644 index 70ce04f988d..00000000000 --- a/pkg/services/pluginsintegration/angulardetector/gcom.go +++ /dev/null @@ -1,157 +0,0 @@ -package angulardetector - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "net/http" - "net/url" - "regexp" - "time" - - "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" - - "github.com/grafana/grafana/pkg/plugins/log" - "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angulardetector" -) - -const ( - // gcomAngularPatternsPath is the relative path to the GCOM API handler that returns angular detection patterns. - gcomAngularPatternsPath = "/api/plugins/angular_patterns" -) - -var _ angulardetector.DetectorsProvider = &GCOMDetectorsProvider{} - -// GCOMDetectorsProvider is a DetectorsProvider which fetches patterns from GCOM. -type GCOMDetectorsProvider struct { - log log.Logger - - httpClient *http.Client - - baseURL string -} - -// NewGCOMDetectorsProvider returns a new GCOMDetectorsProvider. -// baseURL is the GCOM base url, without /api and without a trailing slash (e.g.: https://grafana.com) -func NewGCOMDetectorsProvider(baseURL string) (angulardetector.DetectorsProvider, error) { - cl, err := httpclient.New() - if err != nil { - return nil, fmt.Errorf("httpclient new: %w", err) - } - return &GCOMDetectorsProvider{ - log: log.New("plugins.angulardetector.gcom"), - baseURL: baseURL, - httpClient: cl, - }, nil -} - -// ProvideDetectors gets the dynamic angular detectors from the remote source. -// If an error occurs, the function fails silently by logging an error, and it returns nil. -func (p *GCOMDetectorsProvider) ProvideDetectors(ctx context.Context) []angulardetector.AngularDetector { - patterns, err := p.fetch(ctx) - if err != nil { - p.log.Warn("Could not fetch remote angular patterns", "error", err) - return nil - } - detectors, err := p.patternsToDetectors(patterns) - if err != nil { - p.log.Warn("Could not convert angular patterns to angularDetectors", "error", err) - return nil - } - return detectors -} - -// fetch fetches the angular patterns from GCOM and returns them as gcomPatterns. -// Call angularDetectors() on the returned value to get the corresponding angular detectors. -func (p *GCOMDetectorsProvider) fetch(ctx context.Context) (gcomPatterns, error) { - st := time.Now() - - reqURL, err := url.JoinPath(p.baseURL, gcomAngularPatternsPath) - if err != nil { - return nil, fmt.Errorf("url joinpath: %w", err) - } - - p.log.Debug("Fetching dynamic angular detection patterns", "url", reqURL) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) - if err != nil { - return nil, fmt.Errorf("new request with context: %w", err) - } - resp, err := p.httpClient.Do(req) - if err != nil { - return nil, fmt.Errorf("http do: %w", err) - } - defer func() { - if closeErr := resp.Body.Close(); closeErr != nil { - p.log.Error("response body close error", "error", err) - } - }() - var out gcomPatterns - if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { - return nil, fmt.Errorf("json decode: %w", err) - } - p.log.Debug("Fetched dynamic angular detection patterns", "patterns", len(out), "duration", time.Since(st)) - return out, nil -} - -// patternsToDetectors converts a slice of gcomPattern into a slice of angulardetector.AngularDetector, by calling -// angularDetector() on each gcomPattern. -func (p *GCOMDetectorsProvider) patternsToDetectors(patterns gcomPatterns) ([]angulardetector.AngularDetector, error) { - var finalErr error - detectors := make([]angulardetector.AngularDetector, 0, len(patterns)) - for _, pattern := range patterns { - d, err := pattern.angularDetector() - if err != nil { - // Fail silently in case of an errUnknownPatternType. - // This allows us to introduce new pattern types without breaking old Grafana versions - if errors.Is(err, errUnknownPatternType) { - p.log.Debug("Unknown angular pattern", "name", pattern.Name, "type", pattern.Type, "error", err) - continue - } - // Other error, do not ignore it - finalErr = errors.Join(finalErr, err) - } - detectors = append(detectors, d) - } - if finalErr != nil { - return nil, finalErr - } - return detectors, nil -} - -// gcomPatternType is a pattern type returned by the GCOM API. -type gcomPatternType string - -const ( - gcomPatternTypeContains gcomPatternType = "contains" - gcomPatternTypeRegex gcomPatternType = "regex" -) - -// errUnknownPatternType is returned when a pattern type is not known. -var errUnknownPatternType = errors.New("unknown pattern type") - -// gcomPattern is an Angular detection pattern returned by the GCOM API. -type gcomPattern struct { - Name string - Pattern string - Type gcomPatternType -} - -// angularDetector converts a gcomPattern into an AngularDetector, based on its Type. -// If a pattern type is unknown, it returns an error wrapping errUnknownPatternType. -func (p *gcomPattern) angularDetector() (angulardetector.AngularDetector, error) { - switch p.Type { - case gcomPatternTypeContains: - return &angulardetector.ContainsBytesDetector{Pattern: []byte(p.Pattern)}, nil - case gcomPatternTypeRegex: - re, err := regexp.Compile(p.Pattern) - if err != nil { - return nil, fmt.Errorf("%q regexp compile: %w", p.Pattern, err) - } - return &angulardetector.RegexDetector{Regex: re}, nil - } - return nil, fmt.Errorf("%q: %w", p.Type, errUnknownPatternType) -} - -// gcomPatterns is a slice of gcomPattern s. -type gcomPatterns []gcomPattern diff --git a/pkg/services/pluginsintegration/angulardetector/gcom_test.go b/pkg/services/pluginsintegration/angulardetector/gcom_test.go deleted file mode 100644 index 5ad5c91b986..00000000000 --- a/pkg/services/pluginsintegration/angulardetector/gcom_test.go +++ /dev/null @@ -1,144 +0,0 @@ -package angulardetector - -import ( - "context" - "net/http" - "net/http/httptest" - "testing" - "time" - - "github.com/stretchr/testify/require" - - "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angulardetector" -) - -var mockGCOMResponse = []byte(`[{ - "name": "PanelCtrl", - "type": "contains", - "pattern": "PanelCtrl" -}, -{ - "name": "QueryCtrl", - "type": "regex", - "pattern": "[\"']QueryCtrl[\"']" -}]`) - -func mockGCOMHTTPHandlerFunc(writer http.ResponseWriter, request *http.Request) { - if request.URL.Path != "/api/plugins/angular_patterns" { - writer.WriteHeader(http.StatusNotFound) - return - } - _, _ = writer.Write(mockGCOMResponse) -} - -func checkMockGCOMResponse(t *testing.T, detectors []angulardetector.AngularDetector) { - require.Len(t, detectors, 2) - d, ok := detectors[0].(*angulardetector.ContainsBytesDetector) - require.True(t, ok) - require.Equal(t, []byte(`PanelCtrl`), d.Pattern) - rd, ok := detectors[1].(*angulardetector.RegexDetector) - require.True(t, ok) - require.Equal(t, `["']QueryCtrl["']`, rd.Regex.String()) -} - -type gcomScenario struct { - gcomHTTPHandlerFunc http.HandlerFunc - gcomHTTPCalls int -} - -func (s *gcomScenario) newHTTPTestServer() *httptest.Server { - return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - s.gcomHTTPCalls++ - s.gcomHTTPHandlerFunc(w, r) - })) -} - -func newDefaultGCOMScenario() *gcomScenario { - return &gcomScenario{gcomHTTPHandlerFunc: mockGCOMHTTPHandlerFunc} -} - -func TestGCOMDetectorsProvider(t *testing.T) { - t.Run("returns value returned from gcom api", func(t *testing.T) { - scenario := newDefaultGCOMScenario() - srv := scenario.newHTTPTestServer() - t.Cleanup(srv.Close) - gcomProvider, err := NewGCOMDetectorsProvider(srv.URL) - require.NoError(t, err) - detectors := gcomProvider.ProvideDetectors(context.Background()) - require.Equal(t, 1, scenario.gcomHTTPCalls, "gcom api should be called") - checkMockGCOMResponse(t, detectors) - }) - - t.Run("error handling", func(t *testing.T) { - for _, tc := range []struct { - *gcomScenario - name string - }{ - {name: "http error 500", gcomScenario: &gcomScenario{ - gcomHTTPHandlerFunc: func(writer http.ResponseWriter, request *http.Request) { - writer.WriteHeader(http.StatusInternalServerError) - }, - }}, - {name: "invalid json", gcomScenario: &gcomScenario{ - gcomHTTPHandlerFunc: func(writer http.ResponseWriter, request *http.Request) { - _, _ = writer.Write([]byte(`not json`)) - }, - }}, - {name: "invalid regex", gcomScenario: &gcomScenario{ - gcomHTTPHandlerFunc: func(writer http.ResponseWriter, request *http.Request) { - _, _ = writer.Write([]byte(`[{"name": "test", "type": "regex", "pattern": "((("}]`)) - }, - }}, - } { - t.Run(tc.name, func(t *testing.T) { - srv := tc.newHTTPTestServer() - t.Cleanup(srv.Close) - gcomProvider, err := NewGCOMDetectorsProvider(srv.URL) - require.NoError(t, err) - detectors := gcomProvider.ProvideDetectors(context.Background()) - require.Equal(t, 1, tc.gcomHTTPCalls, "gcom should be called") - require.Empty(t, detectors, "returned AngularDetectors should be empty") - }) - } - }) - - t.Run("handles gcom timeout", func(t *testing.T) { - gcomScenario := &gcomScenario{ - gcomHTTPHandlerFunc: func(writer http.ResponseWriter, request *http.Request) { - time.Sleep(time.Second * 1) - _, _ = writer.Write([]byte(`[{"name": "test", "type": "regex", "pattern": "((("}]`)) - }, - } - srv := gcomScenario.newHTTPTestServer() - t.Cleanup(srv.Close) - gcomProvider, err := NewGCOMDetectorsProvider(srv.URL) - require.NoError(t, err) - // Expired context - ctx, canc := context.WithTimeout(context.Background(), time.Second*-1) - defer canc() - detectors := gcomProvider.ProvideDetectors(ctx) - require.Zero(t, gcomScenario.gcomHTTPCalls, "gcom should be not called due to request timing out") - require.Empty(t, detectors, "returned AngularDetectors should be empty") - }) - - t.Run("unknown pattern types do not break decoding", func(t *testing.T) { - // Tests that we can introduce new pattern types in the future without breaking old Grafana versions. - - scenario := gcomScenario{gcomHTTPHandlerFunc: func(writer http.ResponseWriter, request *http.Request) { - _, _ = writer.Write([]byte(`[ - {"name": "PanelCtrl", "type": "contains", "pattern": "PanelCtrl"}, - {"name": "Another", "type": "unknown", "pattern": "PanelCtrl"} - ]`)) - }} - srv := scenario.newHTTPTestServer() - t.Cleanup(srv.Close) - gcomProvider, err := NewGCOMDetectorsProvider(srv.URL) - require.NoError(t, err) - detectors := gcomProvider.ProvideDetectors(context.Background()) - require.Equal(t, 1, scenario.gcomHTTPCalls, "gcom should be called") - require.Len(t, detectors, 1, "should have decoded only 1 AngularDetector") - d, ok := detectors[0].(*angulardetector.ContainsBytesDetector) - require.True(t, ok, "decoded pattern should be of the correct type") - require.Equal(t, []byte("PanelCtrl"), d.Pattern, "decoded value for known pattern should be correct") - }) -} diff --git a/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic.go b/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic.go new file mode 100644 index 00000000000..d9e272553e4 --- /dev/null +++ b/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic.go @@ -0,0 +1,261 @@ +package angulardetectorsprovider + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net" + "net/http" + "net/url" + "sync" + "time" + + "github.com/grafana/grafana/pkg/plugins/config" + "github.com/grafana/grafana/pkg/plugins/log" + "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angulardetector" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/pluginsintegration/angularpatternsstore" +) + +// backgroundJobInterval is the interval that passes between background job runs. +// It can be overwritten in tests. +var backgroundJobInterval = time.Hour * 1 + +// Dynamic is an angulardetector.DetectorsProvider that calls GCOM to get Angular detection patterns, +// converts them to detectors and caches them for all future calls. +// It also provides a background service that will periodically refresh the patterns from GCOM. +// If the feature flag FlagPluginsDynamicAngularDetectionPatterns is disabled, the background service is disabled. +type Dynamic struct { + log log.Logger + features featuremgmt.FeatureToggles + + httpClient http.Client + baseURL string + + // store is the underlying angular patterns store used as a cache. + store angularpatternsstore.Service + + // detectors contains the cached angular detectors, which are created from the remote angular patterns. + // mux should be acquired before reading from/writing to this field. + detectors []angulardetector.AngularDetector + + // mux is the mutex used to read/write the cached detectors in a concurrency-safe way. + mux sync.RWMutex +} + +func ProvideDynamic(cfg *config.Cfg, store angularpatternsstore.Service, features featuremgmt.FeatureToggles) (*Dynamic, error) { + d := &Dynamic{ + log: log.New("plugin.angulardetectorsprovider.dynamic"), + features: features, + store: store, + httpClient: makeHttpClient(), + baseURL: cfg.GrafanaComURL, + } + if d.IsDisabled() { + // Do not attempt to restore if the background service is disabled (no feature flag) + return d, nil + } + + // Perform the initial restore from db + st := time.Now() + d.log.Debug("Restoring cache") + if err := d.setDetectorsFromCache(context.Background()); err != nil { + d.log.Warn("Cache restore failed", "error", err) + } else { + d.log.Info("Restored cache from database", "duration", time.Since(st)) + } + return d, nil +} + +// patternsToDetectors converts a slice of gcomPattern into a slice of angulardetector.AngularDetector, by calling +// angularDetector() on each gcomPattern. +func (d *Dynamic) patternsToDetectors(patterns GCOMPatterns) ([]angulardetector.AngularDetector, error) { + var finalErr error + detectors := make([]angulardetector.AngularDetector, 0, len(patterns)) + for _, pattern := range patterns { + ad, err := pattern.angularDetector() + if err != nil { + // Fail silently in case of an errUnknownPatternType. + // This allows us to introduce new pattern types without breaking old Grafana versions + if errors.Is(err, errUnknownPatternType) { + d.log.Debug("Unknown angular pattern", "name", pattern.Name, "type", pattern.Type, "error", err) + continue + } + // Other error, do not ignore it + finalErr = errors.Join(finalErr, err) + } + detectors = append(detectors, ad) + } + if finalErr != nil { + return nil, finalErr + } + return detectors, nil +} + +// fetch fetches the angular patterns from GCOM and returns them as GCOMPatterns. +// Call detectors() on the returned value to get the corresponding detectors. +func (d *Dynamic) fetch(ctx context.Context) (GCOMPatterns, error) { + st := time.Now() + + reqURL, err := url.JoinPath(d.baseURL, gcomAngularPatternsPath) + if err != nil { + return nil, fmt.Errorf("url joinpath: %w", err) + } + + d.log.Debug("Fetching dynamic angular detection patterns", "url", reqURL) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) + if err != nil { + return nil, fmt.Errorf("new request with context: %w", err) + } + resp, err := d.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("http do: %w", err) + } + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + d.log.Error("Response body close error", "error", err) + } + }() + var out GCOMPatterns + if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { + return nil, fmt.Errorf("json decode: %w", err) + } + d.log.Debug("Fetched dynamic angular detection patterns", "patterns", len(out), "duration", time.Since(st)) + return out, nil +} + +// updateDetectors fetches the patterns from GCOM, converts them to detectors, +// stores the patterns in the database and update the cached detectors. +func (d *Dynamic) updateDetectors(ctx context.Context) error { + // Fetch patterns from GCOM + d.mux.Lock() + defer d.mux.Unlock() + patterns, err := d.fetch(ctx) + if err != nil { + return fmt.Errorf("fetch: %w", err) + } + + // Convert the patterns to detectors + newDetectors, err := d.patternsToDetectors(patterns) + if err != nil { + return fmt.Errorf("patterns convert to detectors: %w", err) + } + + // Update store only if the patterns can be converted to detectors + if err := d.store.Set(ctx, patterns); err != nil { + return fmt.Errorf("store set: %w", err) + } + + // Update cached detectors + d.detectors = newDetectors + return nil +} + +// setDetectorsFromCache sets the in-memory detectors from the patterns in the store. +// The caller must Lock d.mux before calling this function. +func (d *Dynamic) setDetectorsFromCache(ctx context.Context) error { + d.mux.Lock() + defer d.mux.Unlock() + + var cachedPatterns GCOMPatterns + rawCached, ok, err := d.store.Get(ctx) + if !ok { + // No cached value found, do not alter in-memory detectors + return nil + } + if err != nil { + return fmt.Errorf("get cached value: %w", err) + } + // Try to unmarshal, convert to detectors and set local cache + if err := json.Unmarshal([]byte(rawCached), &cachedPatterns); err != nil { + return fmt.Errorf("json unmarshal: %w", err) + } + cachedDetectors, err := d.patternsToDetectors(cachedPatterns) + if err != nil { + return fmt.Errorf("convert to detectors: %w", err) + } + d.detectors = cachedDetectors + return nil +} + +// IsDisabled returns true if FlagPluginsDynamicAngularDetectionPatterns is not enabled. +func (d *Dynamic) IsDisabled() bool { + return !d.features.IsEnabled(featuremgmt.FlagPluginsDynamicAngularDetectionPatterns) +} + +// Run is the function implementing the background service and updates the detectors periodically. +func (d *Dynamic) Run(ctx context.Context) error { + d.log.Debug("Started background service") + + // Determine when next run is, and check if we should run immediately + lastUpdate, err := d.store.GetLastUpdated(ctx) + if err != nil { + return fmt.Errorf("get last updated: %w", err) + } + nextRunUntil := time.Until(lastUpdate.Add(backgroundJobInterval)) + + ticker := time.NewTicker(backgroundJobInterval) + defer ticker.Stop() + + var tick <-chan time.Time + if nextRunUntil <= 0 { + // Do first run immediately + firstTick := make(chan time.Time, 1) + tick = firstTick + + firstTick <- time.Now() + } else { + // Do first run after a certain amount of time + ticker.Reset(nextRunUntil) + tick = ticker.C + } + + // Keep running periodically + for { + select { + case <-tick: + st := time.Now() + d.log.Debug("Updating patterns") + + if err := d.updateDetectors(context.Background()); err != nil { + d.log.Error("Error while updating detectors", "error", err) + } + d.log.Info("Patterns update finished", "duration", time.Since(st)) + + // Restore default ticker if we run with a shorter interval the first time + ticker.Reset(backgroundJobInterval) + tick = ticker.C + case <-ctx.Done(): + return ctx.Err() + } + } +} + +// ProvideDetectors returns the cached detectors. It returns an empty slice if there's no value. +func (d *Dynamic) ProvideDetectors(_ context.Context) []angulardetector.AngularDetector { + d.mux.RLock() + r := d.detectors + d.mux.RUnlock() + return r +} + +// Same configuration as pkg/plugins/repo/client.go +func makeHttpClient() http.Client { + tr := &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + } + + return http.Client{ + Timeout: 10 * time.Second, + Transport: tr, + } +} diff --git a/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic_test.go b/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic_test.go new file mode 100644 index 00000000000..0179c292553 --- /dev/null +++ b/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic_test.go @@ -0,0 +1,505 @@ +package angulardetectorsprovider + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/infra/kvstore" + "github.com/grafana/grafana/pkg/plugins/config" + "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angulardetector" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/pluginsintegration/angularpatternsstore" +) + +func TestDynamicAngularDetectorsProvider(t *testing.T) { + mockGCOMPatterns := newMockGCOMPatterns() + gcom := newDefaultGCOMScenario() + srv := gcom.newHTTPTestServer() + t.Cleanup(srv.Close) + + svc := provideDynamic(t, srv.URL) + mockGCOMDetectors, err := svc.patternsToDetectors(mockGCOMPatterns) + require.NoError(t, err) + + t.Run("patternsToDetectors", func(t *testing.T) { + t.Run("valid", func(t *testing.T) { + d, err := svc.patternsToDetectors(mockGCOMPatterns) + require.NoError(t, err) + checkMockDetectorsSlice(t, d) + }) + + t.Run("invalid regex", func(t *testing.T) { + _, err := svc.patternsToDetectors(GCOMPatterns{GCOMPattern{Name: "invalid", Type: GCOMPatternTypeRegex, Pattern: `[`}}) + require.Error(t, err) + }) + + t.Run("unknown pattern type is ignored silently", func(t *testing.T) { + // Tests that we can introduce new pattern types in the future without breaking old Grafana versions. + newPatterns := make(GCOMPatterns, len(mockGCOMPatterns)) + copy(newPatterns, mockGCOMPatterns) + + // Add an unknown pattern at the end + newPatterns = append(newPatterns, GCOMPattern{Name: "Unknown", Pattern: "Unknown", Type: "Unknown"}) + + // Convert patterns to detector and the unknown one should be silently ignored + detectors, err := svc.patternsToDetectors(newPatterns) + require.NoError(t, err) + checkMockDetectorsSlice(t, detectors) + }) + }) + + t.Run("ProvideDetectors", func(t *testing.T) { + t.Run("returns empty result by default", func(t *testing.T) { + svc := provideDynamic(t, srv.URL) + r := svc.ProvideDetectors(context.Background()) + require.Empty(t, r) + }) + + t.Run("awaits initial restore", func(t *testing.T) { + // Prepare mock store + mockStore := angularpatternsstore.ProvideService(kvstore.NewFakeKVStore()) + err := mockStore.Set(context.Background(), mockGCOMPatterns) + require.NoError(t, err) + + svc := provideDynamic(t, srv.URL, provideDynamicOpts{ + store: mockStore, + }) + + // First call to ProvideDetectors should restore from store + r := svc.ProvideDetectors(context.Background()) + checkMockDetectorsSlice(t, r) + + // Ensure the state is modified as well for future calls + checkMockDetectors(t, svc) + + // Ensure it doesn't restore on every call, by modifying the detectors directly + svc.mux.Lock() + svc.detectors = nil + svc.mux.Unlock() + newR := svc.ProvideDetectors(context.Background()) + require.Empty(t, newR) // restore would have filled this with mockGCOMPatterns + }) + }) + + t.Run("fetch", func(t *testing.T) { + t.Run("returns value from gcom api", func(t *testing.T) { + r, err := svc.fetch(context.Background()) + require.NoError(t, err) + + require.True(t, gcom.httpCalls.calledOnce(), "gcom api should be called") + require.Equal(t, mockGCOMPatterns, r) + }) + + t.Run("handles timeout", func(t *testing.T) { + // ctx that expired in the past + ctx, canc := context.WithDeadline(context.Background(), time.Now().Add(time.Second*-30)) + defer canc() + _, err := svc.fetch(ctx) + require.ErrorIs(t, err, context.DeadlineExceeded) + require.False(t, gcom.httpCalls.called(), "gcom api should not be called") + require.Empty(t, svc.ProvideDetectors(context.Background())) + }) + }) + + t.Run("updateDetectors", func(t *testing.T) { + t.Run("successful", func(t *testing.T) { + svc := provideDynamic(t, srv.URL) + + // Check that store is initially empty + dbV, ok, err := svc.store.Get(context.Background()) + require.NoError(t, err) + require.False(t, ok) + require.Empty(t, dbV, "initial store should be empty") + lastUpdated, err := svc.store.GetLastUpdated(context.Background()) + require.NoError(t, err) + require.Zero(t, lastUpdated) + + // Also check in-memory detectors + require.Empty(t, svc.ProvideDetectors(context.Background())) + + // Fetch and store value + err = svc.updateDetectors(context.Background()) + require.NoError(t, err) + checkMockDetectors(t, svc) + + // Check that the value has been updated in the kv store, by reading from the store directly + dbV, ok, err = svc.store.Get(context.Background()) + require.NoError(t, err) + require.True(t, ok) + require.NotEmpty(t, dbV, "new store should not be empty") + var patterns GCOMPatterns + require.NoError(t, json.Unmarshal([]byte(dbV), &patterns), "could not unmarshal stored value") + require.Equal(t, mockGCOMPatterns, patterns) + + // Check that last updated has been updated in the kv store (which is used for cache ttl) + lastUpdated, err = svc.store.GetLastUpdated(context.Background()) + require.NoError(t, err) + require.WithinDuration(t, lastUpdated, time.Now(), time.Second*10, "last updated in store has not been updated") + }) + + t.Run("gcom error does not update store", func(t *testing.T) { + // GCOM scenario that always returns a 500 + scenario := newError500GCOMScenario() + srv := scenario.newHTTPTestServer() + t.Cleanup(srv.Close) + + svc := provideDynamic(t, srv.URL) + + // Set initial cached detectors + svc.mux.Lock() + svc.detectors = mockGCOMDetectors + svc.mux.Unlock() + + // Set initial patterns store as well + err = svc.store.Set(context.Background(), mockGCOMPatterns) + require.NoError(t, err) + + // Try to update from GCOM, but it returns an error + err = svc.updateDetectors(context.Background()) + require.Error(t, err) + require.True(t, scenario.httpCalls.calledOnce(), "gcom api should be called once") + + // Patterns in store should not be modified + dbV, ok, err := svc.store.Get(context.Background()) + require.NoError(t, err) + require.True(t, ok) + require.NotEmpty(t, dbV) + var newPatterns GCOMPatterns + err = json.Unmarshal([]byte(dbV), &newPatterns) + require.NoError(t, err) + require.Equal(t, mockGCOMPatterns, newPatterns, "store should not be modified") + + // Same for in-memory detectors + checkMockDetectors(t, svc) + }) + }) + + t.Run("setDetectorsFromCache", func(t *testing.T) { + t.Run("empty store doesn't return an error", func(t *testing.T) { + svc := provideDynamic(t, srv.URL) + + err := svc.setDetectorsFromCache(context.Background()) + require.NoError(t, err) + require.Empty(t, svc.ProvideDetectors(context.Background())) + }) + + t.Run("store is restored before returning the service", func(t *testing.T) { + // Populate store + store := angularpatternsstore.ProvideService(kvstore.NewFakeKVStore()) + err := store.Set(context.Background(), mockGCOMPatterns) + require.NoError(t, err) + + svc := provideDynamic(t, srv.URL, provideDynamicOpts{ + store: store, + }) + + // Restore + detectors := svc.ProvideDetectors(context.Background()) + require.Equal(t, mockGCOMDetectors, detectors) + }) + }) +} + +func TestDynamicAngularDetectorsProviderBackgroundService(t *testing.T) { + mockGCOMPatterns := newMockGCOMPatterns() + gcom := newDefaultGCOMScenario() + srv := gcom.newHTTPTestServer() + t.Cleanup(srv.Close) + + t.Run("background service", func(t *testing.T) { + oldBackgroundJobInterval := backgroundJobInterval + backgroundJobInterval = time.Millisecond * 500 + t.Cleanup(func() { + backgroundJobInterval = oldBackgroundJobInterval + }) + + t.Run("is disabled if feature flag is not present", func(t *testing.T) { + svc := provideDynamic(t, srv.URL) + svc.features = featuremgmt.WithFeatures() + require.True(t, svc.IsDisabled(), "background service should be disabled") + }) + + t.Run("is enabled if feature flag is present", func(t *testing.T) { + svc := provideDynamic(t, srv.URL) + svc.features = featuremgmt.WithFeatures(featuremgmt.FlagPluginsDynamicAngularDetectionPatterns) + require.False(t, svc.IsDisabled(), "background service should be enabled") + }) + + t.Run("fetches value from gcom on start if too much time has passed", func(t *testing.T) { + gcomCallback := make(chan struct{}) + gcom := newDefaultGCOMScenario(func(_ http.ResponseWriter, _ *http.Request) { + gcomCallback <- struct{}{} + }) + srv := gcom.newHTTPTestServer() + svc := provideDynamic(t, srv.URL) + mockStore := &mockLastUpdatePatternsStore{ + Service: svc.store, + // Expire cache + lastUpdated: time.Now().Add(time.Hour * -24), + } + svc.store = mockStore + + // Store mock GCOM patterns + err := mockStore.Set(context.Background(), mockGCOMPatterns) + require.NoError(t, err) + + // Ensure the detectors are initially empty + require.Empty(t, svc.ProvideDetectors(context.Background())) + + // Start bg service and it should call GCOM immediately + bg := newBackgroundServiceScenario(svc) + t.Cleanup(bg.close) + bg.run(context.Background()) + + // Await job call with timeout + select { + case <-time.After(time.Second * 10): + t.Fatal("timeout") + case <-gcomCallback: + break + } + require.True(t, gcom.httpCalls.calledOnce(), "gcom api should be called once") + + // Check new cached value + checkMockDetectors(t, svc) + bg.exitAndWait() + }) + + t.Run("runs the job periodically", func(t *testing.T) { + const tcRuns = 3 + + lastJobTime := time.Now() + var jobCalls counter + const jobInterval = time.Millisecond * 500 + done := make(chan struct{}) + gcom := newDefaultGCOMScenario(func(_ http.ResponseWriter, _ *http.Request) { + now := time.Now() + assert.WithinDuration(t, now, lastJobTime, jobInterval+jobInterval/2) + lastJobTime = now + + jobCalls.inc() + if jobCalls.calls() == tcRuns { + // this is done ONLY once + done <- struct{}{} + close(done) + } + }) + srv := gcom.newHTTPTestServer() + t.Cleanup(srv.Close) + svc := provideDynamic(t, srv.URL) + + bg := newBackgroundServiceScenario(svc) + t.Cleanup(bg.close) + // Refresh cache right before running the service, so we skip the initial run + require.NoError(t, svc.store.Set(context.Background(), mockGCOMPatterns)) + bg.run(context.Background()) + select { + case <-time.After(time.Second * 10): + t.Fatal("timeout") + case <-done: + break + } + bg.exitAndWait() + + require.True(t, jobCalls.calledX(tcRuns), "should have the correct number of job calls") + require.True(t, gcom.httpCalls.calledX(tcRuns), "should have the correct number of gcom api calls") + }) + }) +} + +var mockGCOMResponse = []byte(`[{ + "name": "PanelCtrl", + "type": "contains", + "pattern": "PanelCtrl" +}, +{ + "name": "QueryCtrl", + "type": "regex", + "pattern": "[\"']QueryCtrl[\"']" +}]`) + +func mockGCOMHTTPHandlerFunc(writer http.ResponseWriter, request *http.Request) { + if request.URL.Path != "/api/plugins/angular_patterns" { + writer.WriteHeader(http.StatusNotFound) + return + } + _, _ = writer.Write(mockGCOMResponse) +} + +func checkMockDetectorsSlice(t *testing.T, detectors []angulardetector.AngularDetector) { + require.Len(t, detectors, 2) + d, ok := detectors[0].(*angulardetector.ContainsBytesDetector) + require.True(t, ok) + require.Equal(t, []byte(`PanelCtrl`), d.Pattern) + rd, ok := detectors[1].(*angulardetector.RegexDetector) + require.True(t, ok) + require.Equal(t, `["']QueryCtrl["']`, rd.Regex.String()) +} + +func checkMockDetectors(t *testing.T, d *Dynamic) { + checkMockDetectorsSlice(t, d.ProvideDetectors(context.Background())) +} + +func newMockGCOMPatterns() GCOMPatterns { + var mockGCOMPatterns GCOMPatterns + if err := json.Unmarshal(mockGCOMResponse, &mockGCOMPatterns); err != nil { + panic(err) + } + return mockGCOMPatterns +} + +type counter struct { + count int + lastAssertCount int + mux sync.Mutex +} + +func (c *counter) inc() { + c.mux.Lock() + c.count++ + c.mux.Unlock() +} + +func (c *counter) calls() int { + c.mux.Lock() + defer c.mux.Unlock() + return c.count +} + +func (c *counter) called() bool { + c.mux.Lock() + defer c.mux.Unlock() + r := c.count > c.lastAssertCount + c.lastAssertCount = c.count + return r +} + +func (c *counter) calledX(x int) bool { + c.mux.Lock() + defer c.mux.Unlock() + r := c.count == x + c.lastAssertCount = c.count + return r +} + +func (c *counter) calledOnce() bool { + return c.calledX(1) +} + +type gcomScenario struct { + httpHandlerFunc http.HandlerFunc + httpCalls counter +} + +func (s *gcomScenario) newHTTPTestServer() *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + s.httpCalls.inc() + s.httpHandlerFunc(w, r) + })) +} + +func newDefaultGCOMScenario(middlewares ...http.HandlerFunc) *gcomScenario { + return &gcomScenario{httpHandlerFunc: func(w http.ResponseWriter, req *http.Request) { + mockGCOMHTTPHandlerFunc(w, req) + for _, f := range middlewares { + f(w, req) + } + }} +} + +func newError500GCOMScenario() *gcomScenario { + return &gcomScenario{httpHandlerFunc: func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }} +} + +type provideDynamicOpts struct { + store angularpatternsstore.Service +} + +func provideDynamic(t *testing.T, gcomURL string, opts ...provideDynamicOpts) *Dynamic { + if len(opts) == 0 { + opts = []provideDynamicOpts{{}} + } + opt := opts[0] + if opt.store == nil { + opt.store = angularpatternsstore.ProvideService(kvstore.NewFakeKVStore()) + } + d, err := ProvideDynamic( + &config.Cfg{GrafanaComURL: gcomURL}, + opt.store, + featuremgmt.WithFeatures(featuremgmt.FlagPluginsDynamicAngularDetectionPatterns), + ) + require.NoError(t, err) + return d +} + +// mockLastUpdatePatternsStore wraps an angularpatternsstore.Service and returns a pre-defined value (lastUpdated) +// when calling GetLastUpdated. All other method calls are sent to the wrapped angularpatternsstore.Service. +type mockLastUpdatePatternsStore struct { + angularpatternsstore.Service + lastUpdated time.Time +} + +// GetLastUpdated always returns s.lastUpdated. +func (s *mockLastUpdatePatternsStore) GetLastUpdated(_ context.Context) (time.Time, error) { + return s.lastUpdated, nil +} + +type backgroundServiceScenario struct { + svc *Dynamic + wg sync.WaitGroup + ctxCancFunc context.CancelFunc +} + +func newBackgroundServiceScenario(svc *Dynamic) *backgroundServiceScenario { + return &backgroundServiceScenario{ + svc: svc, + } +} + +func (s *backgroundServiceScenario) close() { + if s.ctxCancFunc == nil { + return + } + s.ctxCancFunc() +} + +func (s *backgroundServiceScenario) exitAndWait() { + if s.ctxCancFunc == nil { + panic("run was not called") + } + // Make bg service exit + s.close() + s.ctxCancFunc = nil + // Wait for bg svc to quit + s.wg.Wait() +} + +func (s *backgroundServiceScenario) run(ctx context.Context) { + if s.ctxCancFunc != nil { + panic("run was called more than once") + } + ctx, canc := context.WithCancel(ctx) + // Store this canc func, so we can make the bg goroutine exit on demand + s.ctxCancFunc = canc + + // Start background service + s.wg.Add(1) + go func() { + defer s.wg.Done() + err := s.svc.Run(ctx) + if err != nil && !errors.Is(err, context.Canceled) { + panic(err) + } + }() +} diff --git a/pkg/services/pluginsintegration/angulardetectorsprovider/gcom.go b/pkg/services/pluginsintegration/angulardetectorsprovider/gcom.go new file mode 100644 index 00000000000..d2958b01aec --- /dev/null +++ b/pkg/services/pluginsintegration/angulardetectorsprovider/gcom.go @@ -0,0 +1,54 @@ +package angulardetectorsprovider + +import ( + "errors" + "fmt" + "regexp" + + "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angulardetector" +) + +// gcomAngularPatternsPath is the relative path to the GCOM API handler that returns angular detection patterns. +const gcomAngularPatternsPath = "/api/plugins/angular_patterns" + +// GCOMPatternType is a pattern type returned by the GCOM API. +type GCOMPatternType string + +const ( + GCOMPatternTypeContains GCOMPatternType = "contains" + GCOMPatternTypeRegex GCOMPatternType = "regex" +) + +// GCOMPattern is an Angular detection pattern returned by the GCOM API. +type GCOMPattern struct { + Name string + Pattern string + Type GCOMPatternType +} + +var ( + // errUnknownPatternType is returned when a pattern type is not known. + errUnknownPatternType = errors.New("unknown pattern type") + + // errInvalidRegex is returned when a regex pattern has an invalid regex. + errInvalidRegex = errors.New("invalid regex") +) + +// angularDetector converts a gcomPattern into a Detector, based on its Type. +// If a pattern type is unknown, it returns an error wrapping errUnknownPatternType. +func (p *GCOMPattern) angularDetector() (angulardetector.AngularDetector, error) { + switch p.Type { + case GCOMPatternTypeContains: + return &angulardetector.ContainsBytesDetector{Pattern: []byte(p.Pattern)}, nil + case GCOMPatternTypeRegex: + re, err := regexp.Compile(p.Pattern) + if err != nil { + return nil, fmt.Errorf("%q regexp compile: %w: %s", p.Pattern, errInvalidRegex, err) + } + return &angulardetector.RegexDetector{Regex: re}, nil + } + return nil, fmt.Errorf("%q: %w", p.Type, errUnknownPatternType) +} + +// GCOMPatterns is a slice of GCOMPattern +type GCOMPatterns []GCOMPattern diff --git a/pkg/services/pluginsintegration/angulardetectorsprovider/gcom_test.go b/pkg/services/pluginsintegration/angulardetectorsprovider/gcom_test.go new file mode 100644 index 00000000000..ef4fc7b228d --- /dev/null +++ b/pkg/services/pluginsintegration/angulardetectorsprovider/gcom_test.go @@ -0,0 +1,55 @@ +package angulardetectorsprovider + +import ( + "regexp" + "testing" + + "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angulardetector" + "github.com/stretchr/testify/require" +) + +func TestGCOMPatterns(t *testing.T) { + t.Run("angularDetector", func(t *testing.T) { + type tc struct { + name string + pattern GCOMPattern + exp func(t *testing.T, d angulardetector.AngularDetector) + expError error + } + for _, c := range []tc{ + { + name: "contains", + pattern: GCOMPattern{Name: "test", Pattern: "pattern", Type: GCOMPatternTypeContains}, + exp: func(t *testing.T, d angulardetector.AngularDetector) { + require.Equal(t, &angulardetector.ContainsBytesDetector{Pattern: []byte("pattern")}, d) + }, + }, + { + name: "regex", + pattern: GCOMPattern{Name: "test", Pattern: `[0-9]+`, Type: GCOMPatternTypeRegex}, + exp: func(t *testing.T, d angulardetector.AngularDetector) { + require.Equal(t, &angulardetector.RegexDetector{Regex: regexp.MustCompile(`[0-9]+`)}, d) + }, + }, + { + name: "invalid regex returns errInvalidRegex", + pattern: GCOMPattern{Name: "test", Pattern: `[`, Type: GCOMPatternTypeRegex}, + expError: errInvalidRegex, + }, + { + name: "invalid type returns errUnknownPatternType", + pattern: GCOMPattern{Name: "test", Pattern: "abc", Type: "unknown"}, + expError: errUnknownPatternType, + }, + } { + t.Run(c.name, func(t *testing.T) { + d, err := c.pattern.angularDetector() + if c.expError != nil { + require.ErrorIs(t, err, c.expError) + } else { + c.exp(t, d) + } + }) + } + }) +} diff --git a/pkg/services/pluginsintegration/angularinspector/angularinspector.go b/pkg/services/pluginsintegration/angularinspector/angularinspector.go index bd945d8c500..b637c11ce73 100644 --- a/pkg/services/pluginsintegration/angularinspector/angularinspector.go +++ b/pkg/services/pluginsintegration/angularinspector/angularinspector.go @@ -1,45 +1,28 @@ package angularinspector import ( - "fmt" - "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angulardetector" "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angularinspector" "github.com/grafana/grafana/pkg/services/featuremgmt" - pAngularDetector "github.com/grafana/grafana/pkg/services/pluginsintegration/angulardetector" + "github.com/grafana/grafana/pkg/services/pluginsintegration/angulardetectorsprovider" ) type Service struct { angularinspector.Inspector } -// newDynamicInspector returns the default dynamic Inspector, which is a PatternsListInspector that will: -// 1. Try to get the Angular detectors from GCOM -// 2. If it fails, it will use the static (hardcoded) detections provided by defaultDetectors. -func newDynamicInspector(cfg *config.Cfg) (angularinspector.Inspector, error) { - dynamicProvider, err := pAngularDetector.NewGCOMDetectorsProvider(cfg.GrafanaComURL) - if err != nil { - return nil, fmt.Errorf("NewGCOMDetectorsProvider: %w", err) - } - return &angularinspector.PatternsListInspector{ - DetectorsProvider: angulardetector.SequenceDetectorsProvider{ - dynamicProvider, - angularinspector.NewDefaultStaticDetectorsProvider(), - }, - }, nil -} - -func ProvideService(cfg *config.Cfg) (*Service, error) { - var underlying angularinspector.Inspector +func ProvideService(cfg *config.Cfg, dynamic *angulardetectorsprovider.Dynamic) (*Service, error) { + var detectorsProvider angulardetector.DetectorsProvider var err error + static := angularinspector.NewDefaultStaticDetectorsProvider() if cfg.Features != nil && cfg.Features.IsEnabled(featuremgmt.FlagPluginsDynamicAngularDetectionPatterns) { - underlying, err = newDynamicInspector(cfg) + detectorsProvider = angulardetector.SequenceDetectorsProvider{dynamic, static} } else { - underlying, err = angularinspector.NewStaticInspector() + detectorsProvider = static } if err != nil { return nil, err } - return &Service{underlying}, nil + return &Service{Inspector: &angularinspector.PatternsListInspector{DetectorsProvider: detectorsProvider}}, nil } diff --git a/pkg/services/pluginsintegration/angularinspector/angularinspector_test.go b/pkg/services/pluginsintegration/angularinspector/angularinspector_test.go index 7fb5f2f932e..de98c2f8382 100644 --- a/pkg/services/pluginsintegration/angularinspector/angularinspector_test.go +++ b/pkg/services/pluginsintegration/angularinspector/angularinspector_test.go @@ -6,18 +6,25 @@ import ( "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angulardetector" "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angularinspector" "github.com/grafana/grafana/pkg/services/featuremgmt" - pAngularDetector "github.com/grafana/grafana/pkg/services/pluginsintegration/angulardetector" + "github.com/grafana/grafana/pkg/services/pluginsintegration/angulardetectorsprovider" + "github.com/grafana/grafana/pkg/services/pluginsintegration/angularpatternsstore" ) func TestProvideService(t *testing.T) { t.Run("uses hardcoded inspector if feature flag is not present", func(t *testing.T) { - inspector, err := ProvideService(&config.Cfg{ - Features: featuremgmt.WithFeatures(), - }) + pCfg := &config.Cfg{Features: featuremgmt.WithFeatures()} + dynamic, err := angulardetectorsprovider.ProvideDynamic( + pCfg, + angularpatternsstore.ProvideService(kvstore.NewFakeKVStore()), + featuremgmt.WithFeatures(featuremgmt.FlagPluginsDynamicAngularDetectionPatterns), + ) + require.NoError(t, err) + inspector, err := ProvideService(pCfg, dynamic) require.NoError(t, err) require.IsType(t, inspector.Inspector, &angularinspector.PatternsListInspector{}) patternsListInspector := inspector.Inspector.(*angularinspector.PatternsListInspector) @@ -26,15 +33,22 @@ func TestProvideService(t *testing.T) { }) t.Run("uses dynamic inspector with hardcoded fallback if feature flag is present", func(t *testing.T) { - inspector, err := ProvideService(&config.Cfg{ - Features: featuremgmt.WithFeatures(featuremgmt.FlagPluginsDynamicAngularDetectionPatterns), - }) + pCfg := &config.Cfg{Features: featuremgmt.WithFeatures( + featuremgmt.FlagPluginsDynamicAngularDetectionPatterns, + )} + dynamic, err := angulardetectorsprovider.ProvideDynamic( + pCfg, + angularpatternsstore.ProvideService(kvstore.NewFakeKVStore()), + featuremgmt.WithFeatures(), + ) + require.NoError(t, err) + inspector, err := ProvideService(pCfg, dynamic) require.NoError(t, err) require.IsType(t, inspector.Inspector, &angularinspector.PatternsListInspector{}) require.IsType(t, inspector.Inspector.(*angularinspector.PatternsListInspector).DetectorsProvider, angulardetector.SequenceDetectorsProvider{}) seq := inspector.Inspector.(*angularinspector.PatternsListInspector).DetectorsProvider.(angulardetector.SequenceDetectorsProvider) require.Len(t, seq, 2, "should return the correct number of providers") - require.IsType(t, seq[0], &pAngularDetector.GCOMDetectorsProvider{}, "first AngularDetector provided should be gcom") + require.IsType(t, seq[0], &angulardetectorsprovider.Dynamic{}, "first AngularDetector provided should be gcom") require.IsType(t, seq[1], &angulardetector.StaticDetectorsProvider{}, "second AngularDetector provided should be static") staticDetectors := seq[1].ProvideDetectors(context.Background()) require.NotEmpty(t, staticDetectors, "provided static detectors should not be empty") diff --git a/pkg/services/pluginsintegration/angularpatternsstore/store.go b/pkg/services/pluginsintegration/angularpatternsstore/store.go new file mode 100644 index 00000000000..3557d972ade --- /dev/null +++ b/pkg/services/pluginsintegration/angularpatternsstore/store.go @@ -0,0 +1,75 @@ +package angularpatternsstore + +import ( + "context" + "encoding/json" + "fmt" + "time" + + "github.com/grafana/grafana/pkg/infra/kvstore" +) + +type Service interface { + Get(ctx context.Context) (string, bool, error) + Set(ctx context.Context, patterns any) error + GetLastUpdated(ctx context.Context) (time.Time, error) +} + +const ( + kvNamespace = "plugin.angularpatterns" + + keyPatterns = "angular_patterns" + keyLastUpdated = "last_updated" +) + +// KVStoreService allows to cache GCOM angular patterns into the database, as a cache. +type KVStoreService struct { + kv *kvstore.NamespacedKVStore +} + +func ProvideService(kv kvstore.KVStore) Service { + return &KVStoreService{ + kv: kvstore.WithNamespace(kv, 0, kvNamespace), + } +} + +// Get returns the raw cached angular detection patterns. The returned value is a JSON-encoded string. +// If no value is present, the second argument is false and the returned error is nil. +func (s *KVStoreService) Get(ctx context.Context) (string, bool, error) { + return s.kv.Get(ctx, keyPatterns) +} + +// Set sets the cached angular detection patterns and the latest update time to time.Now(). +// patterns must implement json.Marshaler. +func (s *KVStoreService) Set(ctx context.Context, patterns any) error { + b, err := json.Marshal(patterns) + if err != nil { + return fmt.Errorf("json marshal: %w", err) + } + if err := s.kv.Set(ctx, keyPatterns, string(b)); err != nil { + return fmt.Errorf("kv set: %w", err) + } + if err := s.kv.Set(ctx, keyLastUpdated, time.Now().Format(time.RFC3339)); err != nil { + return fmt.Errorf("kv last updated set: %w", err) + } + return nil +} + +// GetLastUpdated returns the time when Set was last called. If the value cannot be unmarshalled correctly, +// it returns a zero-value time.Time. +func (s *KVStoreService) GetLastUpdated(ctx context.Context) (time.Time, error) { + v, ok, err := s.kv.Get(ctx, keyLastUpdated) + if err != nil { + return time.Time{}, fmt.Errorf("kv get: %w", err) + } + if !ok { + return time.Time{}, nil + } + t, err := time.Parse(time.RFC3339, v) + if err != nil { + // Ignore decode errors, so we can change the format in future versions + // and keep backwards/forwards compatibility + return time.Time{}, nil + } + return t, nil +} diff --git a/pkg/services/pluginsintegration/angularpatternsstore/store_test.go b/pkg/services/pluginsintegration/angularpatternsstore/store_test.go new file mode 100644 index 00000000000..73c7d8b36d8 --- /dev/null +++ b/pkg/services/pluginsintegration/angularpatternsstore/store_test.go @@ -0,0 +1,70 @@ +package angularpatternsstore + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/infra/kvstore" +) + +func TestAngularPatternsStore(t *testing.T) { + mockPatterns := []map[string]interface{}{ + {"name": "PanelCtrl", "type": "contains", "pattern": "PanelCtrl"}, + {"name": "ConfigCtrl", "type": "contains", "pattern": "ConfigCtrl"}, + } + + t.Run("get set", func(t *testing.T) { + svc := ProvideService(kvstore.NewFakeKVStore()) + + t.Run("get empty", func(t *testing.T) { + _, ok, err := svc.Get(context.Background()) + require.NoError(t, err) + require.False(t, ok) + }) + + t.Run("set and get", func(t *testing.T) { + err := svc.Set(context.Background(), mockPatterns) + require.NoError(t, err) + + expV, err := json.Marshal(mockPatterns) + require.NoError(t, err) + + dbV, ok, err := svc.Get(context.Background()) + require.NoError(t, err) + require.True(t, ok) + require.Equal(t, string(expV), dbV) + }) + }) + + t.Run("latest update", func(t *testing.T) { + svc := ProvideService(kvstore.NewFakeKVStore()) + + t.Run("empty", func(t *testing.T) { + lastUpdated, err := svc.GetLastUpdated(context.Background()) + require.NoError(t, err) + require.Zero(t, lastUpdated) + }) + + t.Run("not empty", func(t *testing.T) { + err := svc.Set(context.Background(), mockPatterns) + require.NoError(t, err) + + lastUpdated, err := svc.GetLastUpdated(context.Background()) + require.NoError(t, err) + require.WithinDuration(t, time.Now(), lastUpdated, time.Second*10) + }) + + t.Run("invalid timestamp stored", func(t *testing.T) { + err := svc.(*KVStoreService).kv.Set(context.Background(), keyLastUpdated, "abcd") + require.NoError(t, err) + + lastUpdated, err := svc.GetLastUpdated(context.Background()) + require.NoError(t, err) + require.Zero(t, lastUpdated) + }) + }) +} diff --git a/pkg/services/pluginsintegration/pluginsintegration.go b/pkg/services/pluginsintegration/pluginsintegration.go index 53afa936696..5b9e86b8671 100644 --- a/pkg/services/pluginsintegration/pluginsintegration.go +++ b/pkg/services/pluginsintegration/pluginsintegration.go @@ -2,7 +2,6 @@ package pluginsintegration import ( "github.com/google/wire" - "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/backendplugin/coreplugin" @@ -26,7 +25,9 @@ import ( "github.com/grafana/grafana/pkg/services/caching" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/oauthtoken" + "github.com/grafana/grafana/pkg/services/pluginsintegration/angulardetectorsprovider" "github.com/grafana/grafana/pkg/services/pluginsintegration/angularinspector" + "github.com/grafana/grafana/pkg/services/pluginsintegration/angularpatternsstore" "github.com/grafana/grafana/pkg/services/pluginsintegration/clientmiddleware" "github.com/grafana/grafana/pkg/services/pluginsintegration/config" "github.com/grafana/grafana/pkg/services/pluginsintegration/keyretriever" @@ -55,8 +56,12 @@ var WireSet = wire.NewSet( coreplugin.ProvideCoreRegistry, pluginscdn.ProvideService, assetpath.ProvideService, + + angularpatternsstore.ProvideService, + angulardetectorsprovider.ProvideDynamic, angularinspector.ProvideService, wire.Bind(new(pAngularInspector.Inspector), new(*angularinspector.Service)), + loader.ProvideService, wire.Bind(new(loader.Service), new(*loader.Loader)), wire.Bind(new(plugins.ErrorResolver), new(*loader.Loader)),