diff --git a/pkg/api/status_types.go b/pkg/api/status_types.go index c0d8b67..b1c87e5 100644 --- a/pkg/api/status_types.go +++ b/pkg/api/status_types.go @@ -21,6 +21,14 @@ const ( AdapterConditionUnknown AdapterConditionStatus = "Unknown" ) +// Condition type constants +const ( + ConditionTypeAvailable = "Available" + ConditionTypeApplied = "Applied" + ConditionTypeHealth = "Health" + ConditionTypeReady = "Ready" +) + // ResourceCondition represents a condition of a resource // Domain equivalent of openapi.ResourceCondition // JSON tags match database JSONB structure diff --git a/pkg/services/cluster.go b/pkg/services/cluster.go index 9a5fd5c..40e2820 100644 --- a/pkg/services/cluster.go +++ b/pkg/services/cluster.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" stderrors "errors" + "fmt" "strings" "time" @@ -237,9 +238,10 @@ func (s *sqlClusterService) UpdateClusterStatusFromAdapters( } // ProcessAdapterStatus handles the business logic for adapter status: -// - If Available is "Unknown" and a status already exists: returns (nil, nil) as no-op -// - If Available is "Unknown" and no status exists (first report): upserts the status -// - Otherwise: upserts the status and triggers aggregation +// - Validates that all mandatory conditions (Available, Applied, Health) are present +// - Rejects updates where any mandatory condition is missing or has status "Unknown" +// - Uses complete replacement semantics: each update replaces all conditions for this adapter +// - Returns (nil, nil) for discarded updates, which results in HTTP 204 No Content func (s *sqlClusterService) ProcessAdapterStatus( ctx context.Context, clusterID string, adapterStatus *api.AdapterStatus, ) (*api.AdapterStatus, *errors.ServiceError) { @@ -264,42 +266,33 @@ func (s *sqlClusterService) ProcessAdapterStatus( } } - // Find the "Available" condition - hasAvailableCondition := false - for _, cond := range conditions { - if cond.Type != "Available" { - continue - } - - hasAvailableCondition = true - if cond.Status == api.AdapterConditionUnknown { - if existingStatus != nil { - // Available condition is "Unknown" and a status already exists, return nil to indicate no-op - return nil, nil - } - // First report from this adapter: allow storing even with Available=Unknown - // but skip aggregation since Unknown should not affect cluster-level conditions - hasAvailableCondition = false - } + // Validate that all mandatory conditions are present and not Unknown + if missingCondition, unknownCondition := ValidateMandatoryConditions(conditions); missingCondition != "" { + // Missing mandatory condition - discard the update + ctx = logger.WithClusterID(ctx, clusterID) + logger.Info(ctx, fmt.Sprintf("Discarding adapter status update from %s: missing mandatory condition %s", + adapterStatus.Adapter, missingCondition)) + return nil, nil + } else if unknownCondition != "" { + // Mandatory condition has Unknown status - discard the update + ctx = logger.WithClusterID(ctx, clusterID) + logger.Info(ctx, fmt.Sprintf("Discarding adapter status update from %s: mandatory condition %s has Unknown status", + adapterStatus.Adapter, unknownCondition)) + return nil, nil } - // Upsert the adapter status + // All validations passed - upsert the adapter status (complete replacement) upsertedStatus, err := s.adapterStatusDao.Upsert(ctx, adapterStatus) if err != nil { return nil, handleCreateError("AdapterStatus", err) } - // Only trigger aggregation when the adapter reported an Available condition. - // If the adapter status doesn't include Available (e.g. it only reports Ready/Progressing), - // saving it should not overwrite the cluster's synthetic Available/Ready conditions. - if hasAvailableCondition { - if _, aggregateErr := s.UpdateClusterStatusFromAdapters( - ctx, clusterID, - ); aggregateErr != nil { - // Log error but don't fail the request - the status will be computed on next update - ctx = logger.WithClusterID(ctx, clusterID) - logger.WithError(ctx, aggregateErr).Warn("Failed to aggregate cluster status") - } + // Trigger aggregation to update cluster-level conditions + if _, aggregateErr := s.UpdateClusterStatusFromAdapters( + ctx, clusterID, + ); aggregateErr != nil { + ctx = logger.WithClusterID(ctx, clusterID) + logger.WithError(ctx, aggregateErr).Warn("Failed to aggregate cluster status") } return upsertedStatus, nil diff --git a/pkg/services/cluster_test.go b/pkg/services/cluster_test.go index a66cfb4..59b01df 100644 --- a/pkg/services/cluster_test.go +++ b/pkg/services/cluster_test.go @@ -163,47 +163,50 @@ func (d *mockAdapterStatusDao) All(ctx context.Context) (api.AdapterStatusList, var _ dao.AdapterStatusDao = &mockAdapterStatusDao{} -// TestProcessAdapterStatus_FirstUnknownCondition tests that the first Unknown Available condition is stored -func TestProcessAdapterStatus_FirstUnknownCondition(t *testing.T) { +// TestProcessAdapterStatus_FirstUnknownCondition tests that updates with Unknown mandatory conditions are rejected +func TestProcessAdapterStatus_MandatoryConditionUnknown(t *testing.T) { RegisterTestingT(t) clusterDao := newMockClusterDao() adapterStatusDao := newMockAdapterStatusDao() - config := testAdapterConfig() service := NewClusterService(clusterDao, adapterStatusDao, config) ctx := context.Background() clusterID := testClusterID - // Create adapter status with Available=Unknown + // Create cluster first + cluster := &api.Cluster{Generation: 1} + cluster.ID = clusterID + _, svcErr := service.Create(ctx, cluster) + Expect(svcErr).To(BeNil()) + + // Send status with Available=Unknown conditions := []api.AdapterCondition{ - { - Type: conditionTypeAvailable, - Status: api.AdapterConditionUnknown, - LastTransitionTime: time.Now(), - }, + {Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, } conditionsJSON, _ := json.Marshal(conditions) - now := time.Now() adapterStatus := &api.AdapterStatus{ - ResourceType: "Cluster", - ResourceID: clusterID, - Adapter: "test-adapter", - Conditions: conditionsJSON, - CreatedTime: &now, + ResourceType: "Cluster", + ResourceID: clusterID, + Adapter: "test-adapter", + Conditions: conditionsJSON, + ObservedGeneration: 1, + CreatedTime: &now, } result, err := service.ProcessAdapterStatus(ctx, clusterID, adapterStatus) + // Should be rejected Expect(err).To(BeNil()) - Expect(result).ToNot(BeNil(), "First report with Available=Unknown should be stored") - Expect(result.Adapter).To(Equal("test-adapter")) + Expect(result).To(BeNil(), "Update with Available=Unknown should be rejected") - // Verify the status was stored + // Verify no status was stored storedStatuses, _ := adapterStatusDao.FindByResource(ctx, "Cluster", clusterID) - Expect(len(storedStatuses)).To(Equal(1), "First Unknown status should be stored") + Expect(len(storedStatuses)).To(Equal(0), "No status should be stored") } // TestProcessAdapterStatus_SubsequentUnknownCondition tests that subsequent Unknown Available conditions are discarded @@ -222,7 +225,7 @@ func TestProcessAdapterStatus_SubsequentUnknownCondition(t *testing.T) { // Pre-populate an existing adapter status to simulate a previously stored report conditions := []api.AdapterCondition{ { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now(), }, @@ -275,10 +278,20 @@ func TestProcessAdapterStatus_TrueCondition(t *testing.T) { _, svcErr := service.Create(ctx, cluster) Expect(svcErr).To(BeNil()) - // Create adapter status with Available=True + // Create adapter status with all mandatory conditions conditions := []api.AdapterCondition{ { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, + Status: api.AdapterConditionTrue, + LastTransitionTime: time.Now(), + }, + { + Type: api.ConditionTypeApplied, + Status: api.AdapterConditionTrue, + LastTransitionTime: time.Now(), + }, + { + Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now(), }, @@ -326,10 +339,20 @@ func TestProcessAdapterStatus_FalseCondition(t *testing.T) { _, svcErr := service.Create(ctx, cluster) Expect(svcErr).To(BeNil()) - // Create adapter status with Available=False + // Create adapter status with all mandatory conditions conditions := []api.AdapterCondition{ { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, + Status: api.AdapterConditionFalse, + LastTransitionTime: time.Now(), + }, + { + Type: api.ConditionTypeApplied, + Status: api.AdapterConditionTrue, + LastTransitionTime: time.Now(), + }, + { + Type: api.ConditionTypeHealth, Status: api.AdapterConditionFalse, LastTransitionTime: time.Now(), }, @@ -355,7 +378,7 @@ func TestProcessAdapterStatus_FalseCondition(t *testing.T) { Expect(len(storedStatuses)).To(Equal(1), "Status should be stored for False condition") } -// TestProcessAdapterStatus_NoAvailableCondition tests when there's no Available condition +// TestProcessAdapterStatus_NoAvailableCondition tests that updates missing mandatory conditions are rejected func TestProcessAdapterStatus_NoAvailableCondition(t *testing.T) { RegisterTestingT(t) @@ -369,73 +392,74 @@ func TestProcessAdapterStatus_NoAvailableCondition(t *testing.T) { clusterID := testClusterID // Create the cluster first - fixedNow := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) - initialConditions := []api.ResourceCondition{ + cluster := &api.Cluster{Generation: 1} + cluster.ID = clusterID + _, svcErr := service.Create(ctx, cluster) + Expect(svcErr).To(BeNil()) + + // First, send a valid complete status + validConditions := []api.AdapterCondition{ { - Type: conditionTypeAvailable, - Status: api.ConditionFalse, - ObservedGeneration: 1, - LastTransitionTime: fixedNow, - CreatedTime: fixedNow, - LastUpdatedTime: fixedNow, + Type: api.ConditionTypeAvailable, + Status: api.AdapterConditionTrue, + LastTransitionTime: time.Now(), }, { - Type: "Ready", - Status: api.ConditionFalse, - ObservedGeneration: 7, - LastTransitionTime: fixedNow, - CreatedTime: fixedNow, - LastUpdatedTime: fixedNow, + Type: api.ConditionTypeApplied, + Status: api.AdapterConditionTrue, + LastTransitionTime: time.Now(), + }, + { + Type: api.ConditionTypeHealth, + Status: api.AdapterConditionTrue, + LastTransitionTime: time.Now(), }, } - initialConditionsJSON, _ := json.Marshal(initialConditions) - - cluster := &api.Cluster{ - Generation: 7, - StatusConditions: initialConditionsJSON, + validConditionsJSON, _ := json.Marshal(validConditions) + now := time.Now() + validStatus := &api.AdapterStatus{ + ResourceType: "Cluster", + ResourceID: clusterID, + Adapter: "test-adapter", + Conditions: validConditionsJSON, + ObservedGeneration: 1, + CreatedTime: &now, } - cluster.ID = clusterID - _, svcErr := service.Create(ctx, cluster) - Expect(svcErr).To(BeNil()) - initialClusterStatusConditions := api.Cluster{}.StatusConditions - initialClusterStatusConditions = append(initialClusterStatusConditions, cluster.StatusConditions...) + result, err := service.ProcessAdapterStatus(ctx, clusterID, validStatus) + Expect(err).To(BeNil()) + Expect(result).ToNot(BeNil()) - // Create adapter status with Health condition (no Available) - conditions := []api.AdapterCondition{ + // Now try to send an update with only Health condition (missing Available and Applied) + incompleteConditions := []api.AdapterCondition{ { - Type: "Health", - Status: api.AdapterConditionTrue, + Type: api.ConditionTypeHealth, + Status: api.AdapterConditionFalse, LastTransitionTime: time.Now(), }, } - conditionsJSON, _ := json.Marshal(conditions) - - now := time.Now() - adapterStatus := &api.AdapterStatus{ - ResourceType: "Cluster", - ResourceID: clusterID, - Adapter: "test-adapter", - Conditions: conditionsJSON, - CreatedTime: &now, + incompleteConditionsJSON, _ := json.Marshal(incompleteConditions) + incompleteStatus := &api.AdapterStatus{ + ResourceType: "Cluster", + ResourceID: clusterID, + Adapter: "test-adapter", + Conditions: incompleteConditionsJSON, + ObservedGeneration: 2, + CreatedTime: &now, } - result, err := service.ProcessAdapterStatus(ctx, clusterID, adapterStatus) + result, err = service.ProcessAdapterStatus(ctx, clusterID, incompleteStatus) Expect(err).To(BeNil()) - Expect(result).ToNot(BeNil(), "ProcessAdapterStatus should proceed when no Available condition") + Expect(result).To(BeNil(), "Update missing Available condition should be rejected") - // Verify the status was stored - storedStatuses, _ := adapterStatusDao.FindByResource(ctx, "Cluster", clusterID) - Expect(len(storedStatuses)).To(Equal(1), "Status should be stored when no Available condition") - - // Verify that saving a non-Available condition did not overwrite cluster Available/Ready - storedCluster, _ := clusterDao.Get(ctx, clusterID) - Expect(storedCluster.StatusConditions).To(Equal(initialClusterStatusConditions), - "Cluster status conditions should not be overwritten when adapter status lacks Available") + // Verify the original valid status is preserved + storedStatus, _ := adapterStatusDao.FindByResourceAndAdapter(ctx, "Cluster", clusterID, "test-adapter") + Expect(storedStatus).ToNot(BeNil()) + Expect(storedStatus.ObservedGeneration).To(Equal(int32(1)), "Original status should be preserved") } -// TestProcessAdapterStatus_FirstMultipleConditions_AvailableUnknown tests that the first report with -// multiple conditions including Available=Unknown is stored +// TestProcessAdapterStatus_FirstMultipleConditions_AvailableUnknown tests that reports with +// Available=Unknown are rejected even when other conditions are present func TestProcessAdapterStatus_FirstMultipleConditions_AvailableUnknown(t *testing.T) { RegisterTestingT(t) @@ -448,20 +472,31 @@ func TestProcessAdapterStatus_FirstMultipleConditions_AvailableUnknown(t *testin ctx := context.Background() clusterID := testClusterID - // Create adapter status with multiple conditions including Available=Unknown + // Create cluster first + cluster := &api.Cluster{Generation: 1} + cluster.ID = clusterID + _, svcErr := service.Create(ctx, cluster) + Expect(svcErr).To(BeNil()) + + // Create adapter status with all mandatory conditions but Available=Unknown conditions := []api.AdapterCondition{ { - Type: "Ready", + Type: api.ConditionTypeAvailable, + Status: api.AdapterConditionUnknown, + LastTransitionTime: time.Now(), + }, + { + Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now(), }, { - Type: conditionTypeAvailable, - Status: api.AdapterConditionUnknown, + Type: api.ConditionTypeHealth, + Status: api.AdapterConditionTrue, LastTransitionTime: time.Now(), }, { - Type: "Progressing", + Type: "Ready", Status: api.AdapterConditionTrue, LastTransitionTime: time.Now(), }, @@ -480,11 +515,11 @@ func TestProcessAdapterStatus_FirstMultipleConditions_AvailableUnknown(t *testin result, err := service.ProcessAdapterStatus(ctx, clusterID, adapterStatus) Expect(err).To(BeNil()) - Expect(result).ToNot(BeNil(), "First report with Available=Unknown should be stored") + Expect(result).To(BeNil(), "Report with Available=Unknown should be rejected") - // Verify the status was stored + // Verify no status was stored storedStatuses, _ := adapterStatusDao.FindByResource(ctx, "Cluster", clusterID) - Expect(len(storedStatuses)).To(Equal(1), "First Unknown status should be stored") + Expect(len(storedStatuses)).To(Equal(0), "No status should be stored when Available=Unknown") } // TestProcessAdapterStatus_SubsequentMultipleConditions_AvailableUnknown tests that subsequent reports @@ -504,7 +539,7 @@ func TestProcessAdapterStatus_SubsequentMultipleConditions_AvailableUnknown(t *t // Pre-populate an existing adapter status existingConditions := []api.AdapterCondition{ { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now(), }, @@ -529,7 +564,7 @@ func TestProcessAdapterStatus_SubsequentMultipleConditions_AvailableUnknown(t *t LastTransitionTime: time.Now(), }, { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now(), }, @@ -585,9 +620,9 @@ func TestClusterAvailableReadyTransitions(t *testing.T) { var available, ready *api.ResourceCondition for i := range conds { switch conds[i].Type { - case conditionTypeAvailable: + case api.ConditionTypeAvailable: available = &conds[i] - case conditionTypeReady: + case api.ConditionTypeReady: ready = &conds[i] } } @@ -598,7 +633,9 @@ func TestClusterAvailableReadyTransitions(t *testing.T) { upsert := func(adapter string, available api.AdapterConditionStatus, observedGen int32) { conditions := []api.AdapterCondition{ - {Type: conditionTypeAvailable, Status: available, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeAvailable, Status: available, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, } conditionsJSON, _ := json.Marshal(conditions) now := time.Now() @@ -689,7 +726,7 @@ func TestClusterAvailableReadyTransitions(t *testing.T) { prevStatus := api.Cluster{}.StatusConditions prevStatus = append(prevStatus, clusterDao.clusters[clusterID].StatusConditions...) unknownConds := []api.AdapterCondition{ - {Type: conditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now()}, } unknownJSON, _ := json.Marshal(unknownConds) unknownStatus := &api.AdapterStatus{ @@ -730,7 +767,7 @@ func TestClusterStaleAdapterStatusUpdatePolicy(t *testing.T) { var conds []api.ResourceCondition Expect(json.Unmarshal(stored.StatusConditions, &conds)).To(Succeed()) for i := range conds { - if conds[i].Type == conditionTypeAvailable { + if conds[i].Type == api.ConditionTypeAvailable { return conds[i] } } @@ -740,7 +777,9 @@ func TestClusterStaleAdapterStatusUpdatePolicy(t *testing.T) { upsert := func(adapter string, available api.AdapterConditionStatus, observedGen int32) { conditions := []api.AdapterCondition{ - {Type: conditionTypeAvailable, Status: available, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeAvailable, Status: available, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, } conditionsJSON, _ := json.Marshal(conditions) now := time.Now() @@ -796,7 +835,7 @@ func TestClusterSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) { fixedNow := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) initialConditions := []api.ResourceCondition{ { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, Status: api.ConditionFalse, ObservedGeneration: 1, LastTransitionTime: fixedNow, @@ -829,9 +868,9 @@ func TestClusterSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) { var createdAvailable, createdReady *api.ResourceCondition for i := range createdConds { switch createdConds[i].Type { - case conditionTypeAvailable: + case api.ConditionTypeAvailable: createdAvailable = &createdConds[i] - case conditionTypeReady: + case api.ConditionTypeReady: createdReady = &createdConds[i] } } @@ -854,9 +893,9 @@ func TestClusterSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) { var updatedAvailable, updatedReady *api.ResourceCondition for i := range updatedConds { switch updatedConds[i].Type { - case conditionTypeAvailable: + case api.ConditionTypeAvailable: updatedAvailable = &updatedConds[i] - case conditionTypeReady: + case api.ConditionTypeReady: updatedReady = &updatedConds[i] } } @@ -869,3 +908,222 @@ func TestClusterSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) { Expect(updatedReady.LastTransitionTime).To(Equal(fixedNow)) Expect(updatedReady.LastUpdatedTime).To(Equal(fixedNow)) } + +// TestProcessAdapterStatus_MissingMandatoryCondition_Available tests that updates missing Available are rejected +func TestProcessAdapterStatus_MissingMandatoryCondition_Available(t *testing.T) { + RegisterTestingT(t) + + clusterDao := newMockClusterDao() + adapterStatusDao := newMockAdapterStatusDao() + config := testAdapterConfig() + service := NewClusterService(clusterDao, adapterStatusDao, config) + + ctx := context.Background() + clusterID := testClusterID + + // Create cluster first + cluster := &api.Cluster{Generation: 1} + cluster.ID = clusterID + _, svcErr := service.Create(ctx, cluster) + Expect(svcErr).To(BeNil()) + + // First, send a valid status + validConditions := []api.AdapterCondition{ + {Type: "Available", Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + } + validConditionsJSON, _ := json.Marshal(validConditions) + now := time.Now() + validStatus := &api.AdapterStatus{ + ResourceType: "Cluster", + ResourceID: clusterID, + Adapter: "test-adapter", + Conditions: validConditionsJSON, + ObservedGeneration: 1, + CreatedTime: &now, + } + result, err := service.ProcessAdapterStatus(ctx, clusterID, validStatus) + Expect(err).To(BeNil()) + Expect(result).ToNot(BeNil()) + + // Now send an update without Available condition + invalidConditions := []api.AdapterCondition{ + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: "CustomCondition", Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + } + invalidConditionsJSON, _ := json.Marshal(invalidConditions) + invalidStatus := &api.AdapterStatus{ + ResourceType: "Cluster", + ResourceID: clusterID, + Adapter: "test-adapter", + Conditions: invalidConditionsJSON, + ObservedGeneration: 2, + CreatedTime: &now, + } + + result, err = service.ProcessAdapterStatus(ctx, clusterID, invalidStatus) + + // Should be rejected (nil, nil) + Expect(err).To(BeNil()) + Expect(result).To(BeNil(), "Update missing Available condition should be rejected") + + // Verify old status is preserved + storedStatus, _ := adapterStatusDao.FindByResourceAndAdapter(ctx, "Cluster", clusterID, "test-adapter") + Expect(storedStatus).ToNot(BeNil()) + Expect(storedStatus.ObservedGeneration).To(Equal(int32(1)), "Old status should be preserved") + + var storedConditions []api.AdapterCondition + unmarshalErr := json.Unmarshal(storedStatus.Conditions, &storedConditions) + Expect(unmarshalErr).To(BeNil()) + Expect(len(storedConditions)).To(Equal(3)) + // Verify Available is still there + hasAvailable := false + for _, cond := range storedConditions { + if cond.Type == "Available" { + hasAvailable = true + break + } + } + Expect(hasAvailable).To(BeTrue(), "Available condition should be preserved") +} + +// TestProcessAdapterStatus_AllMandatoryConditions_WithCustom tests that valid +// updates with all mandatory conditions are accepted +func TestProcessAdapterStatus_AllMandatoryConditions_WithCustom(t *testing.T) { + RegisterTestingT(t) + + clusterDao := newMockClusterDao() + adapterStatusDao := newMockAdapterStatusDao() + config := testAdapterConfig() + service := NewClusterService(clusterDao, adapterStatusDao, config) + + ctx := context.Background() + clusterID := testClusterID + + // Create cluster first + cluster := &api.Cluster{Generation: 1} + cluster.ID = clusterID + _, svcErr := service.Create(ctx, cluster) + Expect(svcErr).To(BeNil()) + + // Send status with all mandatory conditions + custom condition + conditions := []api.AdapterCondition{ + {Type: "Available", Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: "CustomCondition", Status: api.AdapterConditionFalse, LastTransitionTime: time.Now()}, + } + conditionsJSON, _ := json.Marshal(conditions) + now := time.Now() + adapterStatus := &api.AdapterStatus{ + ResourceType: "Cluster", + ResourceID: clusterID, + Adapter: "test-adapter", + Conditions: conditionsJSON, + ObservedGeneration: 1, + CreatedTime: &now, + } + + result, err := service.ProcessAdapterStatus(ctx, clusterID, adapterStatus) + + // Should be accepted + Expect(err).To(BeNil()) + Expect(result).ToNot(BeNil(), "Update with all mandatory conditions should be accepted") + + // Verify status was stored with all 4 conditions + var storedConditions []api.AdapterCondition + unmarshalErr := json.Unmarshal(result.Conditions, &storedConditions) + Expect(unmarshalErr).To(BeNil()) + Expect(len(storedConditions)).To(Equal(4), "All 4 conditions should be stored") + + // Verify all conditions are present + conditionTypes := make(map[string]bool) + for _, cond := range storedConditions { + conditionTypes[cond.Type] = true + } + Expect(conditionTypes[api.ConditionTypeAvailable]).To(BeTrue()) + Expect(conditionTypes[api.ConditionTypeApplied]).To(BeTrue()) + Expect(conditionTypes[api.ConditionTypeHealth]).To(BeTrue()) + Expect(conditionTypes["CustomCondition"]).To(BeTrue()) +} + +// TestProcessAdapterStatus_CustomConditionRemoval tests that custom conditions can be removed +func TestProcessAdapterStatus_CustomConditionRemoval(t *testing.T) { + RegisterTestingT(t) + + clusterDao := newMockClusterDao() + adapterStatusDao := newMockAdapterStatusDao() + config := testAdapterConfig() + service := NewClusterService(clusterDao, adapterStatusDao, config) + + ctx := context.Background() + clusterID := testClusterID + + // Create cluster first + cluster := &api.Cluster{Generation: 1} + cluster.ID = clusterID + _, svcErr := service.Create(ctx, cluster) + Expect(svcErr).To(BeNil()) + + // First update: send all mandatory + custom condition + conditions1 := []api.AdapterCondition{ + {Type: "Available", Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: "CustomCondition", Status: api.AdapterConditionFalse, LastTransitionTime: time.Now()}, + } + conditionsJSON1, _ := json.Marshal(conditions1) + now := time.Now() + adapterStatus1 := &api.AdapterStatus{ + ResourceType: "Cluster", + ResourceID: clusterID, + Adapter: "test-adapter", + Conditions: conditionsJSON1, + ObservedGeneration: 1, + CreatedTime: &now, + } + result1, err := service.ProcessAdapterStatus(ctx, clusterID, adapterStatus1) + Expect(err).To(BeNil()) + Expect(result1).ToNot(BeNil()) + + var storedConditions1 []api.AdapterCondition + unmarshalErr := json.Unmarshal(result1.Conditions, &storedConditions1) + Expect(unmarshalErr).To(BeNil()) + Expect(len(storedConditions1)).To(Equal(4)) + + // Second update: remove custom condition (only send mandatory conditions) + conditions2 := []api.AdapterCondition{ + {Type: "Available", Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionFalse, LastTransitionTime: time.Now()}, + } + conditionsJSON2, _ := json.Marshal(conditions2) + adapterStatus2 := &api.AdapterStatus{ + ResourceType: "Cluster", + ResourceID: clusterID, + Adapter: "test-adapter", + Conditions: conditionsJSON2, + ObservedGeneration: 2, + CreatedTime: &now, + } + result2, err := service.ProcessAdapterStatus(ctx, clusterID, adapterStatus2) + Expect(err).To(BeNil()) + Expect(result2).ToNot(BeNil()) + + // Verify CustomCondition was removed + var storedConditions2 []api.AdapterCondition + unmarshalErr = json.Unmarshal(result2.Conditions, &storedConditions2) + Expect(unmarshalErr).To(BeNil()) + Expect(len(storedConditions2)).To(Equal(3), "CustomCondition should be removed") + + conditionTypes := make(map[string]bool) + for _, cond := range storedConditions2 { + conditionTypes[cond.Type] = true + } + Expect(conditionTypes[api.ConditionTypeAvailable]).To(BeTrue()) + Expect(conditionTypes[api.ConditionTypeApplied]).To(BeTrue()) + Expect(conditionTypes[api.ConditionTypeHealth]).To(BeTrue()) + Expect(conditionTypes["CustomCondition"]).To(BeFalse(), "CustomCondition should not be present") +} diff --git a/pkg/services/node_pool.go b/pkg/services/node_pool.go index c341de4..43025d1 100644 --- a/pkg/services/node_pool.go +++ b/pkg/services/node_pool.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" stderrors "errors" + "fmt" "strings" "time" @@ -173,7 +174,7 @@ func (s *sqlNodePoolService) UpdateNodePoolStatusFromAdapters( // Find the "Available" condition var availableCondition *api.AdapterCondition for i := range conditions { - if conditions[i].Type == conditionTypeAvailable { + if conditions[i].Type == api.ConditionTypeAvailable { availableCondition = &conditions[i] break } @@ -265,42 +266,33 @@ func (s *sqlNodePoolService) ProcessAdapterStatus( } } - // Find the "Available" condition - hasAvailableCondition := false - for _, cond := range conditions { - if cond.Type != conditionTypeAvailable { - continue - } - - hasAvailableCondition = true - if cond.Status == api.AdapterConditionUnknown { - if existingStatus != nil { - // Available condition is "Unknown" and a status already exists, return nil to indicate no-op - return nil, nil - } - // First report from this adapter: allow storing even with Available=Unknown - // but skip aggregation since Unknown should not affect nodepool-level conditions - hasAvailableCondition = false - } + // Validate that all mandatory conditions are present and not Unknown + if missingCondition, unknownCondition := ValidateMandatoryConditions(conditions); missingCondition != "" { + // Missing mandatory condition - discard the update + logger.With(ctx, logger.FieldNodePoolID, nodePoolID). + Info(fmt.Sprintf("Discarding adapter status update from %s: missing mandatory condition %s", + adapterStatus.Adapter, missingCondition)) + return nil, nil + } else if unknownCondition != "" { + // Mandatory condition has Unknown status - discard the update + logger.With(ctx, logger.FieldNodePoolID, nodePoolID). + Info(fmt.Sprintf("Discarding adapter status update from %s: mandatory condition %s has Unknown status", + adapterStatus.Adapter, unknownCondition)) + return nil, nil } - // Upsert the adapter status + // All validations passed - upsert the adapter status (complete replacement) upsertedStatus, err := s.adapterStatusDao.Upsert(ctx, adapterStatus) if err != nil { return nil, handleCreateError("AdapterStatus", err) } - // Only trigger aggregation when the adapter reported an Available condition. - // If the adapter status doesn't include Available, saving it should not overwrite - // the nodepool's synthetic Available/Ready conditions. - if hasAvailableCondition { - if _, aggregateErr := s.UpdateNodePoolStatusFromAdapters( - ctx, nodePoolID, - ); aggregateErr != nil { - // Log error but don't fail the request - the status will be computed on next update - logger.With(ctx, logger.FieldNodePoolID, nodePoolID). - WithError(aggregateErr).Warn("Failed to aggregate nodepool status") - } + // Trigger aggregation to update nodepool-level conditions + if _, aggregateErr := s.UpdateNodePoolStatusFromAdapters( + ctx, nodePoolID, + ); aggregateErr != nil { + logger.With(ctx, logger.FieldNodePoolID, nodePoolID). + WithError(aggregateErr).Warn("Failed to aggregate nodepool status") } return upsertedStatus, nil diff --git a/pkg/services/node_pool_test.go b/pkg/services/node_pool_test.go index 9c5f39e..84c76ea 100644 --- a/pkg/services/node_pool_test.go +++ b/pkg/services/node_pool_test.go @@ -93,13 +93,23 @@ func TestNodePoolProcessAdapterStatus_FirstUnknownCondition(t *testing.T) { ctx := context.Background() nodePoolID := testNodePoolID - // Create adapter status with Available=Unknown + // Create adapter status with all mandatory conditions but Available=Unknown conditions := []api.AdapterCondition{ { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now(), }, + { + Type: api.ConditionTypeApplied, + Status: api.AdapterConditionTrue, + LastTransitionTime: time.Now(), + }, + { + Type: api.ConditionTypeHealth, + Status: api.AdapterConditionTrue, + LastTransitionTime: time.Now(), + }, } conditionsJSON, _ := json.Marshal(conditions) @@ -115,12 +125,11 @@ func TestNodePoolProcessAdapterStatus_FirstUnknownCondition(t *testing.T) { result, err := service.ProcessAdapterStatus(ctx, nodePoolID, adapterStatus) Expect(err).To(BeNil()) - Expect(result).ToNot(BeNil(), "First report with Available=Unknown should be stored") - Expect(result.Adapter).To(Equal("test-adapter")) + Expect(result).To(BeNil(), "Update with Available=Unknown should be rejected") - // Verify the status was stored + // Verify no status was stored storedStatuses, _ := adapterStatusDao.FindByResource(ctx, "NodePool", nodePoolID) - Expect(len(storedStatuses)).To(Equal(1), "First Unknown status should be stored") + Expect(len(storedStatuses)).To(Equal(0), "No status should be stored when mandatory condition is Unknown") } // TestNodePoolProcessAdapterStatus_SubsequentUnknownCondition tests that subsequent Unknown conditions are discarded @@ -139,7 +148,7 @@ func TestNodePoolProcessAdapterStatus_SubsequentUnknownCondition(t *testing.T) { // Pre-populate an existing adapter status conditions := []api.AdapterCondition{ { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now(), }, @@ -192,10 +201,20 @@ func TestNodePoolProcessAdapterStatus_TrueCondition(t *testing.T) { _, svcErr := service.Create(ctx, nodePool) Expect(svcErr).To(BeNil()) - // Create adapter status with Available=True + // Create adapter status with all mandatory conditions conditions := []api.AdapterCondition{ { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, + Status: api.AdapterConditionTrue, + LastTransitionTime: time.Now(), + }, + { + Type: api.ConditionTypeApplied, + Status: api.AdapterConditionTrue, + LastTransitionTime: time.Now(), + }, + { + Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now(), }, @@ -222,8 +241,8 @@ func TestNodePoolProcessAdapterStatus_TrueCondition(t *testing.T) { Expect(len(storedStatuses)).To(Equal(1), "Status should be stored for True condition") } -// TestNodePoolProcessAdapterStatus_FirstMultipleConditions_AvailableUnknown tests that the first report -// with multiple conditions including Available=Unknown is stored +// TestNodePoolProcessAdapterStatus_FirstMultipleConditions_AvailableUnknown tests that reports +// with Available=Unknown are rejected even when other conditions are present func TestNodePoolProcessAdapterStatus_FirstMultipleConditions_AvailableUnknown(t *testing.T) { RegisterTestingT(t) @@ -236,16 +255,26 @@ func TestNodePoolProcessAdapterStatus_FirstMultipleConditions_AvailableUnknown(t ctx := context.Background() nodePoolID := testNodePoolID - // Create adapter status with multiple conditions including Available=Unknown + // Create adapter status with all mandatory conditions but Available=Unknown conditions := []api.AdapterCondition{ { - Type: conditionTypeReady, + Type: api.ConditionTypeAvailable, + Status: api.AdapterConditionUnknown, + LastTransitionTime: time.Now(), + }, + { + Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now(), }, { - Type: conditionTypeAvailable, - Status: api.AdapterConditionUnknown, + Type: api.ConditionTypeHealth, + Status: api.AdapterConditionTrue, + LastTransitionTime: time.Now(), + }, + { + Type: api.ConditionTypeReady, + Status: api.AdapterConditionTrue, LastTransitionTime: time.Now(), }, } @@ -263,11 +292,11 @@ func TestNodePoolProcessAdapterStatus_FirstMultipleConditions_AvailableUnknown(t result, err := service.ProcessAdapterStatus(ctx, nodePoolID, adapterStatus) Expect(err).To(BeNil()) - Expect(result).ToNot(BeNil(), "First report with Available=Unknown should be stored") + Expect(result).To(BeNil(), "Report with Available=Unknown should be rejected") - // Verify the status was stored + // Verify no status was stored storedStatuses, _ := adapterStatusDao.FindByResource(ctx, "NodePool", nodePoolID) - Expect(len(storedStatuses)).To(Equal(1), "First Unknown status should be stored") + Expect(len(storedStatuses)).To(Equal(0), "No status should be stored when Available=Unknown") } // TestNodePoolProcessAdapterStatus_SubsequentMultipleConditions_AvailableUnknown tests that subsequent @@ -287,7 +316,7 @@ func TestNodePoolProcessAdapterStatus_SubsequentMultipleConditions_AvailableUnkn // Pre-populate an existing adapter status existingConditions := []api.AdapterCondition{ { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now(), }, @@ -307,12 +336,12 @@ func TestNodePoolProcessAdapterStatus_SubsequentMultipleConditions_AvailableUnkn // Now send another report with multiple conditions including Available=Unknown conditions := []api.AdapterCondition{ { - Type: conditionTypeReady, + Type: api.ConditionTypeReady, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now(), }, { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now(), }, @@ -362,9 +391,9 @@ func TestNodePoolAvailableReadyTransitions(t *testing.T) { var available, ready *api.ResourceCondition for i := range conds { switch conds[i].Type { - case conditionTypeAvailable: + case api.ConditionTypeAvailable: available = &conds[i] - case conditionTypeReady: + case api.ConditionTypeReady: ready = &conds[i] } } @@ -375,7 +404,9 @@ func TestNodePoolAvailableReadyTransitions(t *testing.T) { upsert := func(adapter string, available api.AdapterConditionStatus, observedGen int32) { conditions := []api.AdapterCondition{ - {Type: conditionTypeAvailable, Status: available, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeAvailable, Status: available, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, } conditionsJSON, _ := json.Marshal(conditions) now := time.Now() @@ -447,11 +478,11 @@ func TestNodePoolAvailableReadyTransitions(t *testing.T) { Expect(avail.ObservedGeneration).To(Equal(int32(0))) Expect(ready.Status).To(Equal(api.ConditionFalse)) - // Adapter status with no Available condition should not overwrite synthetic conditions. + // Adapter status missing mandatory conditions should be rejected and not overwrite synthetic conditions. prevStatus := api.NodePool{}.StatusConditions prevStatus = append(prevStatus, nodePoolDao.nodePools[nodePoolID].StatusConditions...) nonAvailableConds := []api.AdapterCondition{ - {Type: "Health", Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, } nonAvailableJSON, _ := json.Marshal(nonAvailableConds) nonAvailableStatus := &api.AdapterStatus{ @@ -463,14 +494,14 @@ func TestNodePoolAvailableReadyTransitions(t *testing.T) { } result, svcErr := service.ProcessAdapterStatus(ctx, nodePoolID, nonAvailableStatus) Expect(svcErr).To(BeNil()) - Expect(result).ToNot(BeNil()) + Expect(result).To(BeNil(), "Update missing mandatory conditions should be rejected") Expect(nodePoolDao.nodePools[nodePoolID].StatusConditions).To(Equal(prevStatus)) // Available=Unknown is a no-op (does not store, does not overwrite nodepool conditions). prevStatus = api.NodePool{}.StatusConditions prevStatus = append(prevStatus, nodePoolDao.nodePools[nodePoolID].StatusConditions...) unknownConds := []api.AdapterCondition{ - {Type: conditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now()}, } unknownJSON, _ := json.Marshal(unknownConds) unknownStatus := &api.AdapterStatus{ @@ -511,7 +542,7 @@ func TestNodePoolStaleAdapterStatusUpdatePolicy(t *testing.T) { var conds []api.ResourceCondition Expect(json.Unmarshal(stored.StatusConditions, &conds)).To(Succeed()) for i := range conds { - if conds[i].Type == conditionTypeAvailable { + if conds[i].Type == api.ConditionTypeAvailable { return conds[i] } } @@ -521,7 +552,9 @@ func TestNodePoolStaleAdapterStatusUpdatePolicy(t *testing.T) { upsert := func(adapter string, available api.AdapterConditionStatus, observedGen int32) { conditions := []api.AdapterCondition{ - {Type: conditionTypeAvailable, Status: available, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeAvailable, Status: available, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, } conditionsJSON, _ := json.Marshal(conditions) now := time.Now() @@ -577,7 +610,7 @@ func TestNodePoolSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) { fixedNow := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) initialConditions := []api.ResourceCondition{ { - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, Status: api.ConditionFalse, ObservedGeneration: 1, LastTransitionTime: fixedNow, @@ -585,7 +618,7 @@ func TestNodePoolSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) { LastUpdatedTime: fixedNow, }, { - Type: conditionTypeReady, + Type: api.ConditionTypeReady, Status: api.ConditionFalse, ObservedGeneration: 1, LastTransitionTime: fixedNow, @@ -610,9 +643,9 @@ func TestNodePoolSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) { var createdAvailable, createdReady *api.ResourceCondition for i := range createdConds { switch createdConds[i].Type { - case conditionTypeAvailable: + case api.ConditionTypeAvailable: createdAvailable = &createdConds[i] - case conditionTypeReady: + case api.ConditionTypeReady: createdReady = &createdConds[i] } } @@ -635,9 +668,9 @@ func TestNodePoolSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) { var updatedAvailable, updatedReady *api.ResourceCondition for i := range updatedConds { switch updatedConds[i].Type { - case conditionTypeAvailable: + case api.ConditionTypeAvailable: updatedAvailable = &updatedConds[i] - case conditionTypeReady: + case api.ConditionTypeReady: updatedReady = &updatedConds[i] } } diff --git a/pkg/services/status_aggregation.go b/pkg/services/status_aggregation.go index b1a155b..b352f2a 100644 --- a/pkg/services/status_aggregation.go +++ b/pkg/services/status_aggregation.go @@ -10,10 +10,8 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" ) -const ( - conditionTypeAvailable = "Available" - conditionTypeReady = "Ready" -) +// Mandatory condition types that must be present in all adapter status updates +var mandatoryConditionTypes = []string{api.ConditionTypeAvailable, api.ConditionTypeApplied, api.ConditionTypeHealth} // Required adapter lists configured via pkg/config/adapter.go (see AdapterRequirementsConfig) @@ -26,6 +24,45 @@ var adapterConditionSuffixMap = map[string]string{ // Add custom mappings here when needed } +// ValidateMandatoryConditions checks if all mandatory conditions are present and not Unknown. +// Returns (missingCondition, unknownCondition) where: +// - missingCondition: the name of the first missing mandatory condition, or empty string if all present +// - unknownCondition: the name of the first mandatory condition with Unknown status, or empty string if none +// +// Usage: +// +// if missing, unknown := ValidateMandatoryConditions(conditions); missing != "" { +// // Handle missing condition +// } else if unknown != "" { +// // Handle unknown condition +// } +func ValidateMandatoryConditions(conditions []api.AdapterCondition) (missingCondition, unknownCondition string) { + // Build a map of condition types for validation + // If duplicate condition types exist, preserve Unknown status (highest severity) + conditionMap := make(map[string]api.AdapterConditionStatus) + for _, cond := range conditions { + existing, exists := conditionMap[cond.Type] + // If already Unknown, keep it (don't let True/False overwrite Unknown) + if exists && existing == api.AdapterConditionUnknown { + continue + } + conditionMap[cond.Type] = cond.Status + } + + // Check that all mandatory conditions are present and not Unknown + for _, mandatoryType := range mandatoryConditionTypes { + status, exists := conditionMap[mandatoryType] + if !exists { + return mandatoryType, "" + } + if status == api.AdapterConditionUnknown { + return "", mandatoryType + } + } + + return "", "" +} + // MapAdapterToConditionType converts an adapter name to a semantic condition type // by converting the adapter name to PascalCase and appending a suffix. // @@ -87,7 +124,7 @@ func ComputeAvailableCondition(adapterStatuses api.AdapterStatusList, requiredAd if len(adapterStatus.Conditions) > 0 { if err := json.Unmarshal(adapterStatus.Conditions, &conditions); err == nil { for _, cond := range conditions { - if cond.Type == conditionTypeAvailable { + if cond.Type == api.ConditionTypeAvailable { adapterMap[adapterStatus.Adapter] = struct { available string observedGeneration int32 @@ -158,7 +195,7 @@ func ComputeReadyCondition( if len(adapterStatus.Conditions) > 0 { if err := json.Unmarshal(adapterStatus.Conditions, &conditions); err == nil { for _, cond := range conditions { - if cond.Type == conditionTypeAvailable { + if cond.Type == api.ConditionTypeAvailable { adapterMap[adapterStatus.Adapter] = struct { available string observedGeneration int32 @@ -216,9 +253,9 @@ func BuildSyntheticConditions( if err := json.Unmarshal(existingConditionsJSON, &existingConditions); err == nil { for i := range existingConditions { switch existingConditions[i].Type { - case conditionTypeAvailable: + case api.ConditionTypeAvailable: existingAvailable = &existingConditions[i] - case conditionTypeReady: + case api.ConditionTypeReady: existingReady = &existingConditions[i] } } @@ -231,7 +268,7 @@ func BuildSyntheticConditions( availableStatus = api.ConditionTrue } availableCondition := api.ResourceCondition{ - Type: conditionTypeAvailable, + Type: api.ConditionTypeAvailable, Status: availableStatus, ObservedGeneration: minObservedGeneration, LastTransitionTime: now, @@ -246,7 +283,7 @@ func BuildSyntheticConditions( readyStatus = api.ConditionTrue } readyCondition := api.ResourceCondition{ - Type: conditionTypeReady, + Type: api.ConditionTypeReady, Status: readyStatus, ObservedGeneration: resourceGeneration, LastTransitionTime: now, diff --git a/pkg/services/status_aggregation_test.go b/pkg/services/status_aggregation_test.go index 56eedc6..5b942a7 100644 --- a/pkg/services/status_aggregation_test.go +++ b/pkg/services/status_aggregation_test.go @@ -1,6 +1,11 @@ package services -import "testing" +import ( + "testing" + "time" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" +) func TestMapAdapterToConditionType(t *testing.T) { tests := []struct { @@ -51,3 +56,126 @@ func TestMapAdapterToConditionType_DefaultAfterCustom(t *testing.T) { "dns", result, expected) } } + +func TestValidateMandatoryConditions_AllPresent(t *testing.T) { + conditions := []api.AdapterCondition{ + {Type: api.ConditionTypeAvailable, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionFalse, LastTransitionTime: time.Now()}, + } + + missing, unknown := ValidateMandatoryConditions(conditions) + if missing != "" { + t.Errorf("Expected no missing conditions, got missing: %s", missing) + } + if unknown != "" { + t.Errorf("Expected no unknown conditions, got unknown: %s", unknown) + } +} + +func TestValidateMandatoryConditions_MissingAvailable(t *testing.T) { + conditions := []api.AdapterCondition{ + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + } + + missing, unknown := ValidateMandatoryConditions(conditions) + if missing != api.ConditionTypeAvailable { + t.Errorf("Expected missing condition %s, got: %s", api.ConditionTypeAvailable, missing) + } + if unknown != "" { + t.Errorf("Expected no unknown conditions, got: %s", unknown) + } +} + +func TestValidateMandatoryConditions_MandatoryConditionUnknown(t *testing.T) { + conditions := []api.AdapterCondition{ + {Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + } + + missing, unknown := ValidateMandatoryConditions(conditions) + if missing != "" { + t.Errorf("Expected no missing conditions, got: %s", missing) + } + if unknown != api.ConditionTypeAvailable { + t.Errorf("Expected unknown condition %s, got: %s", api.ConditionTypeAvailable, unknown) + } +} + +func TestValidateMandatoryConditions_WithCustomConditions(t *testing.T) { + conditions := []api.AdapterCondition{ + {Type: api.ConditionTypeAvailable, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: "CustomCondition", Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeReady, Status: api.AdapterConditionFalse, LastTransitionTime: time.Now()}, + } + + missing, unknown := ValidateMandatoryConditions(conditions) + if missing != "" { + t.Errorf("Expected no missing conditions, got: %s", missing) + } + if unknown != "" { + t.Errorf("Expected no unknown conditions, got: %s", unknown) + } +} + +func TestValidateMandatoryConditions_EmptyConditions(t *testing.T) { + conditions := []api.AdapterCondition{} + + missing, unknown := ValidateMandatoryConditions(conditions) + if missing != api.ConditionTypeAvailable { + t.Errorf("Expected missing condition %s, got: %s", api.ConditionTypeAvailable, missing) + } + if unknown != "" { + t.Errorf("Expected no unknown conditions, got: %s", unknown) + } +} + +func TestValidateMandatoryConditions_DuplicateCondition_UnknownThenTrue(t *testing.T) { + // Test: If Available appears twice (Unknown first, True second), + // should detect Unknown (Unknown has highest priority) + conditions := []api.AdapterCondition{ + {Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeAvailable, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, // Duplicate! + } + + missing, unknown := ValidateMandatoryConditions(conditions) + + // Should detect Unknown even though True comes later + if missing != "" { + t.Errorf("Expected no missing conditions, got: %s", missing) + } + if unknown != api.ConditionTypeAvailable { + t.Errorf("Expected unknown condition %s (Unknown should be preserved), got: %s", + api.ConditionTypeAvailable, unknown) + } +} + +func TestValidateMandatoryConditions_DuplicateCondition_TrueThenUnknown(t *testing.T) { + // Test: If Available appears twice (True first, Unknown second), + // should detect Unknown (Unknown has highest priority) + conditions := []api.AdapterCondition{ + // First: True + {Type: api.ConditionTypeAvailable, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeApplied, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + {Type: api.ConditionTypeHealth, Status: api.AdapterConditionTrue, LastTransitionTime: time.Now()}, + // Duplicate: Unknown! + {Type: api.ConditionTypeAvailable, Status: api.AdapterConditionUnknown, LastTransitionTime: time.Now()}, + } + + missing, unknown := ValidateMandatoryConditions(conditions) + + // Should detect Unknown even though True comes first + if missing != "" { + t.Errorf("Expected no missing conditions, got: %s", missing) + } + if unknown != api.ConditionTypeAvailable { + t.Errorf("Expected unknown condition %s (Unknown should overwrite True), got: %s", + api.ConditionTypeAvailable, unknown) + } +} diff --git a/test/integration/adapter_status_test.go b/test/integration/adapter_status_test.go index 7e55206..fce48eb 100644 --- a/test/integration/adapter_status_test.go +++ b/test/integration/adapter_status_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/gomega" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/util" "github.com/openshift-hyperfleet/hyperfleet-api/test" @@ -46,7 +47,22 @@ func TestClusterStatusPost(t *testing.T) { cluster.Generation, []openapi.ConditionRequest{ { - Type: "Ready", + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("ClusterAvailable"), + }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("ConfigurationApplied"), + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("HealthyCluster"), + }, + { + Type: api.ConditionTypeReady, Status: openapi.AdapterConditionStatusTrue, Reason: util.PtrString("AdapterReady"), }, @@ -84,7 +100,19 @@ func TestClusterStatusGet(t *testing.T) { cluster.Generation, []openapi.ConditionRequest{ { - Type: "Ready", + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeReady, Status: openapi.AdapterConditionStatusTrue, }, }, @@ -144,7 +172,22 @@ func TestNodePoolStatusPost(t *testing.T) { 1, []openapi.ConditionRequest{ { - Type: "Ready", + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusFalse, + Reason: util.PtrString("Initializing"), + }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("ConfigurationApplied"), + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusFalse, + Reason: util.PtrString("Initializing"), + }, + { + Type: api.ConditionTypeReady, Status: openapi.AdapterConditionStatusFalse, Reason: util.PtrString("Initializing"), }, @@ -182,7 +225,19 @@ func TestNodePoolStatusGet(t *testing.T) { 1, []openapi.ConditionRequest{ { - Type: "Ready", + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeReady, Status: openapi.AdapterConditionStatusTrue, }, }, @@ -222,7 +277,19 @@ func TestAdapterStatusPaging(t *testing.T) { cluster.Generation, []openapi.ConditionRequest{ { - Type: "Ready", + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeReady, Status: openapi.AdapterConditionStatusTrue, }, }, @@ -269,7 +336,22 @@ func TestAdapterStatusIdempotency(t *testing.T) { cluster.Generation, []openapi.ConditionRequest{ { - Type: "Ready", + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusFalse, + Reason: util.PtrString("Initializing"), + }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("ConfigurationApplied"), + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusFalse, + Reason: util.PtrString("Initializing"), + }, + { + Type: api.ConditionTypeReady, Status: openapi.AdapterConditionStatusFalse, Reason: util.PtrString("Initializing"), }, @@ -296,7 +378,22 @@ func TestAdapterStatusIdempotency(t *testing.T) { cluster.Generation, []openapi.ConditionRequest{ { - Type: "Ready", + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("ClusterAvailable"), + }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("ConfigurationApplied"), + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("HealthyCluster"), + }, + { + Type: api.ConditionTypeReady, Status: openapi.AdapterConditionStatusTrue, Reason: util.PtrString("AdapterReady"), }, @@ -335,9 +432,9 @@ func TestAdapterStatusIdempotency(t *testing.T) { To(Equal(openapi.AdapterConditionStatusTrue), "Conditions should be updated to latest") } -// TestClusterStatusPost_UnknownReturns201ThenSubsequent204 tests that the first Unknown Available -// status report is stored (201), while subsequent Unknown reports return 204 No Content. -func TestClusterStatusPost_UnknownReturns201ThenSubsequent204(t *testing.T) { +// TestClusterStatusPost_UnknownReturns204 tests that status reports with Unknown +// mandatory conditions are always rejected (HYPERFLEET-657) +func TestClusterStatusPost_UnknownReturns204(t *testing.T) { h, client := test.RegisterIntegration(t) account := h.NewRandAccount() @@ -347,30 +444,40 @@ func TestClusterStatusPost_UnknownReturns201ThenSubsequent204(t *testing.T) { cluster, err := h.Factories.NewClusters(h.NewID()) Expect(err).NotTo(HaveOccurred()) - // Create an adapter status with Available=Unknown + // Create an adapter status with all mandatory conditions but Available=Unknown statusInput := newAdapterStatusRequest( "test-adapter-unknown", cluster.Generation, []openapi.ConditionRequest{ { - Type: "Available", + Type: api.ConditionTypeAvailable, Status: openapi.AdapterConditionStatusUnknown, Reason: util.PtrString("StartupPending"), }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("ConfigurationApplied"), + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("HealthyCluster"), + }, }, nil, ) - // First report: should be stored (201 Created) + // First report with Unknown mandatory condition: should be rejected (204 No Content) resp, err := client.PostClusterStatusesWithResponse( ctx, cluster.ID, openapi.PostClusterStatusesJSONRequestBody(statusInput), test.WithAuthToken(ctx), ) Expect(err).NotTo(HaveOccurred(), "Error posting cluster status: %v", err) Expect(resp.StatusCode()). - To(Equal(http.StatusCreated), "Expected 201 Created for first Unknown status report") + To(Equal(http.StatusNoContent), "Expected 204 No Content for status with Unknown mandatory condition") - // Verify the status was stored + // Verify no status was stored listResp, err := client.GetClusterStatusesWithResponse(ctx, cluster.ID, nil, test.WithAuthToken(ctx)) Expect(err).NotTo(HaveOccurred()) Expect(listResp.JSON200).NotTo(BeNil()) @@ -382,9 +489,9 @@ func TestClusterStatusPost_UnknownReturns201ThenSubsequent204(t *testing.T) { break } } - Expect(found).To(BeTrue(), "First Unknown status report should be stored") + Expect(found).To(BeFalse(), "Status with Unknown mandatory condition should not be stored") - // Subsequent report with same adapter: should be no-op (204 No Content) + // Subsequent report with same adapter: should also be rejected (204 No Content) resp2, err := client.PostClusterStatusesWithResponse( ctx, cluster.ID, openapi.PostClusterStatusesJSONRequestBody(statusInput), test.WithAuthToken(ctx), @@ -394,9 +501,9 @@ func TestClusterStatusPost_UnknownReturns201ThenSubsequent204(t *testing.T) { To(Equal(http.StatusNoContent), "Expected 204 No Content for subsequent Unknown status report") } -// TestNodePoolStatusPost_UnknownReturns201ThenSubsequent204 tests that the first Unknown Available -// status report is stored (201), while subsequent Unknown reports return 204 No Content. -func TestNodePoolStatusPost_UnknownReturns201ThenSubsequent204(t *testing.T) { +// TestNodePoolStatusPost_UnknownReturns204 tests that status reports with Unknown +// mandatory conditions are always rejected (HYPERFLEET-657) +func TestNodePoolStatusPost_UnknownReturns204(t *testing.T) { h, client := test.RegisterIntegration(t) account := h.NewRandAccount() @@ -406,30 +513,40 @@ func TestNodePoolStatusPost_UnknownReturns201ThenSubsequent204(t *testing.T) { nodePool, err := h.Factories.NewNodePools(h.NewID()) Expect(err).NotTo(HaveOccurred()) - // Create an adapter status with Available=Unknown + // Create an adapter status with all mandatory conditions but Available=Unknown statusInput := newAdapterStatusRequest( "test-nodepool-adapter-unknown", 1, []openapi.ConditionRequest{ { - Type: "Available", + Type: api.ConditionTypeAvailable, Status: openapi.AdapterConditionStatusUnknown, Reason: util.PtrString("StartupPending"), }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("ConfigurationApplied"), + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("HealthyCluster"), + }, }, nil, ) - // First report: should be stored (201 Created) + // First report with Unknown mandatory condition: should be rejected (204 No Content) resp, err := client.PostNodePoolStatusesWithResponse( ctx, nodePool.OwnerID, nodePool.ID, openapi.PostNodePoolStatusesJSONRequestBody(statusInput), test.WithAuthToken(ctx), ) Expect(err).NotTo(HaveOccurred(), "Error posting nodepool status: %v", err) Expect(resp.StatusCode()). - To(Equal(http.StatusCreated), "Expected 201 Created for first Unknown status report") + To(Equal(http.StatusNoContent), "Expected 204 No Content for status with Unknown mandatory condition") - // Verify the status was stored + // Verify no status was stored listResp, err := client.GetNodePoolsStatusesWithResponse( ctx, nodePool.OwnerID, nodePool.ID, nil, test.WithAuthToken(ctx), ) @@ -443,9 +560,9 @@ func TestNodePoolStatusPost_UnknownReturns201ThenSubsequent204(t *testing.T) { break } } - Expect(found).To(BeTrue(), "First Unknown status report should be stored") + Expect(found).To(BeFalse(), "Status with Unknown mandatory condition should not be stored") - // Subsequent report with same adapter: should be no-op (204 No Content) + // Subsequent report with same adapter: should also be rejected (204 No Content) resp2, err := client.PostNodePoolStatusesWithResponse( ctx, nodePool.OwnerID, nodePool.ID, openapi.PostNodePoolStatusesJSONRequestBody(statusInput), test.WithAuthToken(ctx), @@ -456,7 +573,7 @@ func TestNodePoolStatusPost_UnknownReturns201ThenSubsequent204(t *testing.T) { } // TestClusterStatusPost_MultipleConditionsWithUnknownAvailable tests that -// Unknown Available is detected among multiple conditions +// Unknown Available is always rejected even among multiple conditions (HYPERFLEET-657) func TestClusterStatusPost_MultipleConditionsWithUnknownAvailable(t *testing.T) { h, client := test.RegisterIntegration(t) @@ -467,19 +584,29 @@ func TestClusterStatusPost_MultipleConditionsWithUnknownAvailable(t *testing.T) cluster, err := h.Factories.NewClusters(h.NewID()) Expect(err).NotTo(HaveOccurred()) - // Create an adapter status with multiple conditions including Available=Unknown + // Create an adapter status with all mandatory conditions but Available=Unknown statusInput := newAdapterStatusRequest( "test-adapter-multi-unknown", cluster.Generation, []openapi.ConditionRequest{ { - Type: "Ready", + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusUnknown, + Reason: util.PtrString("StartupPending"), + }, + { + Type: api.ConditionTypeApplied, Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("ConfigurationApplied"), }, { - Type: "Available", - Status: openapi.AdapterConditionStatusUnknown, - Reason: util.PtrString("StartupPending"), + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("HealthyCluster"), + }, + { + Type: api.ConditionTypeReady, + Status: openapi.AdapterConditionStatusTrue, }, { Type: "Progressing", @@ -489,16 +616,16 @@ func TestClusterStatusPost_MultipleConditionsWithUnknownAvailable(t *testing.T) nil, ) - // First report: should be stored (201 Created) even with Available=Unknown + // First report with Unknown mandatory condition: should be rejected (204 No Content) resp, err := client.PostClusterStatusesWithResponse( ctx, cluster.ID, openapi.PostClusterStatusesJSONRequestBody(statusInput), test.WithAuthToken(ctx), ) Expect(err).NotTo(HaveOccurred(), "Error posting cluster status: %v", err) - Expect(resp.StatusCode()).To(Equal(http.StatusCreated), - "Expected 201 Created for first report with Available=Unknown among multiple conditions") + Expect(resp.StatusCode()).To(Equal(http.StatusNoContent), + "Expected 204 No Content for first report with Available=Unknown among multiple conditions") - // Subsequent report: should be no-op (204 No Content) + // Subsequent report: should also be rejected (204 No Content) resp2, err := client.PostClusterStatusesWithResponse( ctx, cluster.ID, openapi.PostClusterStatusesJSONRequestBody(statusInput), test.WithAuthToken(ctx), @@ -526,7 +653,19 @@ func TestAdapterStatusPagingEdgeCases(t *testing.T) { cluster.Generation, []openapi.ConditionRequest{ { - Type: "Ready", + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeReady, Status: openapi.AdapterConditionStatusTrue, }, }, @@ -571,7 +710,19 @@ func TestAdapterStatusPagingEdgeCases(t *testing.T) { singleCluster.Generation, []openapi.ConditionRequest{ { - Type: "Ready", + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusTrue, + }, + { + Type: api.ConditionTypeReady, Status: openapi.AdapterConditionStatusTrue, }, }, @@ -623,3 +774,165 @@ func TestAdapterStatusPagingEdgeCases(t *testing.T) { // Verify we got all 10 unique adapters Expect(len(allItems)).To(Equal(10), "Should retrieve all items exactly once across pages") } +// TestHYPERFLEET657_MandatoryConditionsPreserved tests that adapter status updates +// without mandatory conditions are rejected and existing conditions are preserved +func TestHYPERFLEET657_MandatoryConditionsPreserved(t *testing.T) { + h, client := test.RegisterIntegration(t) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + + // Create a cluster first + cluster, err := h.Factories.NewClusters(h.NewID()) + Expect(err).NotTo(HaveOccurred()) + + // Send initial valid status with all mandatory conditions + initialStatus := newAdapterStatusRequest( + "adapter1", + cluster.Generation, + []openapi.ConditionRequest{ + { + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusFalse, + Reason: util.PtrString("ConfigMap data not yet available"), + Message: util.PtrString("ConfigMap data not yet available"), + }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("ConfigMapApplied"), + Message: util.PtrString("ConfigMap has been applied correctly"), + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusFalse, + Reason: util.PtrString("ConfigMap data not yet available"), + Message: util.PtrString("ConfigMap data not yet available"), + }, + }, + nil, + ) + + resp1, err := client.PostClusterStatusesWithResponse( + ctx, cluster.ID, + openapi.PostClusterStatusesJSONRequestBody(initialStatus), test.WithAuthToken(ctx), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(resp1.StatusCode()).To(Equal(http.StatusCreated)) + Expect(resp1.JSON201).ToNot(BeNil()) + Expect(len(resp1.JSON201.Conditions)).To(Equal(3)) + + // Verify Available, Applied, Health are present + conditionTypes := make(map[string]bool) + for _, cond := range resp1.JSON201.Conditions { + conditionTypes[cond.Type] = true + } + Expect(conditionTypes[api.ConditionTypeAvailable]).To(BeTrue()) + Expect(conditionTypes[api.ConditionTypeApplied]).To(BeTrue()) + Expect(conditionTypes[api.ConditionTypeHealth]).To(BeTrue()) + + // Now send an incomplete update (missing mandatory conditions) - this is the bug scenario + incompleteStatus := newAdapterStatusRequest( + "adapter1", + cluster.Generation, + []openapi.ConditionRequest{ + { + Type: "Available2", + Status: openapi.AdapterConditionStatusUnknown, + Reason: util.PtrString("ClusterProvisioned"), + Message: util.PtrString("Cluster successfully provisioned"), + }, + { + Type: "Health2", + Status: openapi.AdapterConditionStatusUnknown, + Reason: util.PtrString("ClusterProvisioned"), + Message: util.PtrString("Cluster successfully provisioned"), + }, + }, + &map[string]interface{}{ + "duration": "10m", + "job_name": "provision-job-123", + }, + ) + + resp2, err := client.PostClusterStatusesWithResponse( + ctx, cluster.ID, + openapi.PostClusterStatusesJSONRequestBody(incompleteStatus), test.WithAuthToken(ctx), + ) + Expect(err).NotTo(HaveOccurred()) + // Should return 204 No Content (update was discarded) + Expect(resp2.StatusCode()).To(Equal(http.StatusNoContent)) + + // Verify that the original conditions are preserved + respGet, err := client.GetClusterStatusesWithResponse(ctx, cluster.ID, nil, test.WithAuthToken(ctx)) + Expect(err).NotTo(HaveOccurred()) + Expect(respGet.StatusCode()).To(Equal(http.StatusOK)) + Expect(respGet.JSON200).ToNot(BeNil()) + Expect(len(respGet.JSON200.Items)).To(Equal(1)) + + // Verify Available, Applied, Health are still present (not replaced by Available2/Health2) + storedConditionTypes := make(map[string]bool) + for _, cond := range respGet.JSON200.Items[0].Conditions { + storedConditionTypes[cond.Type] = true + } + Expect(storedConditionTypes[api.ConditionTypeAvailable]).To(BeTrue(), "Available condition should be preserved") + Expect(storedConditionTypes[api.ConditionTypeApplied]).To(BeTrue(), "Applied condition should be preserved") + Expect(storedConditionTypes[api.ConditionTypeHealth]).To(BeTrue(), "Health condition should be preserved") + Expect(storedConditionTypes["Available2"]).To(BeFalse(), "Available2 should not be present") + Expect(storedConditionTypes["Health2"]).To(BeFalse(), "Health2 should not be present") +} + +// TestHYPERFLEET657_UnknownMandatoryConditionsRejected tests that updates with +// Unknown status for mandatory conditions are rejected +func TestHYPERFLEET657_UnknownMandatoryConditionsRejected(t *testing.T) { + h, client := test.RegisterIntegration(t) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + + // Create a cluster first + cluster, err := h.Factories.NewClusters(h.NewID()) + Expect(err).NotTo(HaveOccurred()) + + // Try to send status with all mandatory conditions but Available=Unknown + statusWithUnknown := newAdapterStatusRequest( + "adapter1", + cluster.Generation, + []openapi.ConditionRequest{ + { + Type: api.ConditionTypeAvailable, + Status: openapi.AdapterConditionStatusUnknown, + Reason: util.PtrString("Checking"), + Message: util.PtrString("Checking availability"), + }, + { + Type: api.ConditionTypeApplied, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("Applied"), + Message: util.PtrString("Configuration applied"), + }, + { + Type: api.ConditionTypeHealth, + Status: openapi.AdapterConditionStatusTrue, + Reason: util.PtrString("Healthy"), + Message: util.PtrString("Cluster is healthy"), + }, + }, + nil, + ) + + resp, err := client.PostClusterStatusesWithResponse( + ctx, cluster.ID, + openapi.PostClusterStatusesJSONRequestBody(statusWithUnknown), test.WithAuthToken(ctx), + ) + Expect(err).NotTo(HaveOccurred()) + // Should return 204 No Content (update was discarded because Available=Unknown) + Expect(resp.StatusCode()).To(Equal(http.StatusNoContent)) + + // Verify no status was stored + respGet, err := client.GetClusterStatusesWithResponse(ctx, cluster.ID, nil, test.WithAuthToken(ctx)) + Expect(err).NotTo(HaveOccurred()) + Expect(respGet.StatusCode()).To(Equal(http.StatusOK)) + Expect(respGet.JSON200).ToNot(BeNil()) + Expect(len(respGet.JSON200.Items)).To(Equal(0), "No status should be stored when mandatory condition is Unknown") +}