From ec6412bccae7bb35ac043ae6d8349591d11ec945 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Fri, 28 Jul 2023 15:18:25 +0200 Subject: [PATCH] Plugins: Use plugins config to source app URL (#72490) * use plugins config for app URL * merge with main * add missing file * add fg * fix tests --- pkg/api/plugin_resource_test.go | 2 +- pkg/plugins/manager/loader/loader_test.go | 37 +++++-------------- .../manager/manager_integration_test.go | 2 +- .../manager/pipeline/bootstrap/bootstrap.go | 2 +- pkg/plugins/manager/signature/manifest.go | 19 ++++++---- .../manager/signature/manifest_test.go | 24 +++++------- 6 files changed, 32 insertions(+), 54 deletions(-) diff --git a/pkg/api/plugin_resource_test.go b/pkg/api/plugin_resource_test.go index b3497f5e6d4..68353159a5e 100644 --- a/pkg/api/plugin_resource_test.go +++ b/pkg/api/plugin_resource_test.go @@ -72,7 +72,7 @@ func TestCallResource(t *testing.T) { require.NoError(t, err) discovery := pipeline.ProvideDiscoveryStage(pCfg, finder.NewLocalFinder(pCfg.DevMode), reg) - bootstrap := pipeline.ProvideBootstrapStage(pCfg, signature.ProvideService(statickey.New()), assetpath.ProvideService(pluginscdn.ProvideService(pCfg))) + bootstrap := pipeline.ProvideBootstrapStage(pCfg, signature.ProvideService(pCfg, statickey.New()), assetpath.ProvideService(pluginscdn.ProvideService(pCfg))) l := loader.ProvideService(pCfg, fakes.NewFakeLicensingService(), signature.NewUnsignedAuthorizer(pCfg), reg, provider.ProvideService(coreRegistry), fakes.NewFakeRoleRegistry(), diff --git a/pkg/plugins/manager/loader/loader_test.go b/pkg/plugins/manager/loader/loader_test.go index 68f3a3275ab..a1cfcc11205 100644 --- a/pkg/plugins/manager/loader/loader_test.go +++ b/pkg/plugins/manager/loader/loader_test.go @@ -556,15 +556,15 @@ func TestLoader_Load_MultiplePlugins(t *testing.T) { name string cfg *config.Cfg pluginPaths []string - appURL string existingPlugins map[string]struct{} want []*plugins.Plugin pluginErrors map[string]*plugins.Error }{ { - name: "Load multiple plugins (broken, valid, unsigned)", - cfg: &config.Cfg{}, - appURL: "http://localhost:3000", + name: "Load multiple plugins (broken, valid, unsigned)", + cfg: &config.Cfg{ + GrafanaAppURL: "http://localhost:3000", + }, pluginPaths: []string{ "../testdata/invalid-plugin-json", // test-app "../testdata/valid-v2-pvt-signature", // test @@ -624,12 +624,6 @@ func TestLoader_Load_MultiplePlugins(t *testing.T) { l.pluginInitializer = initializer.New(tt.cfg, procPrvdr, fakes.NewFakeLicensingService()) }) t.Run(tt.name, func(t *testing.T) { - origAppURL := setting.AppUrl - t.Cleanup(func() { - setting.AppUrl = origAppURL - }) - setting.AppUrl = tt.appURL - got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal @@ -667,14 +661,14 @@ func TestLoader_Load_RBACReady(t *testing.T) { name string cfg *config.Cfg pluginPaths []string - appURL string existingPlugins map[string]struct{} want []*plugins.Plugin }{ { - name: "Load plugin defining one RBAC role", - cfg: &config.Cfg{}, - appURL: "http://localhost:3000", + name: "Load plugin defining one RBAC role", + cfg: &config.Cfg{ + GrafanaAppURL: "http://localhost:3000", + }, pluginPaths: []string{"../testdata/test-app-with-roles"}, want: []*plugins.Plugin{ { @@ -731,11 +725,6 @@ func TestLoader_Load_RBACReady(t *testing.T) { } for _, tt := range tests { - origAppURL := setting.AppUrl - t.Cleanup(func() { - setting.AppUrl = origAppURL - }) - setting.AppUrl = "http://localhost:3000" reg := fakes.NewFakePluginRegistry() procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() @@ -775,14 +764,6 @@ func TestLoader_Load_Signature_RootURL(t *testing.T) { } t.Run("Private signature verification ignores trailing slash in root URL", func(t *testing.T) { - origAppURL := setting.AppUrl - origAppSubURL := setting.AppSubUrl - t.Cleanup(func() { - setting.AppUrl = origAppURL - setting.AppSubUrl = origAppSubURL - }) - setting.AppUrl = defaultAppURL - paths := []string{"../testdata/valid-v2-pvt-signature-root-url-uri"} expected := []*plugins.Plugin{ @@ -818,7 +799,7 @@ func TestLoader_Load_Signature_RootURL(t *testing.T) { reg := fakes.NewFakePluginRegistry() procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() - l := newLoader(t, &config.Cfg{}, func(l *Loader) { + l := newLoader(t, &config.Cfg{GrafanaAppURL: defaultAppURL}, func(l *Loader) { l.pluginRegistry = reg l.processManager = procMgr l.pluginInitializer = initializer.New(&config.Cfg{}, procPrvdr, fakes.NewFakeLicensingService()) diff --git a/pkg/plugins/manager/manager_integration_test.go b/pkg/plugins/manager/manager_integration_test.go index 9526f49b7f7..17fd7789919 100644 --- a/pkg/plugins/manager/manager_integration_test.go +++ b/pkg/plugins/manager/manager_integration_test.go @@ -123,7 +123,7 @@ func TestIntegrationPluginManager(t *testing.T) { require.NoError(t, err) discovery := pipeline.ProvideDiscoveryStage(pCfg, finder.NewLocalFinder(pCfg.DevMode), reg) - bootstrap := pipeline.ProvideBootstrapStage(pCfg, signature.ProvideService(statickey.New()), assetpath.ProvideService(pluginscdn.ProvideService(pCfg))) + bootstrap := pipeline.ProvideBootstrapStage(pCfg, signature.ProvideService(pCfg, statickey.New()), assetpath.ProvideService(pluginscdn.ProvideService(pCfg))) l := loader.ProvideService(pCfg, lic, signature.NewUnsignedAuthorizer(pCfg), reg, provider.ProvideService(coreRegistry), fakes.NewFakeRoleRegistry(), diff --git a/pkg/plugins/manager/pipeline/bootstrap/bootstrap.go b/pkg/plugins/manager/pipeline/bootstrap/bootstrap.go index e4b8f2a45b8..67708a8a513 100644 --- a/pkg/plugins/manager/pipeline/bootstrap/bootstrap.go +++ b/pkg/plugins/manager/pipeline/bootstrap/bootstrap.go @@ -44,7 +44,7 @@ type Opts struct { // New returns a new Bootstrap stage. func New(cfg *config.Cfg, opts Opts) *Bootstrap { if opts.ConstructFunc == nil { - opts.ConstructFunc = DefaultConstructFunc(signature.DefaultCalculator(), assetpath.DefaultService(cfg)) + opts.ConstructFunc = DefaultConstructFunc(signature.DefaultCalculator(cfg), assetpath.DefaultService(cfg)) } if len(opts.DecorateFuncs) == 0 { diff --git a/pkg/plugins/manager/signature/manifest.go b/pkg/plugins/manager/signature/manifest.go index 93829a76809..86b1c7b4832 100644 --- a/pkg/plugins/manager/signature/manifest.go +++ b/pkg/plugins/manager/signature/manifest.go @@ -22,9 +22,9 @@ import ( "github.com/gobwas/glob" "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/log" "github.com/grafana/grafana/pkg/plugins/manager/signature/statickey" - "github.com/grafana/grafana/pkg/setting" ) var ( @@ -58,26 +58,29 @@ func (m *PluginManifest) isV2() bool { } type Signature struct { - log log.Logger kr plugins.KeyRetriever + cfg *config.Cfg + log log.Logger } var _ plugins.SignatureCalculator = &Signature{} -func ProvideService(kr plugins.KeyRetriever) *Signature { - return NewCalculator(kr) +func ProvideService(cfg *config.Cfg, kr plugins.KeyRetriever) *Signature { + return NewCalculator(cfg, kr) } -func NewCalculator(kr plugins.KeyRetriever) *Signature { +func NewCalculator(cfg *config.Cfg, kr plugins.KeyRetriever) *Signature { return &Signature{ kr: kr, + cfg: cfg, log: log.New("plugins.signature"), } } -func DefaultCalculator() *Signature { +func DefaultCalculator(cfg *config.Cfg) *Signature { return &Signature{ kr: statickey.New(), + cfg: cfg, log: log.New("plugins.signature"), } } @@ -173,12 +176,12 @@ func (s *Signature) Calculate(ctx context.Context, src plugins.PluginSource, plu // Validate that plugin is running within defined root URLs if len(manifest.RootURLs) > 0 { - if match, err := urlMatch(manifest.RootURLs, setting.AppUrl, manifest.SignatureType); err != nil { + if match, err := urlMatch(manifest.RootURLs, s.cfg.GrafanaAppURL, manifest.SignatureType); err != nil { s.log.Warn("Could not verify if root URLs match", "plugin", plugin.JSONData.ID, "rootUrls", manifest.RootURLs) return plugins.Signature{}, err } else if !match { s.log.Warn("Could not find root URL that matches running application URL", "plugin", plugin.JSONData.ID, - "appUrl", setting.AppUrl, "rootUrls", manifest.RootURLs) + "appUrl", s.cfg.GrafanaAppURL, "rootUrls", manifest.RootURLs) return plugins.Signature{ Status: plugins.SignatureStatusInvalid, }, nil diff --git a/pkg/plugins/manager/signature/manifest_test.go b/pkg/plugins/manager/signature/manifest_test.go index c37cc6390d5..4e74e70b280 100644 --- a/pkg/plugins/manager/signature/manifest_test.go +++ b/pkg/plugins/manager/signature/manifest_test.go @@ -14,9 +14,9 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/manager/fakes" "github.com/grafana/grafana/pkg/plugins/manager/signature/statickey" - "github.com/grafana/grafana/pkg/setting" ) func TestReadPluginManifest(t *testing.T) { @@ -52,7 +52,7 @@ NR7DnB0CCQHO+4FlSPtXFTzNepoc+CytQyDAeOLMLmf2Tqhk2YShk+G/YlVX -----END PGP SIGNATURE-----` t.Run("valid manifest", func(t *testing.T) { - s := ProvideService(statickey.New()) + s := ProvideService(&config.Cfg{}, statickey.New()) manifest, err := s.readPluginManifest(context.Background(), []byte(txt)) require.NoError(t, err) @@ -69,7 +69,7 @@ NR7DnB0CCQHO+4FlSPtXFTzNepoc+CytQyDAeOLMLmf2Tqhk2YShk+G/YlVX t.Run("invalid manifest", func(t *testing.T) { modified := strings.ReplaceAll(txt, "README.md", "xxxxxxxxxx") - s := ProvideService(statickey.New()) + s := ProvideService(&config.Cfg{}, statickey.New()) _, err := s.readPluginManifest(context.Background(), []byte(modified)) require.Error(t, err) }) @@ -107,7 +107,7 @@ khdr/tZ1PDgRxMqB/u+Vtbpl0xSxgblnrDOYMSI= -----END PGP SIGNATURE-----` t.Run("valid manifest", func(t *testing.T) { - s := ProvideService(statickey.New()) + s := ProvideService(&config.Cfg{}, statickey.New()) manifest, err := s.readPluginManifest(context.Background(), []byte(txt)) require.NoError(t, err) @@ -154,14 +154,8 @@ func TestCalculate(t *testing.T) { } for _, tc := range tcs { - origAppURL := setting.AppUrl - t.Cleanup(func() { - setting.AppUrl = origAppURL - }) - setting.AppUrl = tc.appURL - basePath := filepath.Join(parentDir, "testdata/non-pvt-with-root-url/plugin") - s := ProvideService(statickey.New()) + s := ProvideService(&config.Cfg{GrafanaAppURL: tc.appURL}, statickey.New()) sig, err := s.Calculate(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal @@ -189,7 +183,7 @@ func TestCalculate(t *testing.T) { basePath := "../testdata/renderer-added-file/plugin" runningWindows = true - s := ProvideService(statickey.New()) + s := ProvideService(&config.Cfg{}, statickey.New()) sig, err := s.Calculate(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal @@ -253,7 +247,7 @@ func TestCalculate(t *testing.T) { toSlash = tc.platform.toSlashFunc() fromSlash = tc.platform.fromSlashFunc() - s := ProvideService(statickey.New()) + s := ProvideService(&config.Cfg{}, statickey.New()) pfs, err := tc.fsFactory() require.NoError(t, err) pfs, err = newPathSeparatorOverrideFS(string(tc.platform.separator), pfs) @@ -721,7 +715,7 @@ func Test_validateManifest(t *testing.T) { } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - s := ProvideService(statickey.New()) + s := ProvideService(&config.Cfg{}, statickey.New()) err := s.validateManifest(context.Background(), *tc.manifest, nil) require.Errorf(t, err, tc.expectedErr) }) @@ -817,7 +811,7 @@ pHo= } func Test_VerifyRevokedKey(t *testing.T) { - s := ProvideService(&revokedKeyProvider{}) + s := ProvideService(&config.Cfg{}, &revokedKeyProvider{}) m := createV2Manifest(t) txt := `-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512