From 24b70837c1ce5c7574d5df189e912a53b94c72a2 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Tue, 18 Jan 2022 00:29:43 +0100 Subject: [PATCH] cloudapi/v2: validate ostree params using common validation function Use the ostree package error types to keep the existing distinction between Ref- and URL-related errors. Introduce a new error condition for a general InvalidOSTreeParams failure. --- internal/cloudapi/v2/errors.go | 2 ++ internal/cloudapi/v2/v2.go | 35 +++++++++++++++++----------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/internal/cloudapi/v2/errors.go b/internal/cloudapi/v2/errors.go index 90c8ae537..7145bf514 100644 --- a/internal/cloudapi/v2/errors.go +++ b/internal/cloudapi/v2/errors.go @@ -39,6 +39,7 @@ const ( ErrorNoBaseURLInPayloadRepository ServiceErrorCode = 24 ErrorInvalidNumberOfImageBuilds ServiceErrorCode = 25 ErrorInvalidJobType ServiceErrorCode = 26 + ErrorInvalidOSTreeParams ServiceErrorCode = 27 // Internal errors, these are bugs ErrorFailedToInitializeBlueprint ServiceErrorCode = 1000 @@ -102,6 +103,7 @@ func getServiceErrors() serviceErrors { serviceError{ErrorNoBaseURLInPayloadRepository, http.StatusBadRequest, "BaseURL must be specified for payload repositories"}, serviceError{ErrorInvalidJobType, http.StatusNotFound, "Requested job has invalid type"}, serviceError{ErrorInvalidNumberOfImageBuilds, http.StatusBadRequest, "Compose request has unsupported number of image builds"}, + serviceError{ErrorInvalidOSTreeParams, http.StatusBadRequest, "Invalid OSTree parameters or parameter combination"}, serviceError{ErrorFailedToInitializeBlueprint, http.StatusInternalServerError, "Failed to initialize blueprint"}, serviceError{ErrorFailedToGenerateManifestSeed, http.StatusInternalServerError, "Failed to generate manifest seed"}, diff --git a/internal/cloudapi/v2/v2.go b/internal/cloudapi/v2/v2.go index 6dd2ac58f..91868a9fa 100644 --- a/internal/cloudapi/v2/v2.go +++ b/internal/cloudapi/v2/v2.go @@ -272,24 +272,25 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { } } - // set default ostree ref, if one not provided - ostreeOptions := ir.Ostree - if ostreeOptions == nil || ostreeOptions.Ref == nil { - imageOptions.OSTree = ostree.RequestParams{Ref: imageType.OSTreeRef()} - } else if !ostree.VerifyRef(*ostreeOptions.Ref) { - return HTTPError(ErrorInvalidOSTreeRef) - } else { - imageOptions.OSTree = ostree.RequestParams{Ref: *ostreeOptions.Ref} - } - - var parent string - if ostreeOptions != nil && ostreeOptions.Url != nil { - imageOptions.OSTree.URL = *ostreeOptions.Url - parent, err = ostree.ResolveRef(imageOptions.OSTree.URL, imageOptions.OSTree.Ref) - if err != nil { - return HTTPErrorWithInternal(ErrorInvalidOSTreeRepo, err) + ostreeOptions := ostree.RequestParams{} + if ir.Ostree != nil { + if ir.Ostree.Ref != nil { + ostreeOptions.Ref = *ir.Ostree.Ref + } + if ir.Ostree.Url != nil { + ostreeOptions.URL = *ir.Ostree.Url + } + } + if imageOptions.OSTree, err = ostree.ResolveParams(ostreeOptions, imageType.OSTreeRef()); err != nil { + switch v := err.(type) { + case ostree.InvalidParameterError: + return HTTPError(ErrorInvalidOSTreeRef) + case ostree.ResolveRefError: + return HTTPErrorWithInternal(ErrorInvalidOSTreeRepo, v) + default: + // general case + return HTTPError(ErrorInvalidOSTreeParams) } - imageOptions.OSTree.Parent = parent } var irTarget *target.Target