From f6ee44931dacd41b543ef890bc5f44ae5182c1ce Mon Sep 17 00:00:00 2001 From: Siva Parvathi Date: Thu, 14 May 2026 17:31:36 +0200 Subject: [PATCH] feat:Improved error management for catalog_manager --- src/catalog/common/helpers.go | 14 +- src/catalog/models/push_catalog_manifest.go | 21 +- .../models/push_catalog_manifest_test.go | 103 ++++--- src/controllers/catalog.go | 43 +-- src/controllers/catalog_managers.go | 255 ++++++++++-------- src/controllers/machines.go | 15 +- src/errors/main.go | 5 + src/helpers/url.go | 52 ++-- src/helpers/url_test.go | 29 +- 9 files changed, 321 insertions(+), 216 deletions(-) diff --git a/src/catalog/common/helpers.go b/src/catalog/common/helpers.go index 9b838524..58cbd524 100644 --- a/src/catalog/common/helpers.go +++ b/src/catalog/common/helpers.go @@ -1,10 +1,12 @@ package common import ( - "errors" "fmt" + "net/http" + "strconv" "github.com/Parallels/prl-devops-service/basecontext" + "github.com/Parallels/prl-devops-service/errors" "github.com/Parallels/prl-devops-service/serviceprovider" "github.com/Parallels/prl-devops-service/serviceprovider/system" ) @@ -31,14 +33,15 @@ func CalculatePartSize(fileSize int64) int64 { return size } -func ValidateArch(arch string) (string, error) { +func ValidateArch(arch string, diag *errors.Diagnostics) string { currentArch := arch if arch == "" { ctx := basecontext.NewRootBaseContext() svcCtl := system.Get() arch, err := svcCtl.GetArchitecture(ctx) if err != nil { - return "", errors.New("unable to determine architecture") + diag.AddError(strconv.Itoa(http.StatusInternalServerError), "unable to determine architecture", "ValidateArch") + return "" } currentArch = arch @@ -55,10 +58,11 @@ func ValidateArch(arch string) (string, error) { } if currentArch != "x86_64" && currentArch != "arm64" { - return "", errors.New("invalid architecture") + diag.AddError(strconv.Itoa(http.StatusBadRequest), "invalid architecture", "ValidateArch") + return "" } - return currentArch, nil + return currentArch } func ValidatePath(path string, owner string) (string, error) { diff --git a/src/catalog/models/push_catalog_manifest.go b/src/catalog/models/push_catalog_manifest.go index c538ad91..fbf1e310 100644 --- a/src/catalog/models/push_catalog_manifest.go +++ b/src/catalog/models/push_catalog_manifest.go @@ -1,6 +1,9 @@ package models import ( + "net/http" + "strconv" + "github.com/Parallels/prl-devops-service/errors" "github.com/Parallels/prl-devops-service/helpers" ) @@ -49,25 +52,25 @@ type MinimumSpecRequirement struct { Disk int `json:"disk"` } -func (r *PushCatalogManifestRequest) Validate() error { +func (r *PushCatalogManifestRequest) Validate(diag *errors.Diagnostics) { if r.LocalPath == "" { - return ErrPushMissingLocalPath + diag.AddError(strconv.Itoa(http.StatusBadRequest), ErrPushMissingLocalPath.Error(), "Validate") } if r.CatalogId == "" { - return ErrPushMissingCatalogId + diag.AddError(strconv.Itoa(http.StatusBadRequest), ErrPushMissingCatalogId.Error(), "Validate") } if r.Connection == "" { - return ErrMissingConnection + diag.AddError(strconv.Itoa(http.StatusBadRequest), ErrMissingConnection.Error(), "Validate") } if r.Version == "" { - return ErrPushMissingVersion + diag.AddError(strconv.Itoa(http.StatusBadRequest), ErrPushMissingVersion.Error(), "Validate") } if r.Architecture == "" { - return ErrInvalidArchitecture + diag.AddError(strconv.Itoa(http.StatusBadRequest), ErrMissingArchitecture.Error(), "Validate") } // Normalize architecture aliases @@ -80,7 +83,7 @@ func (r *PushCatalogManifestRequest) Validate() error { // Validate architecture if r.Architecture != "x86_64" && r.Architecture != "arm64" { - return ErrInvalidArchitecture + diag.AddError(strconv.Itoa(http.StatusBadRequest), ErrInvalidArchitecture.Error(), "Validate") } // Set compress pack level if compress is true and compress level is not set @@ -114,8 +117,8 @@ func (r *PushCatalogManifestRequest) Validate() error { } if helpers.ContainsIllegalChars(r.Version) { - return ErrPushVersionInvalidChars + diag.AddError(strconv.Itoa(http.StatusBadRequest), ErrPushVersionInvalidChars.Error(), "Validate") } - return nil + return } diff --git a/src/catalog/models/push_catalog_manifest_test.go b/src/catalog/models/push_catalog_manifest_test.go index 9d850589..1814bd8d 100644 --- a/src/catalog/models/push_catalog_manifest_test.go +++ b/src/catalog/models/push_catalog_manifest_test.go @@ -2,6 +2,8 @@ package models import ( "testing" + + "github.com/Parallels/prl-devops-service/errors" ) func TestPushCatalogManifestRequestValidate_MissingLocalPath(t *testing.T) { @@ -11,12 +13,13 @@ func TestPushCatalogManifestRequestValidate_MissingLocalPath(t *testing.T) { Architecture: "x86_64", Connection: "provider://something", } - err := r.Validate() - if err == nil { - t.Fatal("expected error for missing local_path, got nil") + diag := errors.NewDiagnostics("Test") + r.Validate(diag) + if !diag.HasErrors() { + t.Fatal("expected error for missing local_path, got none") } - if err != ErrPushMissingLocalPath { - t.Errorf("expected ErrPushMissingLocalPath, got %v", err) + if diag.GetErrors()[0].Message != ErrPushMissingLocalPath.Error() { + t.Errorf("expected ErrPushMissingLocalPath, got %v", diag.GetErrors()[0].Message) } } @@ -27,12 +30,13 @@ func TestPushCatalogManifestRequestValidate_MissingCatalogId(t *testing.T) { Architecture: "x86_64", Connection: "provider://something", } - err := r.Validate() - if err == nil { - t.Fatal("expected error for missing catalog_id, got nil") + diag := errors.NewDiagnostics("Test") + r.Validate(diag) + if !diag.HasErrors() { + t.Fatal("expected error for missing catalog_id, got none") } - if err != ErrPushMissingCatalogId { - t.Errorf("expected ErrPushMissingCatalogId, got %v", err) + if diag.GetErrors()[0].Message != ErrPushMissingCatalogId.Error() { + t.Errorf("expected ErrPushMissingCatalogId, got %v", diag.GetErrors()[0].Message) } } @@ -43,12 +47,13 @@ func TestPushCatalogManifestRequestValidate_MissingConnection(t *testing.T) { Version: "v1.0", Architecture: "x86_64", } - err := r.Validate() - if err == nil { - t.Fatal("expected error for missing connection, got nil") + diag := errors.NewDiagnostics("Test") + r.Validate(diag) + if !diag.HasErrors() { + t.Fatal("expected error for missing connection, got none") } - if err != ErrMissingConnection { - t.Errorf("expected ErrMissingConnection, got %v", err) + if diag.GetErrors()[0].Message != ErrMissingConnection.Error() { + t.Errorf("expected ErrMissingConnection, got %v", diag.GetErrors()[0].Message) } } @@ -59,12 +64,30 @@ func TestPushCatalogManifestRequestValidate_MissingVersion(t *testing.T) { Architecture: "x86_64", Connection: "provider://something", } - err := r.Validate() - if err == nil { - t.Fatal("expected error for missing version, got nil") + diag := errors.NewDiagnostics("Test") + r.Validate(diag) + if !diag.HasErrors() { + t.Fatal("expected error for missing version, got none") + } + if diag.GetErrors()[0].Message != ErrPushMissingVersion.Error() { + t.Errorf("expected ErrPushMissingVersion, got %v", diag.GetErrors()[0].Message) + } +} + +func TestPushCatalogManifestRequestValidate_MissingArchitecture(t *testing.T) { + r := PushCatalogManifestRequest{ + LocalPath: "/some/path", + CatalogId: "test-catalog", + Version: "v1.0", + Connection: "provider://something", + } + diag := errors.NewDiagnostics("Test") + r.Validate(diag) + if !diag.HasErrors() { + t.Fatal("expected error for missing architecture, got none") } - if err != ErrPushMissingVersion { - t.Errorf("expected ErrPushMissingVersion, got %v", err) + if diag.GetErrors()[0].Message != ErrMissingArchitecture.Error() { + t.Errorf("expected ErrMissingArchitecture, got %v", diag.GetErrors()[0].Message) } } @@ -76,12 +99,13 @@ func TestPushCatalogManifestRequestValidate_InvalidArchitecture(t *testing.T) { Architecture: "mips", Connection: "provider://something", } - err := r.Validate() - if err == nil { - t.Fatal("expected error for invalid architecture, got nil") + diag := errors.NewDiagnostics("Test") + r.Validate(diag) + if !diag.HasErrors() { + t.Fatal("expected error for invalid architecture, got none") } - if err != ErrInvalidArchitecture { - t.Errorf("expected ErrInvalidArchitecture, got %v", err) + if diag.GetErrors()[0].Message != ErrInvalidArchitecture.Error() { + t.Errorf("expected ErrInvalidArchitecture, got %v", diag.GetErrors()[0].Message) } } @@ -105,8 +129,10 @@ func TestPushCatalogManifestRequestValidate_ArchitectureNormalization(t *testing Architecture: tt.input, Connection: "provider://something", } - if err := r.Validate(); err != nil { - t.Errorf("input %q: unexpected error: %v", tt.input, err) + diag := errors.NewDiagnostics("Test") + r.Validate(diag) + if diag.HasErrors() { + t.Errorf("input %q: unexpected error: %v", tt.input, diag.GetErrors()[0].Message) continue } if r.Architecture != tt.expected { @@ -123,12 +149,13 @@ func TestPushCatalogManifestRequestValidate_VersionWithIllegalChars(t *testing.T Architecture: "x86_64", Connection: "provider://something", } - err := r.Validate() - if err == nil { - t.Fatal("expected error for version with illegal chars, got nil") + diag := errors.NewDiagnostics("Test") + r.Validate(diag) + if !diag.HasErrors() { + t.Fatal("expected error for version with illegal chars, got none") } - if err != ErrPushVersionInvalidChars { - t.Errorf("expected ErrPushVersionInvalidChars, got %v", err) + if diag.GetErrors()[0].Message != ErrPushVersionInvalidChars.Error() { + t.Errorf("expected ErrPushVersionInvalidChars, got %v", diag.GetErrors()[0].Message) } } @@ -140,8 +167,10 @@ func TestPushCatalogManifestRequestValidate_Valid(t *testing.T) { Architecture: "x86_64", Connection: "provider://something", } - if err := r.Validate(); err != nil { - t.Errorf("expected no error, got %v", err) + diag := errors.NewDiagnostics("Test") + r.Validate(diag) + if diag.HasErrors() { + t.Errorf("expected no error, got %v", diag.GetErrors()[0].Message) } } @@ -169,8 +198,10 @@ func TestPushCatalogManifestRequestValidate_MinimumSpecDefaults(t *testing.T) { Disk: 20480, }, } - if err := r.Validate(); err != nil { - t.Errorf("expected no error for valid request with min spec requirements, got %v", err) + diag := errors.NewDiagnostics("Test") + r.Validate(diag) + if diag.HasErrors() { + t.Errorf("expected no error for valid request with min spec requirements, got %v", diag.GetErrors()[0].Message) } if r.MinimumSpecRequirements.Cpu != 2 { t.Errorf("expected Cpu=2, got %v", r.MinimumSpecRequirements.Cpu) diff --git a/src/controllers/catalog.go b/src/controllers/catalog.go index 72628eb1..d60f1205 100644 --- a/src/controllers/catalog.go +++ b/src/controllers/catalog.go @@ -3,7 +3,6 @@ package controllers import ( "encoding/base64" "encoding/json" - "errors" "fmt" "net/http" @@ -15,6 +14,7 @@ import ( "github.com/Parallels/prl-devops-service/config" "github.com/Parallels/prl-devops-service/constants" data_models "github.com/Parallels/prl-devops-service/data/models" + "github.com/Parallels/prl-devops-service/errors" "github.com/Parallels/prl-devops-service/helpers" "github.com/Parallels/prl-devops-service/jobs" "github.com/Parallels/prl-devops-service/mappers" @@ -1883,6 +1883,8 @@ func PushCatalogManifestHandler() restapi.ControllerHandler { defer r.Body.Close() ctx := GetBaseContext(r) defer Recover(ctx, r, w) + + pushCatalogManifestDiag := errors.NewDiagnostics("/catalog/push") var request catalog_models.PushCatalogManifestRequest if err := http_helper.MapRequestBody(r, &request); err != nil { ReturnApiError(ctx, w, models.ApiErrorResponse{ @@ -1893,19 +1895,20 @@ func PushCatalogManifestHandler() restapi.ControllerHandler { } // Validate architecture and path - arch, err := catalog_helpers.ValidateArch(request.Architecture) - if err != nil { + arch := catalog_helpers.ValidateArch(request.Architecture, pushCatalogManifestDiag) + if pushCatalogManifestDiag.HasErrors() { ReturnApiError(ctx, w, models.ApiErrorResponse{ - Message: "Invalid request body: " + err.Error(), + Message: "Invalid request body: " + pushCatalogManifestDiag.GetErrors()[0].Message, Code: http.StatusBadRequest, }) return } request.Architecture = arch - if err := request.Validate(); err != nil { + request.Validate(pushCatalogManifestDiag) + if pushCatalogManifestDiag.HasErrors() { ReturnApiError(ctx, w, models.ApiErrorResponse{ - Message: "Invalid request body: " + err.Error(), + Message: "Invalid request body: " + pushCatalogManifestDiag.GetErrors()[0].Message, Code: http.StatusBadRequest, }) return @@ -1994,7 +1997,7 @@ func AsyncPushCatalogManifestHandler() restapi.ControllerHandler { defer r.Body.Close() ctx := GetBaseContext(r) defer Recover(ctx, r, w) - + asyncPushCatalogManifestDiag := errors.NewDiagnostics("/catalog/push/async") userContext := ctx.GetUser() if userContext == nil { ReturnApiError(ctx, w, models.ApiErrorResponse{Code: http.StatusUnauthorized, Message: "User not found"}) @@ -2011,19 +2014,20 @@ func AsyncPushCatalogManifestHandler() restapi.ControllerHandler { } // Validate architecture - arch, err := catalog_helpers.ValidateArch(request.Architecture) - if err != nil { + arch := catalog_helpers.ValidateArch(request.Architecture, asyncPushCatalogManifestDiag) + if asyncPushCatalogManifestDiag.HasErrors() { ReturnApiError(ctx, w, models.ApiErrorResponse{ - Message: "Invalid request body: " + err.Error(), + Message: "Invalid request body: " + asyncPushCatalogManifestDiag.GetErrors()[0].Message, Code: http.StatusBadRequest, }) return } request.Architecture = arch - if err := request.Validate(); err != nil { + request.Validate(asyncPushCatalogManifestDiag) + if asyncPushCatalogManifestDiag.HasErrors() { ReturnApiError(ctx, w, models.ApiErrorResponse{ - Message: "Invalid request body: " + err.Error(), + Message: "Invalid request body: " + asyncPushCatalogManifestDiag.GetErrors()[0].Message, Code: http.StatusBadRequest, }) return @@ -2069,6 +2073,7 @@ func PullCatalogManifestHandler() restapi.ControllerHandler { defer r.Body.Close() ctx := GetBaseContext(r) defer Recover(ctx, r, w) + pullCatalogManifestDiag := errors.NewDiagnostics("/catalog/pull") var request catalog_models.PullCatalogManifestRequest if err := http_helper.MapRequestBody(r, &request); err != nil { ReturnApiError(ctx, w, models.ApiErrorResponse{ @@ -2079,10 +2084,10 @@ func PullCatalogManifestHandler() restapi.ControllerHandler { } // Validate architecture and path - arch, err := catalog_helpers.ValidateArch(request.Architecture) - if err != nil { + arch := catalog_helpers.ValidateArch(request.Architecture, pullCatalogManifestDiag) + if pullCatalogManifestDiag.HasErrors() { ReturnApiError(ctx, w, models.ApiErrorResponse{ - Message: "Invalid request body: " + err.Error(), + Message: "Invalid request body: " + pullCatalogManifestDiag.GetErrors()[0].Message, Code: http.StatusBadRequest, }) return @@ -2207,7 +2212,7 @@ func AsyncPullCatalogManifestHandler() restapi.ControllerHandler { defer r.Body.Close() ctx := GetBaseContext(r) defer Recover(ctx, r, w) - + asyncPullCatalogManifestDiag := errors.NewDiagnostics("/catalog/pull/async") userContext := ctx.GetUser() if userContext == nil { ReturnApiError(ctx, w, models.ApiErrorResponse{Code: http.StatusUnauthorized, Message: "User not found"}) @@ -2224,10 +2229,10 @@ func AsyncPullCatalogManifestHandler() restapi.ControllerHandler { } // Validate architecture and path - arch, err := catalog_helpers.ValidateArch(request.Architecture) - if err != nil { + arch := catalog_helpers.ValidateArch(request.Architecture, asyncPullCatalogManifestDiag) + if asyncPullCatalogManifestDiag.HasErrors() { ReturnApiError(ctx, w, models.ApiErrorResponse{ - Message: "Invalid request body: " + err.Error(), + Message: "Invalid request body: " + asyncPullCatalogManifestDiag.GetErrors()[0].Message, Code: http.StatusBadRequest, }) return diff --git a/src/controllers/catalog_managers.go b/src/controllers/catalog_managers.go index a618639d..6162f940 100644 --- a/src/controllers/catalog_managers.go +++ b/src/controllers/catalog_managers.go @@ -5,13 +5,13 @@ import ( "crypto/tls" "encoding/base64" "encoding/json" - "errors" "fmt" "io" "net" "net/http" "net/url" "path" + "strconv" "strings" "time" @@ -22,6 +22,7 @@ import ( "github.com/Parallels/prl-devops-service/config" "github.com/Parallels/prl-devops-service/constants" data_models "github.com/Parallels/prl-devops-service/data/models" + "github.com/Parallels/prl-devops-service/errors" "github.com/Parallels/prl-devops-service/helpers" "github.com/Parallels/prl-devops-service/jobs" "github.com/Parallels/prl-devops-service/mappers" @@ -362,7 +363,7 @@ func registerCatalogManagerCatalogHandlers(version string) { // @Tags CatalogManagers // @Produce json // @Success 200 {object} []models.CatalogManager -// @Failure 400 {object} models.ApiErrorResponse +// @Failure 400 {object} models.ApiErrorDiagnosticsResponse // @Failure 401 {object} models.OAuthErrorResponse // @Security ApiKeyAuth // @Security BearerAuth @@ -372,6 +373,7 @@ func GetCatalogManagersHandler() restapi.ControllerHandler { defer r.Body.Close() ctx := GetBaseContext(r) defer Recover(ctx, r, w) + getCatalogManagers := errors.NewDiagnostics("/catalog-managers") dbService, err := serviceprovider.GetDatabaseService(ctx) if err != nil { ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusInternalServerError)) @@ -380,7 +382,8 @@ func GetCatalogManagersHandler() restapi.ControllerHandler { user := ctx.GetUser() if user == nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(errors.New("User not contextually found"), http.StatusUnauthorized)) + getCatalogManagers.AddError(strconv.Itoa(http.StatusUnauthorized), "No user context available", "GetCatalogManagersHandler") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getCatalogManagers, http.StatusUnauthorized)) return } @@ -399,7 +402,8 @@ func GetCatalogManagersHandler() restapi.ControllerHandler { } if !canListAll && !canListOwn { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(errors.New("Not authorized to view catalog managers"), http.StatusForbidden)) + getCatalogManagers.AddError(strconv.Itoa(http.StatusForbidden), "User does not have permission to list catalog managers", "GetCatalogManagersHandler") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getCatalogManagers, http.StatusForbidden)) return } @@ -456,7 +460,7 @@ func GetCatalogManagersHandler() restapi.ControllerHandler { // @Produce json // @Param id path string true "Manager ID" // @Success 200 {object} models.CatalogManager -// @Failure 400 {object} models.ApiErrorResponse +// @Failure 400 {object} models.ApiErrorDiagnosticsResponse // @Failure 401 {object} models.OAuthErrorResponse // @Security ApiKeyAuth // @Security BearerAuth @@ -466,18 +470,19 @@ func GetCatalogManagerByIdHandler() restapi.ControllerHandler { defer r.Body.Close() ctx := GetBaseContext(r) defer Recover(ctx, r, w) + vars := mux.Vars(r) + id := vars["id"] + getCatalogManagerById := errors.NewDiagnostics("/catalog-managers" + id) dbService, err := serviceprovider.GetDatabaseService(ctx) if err != nil { ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusInternalServerError)) return } - vars := mux.Vars(r) - id := vars["id"] - user := ctx.GetUser() if user == nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(errors.New("User not contextually found"), http.StatusUnauthorized)) + getCatalogManagerById.AddError(strconv.Itoa(http.StatusUnauthorized), "No user context available", "GetCatalogManagerByIdHandler") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getCatalogManagerById, http.StatusUnauthorized)) return } @@ -523,7 +528,8 @@ func GetCatalogManagerByIdHandler() restapi.ControllerHandler { } if !isAuthorized { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(errors.New("Not authorized to view this catalog manager"), http.StatusForbidden)) + getCatalogManagerById.AddError(strconv.Itoa(http.StatusForbidden), "Not authorized to view this catalog manager", "GetCatalogManagerByIdHandler") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getCatalogManagerById, http.StatusForbidden)) return } @@ -538,7 +544,7 @@ func GetCatalogManagerByIdHandler() restapi.ControllerHandler { // @Tags CatalogManagers // @Produce json // @Success 200 {object} models.CatalogManager -// @Failure 400 {object} models.ApiErrorResponse +// @Failure 400 {object} models.ApiErrorDiagnosticsResponse // @Failure 401 {object} models.OAuthErrorResponse // @Security ApiKeyAuth // @Security BearerAuth @@ -547,11 +553,13 @@ func CreateCatalogManagerHandler() restapi.ControllerHandler { return func(w http.ResponseWriter, r *http.Request) { ctx := GetBaseContext(r) defer Recover(ctx, r, w) - + createCatalogManager := errors.NewDiagnostics("/catalog-managers") var req models.CatalogManagerRequest err := json.NewDecoder(r.Body).Decode(&req) if err != nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusBadRequest)) + rsp := models.NewFromError(err) + createCatalogManager.AddError(strconv.Itoa(rsp.Code), rsp.Message, "Failed to decode request body") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(createCatalogManager, http.StatusBadRequest)) return } defer r.Body.Close() @@ -563,8 +571,10 @@ func CreateCatalogManagerHandler() restapi.ControllerHandler { newMgr.CreatedAt = helpers.GetUtcCurrentDateTime() newMgr.UpdatedAt = helpers.GetUtcCurrentDateTime() - if err := validateCatalogManagerUrl(newMgr.URL); err != nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusBadRequest)) + validateCatalogManagerUrl(newMgr.URL, createCatalogManager) + + if createCatalogManager.HasErrors() { + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(createCatalogManager, http.StatusBadRequest)) return } @@ -576,13 +586,15 @@ func CreateCatalogManagerHandler() restapi.ControllerHandler { } if req.Global || req.Internal { if !canCreateGlobalInternal { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(errors.New("Not authorized to create global or internal catalog manager"), http.StatusForbidden)) + createCatalogManager.AddError(strconv.Itoa(http.StatusForbidden), "Not authorized to create global or internal catalog manager", "Failed to create catalog manager") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(createCatalogManager, http.StatusForbidden)) return } } - if err := validateCatalogManagerConnection(ctx, newMgr.URL, newMgr.Username, decryptCatalogManagerSecret(newMgr.Password), decryptCatalogManagerSecret(newMgr.ApiKey)); err != nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusBadRequest)) + validateCatalogManagerConnection(ctx, newMgr.URL, newMgr.Username, decryptCatalogManagerSecret(newMgr.Password), decryptCatalogManagerSecret(newMgr.ApiKey), createCatalogManager) + if createCatalogManager.HasErrors() { + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(createCatalogManager, http.StatusBadRequest)) return } @@ -609,7 +621,7 @@ func CreateCatalogManagerHandler() restapi.ControllerHandler { // @Produce json // @Param id path string true "Manager ID" // @Success 200 {object} models.CatalogManager -// @Failure 400 {object} models.ApiErrorResponse +// @Failure 400 {object} models.ApiErrorDiagnosticsResponse // @Failure 401 {object} models.OAuthErrorResponse // @Security ApiKeyAuth // @Security BearerAuth @@ -618,14 +630,15 @@ func UpdateCatalogManagerHandler() restapi.ControllerHandler { return func(w http.ResponseWriter, r *http.Request) { ctx := GetBaseContext(r) defer Recover(ctx, r, w) - vars := mux.Vars(r) id := vars["id"] + updateCatalogManager := errors.NewDiagnostics("/catalog-managers/" + id) var req models.CatalogManagerRequest err := json.NewDecoder(r.Body).Decode(&req) if err != nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusBadRequest)) + updateCatalogManager.AddError(strconv.Itoa(http.StatusBadRequest), err.Error(), "Failed to decode request body") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(updateCatalogManager, http.StatusBadRequest)) return } defer r.Body.Close() @@ -659,7 +672,8 @@ func UpdateCatalogManagerHandler() restapi.ControllerHandler { } if mgr.OwnerID != user.ID && !canSystemUpdate { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(errors.New("Not authorized to update this catalog manager"), http.StatusForbidden)) + updateCatalogManager.AddError(strconv.Itoa(http.StatusForbidden), "Not authorized to update this catalog manager", "") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(updateCatalogManager, http.StatusForbidden)) return } @@ -667,13 +681,15 @@ func UpdateCatalogManagerHandler() restapi.ControllerHandler { mappers.UpdateCatalogManagerFromRequest(&updatedMgr, &req) updatedMgr.UpdatedAt = helpers.GetUtcCurrentDateTime() - if err := validateCatalogManagerUrl(updatedMgr.URL); err != nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusBadRequest)) + validateCatalogManagerUrl(updatedMgr.URL, updateCatalogManager) + if updateCatalogManager.HasErrors() { + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(updateCatalogManager, http.StatusBadRequest)) return } - if err := validateCatalogManagerConnection(ctx, updatedMgr.URL, updatedMgr.Username, decryptCatalogManagerSecret(updatedMgr.Password), decryptCatalogManagerSecret(updatedMgr.ApiKey)); err != nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusBadRequest)) + validateCatalogManagerConnection(ctx, updatedMgr.URL, updatedMgr.Username, decryptCatalogManagerSecret(updatedMgr.Password), decryptCatalogManagerSecret(updatedMgr.ApiKey), updateCatalogManager) + if updateCatalogManager.HasErrors() { + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(updateCatalogManager, http.StatusBadRequest)) return } @@ -696,7 +712,7 @@ func UpdateCatalogManagerHandler() restapi.ControllerHandler { // @Produce json // @Param id path string true "Manager ID" // @Success 200 {object} models.ApiCommonResponse -// @Failure 400 {object} models.ApiErrorResponse +// @Failure 400 {object} models.ApiErrorDiagnosticsResponse // @Failure 401 {object} models.OAuthErrorResponse // @Security ApiKeyAuth // @Security BearerAuth @@ -708,6 +724,7 @@ func DeleteCatalogManagerHandler() restapi.ControllerHandler { vars := mux.Vars(r) id := vars["id"] + deleteCatalogManager := errors.NewDiagnostics("/catalog-managers/" + id) dbService, err := serviceprovider.GetDatabaseService(ctx) if err != nil { @@ -738,7 +755,8 @@ func DeleteCatalogManagerHandler() restapi.ControllerHandler { } if mgr.OwnerID != user.ID && !canSystemDelete { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(errors.New("Not authorized to delete this catalog manager"), http.StatusForbidden)) + deleteCatalogManager.AddError(strconv.Itoa(http.StatusForbidden), "Not authorized to delete this catalog manager", "") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(deleteCatalogManager, http.StatusForbidden)) return } @@ -766,76 +784,71 @@ func AsyncPushCatalogManifestToCatalogManagerHandler() restapi.ControllerHandler defer r.Body.Close() ctx := GetBaseContext(r) defer Recover(ctx, r, w) + vars := mux.Vars(r) + managerID := vars["id"] + asyncPushCatalogManifest := errors.NewDiagnostics("/catalog-managers/" + managerID + "/push") userContext := ctx.GetUser() if userContext == nil { - ReturnApiError(ctx, w, models.ApiErrorResponse{Code: http.StatusUnauthorized, Message: "User not found"}) + asyncPushCatalogManifest.AddError(strconv.Itoa(http.StatusUnauthorized), "User not found", "") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(asyncPushCatalogManifest, http.StatusUnauthorized)) return } - vars := mux.Vars(r) - managerID := vars["id"] - mgr, errResp := getAuthorizedCatalogManagerForUse(ctx, managerID) if errResp != nil { - ReturnApiError(ctx, w, *errResp) + asyncPushCatalogManifest.AddError(strconv.Itoa(errResp.Code), errResp.Message, "Failed to get authorized catalog manager") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(asyncPushCatalogManifest, errResp.Code)) return } var request catalog_models.PushCatalogManifestRequest if err := http_helper.MapRequestBody(r, &request); err != nil { - ReturnApiError(ctx, w, models.ApiErrorResponse{ - Message: "Invalid request body: " + err.Error(), - Code: http.StatusBadRequest, - }) + asyncPushCatalogManifest.AddError(strconv.Itoa(http.StatusBadRequest), "Invalid request body: "+err.Error(), "") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(asyncPushCatalogManifest, http.StatusBadRequest)) return } if request.Connection == "" { - ReturnApiError(ctx, w, models.ApiErrorResponse{ - Code: http.StatusBadRequest, - Message: "connection is required in the request body (storage provider connection string, e.g. provider=minio;endpoint=...;bucket=...)", - }) + asyncPushCatalogManifest.AddError(strconv.Itoa(http.StatusBadRequest), "connection is required in the request body (storage provider connection string, e.g. provider=minio;endpoint=...;bucket=...)", "") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(asyncPushCatalogManifest, http.StatusBadRequest)) return } // Build the host= part from the catalog manager credentials and merge with the // user-provided storage connection (stripping any host= they may have included). - hostPart, err := buildCatalogManagerConnection(*mgr) - if err != nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusBadRequest)) + hostPart := buildCatalogManagerConnection(*mgr, asyncPushCatalogManifest) + if asyncPushCatalogManifest.HasErrors() { + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(asyncPushCatalogManifest, http.StatusBadRequest)) return } storageParts := stripHostFromConnection(request.Connection) request.Connection = hostPart + ";" + storageParts - arch, err := catalog_helpers.ValidateArch(request.Architecture) - if err != nil { - ReturnApiError(ctx, w, models.ApiErrorResponse{ - Message: "Invalid architecture: " + err.Error(), - Code: http.StatusBadRequest, - }) + arch := catalog_helpers.ValidateArch(request.Architecture, asyncPushCatalogManifest) + if asyncPushCatalogManifest.HasErrors() { + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(asyncPushCatalogManifest, http.StatusBadRequest)) return } request.Architecture = arch - if err := request.Validate(); err != nil { - ReturnApiError(ctx, w, models.ApiErrorResponse{ - Message: "Invalid request body: " + err.Error(), - Code: http.StatusBadRequest, - }) + request.Validate(asyncPushCatalogManifest) + if asyncPushCatalogManifest.HasErrors() { + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(asyncPushCatalogManifest, http.StatusBadRequest)) return } jobManager := jobs.Get(ctx) if jobManager == nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(errors.New("Job Manager is not available"), http.StatusInternalServerError)) + asyncPushCatalogManifest.AddError(strconv.Itoa(http.StatusInternalServerError), "Job Manager is not available", "") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(asyncPushCatalogManifest, http.StatusInternalServerError)) return } localJob, err := jobManager.CreateNewJob(userContext.ID, "catalog", "push", "Initializing catalog push to manager "+mgr.Name) if err != nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusInternalServerError)) + asyncPushCatalogManifest.AddError(strconv.Itoa(http.StatusInternalServerError), "Failed to create job: "+err.Error(), "CreateNewJob") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(asyncPushCatalogManifest, http.StatusInternalServerError)) return } @@ -855,19 +868,21 @@ func ForwardCatalogManagerCatalogRequestHandler(remotePathTemplate string) resta defer r.Body.Close() ctx := GetBaseContext(r) defer Recover(ctx, r, w) - + diag := errors.NewDiagnostics("/catalog-managers/") vars := mux.Vars(r) managerID := vars["id"] mgr, err := getAuthorizedCatalogManagerForUse(ctx, managerID) if err != nil { - ReturnApiError(ctx, w, *err) + diag.AddError(strconv.Itoa(err.Code), err.Message, "Failed to get authorized catalog manager") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(diag, err.Code)) return } relativePath := resolveMuxPath(remotePathTemplate, vars) - if err := forwardCatalogManagerRequest(ctx, w, r, mgr, relativePath); err != nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusBadGateway)) + forwardCatalogManagerRequest(ctx, w, r, mgr, relativePath, diag) + if diag.HasErrors() { + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(diag, http.StatusBadGateway)) return } } @@ -878,21 +893,17 @@ func PullCatalogManifestFromCatalogManagerHandler(async bool) restapi.Controller defer r.Body.Close() ctx := GetBaseContext(r) defer Recover(ctx, r, w) - + pullCatalogManifest := errors.NewDiagnostics("PullCatalogManifestFromCatalogManagerHandler") var request catalog_models.PullCatalogManifestRequest if err := http_helper.MapRequestBody(r, &request); err != nil { - ReturnApiError(ctx, w, models.ApiErrorResponse{ - Message: "Invalid request body: " + err.Error(), - Code: http.StatusBadRequest, - }) + pullCatalogManifest.AddError(strconv.Itoa(http.StatusBadRequest), "Invalid request body: "+err.Error(), "") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(pullCatalogManifest, http.StatusBadRequest)) return } if request.Connection != "" { - ReturnApiError(ctx, w, models.ApiErrorResponse{ - Message: "connection is not allowed for catalog manager pull endpoints", - Code: http.StatusBadRequest, - }) + pullCatalogManifest.AddError(strconv.Itoa(http.StatusBadRequest), "connection is not allowed for catalog manager pull endpoints", "") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(pullCatalogManifest, http.StatusBadRequest)) return } @@ -901,20 +912,22 @@ func PullCatalogManifestFromCatalogManagerHandler(async bool) restapi.Controller mgr, errResp := getAuthorizedCatalogManagerForUse(ctx, managerID) if errResp != nil { - ReturnApiError(ctx, w, *errResp) + pullCatalogManifest.AddError(strconv.Itoa(errResp.Code), errResp.Message, "Failed to get authorized catalog manager") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(pullCatalogManifest, errResp.Code)) return } - connection, err := buildCatalogManagerConnection(*mgr) - if err != nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusBadRequest)) + connection := buildCatalogManagerConnection(*mgr, pullCatalogManifest) + if pullCatalogManifest.HasErrors() { + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(pullCatalogManifest, http.StatusBadRequest)) return } request.Connection = connection payload, err := json.Marshal(request) if err != nil { - ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusInternalServerError)) + pullCatalogManifest.AddError(strconv.Itoa(http.StatusInternalServerError), "Failed to marshal request body: "+err.Error(), "") + ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(pullCatalogManifest, http.StatusInternalServerError)) return } @@ -1003,10 +1016,12 @@ func resolveMuxPath(pathTemplate string, vars map[string]string) string { return resolvedPath } -func buildCatalogManagerTargetUrl(baseURL string, endpointPath string, rawQuery string) (string, error) { +func buildCatalogManagerTargetUrl(baseURL string, endpointPath string, rawQuery string, diag *errors.Diagnostics) string { parsedURL, err := url.Parse(baseURL) if err != nil { - return "", err + rsp := models.NewFromError(err) + diag.AddError(strconv.Itoa(rsp.Code), rsp.Message, "Failed to build catalog manager target url") + return "" } basePath := strings.TrimRight(parsedURL.Path, "/") @@ -1017,7 +1032,7 @@ func buildCatalogManagerTargetUrl(baseURL string, endpointPath string, rawQuery } parsedURL.RawQuery = rawQuery - return parsedURL.String(), nil + return parsedURL.String() } func decryptCatalogManagerSecret(value string) string { @@ -1058,24 +1073,25 @@ func getCatalogManagerAuthorizer(ctx basecontext.ApiContext, manager data_models return client.Authorize(ctx, targetUrl) } -func buildCatalogManagerConnection(manager data_models.CatalogManager) (string, error) { +func buildCatalogManagerConnection(manager data_models.CatalogManager, diag *errors.Diagnostics) string { host := strings.TrimSpace(manager.URL) if host == "" { - return "", errors.New("catalog manager url is required") + diag.AddError(strconv.Itoa(http.StatusBadRequest), "catalog manager url is required", "") + return "" } password := decryptCatalogManagerSecret(manager.Password) apiKey := decryptCatalogManagerSecret(manager.ApiKey) if apiKey != "" { - return fmt.Sprintf("host=%s@%s", apiKey, host), nil + return fmt.Sprintf("host=%s@%s", apiKey, host) } if manager.Username != "" && password != "" { - return fmt.Sprintf("host=%s:%s@%s", manager.Username, password, host), nil + return fmt.Sprintf("host=%s:%s@%s", manager.Username, password, host) } - return "host=" + host, nil + return "host=" + host } // stripHostFromConnection removes any host= segment from a semicolon-separated @@ -1091,30 +1107,35 @@ func stripHostFromConnection(connection string) string { return strings.Join(filtered, ";") } -func validateCatalogManagerUrl(managerURL string) error { - return helpers.ValidateCatalogManagerUrl(managerURL) +func validateCatalogManagerUrl(managerURL string, diag *errors.Diagnostics) { + helpers.ValidateCatalogManagerUrl(managerURL, diag) + } -func validateCatalogManagerConnection(ctx basecontext.ApiContext, managerURL string, username string, password string, apiKey string) error { +func validateCatalogManagerConnection(ctx basecontext.ApiContext, managerURL string, username string, password string, apiKey string, diag *errors.Diagnostics) { if strings.TrimSpace(managerURL) == "" { - return errors.New("catalog manager url is required") + diag.AddError(strconv.Itoa(http.StatusBadRequest), "catalog manager url is required", "Failed to validate catalog manager connection") + return } if apiKey == "" && username == "" && password == "" { - return errors.New("missing authentication credentials, provide api_key or username/password") + diag.AddError(strconv.Itoa(http.StatusBadRequest), "missing authentication credentials, provide api_key or username/password", "Failed to validate catalog manager connection") + return } if apiKey == "" && ((username != "" && password == "") || (username == "" && password != "")) { - return errors.New("both username and password are required for password authentication") + diag.AddError(strconv.Itoa(http.StatusBadRequest), "both username and password are required for password authentication", "Failed to validate catalog manager connection") + return } - if err := validateCatalogManagerUrl(managerURL); err != nil { - return fmt.Errorf("invalid catalog manager url: %w", err) + validateCatalogManagerUrl(managerURL, diag) + if diag.HasErrors() { + return } - targetURL, err := buildCatalogManagerTargetUrl(managerURL, "/catalog", "") - if err != nil { - return fmt.Errorf("invalid catalog manager url: %w", err) + targetURL := buildCatalogManagerTargetUrl(managerURL, "/catalog", "", diag) + if diag.HasErrors() { + return } client := apiclient.NewHttpClient(ctx) @@ -1126,10 +1147,11 @@ func validateCatalogManagerConnection(ctx basecontext.ApiContext, managerURL str var response interface{} if _, err := client.Get(targetURL, &response); err != nil { - return fmt.Errorf("unable to authenticate catalog manager connection: %w", err) + rsp := models.NewFromError(err) + diag.AddError(strconv.Itoa(rsp.Code), fmt.Sprintf("unable to authenticate catalog manager connection: %v", err), "validateCatalogManagerConnection") + return } - return nil } // isCatalogManifestVisible returns true if the calling user's effective @@ -1258,25 +1280,31 @@ func filterCatalogManifestResponse(authCtx *basecontext.AuthorizationContext, bo return body, originalStatus } -func forwardCatalogManagerRequest(ctx basecontext.ApiContext, w http.ResponseWriter, r *http.Request, manager *data_models.CatalogManager, endpointPath string) error { - targetURL, err := buildCatalogManagerTargetUrl(manager.URL, endpointPath, r.URL.RawQuery) - if err != nil { - return err +func forwardCatalogManagerRequest(ctx basecontext.ApiContext, w http.ResponseWriter, r *http.Request, manager *data_models.CatalogManager, endpointPath string, diag *errors.Diagnostics) { + targetURL := buildCatalogManagerTargetUrl(manager.URL, endpointPath, r.URL.RawQuery, diag) + if diag.HasErrors() { + return } requestBody, err := io.ReadAll(r.Body) if err != nil { - return err + rsp := models.NewFromError(err) + diag.AddError(strconv.Itoa(rsp.Code), "Failed to read request body: "+rsp.Message, "forwardCatalogManagerRequest") + return } outboundRequest, err := http.NewRequestWithContext(r.Context(), r.Method, targetURL, bytes.NewReader(requestBody)) if err != nil { - return err + rsp := models.NewFromError(err) + diag.AddError(strconv.Itoa(rsp.Code), "Failed to create outbound request: "+rsp.Message, "forwardCatalogManagerRequest") + return } authorizer, err := getCatalogManagerAuthorizer(ctx, *manager, targetURL) if err != nil { - return err + rsp := models.NewFromError(err) + diag.AddError(strconv.Itoa(rsp.Code), "Failed to get catalog manager authorizer: "+rsp.Message, "forwardCatalogManagerRequest") + return } for headerKey, values := range r.Header { @@ -1347,7 +1375,9 @@ func forwardCatalogManagerRequest(ctx basecontext.ApiContext, w http.ResponseWri response, err := httpClient.Do(outboundRequest) if err != nil { - return err + rsp := models.NewFromError(err) + diag.AddError(strconv.Itoa(rsp.Code), rsp.Message, "forwardCatalogManagerRequest") + return } defer response.Body.Close() @@ -1383,17 +1413,28 @@ func forwardCatalogManagerRequest(ctx basecontext.ApiContext, w http.ResponseWri if r.Method == http.MethodGet && response.StatusCode == http.StatusOK { bodyBytes, readErr := io.ReadAll(response.Body) if readErr != nil { - return readErr + rsp := models.NewFromError(readErr) + diag.AddError(strconv.Itoa(rsp.Code), rsp.Message, "forwardCatalogManagerRequest") + return } // authCtx := ctx.GetAuthorizationContext() // filteredBody, filteredStatus := filterCatalogManifestResponse(authCtx, bodyBytes, response.StatusCode) // w.WriteHeader(filteredStatus) w.WriteHeader(response.StatusCode) _, err = w.Write(bodyBytes) - return err + if err != nil { + rsp := models.NewFromError(err) + diag.AddError(strconv.Itoa(rsp.Code), rsp.Message, "forwardCatalogManagerRequest") + return + } + return } w.WriteHeader(response.StatusCode) _, err = io.Copy(w, response.Body) - return err + if err != nil { + rsp := models.NewFromError(err) + diag.AddError(strconv.Itoa(rsp.Code), rsp.Message, "forwardCatalogManagerRequest") + return + } } diff --git a/src/controllers/machines.go b/src/controllers/machines.go index 13e27c52..4c45569f 100644 --- a/src/controllers/machines.go +++ b/src/controllers/machines.go @@ -1906,7 +1906,7 @@ func createVagrantBox(ctx basecontext.ApiContext, request models.CreateVirtualMa func createCatalogMachine(ctx basecontext.ApiContext, request models.CreateVirtualMachineRequest, jobID string) (*models.CreateVirtualMachineResponse, error) { provider := serviceprovider.Get() - + createCatalogMachineDiag := errors.NewDiagnostics("createCatalogMachine") parallelsDesktopService := provider.ParallelsDesktopService catalogConnection, err := resolveCatalogMachineConnection(ctx, request.CatalogManifest) @@ -1925,9 +1925,9 @@ func createCatalogMachine(ctx basecontext.ApiContext, request models.CreateVirtu pullRequest.StartAfterPull = request.StartOnCreate // Validate architecture and path - arch, err := catalog_helpers.ValidateArch(pullRequest.Architecture) - if err != nil { - return nil, err + arch := catalog_helpers.ValidateArch(pullRequest.Architecture, createCatalogMachineDiag) + if createCatalogMachineDiag.HasErrors() { + return nil, fmt.Errorf("invalid architecture: %s", createCatalogMachineDiag.GetErrors()[0].Message) } path, err := catalog_helpers.ValidatePath(pullRequest.Path, pullRequest.Owner) if err != nil { @@ -2025,6 +2025,7 @@ func createCatalogMachine(ctx basecontext.ApiContext, request models.CreateVirtu } func resolveCatalogMachineConnection(ctx basecontext.ApiContext, request *models.CreateCatalogVirtualMachineRequest) (string, error) { + resolveCatalogMachineConnectionDiag := errors.NewDiagnostics("resolveCatalogMachineConnection") if request == nil { return "", errors.NewWithCode("missing catalog manifest request", http.StatusBadRequest) } @@ -2041,9 +2042,9 @@ func resolveCatalogMachineConnection(ctx basecontext.ApiContext, request *models return "", errors.NewWithCode(errResp.Message, errResp.Code) } - managerConnection, err := buildCatalogManagerConnection(*mgr) - if err != nil { - return "", errors.NewWithCode(err.Error(), http.StatusBadRequest) + managerConnection := buildCatalogManagerConnection(*mgr, resolveCatalogMachineConnectionDiag) + if resolveCatalogMachineConnectionDiag.HasErrors() { + return "", fmt.Errorf("invalid connection: %s", resolveCatalogMachineConnectionDiag.GetErrors()[0].Message) } return managerConnection, nil } diff --git a/src/errors/main.go b/src/errors/main.go index 3b522750..1fb80329 100644 --- a/src/errors/main.go +++ b/src/errors/main.go @@ -200,3 +200,8 @@ func NewWithCodeAndNestedErrorf(sysError SystemError, code int, format string, a return err } + +const ( + MissingURL = 1000 // Specifically for empty/missing strings + InvalidURL = 1001 // Specifically for bad formatting or schemes +) diff --git a/src/helpers/url.go b/src/helpers/url.go index 9e7f4402..b77f1702 100644 --- a/src/helpers/url.go +++ b/src/helpers/url.go @@ -4,6 +4,7 @@ import ( "net" "net/url" "os" + "strconv" "strings" "github.com/Parallels/prl-devops-service/constants" @@ -48,59 +49,66 @@ func init() { } } -func ValidateUrl(urlStr string) error { +func ValidateUrl(urlStr string, diag *errors.Diagnostics) { if urlStr == "" { - return errors.New("url is required") + diag.AddError(strconv.Itoa(errors.MissingURL), "url is required", "ValidateUrl") + return } parsedURL, err := url.Parse(urlStr) if err != nil { - return errors.New("invalid url format") + diag.AddError(strconv.Itoa(errors.InvalidURL), "invalid url format", "ValidateUrl") + return } if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { - return errors.New("only http and https protocols are allowed") + diag.AddError(strconv.Itoa(errors.InvalidURL), "only http and https protocols are allowed", "") + return } if parsedURL.Host == "" { - return errors.New("url must contain a hostname") + diag.AddError(strconv.Itoa(errors.InvalidURL), "url must contain a hostname", "") + return } - - return nil } -func IsUrlAllowed(urlStr string) error { - if err := ValidateUrl(urlStr); err != nil { - return err +func IsUrlAllowed(urlStr string, diag *errors.Diagnostics) { + ValidateUrl(urlStr, diag) + if diag.HasErrors() { + return } parsedURL, err := url.Parse(urlStr) if err != nil { - return errors.New("invalid url format") + diag.AddError(strconv.Itoa(errors.InvalidURL), "invalid url format", "IsUrlAllowed") + return } host := parsedURL.Hostname() if isWhitelisted(host) { - return nil + return } if isForbiddenHostname(host) { - return errors.New("access to this url is not allowed") + diag.AddError(strconv.Itoa(errors.InvalidURL), "access to this url is not allowed", "") + return } ips, err := net.LookupIP(host) if err != nil { - return errors.New("unable to resolve hostname") + diag.AddError(strconv.Itoa(errors.InvalidURL), "unable to resolve hostname", "") + return } for _, ip := range ips { if isForbiddenIP(ip) { - return errors.New("access to this url is not allowed") + diag.AddError(strconv.Itoa(errors.InvalidURL), "access to this url is not allowed", "") + return } } - return nil + return } func isForbiddenHostname(hostname string) bool { @@ -205,16 +213,18 @@ func GetResolvedIPs(hostname string) ([]net.IP, error) { return ips, nil } -func ValidateCatalogManagerUrl(managerURL string) error { +func ValidateCatalogManagerUrl(managerURL string, diag *errors.Diagnostics) { if isUrlValidationDisabled() { - return nil + return } - if err := IsUrlAllowed(managerURL); err != nil { - return err + IsUrlAllowed(managerURL, diag) + + if diag.HasErrors() { + return } - return nil + return } func isUrlValidationDisabled() bool { diff --git a/src/helpers/url_test.go b/src/helpers/url_test.go index 60dd1d55..1b29abda 100644 --- a/src/helpers/url_test.go +++ b/src/helpers/url_test.go @@ -3,6 +3,8 @@ package helpers import ( "net" "testing" + + "github.com/Parallels/prl-devops-service/errors" ) func TestValidateUrl(t *testing.T) { @@ -23,12 +25,13 @@ func TestValidateUrl(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := ValidateUrl(tt.url) - if tt.expectError && err == nil { + diag := errors.NewDiagnostics("test") + ValidateUrl(tt.url, diag) + if tt.expectError && !diag.HasErrors() { t.Errorf("expected error, got nil") } - if !tt.expectError && err != nil { - t.Errorf("unexpected error: %v", err) + if !tt.expectError && diag.HasErrors() { + t.Errorf("unexpected error: %v", diag.GetErrors()[0].Message) } }) } @@ -57,12 +60,13 @@ func TestIsUrlAllowed(t *testing.T) { continue } t.Run(tt.name, func(t *testing.T) { - err := IsUrlAllowed(tt.url) - if tt.expectError && err == nil { + diag := errors.NewDiagnostics("test") + IsUrlAllowed(tt.url, diag) + if tt.expectError && !diag.HasErrors() { t.Errorf("expected error, got nil") } - if !tt.expectError && err != nil { - t.Errorf("unexpected error: %v", err) + if !tt.expectError && diag.HasErrors() { + t.Errorf("unexpected error: %v", diag.GetErrors()[0].Message) } }) } @@ -185,12 +189,13 @@ func TestIsUrlAllowed_WithWhitelist(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := IsUrlAllowed(tt.url) - if tt.expectError && err == nil { + diag := errors.NewDiagnostics("test") + IsUrlAllowed(tt.url, diag) + if tt.expectError && !diag.HasErrors() { t.Errorf("expected error, got nil") } - if !tt.expectError && err != nil { - t.Errorf("unexpected error: %v", err) + if !tt.expectError && diag.HasErrors() { + t.Errorf("unexpected error: %v", diag.GetErrors()[0].Message) } }) }