From 842f68f91bf283918f0838667f2668a6038052f0 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Thu, 20 Feb 2025 13:01:03 +0300 Subject: [PATCH] Provisioning: Workflows as write access (#101031) * workflow as write access * workflow as write access * workflow as write access * Update pkg/registry/apis/provisioning/repository/test.go Co-authored-by: Mariell Hoversholm * POC/Provisioning: Add wizard (#100596) * update refs * update refs * lint fix * lint fix * lint fix * default everythign to read only * reuse form components * remove main --------- Co-authored-by: Mariell Hoversholm Co-authored-by: Alex Khomenko --- .betterer.results | 4 +- pkg/apis/provisioning/v0alpha1/types.go | 21 +++---- .../v0alpha1/zz_generated.deepcopy.go | 10 ++-- .../v0alpha1/zz_generated.openapi.go | 40 ++++++------- ...enerated.openapi_violation_exceptions.list | 2 +- .../v0alpha1/githubrepositoryconfig.go | 25 ++------ .../provisioning/v0alpha1/repositoryspec.go | 14 +++-- pkg/registry/apis/provisioning/export.go | 8 +-- pkg/registry/apis/provisioning/files.go | 10 ++-- .../apis/provisioning/jobs/export/worker.go | 9 +-- .../apis/provisioning/jobs/migrate/utils.go | 3 +- pkg/registry/apis/provisioning/register.go | 13 +--- .../apis/provisioning/repository/github.go | 39 ------------ .../apis/provisioning/repository/test.go | 15 ++++- .../apis/provisioning/repository/workflows.go | 40 +++++++++++++ pkg/registry/apis/provisioning/routes.go | 2 +- .../provisioning.grafana.app-v0alpha1.json | 42 ++++++------- .../apis/provisioning/provisioning_test.go | 21 +++++-- .../provisioning/testdata/github-example.json | 3 +- .../local-conf-provisioning-sample.json | 2 +- .../provisioning/testdata/local-devenv.json | 2 +- .../apis/provisioning/testdata/local-tmp.json | 1 - .../apis/provisioning/testdata/local-xxx.json | 2 +- .../app/features/provisioning/ConfigForm.tsx | 60 ++++++++++++------- .../provisioning/Wizard/RepositoryStep.tsx | 8 +-- .../provisioning/api/endpoints.gen.ts | 12 ++-- public/app/features/provisioning/types.ts | 2 +- 27 files changed, 205 insertions(+), 205 deletions(-) create mode 100644 pkg/registry/apis/provisioning/repository/workflows.go diff --git a/.betterer.results b/.betterer.results index cdb1c49806f..c9e8cfc6352 100644 --- a/.betterer.results +++ b/.betterer.results @@ -5731,8 +5731,8 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "22"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "23"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "24"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "25"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "26"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "25"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "26"] ], "public/app/features/provisioning/DeleteRepositoryButton.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], diff --git a/pkg/apis/provisioning/v0alpha1/types.go b/pkg/apis/provisioning/v0alpha1/types.go index ab1417b9980..064aab7242c 100644 --- a/pkg/apis/provisioning/v0alpha1/types.go +++ b/pkg/apis/provisioning/v0alpha1/types.go @@ -28,10 +28,10 @@ type LocalRepositoryConfig struct { type Workflow string const ( - // BranchWorkflow creates a branch for changes + // WriteWorkflow allows a user to write directly to the repository + WriteWorkflow Workflow = "write" + // BranchWorkflow allows creating a branch for changes (where we can then make a PR) BranchWorkflow Workflow = "branch" - // PushWorkflow pushes changes directly the configured branch - PushWorkflow Workflow = "push" ) type GitHubRepositoryConfig struct { @@ -39,19 +39,14 @@ type GitHubRepositoryConfig struct { URL string `json:"url,omitempty"` // The branch to use in the repository. - // By default, this is the main branch. - Branch string `json:"branch,omitempty"` + Branch string `json:"branch"` + // Token for accessing the repository. If set, it will be encrypted into encryptedToken, then set to an empty string again. Token string `json:"token,omitempty"` // Token for accessing the repository, but encrypted. This is not possible to read back to a user decrypted. // +listType=atomic EncryptedToken []byte `json:"encryptedToken,omitempty"` - // Workflow allowed for changes to the repository. - // The order is relevant for defining the precedence of the workflows. - // Possible values: pull-request, branch, push. - Workflows []Workflow `json:"workflows,omitempty"` - // Whether we should show dashboard previews for pull requests. // By default, this is false (i.e. we will not create previews). GenerateDashboardPreviews bool `json:"generateDashboardPreviews,omitempty"` @@ -74,8 +69,10 @@ type RepositorySpec struct { // Repository description Description string `json:"description,omitempty"` - // ReadOnly repository does not allow any write commands - ReadOnly bool `json:"readOnly"` + // UI driven Workflow that allow changes to the contends of the repository. + // The order is relevant for defining the precedence of the workflows. + // When empty, the repository does not support any edits (eg, readonly) + Workflows []Workflow `json:"workflows"` // Sync settings -- how values are pulled from the repository into grafana Sync SyncOptions `json:"sync"` diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go b/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go index 6dc88372968..91db1c201ee 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go @@ -98,11 +98,6 @@ func (in *GitHubRepositoryConfig) DeepCopyInto(out *GitHubRepositoryConfig) { *out = make([]byte, len(*in)) copy(*out, *in) } - if in.Workflows != nil { - in, out := &in.Workflows, &out.Workflows - *out = make([]Workflow, len(*in)) - copy(*out, *in) - } return } @@ -469,6 +464,11 @@ func (in *RepositoryList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RepositorySpec) DeepCopyInto(out *RepositorySpec) { *out = *in + if in.Workflows != nil { + in, out := &in.Workflows, &out.Workflows + *out = make([]Workflow, len(*in)) + copy(*out, *in) + } out.Sync = in.Sync if in.Local != nil { in, out := &in.Local, &out.Local diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go index 7fa855a69bc..4c14b254414 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go @@ -240,7 +240,8 @@ func schema_pkg_apis_provisioning_v0alpha1_GitHubRepositoryConfig(ref common.Ref }, "branch": { SchemaProps: spec.SchemaProps{ - Description: "The branch to use in the repository. By default, this is the main branch.", + Description: "The branch to use in the repository.", + Default: "", Type: []string{"string"}, Format: "", }, @@ -264,22 +265,6 @@ func schema_pkg_apis_provisioning_v0alpha1_GitHubRepositoryConfig(ref common.Ref Format: "byte", }, }, - "workflows": { - SchemaProps: spec.SchemaProps{ - Description: "Workflow allowed for changes to the repository. The order is relevant for defining the precedence of the workflows. Possible values: pull-request, branch, push.", - Type: []string{"array"}, - Items: &spec.SchemaOrArray{ - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Default: "", - Type: []string{"string"}, - Format: "", - Enum: []interface{}{"branch", "push"}, - }, - }, - }, - }, - }, "generateDashboardPreviews": { SchemaProps: spec.SchemaProps{ Description: "Whether we should show dashboard previews for pull requests. By default, this is false (i.e. we will not create previews).", @@ -288,6 +273,7 @@ func schema_pkg_apis_provisioning_v0alpha1_GitHubRepositoryConfig(ref common.Ref }, }, }, + Required: []string{"branch"}, }, }, } @@ -1004,12 +990,20 @@ func schema_pkg_apis_provisioning_v0alpha1_RepositorySpec(ref common.ReferenceCa Format: "", }, }, - "readOnly": { + "workflows": { SchemaProps: spec.SchemaProps{ - Description: "ReadOnly repository does not allow any write commands", - Default: false, - Type: []string{"boolean"}, - Format: "", + Description: "UI driven Workflow taht allow changes to the contends of the repository. The order is relevant for defining the precedence of the workflows. When empty, the repository does not support any edits (eg, readonly)", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + Enum: []interface{}{"branch", "write"}, + }, + }, + }, }, }, "sync": { @@ -1041,7 +1035,7 @@ func schema_pkg_apis_provisioning_v0alpha1_RepositorySpec(ref common.ReferenceCa }, }, }, - Required: []string{"title", "readOnly", "sync", "type"}, + Required: []string{"title", "workflows", "sync", "type"}, }, }, Dependencies: []string{ diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi_violation_exceptions.list b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi_violation_exceptions.list index 313713c794b..696b361ec80 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi_violation_exceptions.list +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi_violation_exceptions.list @@ -1,10 +1,10 @@ API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,FileList,Items -API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,GitHubRepositoryConfig,Workflows API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,HistoryList,Items API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,JobResourceSummary,Errors API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,JobStatus,Errors API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,JobStatus,Summary API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,RepositoryList,Items +API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,RepositorySpec,Workflows API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,RepositoryViewList,Items API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,ResourceList,Items API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1,ResourceStats,Items diff --git a/pkg/generated/applyconfiguration/provisioning/v0alpha1/githubrepositoryconfig.go b/pkg/generated/applyconfiguration/provisioning/v0alpha1/githubrepositoryconfig.go index 3003228ef15..e3cc1f0829c 100644 --- a/pkg/generated/applyconfiguration/provisioning/v0alpha1/githubrepositoryconfig.go +++ b/pkg/generated/applyconfiguration/provisioning/v0alpha1/githubrepositoryconfig.go @@ -4,19 +4,14 @@ package v0alpha1 -import ( - provisioningv0alpha1 "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" -) - // GitHubRepositoryConfigApplyConfiguration represents a declarative configuration of the GitHubRepositoryConfig type for use // with apply. type GitHubRepositoryConfigApplyConfiguration struct { - URL *string `json:"url,omitempty"` - Branch *string `json:"branch,omitempty"` - Token *string `json:"token,omitempty"` - EncryptedToken []byte `json:"encryptedToken,omitempty"` - Workflows []provisioningv0alpha1.Workflow `json:"workflows,omitempty"` - GenerateDashboardPreviews *bool `json:"generateDashboardPreviews,omitempty"` + URL *string `json:"url,omitempty"` + Branch *string `json:"branch,omitempty"` + Token *string `json:"token,omitempty"` + EncryptedToken []byte `json:"encryptedToken,omitempty"` + GenerateDashboardPreviews *bool `json:"generateDashboardPreviews,omitempty"` } // GitHubRepositoryConfigApplyConfiguration constructs a declarative configuration of the GitHubRepositoryConfig type for use with @@ -59,16 +54,6 @@ func (b *GitHubRepositoryConfigApplyConfiguration) WithEncryptedToken(values ... return b } -// WithWorkflows adds the given value to the Workflows field in the declarative configuration -// and returns the receiver, so that objects can be build by chaining "With" function invocations. -// If called multiple times, values provided by each call will be appended to the Workflows field. -func (b *GitHubRepositoryConfigApplyConfiguration) WithWorkflows(values ...provisioningv0alpha1.Workflow) *GitHubRepositoryConfigApplyConfiguration { - for i := range values { - b.Workflows = append(b.Workflows, values[i]) - } - return b -} - // WithGenerateDashboardPreviews sets the GenerateDashboardPreviews field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the GenerateDashboardPreviews field is set to the value of the last call. diff --git a/pkg/generated/applyconfiguration/provisioning/v0alpha1/repositoryspec.go b/pkg/generated/applyconfiguration/provisioning/v0alpha1/repositoryspec.go index b0d4769286d..75508c2003b 100644 --- a/pkg/generated/applyconfiguration/provisioning/v0alpha1/repositoryspec.go +++ b/pkg/generated/applyconfiguration/provisioning/v0alpha1/repositoryspec.go @@ -13,7 +13,7 @@ import ( type RepositorySpecApplyConfiguration struct { Title *string `json:"title,omitempty"` Description *string `json:"description,omitempty"` - ReadOnly *bool `json:"readOnly,omitempty"` + Workflows []provisioningv0alpha1.Workflow `json:"workflows,omitempty"` Sync *SyncOptionsApplyConfiguration `json:"sync,omitempty"` Type *provisioningv0alpha1.RepositoryType `json:"type,omitempty"` Local *LocalRepositoryConfigApplyConfiguration `json:"local,omitempty"` @@ -42,11 +42,13 @@ func (b *RepositorySpecApplyConfiguration) WithDescription(value string) *Reposi return b } -// WithReadOnly sets the ReadOnly field in the declarative configuration to the given value -// and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the ReadOnly field is set to the value of the last call. -func (b *RepositorySpecApplyConfiguration) WithReadOnly(value bool) *RepositorySpecApplyConfiguration { - b.ReadOnly = &value +// WithWorkflows adds the given value to the Workflows field in the declarative configuration +// and returns the receiver, so that objects can be build by chaining "With" function invocations. +// If called multiple times, values provided by each call will be appended to the Workflows field. +func (b *RepositorySpecApplyConfiguration) WithWorkflows(values ...provisioningv0alpha1.Workflow) *RepositorySpecApplyConfiguration { + for i := range values { + b.Workflows = append(b.Workflows, values[i]) + } return b } diff --git a/pkg/registry/apis/provisioning/export.go b/pkg/registry/apis/provisioning/export.go index 29858a181ed..df108b0eedb 100644 --- a/pkg/registry/apis/provisioning/export.go +++ b/pkg/registry/apis/provisioning/export.go @@ -12,6 +12,7 @@ import ( provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" ) type exportConnector struct { @@ -52,11 +53,8 @@ func (c *exportConnector) Connect( return nil, err } cfg := repo.Config() - if cfg.Spec.ReadOnly { - return nil, &apierrors.StatusError{ErrStatus: v1.Status{ - Code: http.StatusPreconditionFailed, - Message: "Repository is read only", - }} + if err := repository.IsWriteAllowed(cfg, ""); err != nil { + return nil, err } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/registry/apis/provisioning/files.go b/pkg/registry/apis/provisioning/files.go index 2ac7eb90746..af07496f897 100644 --- a/pkg/registry/apis/provisioning/files.go +++ b/pkg/registry/apis/provisioning/files.go @@ -186,9 +186,8 @@ func (s *filesConnector) doRead(ctx context.Context, repo repository.Repository, } func (s *filesConnector) doWrite(ctx context.Context, update bool, repo repository.Repository, path string, ref string, message string, req *http.Request) (*provisioning.ResourceWrapper, error) { - if repo.Config().Spec.ReadOnly { - return nil, apierrors.NewForbidden(provisioning.RepositoryResourceInfo.GroupResource(), - "this repository is read only", nil) + if err := repository.IsWriteAllowed(repo.Config(), ref); err != nil { + return nil, err } defer func() { _ = req.Body.Close() }() @@ -276,9 +275,8 @@ func (s *filesConnector) doWrite(ctx context.Context, update bool, repo reposito } func (s *filesConnector) doDelete(ctx context.Context, repo repository.Repository, path string, ref string, message string) (*provisioning.ResourceWrapper, error) { - if repo.Config().Spec.ReadOnly { - return nil, apierrors.NewForbidden(provisioning.RepositoryResourceInfo.GroupResource(), - "this repository is read only", nil) + if err := repository.IsWriteAllowed(repo.Config(), ref); err != nil { + return nil, err } err := repo.Delete(ctx, path, ref, message) diff --git a/pkg/registry/apis/provisioning/jobs/export/worker.go b/pkg/registry/apis/provisioning/jobs/export/worker.go index 03b4bf976bd..05c7f9c7af1 100644 --- a/pkg/registry/apis/provisioning/jobs/export/worker.go +++ b/pkg/registry/apis/provisioning/jobs/export/worker.go @@ -48,15 +48,16 @@ func (r *ExportWorker) IsSupported(ctx context.Context, job provisioning.Job) bo // Process will start a job func (r *ExportWorker) Process(ctx context.Context, repo repository.Repository, job provisioning.Job, progress jobs.JobProgressRecorder) error { - if repo.Config().Spec.ReadOnly { - return errors.New("read only repository") - } - options := job.Spec.Export if options == nil { return errors.New("missing export settings") } + // Can write to external branch + if err := repository.IsWriteAllowed(repo.Config(), options.Branch); err != nil { + return err + } + var ( err error buffered *gogit.GoGitRepo diff --git a/pkg/registry/apis/provisioning/jobs/migrate/utils.go b/pkg/registry/apis/provisioning/jobs/migrate/utils.go index 3afcaf58126..f9c42b77ee7 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/utils.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/utils.go @@ -8,7 +8,8 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" ) -func verifyEmptyRepo(tree []repository.FileTreeEntry) error { +// VerifyEmptyRepo -- checks if there is anything useable in the tree +func VerifyEmptyRepo(tree []repository.FileTreeEntry) error { var folders []string var files []string diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 52d0e6267b6..39428a0a050 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -376,21 +376,14 @@ func (b *APIBuilder) Mutate(ctx context.Context, a admission.Attributes, o admis return fmt.Errorf("github configuration is required") } - if r.Spec.GitHub.Branch == "" { - r.Spec.GitHub.Branch = "main" - } - // Trim trailing slash or .git if len(r.Spec.GitHub.URL) > 5 { r.Spec.GitHub.URL = strings.TrimRight(strings.TrimRight(r.Spec.GitHub.URL, "/"), ".git") } + } - if len(r.Spec.GitHub.Workflows) == 0 { - r.Spec.GitHub.Workflows = []provisioning.Workflow{ - provisioning.BranchWorkflow, - provisioning.PushWorkflow, - } - } + if r.Spec.Workflows == nil { + r.Spec.Workflows = []provisioning.Workflow{} } if err := b.encryptSecrets(ctx, r); err != nil { diff --git a/pkg/registry/apis/provisioning/repository/github.go b/pkg/registry/apis/provisioning/repository/github.go index 3147e0c9f31..f6a2a5fbb3a 100644 --- a/pkg/registry/apis/provisioning/repository/github.go +++ b/pkg/registry/apis/provisioning/repository/github.go @@ -98,14 +98,6 @@ func (r *githubRepository) Validate() (list field.ErrorList) { list = append(list, field.Required(field.NewPath("spec", "github", "token"), "a github access token is required")) } - for _, w := range gh.Workflows { - switch w { - case provisioning.PushWorkflow, provisioning.BranchWorkflow: - default: - list = append(list, field.Invalid(field.NewPath("spec", "github", "workflow"), w, "invalid workflow")) - } - } - return list } @@ -262,9 +254,6 @@ func (r *githubRepository) Create(ctx context.Context, path, ref string, data [] ref = r.config.Spec.GitHub.Branch } ctx, _ = r.logger(ctx, ref) - if err := r.isWorkflowAllowed(ref); err != nil { - return err - } if err := r.ensureBranchExists(ctx, ref); err != nil { return fmt.Errorf("create branch on create: %w", err) @@ -297,9 +286,6 @@ func (r *githubRepository) Update(ctx context.Context, path, ref string, data [] ref = r.config.Spec.GitHub.Branch } ctx, _ = r.logger(ctx, ref) - if err := r.isWorkflowAllowed(ref); err != nil { - return err - } if err := r.ensureBranchExists(ctx, ref); err != nil { return fmt.Errorf("create branch on update: %w", err) @@ -333,9 +319,6 @@ func (r *githubRepository) Write(ctx context.Context, path string, ref string, d ref = r.config.Spec.GitHub.Branch } ctx, _ = r.logger(ctx, ref) - if err := r.isWorkflowAllowed(ref); err != nil { - return err - } return writeWithReadThenCreateOrUpdate(ctx, r, path, ref, data, message) } @@ -345,9 +328,6 @@ func (r *githubRepository) Delete(ctx context.Context, path, ref, comment string ref = r.config.Spec.GitHub.Branch } ctx, _ = r.logger(ctx, ref) - if err := r.isWorkflowAllowed(ref); err != nil { - return err - } if err := r.ensureBranchExists(ctx, ref); err != nil { return fmt.Errorf("create branch on delete: %w", err) @@ -440,25 +420,6 @@ func (r *githubRepository) History(ctx context.Context, path, ref string) ([]pro return ret, nil } -func (r *githubRepository) isWorkflowAllowed(ref string) error { - cfg := r.config.Spec - - workflows := make(map[provisioning.Workflow]bool) - for _, w := range cfg.GitHub.Workflows { - workflows[w] = true - } - - if ref == cfg.GitHub.Branch && !workflows[provisioning.PushWorkflow] { - return apierrors.NewBadRequest("direct push not allowed") - } - - if ref != cfg.GitHub.Branch && !workflows[provisioning.BranchWorkflow] { - return apierrors.NewBadRequest("branch push not allowed") - } - - return nil -} - // basicGitBranchNameRegex is a regular expression to validate a git branch name // it does not cover all cases as positive lookaheads are not supported in Go's regexp var basicGitBranchNameRegex = regexp.MustCompile(`^[a-zA-Z0-9\-\_\/\.]+$`) diff --git a/pkg/registry/apis/provisioning/repository/test.go b/pkg/registry/apis/provisioning/repository/test.go index 3f5c14c7d0f..8ba064ea08c 100644 --- a/pkg/registry/apis/provisioning/repository/test.go +++ b/pkg/registry/apis/provisioning/repository/test.go @@ -6,8 +6,9 @@ import ( "net/http" "slices" - provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "k8s.io/apimachinery/pkg/util/validation/field" + + provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" ) // Tester is a struct that implements the Tester interface @@ -70,5 +71,17 @@ func ValidateRepository(repo Repository) field.ErrorList { cfg.Spec.GitHub, "Github config only valid when type is github")) } + for _, w := range cfg.Spec.Workflows { + switch w { + case provisioning.WriteWorkflow: // valid; no fall thru + case provisioning.BranchWorkflow: + if cfg.Spec.Type != provisioning.GitHubRepositoryType { + list = append(list, field.Invalid(field.NewPath("spec", "workflow"), w, "branch is only supported on git repositories")) + } + default: + list = append(list, field.Invalid(field.NewPath("spec", "workflow"), w, "invalid workflow")) + } + } + return list } diff --git a/pkg/registry/apis/provisioning/repository/workflows.go b/pkg/registry/apis/provisioning/repository/workflows.go new file mode 100644 index 00000000000..2b98f2b438a --- /dev/null +++ b/pkg/registry/apis/provisioning/repository/workflows.go @@ -0,0 +1,40 @@ +package repository + +import ( + "net/http" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" +) + +func IsWriteAllowed(repo *provisioning.Repository, ref string) error { + if len(repo.Spec.Workflows) == 0 { + return apierrors.NewBadRequest("this repository is read only") + } + + if ref == "" { + for _, v := range repo.Spec.Workflows { + if v == provisioning.WriteWorkflow { + return nil // found + } + } + return apierrors.NewBadRequest("this repository does not support the write workflow") + } + + // Only github + if repo.Spec.Type != provisioning.GitHubRepositoryType { + return &apierrors.StatusError{ErrStatus: v1.Status{ + Code: http.StatusPreconditionFailed, + Message: "Only github supports writing to a branch", + }} + } + + for _, v := range repo.Spec.Workflows { + if v == provisioning.BranchWorkflow { + return nil + } + } + return apierrors.NewBadRequest("this repository does not support the branch workflow") +} diff --git a/pkg/registry/apis/provisioning/routes.go b/pkg/registry/apis/provisioning/routes.go index b9a0ddc77c6..d119ead9b98 100644 --- a/pkg/registry/apis/provisioning/routes.go +++ b/pkg/registry/apis/provisioning/routes.go @@ -155,7 +155,7 @@ func (b *APIBuilder) handleSettings(w http.ResponseWriter, r *http.Request) { Name: val.ObjectMeta.Name, Title: val.Spec.Title, Type: val.Spec.Type, - ReadOnly: val.Spec.ReadOnly, + ReadOnly: len(val.Spec.Workflows) == 0, Target: val.Spec.Sync.Target, } } diff --git a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json index cc0ac21790c..362ded043bb 100644 --- a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json @@ -2852,10 +2852,14 @@ }, "com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.GitHubRepositoryConfig": { "type": "object", + "required": [ + "branch" + ], "properties": { "branch": { - "description": "The branch to use in the repository. By default, this is the main branch.", - "type": "string" + "description": "The branch to use in the repository.", + "type": "string", + "default": "" }, "encryptedToken": { "description": "Token for accessing the repository, but encrypted. This is not possible to read back to a user decrypted.", @@ -2874,25 +2878,14 @@ "url": { "description": "The repository URL (e.g. `https://github.com/example/test`).", "type": "string" - }, - "workflows": { - "description": "Workflow allowed for changes to the repository. The order is relevant for defining the precedence of the workflows. Possible values: pull-request, branch, push.", - "type": "array", - "items": { - "type": "string", - "default": "", - "enum": [ - "branch", - "push" - ] - } } } }, "com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.HealthStatus": { "type": "object", "required": [ - "healthy" + "healthy", + "message" ], "properties": { "checked": { @@ -3396,7 +3389,7 @@ "type": "object", "required": [ "title", - "readOnly", + "workflows", "sync", "type" ], @@ -3421,11 +3414,6 @@ } ] }, - "readOnly": { - "description": "ReadOnly repository does not allow any write commands", - "type": "boolean", - "default": false - }, "sync": { "description": "Sync settings -- how values are pulled from the repository into grafana", "default": {}, @@ -3448,6 +3436,18 @@ "github", "local" ] + }, + "workflows": { + "description": "UI driven Workflow taht allow changes to the contends of the repository. The order is relevant for defining the precedence of the workflows. When empty, the repository does not support any edits (eg, readonly)", + "type": "array", + "items": { + "type": "string", + "default": "", + "enum": [ + "branch", + "write" + ] + } } } }, diff --git a/pkg/tests/apis/provisioning/provisioning_test.go b/pkg/tests/apis/provisioning/provisioning_test.go index fba70677206..ea2694a1e19 100644 --- a/pkg/tests/apis/provisioning/provisioning_test.go +++ b/pkg/tests/apis/provisioning/provisioning_test.go @@ -12,6 +12,8 @@ import ( "testing" "time" + gh "github.com/google/go-github/v69/github" + ghmock "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/exp/rand" @@ -19,7 +21,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - gh "github.com/google/go-github/v69/github" dashboard "github.com/grafana/grafana/pkg/apis/dashboard/v1alpha1" folder "github.com/grafana/grafana/pkg/apis/folder/v0alpha1" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" @@ -29,7 +30,6 @@ import ( "github.com/grafana/grafana/pkg/tests/apis" "github.com/grafana/grafana/pkg/tests/testinfra" "github.com/grafana/grafana/pkg/tests/testsuite" - ghmock "github.com/migueleliasweb/go-github-mock/src/mock" ) func TestMain(m *testing.M) { @@ -177,17 +177,26 @@ func TestIntegrationProvisioning(t *testing.T) { } { t.Run(inputFilePath, func(t *testing.T) { input := helper.LoadYAMLOrJSONFile(inputFilePath) - expectedOutput, err := json.MarshalIndent(input.Object["spec"], "", " ") - require.NoError(t, err, "failed to marshal JSON from input spec") - _, err = client.Resource.Create(ctx, input, createOptions) + _, err := client.Resource.Create(ctx, input, createOptions) require.NoError(t, err, "failed to create resource") output, err := client.Resource.Get(ctx, mustNestedString(input.Object, "metadata", "name"), metav1.GetOptions{}) require.NoError(t, err, "failed to read back resource") + + // Move encrypted token mutation + token, found, err := unstructured.NestedString(output.Object, "spec", "github", "encryptedToken") + require.NoError(t, err, "encryptedToken is not a string") + if found { + unstructured.RemoveNestedField(input.Object, "spec", "github", "token") + err = unstructured.SetNestedField(input.Object, token, "spec", "github", "encryptedToken") + require.NoError(t, err, "unable to copy encrypted token") + } + + expectedOutput, err := json.MarshalIndent(input.Object["spec"], "", " ") + require.NoError(t, err, "failed to marshal JSON from input spec") outputJSON, err := json.MarshalIndent(output.Object["spec"], "", " ") require.NoError(t, err, "failed to marshal JSON from read back resource") - require.JSONEq(t, string(expectedOutput), string(outputJSON)) }) } diff --git a/pkg/tests/apis/provisioning/testdata/github-example.json b/pkg/tests/apis/provisioning/testdata/github-example.json index 85475eb8613..160db9fbf35 100644 --- a/pkg/tests/apis/provisioning/testdata/github-example.json +++ b/pkg/tests/apis/provisioning/testdata/github-example.json @@ -8,7 +8,7 @@ "title": "Github Example", "description": "load resources from github", "type": "github", - "readOnly": false, + "workflows": ["branch", "write"], "sync": { "enabled": true, "target": "folder", @@ -17,7 +17,6 @@ "github": { "url": "https://github.com/grafana/git-ui-sync-demo", "branch": "dummy-branch", - "workflows": ["push", "branch"], "generateDashboardPreviews": true, "token": "github_pat_dummy" } diff --git a/pkg/tests/apis/provisioning/testdata/local-conf-provisioning-sample.json b/pkg/tests/apis/provisioning/testdata/local-conf-provisioning-sample.json index cff173b0819..f8d99b52945 100644 --- a/pkg/tests/apis/provisioning/testdata/local-conf-provisioning-sample.json +++ b/pkg/tests/apis/provisioning/testdata/local-conf-provisioning-sample.json @@ -11,7 +11,7 @@ }, "title": "Config provisioning files", "type": "local", - "readOnly": false, + "workflows": ["write"], "sync": { "enabled": false, "target": "folder", diff --git a/pkg/tests/apis/provisioning/testdata/local-devenv.json b/pkg/tests/apis/provisioning/testdata/local-devenv.json index 6130961f9c2..6a7dd5e9133 100644 --- a/pkg/tests/apis/provisioning/testdata/local-devenv.json +++ b/pkg/tests/apis/provisioning/testdata/local-devenv.json @@ -12,7 +12,7 @@ "target": "folder", "intervalSeconds": 60 }, - "readOnly": false, + "workflows": ["write"], "type": "local", "local": { "path": "devenv/dev-dashboards" diff --git a/pkg/tests/apis/provisioning/testdata/local-tmp.json b/pkg/tests/apis/provisioning/testdata/local-tmp.json index 7bb254bba14..6aed89a005e 100644 --- a/pkg/tests/apis/provisioning/testdata/local-tmp.json +++ b/pkg/tests/apis/provisioning/testdata/local-tmp.json @@ -14,7 +14,6 @@ "target": "folder", "intervalSeconds": 60 }, - "readOnly": false, "title": "Load tmp dashboards", "type": "local" } diff --git a/pkg/tests/apis/provisioning/testdata/local-xxx.json b/pkg/tests/apis/provisioning/testdata/local-xxx.json index 866d6c73931..a20d30460f6 100644 --- a/pkg/tests/apis/provisioning/testdata/local-xxx.json +++ b/pkg/tests/apis/provisioning/testdata/local-xxx.json @@ -9,12 +9,12 @@ "local": { "path": "conf/provisioning/sample" }, + "workflows": [], "sync": { "enabled": false, "target": "folder", "intervalSeconds": 60 }, - "readOnly": false, "title": "Config provisioning files", "type": "local" } diff --git a/public/app/features/provisioning/ConfigForm.tsx b/public/app/features/provisioning/ConfigForm.tsx index db40f170c55..3d87f26d3a0 100644 --- a/public/app/features/provisioning/ConfigForm.tsx +++ b/public/app/features/provisioning/ConfigForm.tsx @@ -31,10 +31,16 @@ const targetOptions = [ { value: 'folder', label: 'Managed folder' }, ]; -const workflowOptions: Array> = [ - { label: 'Push', value: 'push' }, - { label: 'Branch', value: 'branch' }, -]; +export function GetWorkflowOptions(type?: 'github' | 'local'): Array> { + const opts: Array> = [ + { label: 'Branch', value: 'branch', description: 'Create a branch (and pull request) for changes' }, + { label: 'Write', value: 'write', description: 'Allow writing updates to the remore repository' }, + ]; + if (type === 'github') { + return opts; + } + return [opts[2]]; // only write +} const appEvents = getAppEvents(); @@ -47,13 +53,12 @@ export function getDefaultValues(repository?: RepositorySpec): RepositoryFormDat url: '', branch: 'main', generateDashboardPreviews: true, - workflows: ['push', 'branch'], + workflows: ['write'], // 'branch' if github sync: { enabled: false, target: 'instance', intervalSeconds: 60, }, - readOnly: false, }; } return specToData(repository); @@ -178,17 +183,16 @@ export function ConfigForm({ data }: ConfigFormProps) { - - ( - - )} - /> - - + + Render before/after images and link them to the pull request. +
+ NOTE! these will be public URLs!!!" + + } + > {/* The lint option is intentionally not presented here, as it's an experimental feature. */} @@ -201,6 +205,23 @@ export function ConfigForm({ data }: ConfigFormProps) { )} + + ( + + )} + /> + +
@@ -226,11 +247,6 @@ export function ConfigForm({ data }: ConfigFormProps) {
-
- - - -