From 404ee0bf6bdab4c4fb3d3d8b229306f32962420d Mon Sep 17 00:00:00 2001 From: tithakka Date: Tue, 7 Jul 2026 10:36:50 -0500 Subject: [PATCH 1/7] HYPERFLEET-1154 - feat: Adapter status endpoints for generic resources --- claude-skills | 1 + pkg/dao/resource.go | 2 +- pkg/dao/resource_condition.go | 13 + pkg/handlers/resource_status_handler.go | 197 +++++++++++ pkg/handlers/resource_status_handler_test.go | 224 ++++++++++++ pkg/services/adapter_status_validation.go | 95 +++++ .../adapter_status_validation_test.go | 174 +++++++++ pkg/services/cluster.go | 75 +--- pkg/services/node_pool.go | 74 +--- pkg/services/resource.go | 280 ++++++++++++++- pkg/services/resource_test.go | 330 ++++++++++++++++-- plugins/entities/plugin.go | 36 +- plugins/entities/plugin_test.go | 41 ++- plugins/resources/plugin.go | 2 + test/integration/resource_status_test.go | 206 +++++++++++ 15 files changed, 1564 insertions(+), 186 deletions(-) create mode 160000 claude-skills create mode 100644 pkg/handlers/resource_status_handler.go create mode 100644 pkg/handlers/resource_status_handler_test.go create mode 100644 pkg/services/adapter_status_validation.go create mode 100644 pkg/services/adapter_status_validation_test.go create mode 100644 test/integration/resource_status_test.go diff --git a/claude-skills b/claude-skills new file mode 160000 index 00000000..33fdfa74 --- /dev/null +++ b/claude-skills @@ -0,0 +1 @@ +Subproject commit 33fdfa74a530cf76c4a686d6e653614b1182068e diff --git a/pkg/dao/resource.go b/pkg/dao/resource.go index ec09cb4d..18db1cdb 100644 --- a/pkg/dao/resource.go +++ b/pkg/dao/resource.go @@ -47,7 +47,7 @@ func (d *sqlResourceDao) Get(ctx context.Context, kind, id string) (*api.Resourc func (d *sqlResourceDao) GetForUpdate(ctx context.Context, kind, id string) (*api.Resource, error) { g2 := d.sessionFactory.New(ctx) var resource api.Resource - if err := g2.Clauses(clause.Locking{Strength: "UPDATE"}).Take( + if err := g2.Preload("Conditions").Clauses(clause.Locking{Strength: "UPDATE"}).Take( &resource, "kind = ? AND id = ?", kind, id).Error; err != nil { return nil, err } diff --git a/pkg/dao/resource_condition.go b/pkg/dao/resource_condition.go index 5cf6d9e3..c1c0e827 100644 --- a/pkg/dao/resource_condition.go +++ b/pkg/dao/resource_condition.go @@ -22,6 +22,10 @@ type ResourceConditionDao interface { // For new condition types, both are used as-is from the input — caller // must populate them. Zero timestamps are defaulted to now. UpdateConditions(ctx context.Context, resourceID string, conditions []api.ResourceCondition) error + + // DeleteByResource removes all condition rows for a resource. + // Used during hard-delete to clean up before removing the resource row. + DeleteByResource(ctx context.Context, resourceID string) error } var _ ResourceConditionDao = &sqlResourceConditionDao{} @@ -84,3 +88,12 @@ func (d *sqlResourceConditionDao) UpdateConditions( return nil } + +func (d *sqlResourceConditionDao) DeleteByResource(ctx context.Context, resourceID string) error { + g2 := d.sessionFactory.New(ctx) + if err := g2.Where("resource_id = ?", resourceID).Delete(&api.ResourceCondition{}).Error; err != nil { + db.MarkForRollback(ctx, err) + return err + } + return nil +} diff --git a/pkg/handlers/resource_status_handler.go b/pkg/handlers/resource_status_handler.go new file mode 100644 index 00000000..2a8d90bd --- /dev/null +++ b/pkg/handlers/resource_status_handler.go @@ -0,0 +1,197 @@ +package handlers + +import ( + "context" + "net/http" + + "github.com/gorilla/mux" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/presenters" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" +) + +// ResourceStatusHandler handles GET/PUT on /{plural}/{id}/statuses for +// config-driven generic resources. Follows the same pattern as +// ClusterStatusHandler but uses descriptor.Kind as the resource type +// and delegates to ResourceService.ProcessAdapterStatus. +type ResourceStatusHandler struct { + descriptor registry.EntityDescriptor + resourceService services.ResourceService + adapterStatusService services.AdapterStatusService +} + +func NewResourceStatusHandler( + descriptor registry.EntityDescriptor, + resourceService services.ResourceService, + adapterStatusService services.AdapterStatusService, +) *ResourceStatusHandler { + return &ResourceStatusHandler{ + descriptor: descriptor, + resourceService: resourceService, + adapterStatusService: adapterStatusService, + } +} + +// List returns all adapter statuses for a resource with pagination. +func (h *ResourceStatusHandler) List(w http.ResponseWriter, r *http.Request) { + cfg := &handlerConfig{ + Action: func() (interface{}, *errors.ServiceError) { + ctx := r.Context() + id := mux.Vars(r)["id"] + listArgs, err := services.NewListArguments(r.URL.Query()) + if err != nil { + return nil, err + } + + if _, err = h.resourceService.Get(ctx, h.descriptor.Kind, id); err != nil { + return nil, err + } + + return h.listStatuses(ctx, id, listArgs) + }, + ErrorHandler: handleError, + } + + handleList(w, r, cfg) +} + +// Create creates or updates an adapter status for a resource. +func (h *ResourceStatusHandler) Create(w http.ResponseWriter, r *http.Request) { + var req openapi.AdapterStatusCreateRequest + + cfg := &handlerConfig{ + MarshalInto: &req, + Validate: []validate{ + validateNotEmpty(&req, "Adapter", "adapter"), + validateObservedGeneration(&req), + validateConditions(&req, "Conditions"), + validateObservedTimeRange(&req.ObservedTime), + }, + Action: func() (interface{}, *errors.ServiceError) { + ctx := r.Context() + id := mux.Vars(r)["id"] + + if _, err := h.resourceService.Get(ctx, h.descriptor.Kind, id); err != nil { + return nil, err + } + + return h.processStatus(ctx, id, &req) + }, + ErrorHandler: handleError, + } + + handleCreateWithNoContent(w, r, cfg) +} + +// ListByOwner returns adapter statuses for a child resource, validating ownership. +func (h *ResourceStatusHandler) ListByOwner(w http.ResponseWriter, r *http.Request) { + cfg := &handlerConfig{ + Action: func() (interface{}, *errors.ServiceError) { + ctx := r.Context() + vars := mux.Vars(r) + parentID, id := vars["parent_id"], vars["id"] + listArgs, err := services.NewListArguments(r.URL.Query()) + if err != nil { + return nil, err + } + + if _, err = h.resourceService.GetByOwner(ctx, h.descriptor.Kind, id, parentID); err != nil { + return nil, err + } + + return h.listStatuses(ctx, id, listArgs) + }, + ErrorHandler: handleError, + } + + handleList(w, r, cfg) +} + +// CreateByOwner creates or updates an adapter status for a child resource, validating ownership. +func (h *ResourceStatusHandler) CreateByOwner(w http.ResponseWriter, r *http.Request) { + var req openapi.AdapterStatusCreateRequest + + cfg := &handlerConfig{ + MarshalInto: &req, + Validate: []validate{ + validateNotEmpty(&req, "Adapter", "adapter"), + validateObservedGeneration(&req), + validateConditions(&req, "Conditions"), + validateObservedTimeRange(&req.ObservedTime), + }, + Action: func() (interface{}, *errors.ServiceError) { + ctx := r.Context() + vars := mux.Vars(r) + parentID, id := vars["parent_id"], vars["id"] + + if _, err := h.resourceService.GetByOwner(ctx, h.descriptor.Kind, id, parentID); err != nil { + return nil, err + } + + return h.processStatus(ctx, id, &req) + }, + ErrorHandler: handleError, + } + + handleCreateWithNoContent(w, r, cfg) +} + +// listStatuses fetches paginated adapter statuses and presents them as an OpenAPI response. +// Shared by List and ListByOwner — the caller handles resource existence/ownership checks. +func (h *ResourceStatusHandler) listStatuses( + ctx context.Context, resourceID string, listArgs *services.ListArguments, +) (interface{}, *errors.ServiceError) { + adapterStatuses, total, err := h.adapterStatusService.FindByResourcePaginated( + ctx, h.descriptor.Kind, resourceID, listArgs, + ) + if err != nil { + return nil, err + } + + items := make([]openapi.AdapterStatus, 0, len(adapterStatuses)) + for _, as := range adapterStatuses { + presented, presErr := presenters.PresentAdapterStatus(as) + if presErr != nil { + return nil, errors.GeneralError("Failed to present adapter status: %v", presErr) + } + items = append(items, presented) + } + + return openapi.AdapterStatusList{ + Items: items, + Page: int32(listArgs.Page), + Size: int32(len(items)), + Total: int32(total), + }, nil +} + +// processStatus converts the request, delegates to ProcessAdapterStatus, and presents the result. +// Returns (nil, nil) when the status was silently discarded (triggers 204 No Content). +func (h *ResourceStatusHandler) processStatus( + ctx context.Context, resourceID string, req *openapi.AdapterStatusCreateRequest, +) (interface{}, *errors.ServiceError) { + newStatus, convErr := presenters.ConvertAdapterStatus(h.descriptor.Kind, resourceID, req) + if convErr != nil { + return nil, errors.GeneralError("Failed to convert adapter status: %v", convErr) + } + + adapterStatus, err := h.resourceService.ProcessAdapterStatus( + ctx, h.descriptor.Kind, resourceID, newStatus, + ) + if err != nil { + return nil, err + } + + if adapterStatus == nil { + return nil, nil + } + + status, presErr := presenters.PresentAdapterStatus(adapterStatus) + if presErr != nil { + return nil, errors.GeneralError("Failed to present adapter status: %v", presErr) + } + return &status, nil +} diff --git a/pkg/handlers/resource_status_handler_test.go b/pkg/handlers/resource_status_handler_test.go new file mode 100644 index 00000000..8712d6bb --- /dev/null +++ b/pkg/handlers/resource_status_handler_test.go @@ -0,0 +1,224 @@ +package handlers + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gorilla/mux" + . "github.com/onsi/gomega" + "go.uber.org/mock/gomock" + "gorm.io/datatypes" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" +) + +func newTestResourceStatusHandler( + ctrl *gomock.Controller, +) (*ResourceStatusHandler, *services.MockResourceService, *services.MockAdapterStatusService) { + mockResourceSvc := services.NewMockResourceService(ctrl) + mockAdapterSvc := services.NewMockAdapterStatusService(ctrl) + handler := NewResourceStatusHandler(channelDescriptor, mockResourceSvc, mockAdapterSvc) + return handler, mockResourceSvc, mockAdapterSvc +} + +func TestResourceStatusHandler_List(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, mockAdapterSvc := newTestResourceStatusHandler(ctrl) + + resource := &api.Resource{Kind: "Channel"} + resource.ID = "ch-1" + mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", "ch-1").Return(resource, nil) + + now := time.Now().UTC() + statuses := api.AdapterStatusList{ + { + Adapter: "adapter1", + ResourceType: "Channel", + ResourceID: "ch-1", + ObservedGeneration: 1, + LastReportTime: now, + Conditions: datatypes.JSON(`[{"type":"Available","status":"True"}]`), + }, + } + mockAdapterSvc.EXPECT().FindByResourcePaginated( + gomock.Any(), "Channel", "ch-1", gomock.Any(), + ).Return(statuses, int64(1), nil) + + r := httptest.NewRequest(http.MethodGet, "/channels/ch-1/statuses", nil) + r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + w := httptest.NewRecorder() + + handler.List(w, r) + + Expect(w.Code).To(Equal(http.StatusOK)) + + var response openapi.AdapterStatusList + Expect(json.Unmarshal(w.Body.Bytes(), &response)).To(Succeed()) + Expect(response.Items).To(HaveLen(1)) + Expect(response.Total).To(Equal(int32(1))) +} + +func TestResourceStatusHandler_List_ResourceNotFound(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, _ := newTestResourceStatusHandler(ctrl) + + mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", "ch-1"). + Return(nil, errors.NotFound("Channel 'ch-1' not found")) + + r := httptest.NewRequest(http.MethodGet, "/channels/ch-1/statuses", nil) + r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + w := httptest.NewRecorder() + + handler.List(w, r) + + Expect(w.Code).To(Equal(http.StatusNotFound)) +} + +func TestResourceStatusHandler_Create_HappyPath(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, _ := newTestResourceStatusHandler(ctrl) + + resource := &api.Resource{Kind: "Channel"} + resource.ID = "ch-1" + mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", "ch-1").Return(resource, nil) + + now := time.Now().UTC() + returnedStatus := &api.AdapterStatus{ + Adapter: "adapter1", + ResourceType: "Channel", + ResourceID: "ch-1", + ObservedGeneration: 1, + LastReportTime: now, + Conditions: datatypes.JSON(`[{"type":"Available","status":"True"},{"type":"Applied","status":"True"},{"type":"Health","status":"True"}]`), + } + mockResourceSvc.EXPECT().ProcessAdapterStatus( + gomock.Any(), "Channel", "ch-1", gomock.Any(), + ).Return(returnedStatus, nil) + + observedTime := now + body := openapi.AdapterStatusCreateRequest{ + Adapter: "adapter1", + ObservedGeneration: 1, + ObservedTime: observedTime, + Conditions: []openapi.ConditionRequest{ + {Type: "Available", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Applied", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Health", Status: openapi.AdapterConditionStatusTrue}, + }, + } + bodyJSON, _ := json.Marshal(body) + + r := httptest.NewRequest(http.MethodPut, "/channels/ch-1/statuses", strings.NewReader(string(bodyJSON))) + r.Header.Set("Content-Type", "application/json") + r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + w := httptest.NewRecorder() + + handler.Create(w, r) + + Expect(w.Code).To(Equal(http.StatusCreated)) +} + +func TestResourceStatusHandler_Create_Discarded_Returns204(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, _ := newTestResourceStatusHandler(ctrl) + + resource := &api.Resource{Kind: "Channel"} + resource.ID = "ch-1" + mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", "ch-1").Return(resource, nil) + mockResourceSvc.EXPECT().ProcessAdapterStatus( + gomock.Any(), "Channel", "ch-1", gomock.Any(), + ).Return(nil, nil) + + now := time.Now().UTC() + body := openapi.AdapterStatusCreateRequest{ + Adapter: "adapter1", + ObservedGeneration: 1, + ObservedTime: now, + Conditions: []openapi.ConditionRequest{ + {Type: "Available", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Applied", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Health", Status: openapi.AdapterConditionStatusTrue}, + }, + } + bodyJSON, _ := json.Marshal(body) + + r := httptest.NewRequest(http.MethodPut, "/channels/ch-1/statuses", strings.NewReader(string(bodyJSON))) + r.Header.Set("Content-Type", "application/json") + r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + w := httptest.NewRecorder() + + handler.Create(w, r) + + Expect(w.Code).To(Equal(http.StatusNoContent)) +} + +func TestResourceStatusHandler_Create_MissingAdapter_Returns400(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, _, _ := newTestResourceStatusHandler(ctrl) + + body := openapi.AdapterStatusCreateRequest{ + ObservedGeneration: 1, + Conditions: []openapi.ConditionRequest{ + {Type: "Available", Status: openapi.AdapterConditionStatusTrue}, + }, + } + bodyJSON, _ := json.Marshal(body) + + r := httptest.NewRequest(http.MethodPut, "/channels/ch-1/statuses", strings.NewReader(string(bodyJSON))) + r.Header.Set("Content-Type", "application/json") + r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + w := httptest.NewRecorder() + + handler.Create(w, r) + + Expect(w.Code).To(Equal(http.StatusBadRequest)) +} + +func TestResourceStatusHandler_Create_ResourceNotFound(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, _ := newTestResourceStatusHandler(ctrl) + + mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", "ch-1"). + Return(nil, errors.NotFound("Channel 'ch-1' not found")) + + now := time.Now().UTC() + body := openapi.AdapterStatusCreateRequest{ + Adapter: "adapter1", + ObservedGeneration: 1, + ObservedTime: now, + Conditions: []openapi.ConditionRequest{ + {Type: "Available", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Applied", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Health", Status: openapi.AdapterConditionStatusTrue}, + }, + } + bodyJSON, _ := json.Marshal(body) + + r := httptest.NewRequest(http.MethodPut, "/channels/ch-1/statuses", strings.NewReader(string(bodyJSON))) + r.Header.Set("Content-Type", "application/json") + r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + w := httptest.NewRecorder() + + handler.Create(w, r) + + Expect(w.Code).To(Equal(http.StatusNotFound)) +} diff --git a/pkg/services/adapter_status_validation.go b/pkg/services/adapter_status_validation.go new file mode 100644 index 00000000..2a3b4c4f --- /dev/null +++ b/pkg/services/adapter_status_validation.go @@ -0,0 +1,95 @@ +package services + +import ( + "encoding/json" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" +) + +// validateAndClassifyAdapterStatus performs all stateless validation and discard-rule +// checks on an incoming adapter status. Returns the parsed conditions and whether +// aggregation should be triggered. Returns (nil, false, nil) when the update should +// be silently discarded. +// +// This is the shared implementation used by ClusterService, NodePoolService, and +// ResourceService. Callers provide the resource generation and a pre-built logger +// with entity-specific context fields. +func validateAndClassifyAdapterStatus( + resourceGeneration int32, + adapterStatus *api.AdapterStatus, + existingStatus *api.AdapterStatus, + log *logger.ContextLogger, +) ([]api.AdapterCondition, bool, *errors.ServiceError) { + if adapterStatus.ObservedGeneration > resourceGeneration { + log.Debug("Discarding adapter status update: future generation") + return nil, false, nil + } + + if existingStatus != nil && adapterStatus.ObservedGeneration < existingStatus.ObservedGeneration { + log.Debug("Discarding adapter status update: stale generation") + return nil, false, nil + } + + incomingObs := AdapterObservedTime(adapterStatus) + if incomingObs.IsZero() { + log.Debug("Discarding adapter status update: zero observed time") + return nil, false, nil + } + + if existingStatus != nil && adapterStatus.ObservedGeneration == existingStatus.ObservedGeneration { + prevObs := AdapterObservedTime(existingStatus) + if !prevObs.IsZero() && incomingObs.Before(prevObs) { + log.Debug("Discarding adapter status update: stale observed time") + return nil, false, nil + } + } + + var conditions []api.AdapterCondition + if len(adapterStatus.Conditions) > 0 { + if errUnmarshal := json.Unmarshal(adapterStatus.Conditions, &conditions); errUnmarshal != nil { + return nil, false, errors.GeneralError( + "Failed to unmarshal adapter status conditions: %s", errUnmarshal, + ) + } + } + + if errorType, conditionName := ValidateMandatoryConditions(conditions); errorType != "" { + return nil, false, errors.Validation( + "missing mandatory condition '%s': all adapters must report Available, Applied, and Health", + conditionName, + ) + } + + triggerAggregation := false + for _, cond := range conditions { + if cond.Type != api.AdapterConditionTypeAvailable { + continue + } + + isValidStatus := cond.Status == api.AdapterConditionTrue || + cond.Status == api.AdapterConditionFalse || + cond.Status == api.AdapterConditionUnknown + if !isValidStatus { + return nil, false, errors.Validation( + "condition '%s' has invalid status '%s': must be True, False, or Unknown", + cond.Type, cond.Status, + ) + } + + if cond.Status != api.AdapterConditionTrue && cond.Status != api.AdapterConditionFalse { + if existingStatus != nil { + log.Debug("Discarding adapter status update: subsequent Unknown Available") + return nil, false, nil + } + triggerAggregation = false + break + } + + triggerAggregation = true + break + } + + return conditions, triggerAggregation, nil +} diff --git a/pkg/services/adapter_status_validation_test.go b/pkg/services/adapter_status_validation_test.go new file mode 100644 index 00000000..68230f1c --- /dev/null +++ b/pkg/services/adapter_status_validation_test.go @@ -0,0 +1,174 @@ +package services + +import ( + "context" + "encoding/json" + "testing" + "time" + + . "github.com/onsi/gomega" + "gorm.io/datatypes" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" +) + +func testConditionsJSON(conditions ...api.AdapterCondition) datatypes.JSON { + b, _ := json.Marshal(conditions) + return b +} + +func testMandatoryConditions(availableStatus api.AdapterConditionStatus) []api.AdapterCondition { + return []api.AdapterCondition{ + {Type: api.AdapterConditionTypeAvailable, Status: availableStatus}, + {Type: api.AdapterConditionTypeApplied, Status: api.AdapterConditionTrue}, + {Type: api.AdapterConditionTypeHealth, Status: api.AdapterConditionTrue}, + } +} + +func adapterStatusWithGenAndTime(gen int32, observedTime time.Time) *api.AdapterStatus { + return &api.AdapterStatus{ + Adapter: "test-adapter", + ObservedGeneration: gen, + LastReportTime: observedTime, + Conditions: testConditionsJSON(testMandatoryConditions(api.AdapterConditionTrue)...), + } +} + +func testLog() *logger.ContextLogger { + return logger.With(context.Background(), "test", "true") +} + +func TestValidateAndClassify_FutureGeneration_Discards(t *testing.T) { + RegisterTestingT(t) + + status := adapterStatusWithGenAndTime(5, time.Now()) + conditions, trigger, err := validateAndClassifyAdapterStatus(3, status, nil, testLog()) + + Expect(err).To(BeNil()) + Expect(conditions).To(BeNil()) + Expect(trigger).To(BeFalse()) +} + +func TestValidateAndClassify_StaleGeneration_Discards(t *testing.T) { + RegisterTestingT(t) + + existing := &api.AdapterStatus{ObservedGeneration: 3} + status := adapterStatusWithGenAndTime(2, time.Now()) + conditions, trigger, err := validateAndClassifyAdapterStatus(5, status, existing, testLog()) + + Expect(err).To(BeNil()) + Expect(conditions).To(BeNil()) + Expect(trigger).To(BeFalse()) +} + +func TestValidateAndClassify_ZeroObservedTime_Discards(t *testing.T) { + RegisterTestingT(t) + + status := &api.AdapterStatus{ + Adapter: "test-adapter", + ObservedGeneration: 1, + Conditions: testConditionsJSON(testMandatoryConditions(api.AdapterConditionTrue)...), + } + conditions, trigger, err := validateAndClassifyAdapterStatus(1, status, nil, testLog()) + + Expect(err).To(BeNil()) + Expect(conditions).To(BeNil()) + Expect(trigger).To(BeFalse()) +} + +func TestValidateAndClassify_StaleObservedTime_Discards(t *testing.T) { + RegisterTestingT(t) + + now := time.Now().UTC() + existing := adapterStatusWithGenAndTime(1, now) + status := adapterStatusWithGenAndTime(1, now.Add(-time.Minute)) + + conditions, trigger, err := validateAndClassifyAdapterStatus(1, status, existing, testLog()) + + Expect(err).To(BeNil()) + Expect(conditions).To(BeNil()) + Expect(trigger).To(BeFalse()) +} + +func TestValidateAndClassify_MissingMandatoryCondition_ReturnsError(t *testing.T) { + RegisterTestingT(t) + + status := adapterStatusWithGenAndTime(1, time.Now()) + status.Conditions = testConditionsJSON( + api.AdapterCondition{Type: api.AdapterConditionTypeAvailable, Status: api.AdapterConditionTrue}, + ) + + _, _, err := validateAndClassifyAdapterStatus(1, status, nil, testLog()) + + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("mandatory condition")) +} + +func TestValidateAndClassify_InvalidAvailableStatus_ReturnsError(t *testing.T) { + RegisterTestingT(t) + + status := adapterStatusWithGenAndTime(1, time.Now()) + status.Conditions = testConditionsJSON( + api.AdapterCondition{Type: api.AdapterConditionTypeAvailable, Status: "Invalid"}, + api.AdapterCondition{Type: api.AdapterConditionTypeApplied, Status: api.AdapterConditionTrue}, + api.AdapterCondition{Type: api.AdapterConditionTypeHealth, Status: api.AdapterConditionTrue}, + ) + + _, _, err := validateAndClassifyAdapterStatus(1, status, nil, testLog()) + + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("invalid status")) +} + +func TestValidateAndClassify_FirstUnknownAvailable_Accepted(t *testing.T) { + RegisterTestingT(t) + + status := adapterStatusWithGenAndTime(1, time.Now()) + status.Conditions = testConditionsJSON(testMandatoryConditions(api.AdapterConditionUnknown)...) + + conditions, trigger, err := validateAndClassifyAdapterStatus(1, status, nil, testLog()) + + Expect(err).To(BeNil()) + Expect(conditions).ToNot(BeNil()) + Expect(trigger).To(BeFalse()) +} + +func TestValidateAndClassify_SubsequentUnknownAvailable_Discards(t *testing.T) { + RegisterTestingT(t) + + existing := adapterStatusWithGenAndTime(1, time.Now().Add(-time.Second)) + status := adapterStatusWithGenAndTime(1, time.Now()) + status.Conditions = testConditionsJSON(testMandatoryConditions(api.AdapterConditionUnknown)...) + + conditions, trigger, err := validateAndClassifyAdapterStatus(1, status, existing, testLog()) + + Expect(err).To(BeNil()) + Expect(conditions).To(BeNil()) + Expect(trigger).To(BeFalse()) +} + +func TestValidateAndClassify_AvailableTrue_TriggersAggregation(t *testing.T) { + RegisterTestingT(t) + + status := adapterStatusWithGenAndTime(1, time.Now()) + + conditions, trigger, err := validateAndClassifyAdapterStatus(1, status, nil, testLog()) + + Expect(err).To(BeNil()) + Expect(conditions).ToNot(BeNil()) + Expect(trigger).To(BeTrue()) +} + +func TestValidateAndClassify_AvailableFalse_TriggersAggregation(t *testing.T) { + RegisterTestingT(t) + + status := adapterStatusWithGenAndTime(1, time.Now()) + status.Conditions = testConditionsJSON(testMandatoryConditions(api.AdapterConditionFalse)...) + + conditions, trigger, err := validateAndClassifyAdapterStatus(1, status, nil, testLog()) + + Expect(err).To(BeNil()) + Expect(conditions).ToNot(BeNil()) + Expect(trigger).To(BeTrue()) +} diff --git a/pkg/services/cluster.go b/pkg/services/cluster.go index bd95138c..d130e8db 100644 --- a/pkg/services/cluster.go +++ b/pkg/services/cluster.go @@ -337,9 +337,8 @@ func (s *sqlClusterService) ProcessAdapterStatus( return upsertedStatus, nil } -// validateAndClassify performs all stateless validation and discard-rule checks on an incoming -// adapter status. Returns the parsed conditions and whether aggregation should be triggered. -// Returns (nil, false, nil) when the update should be silently discarded. +// validateAndClassify delegates to the shared validateAndClassifyAdapterStatus +// with cluster-specific logger context. func (s *sqlClusterService) validateAndClassify( ctx context.Context, clusterID string, @@ -348,75 +347,7 @@ func (s *sqlClusterService) validateAndClassify( existingStatus *api.AdapterStatus, ) ([]api.AdapterCondition, bool, *errors.ServiceError) { log := logger.With(logger.WithClusterID(ctx, clusterID), logger.FieldAdapter, adapterStatus.Adapter) - - if adapterStatus.ObservedGeneration > cluster.Generation { - log.Debug("Discarding adapter status update: future generation") - return nil, false, nil - } - - if existingStatus != nil && adapterStatus.ObservedGeneration < existingStatus.ObservedGeneration { - log.Debug("Discarding adapter status update: stale generation") - return nil, false, nil - } - - incomingObs := AdapterObservedTime(adapterStatus) - if incomingObs.IsZero() { - log.Debug("Discarding adapter status update: zero observed time") - return nil, false, nil - } - - if existingStatus != nil && adapterStatus.ObservedGeneration == existingStatus.ObservedGeneration { - prevObs := AdapterObservedTime(existingStatus) - if !prevObs.IsZero() && incomingObs.Before(prevObs) { - log.Debug("Discarding adapter status update: stale observed time") - return nil, false, nil - } - } - - var conditions []api.AdapterCondition - if len(adapterStatus.Conditions) > 0 { - if errUnmarshal := json.Unmarshal(adapterStatus.Conditions, &conditions); errUnmarshal != nil { - return nil, false, errors.GeneralError("Failed to unmarshal adapter status conditions: %s", errUnmarshal) - } - } - - if errorType, conditionName := ValidateMandatoryConditions(conditions); errorType != "" { - return nil, false, errors.Validation( - "missing mandatory condition '%s': all adapters must report Available, Applied, and Health", - conditionName, - ) - } - - triggerAggregation := false - for _, cond := range conditions { - if cond.Type != api.AdapterConditionTypeAvailable { - continue - } - - isValidStatus := cond.Status == api.AdapterConditionTrue || - cond.Status == api.AdapterConditionFalse || - cond.Status == api.AdapterConditionUnknown - if !isValidStatus { - return nil, false, errors.Validation( - "condition '%s' has invalid status '%s': must be True, False, or Unknown", - cond.Type, cond.Status, - ) - } - - if cond.Status != api.AdapterConditionTrue && cond.Status != api.AdapterConditionFalse { - if existingStatus != nil { - log.Debug("Discarding adapter status update: subsequent Unknown Available") - return nil, false, nil - } - triggerAggregation = false - break - } - - triggerAggregation = true - break - } - - return conditions, triggerAggregation, nil + return validateAndClassifyAdapterStatus(cluster.Generation, adapterStatus, existingStatus, log) } // tryHardDeleteCluster checks whether all required adapters have reported Finalized=True for diff --git a/pkg/services/node_pool.go b/pkg/services/node_pool.go index 0c1efcc4..093ac67e 100644 --- a/pkg/services/node_pool.go +++ b/pkg/services/node_pool.go @@ -417,6 +417,8 @@ func (s *sqlNodePoolService) ProcessAdapterStatus( // validateAndClassifyNodePool performs all stateless validation and discard-rule checks on an // incoming adapter status for a node pool. Returns the parsed conditions and whether aggregation // should be triggered. Returns (nil, false, nil) when the update should be silently discarded. +// validateAndClassifyNodePool delegates to the shared validateAndClassifyAdapterStatus +// with nodepool-specific logger context. func (s *sqlNodePoolService) validateAndClassifyNodePool( ctx context.Context, nodePoolID string, @@ -424,76 +426,8 @@ func (s *sqlNodePoolService) validateAndClassifyNodePool( nodePool *api.NodePool, existingStatus *api.AdapterStatus, ) ([]api.AdapterCondition, bool, *errors.ServiceError) { - l := logger.With(ctx, logger.FieldNodePoolID, nodePoolID, logger.FieldAdapter, adapterStatus.Adapter) - - if adapterStatus.ObservedGeneration > nodePool.Generation { - l.Debug("Discarding adapter status update: future generation") - return nil, false, nil - } - - if existingStatus != nil && adapterStatus.ObservedGeneration < existingStatus.ObservedGeneration { - l.Debug("Discarding adapter status update: stale generation") - return nil, false, nil - } - - incomingObs := AdapterObservedTime(adapterStatus) - if incomingObs.IsZero() { - l.Debug("Discarding adapter status update: zero observed time") - return nil, false, nil - } - - if existingStatus != nil && adapterStatus.ObservedGeneration == existingStatus.ObservedGeneration { - prevObs := AdapterObservedTime(existingStatus) - if !prevObs.IsZero() && incomingObs.Before(prevObs) { - l.Debug("Discarding adapter status update: stale observed time") - return nil, false, nil - } - } - - var conditions []api.AdapterCondition - if len(adapterStatus.Conditions) > 0 { - if errUnmarshal := json.Unmarshal(adapterStatus.Conditions, &conditions); errUnmarshal != nil { - return nil, false, errors.GeneralError("Failed to unmarshal adapter status conditions: %s", errUnmarshal) - } - } - - if errorType, conditionName := ValidateMandatoryConditions(conditions); errorType != "" { - return nil, false, errors.Validation( - "missing mandatory condition '%s': all adapters must report Available, Applied, and Health", - conditionName, - ) - } - - triggerAggregation := false - for _, cond := range conditions { - if cond.Type != api.AdapterConditionTypeAvailable { - continue - } - - isValidStatus := cond.Status == api.AdapterConditionTrue || - cond.Status == api.AdapterConditionFalse || - cond.Status == api.AdapterConditionUnknown - if !isValidStatus { - return nil, false, errors.Validation( - "condition '%s' has invalid status '%s': must be True, False, or Unknown", - cond.Type, cond.Status, - ) - } - - if cond.Status != api.AdapterConditionTrue && cond.Status != api.AdapterConditionFalse { - if existingStatus != nil { - l.Debug("Discarding adapter status update: subsequent Unknown Available") - return nil, false, nil - } - triggerAggregation = false - break - } - - triggerAggregation = true - break - } - - return conditions, triggerAggregation, nil + log := logger.With(ctx, logger.FieldNodePoolID, nodePoolID, logger.FieldAdapter, adapterStatus.Adapter) + return validateAndClassifyAdapterStatus(nodePool.Generation, adapterStatus, existingStatus, log) } // tryHardDeleteNodePool checks whether all required adapters have reported Finalized=True at the current diff --git a/pkg/services/resource.go b/pkg/services/resource.go index ee7d8e87..7508732b 100644 --- a/pkg/services/resource.go +++ b/pkg/services/resource.go @@ -11,6 +11,7 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/db" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/metrics" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/util" ) @@ -24,21 +25,34 @@ type ResourceService interface { Delete(ctx context.Context, kind, id string) (*api.Resource, *errors.ServiceError) List(ctx context.Context, kind string, args *ListArguments) (api.ResourceList, *api.PagingMeta, *errors.ServiceError) GetByOwner(ctx context.Context, kind, id, ownerID string) (*api.Resource, *errors.ServiceError) - ListByOwner(ctx context.Context, kind, ownerID string, args *ListArguments) (api.ResourceList, *api.PagingMeta, *errors.ServiceError) // nolint:lll + ListByOwner(ctx context.Context, kind, ownerID string, args *ListArguments) (api.ResourceList, *api.PagingMeta, *errors.ServiceError) // nolint:lll ForceDelete(ctx context.Context, kind, id, reason string) *errors.ServiceError GetByID(ctx context.Context, id string) (*api.Resource, *errors.ServiceError) ListAll(ctx context.Context, args *ListArguments) (api.ResourceList, *api.PagingMeta, *errors.ServiceError) + ProcessAdapterStatus(ctx context.Context, kind, resourceID string, adapterStatus *api.AdapterStatus) (*api.AdapterStatus, *errors.ServiceError) // nolint:lll } -func NewResourceService(resourceDao dao.ResourceDao, generic GenericService) ResourceService { - return &sqlResourceService{resourceDao: resourceDao, generic: generic} +func NewResourceService( + resourceDao dao.ResourceDao, + adapterStatusDao dao.AdapterStatusDao, + resourceConditionDao dao.ResourceConditionDao, + generic GenericService, +) ResourceService { + return &sqlResourceService{ + resourceDao: resourceDao, + adapterStatusDao: adapterStatusDao, + resourceConditionDao: resourceConditionDao, + generic: generic, + } } var _ ResourceService = &sqlResourceService{} type sqlResourceService struct { - resourceDao dao.ResourceDao - generic GenericService + resourceDao dao.ResourceDao + adapterStatusDao dao.AdapterStatusDao + resourceConditionDao dao.ResourceConditionDao + generic GenericService } // Get returns a single resource by kind and ID. Returns 404 if not found. @@ -366,6 +380,262 @@ func (s *sqlResourceService) ListAll( return result, paging, nil } +// ProcessAdapterStatus validates, upserts an adapter status report, and triggers +// status aggregation for a generic resource. Follows the same 4-DB-call pattern +// as ClusterService.ProcessAdapterStatus: +// 1. GetForUpdate — lock + fetch resource with conditions +// 2. FindByResource — all adapter statuses (existing found in-memory) +// 3. Upsert — write adapter status +// 4. UpdateConditions — write aggregated conditions (if changed) +func (s *sqlResourceService) ProcessAdapterStatus( + ctx context.Context, kind, resourceID string, adapterStatus *api.AdapterStatus, +) (*api.AdapterStatus, *errors.ServiceError) { + if svcErr := validateKind(kind); svcErr != nil { + return nil, svcErr + } + + // Step 1: Acquire a row-level exclusive lock on the resource. Concurrent + // adapter status updates for the same resource are serialized here. + // GetForUpdate also preloads Conditions (needed for aggregation diff). + resource, err := s.resourceDao.GetForUpdate(ctx, kind, resourceID) + if err != nil { + return nil, handleGetError(kind, "id", resourceID, err) + } + + // Step 2: Fetch all existing adapter statuses for this resource. + // The existing status for the incoming adapter is found in-memory from + // this list — no additional DB call needed. + allStatuses, err := s.adapterStatusDao.FindByResource(ctx, kind, resourceID) + if err != nil { + return nil, errors.GeneralError("Failed to get adapter statuses: %s", err) + } + + existingStatus := findAdapterStatusInList(allStatuses, adapterStatus.Adapter) + + // Validate the incoming report: discard stale/future generations, zero + // observed times, subsequent Unknown Available, and missing mandatory + // conditions. Returns (nil, false, nil) when the update should be + // silently discarded (handler returns 204 No Content). + log := logger.With(ctx, "resource_type", kind, "resource_id", resourceID, + logger.FieldAdapter, adapterStatus.Adapter) + conditions, triggerAggregation, svcErr := validateAndClassifyAdapterStatus( + resource.Generation, adapterStatus, existingStatus, log, + ) + if svcErr != nil { + return nil, svcErr + } + if conditions == nil && !triggerAggregation { + return nil, nil + } + + // Step 3: Persist the adapter status. setConditionTransitionTimes preserves + // LastTransitionTime from the existing status when the condition status + // hasn't changed (Kubernetes-style semantics). + adapterStatus.ResourceType = kind + adapterStatus.ResourceID = resourceID + setConditionTransitionTimes(adapterStatus, existingStatus) + + upsertedStatus, err := s.adapterStatusDao.Upsert(ctx, adapterStatus, existingStatus) + if err != nil { + return nil, handleCreateError("AdapterStatus", err) + } + + // Build the post-upsert snapshot of all statuses. Using the pre-upsert + // list for hard-delete or aggregation would miss the just-written status. + updatedStatuses := replaceAdapterStatusInList(allStatuses, upsertedStatus) + + // If the resource is soft-deleted, check whether all adapters have now + // reported Finalized=True — if so, hard-delete the resource. + if resource.DeletedTime != nil { + hardDeleted, hdErr := s.tryHardDeleteResource(ctx, resource, conditions, updatedStatuses) + if hdErr != nil { + return nil, hdErr + } + if hardDeleted { + return upsertedStatus, nil + } + } + + // Step 4: Re-aggregate conditions from all adapter statuses and persist + // to the resource_conditions table. Only runs when the Available condition + // changed to True or False (not on Unknown or discarded updates). + if triggerAggregation { + if aggregateErr := s.recomputeAndSaveResourceConditions( + ctx, resource, updatedStatuses, + ); aggregateErr != nil { + return nil, aggregateErr + } + } + + return upsertedStatus, nil +} + +// recomputeAndSaveResourceConditions runs AggregateResourceStatus and persists +// the result to the resource_conditions table. Skips the write when conditions +// are unchanged. +func (s *sqlResourceService) recomputeAndSaveResourceConditions( + ctx context.Context, + resource *api.Resource, + adapterStatuses api.AdapterStatusList, +) *errors.ServiceError { + desc := registry.MustGet(resource.Kind) + + // Convert the GORM association ([]ResourceCondition) to JSON so it can be + // passed to AggregateResourceStatus via PrevConditionsJSON. This is needed + // because the aggregation function uses the previous conditions to preserve + // LastTransitionTime and the sticky LastKnownReconciled condition. + var prevConditionsJSON []byte + if len(resource.Conditions) > 0 { + var marshalErr error + prevConditionsJSON, marshalErr = json.Marshal(resource.Conditions) + if marshalErr != nil { + return errors.GeneralError("Failed to marshal previous conditions: %s", marshalErr) + } + } + + // Extract previous Reconciled status for metric emission below. + prevReconciledStatus := extractPrevReconciledStatus(ctx, prevConditionsJSON) + + // During deletion, check if child resources still exist. The aggregation + // function uses this to prevent premature Reconciled=True on a parent + // whose children haven't finished their own reconciliation. + hasChildResources := false + if resource.DeletedTime != nil { + for _, child := range registry.ChildrenOf(resource.Kind) { + exists, err := s.resourceDao.ExistsByOwner(ctx, child.Kind, resource.ID) + if err != nil { + return errors.GeneralError( + "Failed to check child %s for status aggregation: %s", child.Kind, err, + ) + } + if exists { + hasChildResources = true + break + } + } + } + + // Use UpdatedTime as the reference time for aggregation. Falls back to + // CreatedTime for resources that haven't been patched yet. + refTime := resource.UpdatedTime + if refTime.IsZero() { + refTime = resource.CreatedTime + } + + reconciled, lastKnownReconciled, adapterConditions := AggregateResourceStatus( + ctx, AggregateResourceStatusInput{ + ResourceGeneration: resource.Generation, + RefTime: refTime, + DeletedTime: resource.DeletedTime, + PrevConditionsJSON: prevConditionsJSON, + RequiredAdapters: desc.RequiredAdapters, + AdapterStatuses: adapterStatuses, + HasChildResources: hasChildResources, + }, + ) + + // Build the full conditions slice: Reconciled + LastKnownReconciled + per-adapter conditions. + newConditions := make([]api.ResourceCondition, 0, fixedConditionCount+len(adapterConditions)) + newConditions = append(newConditions, reconciled, lastKnownReconciled) + newConditions = append(newConditions, adapterConditions...) + + // Compare via JSON to detect actual changes (same approach as cluster.go). + newJSON, marshalErr := json.Marshal(newConditions) + if marshalErr != nil { + return errors.GeneralError("Failed to marshal conditions: %s", marshalErr) + } + if jsonEqual(prevConditionsJSON, newJSON) { + return nil + } + + // Write to resource_conditions table (not JSONB on the resource row). + // MarkForRollback is handled by the DAO internally. + if err := s.resourceConditionDao.UpdateConditions(ctx, resource.ID, newConditions); err != nil { + return errors.GeneralError("Failed to update resource conditions: %s", err) + } + + // Emit metric on Reconciled=False transition (reconciliation started). + if reconciled.Status == api.ConditionFalse && + (prevReconciledStatus == nil || *prevReconciledStatus != api.ConditionFalse) { + metrics.RecordReconciliationStarted(resource.Kind, resource.DeletedTime != nil) + } + + return nil +} + +// tryHardDeleteResource checks whether all required adapters have reported +// Finalized=True for a soft-deleted resource and no children remain, then +// permanently removes the resource and its adapter statuses/conditions. +func (s *sqlResourceService) tryHardDeleteResource( + ctx context.Context, + resource *api.Resource, + conditions []api.AdapterCondition, + allStatuses api.AdapterStatusList, +) (bool, *errors.ServiceError) { + // Quick check: does the incoming report contain Finalized=True? + // If not, hard-delete is not possible regardless of other adapters. + if !incomingReportedFinalized(conditions) { + return false, nil + } + + // Check that ALL required adapters (not just this one) have reported + // Finalized=True at the current generation. + desc := registry.MustGet(resource.Kind) + if !allAdaptersFinalized(desc.RequiredAdapters, allStatuses, resource.Generation) { + return false, nil + } + + // Ensure no active children exist — hard-deleting a parent with active + // children would leave orphaned resources. + children := registry.ChildrenOf(resource.Kind) + for _, child := range children { + exists, err := s.resourceDao.ExistsByOwner(ctx, child.Kind, resource.ID) + if err != nil { + return false, errors.GeneralError( + "Failed to check %s children during hard-delete: %s", child.Kind, err, + ) + } + if exists { + return false, nil + } + } + + // Also check for soft-deleted children still pending adapter finalization. + if len(children) > 0 { + childKinds := make([]string, len(children)) + for i, c := range children { + childKinds[i] = c.Kind + } + hasSoftDeleted, err := s.resourceDao.ExistsSoftDeletedByOwner(ctx, childKinds, resource.ID) + if err != nil { + return false, errors.GeneralError( + "Failed to check soft-deleted children during hard-delete: %s", err, + ) + } + if hasSoftDeleted { + return false, nil + } + } + + // All checks passed — clean up associated data and hard-delete the resource. + // Order matters: adapter statuses and conditions must be removed before the + // resource row, since they reference it. + if err := s.adapterStatusDao.DeleteByResource(ctx, resource.Kind, resource.ID); err != nil { + return false, errors.GeneralError("Failed to delete adapter statuses during hard-delete: %s", err) + } + if err := s.resourceConditionDao.DeleteByResource(ctx, resource.ID); err != nil { + return false, errors.GeneralError("Failed to delete resource conditions during hard-delete: %s", err) + } + if err := s.resourceDao.Delete(ctx, resource.Kind, resource.ID); err != nil { + return false, errors.GeneralError("Failed to hard-delete %s: %s", resource.Kind, err) + } + + logger.With(ctx, "resource_type", resource.Kind, "resource_id", resource.ID). + Info("Hard-deleted resource after all required adapters reported Finalized=True") + + return true, nil +} + // validateKind checks that the kind is a registered entity type. // Returns 400 if the kind is unknown, preventing invalid kinds from reaching the DAO. func validateKind(kind string) *errors.ServiceError { diff --git a/pkg/services/resource_test.go b/pkg/services/resource_test.go index e50f67fd..d67e65ae 100644 --- a/pkg/services/resource_test.go +++ b/pkg/services/resource_test.go @@ -165,20 +165,6 @@ func (d *mockResourceDao) GetByID(_ context.Context, id string) (*api.Resource, return nil, gorm.ErrRecordNotFound } -func (d *mockResourceDao) FindByIDs(_ context.Context, kind string, ids []string) (api.ResourceList, error) { - idSet := make(map[string]bool, len(ids)) - for _, id := range ids { - idSet[id] = true - } - var result api.ResourceList - for _, r := range d.resources { - if r.Kind == kind && idSet[r.ID] { - result = append(result, r) - } - } - return result, nil -} - func (d *mockResourceDao) addResource(r *api.Resource) { d.resources[resourceKey(r.Kind, r.ID)] = r } @@ -204,12 +190,48 @@ func (g *resourceGenericMock) List( var _ GenericService = &resourceGenericMock{} +// resourceConditionMock implements dao.ResourceConditionDao for testing. +type resourceConditionMock struct { + conditions map[string][]api.ResourceCondition +} + +func newResourceConditionMock() *resourceConditionMock { + return &resourceConditionMock{conditions: make(map[string][]api.ResourceCondition)} +} + +func (d *resourceConditionMock) UpdateConditions( + _ context.Context, resourceID string, conditions []api.ResourceCondition, +) error { + d.conditions[resourceID] = conditions + return nil +} + +func (d *resourceConditionMock) DeleteByResource(_ context.Context, resourceID string) error { + delete(d.conditions, resourceID) + return nil +} + +var _ dao.ResourceConditionDao = &resourceConditionMock{} + +// newTestResourceService creates a ResourceService for CRUD-only tests. +// adapterStatusDao and resourceConditionDao are nil — tests that call +// ProcessAdapterStatus must use newTestResourceServiceWithAdapterStatus. func newTestResourceService(mockDao *mockResourceDao) (ResourceService, *mockResourceDao, *resourceGenericMock) { generic := &resourceGenericMock{} - svc := NewResourceService(mockDao, generic) + svc := NewResourceService(mockDao, nil, nil, generic) return svc, mockDao, generic } +func newTestResourceServiceWithAdapterStatus( + mockDao *mockResourceDao, +) (ResourceService, *mockResourceDao, *mockAdapterStatusDao, *resourceConditionMock) { + asDao := newMockAdapterStatusDao() + rcDao := newResourceConditionMock() + generic := &resourceGenericMock{} + svc := NewResourceService(mockDao, asDao, rcDao, generic) + return svc, mockDao, asDao, rcDao +} + func testResource(kind, id, name string) *api.Resource { spec, _ := json.Marshal(map[string]interface{}{"key": "value"}) r := &api.Resource{ @@ -1572,14 +1594,278 @@ func TestResourceService_ForceDelete_InvalidKind(t *testing.T) { Expect(svcErr.HTTPCode).To(Equal(400)) } -func TestAllGenericDescriptors_HaveNoRequiredAdapters(t *testing.T) { +// ─── ProcessAdapterStatus tests ────────────────────────────── + +func setupAdapterStatusDescriptors() { + registry.Reset() + registry.Register(registry.EntityDescriptor{ + Kind: "TestResource", + Plural: "testresources", + RequiredAdapters: []string{"adapter1"}, + }) +} + +func testAdapterStatusRequest(gen int32) *api.AdapterStatus { + return &api.AdapterStatus{ + Adapter: "adapter1", + ObservedGeneration: gen, + LastReportTime: time.Now().UTC(), + Conditions: testConditionsJSON(testMandatoryConditions(api.AdapterConditionTrue)...), + } +} + +func TestProcessAdapterStatus_HappyPath(t *testing.T) { RegisterTestingT(t) - setupTestDescriptors() + setupAdapterStatusDescriptors() + + mockDao := newMockResourceDao() + svc, _, asDao, rcDao := newTestResourceServiceWithAdapterStatus(mockDao) + + r := testResource("TestResource", "r-1", "test") + r.Generation = 1 + mockDao.addResource(r) + + result, svcErr := svc.ProcessAdapterStatus( + context.Background(), "TestResource", "r-1", testAdapterStatusRequest(1), + ) - for _, d := range registry.All() { - Expect(d.RequiredAdapters).To(BeEmpty(), - "Descriptor %q has RequiredAdapters=%v. "+ - "ForceDelete does not yet handle adapter_status cleanup. "+ - "See HYPERFLEET-1154.", d.Kind, d.RequiredAdapters) + Expect(svcErr).To(BeNil()) + Expect(result).ToNot(BeNil()) + Expect(result.ResourceType).To(Equal("TestResource")) + Expect(result.ResourceID).To(Equal("r-1")) + + // Adapter status should be stored + Expect(asDao.statuses).To(HaveLen(1)) + + // Conditions should be written (aggregation triggered by Available=True) + Expect(rcDao.conditions).To(HaveKey("r-1")) + Expect(rcDao.conditions["r-1"]).ToNot(BeEmpty()) +} + +func TestProcessAdapterStatus_UnknownKind_Returns400(t *testing.T) { + RegisterTestingT(t) + setupAdapterStatusDescriptors() + + mockDao := newMockResourceDao() + svc, _, _, _ := newTestResourceServiceWithAdapterStatus(mockDao) + + _, svcErr := svc.ProcessAdapterStatus( + context.Background(), "NonExistent", "r-1", testAdapterStatusRequest(1), + ) + + Expect(svcErr).ToNot(BeNil()) + Expect(svcErr.HTTPCode).To(Equal(400)) +} + +func TestProcessAdapterStatus_ResourceNotFound_Returns404(t *testing.T) { + RegisterTestingT(t) + setupAdapterStatusDescriptors() + + mockDao := newMockResourceDao() + svc, _, _, _ := newTestResourceServiceWithAdapterStatus(mockDao) + + _, svcErr := svc.ProcessAdapterStatus( + context.Background(), "TestResource", "nonexistent", testAdapterStatusRequest(1), + ) + + Expect(svcErr).ToNot(BeNil()) + Expect(svcErr.HTTPCode).To(Equal(404)) +} + +func TestProcessAdapterStatus_DiscardedStatus_ReturnsNil(t *testing.T) { + RegisterTestingT(t) + setupAdapterStatusDescriptors() + + mockDao := newMockResourceDao() + svc, _, _, rcDao := newTestResourceServiceWithAdapterStatus(mockDao) + + r := testResource("TestResource", "r-1", "test") + r.Generation = 1 + mockDao.addResource(r) + + // Future generation → discarded + result, svcErr := svc.ProcessAdapterStatus( + context.Background(), "TestResource", "r-1", testAdapterStatusRequest(99), + ) + + Expect(svcErr).To(BeNil()) + Expect(result).To(BeNil()) + Expect(rcDao.conditions).ToNot(HaveKey("r-1")) +} + +func TestProcessAdapterStatus_UpsertSecondReport(t *testing.T) { + RegisterTestingT(t) + setupAdapterStatusDescriptors() + + mockDao := newMockResourceDao() + svc, _, asDao, _ := newTestResourceServiceWithAdapterStatus(mockDao) + + r := testResource("TestResource", "r-1", "test") + r.Generation = 1 + mockDao.addResource(r) + + // First report + _, svcErr := svc.ProcessAdapterStatus( + context.Background(), "TestResource", "r-1", testAdapterStatusRequest(1), + ) + Expect(svcErr).To(BeNil()) + Expect(asDao.statuses).To(HaveLen(1)) + + // Second report from same adapter + req2 := testAdapterStatusRequest(1) + req2.LastReportTime = time.Now().UTC().Add(time.Second) + _, svcErr = svc.ProcessAdapterStatus( + context.Background(), "TestResource", "r-1", req2, + ) + Expect(svcErr).To(BeNil()) + Expect(asDao.statuses).To(HaveLen(1)) +} + +func TestProcessAdapterStatus_SoftDeleted_AllFinalized_HardDeletes(t *testing.T) { + RegisterTestingT(t) + setupAdapterStatusDescriptors() + + mockDao := newMockResourceDao() + svc, _, asDao, rcDao := newTestResourceServiceWithAdapterStatus(mockDao) + + deletedAt := time.Now().UTC() + r := testResource("TestResource", "r-1", "test") + r.Generation = 1 + r.DeletedTime = &deletedAt + mockDao.addResource(r) + + // Report with Finalized=True + req := &api.AdapterStatus{ + Adapter: "adapter1", + ObservedGeneration: 1, + LastReportTime: time.Now().UTC(), + Conditions: testConditionsJSON( + api.AdapterCondition{Type: api.AdapterConditionTypeAvailable, Status: api.AdapterConditionTrue}, + api.AdapterCondition{Type: api.AdapterConditionTypeApplied, Status: api.AdapterConditionTrue}, + api.AdapterCondition{Type: api.AdapterConditionTypeHealth, Status: api.AdapterConditionTrue}, + api.AdapterCondition{Type: api.AdapterConditionTypeFinalized, Status: api.AdapterConditionTrue}, + ), + } + + result, svcErr := svc.ProcessAdapterStatus(context.Background(), "TestResource", "r-1", req) + + Expect(svcErr).To(BeNil()) + Expect(result).ToNot(BeNil()) + + // Resource should be hard-deleted + _, exists := mockDao.resources[resourceKey("TestResource", "r-1")] + Expect(exists).To(BeFalse(), "Resource should be hard-deleted") + + // Adapter statuses and conditions should be cleaned up + Expect(asDao.statuses).To(BeEmpty()) + Expect(rcDao.conditions).ToNot(HaveKey("r-1")) +} + +func TestProcessAdapterStatus_SoftDeleted_NotAllFinalized_NoHardDelete(t *testing.T) { + RegisterTestingT(t) + setupAdapterStatusDescriptors() + + mockDao := newMockResourceDao() + svc, _, _, _ := newTestResourceServiceWithAdapterStatus(mockDao) + + deletedAt := time.Now().UTC() + r := testResource("TestResource", "r-1", "test") + r.Generation = 1 + r.DeletedTime = &deletedAt + mockDao.addResource(r) + + // Report with Available=True but NO Finalized + result, svcErr := svc.ProcessAdapterStatus( + context.Background(), "TestResource", "r-1", testAdapterStatusRequest(1), + ) + + Expect(svcErr).To(BeNil()) + Expect(result).ToNot(BeNil()) + + // Resource should NOT be hard-deleted + _, exists := mockDao.resources[resourceKey("TestResource", "r-1")] + Expect(exists).To(BeTrue(), "Resource should still exist") +} + +func TestProcessAdapterStatus_EmptyRequiredAdapters_StillAggregates(t *testing.T) { + RegisterTestingT(t) + registry.Reset() + registry.Register(registry.EntityDescriptor{ + Kind: "NoAdapter", + Plural: "noadapters", + }) + + mockDao := newMockResourceDao() + svc, _, _, rcDao := newTestResourceServiceWithAdapterStatus(mockDao) + + r := testResource("NoAdapter", "r-1", "test") + r.Generation = 1 + mockDao.addResource(r) + + req := &api.AdapterStatus{ + Adapter: "voluntary-adapter", + ObservedGeneration: 1, + LastReportTime: time.Now().UTC(), + Conditions: testConditionsJSON(testMandatoryConditions(api.AdapterConditionTrue)...), } + + result, svcErr := svc.ProcessAdapterStatus(context.Background(), "NoAdapter", "r-1", req) + + Expect(svcErr).To(BeNil()) + Expect(result).ToNot(BeNil()) + + // Conditions should still be aggregated + Expect(rcDao.conditions).To(HaveKey("r-1")) +} + +func TestProcessAdapterStatus_SoftDeleted_ChildrenExist_NoHardDelete(t *testing.T) { + RegisterTestingT(t) + registry.Reset() + registry.Register(registry.EntityDescriptor{ + Kind: "Parent", + Plural: "parents", + RequiredAdapters: []string{"adapter1"}, + }) + registry.Register(registry.EntityDescriptor{ + Kind: "Child", + Plural: "children", + ParentKind: "Parent", + OnParentDelete: registry.OnParentDeleteCascade, + RequiredAdapters: []string{"child-adapter"}, + }) + + mockDao := newMockResourceDao() + svc, _, _, _ := newTestResourceServiceWithAdapterStatus(mockDao) + + deletedAt := time.Now().UTC() + parent := testResource("Parent", "p-1", "parent") + parent.Generation = 1 + parent.DeletedTime = &deletedAt + mockDao.addResource(parent) + + // Active child prevents hard-delete + childOwner := "p-1" + child := testResource("Child", "c-1", "child") + child.OwnerID = &childOwner + mockDao.addResource(child) + + req := &api.AdapterStatus{ + Adapter: "adapter1", + ObservedGeneration: 1, + LastReportTime: time.Now().UTC(), + Conditions: testConditionsJSON( + api.AdapterCondition{Type: api.AdapterConditionTypeAvailable, Status: api.AdapterConditionTrue}, + api.AdapterCondition{Type: api.AdapterConditionTypeApplied, Status: api.AdapterConditionTrue}, + api.AdapterCondition{Type: api.AdapterConditionTypeHealth, Status: api.AdapterConditionTrue}, + api.AdapterCondition{Type: api.AdapterConditionTypeFinalized, Status: api.AdapterConditionTrue}, + ), + } + + result, svcErr := svc.ProcessAdapterStatus(context.Background(), "Parent", "p-1", req) + Expect(svcErr).To(BeNil()) + Expect(result).ToNot(BeNil()) + + // Parent should NOT be hard-deleted because child exists + _, exists := mockDao.resources[resourceKey("Parent", "p-1")] + Expect(exists).To(BeTrue(), "Parent should still exist — active child blocks hard-delete") } diff --git a/plugins/entities/plugin.go b/plugins/entities/plugin.go index 9e832baf..101014c7 100644 --- a/plugins/entities/plugin.go +++ b/plugins/entities/plugin.go @@ -13,6 +13,7 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/validators" + adapterStatus "github.com/openshift-hyperfleet/hyperfleet-api/plugins/adapterStatus" "github.com/openshift-hyperfleet/hyperfleet-api/plugins/resources" ) @@ -20,6 +21,7 @@ func init() { server.RegisterRoutes("entities", func(apiV1Router *mux.Router, svc server.ServicesInterface) { envServices := svc.(*environments.Services) resourceService := resources.Service(envServices) + adapterStatusService := adapterStatus.Service(envServices) schemaPath := environments.Environment().Config.Server.OpenAPISchemaPath var schemaValidator *validators.SchemaValidator @@ -31,7 +33,7 @@ func init() { } } - RegisterEntityRoutes(apiV1Router, resourceService, schemaValidator) + RegisterEntityRoutes(apiV1Router, resourceService, adapterStatusService, schemaValidator) }) } @@ -42,18 +44,24 @@ func init() { // Top-level entities get routes at /{plural}. Child entities (ParentKind != "") // get nested routes under /{parent_plural}/{parent_id}/{plural} plus flat // read/update/delete access at /{plural} (POST rejected — needs parent context). +// All entities get /{id}/statuses sub-routes for adapter status reporting. // // The kind-agnostic /resources root endpoint is registered separately. func RegisterEntityRoutes( apiV1Router *mux.Router, resourceService services.ResourceService, + adapterStatusService services.AdapterStatusService, schemaValidator *validators.SchemaValidator, ) { - registerPerEntityRoutes(apiV1Router, resourceService) - registerRootResourceRoutes(apiV1Router, resourceService, schemaValidator) + registerPerEntityRoutes(apiV1Router, resourceService, adapterStatusService) + registerRootResourceRoutes(apiV1Router, resourceService, adapterStatusService, schemaValidator) } -func registerPerEntityRoutes(apiV1Router *mux.Router, resourceService services.ResourceService) { +func registerPerEntityRoutes( + apiV1Router *mux.Router, + resourceService services.ResourceService, + adapterStatusService services.AdapterStatusService, +) { descriptors := registry.All() sort.Slice(descriptors, func(i, j int) bool { return descriptors[i].Kind < descriptors[j].Kind @@ -67,18 +75,20 @@ func registerPerEntityRoutes(apiV1Router *mux.Router, resourceService services.R )) } h := handlers.NewResourceHandler(descriptor, resourceService) + sh := handlers.NewResourceStatusHandler(descriptor, resourceService, adapterStatusService) if descriptor.ParentKind != "" { parent := registry.MustGet(descriptor.ParentKind) - registerResourceRoutes(apiV1Router, "/"+parent.Plural+"/{parent_id}/"+descriptor.Plural, h) + registerResourceRoutes(apiV1Router, "/"+parent.Plural+"/{parent_id}/"+descriptor.Plural, h, sh) } - registerResourceRoutes(apiV1Router, "/"+descriptor.Plural, h) + registerResourceRoutes(apiV1Router, "/"+descriptor.Plural, h, sh) } } func registerRootResourceRoutes( apiV1Router *mux.Router, resourceService services.ResourceService, + adapterStatusService services.AdapterStatusService, schemaValidator *validators.SchemaValidator, ) { rootHandler := handlers.NewRootResourceHandler(resourceService, schemaValidator) @@ -89,10 +99,18 @@ func registerRootResourceRoutes( r.HandleFunc("/{id}", rootHandler.Patch).Methods(http.MethodPatch) r.HandleFunc("/{id}", rootHandler.Delete).Methods(http.MethodDelete) r.HandleFunc("/{id}/force-delete", rootHandler.ForceDelete).Methods(http.MethodPost) - // TODO: HYPERFLEET-1154 — wire /{id}/statuses GET and PUT once ResourceStatusHandler exists + + // Root status routes use a descriptor-less handler that resolves the kind + // from the resource itself. For now, use a Channel descriptor as placeholder + // since the root handler fetches the resource by ID regardless of kind. + // TODO: HYPERFLEET-1157 — create a dedicated RootResourceStatusHandler + // that resolves the kind from the resource instead of a fixed descriptor. } -func registerResourceRoutes(apiV1Router *mux.Router, prefix string, h *handlers.ResourceHandler) { +func registerResourceRoutes( + apiV1Router *mux.Router, prefix string, + h *handlers.ResourceHandler, sh *handlers.ResourceStatusHandler, +) { r := apiV1Router.PathPrefix(prefix).Subrouter() r.HandleFunc("", h.List).Methods(http.MethodGet) r.HandleFunc("", h.Create).Methods(http.MethodPost) @@ -100,4 +118,6 @@ func registerResourceRoutes(apiV1Router *mux.Router, prefix string, h *handlers. r.HandleFunc("/{id}", h.Patch).Methods(http.MethodPatch) r.HandleFunc("/{id}", h.Delete).Methods(http.MethodDelete) r.HandleFunc("/{id}/force-delete", h.ForceDelete).Methods(http.MethodPost) + r.HandleFunc("/{id}/statuses", sh.List).Methods(http.MethodGet) + r.HandleFunc("/{id}/statuses", sh.Create).Methods(http.MethodPut) } diff --git a/plugins/entities/plugin_test.go b/plugins/entities/plugin_test.go index 724be845..4de5478d 100644 --- a/plugins/entities/plugin_test.go +++ b/plugins/entities/plugin_test.go @@ -4,6 +4,7 @@ import ( "net/http/httptest" "testing" + "github.com/google/uuid" "github.com/gorilla/mux" . "github.com/onsi/gomega" @@ -21,13 +22,16 @@ func TestRegisterEntityRoutes_TopLevelEntity(t *testing.T) { router := mux.NewRouter() apiV1 := router.PathPrefix("/api/hyperfleet/v1").Subrouter() - RegisterEntityRoutes(apiV1, nil, nil) + RegisterEntityRoutes(apiV1, nil, nil, nil) + id := uuid.NewString() assertRouteMatches(t, router, "GET", "/api/hyperfleet/v1/channels") assertRouteMatches(t, router, "POST", "/api/hyperfleet/v1/channels") - assertRouteMatches(t, router, "GET", "/api/hyperfleet/v1/channels/00000000-0000-0000-0000-000000000001") - assertRouteMatches(t, router, "PATCH", "/api/hyperfleet/v1/channels/00000000-0000-0000-0000-000000000001") - assertRouteMatches(t, router, "DELETE", "/api/hyperfleet/v1/channels/00000000-0000-0000-0000-000000000001") + assertRouteMatches(t, router, "GET", "/api/hyperfleet/v1/channels/"+id) + assertRouteMatches(t, router, "PATCH", "/api/hyperfleet/v1/channels/"+id) + assertRouteMatches(t, router, "DELETE", "/api/hyperfleet/v1/channels/"+id) + assertRouteMatches(t, router, "GET", "/api/hyperfleet/v1/channels/"+id+"/statuses") + assertRouteMatches(t, router, "PUT", "/api/hyperfleet/v1/channels/"+id+"/statuses") } func TestRegisterEntityRoutes_ChildEntity(t *testing.T) { @@ -42,10 +46,10 @@ func TestRegisterEntityRoutes_ChildEntity(t *testing.T) { router := mux.NewRouter() apiV1 := router.PathPrefix("/api/hyperfleet/v1").Subrouter() - RegisterEntityRoutes(apiV1, nil, nil) + RegisterEntityRoutes(apiV1, nil, nil, nil) - parentID := "00000000-0000-0000-0000-000000000001" - childID := "00000000-0000-0000-0000-000000000002" + parentID := uuid.NewString() + childID := uuid.NewString() nested := "/api/hyperfleet/v1/channels/" + parentID + "/versions" assertRouteMatches(t, router, "GET", nested) @@ -53,6 +57,8 @@ func TestRegisterEntityRoutes_ChildEntity(t *testing.T) { assertRouteMatches(t, router, "GET", nested+"/"+childID) assertRouteMatches(t, router, "PATCH", nested+"/"+childID) assertRouteMatches(t, router, "DELETE", nested+"/"+childID) + assertRouteMatches(t, router, "GET", nested+"/"+childID+"/statuses") + assertRouteMatches(t, router, "PUT", nested+"/"+childID+"/statuses") flat := "/api/hyperfleet/v1/versions" assertRouteMatches(t, router, "GET", flat) @@ -60,6 +66,25 @@ func TestRegisterEntityRoutes_ChildEntity(t *testing.T) { assertRouteMatches(t, router, "GET", flat+"/"+childID) assertRouteMatches(t, router, "PATCH", flat+"/"+childID) assertRouteMatches(t, router, "DELETE", flat+"/"+childID) + assertRouteMatches(t, router, "GET", flat+"/"+childID+"/statuses") + assertRouteMatches(t, router, "PUT", flat+"/"+childID+"/statuses") +} + +func TestRegisterEntityRoutes_UnresolvableParentKind_Panics(t *testing.T) { + RegisterTestingT(t) + registry.Reset() + registry.Register(registry.EntityDescriptor{ + Kind: "Version", + Plural: "versions", + ParentKind: "NonExistent", + }) + + router := mux.NewRouter() + apiV1 := router.PathPrefix("/api/hyperfleet/v1").Subrouter() + + Expect(func() { + RegisterEntityRoutes(apiV1, nil, nil, nil) + }).To(PanicWith(ContainSubstring("not registered"))) } func TestRegisterEntityRoutes_EmptyRegistry(t *testing.T) { @@ -70,7 +95,7 @@ func TestRegisterEntityRoutes_EmptyRegistry(t *testing.T) { apiV1 := router.PathPrefix("/api/hyperfleet/v1").Subrouter() Expect(func() { - RegisterEntityRoutes(apiV1, nil, nil) + RegisterEntityRoutes(apiV1, nil, nil, nil) }).ToNot(Panic()) } diff --git a/plugins/resources/plugin.go b/plugins/resources/plugin.go index 1510f9f9..4403e5d2 100644 --- a/plugins/resources/plugin.go +++ b/plugins/resources/plugin.go @@ -15,6 +15,8 @@ func NewServiceLocator(env *environments.Env) ServiceLocator { return func() services.ResourceService { return services.NewResourceService( dao.NewResourceDao(env.Database.SessionFactory), + dao.NewAdapterStatusDao(env.Database.SessionFactory), + dao.NewResourceConditionDao(env.Database.SessionFactory), generic.Service(&env.Services), ) } diff --git a/test/integration/resource_status_test.go b/test/integration/resource_status_test.go new file mode 100644 index 00000000..b2917786 --- /dev/null +++ b/test/integration/resource_status_test.go @@ -0,0 +1,206 @@ +package integration + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "testing" + + "github.com/google/uuid" + . "github.com/onsi/gomega" + "gopkg.in/resty.v1" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" + "github.com/openshift-hyperfleet/hyperfleet-api/test" +) + +const channelsPath = "/channels" + +func createTestChannelForStatus(t *testing.T, svc services.ResourceService) *api.Resource { + t.Helper() + name := fmt.Sprintf("status-ch-%s", uuid.NewString()[:8]) + channel := newChannelResource(name) + created, svcErr := svc.Create(context.Background(), "Channel", channel) + if svcErr != nil { + t.Fatalf("Failed to create channel for status test: %v", svcErr) + } + return created +} + +func TestResourceStatus_PutAndGet(t *testing.T) { + RegisterTestingT(t) + svc, h := setupResourceTest(t) + + channel := createTestChannelForStatus(t, svc) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + token := test.GetAccessTokenFromContext(ctx) + + // PUT adapter status + statusReq := newAdapterStatusRequest( + "test-adapter", channel.Generation, + []openapi.ConditionRequest{ + {Type: api.AdapterConditionTypeAvailable, Status: openapi.AdapterConditionStatusTrue}, + {Type: api.AdapterConditionTypeApplied, Status: openapi.AdapterConditionStatusTrue}, + {Type: api.AdapterConditionTypeHealth, Status: openapi.AdapterConditionStatusTrue}, + }, + nil, + ) + body, err := json.Marshal(statusReq) + Expect(err).NotTo(HaveOccurred()) + + putResp, err := resty.R(). + SetHeader("Content-Type", "application/json"). + SetHeader("Authorization", fmt.Sprintf("Bearer %s", token)). + SetBody(body). + Put(h.RestURL(channelsPath + "/" + channel.ID + "/statuses")) + Expect(err).NotTo(HaveOccurred()) + Expect(putResp.StatusCode()).To(Equal(http.StatusCreated)) + + // GET adapter statuses + getResp, err := resty.R(). + SetHeader("Authorization", fmt.Sprintf("Bearer %s", token)). + Get(h.RestURL(channelsPath + "/" + channel.ID + "/statuses")) + Expect(err).NotTo(HaveOccurred()) + Expect(getResp.StatusCode()).To(Equal(http.StatusOK)) + + var statusList openapi.AdapterStatusList + Expect(json.Unmarshal(getResp.Body(), &statusList)).To(Succeed()) + Expect(statusList.Items).To(HaveLen(1)) + Expect(statusList.Items[0].Adapter).To(Equal("test-adapter")) +} + +func TestResourceStatus_PutTriggersConditionsAggregation(t *testing.T) { + RegisterTestingT(t) + svc, h := setupResourceTest(t) + + channel := createTestChannelForStatus(t, svc) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + token := test.GetAccessTokenFromContext(ctx) + + statusReq := newAdapterStatusRequest( + "test-adapter", channel.Generation, + []openapi.ConditionRequest{ + {Type: api.AdapterConditionTypeAvailable, Status: openapi.AdapterConditionStatusTrue}, + {Type: api.AdapterConditionTypeApplied, Status: openapi.AdapterConditionStatusTrue}, + {Type: api.AdapterConditionTypeHealth, Status: openapi.AdapterConditionStatusTrue}, + }, + nil, + ) + body, _ := json.Marshal(statusReq) + + putResp, err := resty.R(). + SetHeader("Content-Type", "application/json"). + SetHeader("Authorization", fmt.Sprintf("Bearer %s", token)). + SetBody(body). + Put(h.RestURL(channelsPath + "/" + channel.ID + "/statuses")) + Expect(err).NotTo(HaveOccurred()) + Expect(putResp.StatusCode()).To(Equal(http.StatusCreated)) + + // GET the resource and verify conditions are populated + resource, svcErr := svc.Get(context.Background(), "Channel", channel.ID) + Expect(svcErr).To(BeNil()) + Expect(resource.Conditions).ToNot(BeEmpty(), "Conditions should be populated after adapter status report") +} + +func TestResourceStatus_NonExistentResource_Returns404(t *testing.T) { + RegisterTestingT(t) + _, h := setupResourceTest(t) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + token := test.GetAccessTokenFromContext(ctx) + + // GET statuses for non-existent resource + getResp, err := resty.R(). + SetHeader("Authorization", fmt.Sprintf("Bearer %s", token)). + Get(h.RestURL(channelsPath + "/nonexistent-id/statuses")) + Expect(err).NotTo(HaveOccurred()) + Expect(getResp.StatusCode()).To(Equal(http.StatusNotFound)) +} + +func TestResourceStatus_NestedEntityStatuses(t *testing.T) { + RegisterTestingT(t) + svc, h := setupResourceTest(t) + + channel := createTestChannelForStatus(t, svc) + + versionName := fmt.Sprintf("status-ver-%s", uuid.NewString()[:8]) + version := newVersionResource(versionName, channel.ID) + createdVersion, svcErr := svc.Create(context.Background(), "Version", version) + Expect(svcErr).To(BeNil()) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + token := test.GetAccessTokenFromContext(ctx) + + statusReq := newAdapterStatusRequest( + "version-adapter", createdVersion.Generation, + []openapi.ConditionRequest{ + {Type: api.AdapterConditionTypeAvailable, Status: openapi.AdapterConditionStatusTrue}, + {Type: api.AdapterConditionTypeApplied, Status: openapi.AdapterConditionStatusTrue}, + {Type: api.AdapterConditionTypeHealth, Status: openapi.AdapterConditionStatusTrue}, + }, + nil, + ) + body, _ := json.Marshal(statusReq) + + // PUT via nested path + nestedPath := fmt.Sprintf("%s/%s/versions/%s/statuses", channelsPath, channel.ID, createdVersion.ID) + putResp, err := resty.R(). + SetHeader("Content-Type", "application/json"). + SetHeader("Authorization", fmt.Sprintf("Bearer %s", token)). + SetBody(body). + Put(h.RestURL(nestedPath)) + Expect(err).NotTo(HaveOccurred()) + Expect(putResp.StatusCode()).To(Equal(http.StatusCreated)) + + // GET via nested path + getResp, err := resty.R(). + SetHeader("Authorization", fmt.Sprintf("Bearer %s", token)). + Get(h.RestURL(nestedPath)) + Expect(err).NotTo(HaveOccurred()) + Expect(getResp.StatusCode()).To(Equal(http.StatusOK)) + + var statusList openapi.AdapterStatusList + Expect(json.Unmarshal(getResp.Body(), &statusList)).To(Succeed()) + Expect(statusList.Items).To(HaveLen(1)) + Expect(statusList.Items[0].Adapter).To(Equal("version-adapter")) +} + +func TestResourceStatus_DiscardedUpdate_Returns204(t *testing.T) { + RegisterTestingT(t) + svc, h := setupResourceTest(t) + + channel := createTestChannelForStatus(t, svc) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + token := test.GetAccessTokenFromContext(ctx) + + // PUT with future generation → discarded → 204 + statusReq := newAdapterStatusRequest( + "test-adapter", 999, + []openapi.ConditionRequest{ + {Type: api.AdapterConditionTypeAvailable, Status: openapi.AdapterConditionStatusTrue}, + {Type: api.AdapterConditionTypeApplied, Status: openapi.AdapterConditionStatusTrue}, + {Type: api.AdapterConditionTypeHealth, Status: openapi.AdapterConditionStatusTrue}, + }, + nil, + ) + body, _ := json.Marshal(statusReq) + + putResp, err := resty.R(). + SetHeader("Content-Type", "application/json"). + SetHeader("Authorization", fmt.Sprintf("Bearer %s", token)). + SetBody(body). + Put(h.RestURL(channelsPath + "/" + channel.ID + "/statuses")) + Expect(err).NotTo(HaveOccurred()) + Expect(putResp.StatusCode()).To(Equal(http.StatusNoContent)) +} From 563fe73ec1e9a480721f6422fed296a3b20297d8 Mon Sep 17 00:00:00 2001 From: tithakka Date: Tue, 7 Jul 2026 10:58:31 -0500 Subject: [PATCH 2/7] HYPERFLEET-1154 - feat: fix linter error --- claude-skills | 1 - pkg/handlers/resource_status_handler.go | 2 +- pkg/handlers/resource_status_handler_test.go | 43 +++++++++++--------- 3 files changed, 24 insertions(+), 22 deletions(-) delete mode 160000 claude-skills diff --git a/claude-skills b/claude-skills deleted file mode 160000 index 33fdfa74..00000000 --- a/claude-skills +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 33fdfa74a530cf76c4a686d6e653614b1182068e diff --git a/pkg/handlers/resource_status_handler.go b/pkg/handlers/resource_status_handler.go index 2a8d90bd..d1e220a4 100644 --- a/pkg/handlers/resource_status_handler.go +++ b/pkg/handlers/resource_status_handler.go @@ -18,9 +18,9 @@ import ( // ClusterStatusHandler but uses descriptor.Kind as the resource type // and delegates to ResourceService.ProcessAdapterStatus. type ResourceStatusHandler struct { - descriptor registry.EntityDescriptor resourceService services.ResourceService adapterStatusService services.AdapterStatusService + descriptor registry.EntityDescriptor } func NewResourceStatusHandler( diff --git a/pkg/handlers/resource_status_handler_test.go b/pkg/handlers/resource_status_handler_test.go index 8712d6bb..a6c05f65 100644 --- a/pkg/handlers/resource_status_handler_test.go +++ b/pkg/handlers/resource_status_handler_test.go @@ -19,6 +19,8 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" ) +const testChannelID = "ch-1" + func newTestResourceStatusHandler( ctrl *gomock.Controller, ) (*ResourceStatusHandler, *services.MockResourceService, *services.MockAdapterStatusService) { @@ -35,26 +37,26 @@ func TestResourceStatusHandler_List(t *testing.T) { handler, mockResourceSvc, mockAdapterSvc := newTestResourceStatusHandler(ctrl) resource := &api.Resource{Kind: "Channel"} - resource.ID = "ch-1" - mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", "ch-1").Return(resource, nil) + resource.ID = testChannelID + mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", testChannelID).Return(resource, nil) now := time.Now().UTC() statuses := api.AdapterStatusList{ { Adapter: "adapter1", ResourceType: "Channel", - ResourceID: "ch-1", + ResourceID: testChannelID, ObservedGeneration: 1, LastReportTime: now, Conditions: datatypes.JSON(`[{"type":"Available","status":"True"}]`), }, } mockAdapterSvc.EXPECT().FindByResourcePaginated( - gomock.Any(), "Channel", "ch-1", gomock.Any(), + gomock.Any(), "Channel", testChannelID, gomock.Any(), ).Return(statuses, int64(1), nil) r := httptest.NewRequest(http.MethodGet, "/channels/ch-1/statuses", nil) - r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + r = mux.SetURLVars(r, map[string]string{"id": testChannelID}) w := httptest.NewRecorder() handler.List(w, r) @@ -73,11 +75,11 @@ func TestResourceStatusHandler_List_ResourceNotFound(t *testing.T) { handler, mockResourceSvc, _ := newTestResourceStatusHandler(ctrl) - mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", "ch-1"). + mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", testChannelID). Return(nil, errors.NotFound("Channel 'ch-1' not found")) r := httptest.NewRequest(http.MethodGet, "/channels/ch-1/statuses", nil) - r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + r = mux.SetURLVars(r, map[string]string{"id": testChannelID}) w := httptest.NewRecorder() handler.List(w, r) @@ -92,20 +94,21 @@ func TestResourceStatusHandler_Create_HappyPath(t *testing.T) { handler, mockResourceSvc, _ := newTestResourceStatusHandler(ctrl) resource := &api.Resource{Kind: "Channel"} - resource.ID = "ch-1" - mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", "ch-1").Return(resource, nil) + resource.ID = testChannelID + mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", testChannelID).Return(resource, nil) now := time.Now().UTC() returnedStatus := &api.AdapterStatus{ Adapter: "adapter1", ResourceType: "Channel", - ResourceID: "ch-1", + ResourceID: testChannelID, ObservedGeneration: 1, LastReportTime: now, - Conditions: datatypes.JSON(`[{"type":"Available","status":"True"},{"type":"Applied","status":"True"},{"type":"Health","status":"True"}]`), + Conditions: datatypes.JSON( //nolint:lll + `[{"type":"Available","status":"True"},{"type":"Applied","status":"True"},{"type":"Health","status":"True"}]`), } mockResourceSvc.EXPECT().ProcessAdapterStatus( - gomock.Any(), "Channel", "ch-1", gomock.Any(), + gomock.Any(), "Channel", testChannelID, gomock.Any(), ).Return(returnedStatus, nil) observedTime := now @@ -123,7 +126,7 @@ func TestResourceStatusHandler_Create_HappyPath(t *testing.T) { r := httptest.NewRequest(http.MethodPut, "/channels/ch-1/statuses", strings.NewReader(string(bodyJSON))) r.Header.Set("Content-Type", "application/json") - r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + r = mux.SetURLVars(r, map[string]string{"id": testChannelID}) w := httptest.NewRecorder() handler.Create(w, r) @@ -138,10 +141,10 @@ func TestResourceStatusHandler_Create_Discarded_Returns204(t *testing.T) { handler, mockResourceSvc, _ := newTestResourceStatusHandler(ctrl) resource := &api.Resource{Kind: "Channel"} - resource.ID = "ch-1" - mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", "ch-1").Return(resource, nil) + resource.ID = testChannelID + mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", testChannelID).Return(resource, nil) mockResourceSvc.EXPECT().ProcessAdapterStatus( - gomock.Any(), "Channel", "ch-1", gomock.Any(), + gomock.Any(), "Channel", testChannelID, gomock.Any(), ).Return(nil, nil) now := time.Now().UTC() @@ -159,7 +162,7 @@ func TestResourceStatusHandler_Create_Discarded_Returns204(t *testing.T) { r := httptest.NewRequest(http.MethodPut, "/channels/ch-1/statuses", strings.NewReader(string(bodyJSON))) r.Header.Set("Content-Type", "application/json") - r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + r = mux.SetURLVars(r, map[string]string{"id": testChannelID}) w := httptest.NewRecorder() handler.Create(w, r) @@ -183,7 +186,7 @@ func TestResourceStatusHandler_Create_MissingAdapter_Returns400(t *testing.T) { r := httptest.NewRequest(http.MethodPut, "/channels/ch-1/statuses", strings.NewReader(string(bodyJSON))) r.Header.Set("Content-Type", "application/json") - r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + r = mux.SetURLVars(r, map[string]string{"id": testChannelID}) w := httptest.NewRecorder() handler.Create(w, r) @@ -197,7 +200,7 @@ func TestResourceStatusHandler_Create_ResourceNotFound(t *testing.T) { handler, mockResourceSvc, _ := newTestResourceStatusHandler(ctrl) - mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", "ch-1"). + mockResourceSvc.EXPECT().Get(gomock.Any(), "Channel", testChannelID). Return(nil, errors.NotFound("Channel 'ch-1' not found")) now := time.Now().UTC() @@ -215,7 +218,7 @@ func TestResourceStatusHandler_Create_ResourceNotFound(t *testing.T) { r := httptest.NewRequest(http.MethodPut, "/channels/ch-1/statuses", strings.NewReader(string(bodyJSON))) r.Header.Set("Content-Type", "application/json") - r = mux.SetURLVars(r, map[string]string{"id": "ch-1"}) + r = mux.SetURLVars(r, map[string]string{"id": testChannelID}) w := httptest.NewRecorder() handler.Create(w, r) From e799cd2cece3f41eecd8ddd6b88081e712cc01f2 Mon Sep 17 00:00:00 2001 From: tithakka Date: Wed, 8 Jul 2026 10:25:56 -0500 Subject: [PATCH 3/7] HYPERFLEET-1154 - feat: fix linter error and create a private function --- pkg/handlers/resource_status_handler.go | 10 ++-- pkg/services/node_pool.go | 3 -- pkg/services/resource.go | 65 +++++++++++++------------ plugins/entities/plugin.go | 6 --- 4 files changed, 41 insertions(+), 43 deletions(-) diff --git a/pkg/handlers/resource_status_handler.go b/pkg/handlers/resource_status_handler.go index d1e220a4..8b18263f 100644 --- a/pkg/handlers/resource_status_handler.go +++ b/pkg/handlers/resource_status_handler.go @@ -9,6 +9,7 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/presenters" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" ) @@ -155,7 +156,8 @@ func (h *ResourceStatusHandler) listStatuses( for _, as := range adapterStatuses { presented, presErr := presenters.PresentAdapterStatus(as) if presErr != nil { - return nil, errors.GeneralError("Failed to present adapter status: %v", presErr) + logger.WithError(ctx, presErr).Error("Failed to present adapter status") + return nil, errors.GeneralError("Failed to present adapter status") } items = append(items, presented) } @@ -175,7 +177,8 @@ func (h *ResourceStatusHandler) processStatus( ) (interface{}, *errors.ServiceError) { newStatus, convErr := presenters.ConvertAdapterStatus(h.descriptor.Kind, resourceID, req) if convErr != nil { - return nil, errors.GeneralError("Failed to convert adapter status: %v", convErr) + logger.WithError(ctx, convErr).Error("Failed to convert adapter status") + return nil, errors.GeneralError("Failed to convert adapter status") } adapterStatus, err := h.resourceService.ProcessAdapterStatus( @@ -191,7 +194,8 @@ func (h *ResourceStatusHandler) processStatus( status, presErr := presenters.PresentAdapterStatus(adapterStatus) if presErr != nil { - return nil, errors.GeneralError("Failed to present adapter status: %v", presErr) + logger.WithError(ctx, presErr).Error("Failed to present adapter status") + return nil, errors.GeneralError("Failed to present adapter status") } return &status, nil } diff --git a/pkg/services/node_pool.go b/pkg/services/node_pool.go index 093ac67e..e7753186 100644 --- a/pkg/services/node_pool.go +++ b/pkg/services/node_pool.go @@ -414,9 +414,6 @@ func (s *sqlNodePoolService) ProcessAdapterStatus( return upsertedStatus, nil } -// validateAndClassifyNodePool performs all stateless validation and discard-rule checks on an -// incoming adapter status for a node pool. Returns the parsed conditions and whether aggregation -// should be triggered. Returns (nil, false, nil) when the update should be silently discarded. // validateAndClassifyNodePool delegates to the shared validateAndClassifyAdapterStatus // with nodepool-specific logger context. func (s *sqlNodePoolService) validateAndClassifyNodePool( diff --git a/pkg/services/resource.go b/pkg/services/resource.go index 7508732b..32fe7c84 100644 --- a/pkg/services/resource.go +++ b/pkg/services/resource.go @@ -25,7 +25,7 @@ type ResourceService interface { Delete(ctx context.Context, kind, id string) (*api.Resource, *errors.ServiceError) List(ctx context.Context, kind string, args *ListArguments) (api.ResourceList, *api.PagingMeta, *errors.ServiceError) GetByOwner(ctx context.Context, kind, id, ownerID string) (*api.Resource, *errors.ServiceError) - ListByOwner(ctx context.Context, kind, ownerID string, args *ListArguments) (api.ResourceList, *api.PagingMeta, *errors.ServiceError) // nolint:lll + ListByOwner(ctx context.Context, kind, ownerID string, args *ListArguments) (api.ResourceList, *api.PagingMeta, *errors.ServiceError) // nolint:lll ForceDelete(ctx context.Context, kind, id, reason string) *errors.ServiceError GetByID(ctx context.Context, id string) (*api.Resource, *errors.ServiceError) ListAll(ctx context.Context, args *ListArguments) (api.ResourceList, *api.PagingMeta, *errors.ServiceError) @@ -501,17 +501,10 @@ func (s *sqlResourceService) recomputeAndSaveResourceConditions( // whose children haven't finished their own reconciliation. hasChildResources := false if resource.DeletedTime != nil { - for _, child := range registry.ChildrenOf(resource.Kind) { - exists, err := s.resourceDao.ExistsByOwner(ctx, child.Kind, resource.ID) - if err != nil { - return errors.GeneralError( - "Failed to check child %s for status aggregation: %s", child.Kind, err, - ) - } - if exists { - hasChildResources = true - break - } + var err error + hasChildResources, err = s.hasActiveChildren(ctx, resource) + if err != nil { + return errors.GeneralError("Failed to check children for status aggregation: %s", err) } } @@ -585,32 +578,25 @@ func (s *sqlResourceService) tryHardDeleteResource( return false, nil } - // Ensure no active children exist — hard-deleting a parent with active - // children would leave orphaned resources. - children := registry.ChildrenOf(resource.Kind) - for _, child := range children { - exists, err := s.resourceDao.ExistsByOwner(ctx, child.Kind, resource.ID) - if err != nil { - return false, errors.GeneralError( - "Failed to check %s children during hard-delete: %s", child.Kind, err, - ) - } - if exists { - return false, nil - } + // Ensure no children exist (active or soft-deleted) — hard-deleting a + // parent with remaining children would leave orphaned resources. + hasActive, err := s.hasActiveChildren(ctx, resource) + if err != nil { + return false, errors.GeneralError("Failed to check children during hard-delete: %s", err) + } + if hasActive { + return false, nil } - // Also check for soft-deleted children still pending adapter finalization. + children := registry.ChildrenOf(resource.Kind) if len(children) > 0 { childKinds := make([]string, len(children)) for i, c := range children { childKinds[i] = c.Kind } - hasSoftDeleted, err := s.resourceDao.ExistsSoftDeletedByOwner(ctx, childKinds, resource.ID) - if err != nil { - return false, errors.GeneralError( - "Failed to check soft-deleted children during hard-delete: %s", err, - ) + hasSoftDeleted, sdErr := s.resourceDao.ExistsSoftDeletedByOwner(ctx, childKinds, resource.ID) + if sdErr != nil { + return false, errors.GeneralError("Failed to check soft-deleted children during hard-delete: %s", sdErr) } if hasSoftDeleted { return false, nil @@ -636,6 +622,23 @@ func (s *sqlResourceService) tryHardDeleteResource( return true, nil } +// hasActiveChildren returns true if any registered child kind has at least one +// active (non-deleted) resource owned by the given resource. +func (s *sqlResourceService) hasActiveChildren( + ctx context.Context, resource *api.Resource, +) (bool, error) { + for _, child := range registry.ChildrenOf(resource.Kind) { + exists, err := s.resourceDao.ExistsByOwner(ctx, child.Kind, resource.ID) + if err != nil { + return false, err + } + if exists { + return true, nil + } + } + return false, nil +} + // validateKind checks that the kind is a registered entity type. // Returns 400 if the kind is unknown, preventing invalid kinds from reaching the DAO. func validateKind(kind string) *errors.ServiceError { diff --git a/plugins/entities/plugin.go b/plugins/entities/plugin.go index 101014c7..9e73310d 100644 --- a/plugins/entities/plugin.go +++ b/plugins/entities/plugin.go @@ -99,12 +99,6 @@ func registerRootResourceRoutes( r.HandleFunc("/{id}", rootHandler.Patch).Methods(http.MethodPatch) r.HandleFunc("/{id}", rootHandler.Delete).Methods(http.MethodDelete) r.HandleFunc("/{id}/force-delete", rootHandler.ForceDelete).Methods(http.MethodPost) - - // Root status routes use a descriptor-less handler that resolves the kind - // from the resource itself. For now, use a Channel descriptor as placeholder - // since the root handler fetches the resource by ID regardless of kind. - // TODO: HYPERFLEET-1157 — create a dedicated RootResourceStatusHandler - // that resolves the kind from the resource instead of a fixed descriptor. } func registerResourceRoutes( From 373295a59447643f66c7924469bbe9a82088d55c Mon Sep 17 00:00:00 2001 From: tithakka Date: Wed, 8 Jul 2026 11:55:35 -0500 Subject: [PATCH 4/7] HYPERFLEET-1154 - feat: add resource status handlers and routes --- pkg/handlers/resource_status_handler_test.go | 152 +++++++++++++++++++ pkg/handlers/root_resource_handler.go | 104 ++++++++++++- plugins/entities/plugin.go | 4 +- plugins/entities/plugin_test.go | 4 + 4 files changed, 260 insertions(+), 4 deletions(-) diff --git a/pkg/handlers/resource_status_handler_test.go b/pkg/handlers/resource_status_handler_test.go index a6c05f65..a22861e2 100644 --- a/pkg/handlers/resource_status_handler_test.go +++ b/pkg/handlers/resource_status_handler_test.go @@ -225,3 +225,155 @@ func TestResourceStatusHandler_Create_ResourceNotFound(t *testing.T) { Expect(w.Code).To(Equal(http.StatusNotFound)) } + +// ─── RootResourceHandler status tests ────────────────────────────── + +func newTestRootResourceHandler( + ctrl *gomock.Controller, +) (*RootResourceHandler, *services.MockResourceService, *services.MockAdapterStatusService) { + mockResourceSvc := services.NewMockResourceService(ctrl) + mockAdapterSvc := services.NewMockAdapterStatusService(ctrl) + handler := NewRootResourceHandler(mockResourceSvc, mockAdapterSvc, nil) + return handler, mockResourceSvc, mockAdapterSvc +} + +func TestRootResourceHandler_ListStatuses(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, mockAdapterSvc := newTestRootResourceHandler(ctrl) + + resource := &api.Resource{Kind: "Channel"} + resource.ID = testChannelID + mockResourceSvc.EXPECT().GetByID(gomock.Any(), testChannelID).Return(resource, nil) + + now := time.Now().UTC() + statuses := api.AdapterStatusList{ + { + Adapter: "adapter1", + ResourceType: "Channel", + ResourceID: testChannelID, + ObservedGeneration: 1, + LastReportTime: now, + Conditions: datatypes.JSON(`[{"type":"Available","status":"True"}]`), + }, + } + mockAdapterSvc.EXPECT().FindByResourcePaginated( + gomock.Any(), "Channel", testChannelID, gomock.Any(), + ).Return(statuses, int64(1), nil) + + r := httptest.NewRequest(http.MethodGet, "/resources/"+testChannelID+"/statuses", nil) + r = mux.SetURLVars(r, map[string]string{"id": testChannelID}) + w := httptest.NewRecorder() + + handler.ListStatuses(w, r) + + Expect(w.Code).To(Equal(http.StatusOK)) + + var response openapi.AdapterStatusList + Expect(json.Unmarshal(w.Body.Bytes(), &response)).To(Succeed()) + Expect(response.Items).To(HaveLen(1)) + Expect(response.Total).To(Equal(int32(1))) +} + +func TestRootResourceHandler_ListStatuses_ResourceNotFound(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, _ := newTestRootResourceHandler(ctrl) + + mockResourceSvc.EXPECT().GetByID(gomock.Any(), testChannelID). + Return(nil, errors.NotFound("Resource '%s' not found", testChannelID)) + + r := httptest.NewRequest(http.MethodGet, "/resources/"+testChannelID+"/statuses", nil) + r = mux.SetURLVars(r, map[string]string{"id": testChannelID}) + w := httptest.NewRecorder() + + handler.ListStatuses(w, r) + + Expect(w.Code).To(Equal(http.StatusNotFound)) +} + +func TestRootResourceHandler_CreateStatus_HappyPath(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, _ := newTestRootResourceHandler(ctrl) + + resource := &api.Resource{Kind: "Channel"} + resource.ID = testChannelID + mockResourceSvc.EXPECT().GetByID(gomock.Any(), testChannelID).Return(resource, nil) + + now := time.Now().UTC() + returnedStatus := &api.AdapterStatus{ + Adapter: "adapter1", + ResourceType: "Channel", + ResourceID: testChannelID, + ObservedGeneration: 1, + LastReportTime: now, + Conditions: datatypes.JSON( //nolint:lll + `[{"type":"Available","status":"True"},{"type":"Applied","status":"True"},{"type":"Health","status":"True"}]`), + } + mockResourceSvc.EXPECT().ProcessAdapterStatus( + gomock.Any(), "Channel", testChannelID, gomock.Any(), + ).Return(returnedStatus, nil) + + body := openapi.AdapterStatusCreateRequest{ + Adapter: "adapter1", + ObservedGeneration: 1, + ObservedTime: now, + Conditions: []openapi.ConditionRequest{ + {Type: "Available", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Applied", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Health", Status: openapi.AdapterConditionStatusTrue}, + }, + } + bodyJSON, _ := json.Marshal(body) + + r := httptest.NewRequest(http.MethodPut, "/resources/"+testChannelID+"/statuses", + strings.NewReader(string(bodyJSON))) + r.Header.Set("Content-Type", "application/json") + r = mux.SetURLVars(r, map[string]string{"id": testChannelID}) + w := httptest.NewRecorder() + + handler.CreateStatus(w, r) + + Expect(w.Code).To(Equal(http.StatusCreated)) +} + +func TestRootResourceHandler_CreateStatus_Discarded_Returns204(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, _ := newTestRootResourceHandler(ctrl) + + resource := &api.Resource{Kind: "Channel"} + resource.ID = testChannelID + mockResourceSvc.EXPECT().GetByID(gomock.Any(), testChannelID).Return(resource, nil) + mockResourceSvc.EXPECT().ProcessAdapterStatus( + gomock.Any(), "Channel", testChannelID, gomock.Any(), + ).Return(nil, nil) + + now := time.Now().UTC() + body := openapi.AdapterStatusCreateRequest{ + Adapter: "adapter1", + ObservedGeneration: 1, + ObservedTime: now, + Conditions: []openapi.ConditionRequest{ + {Type: "Available", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Applied", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Health", Status: openapi.AdapterConditionStatusTrue}, + }, + } + bodyJSON, _ := json.Marshal(body) + + r := httptest.NewRequest(http.MethodPut, "/resources/"+testChannelID+"/statuses", + strings.NewReader(string(bodyJSON))) + r.Header.Set("Content-Type", "application/json") + r = mux.SetURLVars(r, map[string]string{"id": testChannelID}) + w := httptest.NewRecorder() + + handler.CreateStatus(w, r) + + Expect(w.Code).To(Equal(http.StatusNoContent)) +} diff --git a/pkg/handlers/root_resource_handler.go b/pkg/handlers/root_resource_handler.go index 8a13226f..5f4bc570 100644 --- a/pkg/handlers/root_resource_handler.go +++ b/pkg/handlers/root_resource_handler.go @@ -9,21 +9,28 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/presenters" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/validators" ) type RootResourceHandler struct { - service services.ResourceService - validator *validators.SchemaValidator + service services.ResourceService + adapterStatusService services.AdapterStatusService + validator *validators.SchemaValidator } func NewRootResourceHandler( service services.ResourceService, + adapterStatusService services.AdapterStatusService, validator *validators.SchemaValidator, ) *RootResourceHandler { - return &RootResourceHandler{service: service, validator: validator} + return &RootResourceHandler{ + service: service, + adapterStatusService: adapterStatusService, + validator: validator, + } } func (h *RootResourceHandler) List(w http.ResponseWriter, r *http.Request) { @@ -187,3 +194,94 @@ func (h *RootResourceHandler) Delete(w http.ResponseWriter, r *http.Request) { } handleSoftDelete(w, r, cfg) } + +// ListStatuses returns adapter statuses for a resource resolved by ID. +func (h *RootResourceHandler) ListStatuses(w http.ResponseWriter, r *http.Request) { + cfg := &handlerConfig{ + Action: func() (interface{}, *errors.ServiceError) { + ctx := r.Context() + id := mux.Vars(r)["id"] + listArgs, err := services.NewListArguments(r.URL.Query()) + if err != nil { + return nil, err + } + + resource, err := h.service.GetByID(ctx, id) + if err != nil { + return nil, err + } + + statuses, total, svcErr := h.adapterStatusService.FindByResourcePaginated( + ctx, resource.Kind, id, listArgs, + ) + if svcErr != nil { + return nil, svcErr + } + + items := make([]openapi.AdapterStatus, 0, len(statuses)) + for _, as := range statuses { + presented, presErr := presenters.PresentAdapterStatus(as) + if presErr != nil { + logger.WithError(ctx, presErr).Error("Failed to present adapter status") + return nil, errors.GeneralError("Failed to present adapter status") + } + items = append(items, presented) + } + + return openapi.AdapterStatusList{ + Items: items, + Page: int32(listArgs.Page), + Size: int32(len(items)), + Total: int32(total), + }, nil + }, + } + handleList(w, r, cfg) +} + +// CreateStatus creates or updates an adapter status for a resource resolved by ID. +func (h *RootResourceHandler) CreateStatus(w http.ResponseWriter, r *http.Request) { + var req openapi.AdapterStatusCreateRequest + + cfg := &handlerConfig{ + MarshalInto: &req, + Validate: []validate{ + validateNotEmpty(&req, "Adapter", "adapter"), + validateObservedGeneration(&req), + validateConditions(&req, "Conditions"), + validateObservedTimeRange(&req.ObservedTime), + }, + Action: func() (interface{}, *errors.ServiceError) { + ctx := r.Context() + id := mux.Vars(r)["id"] + + resource, err := h.service.GetByID(ctx, id) + if err != nil { + return nil, err + } + + newStatus, convErr := presenters.ConvertAdapterStatus(resource.Kind, id, &req) + if convErr != nil { + logger.WithError(ctx, convErr).Error("Failed to convert adapter status") + return nil, errors.GeneralError("Failed to convert adapter status") + } + + adapterStatus, svcErr := h.service.ProcessAdapterStatus(ctx, resource.Kind, id, newStatus) + if svcErr != nil { + return nil, svcErr + } + + if adapterStatus == nil { + return nil, nil + } + + status, presErr := presenters.PresentAdapterStatus(adapterStatus) + if presErr != nil { + logger.WithError(ctx, presErr).Error("Failed to present adapter status") + return nil, errors.GeneralError("Failed to present adapter status") + } + return &status, nil + }, + } + handleCreateWithNoContent(w, r, cfg) +} diff --git a/plugins/entities/plugin.go b/plugins/entities/plugin.go index 9e73310d..817ea0c3 100644 --- a/plugins/entities/plugin.go +++ b/plugins/entities/plugin.go @@ -91,7 +91,7 @@ func registerRootResourceRoutes( adapterStatusService services.AdapterStatusService, schemaValidator *validators.SchemaValidator, ) { - rootHandler := handlers.NewRootResourceHandler(resourceService, schemaValidator) + rootHandler := handlers.NewRootResourceHandler(resourceService, adapterStatusService, schemaValidator) r := apiV1Router.PathPrefix("/resources").Subrouter() r.HandleFunc("", rootHandler.List).Methods(http.MethodGet) r.HandleFunc("", rootHandler.Create).Methods(http.MethodPost) @@ -99,6 +99,8 @@ func registerRootResourceRoutes( r.HandleFunc("/{id}", rootHandler.Patch).Methods(http.MethodPatch) r.HandleFunc("/{id}", rootHandler.Delete).Methods(http.MethodDelete) r.HandleFunc("/{id}/force-delete", rootHandler.ForceDelete).Methods(http.MethodPost) + r.HandleFunc("/{id}/statuses", rootHandler.ListStatuses).Methods(http.MethodGet) + r.HandleFunc("/{id}/statuses", rootHandler.CreateStatus).Methods(http.MethodPut) } func registerResourceRoutes( diff --git a/plugins/entities/plugin_test.go b/plugins/entities/plugin_test.go index 4de5478d..d53b693d 100644 --- a/plugins/entities/plugin_test.go +++ b/plugins/entities/plugin_test.go @@ -32,6 +32,10 @@ func TestRegisterEntityRoutes_TopLevelEntity(t *testing.T) { assertRouteMatches(t, router, "DELETE", "/api/hyperfleet/v1/channels/"+id) assertRouteMatches(t, router, "GET", "/api/hyperfleet/v1/channels/"+id+"/statuses") assertRouteMatches(t, router, "PUT", "/api/hyperfleet/v1/channels/"+id+"/statuses") + + // Root /resources routes should also have statuses + assertRouteMatches(t, router, "GET", "/api/hyperfleet/v1/resources/"+id+"/statuses") + assertRouteMatches(t, router, "PUT", "/api/hyperfleet/v1/resources/"+id+"/statuses") } func TestRegisterEntityRoutes_ChildEntity(t *testing.T) { From a6e83042aa58a41d9f15ce310fea17b118fa88ee Mon Sep 17 00:00:00 2001 From: tithakka Date: Wed, 8 Jul 2026 12:54:45 -0500 Subject: [PATCH 5/7] HYPERFLEET-1154 - feat: removed guard --- pkg/services/resource.go | 18 +++++---------- pkg/services/resource_test.go | 22 ++++++------------- .../integration/resource_force_delete_test.go | 12 ---------- 3 files changed, 13 insertions(+), 39 deletions(-) diff --git a/pkg/services/resource.go b/pkg/services/resource.go index 32fe7c84..c2c9804e 100644 --- a/pkg/services/resource.go +++ b/pkg/services/resource.go @@ -720,18 +720,6 @@ func (s *sqlResourceService) ForceDelete(ctx context.Context, kind, id, reason s func (s *sqlResourceService) forceDeleteResourceTree( ctx context.Context, resource *api.Resource, caller, reason string, ) *errors.ServiceError { - desc := registry.MustGet(resource.Kind) - if len(desc.RequiredAdapters) > 0 { - // When HYPERFLEET-1154 adds adapter_status cleanup here, reuse - // buildAdapterSummaries (util.go) for the audit log, matching - // cluster.go's logForceDeleteAudit — don't hand-roll formatting again. - return errors.GeneralError( - "force-delete not implemented for resources with required adapters (kind=%s)"+ - " — adapter_status cleanup needed, see HYPERFLEET-1154", - resource.Kind, - ) - } - children := registry.ChildrenOf(resource.Kind) childIDs := make([]string, 0) @@ -758,6 +746,12 @@ func (s *sqlResourceService) forceDeleteResourceTree( "child_resource_ids", childIDs, ).Info("Force-deleting resource") + if err := s.adapterStatusDao.DeleteByResource(ctx, resource.Kind, resource.ID); err != nil { + return errors.GeneralError("Failed to delete adapter statuses during force-delete: %s", err) + } + if err := s.resourceConditionDao.DeleteByResource(ctx, resource.ID); err != nil { + return errors.GeneralError("Failed to delete resource conditions during force-delete: %s", err) + } if err := s.resourceDao.Delete(ctx, resource.Kind, resource.ID); err != nil { return handleDeleteError(resource.Kind, err) } diff --git a/pkg/services/resource_test.go b/pkg/services/resource_test.go index d67e65ae..640ec061 100644 --- a/pkg/services/resource_test.go +++ b/pkg/services/resource_test.go @@ -213,12 +213,9 @@ func (d *resourceConditionMock) DeleteByResource(_ context.Context, resourceID s var _ dao.ResourceConditionDao = &resourceConditionMock{} -// newTestResourceService creates a ResourceService for CRUD-only tests. -// adapterStatusDao and resourceConditionDao are nil — tests that call -// ProcessAdapterStatus must use newTestResourceServiceWithAdapterStatus. func newTestResourceService(mockDao *mockResourceDao) (ResourceService, *mockResourceDao, *resourceGenericMock) { generic := &resourceGenericMock{} - svc := NewResourceService(mockDao, nil, nil, generic) + svc := NewResourceService(mockDao, newMockAdapterStatusDao(), newResourceConditionMock(), generic) return svc, mockDao, generic } @@ -1528,7 +1525,7 @@ func TestResourceService_ForceDelete_RecursiveGrandchildren(t *testing.T) { Expect(mockDao.resources).To(HaveLen(0)) } -func TestResourceService_ForceDelete_RequiredAdaptersBlocked(t *testing.T) { +func TestResourceService_ForceDelete_WithRequiredAdapters_Succeeds(t *testing.T) { RegisterTestingT(t) setupManagedDescriptor() @@ -1543,11 +1540,11 @@ func TestResourceService_ForceDelete_RequiredAdaptersBlocked(t *testing.T) { mockDao.addResource(existing) svcErr := svc.ForceDelete(context.Background(), "Managed", "m-1", "some reason") - Expect(svcErr).ToNot(BeNil()) - Expect(svcErr.HTTPCode).To(Equal(500)) + Expect(svcErr).To(BeNil()) + Expect(mockDao.resources).To(HaveLen(0)) } -func TestResourceService_ForceDelete_ChildWithRequiredAdapters(t *testing.T) { +func TestResourceService_ForceDelete_ChildWithRequiredAdapters_Succeeds(t *testing.T) { RegisterTestingT(t) registry.Reset() registry.Register(registry.EntityDescriptor{Kind: "Parent", Plural: "parents"}) @@ -1573,13 +1570,8 @@ func TestResourceService_ForceDelete_ChildWithRequiredAdapters(t *testing.T) { mockDao.addResource(child) svcErr := svc.ForceDelete(context.Background(), "Parent", "p-1", "force it") - Expect(svcErr).ToNot(BeNil()) - Expect(svcErr.HTTPCode).To(Equal(500)) - - _, parentExists := mockDao.resources[resourceKey("Parent", "p-1")] - Expect(parentExists).To(BeTrue(), "parent should NOT be deleted when child cascade fails") - _, childExists := mockDao.resources[resourceKey("ManagedChild", "mc-1")] - Expect(childExists).To(BeTrue(), "child should NOT be deleted when guard fires") + Expect(svcErr).To(BeNil()) + Expect(mockDao.resources).To(HaveLen(0)) } func TestResourceService_ForceDelete_InvalidKind(t *testing.T) { diff --git a/test/integration/resource_force_delete_test.go b/test/integration/resource_force_delete_test.go index 95b6f096..a959c4a5 100644 --- a/test/integration/resource_force_delete_test.go +++ b/test/integration/resource_force_delete_test.go @@ -261,15 +261,3 @@ func TestResourceForceDeleteHTTP(t *testing.T) { Expect(resp.StatusCode()).To(Equal(http.StatusBadRequest)) }) } - -func TestGenericDescriptors_HaveNoRequiredAdapters(t *testing.T) { - RegisterTestingT(t) - _, _ = setupResourceTest(t) - - for _, d := range registry.All() { - Expect(d.RequiredAdapters).To(BeEmpty(), - "Descriptor %q has RequiredAdapters=%v. "+ - "ForceDelete does not yet handle adapter_status cleanup. "+ - "See HYPERFLEET-1154.", d.Kind, d.RequiredAdapters) - } -} From 1fcc39a7bf4e7150b94f804d429b158a075a3238 Mon Sep 17 00:00:00 2001 From: tithakka Date: Wed, 8 Jul 2026 13:10:11 -0500 Subject: [PATCH 6/7] HYPERFLEET-1154 - feat: fix linter --- test/integration/resource_force_delete_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/resource_force_delete_test.go b/test/integration/resource_force_delete_test.go index a959c4a5..28e473b0 100644 --- a/test/integration/resource_force_delete_test.go +++ b/test/integration/resource_force_delete_test.go @@ -9,7 +9,6 @@ import ( . "github.com/onsi/gomega" "gopkg.in/resty.v1" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" "github.com/openshift-hyperfleet/hyperfleet-api/test" ) From 180d94a511031a8f99ebcc4da8562de20d2c61ed Mon Sep 17 00:00:00 2001 From: tithakka Date: Wed, 8 Jul 2026 13:34:33 -0500 Subject: [PATCH 7/7] HYPERFLEET-1154 - feat: commonize by owner methods --- pkg/handlers/resource_status_handler.go | 81 ++++---------- pkg/handlers/resource_status_handler_test.go | 105 +++++++++++++++++++ 2 files changed, 128 insertions(+), 58 deletions(-) diff --git a/pkg/handlers/resource_status_handler.go b/pkg/handlers/resource_status_handler.go index 8b18263f..156e36d8 100644 --- a/pkg/handlers/resource_status_handler.go +++ b/pkg/handlers/resource_status_handler.go @@ -15,9 +15,9 @@ import ( ) // ResourceStatusHandler handles GET/PUT on /{plural}/{id}/statuses for -// config-driven generic resources. Follows the same pattern as -// ClusterStatusHandler but uses descriptor.Kind as the resource type -// and delegates to ResourceService.ProcessAdapterStatus. +// config-driven generic resources. Each method branches on whether "parent_id" +// is present in mux.Vars(r) to handle both flat and nested routes — matching +// the pattern established by ResourceHandler in HYPERFLEET-1157. type ResourceStatusHandler struct { resourceService services.ResourceService adapterStatusService services.AdapterStatusService @@ -37,6 +37,7 @@ func NewResourceStatusHandler( } // List returns all adapter statuses for a resource with pagination. +// Verifies ownership when parent_id is present in the route. func (h *ResourceStatusHandler) List(w http.ResponseWriter, r *http.Request) { cfg := &handlerConfig{ Action: func() (interface{}, *errors.ServiceError) { @@ -47,8 +48,8 @@ func (h *ResourceStatusHandler) List(w http.ResponseWriter, r *http.Request) { return nil, err } - if _, err = h.resourceService.Get(ctx, h.descriptor.Kind, id); err != nil { - return nil, err + if svcErr := h.verifyResource(r, id); svcErr != nil { + return nil, svcErr } return h.listStatuses(ctx, id, listArgs) @@ -60,6 +61,7 @@ func (h *ResourceStatusHandler) List(w http.ResponseWriter, r *http.Request) { } // Create creates or updates an adapter status for a resource. +// Verifies ownership when parent_id is present in the route. func (h *ResourceStatusHandler) Create(w http.ResponseWriter, r *http.Request) { var req openapi.AdapterStatusCreateRequest @@ -75,8 +77,8 @@ func (h *ResourceStatusHandler) Create(w http.ResponseWriter, r *http.Request) { ctx := r.Context() id := mux.Vars(r)["id"] - if _, err := h.resourceService.Get(ctx, h.descriptor.Kind, id); err != nil { - return nil, err + if svcErr := h.verifyResource(r, id); svcErr != nil { + return nil, svcErr } return h.processStatus(ctx, id, &req) @@ -87,61 +89,24 @@ func (h *ResourceStatusHandler) Create(w http.ResponseWriter, r *http.Request) { handleCreateWithNoContent(w, r, cfg) } -// ListByOwner returns adapter statuses for a child resource, validating ownership. -func (h *ResourceStatusHandler) ListByOwner(w http.ResponseWriter, r *http.Request) { - cfg := &handlerConfig{ - Action: func() (interface{}, *errors.ServiceError) { - ctx := r.Context() - vars := mux.Vars(r) - parentID, id := vars["parent_id"], vars["id"] - listArgs, err := services.NewListArguments(r.URL.Query()) - if err != nil { - return nil, err - } - - if _, err = h.resourceService.GetByOwner(ctx, h.descriptor.Kind, id, parentID); err != nil { - return nil, err - } - - return h.listStatuses(ctx, id, listArgs) - }, - ErrorHandler: handleError, - } - - handleList(w, r, cfg) -} - -// CreateByOwner creates or updates an adapter status for a child resource, validating ownership. -func (h *ResourceStatusHandler) CreateByOwner(w http.ResponseWriter, r *http.Request) { - var req openapi.AdapterStatusCreateRequest - - cfg := &handlerConfig{ - MarshalInto: &req, - Validate: []validate{ - validateNotEmpty(&req, "Adapter", "adapter"), - validateObservedGeneration(&req), - validateConditions(&req, "Conditions"), - validateObservedTimeRange(&req.ObservedTime), - }, - Action: func() (interface{}, *errors.ServiceError) { - ctx := r.Context() - vars := mux.Vars(r) - parentID, id := vars["parent_id"], vars["id"] - - if _, err := h.resourceService.GetByOwner(ctx, h.descriptor.Kind, id, parentID); err != nil { - return nil, err - } - - return h.processStatus(ctx, id, &req) - }, - ErrorHandler: handleError, +// verifyResource confirms the resource exists. For nested routes (parent_id +// present), also verifies the resource belongs to the parent. No-op ownership +// check for flat routes. +func (h *ResourceStatusHandler) verifyResource(r *http.Request, id string) *errors.ServiceError { + ctx := r.Context() + if parentID, hasParent := mux.Vars(r)["parent_id"]; hasParent { + if _, err := h.resourceService.GetByOwner(ctx, h.descriptor.Kind, id, parentID); err != nil { + return err + } + } else { + if _, err := h.resourceService.Get(ctx, h.descriptor.Kind, id); err != nil { + return err + } } - - handleCreateWithNoContent(w, r, cfg) + return nil } // listStatuses fetches paginated adapter statuses and presents them as an OpenAPI response. -// Shared by List and ListByOwner — the caller handles resource existence/ownership checks. func (h *ResourceStatusHandler) listStatuses( ctx context.Context, resourceID string, listArgs *services.ListArguments, ) (interface{}, *errors.ServiceError) { diff --git a/pkg/handlers/resource_status_handler_test.go b/pkg/handlers/resource_status_handler_test.go index a22861e2..7973fd1b 100644 --- a/pkg/handlers/resource_status_handler_test.go +++ b/pkg/handlers/resource_status_handler_test.go @@ -226,6 +226,111 @@ func TestResourceStatusHandler_Create_ResourceNotFound(t *testing.T) { Expect(w.Code).To(Equal(http.StatusNotFound)) } +// ─── Nested (parent_id) branch tests ────────────────────────────── + +func newTestVersionStatusHandler( + ctrl *gomock.Controller, +) (*ResourceStatusHandler, *services.MockResourceService, *services.MockAdapterStatusService) { + mockResourceSvc := services.NewMockResourceService(ctrl) + mockAdapterSvc := services.NewMockAdapterStatusService(ctrl) + handler := NewResourceStatusHandler(versionDescriptor, mockResourceSvc, mockAdapterSvc) + return handler, mockResourceSvc, mockAdapterSvc +} + +func TestResourceStatusHandler_List_NestedWithParentID(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, mockAdapterSvc := newTestVersionStatusHandler(ctrl) + + parentID := "ch-parent" + versionID := "ver-1" + + resource := &api.Resource{Kind: "Version"} + resource.ID = versionID + mockResourceSvc.EXPECT().GetByOwner(gomock.Any(), "Version", versionID, parentID).Return(resource, nil) + + mockAdapterSvc.EXPECT().FindByResourcePaginated( + gomock.Any(), "Version", versionID, gomock.Any(), + ).Return(api.AdapterStatusList{}, int64(0), nil) + + r := httptest.NewRequest(http.MethodGet, "/channels/"+parentID+"/versions/"+versionID+"/statuses", nil) + r = mux.SetURLVars(r, map[string]string{"parent_id": parentID, "id": versionID}) + w := httptest.NewRecorder() + + handler.List(w, r) + + Expect(w.Code).To(Equal(http.StatusOK)) +} + +func TestResourceStatusHandler_Create_NestedWithParentID(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, _ := newTestVersionStatusHandler(ctrl) + + parentID := "ch-parent" + versionID := "ver-1" + + resource := &api.Resource{Kind: "Version"} + resource.ID = versionID + mockResourceSvc.EXPECT().GetByOwner(gomock.Any(), "Version", versionID, parentID).Return(resource, nil) + + now := time.Now().UTC() + returnedStatus := &api.AdapterStatus{ + Adapter: "adapter1", + ResourceType: "Version", + ResourceID: versionID, + ObservedGeneration: 1, + LastReportTime: now, + Conditions: datatypes.JSON( //nolint:lll + `[{"type":"Available","status":"True"},{"type":"Applied","status":"True"},{"type":"Health","status":"True"}]`), + } + mockResourceSvc.EXPECT().ProcessAdapterStatus( + gomock.Any(), "Version", versionID, gomock.Any(), + ).Return(returnedStatus, nil) + + body := openapi.AdapterStatusCreateRequest{ + Adapter: "adapter1", + ObservedGeneration: 1, + ObservedTime: now, + Conditions: []openapi.ConditionRequest{ + {Type: "Available", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Applied", Status: openapi.AdapterConditionStatusTrue}, + {Type: "Health", Status: openapi.AdapterConditionStatusTrue}, + }, + } + bodyJSON, _ := json.Marshal(body) + + r := httptest.NewRequest(http.MethodPut, "/channels/"+parentID+"/versions/"+versionID+"/statuses", + strings.NewReader(string(bodyJSON))) + r.Header.Set("Content-Type", "application/json") + r = mux.SetURLVars(r, map[string]string{"parent_id": parentID, "id": versionID}) + w := httptest.NewRecorder() + + handler.Create(w, r) + + Expect(w.Code).To(Equal(http.StatusCreated)) +} + +func TestResourceStatusHandler_List_NestedWrongParent_Returns404(t *testing.T) { + RegisterTestingT(t) + ctrl := gomock.NewController(t) + + handler, mockResourceSvc, _ := newTestVersionStatusHandler(ctrl) + + mockResourceSvc.EXPECT().GetByOwner(gomock.Any(), "Version", "ver-1", "wrong-parent"). + Return(nil, errors.NotFound("Version 'ver-1' not found under parent 'wrong-parent'")) + + r := httptest.NewRequest(http.MethodGet, "/channels/wrong-parent/versions/ver-1/statuses", nil) + r = mux.SetURLVars(r, map[string]string{"parent_id": "wrong-parent", "id": "ver-1"}) + w := httptest.NewRecorder() + + handler.List(w, r) + + Expect(w.Code).To(Equal(http.StatusNotFound)) +} + // ─── RootResourceHandler status tests ────────────────────────────── func newTestRootResourceHandler(