diff --git a/apps/provisioning/pkg/jobs/validator.go b/apps/provisioning/pkg/jobs/validator.go new file mode 100644 index 00000000000..77232490b66 --- /dev/null +++ b/apps/provisioning/pkg/jobs/validator.go @@ -0,0 +1,172 @@ +package jobs + +import ( + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/validation/field" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/apps/provisioning/pkg/repository/git" + "github.com/grafana/grafana/apps/provisioning/pkg/safepath" +) + +// ValidateJob performs validation on the Job specification and returns an error if validation fails +func ValidateJob(job *provisioning.Job) error { + list := field.ErrorList{} + + // Validate action is specified + if job.Spec.Action == "" { + list = append(list, field.Required(field.NewPath("spec", "action"), "action must be specified")) + return toError(job.Name, list) // Early return since we can't validate further without knowing the action + } + + // Validate repository is specified + if job.Spec.Repository == "" { + list = append(list, field.Required(field.NewPath("spec", "repository"), "repository must be specified")) + } + + // Validate action-specific options + switch job.Spec.Action { + case provisioning.JobActionPull: + if job.Spec.Pull == nil { + list = append(list, field.Required(field.NewPath("spec", "pull"), "pull options required for pull action")) + } + // Pull options are simple, just incremental bool - no further validation needed + + case provisioning.JobActionPush: + if job.Spec.Push == nil { + list = append(list, field.Required(field.NewPath("spec", "push"), "push options required for push action")) + } else { + list = append(list, validateExportJobOptions(job.Spec.Push)...) + } + + case provisioning.JobActionPullRequest: + if job.Spec.PullRequest == nil { + list = append(list, field.Required(field.NewPath("spec", "pr"), "pull request options required for pr action")) + } + // PullRequest options are mostly informational - no strict validation needed + + case provisioning.JobActionMigrate: + if job.Spec.Migrate == nil { + list = append(list, field.Required(field.NewPath("spec", "migrate"), "migrate options required for migrate action")) + } + // Migrate options are simple - no further validation needed + + case provisioning.JobActionDelete: + if job.Spec.Delete == nil { + list = append(list, field.Required(field.NewPath("spec", "delete"), "delete options required for delete action")) + } else { + list = append(list, validateDeleteJobOptions(job.Spec.Delete)...) + } + + case provisioning.JobActionMove: + if job.Spec.Move == nil { + list = append(list, field.Required(field.NewPath("spec", "move"), "move options required for move action")) + } else { + list = append(list, validateMoveJobOptions(job.Spec.Move)...) + } + default: + list = append(list, field.Invalid(field.NewPath("spec", "action"), job.Spec.Action, "invalid action")) + } + + return toError(job.Name, list) +} + +// toError converts a field.ErrorList to an error, returning nil if the list is empty +func toError(name string, list field.ErrorList) error { + if len(list) == 0 { + return nil + } + return apierrors.NewInvalid( + provisioning.JobResourceInfo.GroupVersionKind().GroupKind(), + name, list) +} + +// validateExportJobOptions validates export (push) job options +func validateExportJobOptions(opts *provisioning.ExportJobOptions) field.ErrorList { + list := field.ErrorList{} + + // Validate branch name if specified + if opts.Branch != "" { + if !git.IsValidGitBranchName(opts.Branch) { + list = append(list, field.Invalid(field.NewPath("spec", "push", "branch"), opts.Branch, "invalid git branch name")) + } + } + + // Validate path if specified + if opts.Path != "" { + if err := safepath.IsSafe(opts.Path); err != nil { + list = append(list, field.Invalid(field.NewPath("spec", "push", "path"), opts.Path, err.Error())) + } + } + + return list +} + +// validateDeleteJobOptions validates delete job options +func validateDeleteJobOptions(opts *provisioning.DeleteJobOptions) field.ErrorList { + list := field.ErrorList{} + + // At least one of paths or resources must be specified + if len(opts.Paths) == 0 && len(opts.Resources) == 0 { + list = append(list, field.Required(field.NewPath("spec", "delete"), "at least one path or resource must be specified")) + return list + } + + // Validate paths + for i, p := range opts.Paths { + if err := safepath.IsSafe(p); err != nil { + list = append(list, field.Invalid(field.NewPath("spec", "delete", "paths").Index(i), p, err.Error())) + } + } + + // Validate resources + for i, r := range opts.Resources { + if r.Name == "" { + list = append(list, field.Required(field.NewPath("spec", "delete", "resources").Index(i).Child("name"), "resource name is required")) + } + if r.Kind == "" { + list = append(list, field.Required(field.NewPath("spec", "delete", "resources").Index(i).Child("kind"), "resource kind is required")) + } + } + + return list +} + +// validateMoveJobOptions validates move job options +func validateMoveJobOptions(opts *provisioning.MoveJobOptions) field.ErrorList { + list := field.ErrorList{} + + // At least one of paths or resources must be specified + if len(opts.Paths) == 0 && len(opts.Resources) == 0 { + list = append(list, field.Required(field.NewPath("spec", "move"), "at least one path or resource must be specified")) + return list + } + + // Target path is required + if opts.TargetPath == "" { + list = append(list, field.Required(field.NewPath("spec", "move", "targetPath"), "target path is required")) + } else { + if err := safepath.IsSafe(opts.TargetPath); err != nil { + list = append(list, field.Invalid(field.NewPath("spec", "move", "targetPath"), opts.TargetPath, err.Error())) + } + } + + // Validate source paths + for i, p := range opts.Paths { + if err := safepath.IsSafe(p); err != nil { + list = append(list, field.Invalid(field.NewPath("spec", "move", "paths").Index(i), p, err.Error())) + } + } + + // Validate resources + for i, r := range opts.Resources { + if r.Name == "" { + list = append(list, field.Required(field.NewPath("spec", "move", "resources").Index(i).Child("name"), "resource name is required")) + } + if r.Kind == "" { + list = append(list, field.Required(field.NewPath("spec", "move", "resources").Index(i).Child("kind"), "resource kind is required")) + } + } + + return list +} diff --git a/apps/provisioning/pkg/jobs/validator_test.go b/apps/provisioning/pkg/jobs/validator_test.go new file mode 100644 index 00000000000..fdd29598ebc --- /dev/null +++ b/apps/provisioning/pkg/jobs/validator_test.go @@ -0,0 +1,593 @@ +package jobs + +import ( + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" +) + +func TestValidateJob(t *testing.T) { + tests := []struct { + name string + job *provisioning.Job + wantErr bool + validateError func(t *testing.T, err error) + }{ + { + name: "valid pull job", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionPull, + Repository: "test-repo", + Pull: &provisioning.SyncJobOptions{ + Incremental: true, + }, + }, + }, + wantErr: false, + }, + { + name: "missing action", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Repository: "test-repo", + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.action: Required value") + }, + }, + { + name: "invalid action", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobAction("invalid"), + Repository: "test-repo", + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.action: Invalid value") + }, + }, + { + name: "missing repository", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionPull, + Pull: &provisioning.SyncJobOptions{ + Incremental: true, + }, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.repository: Required value") + }, + }, + { + name: "pull action without pull options", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionPull, + Repository: "test-repo", + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.pull: Required value") + }, + }, + { + name: "push action without push options", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionPush, + Repository: "test-repo", + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.push: Required value") + }, + }, + { + name: "valid push job with valid branch", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionPush, + Repository: "test-repo", + Push: &provisioning.ExportJobOptions{ + Branch: "main", + Message: "Test commit", + }, + }, + }, + wantErr: false, + }, + { + name: "push job with invalid branch name", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionPush, + Repository: "test-repo", + Push: &provisioning.ExportJobOptions{ + Branch: "feature..branch", // Invalid: contains consecutive dots + Message: "Test commit", + }, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.push.branch") + require.Contains(t, err.Error(), "invalid git branch name") + }, + }, + { + name: "push job with invalid path", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionPush, + Repository: "test-repo", + Push: &provisioning.ExportJobOptions{ + Path: "../../../etc/passwd", // Invalid: path traversal + Message: "Test commit", + }, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.push.path") + }, + }, + { + name: "delete action without options", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Repository: "test-repo", + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.delete: Required value") + }, + }, + { + name: "delete action without paths or resources", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Repository: "test-repo", + Delete: &provisioning.DeleteJobOptions{}, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "at least one path or resource must be specified") + }, + }, + { + name: "valid delete action with paths", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Repository: "test-repo", + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"dashboard.json", "folder/other.json"}, + }, + }, + }, + wantErr: false, + }, + { + name: "valid delete action with resources", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Repository: "test-repo", + Delete: &provisioning.DeleteJobOptions{ + Resources: []provisioning.ResourceRef{ + { + Name: "my-dashboard", + Kind: "Dashboard", + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "delete action with invalid path", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Repository: "test-repo", + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"../../etc/passwd"}, // Invalid: path traversal + }, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.delete.paths[0]") + }, + }, + { + name: "delete action with resource missing name", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Repository: "test-repo", + Delete: &provisioning.DeleteJobOptions{ + Resources: []provisioning.ResourceRef{ + { + Kind: "Dashboard", + }, + }, + }, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.delete.resources[0].name") + }, + }, + { + name: "move action without options", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Repository: "test-repo", + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.move: Required value") + }, + }, + { + name: "move action without paths or resources", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Repository: "test-repo", + Move: &provisioning.MoveJobOptions{ + TargetPath: "new-location/", + }, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "at least one path or resource must be specified") + }, + }, + { + name: "move action without target path", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Repository: "test-repo", + Move: &provisioning.MoveJobOptions{ + Paths: []string{"dashboard.json"}, + }, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.move.targetPath: Required value") + }, + }, + { + name: "valid move action", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Repository: "test-repo", + Move: &provisioning.MoveJobOptions{ + Paths: []string{"old-location/dashboard.json"}, + TargetPath: "new-location/", + }, + }, + }, + wantErr: false, + }, + { + name: "move action with invalid target path", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Repository: "test-repo", + Move: &provisioning.MoveJobOptions{ + Paths: []string{"dashboard.json"}, + TargetPath: "../../../etc/", // Invalid: path traversal + }, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.move.targetPath") + }, + }, + { + name: "valid migrate job", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMigrate, + Repository: "test-repo", + Migrate: &provisioning.MigrateJobOptions{ + History: true, + Message: "Migrate from legacy", + }, + }, + }, + wantErr: false, + }, + { + name: "migrate action without migrate options", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMigrate, + Repository: "test-repo", + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.migrate: Required value") + }, + }, + { + name: "valid pr job", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionPullRequest, + Repository: "test-repo", + PullRequest: &provisioning.PullRequestJobOptions{ + PR: 123, + Ref: "refs/pull/123/head", + }, + }, + }, + wantErr: false, + }, + { + name: "delete action with resource missing kind", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Repository: "test-repo", + Delete: &provisioning.DeleteJobOptions{ + Resources: []provisioning.ResourceRef{ + { + Name: "my-dashboard", + }, + }, + }, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.delete.resources[0].kind") + }, + }, + { + name: "move action with valid resources", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Repository: "test-repo", + Move: &provisioning.MoveJobOptions{ + Resources: []provisioning.ResourceRef{ + { + Name: "my-dashboard", + Kind: "Dashboard", + }, + }, + TargetPath: "new-location/", + }, + }, + }, + wantErr: false, + }, + { + name: "move action with resource missing kind", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Repository: "test-repo", + Move: &provisioning.MoveJobOptions{ + Resources: []provisioning.ResourceRef{ + { + Name: "my-dashboard", + }, + }, + TargetPath: "new-location/", + }, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.move.resources[0].kind") + }, + }, + { + name: "move action with both paths and resources", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Repository: "test-repo", + Move: &provisioning.MoveJobOptions{ + Paths: []string{"dashboard.json"}, + Resources: []provisioning.ResourceRef{ + { + Name: "my-dashboard", + Kind: "Dashboard", + }, + }, + TargetPath: "new-location/", + }, + }, + }, + wantErr: false, + }, + { + name: "move action with invalid source path", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Repository: "test-repo", + Move: &provisioning.MoveJobOptions{ + Paths: []string{"../invalid/path"}, + TargetPath: "valid/target/", + }, + }, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "spec.move.paths[0]") + }, + }, + { + name: "delete action with both paths and resources", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Repository: "test-repo", + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"dashboard.json"}, + Resources: []provisioning.ResourceRef{ + { + Name: "my-dashboard", + Kind: "Dashboard", + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "push action with valid path", + job: &provisioning.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + Spec: provisioning.JobSpec{ + Action: provisioning.JobActionPush, + Repository: "test-repo", + Push: &provisioning.ExportJobOptions{ + Path: "some/valid/path", + Message: "Test commit", + }, + }, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateJob(tt.job) + if tt.wantErr { + require.Error(t, err) + if tt.validateError != nil { + tt.validateError(t, err) + } + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 9899ac2a0da..13104e3e2f5 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -35,6 +35,7 @@ import ( clientset "github.com/grafana/grafana/apps/provisioning/pkg/generated/clientset/versioned" client "github.com/grafana/grafana/apps/provisioning/pkg/generated/clientset/versioned/typed/provisioning/v0alpha1" informers "github.com/grafana/grafana/apps/provisioning/pkg/generated/informers/externalversions" + jobsvalidation "github.com/grafana/grafana/apps/provisioning/pkg/jobs" "github.com/grafana/grafana/apps/provisioning/pkg/loki" "github.com/grafana/grafana/apps/provisioning/pkg/repository" "github.com/grafana/grafana/pkg/apimachinery/identity" @@ -576,10 +577,10 @@ func (b *APIBuilder) Validate(ctx context.Context, a admission.Attributes, o adm return nil } - // FIXME: Do nothing for Jobs for now - _, ok = obj.(*provisioning.Job) + // Validate Jobs + job, ok := obj.(*provisioning.Job) if ok { - return nil + return jobsvalidation.ValidateJob(job) } repo, err := b.asRepository(ctx, obj, a.GetOldObject()) diff --git a/pkg/tests/apis/provisioning/job_validation_test.go b/pkg/tests/apis/provisioning/job_validation_test.go new file mode 100644 index 00000000000..c54ad58500c --- /dev/null +++ b/pkg/tests/apis/provisioning/job_validation_test.go @@ -0,0 +1,182 @@ +package provisioning + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/util/testutil" +) + +func TestIntegrationProvisioning_JobValidation(t *testing.T) { + testutil.SkipIntegrationTestInShortMode(t) + + helper := runGrafana(t) + ctx := context.Background() + + // Create a test repository first + const repo = "job-validation-test-repo" + testRepo := TestRepo{ + Name: repo, + Target: "instance", + Copies: map[string]string{}, + ExpectedDashboards: 0, + ExpectedFolders: 0, + } + helper.CreateRepo(t, testRepo) + + tests := []struct { + name string + jobSpec map[string]interface{} + expectedErr string + }{ + { + name: "job without action", + jobSpec: map[string]interface{}{ + "repository": repo, + }, + expectedErr: "spec.action: Required value: action must be specified", + }, + { + name: "job with invalid action", + jobSpec: map[string]interface{}{ + "action": "invalid-action", + "repository": repo, + }, + expectedErr: "spec.action: Invalid value: \"invalid-action\": invalid action", + }, + { + name: "pull job without pull options", + jobSpec: map[string]interface{}{ + "action": string(provisioning.JobActionPull), + "repository": repo, + }, + expectedErr: "spec.pull: Required value: pull options required for pull action", + }, + { + name: "push job without push options", + jobSpec: map[string]interface{}{ + "action": string(provisioning.JobActionPush), + "repository": repo, + }, + expectedErr: "spec.push: Required value: push options required for push action", + }, + { + name: "push job with invalid branch name", + jobSpec: map[string]interface{}{ + "action": string(provisioning.JobActionPush), + "repository": repo, + "push": map[string]interface{}{ + "branch": "feature..branch", // Invalid: consecutive dots + "message": "Test commit", + }, + }, + expectedErr: "spec.push.branch: Invalid value: \"feature..branch\": invalid git branch name", + }, + { + name: "push job with path traversal", + jobSpec: map[string]interface{}{ + "action": string(provisioning.JobActionPush), + "repository": repo, + "push": map[string]interface{}{ + "path": "../../etc/passwd", // Invalid: path traversal + "message": "Test commit", + }, + }, + expectedErr: "spec.push.path: Invalid value: \"../../etc/passwd\"", + }, + { + name: "delete job without paths or resources", + jobSpec: map[string]interface{}{ + "action": string(provisioning.JobActionDelete), + "repository": repo, + "delete": map[string]interface{}{}, + }, + expectedErr: "spec.delete: Required value: at least one path or resource must be specified", + }, + { + name: "delete job with invalid path", + jobSpec: map[string]interface{}{ + "action": string(provisioning.JobActionDelete), + "repository": repo, + "delete": map[string]interface{}{ + "paths": []string{"../invalid/path"}, + }, + }, + expectedErr: "spec.delete.paths[0]: Invalid value: \"../invalid/path\"", + }, + { + name: "move job without target path", + jobSpec: map[string]interface{}{ + "action": string(provisioning.JobActionMove), + "repository": repo, + "move": map[string]interface{}{ + "paths": []string{"dashboard.json"}, + }, + }, + expectedErr: "spec.move.targetPath: Required value: target path is required", + }, + { + name: "move job without paths or resources", + jobSpec: map[string]interface{}{ + "action": string(provisioning.JobActionMove), + "repository": repo, + "move": map[string]interface{}{ + "targetPath": "new-location/", + }, + }, + expectedErr: "spec.move: Required value: at least one path or resource must be specified", + }, + { + name: "move job with invalid target path", + jobSpec: map[string]interface{}{ + "action": string(provisioning.JobActionMove), + "repository": repo, + "move": map[string]interface{}{ + "paths": []string{"dashboard.json"}, + "targetPath": "../../../etc/", // Invalid: path traversal + }, + }, + expectedErr: "spec.move.targetPath: Invalid value: \"../../../etc/\"", + }, + { + name: "migrate job without migrate options", + jobSpec: map[string]interface{}{ + "action": string(provisioning.JobActionMigrate), + "repository": repo, + }, + expectedErr: "spec.migrate: Required value: migrate options required for migrate action", + }, + } + + for i, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create the job object directly + jobObj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "provisioning.grafana.app/v0alpha1", + "kind": "Job", + "metadata": map[string]interface{}{ + "name": fmt.Sprintf("test-job-validation-%d", i), + "namespace": "default", + }, + "spec": tt.jobSpec, + }, + } + + // Try to create the job - should fail with validation error + _, err := helper.Jobs.Resource.Create(ctx, jobObj, metav1.CreateOptions{}) + require.Error(t, err, "expected validation error for invalid job spec") + + // Verify it's a validation error with correct status code + statusError := helper.RequireApiErrorStatus(err, metav1.StatusReasonInvalid, http.StatusUnprocessableEntity) + require.Contains(t, statusError.Message, tt.expectedErr, "error message should contain expected validation message") + }) + } +} diff --git a/pkg/tests/apis/provisioning/movejob_test.go b/pkg/tests/apis/provisioning/movejob_test.go index d7a6c4e7f44..85d227996fd 100644 --- a/pkg/tests/apis/provisioning/movejob_test.go +++ b/pkg/tests/apis/provisioning/movejob_test.go @@ -171,7 +171,7 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) { }) t.Run("move without target path", func(t *testing.T) { - // Create move job without target path (should fail) + // Create move job without target path (should fail validation at creation time) spec := provisioning.JobSpec{ Action: provisioning.JobActionMove, Move: &provisioning.MoveJobOptions{ @@ -180,9 +180,20 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) { }, } - job := helper.TriggerJobAndWaitForComplete(t, repo, spec) - state := mustNestedString(job.Object, "status", "state") - assert.Equal(t, "error", state, "move job should have failed due to missing target path") + // The job should be rejected by the admission controller with validation error + body := asJSON(&spec) + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(body). + SetHeader("Content-Type", "application/json"). + Do(ctx) + + require.Error(t, result.Error(), "move job without target path should fail validation") + statusError := helper.RequireApiErrorStatus(result.Error(), metav1.StatusReasonInvalid, 422) + require.Contains(t, statusError.Message, "spec.move.targetPath", "error should mention missing target path") }) t.Run("move by resource reference", func(t *testing.T) {