diff --git a/pkg/registry/apis/provisioning/files.go b/pkg/registry/apis/provisioning/files.go index f42eb8921d7..19edd69bc16 100644 --- a/pkg/registry/apis/provisioning/files.go +++ b/pkg/registry/apis/provisioning/files.go @@ -92,25 +92,28 @@ func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime. return withTimeout(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { query := r.URL.Query() - ref := query.Get("ref") - message := query.Get("message") - logger := logger.With("url", r.URL.Path, "ref", ref, "message", message) + opts := resources.DualWriteOptions{ + Ref: query.Get("ref"), + Message: query.Get("message"), + SkipDryRun: query.Get("skipDryRun") == "true", + } + logger := logger.With("url", r.URL.Path, "ref", opts.Ref, "message", opts.Message) ctx := logging.Context(r.Context(), logger) - filePath, err := pathAfterPrefix(r.URL.Path, fmt.Sprintf("/%s/files", name)) + opts.Path, err = pathAfterPrefix(r.URL.Path, fmt.Sprintf("/%s/files", name)) if err != nil { responder.Error(apierrors.NewBadRequest(err.Error())) return } - if err := resources.IsPathSupported(filePath); err != nil { + if err := resources.IsPathSupported(opts.Path); err != nil { responder.Error(apierrors.NewBadRequest(err.Error())) return } - isDir := safepath.IsDir(filePath) + isDir := safepath.IsDir(opts.Path) if r.Method == http.MethodGet && isDir { - files, err := c.listFolderFiles(ctx, filePath, ref, readWriter) + files, err := c.listFolderFiles(ctx, opts.Path, opts.Ref, readWriter) if err != nil { responder.Error(err) return @@ -120,7 +123,7 @@ func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime. return } - if filePath == "" { + if opts.Path == "" { responder.Error(apierrors.NewBadRequest("missing request path")) return } @@ -135,7 +138,7 @@ func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime. code := http.StatusOK switch r.Method { case http.MethodGet: - resource, err := dualReadWriter.Read(ctx, filePath, ref) + resource, err := dualReadWriter.Read(ctx, opts.Path, opts.Ref) if err != nil { responder.Error(err) return @@ -143,15 +146,15 @@ func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime. obj = resource.AsResourceWrapper() case http.MethodPost: if isDir { - obj, err = dualReadWriter.CreateFolder(ctx, filePath, ref, message) + obj, err = dualReadWriter.CreateFolder(ctx, opts) } else { - data, err := readBody(r, filesMaxBodySize) + opts.Data, err = readBody(r, filesMaxBodySize) if err != nil { responder.Error(err) return } - resource, err := dualReadWriter.CreateResource(ctx, filePath, ref, message, data) + resource, err := dualReadWriter.CreateResource(ctx, opts) if err != nil { responder.Error(err) return @@ -163,13 +166,13 @@ func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime. if isDir { err = apierrors.NewMethodNotSupported(provisioning.RepositoryResourceInfo.GroupResource(), r.Method) } else { - data, err := readBody(r, filesMaxBodySize) + opts.Data, err = readBody(r, filesMaxBodySize) if err != nil { responder.Error(err) return } - resource, err := dualReadWriter.UpdateResource(ctx, filePath, ref, message, data) + resource, err := dualReadWriter.UpdateResource(ctx, opts) if err != nil { responder.Error(err) return @@ -177,7 +180,7 @@ func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime. obj = resource.AsResourceWrapper() } case http.MethodDelete: - resource, err := dualReadWriter.Delete(ctx, filePath, ref, message) + resource, err := dualReadWriter.Delete(ctx, opts) if err != nil { responder.Error(err) return diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index bccff626351..2f6ed45cba3 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -726,6 +726,15 @@ func (b *APIBuilder) PostProcessOpenAPI(oas *spec3.OpenAPI) (*spec3.OpenAPI, err Required: false, }, }, + { + ParameterProps: spec3.ParameterProps{ + Name: "skipDryRun", + In: "query", + Description: "do not pro-actively verify the payload", + Schema: spec.BooleanProperty(), + Required: false, + }, + }, } sub.Delete.Parameters = comment sub.Post.Parameters = comment diff --git a/pkg/registry/apis/provisioning/resources/dualwriter.go b/pkg/registry/apis/provisioning/resources/dualwriter.go index bfd62fe0db3..b9f6e71c3a7 100644 --- a/pkg/registry/apis/provisioning/resources/dualwriter.go +++ b/pkg/registry/apis/provisioning/resources/dualwriter.go @@ -26,6 +26,14 @@ type DualReadWriter struct { access authlib.AccessChecker } +type DualWriteOptions struct { + Path string + Ref string + Message string + Data []byte + SkipDryRun bool +} + func NewDualReadWriter(repo repository.ReaderWriter, parser Parser, folders *FolderManager, access authlib.AccessChecker) *DualReadWriter { return &DualReadWriter{repo: repo, parser: parser, folders: folders, access: access} } @@ -63,17 +71,17 @@ func (r *DualReadWriter) Read(ctx context.Context, path string, ref string) (*Pa return parsed, nil } -func (r *DualReadWriter) Delete(ctx context.Context, path string, ref string, message string) (*ParsedResource, error) { - if err := repository.IsWriteAllowed(r.repo.Config(), ref); err != nil { +func (r *DualReadWriter) Delete(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) { + if err := repository.IsWriteAllowed(r.repo.Config(), opts.Ref); err != nil { return nil, err } // TODO: implement this - if safepath.IsDir(path) { + if safepath.IsDir(opts.Path) { return nil, fmt.Errorf("folder delete not supported") } - file, err := r.repo.Read(ctx, path, ref) + file, err := r.repo.Read(ctx, opts.Path, opts.Ref) if err != nil { return nil, fmt.Errorf("read file: %w", err) } @@ -90,13 +98,13 @@ func (r *DualReadWriter) Delete(ctx context.Context, path string, ref string, me } parsed.Action = provisioning.ResourceActionDelete - err = r.repo.Delete(ctx, path, ref, message) + err = r.repo.Delete(ctx, opts.Path, opts.Ref, opts.Message) if err != nil { return nil, fmt.Errorf("delete file from repository: %w", err) } // Delete the file in the grafana database - if ref == "" { + if opts.Ref == "" { ctx, _, err := identity.WithProvisioningIdentity(ctx, parsed.Obj.GetNamespace()) if err != nil { return parsed, err @@ -119,28 +127,28 @@ func (r *DualReadWriter) Delete(ctx context.Context, path string, ref string, me // CreateFolder creates a new folder in the repository // FIXME: fix signature to return ParsedResource -func (r *DualReadWriter) CreateFolder(ctx context.Context, path string, ref string, message string) (*provisioning.ResourceWrapper, error) { - if err := repository.IsWriteAllowed(r.repo.Config(), ref); err != nil { +func (r *DualReadWriter) CreateFolder(ctx context.Context, opts DualWriteOptions) (*provisioning.ResourceWrapper, error) { + if err := repository.IsWriteAllowed(r.repo.Config(), opts.Ref); err != nil { return nil, err } - if !safepath.IsDir(path) { + if !safepath.IsDir(opts.Path) { return nil, fmt.Errorf("not a folder path") } - if err := r.authorizeCreateFolder(ctx, path); err != nil { + if err := r.authorizeCreateFolder(ctx, opts.Path); err != nil { return nil, err } // Now actually create the folder - if err := r.repo.Create(ctx, path, ref, nil, message); err != nil { + if err := r.repo.Create(ctx, opts.Path, opts.Ref, nil, opts.Message); err != nil { return nil, fmt.Errorf("failed to create folder: %w", err) } cfg := r.repo.Config() wrap := &provisioning.ResourceWrapper{ - Path: path, - Ref: ref, + Path: opts.Path, + Ref: opts.Ref, Repository: provisioning.ResourceRepositoryInfo{ Type: cfg.Spec.Type, Namespace: cfg.Namespace, @@ -152,8 +160,8 @@ func (r *DualReadWriter) CreateFolder(ctx context.Context, path string, ref stri }, } - if ref == "" { - folderName, err := r.folders.EnsureFolderPathExist(ctx, path) + if opts.Ref == "" { + folderName, err := r.folders.EnsureFolderPathExist(ctx, opts.Path) if err != nil { return nil, err } @@ -171,25 +179,25 @@ func (r *DualReadWriter) CreateFolder(ctx context.Context, path string, ref stri } // CreateResource creates a new resource in the repository -func (r *DualReadWriter) CreateResource(ctx context.Context, path string, ref string, message string, data []byte) (*ParsedResource, error) { - return r.createOrUpdate(ctx, true, path, ref, message, data) +func (r *DualReadWriter) CreateResource(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) { + return r.createOrUpdate(ctx, true, opts) } // UpdateResource updates a resource in the repository -func (r *DualReadWriter) UpdateResource(ctx context.Context, path string, ref string, message string, data []byte) (*ParsedResource, error) { - return r.createOrUpdate(ctx, false, path, ref, message, data) +func (r *DualReadWriter) UpdateResource(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) { + return r.createOrUpdate(ctx, false, opts) } // Create or updates a resource in the repository -func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, path string, ref string, message string, data []byte) (*ParsedResource, error) { - if err := repository.IsWriteAllowed(r.repo.Config(), ref); err != nil { +func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, opts DualWriteOptions) (*ParsedResource, error) { + if err := repository.IsWriteAllowed(r.repo.Config(), opts.Ref); err != nil { return nil, err } info := &repository.FileInfo{ - Data: data, - Path: path, - Ref: ref, + Data: opts.Data, + Path: opts.Path, + Ref: opts.Ref, } parsed, err := r.parser.Parse(ctx, info) @@ -198,12 +206,14 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, path s } // Make sure the value is valid - if err := parsed.DryRun(ctx); err != nil { - logger := logging.FromContext(ctx).With("path", path, "name", parsed.Obj.GetName(), "ref", ref) - logger.Warn("failed to dry run resource on create", "error", err) + if !opts.SkipDryRun { + if err := parsed.DryRun(ctx); err != nil { + logger := logging.FromContext(ctx).With("path", opts.Path, "name", parsed.Obj.GetName(), "ref", opts.Ref) + logger.Warn("failed to dry run resource on create", "error", err) - // TODO: return this as a 400 rather than 500 - return nil, fmt.Errorf("error running dryRun %w", err) + // TODO: return this as a 400 rather than 500 + return nil, fmt.Errorf("error running dryRun %w", err) + } } if len(parsed.Errors) > 0 { @@ -220,7 +230,7 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, path s return nil, err } - data, err = parsed.ToSaveBytes() + data, err := parsed.ToSaveBytes() if err != nil { return nil, err } @@ -233,9 +243,9 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, path s // Create or update if create { - err = r.repo.Create(ctx, path, ref, data, message) + err = r.repo.Create(ctx, opts.Path, opts.Ref, data, opts.Message) } else { - err = r.repo.Update(ctx, path, ref, data, message) + err = r.repo.Update(ctx, opts.Path, opts.Ref, data, opts.Message) } if err != nil { return nil, err // raw error is useful @@ -245,8 +255,8 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, path s // Behaves the same running sync after writing // FIXME: to make sure if behaves in the same way as in sync, we should // we should refactor the code to use the same function. - if ref == "" && parsed.Client != nil { - if _, err := r.folders.EnsureFolderPathExist(ctx, path); err != nil { + if opts.Ref == "" && parsed.Client != nil { + if _, err := r.folders.EnsureFolderPathExist(ctx, opts.Path); err != nil { return nil, fmt.Errorf("ensure folder path exists: %w", err) } diff --git a/pkg/registry/apis/provisioning/resources/parser.go b/pkg/registry/apis/provisioning/resources/parser.go index 0601a9f1e17..352e92fa27a 100644 --- a/pkg/registry/apis/provisioning/resources/parser.go +++ b/pkg/registry/apis/provisioning/resources/parser.go @@ -263,30 +263,33 @@ func (f *ParsedResource) Run(ctx context.Context) error { return err } - // We may have already called DryRun that also calls get - if f.DryRunResponse != nil && f.Action != "" { - // FIXME: shouldn't we check for the specific error? - f.Existing, _ = f.Client.Get(ctx, f.Obj.GetName(), metav1.GetOptions{}) - } - fieldValidation := "Strict" if f.GVR == DashboardResource { fieldValidation = "Ignore" // FIXME: temporary while we improve validation } - // Run update or create - if f.Existing == nil { + // If we have already tried loading existing, start with create + if f.DryRunResponse != nil && f.Existing == nil { f.Action = provisioning.ResourceActionCreate f.Upsert, err = f.Client.Create(ctx, f.Obj, metav1.CreateOptions{ FieldValidation: fieldValidation, }) - } else { - f.Action = provisioning.ResourceActionUpdate - f.Upsert, err = f.Client.Update(ctx, f.Obj, metav1.UpdateOptions{ + if err == nil { + return nil // it worked, return + } + } + + // Try update, otherwise create + f.Action = provisioning.ResourceActionUpdate + f.Upsert, err = f.Client.Update(ctx, f.Obj, metav1.UpdateOptions{ + FieldValidation: fieldValidation, + }) + if apierrors.IsNotFound(err) { + f.Action = provisioning.ResourceActionCreate + f.Upsert, err = f.Client.Create(ctx, f.Obj, metav1.CreateOptions{ FieldValidation: fieldValidation, }) } - return err } diff --git a/pkg/registry/apis/provisioning/resources/resources.go b/pkg/registry/apis/provisioning/resources/resources.go index 9a144e19a34..3ff01b0840e 100644 --- a/pkg/registry/apis/provisioning/resources/resources.go +++ b/pkg/registry/apis/provisioning/resources/resources.go @@ -8,7 +8,6 @@ import ( "fmt" "slices" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" @@ -173,21 +172,7 @@ func (r *ResourcesManager) WriteResourceFromFile(ctx context.Context, path strin parsed.Meta.SetUID("") parsed.Meta.SetResourceVersion("") - // TODO: use parsed.Run() (but that has an extra GET now!!) - fieldValidation := "Strict" - if parsed.GVR == DashboardResource { - fieldValidation = "Ignore" // FIXME: temporary while we improve validation - } - - // Update or Create resource - parsed.Upsert, err = parsed.Client.Update(ctx, parsed.Obj, metav1.UpdateOptions{ - FieldValidation: fieldValidation, - }) - if apierrors.IsNotFound(err) { - parsed.Upsert, err = parsed.Client.Create(ctx, parsed.Obj, metav1.CreateOptions{ - FieldValidation: fieldValidation, - }) - } + err = parsed.Run(ctx) return parsed.Obj.GetName(), parsed.GVK, err } 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 82b2537f563..16431ecceb5 100644 --- a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json @@ -1270,6 +1270,14 @@ "schema": { "type": "string" } + }, + { + "name": "skipDryRun", + "in": "query", + "description": "do not pro-actively verify the payload", + "schema": { + "type": "boolean" + } } ], "requestBody": { @@ -1366,6 +1374,14 @@ "schema": { "type": "string" } + }, + { + "name": "skipDryRun", + "in": "query", + "description": "do not pro-actively verify the payload", + "schema": { + "type": "boolean" + } } ], "requestBody": { @@ -1462,6 +1478,14 @@ "schema": { "type": "string" } + }, + { + "name": "skipDryRun", + "in": "query", + "description": "do not pro-actively verify the payload", + "schema": { + "type": "boolean" + } } ], "responses": { diff --git a/public/app/api/clients/provisioning/endpoints.gen.ts b/public/app/api/clients/provisioning/endpoints.gen.ts index c665973eb50..d46509228ca 100644 --- a/public/app/api/clients/provisioning/endpoints.gen.ts +++ b/public/app/api/clients/provisioning/endpoints.gen.ts @@ -160,6 +160,7 @@ const injectedRtkApi = api params: { ref: queryArg.ref, message: queryArg.message, + skipDryRun: queryArg.skipDryRun, }, }), invalidatesTags: ['Repository'], @@ -175,6 +176,7 @@ const injectedRtkApi = api params: { ref: queryArg.ref, message: queryArg.message, + skipDryRun: queryArg.skipDryRun, }, }), invalidatesTags: ['Repository'], @@ -189,6 +191,7 @@ const injectedRtkApi = api params: { ref: queryArg.ref, message: queryArg.message, + skipDryRun: queryArg.skipDryRun, }, }), invalidatesTags: ['Repository'], @@ -518,6 +521,8 @@ export type ReplaceRepositoryFilesWithPathApiArg = { ref?: string; /** optional message sent with any changes */ message?: string; + /** do not pro-actively verify the payload */ + skipDryRun?: boolean; body: { [key: string]: any; }; @@ -532,6 +537,8 @@ export type CreateRepositoryFilesWithPathApiArg = { ref?: string; /** optional message sent with any changes */ message?: string; + /** do not pro-actively verify the payload */ + skipDryRun?: boolean; body: { [key: string]: any; }; @@ -546,6 +553,8 @@ export type DeleteRepositoryFilesWithPathApiArg = { ref?: string; /** optional message sent with any changes */ message?: string; + /** do not pro-actively verify the payload */ + skipDryRun?: boolean; }; export type GetRepositoryHistoryApiResponse = /** status 200 OK */ string; export type GetRepositoryHistoryApiArg = {