Plugins: Angular detector: Remote patterns fetching (#69843)

* 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

* Fix merge conflict on updated angular patterns

* 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

* Moved dynamic inspector into pluginsintegration

* Move gcom angulardetectorsprovider into pluginsintegration

* Log errUnknownPatternType at debug level

* re-generate feature flags

* fix error log

pkg/services/pluginsintegration/angulardetector/gcom.go

@@ -0,0 +1,157 @@
+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

pkg/services/pluginsintegration/angulardetector/gcom_test.go

@@ -0,0 +1,144 @@
+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")
+	})
+}

pkg/services/pluginsintegration/angularinspector/angularinspector.go

@@ -0,0 +1,45 @@
+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"
+)
+
+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
+	var err error
+	if cfg.Features != nil && cfg.Features.IsEnabled(featuremgmt.FlagPluginsDynamicAngularDetectionPatterns) {
+		underlying, err = newDynamicInspector(cfg)
+	} else {
+		underlying, err = angularinspector.NewStaticInspector()
+	}
+	if err != nil {
+		return nil, err
+	}
+	return &Service{underlying}, nil
+}

pkg/services/pluginsintegration/angularinspector/angularinspector_test.go

@@ -0,0 +1,42 @@
+package angularinspector
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"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"
+)
+
+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(),
+		})
+		require.NoError(t, err)
+		require.IsType(t, inspector.Inspector, &angularinspector.PatternsListInspector{})
+		patternsListInspector := inspector.Inspector.(*angularinspector.PatternsListInspector)
+		detectors := patternsListInspector.DetectorsProvider.ProvideDetectors(context.Background())
+		require.NotEmpty(t, detectors, "provided detectors should not be empty")
+	})
+
+	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),
+		})
+		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[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")
+	})
+}

pkg/services/pluginsintegration/config/config.go

@@ -42,6 +42,7 @@ func ProvideConfig(settingProvider setting.Provider, grafanaCfg *setting.Cfg, fe
 		tracingCfg,
 		featuremgmt.ProvideToggles(features),
 		grafanaCfg.AngularSupportEnabled,
+		grafanaCfg.GrafanaComURL,
 	), nil
 }
 

pkg/services/pluginsintegration/pluginsintegration.go

@@ -12,7 +12,7 @@ import (
 	"github.com/grafana/grafana/pkg/plugins/manager/client"
 	"github.com/grafana/grafana/pkg/plugins/manager/filestore"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader"
-	"github.com/grafana/grafana/pkg/plugins/manager/loader/angulardetector"
+	pAngularInspector "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angularinspector"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/finder"
 	"github.com/grafana/grafana/pkg/plugins/manager/process"
@@ -25,6 +25,7 @@ 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/angularinspector"
 	"github.com/grafana/grafana/pkg/services/pluginsintegration/clientmiddleware"
 	"github.com/grafana/grafana/pkg/services/pluginsintegration/config"
 	"github.com/grafana/grafana/pkg/services/pluginsintegration/keyretriever"
@@ -52,7 +53,8 @@ var WireSet = wire.NewSet(
 	coreplugin.ProvideCoreRegistry,
 	pluginscdn.ProvideService,
 	assetpath.ProvideService,
-	angulardetector.ProvideService,
+	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)),