diff --git a/pkg/registry/apis/provisioning/controller/mutator.go b/pkg/registry/apis/provisioning/controller/mutator.go deleted file mode 100644 index 549c2ba763e..00000000000 --- a/pkg/registry/apis/provisioning/controller/mutator.go +++ /dev/null @@ -1,9 +0,0 @@ -package controller - -import ( - "context" - - "k8s.io/apimachinery/pkg/runtime" -) - -type Mutator func(ctx context.Context, obj runtime.Object) error diff --git a/pkg/registry/apis/provisioning/controller/repository.go b/pkg/registry/apis/provisioning/controller/repository.go index aa9f2399e5f..2e29950e3da 100644 --- a/pkg/registry/apis/provisioning/controller/repository.go +++ b/pkg/registry/apis/provisioning/controller/repository.go @@ -27,13 +27,6 @@ import ( "github.com/grafana/grafana/pkg/storage/legacysql/dualwrite" ) -type RepoGetter interface { - // Given a repository configuration, return it as a repository instance - // This will only error for un-recoverable system errors - // the repository instance may or may not be valid/healthy - RepositoryFromConfig(ctx context.Context, r *provisioning.Repository) (repository.Repository, error) -} - const loggerName = "provisioning-repository-controller" const ( @@ -60,8 +53,7 @@ type RepositoryController struct { finalizer *finalizer statusPatcher StatusPatcher - // Converts config to instance - repoGetter RepoGetter + repoFactory repository.Factory healthChecker *HealthChecker // To allow injection for testing. processFn func(item *queueItem) error @@ -75,7 +67,7 @@ type RepositoryController struct { func NewRepositoryController( provisioningClient client.ProvisioningV0alpha1Interface, repoInformer informer.RepositoryInformer, - repoGetter RepoGetter, + repoFactory repository.Factory, resourceLister resources.ResourceLister, parsers resources.ParserFactory, clients resources.ClientFactory, @@ -96,7 +88,7 @@ func NewRepositoryController( Name: "provisioningRepositoryController", }, ), - repoGetter: repoGetter, + repoFactory: repoFactory, healthChecker: healthChecker, statusPatcher: statusPatcher, parsers: parsers, @@ -223,7 +215,7 @@ func (rc *RepositoryController) handleDelete(ctx context.Context, obj *provision // Process any finalizers if len(obj.Finalizers) > 0 { - repo, err := rc.repoGetter.RepositoryFromConfig(ctx, obj) + repo, err := rc.repoFactory.Build(ctx, obj) if err != nil { logger.Warn("unable to get repository for cleanup") } else { @@ -438,7 +430,7 @@ func (rc *RepositoryController) process(item *queueItem) error { return nil } - repo, err := rc.repoGetter.RepositoryFromConfig(ctx, obj) + repo, err := rc.repoFactory.Build(ctx, obj) if err != nil { return fmt.Errorf("unable to create repository from configuration: %w", err) } diff --git a/pkg/registry/apis/provisioning/extra.go b/pkg/registry/apis/provisioning/extra.go index b1ab565d1e3..af642a7216c 100644 --- a/pkg/registry/apis/provisioning/extra.go +++ b/pkg/registry/apis/provisioning/extra.go @@ -3,14 +3,10 @@ package provisioning import ( "context" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/kube-openapi/pkg/spec3" - - provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/controller" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" ) type Extra interface { @@ -18,9 +14,6 @@ type Extra interface { UpdateStorage(storage map[string]rest.Storage) error PostProcessOpenAPI(oas *spec3.OpenAPI) error GetJobWorkers() []jobs.Worker - AsRepository(ctx context.Context, r *provisioning.Repository, secure repository.SecureValues) (repository.Repository, error) - RepositoryTypes() []provisioning.RepositoryType - Mutators() []controller.Mutator } type ExtraBuilder func(b *APIBuilder) Extra diff --git a/pkg/registry/apis/provisioning/extras/register.go b/pkg/registry/apis/provisioning/extras/register.go index 30078594ed5..30aaaae9049 100644 --- a/pkg/registry/apis/provisioning/extras/register.go +++ b/pkg/registry/apis/provisioning/extras/register.go @@ -2,12 +2,33 @@ package extras import ( "github.com/grafana/grafana/pkg/registry/apis/provisioning" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/local" "github.com/grafana/grafana/pkg/registry/apis/provisioning/webhooks" + "github.com/grafana/grafana/pkg/registry/apis/secret" + "github.com/grafana/grafana/pkg/setting" ) // HACK: This is a hack so that wire can uniquely identify dependencies -func ProvideProvisioningOSSExtras(webhook webhooks.WebhookExtraBuilder) []provisioning.ExtraBuilder { +func ProvideProvisioningOSSExtras(webhook *webhooks.WebhookExtraBuilder) []provisioning.ExtraBuilder { return []provisioning.ExtraBuilder{ webhook.ExtraBuilder, } } + +func ProvideProvisioningOSSRepositoryExtras( + cfg *setting.Cfg, + decryptSvc secret.DecryptService, + ghFactory *github.Factory, + webhooksBuilder *webhooks.WebhookExtraBuilder, +) []repository.Extra { + return []repository.Extra{ + local.Extra(cfg), + github.Extra( + repository.DecryptService(decryptSvc), + ghFactory, + webhooksBuilder, + ), + } +} diff --git a/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go b/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go index e7f8a7f3dad..1da6d158d8d 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go @@ -87,7 +87,7 @@ func TestMigrationWorker_WithHistory(t *testing.T) { progressRecorder := jobs.NewMockJobProgressRecorder(t) progressRecorder.On("SetTotal", mock.Anything, 10).Return() - repo := local.NewLocal(&provisioning.Repository{}, nil) + repo := local.NewRepository(&provisioning.Repository{}, nil) err := worker.Process(context.Background(), repo, job, progressRecorder) require.EqualError(t, err, "history is only supported for github repositories") }) diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 0f264a66d32..061cdf3e5c6 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -2,7 +2,6 @@ package provisioning import ( "context" - "errors" "fmt" "net/http" "net/url" @@ -51,14 +50,9 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs/sync" "github.com/grafana/grafana/pkg/registry/apis/provisioning/loki" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/git" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/local" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources/signature" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/safepath" "github.com/grafana/grafana/pkg/registry/apis/provisioning/usage" - "github.com/grafana/grafana/pkg/registry/apis/secret" "github.com/grafana/grafana/pkg/services/apiserver" "github.com/grafana/grafana/pkg/services/apiserver/builder" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -84,16 +78,19 @@ type JobHistoryConfig struct { } type APIBuilder struct { + // onlyApiServer used to disable starting controllers for the standalone API server. + // HACK:This will be removed once we have proper wire providers for the controllers. + // TODO: Set this up in the standalone API server + onlyApiServer bool + features featuremgmt.FeatureToggles usageStats usagestats.Service tracer tracing.Tracer getter rest.Getter - localFileResolver *local.LocalFolderResolver parsers resources.ParserFactory repositoryResources resources.RepositoryResourcesFactory clients resources.ClientFactory - ghFactory *github.Factory jobs interface { jobs.Queue jobs.Store @@ -105,30 +102,27 @@ type APIBuilder struct { legacyMigrator legacy.LegacyMigrator storageStatus dualwrite.Service unified resource.ResourceClient - decrypter repository.Decrypter + repoFactory repository.Factory client client.ProvisioningV0alpha1Interface access authlib.AccessChecker - mutators []controller.Mutator statusPatcher *controller.RepositoryStatusPatcher healthChecker *controller.HealthChecker // Extras provides additional functionality to the API. - extras []Extra - availableRepositoryTypes map[provisioning.RepositoryType]bool + extras []Extra } // NewAPIBuilder creates an API builder. // It avoids anything that is core to Grafana, such that it can be used in a multi-tenant service down the line. // This means there are no hidden dependencies, and no use of e.g. *settings.Cfg. func NewAPIBuilder( - local *local.LocalFolderResolver, + onlyApiServer bool, + repoFactory repository.Factory, features featuremgmt.FeatureToggles, unified resource.ResourceClient, configProvider apiserver.RestConfigProvider, - ghFactory *github.Factory, legacyMigrator legacy.LegacyMigrator, storageStatus dualwrite.Service, usageStats usagestats.Service, - decryptSvc secret.DecryptService, access authlib.AccessChecker, tracer tracing.Tracer, extraBuilders []ExtraBuilder, @@ -138,18 +132,12 @@ func NewAPIBuilder( parsers := resources.NewParserFactory(clients) resourceLister := resources.NewResourceLister(unified, unified, legacyMigrator, storageStatus) - mutators := []controller.Mutator{ - git.Mutator(), - github.Mutator(), - } - b := &APIBuilder{ - mutators: mutators, + onlyApiServer: onlyApiServer, tracer: tracer, usageStats: usageStats, - localFileResolver: local, features: features, - ghFactory: ghFactory, + repoFactory: repoFactory, clients: clients, parsers: parsers, repositoryResources: resources.NewRepositoryResourcesFactory(parsers, clients, resourceLister), @@ -157,28 +145,14 @@ func NewAPIBuilder( legacyMigrator: legacyMigrator, storageStatus: storageStatus, unified: unified, - decrypter: repository.DecryptService(decryptSvc), access: access, jobHistoryConfig: jobHistoryConfig, - availableRepositoryTypes: map[provisioning.RepositoryType]bool{ - provisioning.LocalRepositoryType: true, - provisioning.GitHubRepositoryType: true, - }, } for _, builder := range extraBuilders { b.extras = append(b.extras, builder(b)) } - // Add the available repository types and mutators from the extras - for _, extra := range b.extras { - for _, t := range extra.RepositoryTypes() { - b.availableRepositoryTypes[t] = true - } - - b.mutators = append(b.mutators, extra.Mutators()...) - } - return b } @@ -224,29 +198,26 @@ func RegisterAPIService( reg prometheus.Registerer, client resource.ResourceClient, // implements resource.RepositoryClient configProvider apiserver.RestConfigProvider, - ghFactory *github.Factory, access authlib.AccessClient, legacyMigrator legacy.LegacyMigrator, storageStatus dualwrite.Service, usageStats usagestats.Service, - decryptSvc secret.DecryptService, tracer tracing.Tracer, extraBuilders []ExtraBuilder, + repoFactory repository.Factory, ) (*APIBuilder, error) { if !features.IsEnabledGlobally(featuremgmt.FlagProvisioning) { return nil, nil } - folderResolver := &local.LocalFolderResolver{ - PermittedPrefixes: cfg.PermittedProvisioningPaths, - HomePath: safepath.Clean(cfg.HomePath), - } - builder := NewAPIBuilder(folderResolver, features, + builder := NewAPIBuilder( + false, // Run controllers + repoFactory, + features, client, - configProvider, ghFactory, + configProvider, legacyMigrator, storageStatus, usageStats, - decryptSvc, access, tracer, extraBuilders, @@ -459,7 +430,7 @@ func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupI storage[provisioning.RepositoryResourceInfo.StoragePath("status")] = repositoryStatusStorage // TODO: Add some logic so that the connectors can registered themselves and we don't have logic all over the place - storage[provisioning.RepositoryResourceInfo.StoragePath("test")] = NewTestConnector(b, &repository.Tester{}, b) + storage[provisioning.RepositoryResourceInfo.StoragePath("test")] = NewTestConnector(b, b.repoFactory, &repository.Tester{}, b) storage[provisioning.RepositoryResourceInfo.StoragePath("files")] = NewFilesConnector(b, b.parsers, b.clients, b.access) storage[provisioning.RepositoryResourceInfo.StoragePath("refs")] = NewRefsConnector(b) storage[provisioning.RepositoryResourceInfo.StoragePath("resources")] = &listConnector{ @@ -522,11 +493,9 @@ func (b *APIBuilder) Mutate(ctx context.Context, a admission.Attributes, o admis r.Spec.Workflows = []provisioning.Workflow{} } - // Mutate the repository with any extra mutators - for _, mutator := range b.mutators { - if err := mutator(ctx, r); err != nil { - return fmt.Errorf("failed to mutate repository: %w", err) - } + // Extra mutators + if err := b.repoFactory.Mutate(ctx, r); err != nil { + return fmt.Errorf("failed to mutate repository: %w", err) } return nil @@ -671,7 +640,7 @@ func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartH b.healthChecker = controller.NewHealthChecker(&repository.Tester{}, b.statusPatcher) // if running solely CRUD, skip the rest of the setup - if b.localFileResolver == nil { + if b.onlyApiServer { return nil } @@ -786,7 +755,7 @@ func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartH repoController, err := controller.NewRepositoryController( b.GetClient(), repoInformer, - b, // repoGetter + b.repoFactory, b.resourceLister, b.parsers, b.clients, @@ -1288,6 +1257,7 @@ func (b *APIBuilder) asRepository(ctx context.Context, obj runtime.Object, old r if obj == nil { return nil, fmt.Errorf("missing repository object") } + r, ok := obj.(*provisioning.Repository) if !ok { return nil, fmt.Errorf("expected repository configuration") @@ -1306,77 +1276,7 @@ func (b *APIBuilder) asRepository(ctx context.Context, obj runtime.Object, old r } } - return b.RepositoryFromConfig(ctx, r) -} - -func (b *APIBuilder) RepositoryFromConfig(ctx context.Context, r *provisioning.Repository) (repository.Repository, error) { - // Prepare a decrypter - secure := b.decrypter(r) - - // Try first with any extra - for _, extra := range b.extras { - r, err := extra.AsRepository(ctx, r, secure) - if err != nil { - return nil, fmt.Errorf("convert repository for extra %T: %w", extra, err) - } - - if r != nil { - return r, nil - } - } - - token, err := secure.Token(ctx) - if err != nil { - return nil, fmt.Errorf("unable to decrypt token: %w", err) - } - - switch r.Spec.Type { - case provisioning.BitbucketRepositoryType: - return nil, errors.New("repository type bitbucket is not available") - case provisioning.GitLabRepositoryType: - return nil, errors.New("repository type gitlab is not available") - case provisioning.LocalRepositoryType: - return local.NewLocal(r, b.localFileResolver), nil - case provisioning.GitRepositoryType: - cfg := git.RepositoryConfig{ - URL: r.Spec.Git.URL, - Branch: r.Spec.Git.Branch, - Path: r.Spec.Git.Path, - TokenUser: r.Spec.Git.TokenUser, - Token: token, - } - - return git.NewGitRepository(ctx, r, cfg) - case provisioning.GitHubRepositoryType: - logger := logging.FromContext(ctx).With("url", r.Spec.GitHub.URL, "branch", r.Spec.GitHub.Branch, "path", r.Spec.GitHub.Path) - logger.Info("Instantiating Github repository") - - ghCfg := r.Spec.GitHub - if ghCfg == nil { - return nil, fmt.Errorf("github configuration is required for nano git") - } - - gitCfg := git.RepositoryConfig{ - URL: ghCfg.URL, - Branch: ghCfg.Branch, - Path: ghCfg.Path, - Token: token, - } - - gitRepo, err := git.NewGitRepository(ctx, r, gitCfg) - if err != nil { - return nil, fmt.Errorf("error creating git repository: %w", err) - } - - ghRepo, err := github.NewGitHub(ctx, r, gitRepo, b.ghFactory, token) - if err != nil { - return nil, fmt.Errorf("error creating github repository: %w", err) - } - - return ghRepo, nil - default: - return nil, fmt.Errorf("unknown repository type (%s)", r.Spec.Type) - } + return b.repoFactory.Build(ctx, r) } func getJSONResponse(ref string) *spec3.Responses { diff --git a/pkg/registry/apis/provisioning/repository/extra_mock.go b/pkg/registry/apis/provisioning/repository/extra_mock.go new file mode 100644 index 00000000000..afba0c49860 --- /dev/null +++ b/pkg/registry/apis/provisioning/repository/extra_mock.go @@ -0,0 +1,190 @@ +// Code generated by mockery v2.53.4. DO NOT EDIT. + +package repository + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" + runtime "k8s.io/apimachinery/pkg/runtime" + + v0alpha1 "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" +) + +// MockExtra is an autogenerated mock type for the Extra type +type MockExtra struct { + mock.Mock +} + +type MockExtra_Expecter struct { + mock *mock.Mock +} + +func (_m *MockExtra) EXPECT() *MockExtra_Expecter { + return &MockExtra_Expecter{mock: &_m.Mock} +} + +// Build provides a mock function with given fields: ctx, r +func (_m *MockExtra) Build(ctx context.Context, r *v0alpha1.Repository) (Repository, error) { + ret := _m.Called(ctx, r) + + if len(ret) == 0 { + panic("no return value specified for Build") + } + + var r0 Repository + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *v0alpha1.Repository) (Repository, error)); ok { + return rf(ctx, r) + } + if rf, ok := ret.Get(0).(func(context.Context, *v0alpha1.Repository) Repository); ok { + r0 = rf(ctx, r) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(Repository) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *v0alpha1.Repository) error); ok { + r1 = rf(ctx, r) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockExtra_Build_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Build' +type MockExtra_Build_Call struct { + *mock.Call +} + +// Build is a helper method to define mock.On call +// - ctx context.Context +// - r *v0alpha1.Repository +func (_e *MockExtra_Expecter) Build(ctx interface{}, r interface{}) *MockExtra_Build_Call { + return &MockExtra_Build_Call{Call: _e.mock.On("Build", ctx, r)} +} + +func (_c *MockExtra_Build_Call) Run(run func(ctx context.Context, r *v0alpha1.Repository)) *MockExtra_Build_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*v0alpha1.Repository)) + }) + return _c +} + +func (_c *MockExtra_Build_Call) Return(_a0 Repository, _a1 error) *MockExtra_Build_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockExtra_Build_Call) RunAndReturn(run func(context.Context, *v0alpha1.Repository) (Repository, error)) *MockExtra_Build_Call { + _c.Call.Return(run) + return _c +} + +// Mutate provides a mock function with given fields: ctx, obj +func (_m *MockExtra) Mutate(ctx context.Context, obj runtime.Object) error { + ret := _m.Called(ctx, obj) + + if len(ret) == 0 { + panic("no return value specified for Mutate") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, runtime.Object) error); ok { + r0 = rf(ctx, obj) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MockExtra_Mutate_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Mutate' +type MockExtra_Mutate_Call struct { + *mock.Call +} + +// Mutate is a helper method to define mock.On call +// - ctx context.Context +// - obj runtime.Object +func (_e *MockExtra_Expecter) Mutate(ctx interface{}, obj interface{}) *MockExtra_Mutate_Call { + return &MockExtra_Mutate_Call{Call: _e.mock.On("Mutate", ctx, obj)} +} + +func (_c *MockExtra_Mutate_Call) Run(run func(ctx context.Context, obj runtime.Object)) *MockExtra_Mutate_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(runtime.Object)) + }) + return _c +} + +func (_c *MockExtra_Mutate_Call) Return(_a0 error) *MockExtra_Mutate_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockExtra_Mutate_Call) RunAndReturn(run func(context.Context, runtime.Object) error) *MockExtra_Mutate_Call { + _c.Call.Return(run) + return _c +} + +// Type provides a mock function with no fields +func (_m *MockExtra) Type() v0alpha1.RepositoryType { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for Type") + } + + var r0 v0alpha1.RepositoryType + if rf, ok := ret.Get(0).(func() v0alpha1.RepositoryType); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(v0alpha1.RepositoryType) + } + + return r0 +} + +// MockExtra_Type_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Type' +type MockExtra_Type_Call struct { + *mock.Call +} + +// Type is a helper method to define mock.On call +func (_e *MockExtra_Expecter) Type() *MockExtra_Type_Call { + return &MockExtra_Type_Call{Call: _e.mock.On("Type")} +} + +func (_c *MockExtra_Type_Call) Run(run func()) *MockExtra_Type_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *MockExtra_Type_Call) Return(_a0 v0alpha1.RepositoryType) *MockExtra_Type_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockExtra_Type_Call) RunAndReturn(run func() v0alpha1.RepositoryType) *MockExtra_Type_Call { + _c.Call.Return(run) + return _c +} + +// NewMockExtra creates a new instance of MockExtra. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMockExtra(t interface { + mock.TestingT + Cleanup(func()) +}) *MockExtra { + mock := &MockExtra{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/registry/apis/provisioning/repository/factory.go b/pkg/registry/apis/provisioning/repository/factory.go new file mode 100644 index 00000000000..3e9570d8548 --- /dev/null +++ b/pkg/registry/apis/provisioning/repository/factory.go @@ -0,0 +1,75 @@ +package repository + +import ( + "context" + "fmt" + "maps" + "slices" + "sort" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "k8s.io/apimachinery/pkg/runtime" +) + +type Mutator func(ctx context.Context, obj runtime.Object) error + +//go:generate mockery --name=Extra --structname=MockExtra --inpackage --filename=extra_mock.go --with-expecter +type Extra interface { + Type() provisioning.RepositoryType + Build(ctx context.Context, r *provisioning.Repository) (Repository, error) + Mutate(ctx context.Context, obj runtime.Object) error +} + +//go:generate mockery --name=Factor --structname=MockFactory --inpackage --filename=factory_mock.go --with-expecter +type Factory interface { + Types() []provisioning.RepositoryType + Build(ctx context.Context, r *provisioning.Repository) (Repository, error) + Mutate(ctx context.Context, obj runtime.Object) error +} + +type factory struct { + extras map[provisioning.RepositoryType]Extra +} + +func ProvideFactory(extras []Extra) (Factory, error) { + f := &factory{ + extras: make(map[provisioning.RepositoryType]Extra, len(extras)), + } + + for _, e := range extras { + if _, exists := f.extras[e.Type()]; exists { + return nil, fmt.Errorf("repository type %q is already registered", e.Type()) + } + f.extras[e.Type()] = e + } + + return f, nil +} + +func (f *factory) Types() []provisioning.RepositoryType { + types := slices.Collect(maps.Keys(f.extras)) + sort.Slice(types, func(i, j int) bool { + return string(types[i]) < string(types[j]) + }) + return types +} + +func (f *factory) Build(ctx context.Context, r *provisioning.Repository) (Repository, error) { + for _, e := range f.extras { + if e.Type() == r.Spec.Type { + return e.Build(ctx, r) + } + } + + return nil, fmt.Errorf("repository type %q is not supported", r.Spec.Type) +} + +func (f *factory) Mutate(ctx context.Context, obj runtime.Object) error { + for _, e := range f.extras { + if err := e.Mutate(ctx, obj); err != nil { + return err + } + } + + return nil +} diff --git a/pkg/registry/apis/provisioning/repository/factory_test.go b/pkg/registry/apis/provisioning/repository/factory_test.go new file mode 100644 index 00000000000..b199f02526d --- /dev/null +++ b/pkg/registry/apis/provisioning/repository/factory_test.go @@ -0,0 +1,343 @@ +package repository + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" +) + +func TestNewFactory(t *testing.T) { + t.Run("creates factory with empty extras", func(t *testing.T) { + factory, err := ProvideFactory([]Extra{}) + + require.NoError(t, err) + require.NotNil(t, factory) + types := factory.Types() + assert.Empty(t, types) + }) + + t.Run("creates factory with multiple extras", func(t *testing.T) { + localExtra := &MockExtra{} + localExtra.On("Type").Return(provisioning.LocalRepositoryType) + + gitExtra := &MockExtra{} + gitExtra.On("Type").Return(provisioning.GitRepositoryType) + + githubExtra := &MockExtra{} + githubExtra.On("Type").Return(provisioning.GitHubRepositoryType) + + extras := []Extra{localExtra, gitExtra, githubExtra} + factory, err := ProvideFactory(extras) + + require.NoError(t, err) + require.NotNil(t, factory) + types := factory.Types() + assert.Len(t, types, 3) + + // Verify stable ordering - types should be sorted alphabetically + expectedTypes := []provisioning.RepositoryType{ + provisioning.GitRepositoryType, + provisioning.GitHubRepositoryType, + provisioning.LocalRepositoryType, + } + assert.Equal(t, expectedTypes, types) + + localExtra.AssertExpectations(t) + gitExtra.AssertExpectations(t) + githubExtra.AssertExpectations(t) + }) + + t.Run("returns error for duplicate repository types", func(t *testing.T) { + firstExtra := &MockExtra{} + firstExtra.On("Type").Return(provisioning.LocalRepositoryType) + + secondExtra := &MockExtra{} + secondExtra.On("Type").Return(provisioning.LocalRepositoryType) + + extras := []Extra{firstExtra, secondExtra} + factory, err := ProvideFactory(extras) + + assert.Error(t, err) + assert.Nil(t, factory) + assert.Contains(t, err.Error(), "repository type \"local\" is already registered") + + firstExtra.AssertExpectations(t) + secondExtra.AssertExpectations(t) + }) + + t.Run("returns error for duplicate among multiple different types", func(t *testing.T) { + localExtra := &MockExtra{} + localExtra.On("Type").Return(provisioning.LocalRepositoryType) + + gitExtra := &MockExtra{} + gitExtra.On("Type").Return(provisioning.GitRepositoryType) + + duplicateGitExtra := &MockExtra{} + duplicateGitExtra.On("Type").Return(provisioning.GitRepositoryType) + + extras := []Extra{localExtra, gitExtra, duplicateGitExtra} + factory, err := ProvideFactory(extras) + + assert.Error(t, err) + assert.Nil(t, factory) + assert.Contains(t, err.Error(), "repository type \"git\" is already registered") + + localExtra.AssertExpectations(t) + gitExtra.AssertExpectations(t) + duplicateGitExtra.AssertExpectations(t) + }) + + t.Run("handles nil extras slice", func(t *testing.T) { + factory, err := ProvideFactory(nil) + + require.NoError(t, err) + require.NotNil(t, factory) + types := factory.Types() + assert.Empty(t, types) + }) +} + +func TestFactory_Types(t *testing.T) { + t.Run("returns empty slice for factory with no extras", func(t *testing.T) { + factory, err := ProvideFactory([]Extra{}) + + require.NoError(t, err) + types := factory.Types() + assert.Empty(t, types) + }) + + t.Run("returns all registered repository types in stable order", func(t *testing.T) { + localExtra := &MockExtra{} + localExtra.On("Type").Return(provisioning.LocalRepositoryType) + + gitExtra := &MockExtra{} + gitExtra.On("Type").Return(provisioning.GitRepositoryType) + + githubExtra := &MockExtra{} + githubExtra.On("Type").Return(provisioning.GitHubRepositoryType) + + bitbucketExtra := &MockExtra{} + bitbucketExtra.On("Type").Return(provisioning.BitbucketRepositoryType) + + gitlabExtra := &MockExtra{} + gitlabExtra.On("Type").Return(provisioning.GitLabRepositoryType) + + extras := []Extra{localExtra, gitExtra, githubExtra, bitbucketExtra, gitlabExtra} + factory, err := ProvideFactory(extras) + + require.NoError(t, err) + types := factory.Types() + + assert.Len(t, types, 5) + + // Verify stable ordering - types should be sorted alphabetically + expectedTypes := []provisioning.RepositoryType{ + provisioning.BitbucketRepositoryType, + provisioning.GitRepositoryType, + provisioning.GitHubRepositoryType, + provisioning.GitLabRepositoryType, + provisioning.LocalRepositoryType, + } + assert.Equal(t, expectedTypes, types) + + localExtra.AssertExpectations(t) + gitExtra.AssertExpectations(t) + githubExtra.AssertExpectations(t) + bitbucketExtra.AssertExpectations(t) + gitlabExtra.AssertExpectations(t) + }) + + t.Run("returns consistent order across multiple calls", func(t *testing.T) { + localExtra := &MockExtra{} + localExtra.On("Type").Return(provisioning.LocalRepositoryType) + + gitExtra := &MockExtra{} + gitExtra.On("Type").Return(provisioning.GitRepositoryType) + + githubExtra := &MockExtra{} + githubExtra.On("Type").Return(provisioning.GitHubRepositoryType) + + extras := []Extra{githubExtra, localExtra, gitExtra} // Intentionally unordered + factory, err := ProvideFactory(extras) + + require.NoError(t, err) + types1 := factory.Types() + types2 := factory.Types() + types3 := factory.Types() + + // All calls should return the same order + assert.Equal(t, types1, types2) + assert.Equal(t, types2, types3) + + // Verify the order is alphabetical + expectedTypes := []provisioning.RepositoryType{ + provisioning.GitRepositoryType, + provisioning.GitHubRepositoryType, + provisioning.LocalRepositoryType, + } + assert.Equal(t, expectedTypes, types1) + + localExtra.AssertExpectations(t) + gitExtra.AssertExpectations(t) + githubExtra.AssertExpectations(t) + }) +} + +func TestFactory_Build(t *testing.T) { + t.Run("successfully builds repository with matching type", func(t *testing.T) { + expectedRepo := &MockConfigRepository{} + localExtra := &MockExtra{} + localExtra.On("Type").Return(provisioning.LocalRepositoryType) + localExtra.On("Build", mock.Anything, mock.Anything).Return(expectedRepo, nil) + + factory, err := ProvideFactory([]Extra{localExtra}) + require.NoError(t, err) + + ctx := context.Background() + repoConfig := &provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Type: provisioning.LocalRepositoryType, + }, + } + + result, err := factory.Build(ctx, repoConfig) + + require.NoError(t, err) + assert.Equal(t, expectedRepo, result) + localExtra.AssertExpectations(t) + }) + + t.Run("returns error for unsupported repository type", func(t *testing.T) { + gitExtra := &MockExtra{} + gitExtra.On("Type").Return(provisioning.GitRepositoryType) + + factory, err := ProvideFactory([]Extra{gitExtra}) + require.NoError(t, err) + + ctx := context.Background() + repoConfig := &provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Type: provisioning.LocalRepositoryType, // Different from registered type + }, + } + + result, err := factory.Build(ctx, repoConfig) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "repository type \"local\" is not supported") + gitExtra.AssertNotCalled(t, "Build") + gitExtra.AssertExpectations(t) + }) + + t.Run("propagates error from extra.Build", func(t *testing.T) { + expectedError := errors.New("build failed") + localExtra := &MockExtra{} + localExtra.On("Type").Return(provisioning.LocalRepositoryType) + localExtra.On("Build", mock.Anything, mock.Anything).Return(nil, expectedError) + + factory, err := ProvideFactory([]Extra{localExtra}) + require.NoError(t, err) + + ctx := context.Background() + repoConfig := &provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Type: provisioning.LocalRepositoryType, + }, + } + + result, err := factory.Build(ctx, repoConfig) + + assert.Error(t, err) + assert.Equal(t, expectedError, err) + assert.Nil(t, result) + localExtra.AssertExpectations(t) + }) + + t.Run("finds correct extra among multiple", func(t *testing.T) { + gitRepo := &MockConfigRepository{} + + localExtra := &MockExtra{} + localExtra.On("Type").Return(provisioning.LocalRepositoryType) + + gitExtra := &MockExtra{} + gitExtra.On("Type").Return(provisioning.GitRepositoryType) + gitExtra.On("Build", mock.Anything, mock.Anything).Return(gitRepo, nil) + + factory, err := ProvideFactory([]Extra{localExtra, gitExtra}) + require.NoError(t, err) + + ctx := context.Background() + repoConfig := &provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitRepositoryType, + }, + } + + result, err := factory.Build(ctx, repoConfig) + + require.NoError(t, err) + assert.Equal(t, gitRepo, result) + localExtra.AssertNotCalled(t, "Build") // Should not be called + gitExtra.AssertExpectations(t) // Should be called + localExtra.AssertExpectations(t) + }) + + t.Run("handles empty repository type", func(t *testing.T) { + localExtra := &MockExtra{} + localExtra.On("Type").Return(provisioning.LocalRepositoryType) + + factory, err := ProvideFactory([]Extra{localExtra}) + require.NoError(t, err) + + ctx := context.Background() + repoConfig := &provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Type: "", // Empty type + }, + } + + result, err := factory.Build(ctx, repoConfig) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "repository type \"\" is not supported") + localExtra.AssertNotCalled(t, "Build") + localExtra.AssertExpectations(t) + }) + + t.Run("passes context correctly to extra.Build", func(t *testing.T) { + localRepo := &MockConfigRepository{} + localExtra := &MockExtra{} + localExtra.On("Type").Return(provisioning.LocalRepositoryType) + + // Create context with value to verify it's passed through + type testKey string + ctx := context.WithValue(context.Background(), testKey("test"), "value") + + // Use a custom matcher to verify the context is passed correctly + localExtra.On("Build", mock.MatchedBy(func(c context.Context) bool { + return c.Value(testKey("test")) == "value" + }), mock.Anything).Return(localRepo, nil) + + factory, err := ProvideFactory([]Extra{localExtra}) + require.NoError(t, err) + + repoConfig := &provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Type: provisioning.LocalRepositoryType, + }, + } + + _, err = factory.Build(ctx, repoConfig) + + require.NoError(t, err) + localExtra.AssertExpectations(t) + }) +} diff --git a/pkg/registry/apis/provisioning/repository/git/extra.go b/pkg/registry/apis/provisioning/repository/git/extra.go new file mode 100644 index 00000000000..31793505d5d --- /dev/null +++ b/pkg/registry/apis/provisioning/repository/git/extra.go @@ -0,0 +1,49 @@ +package git + +import ( + "context" + "fmt" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + "k8s.io/apimachinery/pkg/runtime" +) + +type extra struct { + decrypter repository.Decrypter +} + +func Extra(decrypter repository.Decrypter) repository.Extra { + return &extra{ + decrypter: decrypter, + } +} + +func (e *extra) Type() provisioning.RepositoryType { + return provisioning.GitRepositoryType +} + +func (e *extra) Build(ctx context.Context, r *provisioning.Repository) (repository.Repository, error) { + secure := e.decrypter(r) + cfg := r.Spec.Git + if cfg == nil { + return nil, fmt.Errorf("git configuration is required") + } + + token, err := secure.Token(ctx) + if err != nil { + return nil, fmt.Errorf("unable to decrypt token: %w", err) + } + + return NewRepository(ctx, r, RepositoryConfig{ + URL: cfg.URL, + Branch: cfg.Branch, + Path: cfg.Path, + TokenUser: cfg.TokenUser, + Token: token, + }) +} + +func (e *extra) Mutate(ctx context.Context, obj runtime.Object) error { + return Mutate(ctx, obj) +} diff --git a/pkg/registry/apis/provisioning/repository/git/mutator.go b/pkg/registry/apis/provisioning/repository/git/mutator.go index 44b5535c4ed..707b18e0fa7 100644 --- a/pkg/registry/apis/provisioning/repository/git/mutator.go +++ b/pkg/registry/apis/provisioning/repository/git/mutator.go @@ -8,37 +8,34 @@ import ( "k8s.io/apimachinery/pkg/runtime" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/controller" ) -func Mutator() controller.Mutator { - return func(ctx context.Context, obj runtime.Object) error { - repo, ok := obj.(*provisioning.Repository) - if !ok { - return nil - } - - if repo.Spec.Type != provisioning.GitRepositoryType { - return nil - } - - if repo.Spec.Git == nil { - return fmt.Errorf("git configuration is required for git repository type") - } - - if repo.Spec.Git.URL != "" { - url := strings.TrimSpace(repo.Spec.Git.URL) - if url != "" { - // Remove any trailing slashes - url = strings.TrimRight(url, "/") - // Only add .git if it's not already present - if !strings.HasSuffix(url, ".git") { - url = url + ".git" - } - repo.Spec.Git.URL = url - } - } - +func Mutate(ctx context.Context, obj runtime.Object) error { + repo, ok := obj.(*provisioning.Repository) + if !ok { return nil } + + if repo.Spec.Type != provisioning.GitRepositoryType { + return nil + } + + if repo.Spec.Git == nil { + return fmt.Errorf("git configuration is required for git repository type") + } + + if repo.Spec.Git.URL != "" { + url := strings.TrimSpace(repo.Spec.Git.URL) + if url != "" { + // Remove any trailing slashes + url = strings.TrimRight(url, "/") + // Only add .git if it's not already present + if !strings.HasSuffix(url, ".git") { + url = url + ".git" + } + repo.Spec.Git.URL = url + } + } + + return nil } diff --git a/pkg/registry/apis/provisioning/repository/git/mutator_test.go b/pkg/registry/apis/provisioning/repository/git/mutator_test.go index e874cd304f9..284d51c86e4 100644 --- a/pkg/registry/apis/provisioning/repository/git/mutator_test.go +++ b/pkg/registry/apis/provisioning/repository/git/mutator_test.go @@ -11,7 +11,7 @@ import ( provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" ) -func TestMutator(t *testing.T) { +func TestMutate(t *testing.T) { tests := []struct { name string obj runtime.Object @@ -147,9 +147,7 @@ func TestMutator(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mutator := Mutator() - err := mutator(context.Background(), tt.obj) - + err := Mutate(context.Background(), tt.obj) if tt.expectedError != "" { assert.Error(t, err) assert.Contains(t, err.Error(), tt.expectedError) diff --git a/pkg/registry/apis/provisioning/repository/git/repository.go b/pkg/registry/apis/provisioning/repository/git/repository.go index 04d2d53568a..0d749b3673c 100644 --- a/pkg/registry/apis/provisioning/repository/git/repository.go +++ b/pkg/registry/apis/provisioning/repository/git/repository.go @@ -42,7 +42,7 @@ type gitRepository struct { client nanogit.Client } -func NewGitRepository( +func NewRepository( ctx context.Context, config *provisioning.Repository, gitConfig RepositoryConfig, diff --git a/pkg/registry/apis/provisioning/repository/git/repository_test.go b/pkg/registry/apis/provisioning/repository/git/repository_test.go index 2b556110beb..e158c3cb2ce 100644 --- a/pkg/registry/apis/provisioning/repository/git/repository_test.go +++ b/pkg/registry/apis/provisioning/repository/git/repository_test.go @@ -292,7 +292,7 @@ func TestNewGit(t *testing.T) { // This should succeed in creating the client but won't be able to connect // We just test that the basic structure is created correctly - gitRepo, err := NewGitRepository(ctx, config, gitConfig) + gitRepo, err := NewRepository(ctx, config, gitConfig) require.NoError(t, err) require.NotNil(t, gitRepo) require.Equal(t, "https://git.example.com/owner/repo.git", gitRepo.URL()) @@ -1860,7 +1860,7 @@ func TestNewGitRepository(t *testing.T) { }, } - gitRepo, err := NewGitRepository(ctx, config, tt.gitConfig) + gitRepo, err := NewRepository(ctx, config, tt.gitConfig) if tt.wantError { require.Error(t, err) @@ -2819,7 +2819,7 @@ func TestGitRepository_NewGitRepository_ClientError(t *testing.T) { Path: "configs", } - gitRepo, err := NewGitRepository(ctx, config, gitConfig) + gitRepo, err := NewRepository(ctx, config, gitConfig) // We expect this to fail during client creation require.Error(t, err) diff --git a/pkg/registry/apis/provisioning/repository/github/extra.go b/pkg/registry/apis/provisioning/repository/github/extra.go new file mode 100644 index 00000000000..6e357d4b7bc --- /dev/null +++ b/pkg/registry/apis/provisioning/repository/github/extra.go @@ -0,0 +1,83 @@ +package github + +import ( + "context" + "fmt" + + "github.com/grafana/grafana-app-sdk/logging" + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/git" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/webhooks" + "k8s.io/apimachinery/pkg/runtime" +) + +type extra struct { + factory *Factory + decrypter repository.Decrypter + webhookBuilder *webhooks.WebhookExtraBuilder +} + +func Extra(decrypter repository.Decrypter, factory *Factory, webhookBuilder *webhooks.WebhookExtraBuilder) repository.Extra { + return &extra{ + decrypter: decrypter, + factory: factory, + webhookBuilder: webhookBuilder, + } +} + +func (e *extra) Type() provisioning.RepositoryType { + return provisioning.GitHubRepositoryType +} + +func (e *extra) Build(ctx context.Context, r *provisioning.Repository) (repository.Repository, error) { + logger := logging.FromContext(ctx).With("url", r.Spec.GitHub.URL, "branch", r.Spec.GitHub.Branch, "path", r.Spec.GitHub.Path) + logger.Info("Instantiating Github repository") + + secure := e.decrypter(r) + cfg := r.Spec.GitHub + if cfg == nil { + return nil, fmt.Errorf("github configuration is required") + } + + token, err := secure.Token(ctx) + if err != nil { + return nil, fmt.Errorf("unable to decrypt token: %w", err) + } + + gitRepo, err := git.NewRepository(ctx, r, git.RepositoryConfig{ + URL: cfg.URL, + Branch: cfg.Branch, + Path: cfg.Path, + Token: token, + }) + if err != nil { + return nil, fmt.Errorf("error creating git repository: %w", err) + } + + ghRepo, err := NewRepository(ctx, r, gitRepo, e.factory, token) + if err != nil { + return nil, fmt.Errorf("error creating github repository: %w", err) + } + + if e.webhookBuilder == nil { + return ghRepo, nil + } + + webhookURL := e.webhookBuilder.WebhookURL(ctx, r) + if len(webhookURL) == 0 { + logger.Debug("Skipping webhook setup as no webhooks are not configured") + return ghRepo, nil + } + + webhookSecret, err := secure.WebhookSecret(ctx) + if err != nil { + return nil, fmt.Errorf("decrypt webhookSecret: %w", err) + } + + return NewGithubWebhookRepository(ghRepo, webhookURL, webhookSecret), nil +} + +func (e *extra) Mutate(ctx context.Context, obj runtime.Object) error { + return Mutate(ctx, obj) +} diff --git a/pkg/registry/apis/provisioning/repository/github/mutator.go b/pkg/registry/apis/provisioning/repository/github/mutator.go index 1a0b165cf9b..dce44a3b1e8 100644 --- a/pkg/registry/apis/provisioning/repository/github/mutator.go +++ b/pkg/registry/apis/provisioning/repository/github/mutator.go @@ -7,29 +7,26 @@ import ( "k8s.io/apimachinery/pkg/runtime" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/controller" ) -func Mutator() controller.Mutator { - return func(ctx context.Context, obj runtime.Object) error { - repo, ok := obj.(*provisioning.Repository) - if !ok { - return nil - } - - if repo.Spec.GitHub == nil { - return nil - } - - // Trim trailing ".git" and any trailing slash from the GitHub URL, if present, using the strings package. - if repo.Spec.GitHub.URL != "" { - url := repo.Spec.GitHub.URL - url = strings.TrimRight(url, "/") - url = strings.TrimSuffix(url, ".git") - url = strings.TrimRight(url, "/") - repo.Spec.GitHub.URL = url - } - +func Mutate(ctx context.Context, obj runtime.Object) error { + repo, ok := obj.(*provisioning.Repository) + if !ok { return nil } + + if repo.Spec.GitHub == nil { + return nil + } + + // Trim trailing ".git" and any trailing slash from the GitHub URL, if present, using the strings package. + if repo.Spec.GitHub.URL != "" { + url := repo.Spec.GitHub.URL + url = strings.TrimRight(url, "/") + url = strings.TrimSuffix(url, ".git") + url = strings.TrimRight(url, "/") + repo.Spec.GitHub.URL = url + } + + return nil } diff --git a/pkg/registry/apis/provisioning/repository/github/mutator_test.go b/pkg/registry/apis/provisioning/repository/github/mutator_test.go index dfeffa485ea..03b1ab6f39f 100644 --- a/pkg/registry/apis/provisioning/repository/github/mutator_test.go +++ b/pkg/registry/apis/provisioning/repository/github/mutator_test.go @@ -106,9 +106,7 @@ func TestMutator(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mutator := Mutator() - err := mutator(context.Background(), tt.obj) - + err := Mutate(context.Background(), tt.obj) if tt.expectedError != "" { assert.Error(t, err) assert.Contains(t, err.Error(), tt.expectedError) diff --git a/pkg/registry/apis/provisioning/repository/github/repository.go b/pkg/registry/apis/provisioning/repository/github/repository.go index 8985e8d8f59..74584bc8dad 100644 --- a/pkg/registry/apis/provisioning/repository/github/repository.go +++ b/pkg/registry/apis/provisioning/repository/github/repository.go @@ -42,7 +42,7 @@ type GithubRepository interface { Client() Client } -func NewGitHub( +func NewRepository( ctx context.Context, config *provisioning.Repository, gitRepo git.GitRepository, diff --git a/pkg/registry/apis/provisioning/repository/github/repository_test.go b/pkg/registry/apis/provisioning/repository/github/repository_test.go index 1e91242ca61..1407b4cbe05 100644 --- a/pkg/registry/apis/provisioning/repository/github/repository_test.go +++ b/pkg/registry/apis/provisioning/repository/github/repository_test.go @@ -82,7 +82,7 @@ func TestNewGitHub(t *testing.T) { gitRepo := git.NewMockGitRepository(t) // Call the function under test - repo, err := NewGitHub( + repo, err := NewRepository( context.Background(), tt.config, gitRepo, diff --git a/pkg/registry/apis/provisioning/webhooks/testdata/webhook-issue_comment-created.json b/pkg/registry/apis/provisioning/repository/github/testdata/webhook-issue_comment-created.json similarity index 100% rename from pkg/registry/apis/provisioning/webhooks/testdata/webhook-issue_comment-created.json rename to pkg/registry/apis/provisioning/repository/github/testdata/webhook-issue_comment-created.json diff --git a/pkg/registry/apis/provisioning/webhooks/testdata/webhook-ping-check.json b/pkg/registry/apis/provisioning/repository/github/testdata/webhook-ping-check.json similarity index 100% rename from pkg/registry/apis/provisioning/webhooks/testdata/webhook-ping-check.json rename to pkg/registry/apis/provisioning/repository/github/testdata/webhook-ping-check.json diff --git a/pkg/registry/apis/provisioning/webhooks/testdata/webhook-pull_request-opened.json b/pkg/registry/apis/provisioning/repository/github/testdata/webhook-pull_request-opened.json similarity index 100% rename from pkg/registry/apis/provisioning/webhooks/testdata/webhook-pull_request-opened.json rename to pkg/registry/apis/provisioning/repository/github/testdata/webhook-pull_request-opened.json diff --git a/pkg/registry/apis/provisioning/webhooks/testdata/webhook-push-different_branch.json b/pkg/registry/apis/provisioning/repository/github/testdata/webhook-push-different_branch.json similarity index 100% rename from pkg/registry/apis/provisioning/webhooks/testdata/webhook-push-different_branch.json rename to pkg/registry/apis/provisioning/repository/github/testdata/webhook-push-different_branch.json diff --git a/pkg/registry/apis/provisioning/webhooks/testdata/webhook-push-nested.json b/pkg/registry/apis/provisioning/repository/github/testdata/webhook-push-nested.json similarity index 100% rename from pkg/registry/apis/provisioning/webhooks/testdata/webhook-push-nested.json rename to pkg/registry/apis/provisioning/repository/github/testdata/webhook-push-nested.json diff --git a/pkg/registry/apis/provisioning/webhooks/testdata/webhook-push-nothing_relevant.json b/pkg/registry/apis/provisioning/repository/github/testdata/webhook-push-nothing_relevant.json similarity index 100% rename from pkg/registry/apis/provisioning/webhooks/testdata/webhook-push-nothing_relevant.json rename to pkg/registry/apis/provisioning/repository/github/testdata/webhook-push-nothing_relevant.json diff --git a/pkg/registry/apis/provisioning/webhooks/repository.go b/pkg/registry/apis/provisioning/repository/github/webhook.go similarity index 92% rename from pkg/registry/apis/provisioning/webhooks/repository.go rename to pkg/registry/apis/provisioning/repository/github/webhook.go index 3359a568350..60ea8db0f14 100644 --- a/pkg/registry/apis/provisioning/webhooks/repository.go +++ b/pkg/registry/apis/provisioning/repository/github/webhook.go @@ -1,4 +1,4 @@ -package webhooks +package github import ( "context" @@ -16,7 +16,6 @@ import ( provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" - pgh "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github" ) var subscribedEvents = []string{"pull_request", "push"} // same order as slices.Sort() @@ -26,24 +25,24 @@ type WebhookRepository interface { } type GithubWebhookRepository interface { - pgh.GithubRepository + GithubRepository repository.Hooks WebhookRepository } type githubWebhookRepository struct { - pgh.GithubRepository + GithubRepository config *provisioning.Repository owner string repo string secret common.RawSecureValue - gh pgh.Client + gh Client webhookURL string } func NewGithubWebhookRepository( - basic pgh.GithubRepository, + basic GithubRepository, webhookURL string, secret common.RawSecureValue, ) GithubWebhookRepository { @@ -187,13 +186,13 @@ func (r *githubWebhookRepository) CommentPullRequest(ctx context.Context, prNumb return r.gh.CreatePullRequestComment(ctx, r.owner, r.repo, prNumber, comment) } -func (r *githubWebhookRepository) createWebhook(ctx context.Context) (pgh.WebhookConfig, error) { +func (r *githubWebhookRepository) createWebhook(ctx context.Context) (WebhookConfig, error) { secret, err := uuid.NewRandom() if err != nil { - return pgh.WebhookConfig{}, fmt.Errorf("could not generate secret: %w", err) + return WebhookConfig{}, fmt.Errorf("could not generate secret: %w", err) } - cfg := pgh.WebhookConfig{ + cfg := WebhookConfig{ URL: r.webhookURL, Secret: secret.String(), ContentType: "json", @@ -203,7 +202,7 @@ func (r *githubWebhookRepository) createWebhook(ctx context.Context) (pgh.Webhoo hook, err := r.gh.CreateWebhook(ctx, r.owner, r.repo, cfg) if err != nil { - return pgh.WebhookConfig{}, err + return WebhookConfig{}, err } // HACK: GitHub does not return the secret, so we need to update it manually @@ -215,25 +214,25 @@ func (r *githubWebhookRepository) createWebhook(ctx context.Context) (pgh.Webhoo // updateWebhook checks if the webhook needs to be updated and updates it if necessary. // if the webhook does not exist, it will create it. -func (r *githubWebhookRepository) updateWebhook(ctx context.Context) (pgh.WebhookConfig, bool, error) { +func (r *githubWebhookRepository) updateWebhook(ctx context.Context) (WebhookConfig, bool, error) { if r.config.Status.Webhook == nil || r.config.Status.Webhook.ID == 0 { hook, err := r.createWebhook(ctx) if err != nil { - return pgh.WebhookConfig{}, false, err + return WebhookConfig{}, false, err } return hook, true, nil } hook, err := r.gh.GetWebhook(ctx, r.owner, r.repo, r.config.Status.Webhook.ID) switch { - case errors.Is(err, pgh.ErrResourceNotFound): + case errors.Is(err, ErrResourceNotFound): hook, err := r.createWebhook(ctx) if err != nil { - return pgh.WebhookConfig{}, false, err + return WebhookConfig{}, false, err } return hook, true, nil case err != nil: - return pgh.WebhookConfig{}, false, fmt.Errorf("get webhook: %w", err) + return WebhookConfig{}, false, fmt.Errorf("get webhook: %w", err) } var mustUpdate bool @@ -256,11 +255,11 @@ func (r *githubWebhookRepository) updateWebhook(ctx context.Context) (pgh.Webhoo // Something has changed in the webhook. Let's rotate the secret as well, so as to ensure we end up with a 100% correct webhook. secret, err := uuid.NewRandom() if err != nil { - return pgh.WebhookConfig{}, false, fmt.Errorf("could not generate secret: %w", err) + return WebhookConfig{}, false, fmt.Errorf("could not generate secret: %w", err) } hook.Secret = secret.String() if err := r.gh.EditWebhook(ctx, r.owner, r.repo, hook); err != nil { - return pgh.WebhookConfig{}, false, fmt.Errorf("edit webhook: %w", err) + return WebhookConfig{}, false, fmt.Errorf("edit webhook: %w", err) } return hook, true, nil diff --git a/pkg/registry/apis/provisioning/webhooks/repository_test.go b/pkg/registry/apis/provisioning/repository/github/webhook_test.go similarity index 95% rename from pkg/registry/apis/provisioning/webhooks/repository_test.go rename to pkg/registry/apis/provisioning/repository/github/webhook_test.go index 196b3276469..24127f7359d 100644 --- a/pkg/registry/apis/provisioning/webhooks/repository_test.go +++ b/pkg/registry/apis/provisioning/repository/github/webhook_test.go @@ -1,4 +1,4 @@ -package webhooks +package github import ( "context" @@ -22,7 +22,6 @@ import ( provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github" ) func TestParseWebhooks(t *testing.T) { @@ -919,14 +918,14 @@ func TestGitHubRepository_Webhook(t *testing.T) { func TestGitHubRepository_CommentPullRequest(t *testing.T) { tests := []struct { name string - setupMock func(m *github.MockClient) + setupMock func(m *MockClient) prNumber int comment string expectedError error }{ { name: "successfully comment on pull request", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { m.On("CreatePullRequestComment", mock.Anything, "grafana", "grafana", 123, "Test comment"). Return(nil) }, @@ -936,7 +935,7 @@ func TestGitHubRepository_CommentPullRequest(t *testing.T) { }, { name: "error commenting on pull request", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { m.On("CreatePullRequestComment", mock.Anything, "grafana", "grafana", 456, "Error comment"). Return(fmt.Errorf("failed to create comment")) }, @@ -949,7 +948,7 @@ func TestGitHubRepository_CommentPullRequest(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Setup mock GitHub client - mockGH := github.NewMockClient(t) + mockGH := NewMockClient(t) tt.setupMock(mockGH) // Create repository with mock @@ -986,7 +985,7 @@ func TestGitHubRepository_CommentPullRequest(t *testing.T) { func TestGitHubRepository_OnCreate(t *testing.T) { tests := []struct { name string - setupMock func(m *github.MockClient) + setupMock func(m *MockClient) config *provisioning.Repository webhookURL string expectedHook *provisioning.WebhookStatus @@ -994,12 +993,12 @@ func TestGitHubRepository_OnCreate(t *testing.T) { }{ { name: "successfully create webhook", - setupMock: func(m *github.MockClient) { - m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.MatchedBy(func(cfg github.WebhookConfig) bool { + setupMock: func(m *MockClient) { + m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.MatchedBy(func(cfg WebhookConfig) bool { return cfg.URL == "https://example.com/webhook" && cfg.ContentType == "json" && cfg.Active == true - })).Return(github.WebhookConfig{ + })).Return(WebhookConfig{ ID: 123, URL: "https://example.com/webhook", Secret: "test-secret", @@ -1021,7 +1020,7 @@ func TestGitHubRepository_OnCreate(t *testing.T) { }, { name: "no webhook URL", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // No webhook creation expected }, config: &provisioning.Repository{ @@ -1037,9 +1036,9 @@ func TestGitHubRepository_OnCreate(t *testing.T) { }, { name: "error creating webhook", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.Anything). - Return(github.WebhookConfig{}, fmt.Errorf("failed to create webhook")) + Return(WebhookConfig{}, fmt.Errorf("failed to create webhook")) }, config: &provisioning.Repository{ Spec: provisioning.RepositorySpec{ @@ -1057,7 +1056,7 @@ func TestGitHubRepository_OnCreate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Setup mock GitHub client - mockGH := github.NewMockClient(t) + mockGH := NewMockClient(t) tt.setupMock(mockGH) // Create repository with mock @@ -1110,7 +1109,7 @@ func TestGitHubRepository_OnCreate(t *testing.T) { func TestGitHubRepository_OnUpdate(t *testing.T) { tests := []struct { name string - setupMock func(m *github.MockClient) + setupMock func(m *MockClient) config *provisioning.Repository webhookURL string expectedHook *provisioning.WebhookStatus @@ -1118,17 +1117,17 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { }{ { name: "successfully update webhook when webhook exists", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // Mock getting the existing webhook m.On("GetWebhook", mock.Anything, "grafana", "grafana", int64(123)). - Return(github.WebhookConfig{ + Return(WebhookConfig{ ID: 123, URL: "https://example.com/webhook", Events: []string{"push"}, }, nil) // Mock editing the webhook - m.On("EditWebhook", mock.Anything, "grafana", "grafana", mock.MatchedBy(func(hook github.WebhookConfig) bool { + m.On("EditWebhook", mock.Anything, "grafana", "grafana", mock.MatchedBy(func(hook WebhookConfig) bool { return hook.ID == 123 && hook.URL == "https://example.com/webhook-updated" && slices.Equal(hook.Events, subscribedEvents) })).Return(nil) @@ -1156,18 +1155,18 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { }, { name: "create webhook when it doesn't exist", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // Mock webhook not found m.On("GetWebhook", mock.Anything, "grafana", "grafana", int64(123)). - Return(github.WebhookConfig{}, github.ErrResourceNotFound) + Return(WebhookConfig{}, ErrResourceNotFound) // Mock creating a new webhook - m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.MatchedBy(func(hook github.WebhookConfig) bool { + m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.MatchedBy(func(hook WebhookConfig) bool { return hook.URL == "https://example.com/webhook" && hook.ContentType == "json" && slices.Equal(hook.Events, subscribedEvents) && hook.Active == true - })).Return(github.WebhookConfig{ + })).Return(WebhookConfig{ ID: 456, URL: "https://example.com/webhook", Events: subscribedEvents, @@ -1196,7 +1195,7 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { }, { name: "no webhook URL provided", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // No mocks needed }, config: &provisioning.Repository{}, @@ -1206,9 +1205,9 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { }, { name: "error getting webhook", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { m.On("GetWebhook", mock.Anything, "grafana", "grafana", int64(123)). - Return(github.WebhookConfig{}, fmt.Errorf("failed to get webhook")) + Return(WebhookConfig{}, fmt.Errorf("failed to get webhook")) }, config: &provisioning.Repository{ Spec: provisioning.RepositorySpec{ @@ -1229,10 +1228,10 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { }, { name: "error editing webhook", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // Mock getting the existing webhook m.On("GetWebhook", mock.Anything, "grafana", "grafana", int64(123)). - Return(github.WebhookConfig{ + Return(WebhookConfig{ ID: 123, URL: "https://example.com/webhook", Events: []string{"push"}, @@ -1261,10 +1260,10 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { }, { name: "create webhook when webhook status is nil", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // Mock creating a new webhook m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.Anything). - Return(github.WebhookConfig{ + Return(WebhookConfig{ ID: 456, URL: "https://example.com/webhook", Events: subscribedEvents, @@ -1292,10 +1291,10 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { }, { name: "create webhook when webhook ID is zero", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // Mock creating a new webhook m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.Anything). - Return(github.WebhookConfig{ + Return(WebhookConfig{ ID: 789, URL: "https://example.com/webhook", Events: subscribedEvents, @@ -1326,10 +1325,10 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { }, { name: "error when creating webhook fails", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // Mock webhook creation failure m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.Anything). - Return(github.WebhookConfig{}, fmt.Errorf("failed to create webhook")) + Return(WebhookConfig{}, fmt.Errorf("failed to create webhook")) }, config: &provisioning.Repository{ Spec: provisioning.RepositorySpec{ @@ -1347,18 +1346,18 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { }, { name: "creates webhook when ErrResourceNotFound", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // Mock webhook not found m.On("GetWebhook", mock.Anything, "grafana", "grafana", int64(123)). - Return(github.WebhookConfig{}, github.ErrResourceNotFound) + Return(WebhookConfig{}, ErrResourceNotFound) // Mock creating a new webhook - m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.MatchedBy(func(hook github.WebhookConfig) bool { + m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.MatchedBy(func(hook WebhookConfig) bool { return hook.URL == "https://example.com/webhook" && hook.ContentType == "json" && slices.Equal(hook.Events, subscribedEvents) && hook.Active == true - })).Return(github.WebhookConfig{ + })).Return(WebhookConfig{ ID: 456, URL: "https://example.com/webhook", Events: subscribedEvents, @@ -1387,18 +1386,18 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { }, { name: "error on create when not found", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // Mock webhook not found m.On("GetWebhook", mock.Anything, "grafana", "grafana", int64(123)). - Return(github.WebhookConfig{}, github.ErrResourceNotFound) + Return(WebhookConfig{}, ErrResourceNotFound) // Mock error when creating a new webhook - m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.MatchedBy(func(hook github.WebhookConfig) bool { + m.On("CreateWebhook", mock.Anything, "grafana", "grafana", mock.MatchedBy(func(hook WebhookConfig) bool { return hook.URL == "https://example.com/webhook" && hook.ContentType == "json" && slices.Equal(hook.Events, subscribedEvents) && hook.Active == true - })).Return(github.WebhookConfig{}, fmt.Errorf("failed to create webhook")) + })).Return(WebhookConfig{}, fmt.Errorf("failed to create webhook")) }, config: &provisioning.Repository{ Spec: provisioning.RepositorySpec{ @@ -1419,10 +1418,10 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { }, { name: "no update needed when URL and events match", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // Mock getting the existing webhook with matching URL and events m.On("GetWebhook", mock.Anything, "grafana", "grafana", int64(123)). - Return(github.WebhookConfig{ + Return(WebhookConfig{ ID: 123, URL: "https://example.com/webhook", Events: subscribedEvents, @@ -1456,7 +1455,7 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Setup mock GitHub client - mockGH := github.NewMockClient(t) + mockGH := NewMockClient(t) tt.setupMock(mockGH) // Create repository with mock @@ -1510,14 +1509,14 @@ func TestGitHubRepository_OnUpdate(t *testing.T) { func TestGitHubRepository_OnDelete(t *testing.T) { tests := []struct { name string - setupMock func(m *github.MockClient) + setupMock func(m *MockClient) config *provisioning.Repository webhookURL string expectedError error }{ { name: "successfully delete webhook", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { m.On("DeleteWebhook", mock.Anything, "grafana", "grafana", int64(123)). Return(nil) }, @@ -1542,7 +1541,7 @@ func TestGitHubRepository_OnDelete(t *testing.T) { }, { name: "no webhook URL provided", - setupMock: func(_ *github.MockClient) {}, + setupMock: func(_ *MockClient) {}, config: &provisioning.Repository{ ObjectMeta: metav1.ObjectMeta{ Name: "test-repo", @@ -1558,7 +1557,7 @@ func TestGitHubRepository_OnDelete(t *testing.T) { }, { name: "webhook not found in status", - setupMock: func(_ *github.MockClient) { + setupMock: func(_ *MockClient) { // No secrets deletion or webhook deletion mocks needed - method returns early when webhook is nil }, config: &provisioning.Repository{ @@ -1579,7 +1578,7 @@ func TestGitHubRepository_OnDelete(t *testing.T) { }, { name: "error deleting webhook", - setupMock: func(m *github.MockClient) { + setupMock: func(m *MockClient) { // Mock webhook deletion failure m.On("DeleteWebhook", mock.Anything, "grafana", "grafana", int64(123)). Return(fmt.Errorf("failed to delete webhook")) @@ -1608,8 +1607,8 @@ func TestGitHubRepository_OnDelete(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Setup mock GitHub client - mockGH := github.NewMockClient(t) - mockRepo := github.NewMockGithubRepository(t) + mockGH := NewMockClient(t) + mockRepo := NewMockGithubRepository(t) tt.setupMock(mockGH) // Create repository with mock diff --git a/pkg/registry/apis/provisioning/repository/local/extra.go b/pkg/registry/apis/provisioning/repository/local/extra.go new file mode 100644 index 00000000000..3209a8e3bc1 --- /dev/null +++ b/pkg/registry/apis/provisioning/repository/local/extra.go @@ -0,0 +1,36 @@ +package local + +import ( + "context" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/safepath" + "github.com/grafana/grafana/pkg/setting" + "k8s.io/apimachinery/pkg/runtime" +) + +type extra struct { + resolver *LocalFolderResolver +} + +func Extra(cfg *setting.Cfg) repository.Extra { + resolver := &LocalFolderResolver{ + PermittedPrefixes: cfg.PermittedProvisioningPaths, + HomePath: safepath.Clean(cfg.HomePath), + } + + return &extra{resolver: resolver} +} + +func (e *extra) Type() provisioning.RepositoryType { + return provisioning.LocalRepositoryType +} + +func (e *extra) Build(_ context.Context, r *provisioning.Repository) (repository.Repository, error) { + return NewRepository(r, e.resolver), nil +} + +func (e *extra) Mutate(_ context.Context, _ runtime.Object) error { + return nil +} diff --git a/pkg/registry/apis/provisioning/repository/local/local.go b/pkg/registry/apis/provisioning/repository/local/local.go index 98413cc1094..f5833f2fa2c 100644 --- a/pkg/registry/apis/provisioning/repository/local/local.go +++ b/pkg/registry/apis/provisioning/repository/local/local.go @@ -89,11 +89,12 @@ type localRepository struct { path string } -func NewLocal(config *provisioning.Repository, resolver *LocalFolderResolver) *localRepository { +func NewRepository(config *provisioning.Repository, resolver *LocalFolderResolver) *localRepository { r := &localRepository{ config: config, resolver: resolver, } + if config.Spec.Local != nil { r.path, _ = resolver.LocalPath(config.Spec.Local.Path) if r.path != "" && !safepath.IsDir(r.path) { diff --git a/pkg/registry/apis/provisioning/repository/local/local_test.go b/pkg/registry/apis/provisioning/repository/local/local_test.go index 378a3170478..7e20c838baa 100644 --- a/pkg/registry/apis/provisioning/repository/local/local_test.go +++ b/pkg/registry/apis/provisioning/repository/local/local_test.go @@ -69,7 +69,7 @@ func TestLocalResolver(t *testing.T) { } // Test repository with the temp directory - repo := NewLocal(&provisioning.Repository{ + repo := NewRepository(&provisioning.Repository{ Spec: provisioning.RepositorySpec{ Local: &provisioning.LocalRepositoryConfig{ Path: tempDir, @@ -126,7 +126,7 @@ func TestLocal(t *testing.T) { {"absolute path with multiple prefixes", "/devenv/test", []string{"/home/grafana", "/devenv"}, "/devenv/test/"}, } { t.Run("valid: "+tc.Name, func(t *testing.T) { - r := NewLocal(&provisioning.Repository{ + r := NewRepository(&provisioning.Repository{ Spec: provisioning.RepositorySpec{ Local: &provisioning.LocalRepositoryConfig{ Path: tc.Path, @@ -152,7 +152,7 @@ func TestLocal(t *testing.T) { {"unconfigured prefix", "invalid/path", []string{"devenv", "/tmp", "test"}}, } { t.Run("invalid: "+tc.Name, func(t *testing.T) { - r := NewLocal(&provisioning.Repository{ + r := NewRepository(&provisioning.Repository{ Spec: provisioning.RepositorySpec{ Local: &provisioning.LocalRepositoryConfig{ Path: tc.Path, @@ -238,7 +238,7 @@ func TestLocalRepository_Test(t *testing.T) { } // Create the repository with the test path - repo := NewLocal(&provisioning.Repository{ + repo := NewRepository(&provisioning.Repository{ Spec: provisioning.RepositorySpec{ Local: &provisioning.LocalRepositoryConfig{ Path: tc.path, @@ -353,7 +353,7 @@ func TestLocalRepository_Validate(t *testing.T) { } // Create the repository - repo := NewLocal(repoConfig, resolver) + repo := NewRepository(repoConfig, resolver) // Call the Validate method errors := repo.Validate() diff --git a/pkg/registry/apis/provisioning/routes.go b/pkg/registry/apis/provisioning/routes.go index 5136555bf41..dd063f0fe0e 100644 --- a/pkg/registry/apis/provisioning/routes.go +++ b/pkg/registry/apis/provisioning/routes.go @@ -156,16 +156,11 @@ func (b *APIBuilder) handleSettings(w http.ResponseWriter, r *http.Request) { return } - availableTypes := []provisioning.RepositoryType{} - for t := range b.availableRepositoryTypes { - availableTypes = append(availableTypes, t) - } - settings := provisioning.RepositoryViewList{ Items: make([]provisioning.RepositoryView, len(all)), // FIXME: this shouldn't be here in provisioning but at the dual writer or something about the storage LegacyStorage: dualwrite.IsReadingLegacyDashboardsAndFolders(ctx, b.storageStatus), - AvailableRepositoryTypes: availableTypes, + AvailableRepositoryTypes: b.repoFactory.Types(), } for i, val := range all { diff --git a/pkg/registry/apis/provisioning/test.go b/pkg/registry/apis/provisioning/test.go index 89733b26b32..24f68372ddc 100644 --- a/pkg/registry/apis/provisioning/test.go +++ b/pkg/registry/apis/provisioning/test.go @@ -29,12 +29,19 @@ type HealthCheckerProvider interface { type testConnector struct { getter RepoGetter + factory repository.Factory tester controller.RepositoryTester healthProvider HealthCheckerProvider } -func NewTestConnector(getter RepoGetter, tester controller.RepositoryTester, healthProvider HealthCheckerProvider) *testConnector { +func NewTestConnector( + getter RepoGetter, + factory repository.Factory, + tester controller.RepositoryTester, + healthProvider HealthCheckerProvider, +) *testConnector { return &testConnector{ + factory: factory, getter: getter, tester: tester, healthProvider: healthProvider, @@ -113,7 +120,7 @@ func (s *testConnector) Connect(ctx context.Context, name string, opts runtime.O } // Create a temporary repository - tmp, err := s.getter.RepositoryFromConfig(ctx, &cfg) + tmp, err := s.factory.Build(ctx, &cfg) if err != nil { responder.Error(err) return diff --git a/pkg/registry/apis/provisioning/types.go b/pkg/registry/apis/provisioning/types.go index 9f3db292b0b..a5e9cd6b9f6 100644 --- a/pkg/registry/apis/provisioning/types.go +++ b/pkg/registry/apis/provisioning/types.go @@ -3,7 +3,6 @@ package provisioning import ( "context" - provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" client "github.com/grafana/grafana/apps/provisioning/pkg/generated/clientset/versioned/typed/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" ) @@ -14,11 +13,6 @@ type RepoGetter interface { // This will return a healthy repository, or an error saying the repository is not healthy GetHealthyRepository(ctx context.Context, name string) (repository.Repository, error) - - // Given a repository configuration, return it as a repository instance - // This will only error for un-recoverable system errors - // the repository instance may or may not be valid/healthy - RepositoryFromConfig(ctx context.Context, cfg *provisioning.Repository) (repository.Repository, error) } type ClientGetter interface { diff --git a/pkg/registry/apis/provisioning/webhooks/register.go b/pkg/registry/apis/provisioning/webhooks/register.go index 55bb26e02f5..a26b6327d5a 100644 --- a/pkg/registry/apis/provisioning/webhooks/register.go +++ b/pkg/registry/apis/provisioning/webhooks/register.go @@ -9,18 +9,12 @@ import ( "k8s.io/apiserver/pkg/registry/rest" "k8s.io/kube-openapi/pkg/spec3" - "github.com/grafana/grafana-app-sdk/logging" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" provisioningapis "github.com/grafana/grafana/pkg/registry/apis/provisioning" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/controller" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/git" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" "github.com/grafana/grafana/pkg/registry/apis/provisioning/webhooks/pullrequest" "github.com/grafana/grafana/pkg/services/apiserver" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/rendering" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/unified/resource" @@ -29,8 +23,28 @@ import ( // WebhookExtraBuilder is a function that returns an ExtraBuilder. // It is used to add additional functionality for webhooks type WebhookExtraBuilder struct { - // HACK: We need to wrap the builder to please wire so that it can uniquely identify the dependency provisioningapis.ExtraBuilder + isPublic bool + urlProvider func(namespace string) string +} + +func (b *WebhookExtraBuilder) WebhookURL(ctx context.Context, r *provisioning.Repository) string { + if !b.isPublic { + return "" + } + + gvr := provisioning.RepositoryResourceInfo.GroupVersionResource() + webhookURL := fmt.Sprintf( + "%sapis/%s/%s/namespaces/%s/%s/%s/webhook", + b.urlProvider(r.GetNamespace()), + gvr.Group, + gvr.Version, + r.GetNamespace(), + gvr.Resource, + r.GetName(), + ) + + return webhookURL } // HACK: assume that the URL is public if it starts with "https://" and does not contain any local IP ranges @@ -45,19 +59,19 @@ func isPublicURL(url string) bool { func ProvideWebhooks( cfg *setting.Cfg, - features featuremgmt.FeatureToggles, - ghFactory *github.Factory, renderer rendering.Service, blobstore resource.ResourceClient, configProvider apiserver.RestConfigProvider, -) WebhookExtraBuilder { - return WebhookExtraBuilder{ - ExtraBuilder: func(b *provisioningapis.APIBuilder) provisioningapis.Extra { - urlProvider := func(_ string) string { - return cfg.AppURL - } +) *WebhookExtraBuilder { + urlProvider := func(_ string) string { + return cfg.AppURL + } + isPublic := isPublicURL(urlProvider("")) - isPublic := isPublicURL(urlProvider("")) + return &WebhookExtraBuilder{ + isPublic: isPublic, + urlProvider: urlProvider, + ExtraBuilder: func(b *provisioningapis.APIBuilder) provisioningapis.Extra { clients := resources.NewClientFactory(configProvider) parsers := resources.NewParserFactory(clients) @@ -77,10 +91,7 @@ func ProvideWebhooks( render, webhook, urlProvider, - ghFactory, - parsers, []jobs.Worker{pullRequestWorker}, - isPublic, // Pass the public URL flag ) }, } @@ -89,32 +100,21 @@ func ProvideWebhooks( // WebhookExtra implements the Extra interface for webhooks // to wrap around type WebhookExtra struct { - render *renderConnector - webhook *webhookConnector - urlProvider func(namespace string) string - ghFactory *github.Factory - parsers resources.ParserFactory - workers []jobs.Worker - isPublic bool // Flag to determine if webhook-enhanced repositories should be created + render *renderConnector + webhook *webhookConnector + workers []jobs.Worker } func NewWebhookExtra( render *renderConnector, webhook *webhookConnector, urlProvider func(namespace string) string, - ghFactory *github.Factory, - parsers resources.ParserFactory, workers []jobs.Worker, - isPublic bool, ) *WebhookExtra { return &WebhookExtra{ - render: render, - webhook: webhook, - urlProvider: urlProvider, - ghFactory: ghFactory, - parsers: parsers, - workers: workers, - isPublic: isPublic, + render: render, + webhook: webhook, + workers: workers, } } @@ -128,16 +128,12 @@ func (e *WebhookExtra) Authorize(ctx context.Context, a authorizer.Attributes) ( return e.render.Authorize(ctx, a) } -// Mutators returns the mutators for the webhook extra -func (e *WebhookExtra) Mutators() []controller.Mutator { - return nil -} - // UpdateStorage updates the storage with both render and webhook connectors func (e *WebhookExtra) UpdateStorage(storage map[string]rest.Storage) error { if err := e.webhook.UpdateStorage(storage); err != nil { return err } + return e.render.UpdateStorage(storage) } @@ -154,68 +150,3 @@ func (e *WebhookExtra) PostProcessOpenAPI(oas *spec3.OpenAPI) error { func (e *WebhookExtra) GetJobWorkers() []jobs.Worker { return e.workers } - -// AsRepository delegates repository creation to the webhook connector -func (e *WebhookExtra) AsRepository(ctx context.Context, r *provisioning.Repository, secure repository.SecureValues) (repository.Repository, error) { - // Only handle GitHub repositories with webhooks if URL is public - if r.Spec.Type == provisioning.GitHubRepositoryType && e.isPublic { - gvr := provisioning.RepositoryResourceInfo.GroupVersionResource() - webhookURL := fmt.Sprintf( - "%sapis/%s/%s/namespaces/%s/%s/%s/webhook", - e.urlProvider(r.GetNamespace()), - gvr.Group, - gvr.Version, - r.GetNamespace(), - gvr.Resource, - r.GetName(), - ) - - logger := logging.FromContext(ctx).With("url", r.Spec.GitHub.URL, "branch", r.Spec.GitHub.Branch, "path", r.Spec.GitHub.Path) - logger.Info("Instantiating Github repository with webhooks") - ghCfg := r.Spec.GitHub - if ghCfg == nil { - return nil, fmt.Errorf("github configuration is required for nano git") - } - - // Decrypt GitHub token if needed - ghToken, err := secure.Token(ctx) - if err != nil { - return nil, fmt.Errorf("decrypt github token: %w", err) - } - webhookSecret, err := secure.WebhookSecret(ctx) - if err != nil { - return nil, fmt.Errorf("decrypt webhookSecret: %w", err) - } - - gitCfg := git.RepositoryConfig{ - URL: ghCfg.URL, - Branch: ghCfg.Branch, - Path: ghCfg.Path, - Token: ghToken, - } - - gitRepo, err := git.NewGitRepository(ctx, r, gitCfg) - if err != nil { - return nil, fmt.Errorf("error creating git repository: %w", err) - } - - basicRepo, err := github.NewGitHub(ctx, r, gitRepo, e.ghFactory, ghToken) - if err != nil { - return nil, fmt.Errorf("error creating github repository: %w", err) - } - - return NewGithubWebhookRepository(basicRepo, webhookURL, webhookSecret), nil - } - - return nil, nil -} - -func (e *WebhookExtra) RepositoryTypes() []provisioning.RepositoryType { - // Only claim to handle GitHub repositories if URL is public - if e.isPublic { - return []provisioning.RepositoryType{ - provisioning.GitHubRepositoryType, - } - } - return []provisioning.RepositoryType{} -} diff --git a/pkg/registry/apis/provisioning/webhooks/webhook.go b/pkg/registry/apis/provisioning/webhooks/webhook.go index c922ddfe3ee..c3e31d6856a 100644 --- a/pkg/registry/apis/provisioning/webhooks/webhook.go +++ b/pkg/registry/apis/provisioning/webhooks/webhook.go @@ -21,6 +21,10 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/provisioning/webhooks/pullrequest" ) +type WebhookRepository interface { + Webhook(ctx context.Context, req *http.Request) (*provisioning.WebhookResponse, error) +} + // Webhook endpoint max size (25MB) // See https://docs.github.com/en/webhooks/webhook-events-and-payloads const webhookMaxBodySize = 25 * 1024 * 1024 diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index 2b5d43803c4..b8e0294b7c2 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -59,6 +59,7 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/ofrep" provisioning2 "github.com/grafana/grafana/pkg/registry/apis/provisioning" "github.com/grafana/grafana/pkg/registry/apis/provisioning/extras" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github" "github.com/grafana/grafana/pkg/registry/apis/provisioning/webhooks" query2 "github.com/grafana/grafana/pkg/registry/apis/query" @@ -801,20 +802,25 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api return nil, err } userStorageAPIBuilder := userstorage.RegisterAPIService(featureToggles, apiserverService, registerer) - factory := github.ProvideFactory() legacyMigrator := legacy.ProvideLegacyMigrator(sqlStore, provisioningServiceImpl, libraryPanelService, dashboardPermissionsService, accessControl, featureToggles) + webhookExtraBuilder := webhooks.ProvideWebhooks(cfg, renderingService, resourceClient, eventualRestConfigProvider) + v3 := extras.ProvideProvisioningOSSExtras(webhookExtraBuilder) decryptAuthorizer := decrypt.ProvideDecryptAuthorizer(tracer) decryptStorage, err := metadata.ProvideDecryptStorage(tracer, ossKeeperService, keeperMetadataStorage, secureValueMetadataStorage, decryptAuthorizer, registerer) if err != nil { return nil, err } - v3, err := decrypt.ProvideDecryptService(cfg, tracer, decryptStorage) + v4, err := decrypt.ProvideDecryptService(cfg, tracer, decryptStorage) if err != nil { return nil, err } - webhookExtraBuilder := webhooks.ProvideWebhooks(cfg, featureToggles, factory, renderingService, resourceClient, eventualRestConfigProvider) - v4 := extras.ProvideProvisioningOSSExtras(webhookExtraBuilder) - apiBuilder, err := provisioning2.RegisterAPIService(cfg, featureToggles, apiserverService, registerer, resourceClient, eventualRestConfigProvider, factory, accessClient, legacyMigrator, dualwriteService, usageStats, v3, tracingService, v4) + factory := github.ProvideFactory() + v5 := extras.ProvideProvisioningOSSRepositoryExtras(cfg, v4, factory, webhookExtraBuilder) + repositoryFactory, err := repository.ProvideFactory(v5) + if err != nil { + return nil, err + } + apiBuilder, err := provisioning2.RegisterAPIService(cfg, featureToggles, apiserverService, registerer, resourceClient, eventualRestConfigProvider, accessClient, legacyMigrator, dualwriteService, usageStats, tracingService, v3, repositoryFactory) if err != nil { return nil, err } @@ -1377,20 +1383,25 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac return nil, err } userStorageAPIBuilder := userstorage.RegisterAPIService(featureToggles, apiserverService, registerer) - factory := github.ProvideFactory() legacyMigrator := legacy.ProvideLegacyMigrator(sqlStore, provisioningServiceImpl, libraryPanelService, dashboardPermissionsService, accessControl, featureToggles) + webhookExtraBuilder := webhooks.ProvideWebhooks(cfg, renderingService, resourceClient, eventualRestConfigProvider) + v3 := extras.ProvideProvisioningOSSExtras(webhookExtraBuilder) decryptAuthorizer := decrypt.ProvideDecryptAuthorizer(tracer) decryptStorage, err := metadata.ProvideDecryptStorage(tracer, ossKeeperService, keeperMetadataStorage, secureValueMetadataStorage, decryptAuthorizer, registerer) if err != nil { return nil, err } - v3, err := decrypt.ProvideDecryptService(cfg, tracer, decryptStorage) + v4, err := decrypt.ProvideDecryptService(cfg, tracer, decryptStorage) if err != nil { return nil, err } - webhookExtraBuilder := webhooks.ProvideWebhooks(cfg, featureToggles, factory, renderingService, resourceClient, eventualRestConfigProvider) - v4 := extras.ProvideProvisioningOSSExtras(webhookExtraBuilder) - apiBuilder, err := provisioning2.RegisterAPIService(cfg, featureToggles, apiserverService, registerer, resourceClient, eventualRestConfigProvider, factory, accessClient, legacyMigrator, dualwriteService, usageStats, v3, tracingService, v4) + factory := github.ProvideFactory() + v5 := extras.ProvideProvisioningOSSRepositoryExtras(cfg, v4, factory, webhookExtraBuilder) + repositoryFactory, err := repository.ProvideFactory(v5) + if err != nil { + return nil, err + } + apiBuilder, err := provisioning2.RegisterAPIService(cfg, featureToggles, apiserverService, registerer, resourceClient, eventualRestConfigProvider, accessClient, legacyMigrator, dualwriteService, usageStats, tracingService, v3, repositoryFactory) if err != nil { return nil, err } @@ -1425,7 +1436,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac if err != nil { return nil, err } - testEnv, err := ProvideTestEnv(testingT, server, sqlStore, cfg, notificationServiceMock, grpcserverProvider, inMemory, httpclientProvider, oauthtokentestService, featureToggles, resourceClient, idimplService, factory, v3) + testEnv, err := ProvideTestEnv(testingT, server, sqlStore, cfg, notificationServiceMock, grpcserverProvider, inMemory, httpclientProvider, oauthtokentestService, featureToggles, resourceClient, idimplService, factory, v4) if err != nil { return nil, err } diff --git a/pkg/server/wireexts_oss.go b/pkg/server/wireexts_oss.go index 57c18d86cc4..84dbcef729e 100644 --- a/pkg/server/wireexts_oss.go +++ b/pkg/server/wireexts_oss.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/registry" apisregistry "github.com/grafana/grafana/pkg/registry/apis" "github.com/grafana/grafana/pkg/registry/apis/provisioning/extras" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/webhooks" "github.com/grafana/grafana/pkg/registry/apis/secret" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" @@ -69,7 +70,9 @@ import ( var provisioningExtras = wire.NewSet( webhooks.ProvideWebhooks, + repository.ProvideFactory, extras.ProvideProvisioningOSSExtras, + extras.ProvideProvisioningOSSRepositoryExtras, ) var configProviderExtras = wire.NewSet(