diff --git a/components/api-server/pkg/gatewayhealth/gatewayhealth.go b/components/api-server/pkg/gatewayhealth/gatewayhealth.go new file mode 100644 index 00000000..db3eee25 --- /dev/null +++ b/components/api-server/pkg/gatewayhealth/gatewayhealth.go @@ -0,0 +1,72 @@ +// Package gatewayhealth is the single source of truth for the vocabulary the +// HyperShell platform uses to describe a Gateway's health: its lifecycle Phase +// and the canonical status reason recorded alongside a healthy gateway. +// +// Both the API server (phase validation on writes and the per-phase metric) and +// the control plane (which writes phase/status back and gates on phase) import +// this package, so the vocabulary cannot drift between components. See +// specs/platform/gateway-phase-vocabulary.spec.md. +package gatewayhealth + +// Phase is the canonical lifecycle state of a Gateway. Values are TitleCase and +// compared case-sensitively. +type Phase string + +const ( + // PhasePending is accepted but not yet acted on by the reconciler. + PhasePending Phase = "Pending" + // PhaseProvisioning indicates manifests are being applied and the workload + // (and, for routed gateways, its external exposure) is not yet Ready. + PhaseProvisioning Phase = "Provisioning" + // PhaseRunning indicates the gateway is fully serving. + PhaseRunning Phase = "Running" + // PhaseDegraded indicates the gateway was provisioned but is currently + // unhealthy; it is recoverable without user action. + PhaseDegraded Phase = "Degraded" + // PhaseFailed indicates provisioning could not complete; recovery requires a + // change. + PhaseFailed Phase = "Failed" +) + +// StatusHealthy is the canonical human-readable status recorded alongside +// PhaseRunning when a gateway's workload - and, for routed gateways, its +// external exposure - is fully Ready. +const StatusHealthy = "Healthy" + +// canonicalPhases lists every allowed phase in lifecycle order. It is the one +// place the allowed-phase set is defined; all other consumers derive from it. +var canonicalPhases = []Phase{ + PhasePending, + PhaseProvisioning, + PhaseRunning, + PhaseDegraded, + PhaseFailed, +} + +// Phases returns the canonical phase set in lifecycle order. The returned slice +// is a copy, so callers cannot mutate the canonical set. +func Phases() []Phase { + out := make([]Phase, len(canonicalPhases)) + copy(out, canonicalPhases) + return out +} + +// PhaseStrings returns the canonical phase set as strings, in lifecycle order. +func PhaseStrings() []string { + out := make([]string, len(canonicalPhases)) + for i, p := range canonicalPhases { + out[i] = string(p) + } + return out +} + +// IsValidPhase reports whether s is exactly one of the canonical phase values. +// Comparison is case-sensitive: the platform vocabulary is TitleCase. +func IsValidPhase(s string) bool { + for _, p := range canonicalPhases { + if string(p) == s { + return true + } + } + return false +} diff --git a/components/api-server/pkg/gatewayhealth/gatewayhealth_test.go b/components/api-server/pkg/gatewayhealth/gatewayhealth_test.go new file mode 100644 index 00000000..e1af33b8 --- /dev/null +++ b/components/api-server/pkg/gatewayhealth/gatewayhealth_test.go @@ -0,0 +1,49 @@ +package gatewayhealth + +import "testing" + +func TestIsValidPhase(t *testing.T) { + cases := []struct { + name string + phase string + want bool + }{ + {"pending", "Pending", true}, + {"provisioning", "Provisioning", true}, + {"running", "Running", true}, + {"degraded", "Degraded", true}, + {"failed", "Failed", true}, + {"empty is not valid", "", false}, + {"unknown value", "Booting", false}, + {"wrong case is rejected", "running", false}, + {"trailing space is rejected", "Running ", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := IsValidPhase(tc.phase); got != tc.want { + t.Fatalf("IsValidPhase(%q) = %v, want %v", tc.phase, got, tc.want) + } + }) + } +} + +func TestPhaseStringsCoversEveryConstant(t *testing.T) { + got := PhaseStrings() + want := []string{"Pending", "Provisioning", "Running", "Degraded", "Failed"} + if len(got) != len(want) { + t.Fatalf("PhaseStrings() = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("PhaseStrings()[%d] = %q, want %q (order must be canonical)", i, got[i], want[i]) + } + } +} + +func TestPhasesReturnsCopy(t *testing.T) { + first := Phases() + first[0] = "Mutated" + if second := Phases(); second[0] != PhasePending { + t.Fatalf("Phases() returned a mutable view: got %q after mutation, want %q", second[0], PhasePending) + } +} diff --git a/components/api-server/plugins/gateways/grpc_handler.go b/components/api-server/plugins/gateways/grpc_handler.go index 977555cb..e6441f6b 100644 --- a/components/api-server/plugins/gateways/grpc_handler.go +++ b/components/api-server/plugins/gateways/grpc_handler.go @@ -11,6 +11,7 @@ import ( "google.golang.org/grpc/status" pb "github.com/openshift-online/hypershell/components/api-server/pkg/api/grpc/hypershell/v1" + "github.com/openshift-online/hypershell/components/api-server/pkg/gatewayhealth" "github.com/openshift-online/rh-trex-ai/pkg/api" pkgserver "github.com/openshift-online/rh-trex-ai/pkg/server" "github.com/openshift-online/rh-trex-ai/pkg/server/grpcutil" @@ -28,6 +29,18 @@ func NewGatewayGRPCHandler(svc GatewayService, generic services.GenericService, return &gatewayGRPCHandler{service: svc, generic: generic, brokerFunc: brokerFunc} } +// validateGatewayPhase rejects a phase outside the canonical vocabulary. An +// absent or empty phase is accepted so the field stays optional. +func validateGatewayPhase(phase *string) error { + if phase == nil || *phase == "" { + return nil + } + if !gatewayhealth.IsValidPhase(*phase) { + return status.Errorf(codes.InvalidArgument, "phase %q is not a valid gateway phase; allowed: %v", *phase, gatewayhealth.PhaseStrings()) + } + return nil +} + func (h *gatewayGRPCHandler) GetGateway(ctx context.Context, req *pb.GetGatewayRequest) (*pb.GetGatewayResponse, error) { if err := grpcutil.ValidateRequiredID(req.Id); err != nil { return nil, err @@ -53,6 +66,9 @@ func (h *gatewayGRPCHandler) CreateGateway(ctx context.Context, req *pb.CreateGa if err := grpcutil.ValidateStringField("database_id", req.DatabaseId, false); err != nil { return nil, err } + if err := validateGatewayPhase(req.Phase); err != nil { + return nil, err + } var serverDnsNamesJSON *string if len(req.ServerDnsNames) > 0 { data, _ := json.Marshal(req.ServerDnsNames) @@ -122,7 +138,10 @@ func (h *gatewayGRPCHandler) UpdateGateway(ctx context.Context, req *pb.UpdateGa } } if req.Phase != nil { - if err := grpcutil.ValidateStringField("phase", *req.Phase, false); err != nil { + // validateGatewayPhase enforces the canonical vocabulary, a strict subset + // of the generic non-empty/length check, so no separate ValidateStringField + // call is needed here. + if err := validateGatewayPhase(req.Phase); err != nil { return nil, err } } diff --git a/components/api-server/plugins/gateways/grpc_integration_test.go b/components/api-server/plugins/gateways/grpc_integration_test.go index ca1a4cda..8eddbc92 100644 --- a/components/api-server/plugins/gateways/grpc_integration_test.go +++ b/components/api-server/plugins/gateways/grpc_integration_test.go @@ -60,7 +60,7 @@ func TestGRPCGatewayCRUD(t *testing.T) { TlsMode: func() *string { s := "TestTlsMode"; return &s }(), ServiceType: func() *string { s := "TestServiceType"; return &s }(), Status: func() *string { s := "TestStatus"; return &s }(), - Phase: func() *string { s := "TestPhase"; return &s }(), + Phase: func() *string { s := "Provisioning"; return &s }(), } created, err := grpcClient.CreateGateway(ctx, createReq) Expect(err).NotTo(HaveOccurred()) @@ -88,7 +88,7 @@ func TestGRPCGatewayCRUD(t *testing.T) { TlsMode: func() *string { s := "UpdatedTlsMode"; return &s }(), ServiceType: func() *string { s := "UpdatedServiceType"; return &s }(), Status: func() *string { s := "UpdatedStatus"; return &s }(), - Phase: func() *string { s := "UpdatedPhase"; return &s }(), + Phase: func() *string { s := "Running"; return &s }(), } updated, err := grpcClient.UpdateGateway(ctx, updateReq) Expect(err).NotTo(HaveOccurred()) diff --git a/components/api-server/plugins/gateways/handler.go b/components/api-server/plugins/gateways/handler.go index ed519e32..c0344658 100644 --- a/components/api-server/plugins/gateways/handler.go +++ b/components/api-server/plugins/gateways/handler.go @@ -10,6 +10,7 @@ import ( "github.com/gorilla/mux" "github.com/openshift-online/hypershell/components/api-server/pkg/api/openapi" + "github.com/openshift-online/hypershell/components/api-server/pkg/gatewayhealth" "github.com/openshift-online/hypershell/components/api-server/pkg/rbac" "github.com/openshift-online/rh-trex-ai/pkg/api/presenters" "github.com/openshift-online/rh-trex-ai/pkg/auth" @@ -40,6 +41,18 @@ type gatewayHandler struct { ownerLookup GatewayOwnerLookup } +// validateGatewayPhaseValue rejects a phase outside the canonical vocabulary. An +// absent or empty phase is accepted so the field stays optional. +func validateGatewayPhaseValue(phase *string) *errors.ServiceError { + if phase == nil || *phase == "" { + return nil + } + if !gatewayhealth.IsValidPhase(*phase) { + return errors.Validation("phase %q is not a valid gateway phase; allowed: %v", *phase, gatewayhealth.PhaseStrings()) + } + return nil +} + func NewGatewayHandler(gateway GatewayService, generic services.GenericService, ownerBinding OwnerBindingCreator, visibilityFilter GatewayVisibilityFilter, ownerLookup GatewayOwnerLookup) *gatewayHandler { return &gatewayHandler{ gateway: gateway, @@ -58,6 +71,9 @@ func (h gatewayHandler) Create(w http.ResponseWriter, r *http.Request) { Action: func() (interface{}, *errors.ServiceError) { ctx := r.Context() gatewayModel := ConvertGateway(gateway) + if phaseErr := validateGatewayPhaseValue(gatewayModel.Phase); phaseErr != nil { + return nil, phaseErr + } gatewayModel, err := h.gateway.Create(ctx, gatewayModel) if err != nil { return nil, err @@ -116,6 +132,9 @@ func (h gatewayHandler) Patch(w http.ResponseWriter, r *http.Request) { found.Status = patch.Status } if patch.Phase != nil { + if phaseErr := validateGatewayPhaseValue(patch.Phase); phaseErr != nil { + return nil, phaseErr + } found.Phase = patch.Phase } if patch.Image != nil { diff --git a/components/api-server/plugins/gateways/integration_test.go b/components/api-server/plugins/gateways/integration_test.go index 095ec446..545e4166 100644 --- a/components/api-server/plugins/gateways/integration_test.go +++ b/components/api-server/plugins/gateways/integration_test.go @@ -58,7 +58,7 @@ func TestGatewayPost(t *testing.T) { TlsMode: openapi.PtrString("test-tls_mode"), ServiceType: openapi.PtrString("test-service_type"), Status: openapi.PtrString("test-status"), - Phase: openapi.PtrString("test-phase"), + Phase: openapi.PtrString("Provisioning"), } gatewayOutput, resp, err := client.DefaultAPI.CreateGateway(ctx).GatewayCreateRequest(gatewayInput).Execute() diff --git a/components/api-server/plugins/gateways/metrics.go b/components/api-server/plugins/gateways/metrics.go index 63ca6ac6..e186707e 100644 --- a/components/api-server/plugins/gateways/metrics.go +++ b/components/api-server/plugins/gateways/metrics.go @@ -4,6 +4,7 @@ import ( "context" "sync" + "github.com/openshift-online/hypershell/components/api-server/pkg/gatewayhealth" "github.com/prometheus/client_golang/prometheus" ) @@ -17,10 +18,20 @@ var ( metricsOnce sync.Once ) +// metricsHelp describes the per-phase gateway gauge. The canonical phase set is +// owned by the gatewayhealth package (single source of truth); any non-canonical +// or blank phase is aggregated into the "other" bucket. +const metricsHelp = "Number of gateways by phase (Pending, Provisioning, Running, Degraded, Failed, other)." + +// gatewayPhaseOther is the catch-all label for gateways whose stored phase is +// outside the canonical vocabulary (legacy or blank rows the service layer still +// tolerates), so the total never silently under-reports. +const gatewayPhaseOther = "other" + // RegisterGatewayMetrics registers a Prometheus GaugeVec that reports the -// number of gateways broken down by phase (Running, Provisioning, Degraded, -// Failed). The gauge is refreshed on every scrape by querying the database. -// It is safe to call multiple times; subsequent calls are no-ops. +// number of gateways broken down by phase. The gauge is refreshed on every +// scrape by querying the database. It is safe to call multiple times; +// subsequent calls are no-ops. func RegisterGatewayMetrics(dao GatewayDao) { metricsOnce.Do(func() { gatewayTotalVec = prometheus.NewGaugeVec( @@ -28,14 +39,14 @@ func RegisterGatewayMetrics(dao GatewayDao) { Namespace: metricsNamespace, Subsystem: metricsSubsystem, Name: "total", - Help: "Number of gateways by phase (Running, Provisioning, Degraded, Failed).", + Help: metricsHelp, }, []string{"phase"}, ) - // Pre-seed the known phases so they always appear in the output even + // Pre-seed the canonical phases so they always appear in the output even // when the count is zero, avoiding gaps in graphs. - for _, phase := range []string{"Running", "Provisioning", "Degraded", "Failed"} { + for _, phase := range gatewayhealth.PhaseStrings() { gatewayTotalVec.WithLabelValues(phase).Set(0) } @@ -55,7 +66,7 @@ func newGatewayCollector(dao GatewayDao) *gatewayCollector { dao: dao, desc: prometheus.NewDesc( prometheus.BuildFQName(metricsNamespace, metricsSubsystem, "total"), - "Number of gateways by phase (Running, Provisioning, Degraded, Failed).", + metricsHelp, []string{"phase"}, nil, ), @@ -74,14 +85,19 @@ func (c *gatewayCollector) Collect(ch chan<- prometheus.Metric) { return } - // Always emit the known phases so graphs never have gaps. - known := map[string]struct{}{ - "Running": {}, - "Provisioning": {}, - "Degraded": {}, - "Failed": {}, - } - for phase := range known { + // Always emit the canonical phases so graphs never have gaps. + for _, phase := range gatewayhealth.PhaseStrings() { ch <- prometheus.MustNewConstMetric(c.desc, prometheus.GaugeValue, float64(counts[phase]), phase) } + + // Aggregate any gateway whose stored phase is outside the canonical set + // (legacy or blank rows) into a single "other" bucket, emitted every scrape + // even at zero, so total drift stays visible rather than silently dropped. + var other float64 + for phase, count := range counts { + if !gatewayhealth.IsValidPhase(phase) { + other += float64(count) + } + } + ch <- prometheus.MustNewConstMetric(c.desc, prometheus.GaugeValue, other, gatewayPhaseOther) } diff --git a/components/api-server/plugins/gateways/phase_validation_test.go b/components/api-server/plugins/gateways/phase_validation_test.go new file mode 100644 index 00000000..c0936a66 --- /dev/null +++ b/components/api-server/plugins/gateways/phase_validation_test.go @@ -0,0 +1,138 @@ +package gateways_test + +import ( + "net/http" + "testing" + + . "github.com/onsi/gomega" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" + + pb "github.com/openshift-online/hypershell/components/api-server/pkg/api/grpc/hypershell/v1" + "github.com/openshift-online/hypershell/components/api-server/pkg/api/openapi" + "github.com/openshift-online/hypershell/components/api-server/test" +) + +// TestGRPCGatewayRejectsUnknownPhase proves the API server enforces the +// canonical phase vocabulary (see specs/platform/gateway-phase-vocabulary.spec.md): +// a create or update with a phase outside the canonical set is rejected with +// InvalidArgument and never persisted, while a canonical phase is accepted. +func TestGRPCGatewayRejectsUnknownPhase(t *testing.T) { + h, _ := test.RegisterIntegration(t) + h.StartControllersServer() + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + jwtToken := h.CreateJWTString(account) + + conn, err := grpc.NewClient( + h.GRPCAddress(), + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithPerRPCCredentials(&bearerToken{token: jwtToken}), + ) + Expect(err).NotTo(HaveOccurred()) + t.Cleanup(func() { + Expect(conn.Close()).To(Succeed()) + }) + + grpcClient := pb.NewGatewayServiceClient(conn) + + badPhase := "Booting" + _, err = grpcClient.CreateGateway(ctx, &pb.CreateGatewayRequest{ + Name: "reject-create", + ClusterId: "test-cluster_id", + ReleaseId: "test-release_id", + DatabaseId: "test-database_id", + Phase: &badPhase, + }) + Expect(err).To(HaveOccurred(), "create with an unknown phase must be rejected") + Expect(status.Code(err)).To(Equal(codes.InvalidArgument)) + + // A canonical phase on create is accepted. + goodPhase := "Provisioning" + created, err := grpcClient.CreateGateway(ctx, &pb.CreateGatewayRequest{ + Name: "accept-create", + ClusterId: "test-cluster_id", + ReleaseId: "test-release_id", + DatabaseId: "test-database_id", + Phase: &goodPhase, + }) + Expect(err).NotTo(HaveOccurred()) + + // An unknown phase on update is rejected... + _, err = grpcClient.UpdateGateway(ctx, &pb.UpdateGatewayRequest{ + Id: created.Gateway.Metadata.Id, + Phase: &badPhase, + }) + Expect(err).To(HaveOccurred(), "update with an unknown phase must be rejected") + Expect(status.Code(err)).To(Equal(codes.InvalidArgument)) + + // ...and the rejected value was not persisted. + got, err := grpcClient.GetGateway(ctx, &pb.GetGatewayRequest{Id: created.Gateway.Metadata.Id}) + Expect(err).NotTo(HaveOccurred()) + Expect(got.Gateway.GetPhase()).To(Equal("Provisioning")) + + // A canonical phase on update is accepted. + runningPhase := "Running" + _, err = grpcClient.UpdateGateway(ctx, &pb.UpdateGatewayRequest{ + Id: created.Gateway.Metadata.Id, + Phase: &runningPhase, + }) + Expect(err).NotTo(HaveOccurred()) + + // An absent phase is accepted so the field stays optional. + _, err = grpcClient.CreateGateway(ctx, &pb.CreateGatewayRequest{ + Name: "accept-absent-phase", + ClusterId: "test-cluster_id", + ReleaseId: "test-release_id", + DatabaseId: "test-database_id", + }) + Expect(err).NotTo(HaveOccurred(), "create without a phase must be accepted") +} + +// TestGatewayPatchNotTouchingPhaseAcceptsLegacyRecord proves the vocabulary +// tightening does not lock out pre-existing rows: a gateway whose stored phase +// predates validation (seeded through the service, which is not a write path) +// can still be patched on unrelated fields, because validation only fires when a +// write actually sets phase. +func TestGatewayPatchNotTouchingPhaseAcceptsLegacyRecord(t *testing.T) { + h, client := test.RegisterIntegration(t) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + + // Seed a record with a non-canonical stored phase via the service layer, + // which intentionally has no phase validation (only the REST/gRPC handlers do). + legacy, err := newGateway("legacy-phase") + Expect(err).NotTo(HaveOccurred()) + Expect(legacy.Phase).NotTo(BeNil()) + Expect(*legacy.Phase).To(Equal("test-phase"), "factory seeds a non-canonical phase") + + // A PATCH that does not touch phase must succeed despite the stored value. + _, resp, err := client.DefaultAPI.UpdateGateway(ctx, legacy.ID).GatewayPatchRequest(openapi.GatewayPatchRequest{ + TlsMode: openapi.PtrString("updated-tls-mode"), + }).Execute() + Expect(err).NotTo(HaveOccurred(), "patching a non-phase field on a legacy record must be accepted") + Expect(resp.StatusCode).To(Equal(http.StatusOK)) +} + +// TestRESTGatewayRejectsUnknownPhase proves the REST create path enforces the +// same canonical phase vocabulary with HTTP 400. +func TestRESTGatewayRejectsUnknownPhase(t *testing.T) { + h, client := test.RegisterIntegration(t) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + + _, resp, err := client.DefaultAPI.CreateGateway(ctx).GatewayCreateRequest(openapi.GatewayCreateRequest{ + Name: "reject-rest-create", + ClusterId: "test-cluster_id", + ReleaseId: "test-release_id", + DatabaseId: "test-database_id", + Phase: openapi.PtrString("Booting"), + }).Execute() + Expect(err).To(HaveOccurred(), "REST create with an unknown phase must be rejected") + Expect(resp.StatusCode).To(Equal(http.StatusBadRequest)) +} diff --git a/components/api-server/plugins/serviceAccounts/service.go b/components/api-server/plugins/serviceAccounts/service.go index f947a5ba..ea0385a1 100644 --- a/components/api-server/plugins/serviceAccounts/service.go +++ b/components/api-server/plugins/serviceAccounts/service.go @@ -11,6 +11,7 @@ import ( "time" "github.com/golang/glog" + "github.com/openshift-online/hypershell/components/api-server/pkg/gatewayhealth" "github.com/openshift-online/hypershell/components/api-server/pkg/rbac" "github.com/openshift-online/hypershell/components/api-server/plugins/gateways" "github.com/openshift-online/rh-trex-ai/pkg/api" @@ -956,7 +957,7 @@ func (s *service) readyGateway(ctx context.Context, gatewayID string) (*gateways if problem != nil { return nil, GatewayOIDC{}, problem } - if gateway.Phase == nil || !strings.EqualFold(*gateway.Phase, "Running") || gateway.Status == nil || !strings.EqualFold(*gateway.Status, "Healthy") { + if gateway.Phase == nil || !strings.EqualFold(*gateway.Phase, string(gatewayhealth.PhaseRunning)) || gateway.Status == nil || !strings.EqualFold(*gateway.Status, gatewayhealth.StatusHealthy) { return nil, GatewayOIDC{}, &APIError{Status: http.StatusConflict, Code: "gateway_not_ready", Message: "The gateway is not ready for service-account provisioning"} } return gateway, oidc, nil diff --git a/components/control-plane/internal/reconciler/gateway_vocabulary.go b/components/control-plane/internal/reconciler/gateway_vocabulary.go deleted file mode 100644 index e4c19799..00000000 --- a/components/control-plane/internal/reconciler/gateway_vocabulary.go +++ /dev/null @@ -1,11 +0,0 @@ -package reconciler - -// These values are the shared Gateway health vocabulary for the reconciler and -// health paths. Keep phase comparisons and phase updates on this vocabulary. -const ( - gatewayPhaseProvisioning = "Provisioning" - gatewayPhaseRunning = "Running" - gatewayPhaseDegraded = "Degraded" - gatewayPhaseFailed = "Failed" - gatewayStatusHealthy = "Healthy" -) diff --git a/components/control-plane/internal/reconciler/health.go b/components/control-plane/internal/reconciler/health.go index e68a3833..afafdac6 100644 --- a/components/control-plane/internal/reconciler/health.go +++ b/components/control-plane/internal/reconciler/health.go @@ -10,6 +10,7 @@ import ( "time" pb "github.com/openshift-online/hypershell/components/api-server/pkg/api/grpc/hypershell/v1" + "github.com/openshift-online/hypershell/components/api-server/pkg/gatewayhealth" "github.com/openshift-online/hypershell/components/control-plane/internal/exposure" "github.com/openshift-online/hypershell/components/control-plane/internal/gateway" "github.com/openshift-online/hypershell/components/control-plane/internal/keycloak" @@ -197,7 +198,7 @@ func (h *GatewayHealthReconciler) reconcileGatewayHealth(ctx context.Context, cl // observable workload. Leave Pending gateways to the provisioning path and // Failed gateways to a subsequent spec change. switch phase { - case gatewayPhaseRunning, gatewayPhaseDegraded, gatewayPhaseProvisioning: + case string(gatewayhealth.PhaseRunning), string(gatewayhealth.PhaseDegraded), string(gatewayhealth.PhaseProvisioning): default: return } @@ -247,7 +248,7 @@ func (h *GatewayHealthReconciler) reconcileGatewayHealth(ctx context.Context, cl return } h.clearRouteTimer(gatewayID) - desiredPhase, desiredStatus = gatewayPhaseDegraded, reason + desiredPhase, desiredStatus = string(gatewayhealth.PhaseDegraded), reason case h.exposure != nil && isRoutedGateway(gw): // Deployment is Ready; a routed gateway additionally requires its external // exposure to be observed Ready before it can be Running. @@ -259,7 +260,7 @@ func (h *GatewayHealthReconciler) reconcileGatewayHealth(ctx context.Context, cl } default: h.clearRouteTimer(gatewayID) - desiredPhase, desiredStatus = gatewayPhaseRunning, gatewayStatusHealthy + desiredPhase, desiredStatus = string(gatewayhealth.PhaseRunning), gatewayhealth.StatusHealthy } // active_sandbox_count is maintained independently by the event-driven @@ -292,7 +293,7 @@ func (h *GatewayHealthReconciler) reconcileGatewayHealth(ctx context.Context, cl // observations retain normal ownership of phase and status so operational // failures remain visible. func observedGatewayHealthUpdate(gatewayID, currentPhase, currentStatus, desiredPhase, desiredStatus string, keycloakConfigured bool) *pb.UpdateGatewayRequest { - if keycloakConfigured && isGatewayKeycloakClientStatus(currentStatus) && desiredPhase == gatewayPhaseRunning && desiredStatus == gatewayStatusHealthy { + if keycloakConfigured && isGatewayKeycloakClientStatus(currentStatus) && desiredPhase == string(gatewayhealth.PhaseRunning) && desiredStatus == gatewayhealth.StatusHealthy { if currentPhase == desiredPhase { return nil } @@ -538,21 +539,21 @@ func (h *GatewayHealthReconciler) evaluateRouteReadiness(ctx context.Context, ga } if rr.Ready { h.clearRouteTimer(gatewayID) - return gatewayPhaseRunning, gatewayStatusHealthy + return string(gatewayhealth.PhaseRunning), gatewayhealth.StatusHealthy } - if currentPhase == gatewayPhaseProvisioning { + if currentPhase == string(gatewayhealth.PhaseProvisioning) { since := h.markRouteNotReady(gatewayID) if h.now().Sub(since) >= h.routeReadyTimeout { h.clearRouteTimer(gatewayID) - return gatewayPhaseDegraded, fmt.Sprintf("route not ready after %s: %s", h.routeReadyTimeout, rr.Reason) + return string(gatewayhealth.PhaseDegraded), fmt.Sprintf("route not ready after %s: %s", h.routeReadyTimeout, rr.Reason) } - return gatewayPhaseProvisioning, rr.Reason + return string(gatewayhealth.PhaseProvisioning), rr.Reason } // currentPhase is Running (lost readiness) or Degraded (still unhealthy). h.clearRouteTimer(gatewayID) - return gatewayPhaseDegraded, rr.Reason + return string(gatewayhealth.PhaseDegraded), rr.Reason } // markRouteNotReady records the first time the gateway's Deployment was observed diff --git a/components/control-plane/internal/reconciler/metrics.go b/components/control-plane/internal/reconciler/metrics.go index 3028f473..8872ff61 100644 --- a/components/control-plane/internal/reconciler/metrics.go +++ b/components/control-plane/internal/reconciler/metrics.go @@ -6,6 +6,7 @@ import ( "time" pb "github.com/openshift-online/hypershell/components/api-server/pkg/api/grpc/hypershell/v1" + "github.com/openshift-online/hypershell/components/api-server/pkg/gatewayhealth" cpotel "github.com/openshift-online/hypershell/components/control-plane/internal/otel" ) @@ -47,7 +48,7 @@ func forgetGatewayProvisionObservation(gatewayID string) { // the stored phase for a normal event. The retry adapter supplies the phase that // it cleared when it bypasses the phase gate. func suppressGatewayProvisionObservation(gatewayID, previousPhase string) { - if previousPhase == gatewayPhaseRunning || previousPhase == gatewayPhaseDegraded { + if previousPhase == string(gatewayhealth.PhaseRunning) || previousPhase == string(gatewayhealth.PhaseDegraded) { claimGatewayProvisionObservation(gatewayID) } } @@ -78,5 +79,5 @@ func gatewayProvisionDuration(gw *pb.Gateway) (time.Duration, bool) { // gateway can stay in Provisioning after the first reconcile while its route // becomes ready. A Running gateway that fails moves through Degraded instead. func isGatewayProvisionCompletion(currentPhase, desiredPhase string) bool { - return currentPhase == gatewayPhaseProvisioning && desiredPhase == gatewayPhaseRunning + return currentPhase == string(gatewayhealth.PhaseProvisioning) && desiredPhase == string(gatewayhealth.PhaseRunning) } diff --git a/components/control-plane/internal/reconciler/reconciler.go b/components/control-plane/internal/reconciler/reconciler.go index 866f965d..85928761 100644 --- a/components/control-plane/internal/reconciler/reconciler.go +++ b/components/control-plane/internal/reconciler/reconciler.go @@ -18,6 +18,7 @@ import ( "unicode" pb "github.com/openshift-online/hypershell/components/api-server/pkg/api/grpc/hypershell/v1" + "github.com/openshift-online/hypershell/components/api-server/pkg/gatewayhealth" "github.com/openshift-online/hypershell/components/control-plane/internal/exposure" "github.com/openshift-online/hypershell/components/control-plane/internal/gateway" "github.com/openshift-online/hypershell/components/control-plane/internal/keycloak" @@ -1364,7 +1365,7 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. // gated. In particular, controller startup seeds existing Running gateways; // reconciling before the return below lets newly introduced client settings // converge without forcing a full gateway rollout. - if gw.Phase != nil && (*gw.Phase == gatewayPhaseRunning || *gw.Phase == gatewayPhaseProvisioning || *gw.Phase == gatewayPhaseDegraded) { + if gw.Phase != nil && (*gw.Phase == string(gatewayhealth.PhaseRunning) || *gw.Phase == string(gatewayhealth.PhaseProvisioning) || *gw.Phase == string(gatewayhealth.PhaseDegraded)) { if err := r.reconcileExistingGatewayKeycloakClient(ctx, event.ResourceID, gw); err != nil { var identityErr *gatewayKeycloakClientIdentityError if errors.As(err, &identityErr) { @@ -1512,10 +1513,10 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. RouteStillDesired: r.makeRouteStillDesired(event.ResourceID), } - r.updateGatewayPhase(ctx, event.ResourceID, gatewayPhaseProvisioning) + r.updateGatewayPhase(ctx, event.ResourceID, string(gatewayhealth.PhaseProvisioning)) if err := gateway.ReconcileGateway(ctx, r.dynamicClient, r.clientset, nsConfig, r.manifests, opts); err != nil { - r.updateGatewayPhase(ctx, event.ResourceID, gatewayPhaseFailed) + r.updateGatewayPhase(ctx, event.ResourceID, string(gatewayhealth.PhaseFailed)) reconcileErr = fmt.Errorf("reconcile gateway %s: %w", gw.Name, err) return reconcileErr } @@ -1525,7 +1526,7 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. // Deployment never becomes ready, set Degraded and record why. ready, reason := gateway.WaitForGatewayReady(ctx, r.clientset, namespace, 2*time.Minute) if !ready { - r.updateGatewayHealth(ctx, event.ResourceID, gatewayPhaseDegraded, reason) + r.updateGatewayHealth(ctx, event.ResourceID, string(gatewayhealth.PhaseDegraded), reason) log.Printf("WARN gateway %s applied but not ready in namespace %s: %s", gw.Name, namespace, reason) return nil } @@ -1545,17 +1546,17 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. if r.exposure != nil && routed { if r.waitForRouteReady(ctx, namespace) { // The observation guard rejects work that started in Running or Degraded. - if runningGateway := r.updateGatewayHealth(ctx, event.ResourceID, gatewayPhaseRunning, gatewayStatusHealthy); runningGateway != nil { + if runningGateway := r.updateGatewayHealth(ctx, event.ResourceID, string(gatewayhealth.PhaseRunning), gatewayhealth.StatusHealthy); runningGateway != nil { observeGatewayProvisionDuration(ctx, runningGateway) } log.Printf("INFO gateway %s provisioned and route ready in namespace %s", gw.Name, namespace) } else { - r.updateGatewayHealth(ctx, event.ResourceID, gatewayPhaseProvisioning, "Deployment ready; awaiting route readiness") + r.updateGatewayHealth(ctx, event.ResourceID, string(gatewayhealth.PhaseProvisioning), "Deployment ready; awaiting route readiness") log.Printf("INFO gateway %s deployment ready in namespace %s; awaiting route readiness", gw.Name, namespace) } } else { // The observation guard rejects work that started in Running or Degraded. - if runningGateway := r.updateGatewayHealth(ctx, event.ResourceID, gatewayPhaseRunning, gatewayStatusHealthy); runningGateway != nil { + if runningGateway := r.updateGatewayHealth(ctx, event.ResourceID, string(gatewayhealth.PhaseRunning), gatewayhealth.StatusHealthy); runningGateway != nil { observeGatewayProvisionDuration(ctx, runningGateway) } log.Printf("INFO gateway %s provisioned and ready in namespace %s", gw.Name, namespace) diff --git a/components/control-plane/internal/watcher/watcher.go b/components/control-plane/internal/watcher/watcher.go index e07297f1..b2e34b48 100644 --- a/components/control-plane/internal/watcher/watcher.go +++ b/components/control-plane/internal/watcher/watcher.go @@ -9,6 +9,7 @@ import ( "time" pb "github.com/openshift-online/hypershell/components/api-server/pkg/api/grpc/hypershell/v1" + "github.com/openshift-online/hypershell/components/api-server/pkg/gatewayhealth" "github.com/openshift-online/hypershell/components/control-plane/internal/keycloak" cpotel "github.com/openshift-online/hypershell/components/control-plane/internal/otel" "google.golang.org/grpc" @@ -756,7 +757,7 @@ func sameIDSet(a, b map[string]struct{}) bool { // gateways are not re-provisioned on every reconnect. func forceSeedRecovery(gw *pb.Gateway) bool { switch gw.GetPhase() { - case "Provisioning", "Degraded": + case string(gatewayhealth.PhaseProvisioning), string(gatewayhealth.PhaseDegraded): return true default: return false diff --git a/packages/gateway-management-ui/src/gateways/gateway-connections.ts b/packages/gateway-management-ui/src/gateways/gateway-connections.ts index c2855571..b9fd2f34 100644 --- a/packages/gateway-management-ui/src/gateways/gateway-connections.ts +++ b/packages/gateway-management-ui/src/gateways/gateway-connections.ts @@ -1,3 +1,5 @@ +import { gatewayCanonicalPhases } from "./gateway-data"; + export interface GatewayConnection { activeSandboxCount?: number; clusterId?: string; @@ -27,10 +29,8 @@ export interface GatewayConnection { * Only When Ready. */ export function isGatewayReadyToConnect(gateway: GatewayConnection): boolean { - return ( - gateway.phase?.trim().toLocaleLowerCase() === "running" && - Boolean(gateway.endpoint) - ); + const phase = gateway.phase?.trim().toLocaleLowerCase(); + return phase === gatewayCanonicalPhases.running && Boolean(gateway.endpoint); } const safeShellArgument = /^[A-Za-z0-9_./:@%+=,-]+$/; diff --git a/packages/gateway-management-ui/src/gateways/gateway-data.ts b/packages/gateway-management-ui/src/gateways/gateway-data.ts index 9d11c5d2..b7f66156 100644 --- a/packages/gateway-management-ui/src/gateways/gateway-data.ts +++ b/packages/gateway-management-ui/src/gateways/gateway-data.ts @@ -23,14 +23,36 @@ export const gatewayStatusPollMilliseconds = 5_000; // creation -- from being marked unavailable the instant it loads. export const gatewayConsoleReadyDeadlineMilliseconds = 600_000; -const gatewayPollingStates = new Set([ - "pending", - "provisioning", +// Canonical Gateway phase vocabulary emitted by the control plane, lowercased. +// Single source of truth on the console side, mirroring the Go gatewayhealth +// package (components/api-server/pkg/gatewayhealth). See +// specs/platform/gateway-phase-vocabulary.spec.md; keep in sync when the +// canonical phases change. +export const gatewayCanonicalPhases = { + pending: "pending", + provisioning: "provisioning", + running: "running", + degraded: "degraded", + failed: "failed", +} as const; + +// Recoverable (non-terminal) canonical phases keep the UI polling. The extra +// transitional descriptors (reconciling/updating) are tolerated in case they +// surface through the free-form health status, but the canonical phase set above +// is the source of truth for classification. +const gatewayPollingStates = new Set([ + gatewayCanonicalPhases.pending, + gatewayCanonicalPhases.provisioning, + gatewayCanonicalPhases.degraded, "reconciling", "updating", - "degraded", ]); -const gatewayFailedLifecycleStates = new Set(["error", "failed"]); +// The canonical terminal-failure phase; "error" is tolerated from free-form +// status text. +const gatewayFailedLifecycleStates = new Set([ + gatewayCanonicalPhases.failed, + "error", +]); type GatewayConsoleRecord = Pick< GatewayRecord, diff --git a/specs/platform/gateway-phase-vocabulary.spec.md b/specs/platform/gateway-phase-vocabulary.spec.md new file mode 100644 index 00000000..094aeae5 --- /dev/null +++ b/specs/platform/gateway-phase-vocabulary.spec.md @@ -0,0 +1,142 @@ +# Gateway Phase & Health Vocabulary + +**Date:** 2026-09-03 +**Status:** Active + +## Purpose + +A Gateway's health is reported through two fields - a lifecycle `phase` and a +human-readable `status` - that flow across three components: the control plane +writes them, the API server persists and exposes them (including a per-phase +metric), and the web console reads them to drive polling and the +ready-to-connect affordance. This spec defines the **canonical vocabulary** for +those values and requires every component to draw from a single shared source of +truth rather than duplicating string literals. + +The *behavioral* semantics of each phase (when a gateway becomes `Running`, when +it degrades, how health is continuously reconciled) are defined in +[`openshell-gateway-health.spec.md`](./openshell-gateway-health.spec.md). This +spec is a companion that standardizes the **representation** of that vocabulary +so the phase a client reads means the same thing in every component and cannot +silently drift. + +## Domain Vocabulary + +A Gateway `phase` SHALL be exactly one of the following canonical values, +written in TitleCase: + +| Phase | Meaning (see health spec for full semantics) | +|---|---| +| `Pending` | Accepted, not yet acted on by the reconciler. | +| `Provisioning` | Manifests being applied; workload/exposure not yet Ready. | +| `Running` | Fully serving (workload Ready, and exposure Ready if routed). | +| `Degraded` | Provisioned but currently unhealthy; recoverable. | +| `Failed` | Provisioning could not complete; requires a change to recover. | + +`Pending`, `Provisioning`, and `Degraded` are the **recoverable** (non-terminal) +phases; `Running` is the healthy terminal phase and `Failed` is the +non-recoverable terminal phase. + +The `status` field is a short human-readable descriptor that complements the +phase. When a gateway is fully healthy, its canonical status value is `Healthy`; +other status values carry a specific reason (e.g. a crash reason or +"route not ready after ") and remain human-readable rather than an +enumerated code. + +## Requirements + +### Requirement: Single Source of Truth for the Phase Vocabulary + +The platform SHALL define the canonical Gateway phase vocabulary in exactly one +shared location that both the API server and the control plane import. No +component SHALL hardcode its own independent copy of the phase strings or the +allowed-phase set. + +The shared definition SHALL expose, at minimum: the canonical phase constants, +the ordered set of all canonical phases, and a predicate that reports whether an +arbitrary string is a valid canonical phase (case-sensitive). + +#### Scenario: Control plane and API server agree on the vocabulary + +- GIVEN the control plane writes a Gateway `phase` +- AND the API server validates and reports that same `phase` +- WHEN a new phase value is added to or removed from the shared definition +- THEN the change SHALL take effect in both components without editing duplicated + literals in either one + +### Requirement: API Server Rejects Unknown Phase Values + +The API server SHALL reject any write (REST create/patch or gRPC +create/update) that sets a Gateway `phase` to a value outside the canonical +vocabulary, returning a validation error (HTTP 400 / gRPC `InvalidArgument`). An +absent or empty `phase` SHALL be accepted, so the field remains optional and a +caller that does not set it is unaffected. + +#### Scenario: Unknown phase rejected on gRPC update + +- GIVEN a client calls `UpdateGateway` with `phase` set to `"Booting"` +- WHEN the API server validates the request +- THEN it SHALL reject the request with `InvalidArgument` +- AND it SHALL NOT persist the value + +#### Scenario: Canonical phase accepted + +- GIVEN the control plane calls `UpdateGateway` with `phase` set to `"Running"` +- WHEN the API server validates the request +- THEN it SHALL accept the request and persist the value + +#### Scenario: Absent phase accepted + +- GIVEN a client creates a Gateway without setting `phase` +- WHEN the API server validates the request +- THEN it SHALL accept the request + +### Requirement: Phase Metric Covers the Full Canonical Set + +The API server's per-phase gateway gauge SHALL pre-seed and report every +canonical phase, so graphs never omit a phase. The set of phases the metric +reports SHALL be derived from the shared vocabulary rather than an independent +hardcoded list. + +#### Scenario: Every canonical phase appears in the metric + +- GIVEN no gateways exist in the `Pending` phase +- WHEN the gateway phase metric is scraped +- THEN the gauge SHALL still emit a `Pending` series with value `0` +- AND it SHALL emit a series for every other canonical phase + +### Requirement: Control Plane Uses the Canonical Vocabulary + +The control plane SHALL derive every Gateway `phase` value it reads (the phase +gate) or writes (provisioning path and continuous health reconciliation) from +the shared vocabulary, and SHALL use the canonical `Healthy` status constant for +the fully-healthy case, rather than duplicating string literals. + +#### Scenario: Health reconciler writes canonical values + +- GIVEN the control plane observes a gateway workload return to Ready +- WHEN it writes the recovered health back to the API server +- THEN the `phase` it writes SHALL be the canonical `Running` value +- AND the `status` it writes SHALL be the canonical `Healthy` value + +### Requirement: Console Vocabulary Aligns with the Canonical Phases + +The web console SHALL recognize the canonical phase set as its source of truth +for classifying a gateway as recoverable (keep polling), healthy, or terminally +failed, so its polling and ready-to-connect decisions stay consistent with the +phases the control plane actually emits. The console MAY additionally tolerate +broader status descriptors, but its canonical phase classification SHALL match +this vocabulary. + +#### Scenario: Recoverable phase keeps polling + +- GIVEN a Gateway displayed with a canonical recoverable phase + (`Pending`, `Provisioning`, or `Degraded`) +- WHEN the console evaluates whether to poll for status +- THEN it SHALL continue polling + +#### Scenario: Failed phase stops polling + +- GIVEN a Gateway displayed with the canonical `Failed` phase +- WHEN the console evaluates whether to poll for status +- THEN it SHALL treat the gateway as terminally failed and stop polling diff --git a/specs/platform/openshell-gateway-health.spec.md b/specs/platform/openshell-gateway-health.spec.md index ac394da1..ff05c9b4 100644 --- a/specs/platform/openshell-gateway-health.spec.md +++ b/specs/platform/openshell-gateway-health.spec.md @@ -16,7 +16,10 @@ manifests. A gateway whose pod is crash-looping must never be reported as This spec is a sub-spec of [`control-plane.spec.md`](./control-plane.spec.md) and refines its "Gateway Reconciliation" and "Status Synchronization" requirements. Provisioning mechanics are defined in -[`openshell-gateway.spec.md`](./openshell-gateway.spec.md). +[`openshell-gateway.spec.md`](./openshell-gateway.spec.md). The canonical +representation of the `phase`/`status` vocabulary used here - the allowed values +and the single shared source of truth every component draws from - is defined in +[`gateway-phase-vocabulary.spec.md`](./gateway-phase-vocabulary.spec.md). ## Domain Vocabulary