Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ai-services/Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
REGISTRY?=icr.io/ai-services-private
IMAGE=ai-services
TAG?=v0.0.258
TAG?=v0.0.259
CONTAINER_BUILDER?=podman
CREDS_ARG := $(if $(and $(REGISTRY_USER),$(REGISTRY_PASSWORD)),--creds="$(REGISTRY_USER):$(REGISTRY_PASSWORD)")

Expand Down
2 changes: 1 addition & 1 deletion ai-services/assets/catalog/openshift/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ ui:
cpu: "500m"

backend:
image: icr.io/ai-services-cicd/ai-services:v0.0.258
image: icr.io/ai-services-cicd/ai-services:v0.0.259
runtime: "openshift"
adminPasswordHash: ""
# @generate:password length=32, special=false
Expand Down
2 changes: 1 addition & 1 deletion ai-services/assets/catalog/podman/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ ui:

backend:
port: ""
image: icr.io/ai-services-cicd/ai-services:v0.0.258
image: icr.io/ai-services-cicd/ai-services:v0.0.259
runtime: ""
adminPasswordHash: ""
# @generate:password length=32, special=false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"strings"

"github.com/gin-gonic/gin"
"github.com/google/uuid"
"github.com/project-ai-services/ai-services/internal/pkg/catalog/apiserver/middleware"
"github.com/project-ai-services/ai-services/internal/pkg/catalog/apiserver/repository"
bundlesvc "github.com/project-ai-services/ai-services/internal/pkg/catalog/apiserver/services/bundle"
Expand Down Expand Up @@ -125,15 +124,6 @@ func (h *BundleHandler) UpdateBundle(c *gin.Context) {

bundleID := c.Param("id")

// Validate UUID format upfront — avoids a round-trip to the DB for a malformed ID.
if _, err := uuid.Parse(bundleID); err != nil {
c.JSON(http.StatusBadRequest, ErrorResponse{
Error: fmt.Sprintf("bundle_id %q is not a valid UUID; use the bundle ID returned by list or create", bundleID),
})

return
}

existing, err := h.bundleService.GetBundleByID(c.Request.Context(), bundleID)
if err != nil {
h.mapServiceError(c, err)
Expand Down Expand Up @@ -186,11 +176,27 @@ func (h *BundleHandler) UpdateBundle(c *gin.Context) {
// @Failure 404 {object} ErrorResponse "Bundle not found"
// @Router /catalog/bundles/{id} [delete]
func (h *BundleHandler) DeleteBundle(c *gin.Context) {
// TODO: read id path param via c.Param("id")
// TODO: call h.bundleService.GetBundleRecord — return 404 if nil
// TODO: call h.bundleService.DeleteBundle(c.Request.Context(), existing)
// TODO: return 204 on success
c.Status(http.StatusNotImplemented)
bundleID := c.Param("id")

existing, err := h.bundleService.GetBundleByID(c.Request.Context(), bundleID)
if err != nil {
h.mapServiceError(c, err)

return
}
if existing == nil {
c.JSON(http.StatusNotFound, ErrorResponse{Error: fmt.Sprintf("bundle %q not found", bundleID)})

return
}

if err := h.bundleService.DeleteBundle(c.Request.Context(), existing); err != nil {
h.mapServiceError(c, err)

return
}

c.Status(http.StatusNoContent)
}

// ListBundles godoc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type mockBundleService struct {
validateBundle func(ctx context.Context, file io.Reader) (any, error)
replaceBundle func(ctx context.Context, existing *bundlesvc.BundleResponse, file io.Reader, userID string) (*bundlesvc.BundleResponse, error)
getBundleByID func(ctx context.Context, id string) (*bundlesvc.BundleResponse, error)
deleteBundle func(ctx context.Context, existing *bundlesvc.BundleResponse) error
listBundles func(ctx context.Context, params bundlesvc.BundleListRequest) (*bundlesvc.BundleListResponse, error)
}

Expand All @@ -53,7 +54,10 @@ func (m *mockBundleService) GetBundleByID(ctx context.Context, id string) (*bund
}
panic("GetBundleByID not set")
}
func (m *mockBundleService) DeleteBundle(_ context.Context, _ *bundlesvc.BundleRecord) error {
func (m *mockBundleService) DeleteBundle(ctx context.Context, existing *bundlesvc.BundleResponse) error {
if m.deleteBundle != nil {
return m.deleteBundle(ctx, existing)
}
panic("DeleteBundle not set")
}
func (m *mockBundleService) ListBundles(ctx context.Context, params bundlesvc.BundleListRequest) (*bundlesvc.BundleListResponse, error) {
Expand All @@ -77,6 +81,7 @@ func setupBundleRouter(svc bundlesvc.BundleServiceInterface) *gin.Engine {
r.GET("/api/v1/catalog/bundles", h.ListBundles)
r.GET("/api/v1/catalog/bundles/:id", h.GetBundle)
r.PUT("/api/v1/catalog/bundles/:id", h.UpdateBundle)
r.DELETE("/api/v1/catalog/bundles/:id", h.DeleteBundle)
return r
}

Expand Down Expand Up @@ -551,12 +556,13 @@ func TestUpdateBundle(t *testing.T) {
wantLocationOf: "/api/v1/catalog/bundles/" + fixedID,
},
{
name: "400 — malformed UUID rejected before DB lookup",
name: "400 — malformed UUID rejected by GetBundleByID",
bundleID: "not-a-uuid",
filename: "bundle-v2.tar.gz",
fileContent: validTarGz,
getErr: &validators.ValidationError{Code: http.StatusBadRequest, Message: `invalid bundle id "not-a-uuid"`},
wantStatus: http.StatusBadRequest,
wantErrContains: "not a valid UUID",
wantErrContains: "invalid bundle id",
},
{
name: "404 — bundle not found (GetBundleByID returns nil)",
Expand Down Expand Up @@ -822,3 +828,143 @@ func TestUpdateBundle_FilenameExtensionCaseInsensitive(t *testing.T) {
router.ServeHTTP(w, req)
assert.Equal(t, http.StatusOK, w.Code)
}

// -----------------------------------------------------------------------
// TestDeleteBundle
// -----------------------------------------------------------------------

func TestDeleteBundle(t *testing.T) {
fixedID := "550e8400-e29b-41d4-a716-446655440000"

tests := []struct {
name string
bundleID string
// getBundleByID stub
getResp *bundlesvc.BundleResponse
getErr error
// deleteBundle stub — only consulted when getResp != nil
deleteErr error
wantStatus int
wantErrContains string
}{
{
name: "204 — successful delete",
bundleID: fixedID,
getResp: fixedBundleResponse(),
wantStatus: http.StatusNoContent,
},
{
name: "400 — malformed UUID rejected by GetBundleByID",
bundleID: "not-a-uuid",
getErr: &validators.ValidationError{Code: http.StatusBadRequest, Message: `invalid bundle id "not-a-uuid"`},
wantStatus: http.StatusBadRequest,
wantErrContains: "invalid bundle id",
},
{
name: "404 — bundle not found (GetBundleByID returns nil)",
bundleID: fixedID,
getResp: nil,
wantStatus: http.StatusNotFound,
wantErrContains: "not found",
},
{
name: "400 — GetBundleByID returns ValidationError",
bundleID: fixedID,
getErr: &validators.ValidationError{Code: http.StatusBadRequest, Message: "invalid bundle id"},
wantStatus: http.StatusBadRequest,
wantErrContains: "invalid bundle id",
},
{
name: "500 — GetBundleByID returns unexpected error",
bundleID: fixedID,
getErr: assert.AnError,
wantStatus: http.StatusInternalServerError,
},
{
name: "409 — running instances from DeleteBundle",
bundleID: fixedID,
getResp: fixedBundleResponse(),
deleteErr: &validators.ValidationError{Code: http.StatusConflict, Message: "cannot replace bundle"},
wantStatus: http.StatusConflict,
wantErrContains: "cannot replace bundle",
},
{
name: "500 — unexpected error from DeleteBundle",
bundleID: fixedID,
getResp: fixedBundleResponse(),
deleteErr: assert.AnError,
wantStatus: http.StatusInternalServerError,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
svc := &mockBundleService{
getBundleByID: func(_ context.Context, id string) (*bundlesvc.BundleResponse, error) {
assert.Equal(t, tt.bundleID, id)
return tt.getResp, tt.getErr
},
deleteBundle: func(_ context.Context, existing *bundlesvc.BundleResponse) error {
if tt.getResp != nil {
assert.Equal(t, tt.getResp.ID, existing.ID)
assert.Equal(t, tt.getResp.CatalogType, existing.CatalogType)
assert.Equal(t, tt.getResp.CatalogID, existing.CatalogID)
assert.Equal(t, tt.getResp.Version, existing.Version)
}
return tt.deleteErr
},
}

router := setupBundleRouter(svc)
w := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodDelete, "/api/v1/catalog/bundles/"+tt.bundleID, nil)
router.ServeHTTP(w, req)

assert.Equal(t, tt.wantStatus, w.Code)

if tt.wantErrContains != "" {
var body map[string]string
require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body))
assert.Contains(t, body["error"], tt.wantErrContains)
}
})
}
}

