From 069613e24b1705f47cbd1d30ae6c99a4c601eed9 Mon Sep 17 00:00:00 2001 From: Mallory Hill Date: Mon, 15 Jun 2026 15:44:49 -0400 Subject: [PATCH] HYPERFLEET-1183 - test: Updating api tests for Versions/Channels --- .gitignore | 5 + Makefile | 62 +++ pkg/services/resource_test.go | 70 +++ test/integration/channels_test.go | 370 +++++++++++++++ test/integration/resource_delete_test.go | 97 ---- test/integration/resource_helpers.go | 57 +++ test/integration/versions_test.go | 568 +++++++++++++++++++++++ 7 files changed, 1132 insertions(+), 97 deletions(-) create mode 100644 test/integration/channels_test.go delete mode 100644 test/integration/resource_delete_test.go create mode 100644 test/integration/resource_helpers.go create mode 100644 test/integration/versions_test.go diff --git a/.gitignore b/.gitignore index a571a42d..837cac6c 100755 --- a/.gitignore +++ b/.gitignore @@ -27,7 +27,12 @@ # Ignore master key for decrypting credentials and more. /config/master.key +# Coverage files /coverage +coverage.out +coverage-integration.out +coverage-filtered.out +coverage-integration-filtered.out # Ignore built binaries /bin/ diff --git a/Makefile b/Makefile index 4a737cfa..28cf45c5 100755 --- a/Makefile +++ b/Makefile @@ -208,6 +208,68 @@ ci-test-integration: install $(GOTESTSUM) ## Run integration tests with JSON out .PHONY: test-all test-all: lint test test-integration test-helm ## Run all checks (lint, unit, integration, helm) +.PHONY: test-coverage +test-coverage: ## Run unit tests with coverage (excludes generated code) + @echo "Running unit tests with coverage..." + @$(MAKE) test TESTFLAGS="-coverprofile=coverage.out -covermode=atomic -count=1" + @if [ -f coverage.out ]; then \ + echo "Filtering out generated code (mocks, openapi) from coverage..."; \ + grep -v -E '(_mock\.go|/mocks/|/openapi/)' coverage.out > coverage-filtered.out || true; \ + mv coverage-filtered.out coverage.out; \ + fi + @echo "" + @if [ -f coverage.out ]; then \ + echo ""; \ + echo "Coverage summary (excluding generated code):"; \ + echo "Total Coverage: $$(go tool cover -func=coverage.out | tail -1 | awk '{print $$3}')"; \ + echo ""; \ + echo "To view detailed HTML coverage report, run: make coverage-html"; \ + else \ + echo "No coverage file generated."; \ + fi + +.PHONY: test-coverage-integration +test-coverage-integration: ## Run integration tests with coverage (excludes generated code) + @echo "Running integration tests with coverage..." + @$(MAKE) test-integration TESTFLAGS="-coverprofile=coverage-integration.out -covermode=atomic -coverpkg=./pkg/...,./cmd/... -count=1" + @if [ -f coverage-integration.out ]; then \ + echo "Filtering out generated code (mocks, openapi) from coverage..."; \ + grep -v -E '(_mock\.go|/mocks/|/openapi/)' coverage-integration.out > coverage-integration-filtered.out || true; \ + mv coverage-integration-filtered.out coverage-integration.out; \ + fi + @echo "" + @if [ -f coverage-integration.out ]; then \ + echo ""; \ + echo "Coverage summary (excluding generated code):"; \ + echo "Total Coverage: $$(go tool cover -func=coverage-integration.out | tail -1 | awk '{print $$3}')"; \ + echo ""; \ + echo "To view detailed HTML coverage report, run: make coverage-integration-html"; \ + fi + +.PHONY: coverage-html +coverage-html: ## Open HTML coverage report for unit tests + @if [ ! -f coverage.out ]; then \ + echo "No coverage.out file found. Run 'make test-coverage' first."; \ + exit 1; \ + fi + @echo "Opening coverage report in browser..." + @go tool cover -html=coverage.out + +.PHONY: coverage-integration-html +coverage-integration-html: ## Open HTML coverage report for integration tests + @if [ ! -f coverage-integration.out ]; then \ + echo "No coverage-integration.out file found. Run 'make test-coverage-integration' first."; \ + exit 1; \ + fi + @echo "Opening coverage report in browser..." + @go tool cover -html=coverage-integration.out + +.PHONY: coverage-clean +coverage-clean: ## Remove all coverage files + @echo "Cleaning coverage files..." + @rm -f coverage.out coverage-integration.out coverage-unfiltered.out + @echo "Coverage files removed." + ##@ Agent Verification .PHONY: verify-all diff --git a/pkg/services/resource_test.go b/pkg/services/resource_test.go index 35312a16..460e67ec 100644 --- a/pkg/services/resource_test.go +++ b/pkg/services/resource_test.go @@ -843,6 +843,76 @@ func TestResourceService_Delete_CascadeSkipsAlreadyDeletedChild(t *testing.T) { Expect(preDeletedExists).To(BeFalse()) } +// --- List --- + +func TestResourceService_List_InjectsKindFilter(t *testing.T) { + RegisterTestingT(t) + setupTestDescriptors() + + mockDao := newMockResourceDao() + svc, _, generic := newTestResourceService(mockDao) + + args := &ListArguments{Page: 1, Size: 100} + _, _, svcErr := svc.List(context.Background(), "Channel", args) + Expect(svcErr).To(BeNil()) + Expect(args.Search).To(Equal(""), "original args should not be mutated") + Expect(generic.listCalled).To(BeTrue()) + Expect(generic.lastSearch).To(Equal("kind = 'Channel'")) +} + +func TestResourceService_List_NilArgs(t *testing.T) { + RegisterTestingT(t) + setupTestDescriptors() + + mockDao := newMockResourceDao() + svc, _, generic := newTestResourceService(mockDao) + + _, _, svcErr := svc.List(context.Background(), "Version", nil) + Expect(svcErr).To(BeNil()) + Expect(generic.listCalled).To(BeTrue()) + Expect(generic.lastSearch).To(Equal("kind = 'Version'")) +} + +func TestResourceService_List_AppendsToExistingSearch(t *testing.T) { + RegisterTestingT(t) + setupTestDescriptors() + + mockDao := newMockResourceDao() + svc, _, generic := newTestResourceService(mockDao) + + args := &ListArguments{Page: 1, Size: 100, Search: "name = 'stable'"} + _, _, svcErr := svc.List(context.Background(), "Channel", args) + Expect(svcErr).To(BeNil()) + Expect(args.Search).To(Equal("name = 'stable'"), "original args should not be mutated") + Expect(generic.lastSearch).To(Equal("(name = 'stable') AND kind = 'Channel'")) +} + +func TestResourceService_List_UnknownKind(t *testing.T) { + RegisterTestingT(t) + setupTestDescriptors() + + mockDao := newMockResourceDao() + svc, _, _ := newTestResourceService(mockDao) + + _, _, svcErr := svc.List(context.Background(), "UnknownKind", nil) + Expect(svcErr).ToNot(BeNil()) + Expect(svcErr.Reason).To(ContainSubstring("Unknown entity kind")) +} + +func TestResourceService_List_GenericServiceError(t *testing.T) { + RegisterTestingT(t) + setupTestDescriptors() + + mockDao := newMockResourceDao() + svc, _, generic := newTestResourceService(mockDao) + + generic.listErr = errors.GeneralError("database connection lost") + + _, _, svcErr := svc.List(context.Background(), "Channel", nil) + Expect(svcErr).ToNot(BeNil()) + Expect(svcErr.Reason).To(ContainSubstring("database connection lost")) +} + // --- GetByOwner --- func TestResourceService_GetByOwner_HappyPath(t *testing.T) { diff --git a/test/integration/channels_test.go b/test/integration/channels_test.go new file mode 100644 index 00000000..a96471a2 --- /dev/null +++ b/test/integration/channels_test.go @@ -0,0 +1,370 @@ +package integration + +import ( + "context" + "encoding/json" + "fmt" + "testing" + "time" + + "github.com/google/uuid" + . "github.com/onsi/gomega" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" + "github.com/openshift-hyperfleet/hyperfleet-api/test" + + _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/channels" +) + +// Test helpers +func setupChannelTest(t *testing.T) (services.ResourceService, *test.Helper) { + h, _ := test.RegisterIntegration(t) + return resourceService(), h +} + +func createChannel(t *testing.T, svc services.ResourceService, name string) *api.Resource { + channel := newChannelResource(name) + created, err := svc.Create(context.Background(), "Channel", channel) + if err != nil { + t.Fatalf("Failed to create channel: %v", err) + } + return created +} + +// Channel Create +func TestChannelCreate(t *testing.T) { + t.Run("UniqueConstraint", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + channelName := fmt.Sprintf("unique-channel-%s", uuid.NewString()[:8]) + channel1 := createChannel(t, svc, channelName) + Expect(channel1.ID).NotTo(BeEmpty()) + + // Attempt to create duplicate - should fail + duplicate := newChannelResource(channelName) + _, svcErr := svc.Create(context.Background(), "Channel", duplicate) + Expect(svcErr).ToNot(BeNil(), "duplicate channel name should fail") + Expect(svcErr.HTTPCode).To(Equal(409)) + }) + + t.Run("EmptyName", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + channel := newChannelResource("") + _, svcErr := svc.Create(context.Background(), "Channel", channel) + Expect(svcErr).ToNot(BeNil(), "empty channel name should fail") + Expect(svcErr.HTTPCode).To(Equal(400)) + }) + + t.Run("WithLabels", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + channelName := fmt.Sprintf("channel-with-labels-%s", uuid.NewString()[:8]) + channel := newChannelResource(channelName) + labels := map[string]string{ + "environment": "test", + "team": "platform", + } + + channel.Labels, _ = json.Marshal(labels) + createdChannel, svcErr := svc.Create(context.Background(), "Channel", channel) + Expect(svcErr).To(BeNil()) + Expect(createdChannel.Labels).NotTo(BeNil()) + + // Get the resource and verify labels persisted + retrieved, getErr := svc.Get(context.Background(), "Channel", createdChannel.ID) + Expect(getErr).To(BeNil(), "should retrieve channel") + var retrievedLabels map[string]string + json.Unmarshal(retrieved.Labels, &retrievedLabels) + Expect(retrievedLabels).To(Equal(labels), "retrieved labels should match") + }) + + t.Run("SetsTimestamps", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + channelName := fmt.Sprintf("timestamp-test-%s", uuid.NewString()[:8]) + before := time.Now() + channel := createChannel(t, svc, channelName) + after := time.Now() + + Expect(channel.CreatedTime).To(BeTemporally(">=", before)) + Expect(channel.CreatedTime).To(BeTemporally("<=", after)) + Expect(channel.UpdatedTime).To(BeTemporally(">=", before)) + Expect(channel.UpdatedTime).To(BeTemporally("<=", after)) + }) +} + +// Channel Delete +func TestChannelDelete(t *testing.T) { + t.Run("NotFound", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + _, svcErr := svc.Delete(context.Background(), "Channel", "nonexistent-id") + Expect(svcErr).ToNot(BeNil(), "delete of nonexistent channel should fail") + Expect(svcErr.HTTPCode).To(Equal(404)) + }) + + t.Run("SetsDeletedTime", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + channelName := fmt.Sprintf("delete-timestamp-%s", uuid.NewString()[:8]) + channel := createChannel(t, svc, channelName) + + before := time.Now() + deleted, svcErr := svc.Delete(context.Background(), "Channel", channel.ID) + after := time.Now() + + Expect(svcErr).To(BeNil()) + Expect(deleted.DeletedTime).NotTo(BeNil()) + Expect(*deleted.DeletedTime).To(BeTemporally(">=", before)) + Expect(*deleted.DeletedTime).To(BeTemporally("<=", after)) + }) + + t.Run("ReDeleteReturns404", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + channelName := fmt.Sprintf("redelete-test-%s", uuid.NewString()[:8]) + channel := createChannel(t, svc, channelName) + + // Delete once - should succeed + _, svcErr := svc.Delete(context.Background(), "Channel", channel.ID) + Expect(svcErr).To(BeNil()) + + // Delete again - should return 404 + _, svcErr = svc.Delete(context.Background(), "Channel", channel.ID) + Expect(svcErr).ToNot(BeNil()) + Expect(svcErr.HTTPCode).To(Equal(404)) + }) + + t.Run("HardDeleteRemovesRow", func(t *testing.T) { + svc, h := setupChannelTest(t) + + channelName := fmt.Sprintf("hard-delete-%s", uuid.NewString()[:8]) + channel := createChannel(t, svc, channelName) + + // Delete the channel + result, svcErr := svc.Delete(context.Background(), "Channel", channel.ID) + Expect(svcErr).To(BeNil()) + Expect(result.DeletedTime).ToNot(BeNil()) + + // Verify hard delete removed the row + dbErr := checkResourceCount(h, []string{channel.ID}, 0) + Expect(dbErr).To(BeNil()) + }) +} + +// Channel List +func TestChannelList(t *testing.T) { + t.Run("Basic", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + // Create test channels with unique names + channels := make([]*api.Resource, 3) + for i := range 3 { + name := fmt.Sprintf("list-test-%s-%d", uuid.NewString()[:8], i) + channels[i] = createChannel(t, svc, name) + } + + // List channels + args := &services.ListArguments{ + Page: 1, + Size: 100, + } + list, _, svcErr := svc.List(context.Background(), "Channel", args) + Expect(svcErr).To(BeNil(), "list should succeed") + Expect(len(list)).To(BeNumerically(">=", 3), "should have at least 3 channels") + + // Verify our channels are in the list + foundIDs := make(map[string]bool) + for _, item := range list { + foundIDs[item.ID] = true + } + + for _, ch := range channels { + Expect(foundIDs[ch.ID]).To(BeTrue(), "should find created channel") + } + }) + + t.Run("Pagination", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + // Create 15 channels with unique prefix + prefix := uuid.NewString()[:8] + for i := range 15 { + name := fmt.Sprintf("pagination-%s-%d", prefix, i) + createChannel(t, svc, name) + } + + // Page 1 + args := &services.ListArguments{ + Page: 1, + Size: 10, + } + list, _, svcErr := svc.List(context.Background(), "Channel", args) + Expect(svcErr).To(BeNil(), "list should succeed") + Expect(len(list)).To(BeNumerically(">=", 10), "page 1 should have at least 10 items") + + // Page 2 + args.Page = 2 + list, _, svcErr = svc.List(context.Background(), "Channel", args) + Expect(svcErr).To(BeNil(), "list should succeed") + Expect(len(list)).To(BeNumerically(">=", 5), "page 2 should have at least 5 items") + }) + + t.Run("WithOrdering", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + // Create channels with specific names for ordering + prefix := uuid.NewString()[:8] + ch1 := createChannel(t, svc, fmt.Sprintf("order-c-%s", prefix)) + ch2 := createChannel(t, svc, fmt.Sprintf("order-a-%s", prefix)) + ch3 := createChannel(t, svc, fmt.Sprintf("order-b-%s", prefix)) + + // Test ordering by name ascending + args := &services.ListArguments{ + Page: 1, + Size: 100, + OrderBy: []string{"name asc"}, + } + list, _, svcErr := svc.List(context.Background(), "Channel", args) + Expect(svcErr).To(BeNil(), "list should succeed") + + // Find our test channels in the list + channelPositions := make(map[string]int) + for i, item := range list { + if item.ID == ch1.ID || item.ID == ch2.ID || item.ID == ch3.ID { + channelPositions[item.ID] = i + } + } + + Expect(len(channelPositions)).To(Equal(3), "should find all 3 test channels") + + // Verify order-a comes before order-b comes before order-c + Expect(channelPositions[ch2.ID]).To(BeNumerically("<", channelPositions[ch3.ID]), + "order-a should come before order-b") + Expect(channelPositions[ch3.ID]).To(BeNumerically("<", channelPositions[ch1.ID]), + "order-b should come before order-c") + }) +} + +// Channel Get +func TestChannelGet(t *testing.T) { + t.Run("NotFound", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + _, svcErr := svc.Get(context.Background(), "Channel", "nonexistent-id") + Expect(svcErr).ToNot(BeNil(), "get of nonexistent channel should fail") + Expect(svcErr.HTTPCode).To(Equal(404)) + }) + + t.Run("Success", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + channelName := fmt.Sprintf("get-test-%s", uuid.NewString()[:8]) + channel := createChannel(t, svc, channelName) + + // Get channel should succeed + retrieved, svcErr := svc.Get(context.Background(), "Channel", channel.ID) + Expect(svcErr).To(BeNil(), "get should succeed") + Expect(retrieved.ID).To(Equal(channel.ID)) + Expect(retrieved.Name).To(Equal(channel.Name)) + }) +} + +// Channel Patch +func TestChannelPatch(t *testing.T) { + t.Run("NotFound", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + req := &api.ResourcePatchRequest{ + Spec: &map[string]any{"enabled_regex": ".*"}, + } + + _, svcErr := svc.Patch(context.Background(), "Channel", "nonexistent-id", req) + Expect(svcErr).ToNot(BeNil(), "patch of nonexistent channel should fail") + Expect(svcErr.HTTPCode).To(Equal(404)) + }) + + t.Run("SpecChanged", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + channelName := fmt.Sprintf("patch-spec-%s", uuid.NewString()[:8]) + channel := createChannel(t, svc, channelName) + Expect(channel.Generation).To(Equal(int32(1)), "initial generation should be 1") + + // Patch spec + newSpec := map[string]any{ + "is_default": true, + "enabled_regex": "4\\.17\\..*", + } + req := &api.ResourcePatchRequest{ + Spec: &newSpec, + } + patched, svcErr := svc.Patch(context.Background(), "Channel", channel.ID, req) + Expect(svcErr).To(BeNil(), "patch should succeed") + Expect(patched.Generation).To(Equal(int32(2)), "generation should increment to 2") + + // Unmarshal and verify spec + var patchedSpec map[string]any + json.Unmarshal(patched.Spec, &patchedSpec) + Expect(patchedSpec["is_default"]).To(Equal(true)) + Expect(patchedSpec["enabled_regex"]).To(Equal("4\\.17\\..*")) + + // Verify persisted + retrieved, getErr := svc.Get(context.Background(), "Channel", channel.ID) + Expect(getErr).To(BeNil()) + Expect(retrieved.Generation).To(Equal(int32(2))) + + var retrievedSpec map[string]any + json.Unmarshal(retrieved.Spec, &retrievedSpec) + Expect(retrievedSpec).To(Equal(patchedSpec)) + }) + + t.Run("LabelsOnly", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + channelName := fmt.Sprintf("patch-labels-%s", uuid.NewString()[:8]) + channel := createChannel(t, svc, channelName) + Expect(channel.Generation).To(Equal(int32(1)), "initial generation should be 1") + + // Patch labels only + newLabels := map[string]string{ + "patched": "true", + "env": "staging", + } + req := &api.ResourcePatchRequest{ + Labels: &newLabels, + } + patched, svcErr := svc.Patch(context.Background(), "Channel", channel.ID, req) + Expect(svcErr).To(BeNil(), "patch should succeed") + Expect(patched.Generation).To(Equal(int32(2)), "generation should increment to 2") + + // Verify labels + var patchedLabels map[string]string + json.Unmarshal(patched.Labels, &patchedLabels) + Expect(patchedLabels).To(Equal(newLabels)) + }) + + t.Run("UpdatesTimestamps", func(t *testing.T) { + svc, _ := setupChannelTest(t) + + channelName := fmt.Sprintf("patch-timestamp-%s", uuid.NewString()[:8]) + channel := createChannel(t, svc, channelName) + originalUpdatedTime := channel.UpdatedTime + + // Sleep briefly to ensure timestamp difference + time.Sleep(5 * time.Millisecond) + + // Patch the channel + newSpec := map[string]any{"is_default": true} + req := &api.ResourcePatchRequest{ + Spec: &newSpec, + } + patched, svcErr := svc.Patch(context.Background(), "Channel", channel.ID, req) + + Expect(svcErr).To(BeNil()) + Expect(patched.UpdatedTime).To(BeTemporally(">", originalUpdatedTime), + "updated_time should be later than original after patch") + }) +} diff --git a/test/integration/resource_delete_test.go b/test/integration/resource_delete_test.go deleted file mode 100644 index dc455994..00000000 --- a/test/integration/resource_delete_test.go +++ /dev/null @@ -1,97 +0,0 @@ -package integration - -import ( - "context" - "testing" - - . "github.com/onsi/gomega" - - "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/environments" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" - "github.com/openshift-hyperfleet/hyperfleet-api/test" - - _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/channels" - "github.com/openshift-hyperfleet/hyperfleet-api/plugins/resources" - _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/versions" -) - -func resourceService() services.ResourceService { - return resources.Service(&environments.Environment().Services) -} - -func createChannel(svc services.ResourceService, name string) *api.Resource { - channel := &api.Resource{ - Kind: "Channel", - Name: name, - Spec: []byte(`{"stability": "stable"}`), - CreatedBy: "test@example.com", - UpdatedBy: "test@example.com", - } - created, svcErr := svc.Create(context.Background(), "Channel", channel) - Expect(svcErr).To(BeNil()) - return created -} - -func createVersion(svc services.ResourceService, name string, parent *api.Resource) *api.Resource { - version := &api.Resource{ - Kind: "Version", - Name: name, - Spec: []byte(`{"version": "` + name + `"}`), - CreatedBy: "test@example.com", - UpdatedBy: "test@example.com", - } - version.SetOwner(parent.ID, parent.Kind, parent.Href) - created, svcErr := svc.Create(context.Background(), "Version", version) - Expect(svcErr).To(BeNil()) - return created -} - -func assertResourceCount(h *test.Helper, ids []string, expected int64) { - dbSession := h.DBFactory.New(context.Background()) - var count int64 - err := dbSession.Model(&api.Resource{}).Where("id IN ?", ids).Count(&count).Error - Expect(err).To(BeNil()) - Expect(count).To(Equal(expected)) -} - -func TestResourceDelete_HardDeleteRemovesRow(t *testing.T) { - h, _ := test.RegisterIntegration(t) - svc := resourceService() - - created := createChannel(svc, "hard-delete-test") - - result, svcErr := svc.Delete(context.Background(), "Channel", created.ID) - Expect(svcErr).To(BeNil()) - Expect(result.DeletedTime).ToNot(BeNil()) - - assertResourceCount(h, []string{created.ID}, 0) -} - -func TestResourceDelete_RestrictBlocksWithActiveChild(t *testing.T) { - h, _ := test.RegisterIntegration(t) - svc := resourceService() - - parent := createChannel(svc, "restrict-parent") - child := createVersion(svc, "4.18", parent) - - _, svcErr := svc.Delete(context.Background(), "Channel", parent.ID) - Expect(svcErr).ToNot(BeNil()) - Expect(svcErr.HTTPCode).To(Equal(409)) - - assertResourceCount(h, []string{parent.ID, child.ID}, 2) -} - -func TestResourceDelete_ReDeleteReturns404(t *testing.T) { - _, _ = test.RegisterIntegration(t) - svc := resourceService() - - created := createChannel(svc, "redelete-test") - - _, svcErr := svc.Delete(context.Background(), "Channel", created.ID) - Expect(svcErr).To(BeNil()) - - _, svcErr = svc.Delete(context.Background(), "Channel", created.ID) - Expect(svcErr).ToNot(BeNil()) - Expect(svcErr.HTTPCode).To(Equal(404)) -} diff --git a/test/integration/resource_helpers.go b/test/integration/resource_helpers.go new file mode 100644 index 00000000..30682881 --- /dev/null +++ b/test/integration/resource_helpers.go @@ -0,0 +1,57 @@ +package integration + +import ( + "context" + "fmt" + + "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/environments" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" + "github.com/openshift-hyperfleet/hyperfleet-api/plugins/resources" + "github.com/openshift-hyperfleet/hyperfleet-api/test" +) + +func resourceService() services.ResourceService { + return resources.Service(&environments.Environment().Services) +} + +func checkResourceCount(h *test.Helper, ids []string, expected int64) error { + dbSession := h.DBFactory.New(context.Background()) + var count int64 + err := dbSession.Model(&api.Resource{}).Where("id IN ?", ids).Count(&count).Error + if err != nil { + return fmt.Errorf("failed to count resources with ids %v: %w", ids, err) + } + if count != expected { + return fmt.Errorf("expected %d resources, got %d", expected, count) + } + return nil +} + +// newChannelResource creates a Channel resource struct with default spec. +// Does NOT persist to database - use svc.Create() to persist. +func newChannelResource(name string) *api.Resource { + return &api.Resource{ + Kind: "Channel", + Name: name, + Spec: []byte(`{"is_default": false, "enabled_regex": ".*"}`), + CreatedBy: "test@example.com", + UpdatedBy: "test@example.com", + } +} + +// newVersionResource creates a Version resource struct with default spec. +// Does NOT persist to database - use svc.Create() to persist. +func newVersionResource(name, channelID string) *api.Resource { + ownerKind := "Channel" + return &api.Resource{ + Kind: "Version", + Name: name, + OwnerID: &channelID, + OwnerKind: &ownerKind, + Spec: []byte(`{"raw_version": "4.17.0", "enabled": true, "is_default": false,` + + `"release_image": "quay.io/openshift-release-dev/ocp-release:4.17.0"}`), + CreatedBy: "test@example.com", + UpdatedBy: "test@example.com", + } +} diff --git a/test/integration/versions_test.go b/test/integration/versions_test.go new file mode 100644 index 00000000..87f46b21 --- /dev/null +++ b/test/integration/versions_test.go @@ -0,0 +1,568 @@ +package integration + +import ( + "context" + "encoding/json" + "fmt" + "testing" + "time" + + "github.com/google/uuid" + . "github.com/onsi/gomega" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" + "github.com/openshift-hyperfleet/hyperfleet-api/test" + + _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/channels" + _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/versions" +) + +// Version Test helpers +func setupVersionTest(t *testing.T) (services.ResourceService, *test.Helper) { + h, _ := test.RegisterIntegration(t) + return resourceService(), h +} + +func createTestChannel(t *testing.T, svc services.ResourceService) *api.Resource { + // unique string for channel name + uniqueSuffix := uuid.NewString()[:8] + channelName := fmt.Sprintf("channel-%s", uniqueSuffix) + + channel := newChannelResource(channelName) + created, err := svc.Create(context.Background(), "Channel", channel) + if err != nil { + t.Fatalf("Failed to create channel: %v", err) + } + return created +} + +func createTestVersion(t *testing.T, svc services.ResourceService, name, channelID string) *api.Resource { + version := newVersionResource(name, channelID) + created, err := svc.Create(context.Background(), "Version", version) + if err != nil { + t.Fatalf("Failed to create version: %v", err) + } + return created +} + +func expectCreateError(svc services.ResourceService, resource *api.Resource, expectedCode int, msg string) { + _, svcErr := svc.Create(context.Background(), resource.Kind, resource) + Expect(svcErr).ToNot(BeNil(), msg) + Expect(svcErr.HTTPCode).To(Equal(expectedCode)) +} + +// Version Create +func TestVersionCreate(t *testing.T) { + t.Run("UniqueConstraintPerChannel", func(t *testing.T) { + svc, _ := setupVersionTest(t) + channel := createTestChannel(t, svc) + + versionName := "4.17.0" + version1 := createTestVersion(t, svc, versionName, channel.ID) + Expect(version1.ID).NotTo(BeEmpty()) + + // Attempt to create duplicate - should fail + duplicate := newVersionResource(versionName, channel.ID) + expectCreateError(svc, duplicate, 409, "duplicate version name should fail") + }) + + t.Run("SameVersionNameInDifferentChannels", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel1 := createTestChannel(t, svc) + channel2 := createTestChannel(t, svc) + + versionName := "version" + version1 := createTestVersion(t, svc, versionName, channel1.ID) + version2 := createTestVersion(t, svc, versionName, channel2.ID) + + Expect(version1.ID).NotTo(BeEmpty()) + Expect(version2.ID).NotTo(BeEmpty()) + }) + + t.Run("EmptyName", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version := newVersionResource("", channel.ID) + expectCreateError(svc, version, 400, "empty version name should fail") + }) + + t.Run("WrongParentKind", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + ownerKind := "RandomKind_NotChannel" + randomID := uuid.NewString() + version := &api.Resource{ + Kind: "Version", + Name: "version-wrong-parent-kind", + OwnerID: &randomID, + OwnerKind: &ownerKind, + Spec: []byte(`{"raw_version": "4.17.0", "enabled": true}`), + CreatedBy: "test@example.com", + UpdatedBy: "test@example.com", + } + expectCreateError(svc, version, 404, "wrong parent kind should fail") + }) + + t.Run("ParentDeleted", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + _, svcErr := svc.Delete(context.Background(), "Channel", channel.ID) + Expect(svcErr).To(BeNil()) + + version := newVersionResource("version-marked-for-deletion", channel.ID) + expectCreateError(svc, version, 404, "parent marked for deletion should fail") + }) + + t.Run("ParentNotFound", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + nonExistentParentID := "non-existent-channel-id" + version := newVersionResource("orphan-version", nonExistentParentID) + expectCreateError(svc, version, 404, "non-existent parent should fail") + }) + + t.Run("WithDeletedParent", func(t *testing.T) { + svc, h := setupVersionTest(t) + + channel := createTestChannel(t, svc) + _, svcErr := svc.Delete(context.Background(), "Channel", channel.ID) + Expect(svcErr).To(BeNil(), "channel deletion should succeed") + + dbErr := checkResourceCount(h, []string{channel.ID}, 0) + Expect(dbErr).To(BeNil()) + + version := newVersionResource("orphan-version", channel.ID) + expectCreateError(svc, version, 404, "deleted parent should fail") + }) + + t.Run("WithLabels", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version := newVersionResource("version-with-labels", channel.ID) + labels := map[string]string{ + "environment": "test", + "team": "platform", + } + + version.Labels, _ = json.Marshal(labels) + createdVersion, svcErr := svc.Create(context.Background(), "Version", version) + Expect(svcErr).To(BeNil()) + Expect(createdVersion.Labels).NotTo(BeNil()) + + // Get the resource and verify labels persisted + retrieved, getErr := svc.Get(context.Background(), "Version", createdVersion.ID) + Expect(getErr).To(BeNil(), "should retrieve version") + var retrievedLabels map[string]string + jsonErr := json.Unmarshal(retrieved.Labels, &retrievedLabels) + Expect(jsonErr).To(BeNil(), "should unmarshal retrieved labels") + Expect(retrievedLabels).To(Equal(labels), "retrieved labels should match") + }) + + t.Run("SetsTimestamps", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + before := time.Now() + version := createTestVersion(t, svc, "timestamp-test-version", channel.ID) + after := time.Now() + + Expect(version.CreatedTime).To(BeTemporally(">=", before), + "created_time should be set during create") + Expect(version.CreatedTime).To(BeTemporally("<=", after), + "created_time should be set during create") + Expect(version.UpdatedTime).To(BeTemporally(">=", before), + "updated_time should be set during create") + Expect(version.UpdatedTime).To(BeTemporally("<=", after), + "updated_time should be set during create") + }) +} + +// Version Delete +func TestVersionDelete(t *testing.T) { + t.Run("CascadeFromChannel", func(t *testing.T) { + svc, h := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version1 := createTestVersion(t, svc, "version1", channel.ID) + version2 := createTestVersion(t, svc, "version2", channel.ID) + + // Delete all versions first + _, svcErr := svc.Delete(context.Background(), "Version", version1.ID) + Expect(svcErr).To(BeNil()) + _, svcErr = svc.Delete(context.Background(), "Version", version2.ID) + Expect(svcErr).To(BeNil()) + + // Now delete channel + _, svcErr = svc.Delete(context.Background(), "Channel", channel.ID) + Expect(svcErr).To(BeNil()) + + // Verify all deleted + dbErr := checkResourceCount(h, []string{channel.ID, version1.ID, version2.ID}, 0) + Expect(dbErr).To(BeNil()) + }) + + t.Run("NotFound", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + _, svcErr := svc.Delete(context.Background(), "Version", "nonexistent-id") + Expect(svcErr).ToNot(BeNil(), "delete of nonexistent version should fail") + Expect(svcErr.HTTPCode).To(Equal(404), "should return 404 Not Found") + }) + + t.Run("SetsDeletedTime", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version := createTestVersion(t, svc, "delete-timestamp-test", channel.ID) + + before := time.Now() + deleted, svcErr := svc.Delete(context.Background(), "Version", version.ID) + after := time.Now() + + Expect(svcErr).To(BeNil()) + Expect(deleted.DeletedTime).NotTo(BeNil(), "deleted_time should be set on delete") + Expect(*deleted.DeletedTime).To(BeTemporally(">=", before)) + Expect(*deleted.DeletedTime).To(BeTemporally("<=", after)) + }) + + t.Run("ReDeleteReturns404", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version := createTestVersion(t, svc, "redelete-test", channel.ID) + + // Delete once - should succeed + _, svcErr := svc.Delete(context.Background(), "Version", version.ID) + Expect(svcErr).To(BeNil()) + + // Delete again - should return 404 + _, svcErr = svc.Delete(context.Background(), "Version", version.ID) + Expect(svcErr).ToNot(BeNil()) + Expect(svcErr.HTTPCode).To(Equal(404)) + }) + + t.Run("HardDeleteRemovesRow", func(t *testing.T) { + svc, h := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version := createTestVersion(t, svc, "hard-delete-test", channel.ID) + + // Delete the version + result, svcErr := svc.Delete(context.Background(), "Version", version.ID) + Expect(svcErr).To(BeNil()) + Expect(result.DeletedTime).ToNot(BeNil()) + + // Verify hard delete removed the row + dbErr := checkResourceCount(h, []string{version.ID}, 0) + Expect(dbErr).To(BeNil()) + }) + + t.Run("RestrictBlocksWithActiveParent", func(t *testing.T) { + svc, h := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version := createTestVersion(t, svc, "restrict-test", channel.ID) + + // Attempt to delete parent with active child - should fail with 409 + _, svcErr := svc.Delete(context.Background(), "Channel", channel.ID) + Expect(svcErr).ToNot(BeNil()) + Expect(svcErr.HTTPCode).To(Equal(409)) + + // Verify both still exist + dbErr := checkResourceCount(h, []string{channel.ID, version.ID}, 2) + Expect(dbErr).To(BeNil()) + }) +} + +// Version List +func TestVersionList(t *testing.T) { + t.Run("ListByOwnerID", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + + version1 := createTestVersion(t, svc, "version1", channel.ID) + version2 := createTestVersion(t, svc, "version2", channel.ID) + version3 := createTestVersion(t, svc, "version3", channel.ID) + + // List versions in channel1 + args := &services.ListArguments{ + Page: 1, + Size: 100, + Search: "owner_id='" + channel.ID + "'", + } + list, _, svcErr := svc.List(context.Background(), "Version", args) + Expect(svcErr).To(BeNil(), "list should succeed") + Expect(list).To(HaveLen(3), "channel1 should have 3 versions") + + // Verify all versions belong to channel1 + foundIDs := make(map[string]bool) + for _, item := range list { + Expect(item.OwnerID).NotTo(BeNil()) + Expect(*item.OwnerID).To(Equal(channel.ID), "all items should belong to channel") + foundIDs[item.ID] = true + } + + // Verify we got all 3 expected versions + Expect(foundIDs[version1.ID]).To(BeTrue(), "should find version1") + Expect(foundIDs[version2.ID]).To(BeTrue(), "should find version2") + Expect(foundIDs[version3.ID]).To(BeTrue(), "should find version3") + }) + + t.Run("ListByLabel", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + uniqueLabel := uuid.NewString()[:8] + + // Create version with unique label + version1 := newVersionResource("version-with-label-1", channel.ID) + labels := map[string]string{ + "environment": uniqueLabel, + } + version1.Labels, _ = json.Marshal(labels) + created1, svcErr := svc.Create(context.Background(), "Version", version1) + Expect(svcErr).To(BeNil()) + Expect(created1.Labels).NotTo(BeNil()) + + // Create another version with the same label + version2 := newVersionResource("version-with-label-2", channel.ID) + version2.Labels, _ = json.Marshal(labels) + created2, svcErr := svc.Create(context.Background(), "Version", version2) + Expect(svcErr).To(BeNil()) + Expect(created2.Labels).NotTo(BeNil()) + + // Create version without the label + version3 := createTestVersion(t, svc, "version-no-label", channel.ID) + + // List by label + args := &services.ListArguments{ + Page: 1, + Size: 100, + Search: "labels.environment='" + uniqueLabel + "'", + } + list, _, svcErr := svc.List(context.Background(), "Version", args) + Expect(svcErr).To(BeNil(), "list by label should succeed") + Expect(len(list)).To(BeNumerically(">=", 2), "should find at least 2 versions with the label") + + // Verify all returned versions have the label + for _, item := range list { + var itemLabels map[string]string + err := json.Unmarshal(item.Labels, &itemLabels) + Expect(err).To(BeNil()) + Expect(itemLabels["environment"]).To(Equal(uniqueLabel)) + } + + // Verify version3 is not in the list + for _, item := range list { + Expect(item.ID).NotTo(Equal(version3.ID), "version without label should not be in results") + } + }) + + t.Run("Pagination", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + for i := range 15 { + version := createTestVersion(t, svc, fmt.Sprintf("version-%d", i), channel.ID) + Expect(version.ID).ToNot(BeNil()) + } + // Page 1 + args := &services.ListArguments{ + Page: 1, + Size: 10, + Search: "owner_id='" + channel.ID + "'", + } + list, _, svcErr := svc.List(context.Background(), "Version", args) + Expect(svcErr).To(BeNil(), "list should succeed") + Expect(list).To(HaveLen(10), "page 1 should have 10 items") + + // Page 2 + args.Page = 2 + list, _, svcErr = svc.List(context.Background(), "Version", args) + Expect(svcErr).To(BeNil(), "list should succeed") + Expect(len(list)).To(BeNumerically(">=", 5), "page 2 should have at least 5 items") + }) + + t.Run("ByOwner", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version1 := createTestVersion(t, svc, "version1", channel.ID) + version2 := createTestVersion(t, svc, "version2", channel.ID) + version3 := createTestVersion(t, svc, "version3", channel.ID) + + // List versions by owner + list, _, svcErr := svc.ListByOwner(context.Background(), "Version", channel.ID, &services.ListArguments{ + Page: 1, + Size: 100, + }) + Expect(svcErr).To(BeNil(), "list by owner should succeed") + Expect(list).To(HaveLen(3), "should have 3 versions") + + // Verify all versions belong to the channel + foundIDs := make(map[string]bool) + for _, item := range list { + Expect(item.OwnerID).NotTo(BeNil()) + Expect(*item.OwnerID).To(Equal(channel.ID), "all items should belong to channel") + foundIDs[item.ID] = true + } + + // Verify we got all 3 expected versions + Expect(foundIDs[version1.ID]).To(BeTrue(), "should find version1") + Expect(foundIDs[version2.ID]).To(BeTrue(), "should find version2") + Expect(foundIDs[version3.ID]).To(BeTrue(), "should find version3") + }) +} + +// Version Get +func TestVersionGet(t *testing.T) { + t.Run("ByOwnerWrongParent", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel1 := createTestChannel(t, svc) + version := createTestVersion(t, svc, "version", channel1.ID) + + channel2 := createTestChannel(t, svc) + + // Attempt to get version from channel2 (wrong parent) + _, svcErr := svc.GetByOwner(context.Background(), "Version", version.ID, channel2.ID) + Expect(svcErr).ToNot(BeNil(), "version should not be found under wrong parent") + Expect(svcErr.HTTPCode).To(Equal(404), "should return 404 Not Found") + }) + + t.Run("NotFound", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + _, svcErr := svc.Get(context.Background(), "Version", "nonexistent-id") + Expect(svcErr).ToNot(BeNil(), "get of nonexistent version should fail") + Expect(svcErr.HTTPCode).To(Equal(404), "should return 404 Not Found") + }) + + t.Run("ByOwnerNotFound", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + + // Try to get non-existent version under a real parent + _, svcErr := svc.GetByOwner(context.Background(), "Version", "nonexistent-version-id", channel.ID) + Expect(svcErr).ToNot(BeNil(), "get of nonexistent version should fail") + Expect(svcErr.HTTPCode).To(Equal(404), "should return 404 Not Found") + }) + + t.Run("ByOwnerSuccess", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version := createTestVersion(t, svc, "test-version", channel.ID) + + // Get version by owner should succeed + retrieved, svcErr := svc.GetByOwner(context.Background(), "Version", version.ID, channel.ID) + Expect(svcErr).To(BeNil(), "get by owner should succeed") + Expect(retrieved.ID).To(Equal(version.ID)) + Expect(retrieved.Name).To(Equal(version.Name)) + Expect(retrieved.OwnerID).NotTo(BeNil()) + Expect(*retrieved.OwnerID).To(Equal(channel.ID)) + }) +} + +// Version Patch +func TestVersionPatch(t *testing.T) { + t.Run("NotFound", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + req := &api.ResourcePatchRequest{ + Spec: &map[string]any{"enabled": true}, + } + + _, svcErr := svc.Patch(context.Background(), "Version", "nonexistent-id", req) + Expect(svcErr).ToNot(BeNil(), "patch of nonexistent version should fail") + Expect(svcErr.HTTPCode).To(Equal(404), "should return 404 Not Found") + }) + + t.Run("SpecChanged", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version := createTestVersion(t, svc, "patch-spec-test", channel.ID) + Expect(version.Generation).To(Equal(int32(1)), "initial generation should be 1") + + // Patch spec + newSpec := map[string]any{ + "raw_version": "4.18.0", + "enabled": true, + "is_default": false, + "release_image": "quay.io/openshift-release-dev/ocp-release:4.18.0", + } + req := &api.ResourcePatchRequest{ + Spec: &newSpec, + } + _, svcErr := svc.Patch(context.Background(), "Version", version.ID, req) + + Expect(svcErr).To(BeNil(), "patch should succeed") + + // Verify patched request persisted + retrieved, svcErr := svc.Get(context.Background(), "Version", version.ID) + Expect(svcErr).To(BeNil()) + // Verify updated version is incremented + Expect(retrieved.Generation).To(Equal(int32(2))) + // Verify updated spec persisted + var retrievedSpecValues map[string]any + jsonErr := json.Unmarshal(retrieved.Spec, &retrievedSpecValues) + Expect(jsonErr).To(BeNil()) + Expect(retrievedSpecValues).To(Equal(newSpec), "patched spec should match retrieved spec") + }) + + t.Run("LabelsOnly", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version := createTestVersion(t, svc, "patch-labels-test", channel.ID) + Expect(version.Generation).To(Equal(int32(1)), "initial generation should be 1") + + // Patch labels only, no spec change + newLabels := map[string]string{ + "patched": "true", + "environment": "test", + } + req := &api.ResourcePatchRequest{ + Labels: &newLabels, + } + _, svcErr := svc.Patch(context.Background(), "Version", version.ID, req) + Expect(svcErr).To(BeNil()) + + retrieved, getErr := svc.Get(context.Background(), "Version", version.ID) + Expect(getErr).To(BeNil()) + // Verify updated version is incremented + Expect(retrieved.Generation).To(Equal(int32(2))) + var retrievedLabelValues map[string]string + jsonErr := json.Unmarshal(retrieved.Labels, &retrievedLabelValues) + Expect(jsonErr).To(BeNil()) + Expect(retrievedLabelValues).To(Equal(newLabels), "patched labels should match retrieved labels") + + }) + + t.Run("UpdatesTimestamps", func(t *testing.T) { + svc, _ := setupVersionTest(t) + + channel := createTestChannel(t, svc) + version := createTestVersion(t, svc, "timestamp-update-test", channel.ID) + originalUpdatedTime := version.UpdatedTime + + // Patch the version + newSpec := map[string]any{"enabled": false} + req := &api.ResourcePatchRequest{ + Spec: &newSpec, + } + patched, svcErr := svc.Patch(context.Background(), "Version", version.ID, req) + + Expect(svcErr).To(BeNil()) + Expect(patched.UpdatedTime).To(BeTemporally(">", originalUpdatedTime), + "updated_time should be later than original after patch") + }) +}