// TestDeleteBundle_ResponseForwardedToDeleteBundle verifies that the *BundleResponse
// returned by GetBundleByID is passed directly to DeleteBundle without any conversion.
func TestDeleteBundle_ResponseForwardedToDeleteBundle(t *testing.T) {
fixedID := "550e8400-e29b-41d4-a716-446655440000"
sz := int64(2048)
now := time.Now().UTC()
stubResp := &bundlesvc.BundleResponse{
ID: fixedID,
Name: "Full Fields Service",
Status: "active",
CatalogType: "service",
CatalogID: "full-svc",
Version: "3.0.0",
CreatedBy: "some-user",
SizeBytes: &sz,
CreatedAt: now,
UpdatedAt: now,
}

svc := &mockBundleService{
getBundleByID: func(_ context.Context, _ string) (*bundlesvc.BundleResponse, error) {
return stubResp, nil
},
deleteBundle: func(_ context.Context, existing *bundlesvc.BundleResponse) error {
// The handler must pass the exact *BundleResponse pointer — no intermediate
// BundleRecord construction allowed.
assert.Same(t, stubResp, existing)
return nil
},
}

router := setupBundleRouter(svc)
w := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodDelete, "/api/v1/catalog/bundles/"+fixedID, nil)
router.ServeHTTP(w, req)
assert.Equal(t, http.StatusNoContent, w.Code)
}
Loading
Loading