diff --git a/README.md b/README.md index 33ca033..3ad1d9c 100755 --- a/README.md +++ b/README.md @@ -260,6 +260,48 @@ curl -X POST http://localhost:8000/api/hyperfleet/v1/clusters \ }' | jq ``` +### Configuration + +HyperFleet API can be configured via environment variables: + +#### Schema Validation + +**`OPENAPI_SCHEMA_PATH`** +- **Description**: Path to the OpenAPI specification file used for validating cluster and nodepool spec fields +- **Default**: `openapi/openapi.yaml` (repository base schema) +- **Required**: No (service will start with default schema if not specified) +- **Usage**: + - **Local development**: Uses default repository schema + - **Production**: Set via Helm deployment to inject provider-specific schema from ConfigMap + +**Example:** +```bash +# Local development (uses default) +./hyperfleet-api serve + +# Custom schema path +export OPENAPI_SCHEMA_PATH=/path/to/custom/openapi.yaml +./hyperfleet-api serve + +# Production (Helm sets this automatically) +# OPENAPI_SCHEMA_PATH=/etc/hyperfleet/schemas/openapi.yaml +``` + +**How it works:** +1. The schema validator loads the OpenAPI specification at startup +2. When POST/PATCH requests are made to create or update resources, the `spec` field is validated against the schema +3. Invalid specs return HTTP 400 with detailed field-level error messages +4. Unknown resource types or missing schemas are gracefully handled (validation skipped) + +**Provider-specific schemas:** +In production deployments, cloud providers can inject their own OpenAPI schemas via Helm: +```bash +helm install hyperfleet-api ./chart \ + --set-file provider.schema=gcp-schema.yaml +``` + +The injected schema is mounted at `/etc/hyperfleet/schemas/openapi.yaml` and automatically used for validation. + ### Testing ```bash diff --git a/cmd/hyperfleet-api/server/routes.go b/cmd/hyperfleet-api/server/routes.go index d782234..2e077d5 100755 --- a/cmd/hyperfleet-api/server/routes.go +++ b/cmd/hyperfleet-api/server/routes.go @@ -1,8 +1,10 @@ package server import ( + "context" "fmt" "net/http" + "os" gorillahandlers "github.com/gorilla/handlers" "github.com/gorilla/mux" @@ -13,6 +15,8 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/db" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/handlers" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/middleware" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/validators" ) type ServicesInterface interface { @@ -95,6 +99,34 @@ func (s *apiServer) routes() *mux.Router { func registerApiMiddleware(router *mux.Router) { router.Use(MetricsMiddleware) + // Schema validation middleware (validates cluster/nodepool spec fields) + // Load schema from environment variable, default to repo base schema + schemaPath := os.Getenv("OPENAPI_SCHEMA_PATH") + if schemaPath == "" { + // Default: use base schema in repo (provider-agnostic) + // Production: Helm sets OPENAPI_SCHEMA_PATH=/etc/hyperfleet/schemas/openapi.yaml + schemaPath = "openapi/openapi.yaml" + } + + // Initialize schema validator (non-blocking - will warn if schema not found) + // Use background context for initialization logging + ctx := context.Background() + log := logger.NewOCMLogger(ctx) + + schemaValidator, err := validators.NewSchemaValidator(schemaPath) + if err != nil { + // Log warning but don't fail - schema validation is optional + log.Extra("schema_path", schemaPath).Extra("error", err.Error()).Warning("Failed to load schema validator") + log.Warning("Schema validation is disabled. Spec fields will not be validated.") + log.Info("To enable schema validation:") + log.Info(" - Local: Run from repo root, or set OPENAPI_SCHEMA_PATH=openapi/openapi.yaml") + log.Info(" - Production: Helm sets OPENAPI_SCHEMA_PATH=/etc/hyperfleet/schemas/openapi.yaml") + } else { + // Apply schema validation middleware + log.Extra("schema_path", schemaPath).Info("Schema validation enabled") + router.Use(middleware.SchemaValidationMiddleware(schemaValidator)) + } + router.Use( func(next http.Handler) http.Handler { return db.TransactionMiddleware(next, env().Database.SessionFactory) diff --git a/go.mod b/go.mod index fe858e9..88bdc01 100755 --- a/go.mod +++ b/go.mod @@ -9,15 +9,15 @@ require ( github.com/Masterminds/squirrel v1.1.0 github.com/auth0/go-jwt-middleware v0.0.0-20190805220309-36081240882b github.com/bxcodec/faker/v3 v3.2.0 - github.com/docker/go-connections v0.6.0 github.com/docker/go-healthcheck v0.1.0 + github.com/getkin/kin-openapi v0.133.0 github.com/ghodss/yaml v1.0.0 github.com/go-gormigrate/gormigrate/v2 v2.0.0 - github.com/golang-jwt/jwt/v4 v4.5.0 + github.com/golang-jwt/jwt/v4 v4.5.2 github.com/golang/glog v1.2.5 github.com/google/uuid v1.6.0 github.com/gorilla/handlers v1.4.2 - github.com/gorilla/mux v1.7.3 + github.com/gorilla/mux v1.8.0 github.com/jinzhu/inflection v1.0.0 github.com/lib/pq v1.10.9 github.com/mendsley/gojwk v0.0.0-20141217222730-4d5ec6e58103 @@ -56,11 +56,14 @@ require ( github.com/distribution/reference v0.6.0 // indirect github.com/docker/distribution v2.8.1+incompatible // indirect github.com/docker/docker v28.5.1+incompatible // indirect + github.com/docker/go-connections v0.6.0 // indirect github.com/docker/go-units v0.5.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-ole/go-ole v1.2.6 // indirect + github.com/go-openapi/jsonpointer v0.21.0 // indirect + github.com/go-openapi/swag v0.23.0 // indirect github.com/go-sql-driver/mysql v1.8.1 // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/google/go-cmp v0.7.0 // indirect @@ -72,12 +75,14 @@ require ( github.com/jackc/pgx/v5 v5.6.0 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect github.com/jinzhu/now v1.1.5 // indirect + github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/klauspost/compress v1.18.0 // indirect github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect github.com/magiconair/properties v1.8.10 // indirect + github.com/mailru/easyjson v0.7.7 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/microcosm-cc/bluemonday v1.0.23 // indirect github.com/moby/docker-image-spec v1.3.1 // indirect @@ -90,9 +95,13 @@ require ( github.com/moby/term v0.5.0 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect + github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 // indirect github.com/morikuni/aec v1.0.0 // indirect + github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037 // indirect + github.com/oasdiff/yaml3 v0.0.0-20250309153720-d2182401db90 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.1.1 // indirect + github.com/perimeterx/marshmallow v1.1.5 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect github.com/prometheus/client_model v0.3.0 // indirect @@ -105,6 +114,7 @@ require ( github.com/tklauser/go-sysconf v0.3.12 // indirect github.com/tklauser/numcpus v0.6.1 // indirect github.com/urfave/negroni v1.0.0 // indirect + github.com/woodsbury/decimal128 v1.3.0 // indirect github.com/yusufpapurcu/wmi v1.2.4 // indirect go.opentelemetry.io/auto/sdk v1.1.0 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect diff --git a/go.sum b/go.sum index 25910e1..f143ba0 100755 --- a/go.sum +++ b/go.sum @@ -143,6 +143,8 @@ github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2 github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= +github.com/getkin/kin-openapi v0.133.0 h1:pJdmNohVIJ97r4AUFtEXRXwESr8b0bD721u/Tz6k8PQ= +github.com/getkin/kin-openapi v0.133.0/go.mod h1:boAciF6cXk5FhPqe/NQeBTeenbjqU4LhWBf09ILVvWE= github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= @@ -163,6 +165,10 @@ github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/go-ole/go-ole v1.2.6 h1:/Fpf6oFPoeFik9ty7siob0G6Ke8QvQEuVcuChpwXzpY= github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= +github.com/go-openapi/jsonpointer v0.21.0 h1:YgdVicSA9vH5RiHs9TZW5oyafXZFc6+2Vc1rr/O9oNQ= +github.com/go-openapi/jsonpointer v0.21.0/go.mod h1:IUyH9l/+uyhIYQ/PXVA41Rexl+kOkAPDdXEYns6fzUY= +github.com/go-openapi/swag v0.23.0 h1:vsEVJDUo2hPJ2tu0/Xc+4noaxyEffXNIs3cOULZ+GrE= +github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ577vPjgQ= github.com/go-sql-driver/mysql v1.4.1/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg= github.com/go-sql-driver/mysql v1.7.0/go.mod h1:OXbVy3sEdcQ2Doequ6Z5BW6fXNQTmx+9S1MCJN5yJMI= @@ -170,13 +176,15 @@ github.com/go-sql-driver/mysql v1.8.1 h1:LedoTUt/eveggdHS9qUFC1EFSa8bU2+1pZjSRpv github.com/go-sql-driver/mysql v1.8.1/go.mod h1:wEBSXgmK//2ZFJyE+qWnIsVGmvmEKlqwuVSjsCm7DZg= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE= +github.com/go-test/deep v1.0.8 h1:TDsG77qcSprGbC6vTN8OuXp5g+J+b5Pcguhf7Zt61VM= +github.com/go-test/deep v1.0.8/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE= github.com/gofrs/uuid v3.2.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= github.com/gofrs/uuid v4.0.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.2.0/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/golang-jwt/jwt/v4 v4.4.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= -github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= -github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= +github.com/golang-jwt/jwt/v4 v4.5.2 h1:YtQM7lnr8iZ+j5q71MGKkNw9Mn7AjHM68uc9g5fXeUI= +github.com/golang-jwt/jwt/v4 v4.5.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe/go.mod h1:8vg3r2VgvsThLBIFL93Qb5yWzgyZWhEmBwUJWevAkK0= github.com/golang-sql/civil v0.0.0-20220223132316-b832511892a9 h1:au07oEsX2xN0ktxqI+Sida1w446QrXBRJ0nee3SNZlA= github.com/golang-sql/civil v0.0.0-20220223132316-b832511892a9/go.mod h1:8vg3r2VgvsThLBIFL93Qb5yWzgyZWhEmBwUJWevAkK0= @@ -257,8 +265,8 @@ github.com/gorilla/css v1.0.0/go.mod h1:Dn721qIggHpt4+EFCcTLTU/vk5ySda2ReITrtgBl github.com/gorilla/handlers v1.4.2 h1:0QniY0USkHQ1RGCLfKxeNHK9bkDHGRYGNDFBCS+YARg= github.com/gorilla/handlers v1.4.2/go.mod h1:Qkdc/uu4tH4g6mTK6auzZ766c4CA0Ng8+o/OAirnOIQ= github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= -github.com/gorilla/mux v1.7.3 h1:gnP5JzjVOuiZD07fKKToCAOjS0yOpj/qPETTXCCS6hw= -github.com/gorilla/mux v1.7.3/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= +github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= +github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/graphql-go/graphql v0.7.8/go.mod h1:k6yrAYQaSP59DC5UVxbgxESlmVyojThKdORUqGDGmrI= github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 h1:NmZ1PKzSTQbuGHw9DGPFomqkkLWMC+vZCkfs+FHv1Vg= github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3/go.mod h1:zQrxl1YP88HQlA6i9c63DSVPFklWpGX4OWAc9bFuaH4= @@ -356,6 +364,8 @@ github.com/jinzhu/now v1.1.5 h1:/o9tlHleP7gOFmsnYNz3RGnqzefHA47wQpKrrdTIwXQ= github.com/jinzhu/now v1.1.5/go.mod h1:d3SSVoowX0Lcu0IBviAWJpolVfI5UJVZZ7cO71lE/z8= github.com/joho/godotenv v1.3.0 h1:Zjp+RcGpHhGlrMbJzXTrZZPrWj+1vfm90La1wgB6Bhc= github.com/joho/godotenv v1.3.0/go.mod h1:7hK45KPybAkOC6peb+G5yklZfMxEjkZhHbwpqxOKXbg= +github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= +github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4= github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU= github.com/json-iterator/go v1.1.10/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4= @@ -403,6 +413,8 @@ github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2 github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/magiconair/properties v1.8.10 h1:s31yESBquKXCV9a/ScB3ESkOjUYYv+X0rg8SYxI99mE= github.com/magiconair/properties v1.8.10/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0= +github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= +github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcnceauSikq3lYCQ= github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= @@ -452,12 +464,18 @@ github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lN github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0= github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M= github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= +github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 h1:RWengNIwukTxcDr9M+97sNutRR1RKhG96O6jWumTTnw= +github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826/go.mod h1:TaXosZuwdSHYgviHp1DAtfrULt5eUgsSMsZf+YrPgl8= github.com/morikuni/aec v1.0.0 h1:nP9CBfwrvYnBRgY6qfDQkygYDmYwOilePFkwzv4dU8A= github.com/morikuni/aec v1.0.0/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7PXmsc= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= +github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037 h1:G7ERwszslrBzRxj//JalHPu/3yz+De2J+4aLtSRlHiY= +github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037/go.mod h1:2bpvgLBZEtENV5scfDFEtB/5+1M4hkQhDQrccEJ/qGw= +github.com/oasdiff/yaml3 v0.0.0-20250309153720-d2182401db90 h1:bQx3WeLcUWy+RletIKwUIt4x3t8n2SxavmoclizMb8c= +github.com/oasdiff/yaml3 v0.0.0-20250309153720-d2182401db90/go.mod h1:y5+oSEHCPT/DGrS++Wc/479ERge0zTFxaF8PbGKcg2o= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.10.1/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= @@ -484,6 +502,8 @@ github.com/openshift-online/ocm-sdk-go v0.1.334 h1:45WSkXEsmpGekMa9kO6NpEG8PW5/g github.com/openshift-online/ocm-sdk-go v0.1.334/go.mod h1:KYOw8kAKAHyPrJcQoVR82CneQ4ofC02Na4cXXaTq4Nw= github.com/openzipkin/zipkin-go v0.1.6/go.mod h1:QgAqvLzwWbR/WpD4A3cGpPtJrZXNIiJc5AZX7/PBEpw= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= +github.com/perimeterx/marshmallow v1.1.5 h1:a2LALqQ1BlHM8PZblsDdidgv1mWi1DgC2UmX50IvK2s= +github.com/perimeterx/marshmallow v1.1.5/go.mod h1:dsXbUu8CRzfYP5a87xpp0xq9S3u0Vchtcl8we9tYaXw= github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -587,8 +607,12 @@ github.com/tklauser/go-sysconf v0.3.12/go.mod h1:Ho14jnntGE1fpdOqQEEaiKRpvIavV0h github.com/tklauser/numcpus v0.6.1 h1:ng9scYS7az0Bk4OZLvrNXNSAO2Pxr1XXRAPyjhIx+Fk= github.com/tklauser/numcpus v0.6.1/go.mod h1:1XfjsgE2zo8GVw7POkMbHENHzVg3GzmoZ9fESEdAacY= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= +github.com/ugorji/go/codec v1.2.7 h1:YPXUKf7fYbp/y8xloBqZOw2qaVggbfwMlI8WM3wZUJ0= +github.com/ugorji/go/codec v1.2.7/go.mod h1:WGN1fab3R1fzQlVQTkfxVtIBhWDRqOviHU95kRgeqEY= github.com/urfave/negroni v1.0.0 h1:kIimOitoypq34K7TG7DUaJ9kq/N4Ofuwi1sjz0KipXc= github.com/urfave/negroni v1.0.0/go.mod h1:Meg73S6kFm/4PpbYdq35yYWoCZ9mS/YSx+lKnmiohz4= +github.com/woodsbury/decimal128 v1.3.0 h1:8pffMNWIlC0O5vbyHWFZAt5yWvWcrHA+3ovIIjVWss0= +github.com/woodsbury/decimal128 v1.3.0/go.mod h1:C5UTmyTjW3JftjUFzOVhC20BEQa2a4ZKOB5I6Zjb+ds= github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c/go.mod h1:lB8K/P019DLNhemzwFU4jHLhdvlE6uDZjXFejJXr49I= github.com/xdg/stringprep v1.0.0/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 5d32c17..f7c17ba 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -366,7 +366,6 @@ components: required: false schema: $ref: '#/components/schemas/OrderDirection' - default: desc explode: false QueryParams.orderBy: name: orderBy @@ -572,13 +571,8 @@ components: AdapterStatusList: type: object required: - - kind - items properties: - kind: - type: string - enum: - - AdapterStatusList items: type: array items: @@ -615,6 +609,20 @@ components: last_transition_time: '2021-01-01T10:01:00Z' created_time: '2021-01-01T10:01:00Z' last_report_time: '2021-01-01T10:01:30Z' + BearerAuth: + type: object + required: + - type + - scheme + properties: + type: + type: string + enum: + - http + scheme: + type: string + enum: + - bearer Cluster: type: object required: @@ -700,7 +708,7 @@ components: description: Cluster name (unique) spec: allOf: - - $ref: '#/components/schemas/ClusterSpecCore' + - $ref: '#/components/schemas/ClusterSpec' description: |- Cluster specification CLM doesn't know how to unmarshall the spec, it only stores and forwards to adapters to do their job @@ -729,7 +737,7 @@ components: $ref: '#/components/schemas/Cluster' allOf: - $ref: '#/components/schemas/List' - ClusterSpecCore: + ClusterSpec: type: object description: |- Core cluster specification. @@ -745,11 +753,8 @@ components: - conditions properties: phase: - type: string - enum: - - NotReady - - Ready - - Failed + allOf: + - $ref: '#/components/schemas/ResourcePhase' description: |- Current cluster phase (native database column). Updated when conditions are reported. @@ -798,11 +803,8 @@ components: type: string description: Condition type status: - type: string - enum: - - 'True' - - 'False' - - Unknown + allOf: + - $ref: '#/components/schemas/ConditionStatus' description: Condition status reason: type: string @@ -826,11 +828,7 @@ components: type: type: string status: - type: string - enum: - - 'True' - - 'False' - - Unknown + $ref: '#/components/schemas/ConditionStatus' reason: type: string message: @@ -838,6 +836,13 @@ components: description: |- Condition data for create/update requests (from adapters) observed_generation and observed_time are now at AdapterStatusCreateRequest level + ConditionStatus: + type: string + enum: + - 'True' + - 'False' + - Unknown + description: Status value for conditions Error: type: object properties: @@ -855,6 +860,18 @@ components: type: string operation_id: type: string + details: + type: array + items: + type: object + properties: + field: + type: string + description: Field path that failed validation + error: + type: string + description: Validation error message for this field + description: Field-level validation errors (optional) List: type: object required: @@ -970,7 +987,7 @@ components: description: NodePool name (unique in a cluster) spec: allOf: - - $ref: '#/components/schemas/NodePoolSpecCore' + - $ref: '#/components/schemas/NodePoolSpec' description: |- Cluster specification CLM doesn't know how to unmarshall the spec, it only stores and forwards to adapters to do their job @@ -1000,7 +1017,7 @@ components: description: NodePool name (unique in a cluster) spec: allOf: - - $ref: '#/components/schemas/NodePoolSpecCore' + - $ref: '#/components/schemas/NodePoolSpec' description: |- Cluster specification CLM doesn't know how to unmarshall the spec, it only stores and forwards to adapters to do their job @@ -1058,7 +1075,7 @@ components: description: NodePool name (unique in a cluster) spec: allOf: - - $ref: '#/components/schemas/NodePoolSpecCore' + - $ref: '#/components/schemas/NodePoolSpec' description: |- Cluster specification CLM doesn't know how to unmarshall the spec, it only stores and forwards to adapters to do their job @@ -1074,7 +1091,7 @@ components: $ref: '#/components/schemas/NodePool' allOf: - $ref: '#/components/schemas/List' - NodePoolSpecCore: + NodePoolSpec: type: object description: |- Core nodepool specification. @@ -1090,11 +1107,8 @@ components: - conditions properties: phase: - type: string - enum: - - NotReady - - Ready - - Failed + allOf: + - $ref: '#/components/schemas/ResourcePhase' description: |- Current NodePool phase (native database column). Updated when conditions are reported. @@ -1173,10 +1187,17 @@ components: Condition in Cluster/NodePool status Used for semantic condition types: "ValidationSuccessful", "DNSSuccessful", "NodePoolSuccessful", etc. Includes observed_generation and last_updated_time to track adapter-specific state + ResourcePhase: + type: string + enum: + - NotReady + - Ready + - Failed + description: Phase of a resource (Cluster or NodePool) securitySchemes: BearerAuth: type: http - scheme: Bearer + scheme: bearer servers: - url: http://localhost:8000 description: Development diff --git a/pkg/api/adapter_status_types.go b/pkg/api/adapter_status_types.go index f243c6b..2e70aea 100644 --- a/pkg/api/adapter_status_types.go +++ b/pkg/api/adapter_status_types.go @@ -1,10 +1,8 @@ package api import ( - "encoding/json" "time" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" "gorm.io/datatypes" "gorm.io/gorm" ) @@ -46,114 +44,3 @@ func (as *AdapterStatus) BeforeCreate(tx *gorm.DB) error { as.ID = NewID() return nil } - -// ToOpenAPI converts to OpenAPI model -func (as *AdapterStatus) ToOpenAPI() *openapi.AdapterStatus { - // Unmarshal Conditions - var conditions []openapi.AdapterCondition - if len(as.Conditions) > 0 { - if err := json.Unmarshal(as.Conditions, &conditions); err != nil { - // If unmarshal fails, use empty slice - conditions = []openapi.AdapterCondition{} - } - } - - // Unmarshal Data - var data map[string]map[string]interface{} - if len(as.Data) > 0 { - if err := json.Unmarshal(as.Data, &data); err != nil { - // If unmarshal fails, use empty map - data = make(map[string]map[string]interface{}) - } - } - - // Unmarshal Metadata - var metadata *openapi.AdapterStatusBaseMetadata - if len(as.Metadata) > 0 { - _ = json.Unmarshal(as.Metadata, &metadata) - } - - // Set default times if nil (shouldn't happen in normal operation) - createdTime := time.Time{} - if as.CreatedTime != nil { - createdTime = *as.CreatedTime - } - - lastReportTime := time.Time{} - if as.LastReportTime != nil { - lastReportTime = *as.LastReportTime - } - - return &openapi.AdapterStatus{ - Adapter: as.Adapter, - ObservedGeneration: as.ObservedGeneration, - Conditions: conditions, - Data: data, - Metadata: metadata, - CreatedTime: createdTime, - LastReportTime: lastReportTime, - } -} - -// AdapterStatusFromOpenAPICreate creates GORM model from CreateRequest -func AdapterStatusFromOpenAPICreate( - resourceType, resourceID string, - req *openapi.AdapterStatusCreateRequest, -) *AdapterStatus { - // Set timestamps - // CreatedTime and LastReportTime should be set from req.ObservedTime - now := time.Now() - if !req.ObservedTime.IsZero() { - now = req.ObservedTime - } - - // Convert ConditionRequest to AdapterCondition (adding LastTransitionTime) - adapterConditions := make([]openapi.AdapterCondition, len(req.Conditions)) - for i, condReq := range req.Conditions { - adapterConditions[i] = openapi.AdapterCondition{ - Type: condReq.Type, - Status: condReq.Status, - Reason: condReq.Reason, - Message: condReq.Message, - LastTransitionTime: now, - } - } - - // Marshal Conditions - if this fails, it's a programming error - conditionsJSON, err := json.Marshal(adapterConditions) - if err != nil { - // Fallback to empty array JSON - // Log marshal failure - this indicates a programming error - // logger.Errorf("Failed to marshal adapter conditions: %v", err) - conditionsJSON = []byte("[]") - } - - dataJSON := []byte("{}") // default fallback - - if req.Data != nil { - if b, err := json.Marshal(req.Data); err == nil { - dataJSON = b - } - } - - // Marshal Metadata (if provided) - metadataJSON := []byte("{}") // safe fallback - - if req.Metadata != nil { - if b, err := json.Marshal(req.Metadata); err == nil { - metadataJSON = b - } - } - - return &AdapterStatus{ - ResourceType: resourceType, - ResourceID: resourceID, - Adapter: req.Adapter, - ObservedGeneration: req.ObservedGeneration, - Conditions: conditionsJSON, - Data: dataJSON, - Metadata: metadataJSON, - CreatedTime: &now, - LastReportTime: &now, - } -} diff --git a/pkg/api/adapter_status_types_test.go b/pkg/api/adapter_status_types_test.go new file mode 100644 index 0000000..8b28cba --- /dev/null +++ b/pkg/api/adapter_status_types_test.go @@ -0,0 +1,94 @@ +package api + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +// TestAdapterStatusList_Index tests the Index() method for AdapterStatusList +func TestAdapterStatusList_Index(t *testing.T) { + RegisterTestingT(t) + + // Test empty list + emptyList := AdapterStatusList{} + emptyIndex := emptyList.Index() + Expect(len(emptyIndex)).To(Equal(0)) + + // Test single adapter status + status1 := &AdapterStatus{} + status1.ID = "status-1" + status1.Adapter = "test-adapter-1" + + singleList := AdapterStatusList{status1} + singleIndex := singleList.Index() + Expect(len(singleIndex)).To(Equal(1)) + Expect(singleIndex["status-1"]).To(Equal(status1)) + + // Test multiple adapter statuses + status2 := &AdapterStatus{} + status2.ID = "status-2" + status2.Adapter = "test-adapter-2" + + status3 := &AdapterStatus{} + status3.ID = "status-3" + status3.Adapter = "test-adapter-3" + + multiList := AdapterStatusList{status1, status2, status3} + multiIndex := multiList.Index() + Expect(len(multiIndex)).To(Equal(3)) + Expect(multiIndex["status-1"]).To(Equal(status1)) + Expect(multiIndex["status-2"]).To(Equal(status2)) + Expect(multiIndex["status-3"]).To(Equal(status3)) + + // Test duplicate IDs (later one overwrites earlier one) + status1Duplicate := &AdapterStatus{} + status1Duplicate.ID = "status-1" + status1Duplicate.Adapter = "duplicate-adapter" + + duplicateList := AdapterStatusList{status1, status1Duplicate} + duplicateIndex := duplicateList.Index() + Expect(len(duplicateIndex)).To(Equal(1)) + Expect(duplicateIndex["status-1"]).To(Equal(status1Duplicate)) + Expect(duplicateIndex["status-1"].Adapter).To(Equal("duplicate-adapter")) +} + +// TestAdapterStatus_BeforeCreate_IDGeneration tests ID auto-generation +func TestAdapterStatus_BeforeCreate_IDGeneration(t *testing.T) { + RegisterTestingT(t) + + status := &AdapterStatus{ + ResourceType: "Cluster", + ResourceID: "cluster-123", + Adapter: "test-adapter", + ObservedGeneration: 1, + } + + err := status.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(status.ID).ToNot(BeEmpty()) + Expect(len(status.ID)).To(BeNumerically(">", 0)) +} + +// TestAdapterStatus_BeforeCreate_NoOtherDefaults tests that no other fields are modified +func TestAdapterStatus_BeforeCreate_NoOtherDefaults(t *testing.T) { + RegisterTestingT(t) + + // Create status with all fields explicitly set + status := &AdapterStatus{ + ResourceType: "NodePool", + ResourceID: "nodepool-456", + Adapter: "custom-adapter", + ObservedGeneration: 5, + } + + err := status.BeforeCreate(nil) + Expect(err).To(BeNil()) + + // Verify only ID was set, other fields preserved + Expect(status.ID).ToNot(BeEmpty()) + Expect(status.ResourceType).To(Equal("NodePool")) + Expect(status.ResourceID).To(Equal("nodepool-456")) + Expect(status.Adapter).To(Equal("custom-adapter")) + Expect(status.ObservedGeneration).To(Equal(int32(5))) +} diff --git a/pkg/api/cluster_types.go b/pkg/api/cluster_types.go index 9461ec7..b05b4b6 100644 --- a/pkg/api/cluster_types.go +++ b/pkg/api/cluster_types.go @@ -1,17 +1,15 @@ package api import ( - "encoding/json" "time" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" "gorm.io/datatypes" "gorm.io/gorm" ) // Cluster database model type Cluster struct { - Meta // Contains ID, CreatedAt, UpdatedAt, DeletedAt + Meta // Contains ID, CreatedTime, UpdatedTime, DeletedTime // Core fields Kind string `json:"kind" gorm:"default:'Cluster'"` @@ -47,7 +45,10 @@ func (l ClusterList) Index() ClusterIndex { } func (c *Cluster) BeforeCreate(tx *gorm.DB) error { + now := time.Now() c.ID = NewID() + c.CreatedTime = now + c.UpdatedTime = now if c.Kind == "" { c.Kind = "Cluster" } @@ -55,7 +56,7 @@ func (c *Cluster) BeforeCreate(tx *gorm.DB) error { c.Generation = 1 } if c.StatusPhase == "" { - c.StatusPhase = "NotReady" + c.StatusPhase = string(PhaseNotReady) } // Set Href if not already set if c.Href == "" { @@ -64,108 +65,9 @@ func (c *Cluster) BeforeCreate(tx *gorm.DB) error { return nil } -// ToOpenAPI converts to OpenAPI model -func (c *Cluster) ToOpenAPI() *openapi.Cluster { - // Unmarshal Spec - spec := make(map[string]interface{}) - if len(c.Spec) > 0 { - _ = json.Unmarshal(c.Spec, &spec) - } - - // Unmarshal Labels - var labels map[string]string - if len(c.Labels) > 0 { - if err := json.Unmarshal(c.Labels, &labels); err != nil { - labels = make(map[string]string) - } - } - - // Unmarshal StatusConditions (stored as ResourceCondition in DB) - var statusConditions []openapi.ResourceCondition - if len(c.StatusConditions) > 0 { - if err := json.Unmarshal(c.StatusConditions, &statusConditions); err != nil { - statusConditions = []openapi.ResourceCondition{} - } - } - - // Generate Href if not set (fallback) - href := c.Href - if href == "" { - href = "/api/hyperfleet/v1/clusters/" + c.ID - } - - cluster := &openapi.Cluster{ - Id: &c.ID, - Kind: c.Kind, - Href: &href, - Name: c.Name, - Spec: spec, - Labels: &labels, - Generation: c.Generation, - CreatedTime: c.CreatedTime, - UpdatedTime: c.UpdatedTime, - CreatedBy: c.CreatedBy, - UpdatedBy: c.UpdatedBy, - } - - // Build ClusterStatus - set required fields with defaults if nil - lastTransitionTime := time.Time{} - if c.StatusLastTransitionTime != nil { - lastTransitionTime = *c.StatusLastTransitionTime - } - - lastUpdatedTime := time.Time{} - if c.StatusLastUpdatedTime != nil { - lastUpdatedTime = *c.StatusLastUpdatedTime - } - - cluster.Status = openapi.ClusterStatus{ - Phase: c.StatusPhase, - ObservedGeneration: c.StatusObservedGeneration, - Conditions: statusConditions, - LastTransitionTime: lastTransitionTime, - LastUpdatedTime: lastUpdatedTime, - } - - return cluster -} - -// ClusterFromOpenAPICreate creates GORM model from OpenAPI CreateRequest -func ClusterFromOpenAPICreate(req *openapi.ClusterCreateRequest, createdBy string) *Cluster { - // Marshal Spec - specJSON, err := json.Marshal(req.Spec) - if err != nil { - specJSON = []byte("{}") - } - - // Marshal Labels - labels := make(map[string]string) - if req.Labels != nil { - labels = *req.Labels - } - labelsJSON, err := json.Marshal(labels) - if err != nil { - labelsJSON = []byte("{}") - } - - // Marshal empty StatusConditions - statusConditionsJSON, err := json.Marshal([]openapi.ResourceCondition{}) - if err != nil { - statusConditionsJSON = []byte("[]") - } - - return &Cluster{ - Kind: req.Kind, - Name: req.Name, - Spec: specJSON, - Labels: labelsJSON, - Generation: 1, - StatusPhase: "NotReady", - StatusObservedGeneration: 0, - StatusConditions: statusConditionsJSON, - CreatedBy: createdBy, - UpdatedBy: createdBy, - } +func (c *Cluster) BeforeUpdate(tx *gorm.DB) error { + c.UpdatedTime = time.Now() + return nil } type ClusterPatchRequest struct { diff --git a/pkg/api/cluster_types_test.go b/pkg/api/cluster_types_test.go new file mode 100644 index 0000000..c5a3b59 --- /dev/null +++ b/pkg/api/cluster_types_test.go @@ -0,0 +1,203 @@ +package api + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +// TestClusterList_Index tests the Index() method for ClusterList +func TestClusterList_Index(t *testing.T) { + RegisterTestingT(t) + + // Test empty list + emptyList := ClusterList{} + emptyIndex := emptyList.Index() + Expect(len(emptyIndex)).To(Equal(0)) + + // Test single cluster + cluster1 := &Cluster{} + cluster1.ID = "cluster-1" + cluster1.Name = "test-cluster-1" + + singleList := ClusterList{cluster1} + singleIndex := singleList.Index() + Expect(len(singleIndex)).To(Equal(1)) + Expect(singleIndex["cluster-1"]).To(Equal(cluster1)) + + // Test multiple clusters + cluster2 := &Cluster{} + cluster2.ID = "cluster-2" + cluster2.Name = "test-cluster-2" + + cluster3 := &Cluster{} + cluster3.ID = "cluster-3" + cluster3.Name = "test-cluster-3" + + multiList := ClusterList{cluster1, cluster2, cluster3} + multiIndex := multiList.Index() + Expect(len(multiIndex)).To(Equal(3)) + Expect(multiIndex["cluster-1"]).To(Equal(cluster1)) + Expect(multiIndex["cluster-2"]).To(Equal(cluster2)) + Expect(multiIndex["cluster-3"]).To(Equal(cluster3)) + + // Test duplicate IDs (later one overwrites earlier one) + cluster1Duplicate := &Cluster{} + cluster1Duplicate.ID = "cluster-1" + cluster1Duplicate.Name = "duplicate-cluster" + + duplicateList := ClusterList{cluster1, cluster1Duplicate} + duplicateIndex := duplicateList.Index() + Expect(len(duplicateIndex)).To(Equal(1)) + Expect(duplicateIndex["cluster-1"]).To(Equal(cluster1Duplicate)) + Expect(duplicateIndex["cluster-1"].Name).To(Equal("duplicate-cluster")) +} + +// TestCluster_BeforeCreate_IDGeneration tests ID auto-generation +func TestCluster_BeforeCreate_IDGeneration(t *testing.T) { + RegisterTestingT(t) + + cluster := &Cluster{ + Name: "test-cluster", + } + + err := cluster.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(cluster.ID).ToNot(BeEmpty()) + Expect(len(cluster.ID)).To(BeNumerically(">", 0)) +} + +// TestCluster_BeforeCreate_KindDefault tests Kind default value +func TestCluster_BeforeCreate_KindDefault(t *testing.T) { + RegisterTestingT(t) + + // Test default Kind + cluster := &Cluster{ + Name: "test-cluster", + } + + err := cluster.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(cluster.Kind).To(Equal("Cluster")) +} + +// TestCluster_BeforeCreate_KindPreserved tests Kind is not overwritten +func TestCluster_BeforeCreate_KindPreserved(t *testing.T) { + RegisterTestingT(t) + + // Test Kind preservation + cluster := &Cluster{ + Name: "test-cluster", + Kind: "CustomCluster", + } + + err := cluster.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(cluster.Kind).To(Equal("CustomCluster")) +} + +// TestCluster_BeforeCreate_GenerationDefault tests Generation default value +func TestCluster_BeforeCreate_GenerationDefault(t *testing.T) { + RegisterTestingT(t) + + // Test default Generation + cluster := &Cluster{ + Name: "test-cluster", + } + + err := cluster.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(cluster.Generation).To(Equal(int32(1))) +} + +// TestCluster_BeforeCreate_GenerationPreserved tests Generation is not overwritten +func TestCluster_BeforeCreate_GenerationPreserved(t *testing.T) { + RegisterTestingT(t) + + // Test Generation preservation + cluster := &Cluster{ + Name: "test-cluster", + Generation: 5, + } + + err := cluster.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(cluster.Generation).To(Equal(int32(5))) +} + +// TestCluster_BeforeCreate_StatusPhaseDefault tests StatusPhase default value +func TestCluster_BeforeCreate_StatusPhaseDefault(t *testing.T) { + RegisterTestingT(t) + + // Test default StatusPhase + cluster := &Cluster{ + Name: "test-cluster", + } + + err := cluster.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(cluster.StatusPhase).To(Equal("NotReady")) +} + +// TestCluster_BeforeCreate_StatusPhasePreserved tests StatusPhase is not overwritten +func TestCluster_BeforeCreate_StatusPhasePreserved(t *testing.T) { + RegisterTestingT(t) + + // Test StatusPhase preservation + cluster := &Cluster{ + Name: "test-cluster", + StatusPhase: "Ready", + } + + err := cluster.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(cluster.StatusPhase).To(Equal("Ready")) +} + +// TestCluster_BeforeCreate_HrefGeneration tests Href auto-generation +func TestCluster_BeforeCreate_HrefGeneration(t *testing.T) { + RegisterTestingT(t) + + // Test Href generation + cluster := &Cluster{ + Name: "test-cluster", + } + + err := cluster.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(cluster.Href).To(Equal("/api/hyperfleet/v1/clusters/" + cluster.ID)) +} + +// TestCluster_BeforeCreate_HrefPreserved tests Href is not overwritten +func TestCluster_BeforeCreate_HrefPreserved(t *testing.T) { + RegisterTestingT(t) + + // Test Href preservation + cluster := &Cluster{ + Name: "test-cluster", + Href: "/custom/href", + } + + err := cluster.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(cluster.Href).To(Equal("/custom/href")) +} + +// TestCluster_BeforeCreate_Complete tests all defaults set together +func TestCluster_BeforeCreate_Complete(t *testing.T) { + RegisterTestingT(t) + + cluster := &Cluster{ + Name: "test-cluster", + } + + err := cluster.BeforeCreate(nil) + Expect(err).To(BeNil()) + + // Verify all defaults + Expect(cluster.ID).ToNot(BeEmpty()) + Expect(cluster.Kind).To(Equal("Cluster")) + Expect(cluster.Generation).To(Equal(int32(1))) + Expect(cluster.StatusPhase).To(Equal("NotReady")) + Expect(cluster.Href).To(Equal("/api/hyperfleet/v1/clusters/" + cluster.ID)) +} diff --git a/pkg/api/node_pool_types.go b/pkg/api/node_pool_types.go index 838ec71..aed130c 100644 --- a/pkg/api/node_pool_types.go +++ b/pkg/api/node_pool_types.go @@ -1,18 +1,16 @@ package api import ( - "encoding/json" "fmt" "time" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" "gorm.io/datatypes" "gorm.io/gorm" ) // NodePool database model type NodePool struct { - Meta // Contains ID, CreatedAt, UpdatedAt, DeletedAt + Meta // Contains ID, CreatedTime, UpdatedTime, DeletedAt // Core fields Kind string `json:"kind" gorm:"default:'NodePool'"` @@ -53,7 +51,10 @@ func (l NodePoolList) Index() NodePoolIndex { } func (np *NodePool) BeforeCreate(tx *gorm.DB) error { + now := time.Now() np.ID = NewID() + np.CreatedTime = now + np.UpdatedTime = now if np.Kind == "" { np.Kind = "NodePool" } @@ -61,7 +62,7 @@ func (np *NodePool) BeforeCreate(tx *gorm.DB) error { np.OwnerKind = "Cluster" } if np.StatusPhase == "" { - np.StatusPhase = "NotReady" + np.StatusPhase = string(PhaseNotReady) } // Set Href if not already set if np.Href == "" { @@ -74,128 +75,9 @@ func (np *NodePool) BeforeCreate(tx *gorm.DB) error { return nil } -// ToOpenAPI converts to OpenAPI model -func (np *NodePool) ToOpenAPI() *openapi.NodePool { - // Unmarshal Spec - var spec map[string]interface{} - if len(np.Spec) > 0 { - if err := json.Unmarshal(np.Spec, &spec); err != nil { - spec = make(map[string]interface{}) - } - } - - // Unmarshal Labels - var labels map[string]string - if len(np.Labels) > 0 { - if err := json.Unmarshal(np.Labels, &labels); err != nil { - labels = make(map[string]string) - } - } - - // Unmarshal StatusConditions (stored as ResourceCondition in DB) - var statusConditions []openapi.ResourceCondition - if len(np.StatusConditions) > 0 { - if err := json.Unmarshal(np.StatusConditions, &statusConditions); err != nil { - statusConditions = []openapi.ResourceCondition{} - } - } - - // Generate Href if not set (fallback) - href := np.Href - if href == "" { - href = fmt.Sprintf("/api/hyperfleet/v1/clusters/%s/nodepools/%s", np.OwnerID, np.ID) - } - - // Generate OwnerHref if not set (fallback) - ownerHref := np.OwnerHref - if ownerHref == "" { - ownerHref = "/api/hyperfleet/v1/clusters/" + np.OwnerID - } - - kind := np.Kind - nodePool := &openapi.NodePool{ - Id: &np.ID, - Kind: &kind, - Href: &href, - Name: np.Name, - Spec: spec, - Labels: &labels, - OwnerReferences: openapi.ObjectReference{ - Id: &np.OwnerID, - Kind: &np.OwnerKind, - Href: &ownerHref, - }, - CreatedTime: np.CreatedTime, - UpdatedTime: np.UpdatedTime, - CreatedBy: np.CreatedBy, - UpdatedBy: np.UpdatedBy, - } - - // Build NodePoolStatus - set required fields with defaults if nil - lastTransitionTime := time.Time{} - if np.StatusLastTransitionTime != nil { - lastTransitionTime = *np.StatusLastTransitionTime - } - - lastUpdatedTime := time.Time{} - if np.StatusLastUpdatedTime != nil { - lastUpdatedTime = *np.StatusLastUpdatedTime - } - - nodePool.Status = openapi.NodePoolStatus{ - Phase: np.StatusPhase, - ObservedGeneration: np.StatusObservedGeneration, - Conditions: statusConditions, - LastTransitionTime: lastTransitionTime, - LastUpdatedTime: lastUpdatedTime, - } - - return nodePool -} - -// NodePoolFromOpenAPICreate creates GORM model from OpenAPI CreateRequest -func NodePoolFromOpenAPICreate(req *openapi.NodePoolCreateRequest, ownerID, createdBy string) *NodePool { - // Marshal Spec - specJSON, err := json.Marshal(req.Spec) - if err != nil { - //logger.Errorf("Failed to marshal NodePool spec: %v", err) - specJSON = []byte("{}") - } - - // Marshal Labels - labels := make(map[string]string) - if req.Labels != nil { - labels = *req.Labels - } - labelsJSON, err := json.Marshal(labels) - if err != nil { - labelsJSON = []byte("{}") - } - - // Marshal empty StatusConditions - statusConditionsJSON, err := json.Marshal([]openapi.ResourceCondition{}) - if err != nil { - statusConditionsJSON = []byte("[]") - } - - kind := "NodePool" - if req.Kind != nil { - kind = *req.Kind - } - - return &NodePool{ - Kind: kind, - Name: req.Name, - Spec: specJSON, - Labels: labelsJSON, - OwnerID: ownerID, - OwnerKind: "Cluster", - StatusPhase: "NotReady", - StatusObservedGeneration: 0, - StatusConditions: statusConditionsJSON, - CreatedBy: createdBy, - UpdatedBy: createdBy, - } +func (np *NodePool) BeforeUpdate(tx *gorm.DB) error { + np.UpdatedTime = time.Now() + return nil } type NodePoolPatchRequest struct { diff --git a/pkg/api/node_pool_types_test.go b/pkg/api/node_pool_types_test.go new file mode 100644 index 0000000..c747594 --- /dev/null +++ b/pkg/api/node_pool_types_test.go @@ -0,0 +1,245 @@ +package api + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +// TestNodePoolList_Index tests the Index() method for NodePoolList +func TestNodePoolList_Index(t *testing.T) { + RegisterTestingT(t) + + // Test empty list + emptyList := NodePoolList{} + emptyIndex := emptyList.Index() + Expect(len(emptyIndex)).To(Equal(0)) + + // Test single nodepool + nodepool1 := &NodePool{} + nodepool1.ID = "nodepool-1" + nodepool1.Name = "test-nodepool-1" + + singleList := NodePoolList{nodepool1} + singleIndex := singleList.Index() + Expect(len(singleIndex)).To(Equal(1)) + Expect(singleIndex["nodepool-1"]).To(Equal(nodepool1)) + + // Test multiple nodepools + nodepool2 := &NodePool{} + nodepool2.ID = "nodepool-2" + nodepool2.Name = "test-nodepool-2" + + nodepool3 := &NodePool{} + nodepool3.ID = "nodepool-3" + nodepool3.Name = "test-nodepool-3" + + multiList := NodePoolList{nodepool1, nodepool2, nodepool3} + multiIndex := multiList.Index() + Expect(len(multiIndex)).To(Equal(3)) + Expect(multiIndex["nodepool-1"]).To(Equal(nodepool1)) + Expect(multiIndex["nodepool-2"]).To(Equal(nodepool2)) + Expect(multiIndex["nodepool-3"]).To(Equal(nodepool3)) + + // Test duplicate IDs (later one overwrites earlier one) + nodepool1Duplicate := &NodePool{} + nodepool1Duplicate.ID = "nodepool-1" + nodepool1Duplicate.Name = "duplicate-nodepool" + + duplicateList := NodePoolList{nodepool1, nodepool1Duplicate} + duplicateIndex := duplicateList.Index() + Expect(len(duplicateIndex)).To(Equal(1)) + Expect(duplicateIndex["nodepool-1"]).To(Equal(nodepool1Duplicate)) + Expect(duplicateIndex["nodepool-1"].Name).To(Equal("duplicate-nodepool")) +} + +// TestNodePool_BeforeCreate_IDGeneration tests ID auto-generation +func TestNodePool_BeforeCreate_IDGeneration(t *testing.T) { + RegisterTestingT(t) + + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "cluster-123", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(nodepool.ID).ToNot(BeEmpty()) + Expect(len(nodepool.ID)).To(BeNumerically(">", 0)) +} + +// TestNodePool_BeforeCreate_KindDefault tests Kind default value +func TestNodePool_BeforeCreate_KindDefault(t *testing.T) { + RegisterTestingT(t) + + // Test default Kind + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "cluster-123", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(nodepool.Kind).To(Equal("NodePool")) +} + +// TestNodePool_BeforeCreate_KindPreserved tests Kind is not overwritten +func TestNodePool_BeforeCreate_KindPreserved(t *testing.T) { + RegisterTestingT(t) + + // Test Kind preservation + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "cluster-123", + Kind: "CustomNodePool", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(nodepool.Kind).To(Equal("CustomNodePool")) +} + +// TestNodePool_BeforeCreate_OwnerKindDefault tests OwnerKind default value +func TestNodePool_BeforeCreate_OwnerKindDefault(t *testing.T) { + RegisterTestingT(t) + + // Test default OwnerKind + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "cluster-123", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(nodepool.OwnerKind).To(Equal("Cluster")) +} + +// TestNodePool_BeforeCreate_OwnerKindPreserved tests OwnerKind is not overwritten +func TestNodePool_BeforeCreate_OwnerKindPreserved(t *testing.T) { + RegisterTestingT(t) + + // Test OwnerKind preservation + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "custom-owner-123", + OwnerKind: "CustomOwner", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(nodepool.OwnerKind).To(Equal("CustomOwner")) +} + +// TestNodePool_BeforeCreate_StatusPhaseDefault tests StatusPhase default value +func TestNodePool_BeforeCreate_StatusPhaseDefault(t *testing.T) { + RegisterTestingT(t) + + // Test default StatusPhase + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "cluster-123", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(nodepool.StatusPhase).To(Equal("NotReady")) +} + +// TestNodePool_BeforeCreate_StatusPhasePreserved tests StatusPhase is not overwritten +func TestNodePool_BeforeCreate_StatusPhasePreserved(t *testing.T) { + RegisterTestingT(t) + + // Test StatusPhase preservation + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "cluster-123", + StatusPhase: "Ready", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(nodepool.StatusPhase).To(Equal("Ready")) +} + +// TestNodePool_BeforeCreate_HrefGeneration tests Href auto-generation +func TestNodePool_BeforeCreate_HrefGeneration(t *testing.T) { + RegisterTestingT(t) + + // Test Href generation + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "cluster-abc", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(nodepool.Href).To(Equal("/api/hyperfleet/v1/clusters/cluster-abc/nodepools/" + nodepool.ID)) +} + +// TestNodePool_BeforeCreate_HrefPreserved tests Href is not overwritten +func TestNodePool_BeforeCreate_HrefPreserved(t *testing.T) { + RegisterTestingT(t) + + // Test Href preservation + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "cluster-abc", + Href: "/custom/href", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(nodepool.Href).To(Equal("/custom/href")) +} + +// TestNodePool_BeforeCreate_OwnerHrefGeneration tests OwnerHref auto-generation +func TestNodePool_BeforeCreate_OwnerHrefGeneration(t *testing.T) { + RegisterTestingT(t) + + // Test OwnerHref generation + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "cluster-xyz", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(nodepool.OwnerHref).To(Equal("/api/hyperfleet/v1/clusters/cluster-xyz")) +} + +// TestNodePool_BeforeCreate_OwnerHrefPreserved tests OwnerHref is not overwritten +func TestNodePool_BeforeCreate_OwnerHrefPreserved(t *testing.T) { + RegisterTestingT(t) + + // Test OwnerHref preservation + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "cluster-xyz", + OwnerHref: "/custom/owner/href", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + Expect(nodepool.OwnerHref).To(Equal("/custom/owner/href")) +} + +// TestNodePool_BeforeCreate_Complete tests all defaults set together +func TestNodePool_BeforeCreate_Complete(t *testing.T) { + RegisterTestingT(t) + + nodepool := &NodePool{ + Name: "test-nodepool", + OwnerID: "cluster-complete", + } + + err := nodepool.BeforeCreate(nil) + Expect(err).To(BeNil()) + + // Verify all defaults + Expect(nodepool.ID).ToNot(BeEmpty()) + Expect(nodepool.Kind).To(Equal("NodePool")) + Expect(nodepool.OwnerKind).To(Equal("Cluster")) + Expect(nodepool.StatusPhase).To(Equal("NotReady")) + Expect(nodepool.Href).To(Equal("/api/hyperfleet/v1/clusters/cluster-complete/nodepools/" + nodepool.ID)) + Expect(nodepool.OwnerHref).To(Equal("/api/hyperfleet/v1/clusters/cluster-complete")) +} diff --git a/pkg/api/presenters/adapter_status.go b/pkg/api/presenters/adapter_status.go new file mode 100644 index 0000000..0b2482e --- /dev/null +++ b/pkg/api/presenters/adapter_status.go @@ -0,0 +1,127 @@ +package presenters + +import ( + "encoding/json" + "fmt" + "time" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" + "gorm.io/datatypes" +) + +// ConvertAdapterStatus converts openapi.AdapterStatusCreateRequest to api.AdapterStatus (GORM model) +func ConvertAdapterStatus( + resourceType, resourceID string, + req *openapi.AdapterStatusCreateRequest, +) (*api.AdapterStatus, error) { + // Set timestamps + // CreatedTime and LastReportTime should be set from req.ObservedTime + now := time.Now() + if !req.ObservedTime.IsZero() { + now = req.ObservedTime + } + + // Convert ConditionRequest to AdapterCondition (adding LastTransitionTime) + adapterConditions := make([]api.AdapterCondition, len(req.Conditions)) + for i, condReq := range req.Conditions { + adapterConditions[i] = api.AdapterCondition{ + Type: condReq.Type, + Status: api.ConditionStatus(string(condReq.Status)), + Reason: condReq.Reason, + Message: condReq.Message, + LastTransitionTime: now, + } + } + + // Marshal Conditions + conditionsJSON, err := json.Marshal(adapterConditions) + if err != nil { + return nil, fmt.Errorf("failed to marshal adapter conditions: %w", err) + } + + // Marshal Data + data := make(map[string]map[string]interface{}) + if req.Data != nil { + data = req.Data + } + dataJSON, err := json.Marshal(data) + if err != nil { + return nil, fmt.Errorf("failed to marshal adapter data: %w", err) + } + + // Marshal Metadata (if provided) + var metadataJSON datatypes.JSON + if req.Metadata != nil { + metadataJSON, err = json.Marshal(req.Metadata) + if err != nil { + return nil, fmt.Errorf("failed to marshal adapter metadata: %w", err) + } + } + + return &api.AdapterStatus{ + ResourceType: resourceType, + ResourceID: resourceID, + Adapter: req.Adapter, + ObservedGeneration: req.ObservedGeneration, + Conditions: conditionsJSON, + Data: dataJSON, + Metadata: metadataJSON, + CreatedTime: &now, + LastReportTime: &now, + }, nil +} + +// PresentAdapterStatus converts api.AdapterStatus (GORM model) to openapi.AdapterStatus +func PresentAdapterStatus(adapterStatus *api.AdapterStatus) openapi.AdapterStatus { + // Unmarshal Conditions + var conditions []api.AdapterCondition + if len(adapterStatus.Conditions) > 0 { + _ = json.Unmarshal(adapterStatus.Conditions, &conditions) + } + + // Convert domain AdapterConditions to openapi format + openapiConditions := make([]openapi.AdapterCondition, len(conditions)) + for i, cond := range conditions { + openapiConditions[i] = openapi.AdapterCondition{ + Type: cond.Type, + Status: openapi.ConditionStatus(string(cond.Status)), + Reason: cond.Reason, + Message: cond.Message, + LastTransitionTime: cond.LastTransitionTime, + } + } + + // Unmarshal Data + var data map[string]map[string]interface{} + if len(adapterStatus.Data) > 0 { + _ = json.Unmarshal(adapterStatus.Data, &data) + } + + // Unmarshal Metadata + var metadata *openapi.AdapterStatusBaseMetadata + if len(adapterStatus.Metadata) > 0 { + _ = json.Unmarshal(adapterStatus.Metadata, &metadata) + } + + // Set default times if nil (shouldn't happen in normal operation) + createdTime := time.Time{} + if adapterStatus.CreatedTime != nil { + createdTime = *adapterStatus.CreatedTime + } + + lastReportTime := time.Time{} + if adapterStatus.LastReportTime != nil { + lastReportTime = *adapterStatus.LastReportTime + } + + return openapi.AdapterStatus{ + Adapter: adapterStatus.Adapter, + ObservedGeneration: adapterStatus.ObservedGeneration, + Conditions: openapiConditions, + Data: data, + Metadata: metadata, + CreatedTime: createdTime, + LastReportTime: lastReportTime, + } +} diff --git a/pkg/api/presenters/adapter_status_test.go b/pkg/api/presenters/adapter_status_test.go new file mode 100644 index 0000000..2503517 --- /dev/null +++ b/pkg/api/presenters/adapter_status_test.go @@ -0,0 +1,431 @@ +package presenters + +import ( + "encoding/json" + "testing" + "time" + + . "github.com/onsi/gomega" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" +) + +// Helper function to create test AdapterStatusCreateRequest +func createTestAdapterStatusRequest() *openapi.AdapterStatusCreateRequest { + reason := "TestReason" + message := "Test message" + observedTime := time.Now() + + return &openapi.AdapterStatusCreateRequest{ + Adapter: "test-adapter", + ObservedGeneration: 5, + Conditions: []openapi.ConditionRequest{ + { + Type: "Ready", + Status: openapi.TRUE, + Reason: &reason, + Message: &message, + }, + }, + Data: map[string]map[string]interface{}{ + "section1": {"key": "value"}, + }, + Metadata: &openapi.AdapterStatusBaseMetadata{ + JobName: strPtr("test-job"), + }, + ObservedTime: observedTime, + } +} + +func strPtr(s string) *string { + return &s +} + +// TestConvertAdapterStatus_Complete tests conversion with all fields populated +func TestConvertAdapterStatus_Complete(t *testing.T) { + RegisterTestingT(t) + + req := createTestAdapterStatusRequest() + resourceType := "Cluster" + resourceID := "test-cluster-id" + + result, err := ConvertAdapterStatus(resourceType, resourceID, req) + Expect(err).To(BeNil()) + + // Verify basic fields + Expect(result.ResourceType).To(Equal(resourceType)) + Expect(result.ResourceID).To(Equal(resourceID)) + Expect(result.Adapter).To(Equal("test-adapter")) + Expect(result.ObservedGeneration).To(Equal(int32(5))) + + // Verify Conditions marshaled correctly + var conditions []api.AdapterCondition + err = json.Unmarshal(result.Conditions, &conditions) + Expect(err).To(BeNil()) + Expect(len(conditions)).To(Equal(1)) + Expect(conditions[0].Type).To(Equal("Ready")) + Expect(conditions[0].Status).To(Equal(api.ConditionTrue)) + Expect(*conditions[0].Reason).To(Equal("TestReason")) + Expect(*conditions[0].Message).To(Equal("Test message")) + + // Verify Data marshaled correctly + var data map[string]map[string]interface{} + err = json.Unmarshal(result.Data, &data) + Expect(err).To(BeNil()) + Expect(data["section1"]["key"]).To(Equal("value")) + + // Verify Metadata marshaled correctly + Expect(result.Metadata).ToNot(BeNil()) + var metadata openapi.AdapterStatusBaseMetadata + err = json.Unmarshal(result.Metadata, &metadata) + Expect(err).To(BeNil()) + Expect(*metadata.JobName).To(Equal("test-job")) + + // Verify timestamps + Expect(result.CreatedTime).ToNot(BeNil()) + Expect(result.LastReportTime).ToNot(BeNil()) + Expect(result.CreatedTime).To(Equal(result.LastReportTime)) +} + +// TestConvertAdapterStatus_WithObservedTime tests that ObservedTime is used when provided +func TestConvertAdapterStatus_WithObservedTime(t *testing.T) { + RegisterTestingT(t) + + specificTime := time.Date(2023, 6, 15, 12, 0, 0, 0, time.UTC) + req := &openapi.AdapterStatusCreateRequest{ + Adapter: "test-adapter", + ObservedGeneration: 3, + Conditions: []openapi.ConditionRequest{}, + ObservedTime: specificTime, + } + + result, err := ConvertAdapterStatus("Cluster", "cluster-123", req) + Expect(err).To(BeNil()) + + Expect(result.CreatedTime).ToNot(BeNil()) + Expect(result.LastReportTime).ToNot(BeNil()) + Expect(result.CreatedTime.Unix()).To(Equal(specificTime.Unix())) + Expect(result.LastReportTime.Unix()).To(Equal(specificTime.Unix())) +} + +// TestConvertAdapterStatus_WithoutObservedTime tests that time.Now() is used when ObservedTime is zero +func TestConvertAdapterStatus_WithoutObservedTime(t *testing.T) { + RegisterTestingT(t) + + req := &openapi.AdapterStatusCreateRequest{ + Adapter: "test-adapter", + ObservedGeneration: 2, + Conditions: []openapi.ConditionRequest{}, + ObservedTime: time.Time{}, // Zero time + } + + before := time.Now() + result, err := ConvertAdapterStatus("Cluster", "cluster-456", req) + after := time.Now() + + Expect(err).To(BeNil()) + Expect(result.CreatedTime).ToNot(BeNil()) + Expect(result.LastReportTime).ToNot(BeNil()) + + // Verify time is approximately now (within a few seconds) + Expect(result.CreatedTime.Unix()).To(BeNumerically(">=", before.Unix()-1)) + Expect(result.CreatedTime.Unix()).To(BeNumerically("<=", after.Unix()+1)) +} + +// TestConvertAdapterStatus_EmptyConditions tests conversion with empty conditions array +func TestConvertAdapterStatus_EmptyConditions(t *testing.T) { + RegisterTestingT(t) + + req := &openapi.AdapterStatusCreateRequest{ + Adapter: "test-adapter", + ObservedGeneration: 1, + Conditions: []openapi.ConditionRequest{}, // Empty + } + + result, err := ConvertAdapterStatus("NodePool", "nodepool-789", req) + Expect(err).To(BeNil()) + + var conditions []api.AdapterCondition + err = json.Unmarshal(result.Conditions, &conditions) + Expect(err).To(BeNil()) + Expect(len(conditions)).To(Equal(0)) +} + +// TestConvertAdapterStatus_NilData tests conversion with nil Data field +func TestConvertAdapterStatus_NilData(t *testing.T) { + RegisterTestingT(t) + + req := &openapi.AdapterStatusCreateRequest{ + Adapter: "test-adapter", + ObservedGeneration: 1, + Conditions: []openapi.ConditionRequest{}, + Data: nil, // Nil data + } + + result, err := ConvertAdapterStatus("Cluster", "cluster-000", req) + Expect(err).To(BeNil()) + + var data map[string]map[string]interface{} + err = json.Unmarshal(result.Data, &data) + Expect(err).To(BeNil()) + Expect(len(data)).To(Equal(0)) // Empty map marshaled +} + +// TestConvertAdapterStatus_NilMetadata tests conversion with nil Metadata field +func TestConvertAdapterStatus_NilMetadata(t *testing.T) { + RegisterTestingT(t) + + req := &openapi.AdapterStatusCreateRequest{ + Adapter: "test-adapter", + ObservedGeneration: 1, + Conditions: []openapi.ConditionRequest{}, + Metadata: nil, // Nil metadata + } + + result, err := ConvertAdapterStatus("Cluster", "cluster-111", req) + Expect(err).To(BeNil()) + + // Metadata should be nil or empty + Expect(len(result.Metadata)).To(Equal(0)) +} + +// TestConvertAdapterStatus_ConditionStatusConversion tests status conversion for all values +func TestConvertAdapterStatus_ConditionStatusConversion(t *testing.T) { + RegisterTestingT(t) + + testCases := []struct { + openapiStatus openapi.ConditionStatus + expectedDomain api.ConditionStatus + }{ + {openapi.TRUE, api.ConditionTrue}, + {openapi.FALSE, api.ConditionFalse}, + {openapi.UNKNOWN, api.ConditionUnknown}, + } + + for _, tc := range testCases { + req := &openapi.AdapterStatusCreateRequest{ + Adapter: "test-adapter", + ObservedGeneration: 1, + Conditions: []openapi.ConditionRequest{ + { + Type: "TestCondition", + Status: tc.openapiStatus, + }, + }, + } + + result, err := ConvertAdapterStatus("Cluster", "test-id", req) + Expect(err).To(BeNil()) + + var conditions []api.AdapterCondition + err = json.Unmarshal(result.Conditions, &conditions) + Expect(err).To(BeNil()) + Expect(conditions[0].Status).To(Equal(tc.expectedDomain)) + } +} + +// TestPresentAdapterStatus_Complete tests presentation with all fields +func TestPresentAdapterStatus_Complete(t *testing.T) { + RegisterTestingT(t) + + now := time.Now() + reason := "Success" + message := "All good" + + // Create domain AdapterCondition + conditions := []api.AdapterCondition{ + { + Type: "Ready", + Status: api.ConditionTrue, + Reason: &reason, + Message: &message, + LastTransitionTime: now, + }, + } + conditionsJSON, _ := json.Marshal(conditions) + + data := map[string]map[string]interface{}{ + "metrics": {"cpu": "50%"}, + } + dataJSON, _ := json.Marshal(data) + + metadata := &openapi.AdapterStatusBaseMetadata{ + JobName: strPtr("adapter-job"), + } + metadataJSON, _ := json.Marshal(metadata) + + adapterStatus := &api.AdapterStatus{ + ResourceType: "Cluster", + ResourceID: "cluster-abc", + Adapter: "aws-adapter", + ObservedGeneration: 10, + Conditions: conditionsJSON, + Data: dataJSON, + Metadata: metadataJSON, + CreatedTime: &now, + LastReportTime: &now, + } + + result := PresentAdapterStatus(adapterStatus) + + // Verify basic fields + Expect(result.Adapter).To(Equal("aws-adapter")) + Expect(result.ObservedGeneration).To(Equal(int32(10))) + + // Verify conditions converted correctly + Expect(len(result.Conditions)).To(Equal(1)) + Expect(result.Conditions[0].Type).To(Equal("Ready")) + Expect(result.Conditions[0].Status).To(Equal(openapi.TRUE)) + Expect(*result.Conditions[0].Reason).To(Equal("Success")) + + // Verify data unmarshaled correctly + Expect(result.Data["metrics"]["cpu"]).To(Equal("50%")) + + // Verify metadata unmarshaled correctly + Expect(result.Metadata).ToNot(BeNil()) + Expect(*result.Metadata.JobName).To(Equal("adapter-job")) + + // Verify timestamps + Expect(result.CreatedTime.Unix()).To(Equal(now.Unix())) + Expect(result.LastReportTime.Unix()).To(Equal(now.Unix())) +} + +// TestPresentAdapterStatus_NilTimestamps tests handling of nil timestamps +func TestPresentAdapterStatus_NilTimestamps(t *testing.T) { + RegisterTestingT(t) + + adapterStatus := &api.AdapterStatus{ + ResourceType: "Cluster", + ResourceID: "cluster-xyz", + Adapter: "test-adapter", + ObservedGeneration: 5, + Conditions: []byte("[]"), + Data: []byte("{}"), + CreatedTime: nil, // Nil timestamp + LastReportTime: nil, // Nil timestamp + } + + result := PresentAdapterStatus(adapterStatus) + + // Verify zero time.Time is returned (not nil) + Expect(result.CreatedTime.IsZero()).To(BeTrue()) + Expect(result.LastReportTime.IsZero()).To(BeTrue()) +} + +// TestPresentAdapterStatus_EmptyConditions tests handling of empty conditions JSON +func TestPresentAdapterStatus_EmptyConditions(t *testing.T) { + RegisterTestingT(t) + + now := time.Now() + adapterStatus := &api.AdapterStatus{ + ResourceType: "NodePool", + ResourceID: "nodepool-123", + Adapter: "test-adapter", + ObservedGeneration: 2, + Conditions: []byte("[]"), // Empty array JSON + Data: []byte("{}"), + CreatedTime: &now, + LastReportTime: &now, + } + + result := PresentAdapterStatus(adapterStatus) + + Expect(result.Conditions).ToNot(BeNil()) + Expect(len(result.Conditions)).To(Equal(0)) +} + +// TestPresentAdapterStatus_EmptyData tests handling of empty data JSON +func TestPresentAdapterStatus_EmptyData(t *testing.T) { + RegisterTestingT(t) + + now := time.Now() + adapterStatus := &api.AdapterStatus{ + ResourceType: "Cluster", + ResourceID: "cluster-empty", + Adapter: "test-adapter", + ObservedGeneration: 3, + Conditions: []byte("[]"), + Data: []byte("{}"), // Empty object JSON + CreatedTime: &now, + LastReportTime: &now, + } + + result := PresentAdapterStatus(adapterStatus) + + Expect(result.Data).ToNot(BeNil()) + Expect(len(result.Data)).To(Equal(0)) +} + +// TestPresentAdapterStatus_ConditionStatusConversion tests status conversion from domain to openapi +func TestPresentAdapterStatus_ConditionStatusConversion(t *testing.T) { + RegisterTestingT(t) + + testCases := []struct { + domainStatus api.ConditionStatus + expectedOpenAPI openapi.ConditionStatus + }{ + {api.ConditionTrue, openapi.TRUE}, + {api.ConditionFalse, openapi.FALSE}, + {api.ConditionUnknown, openapi.UNKNOWN}, + } + + now := time.Now() + for _, tc := range testCases { + conditions := []api.AdapterCondition{ + { + Type: "TestCondition", + Status: tc.domainStatus, + LastTransitionTime: now, + }, + } + conditionsJSON, _ := json.Marshal(conditions) + + adapterStatus := &api.AdapterStatus{ + ResourceType: "Cluster", + ResourceID: "test-id", + Adapter: "test-adapter", + ObservedGeneration: 1, + Conditions: conditionsJSON, + Data: []byte("{}"), + CreatedTime: &now, + LastReportTime: &now, + } + + result := PresentAdapterStatus(adapterStatus) + + Expect(len(result.Conditions)).To(Equal(1)) + Expect(result.Conditions[0].Status).To(Equal(tc.expectedOpenAPI)) + } +} + +// TestConvertAndPresent_RoundTrip tests data integrity through convert and present +func TestConvertAndPresent_RoundTrip(t *testing.T) { + RegisterTestingT(t) + + originalReq := createTestAdapterStatusRequest() + + // Convert from OpenAPI request to domain + adapterStatus, err := ConvertAdapterStatus("Cluster", "cluster-roundtrip", originalReq) + Expect(err).To(BeNil()) + + // Present from domain back to OpenAPI + result := PresentAdapterStatus(adapterStatus) + + // Verify data integrity + Expect(result.Adapter).To(Equal(originalReq.Adapter)) + Expect(result.ObservedGeneration).To(Equal(originalReq.ObservedGeneration)) + + // Verify conditions preserved + Expect(len(result.Conditions)).To(Equal(len(originalReq.Conditions))) + Expect(result.Conditions[0].Type).To(Equal(originalReq.Conditions[0].Type)) + Expect(result.Conditions[0].Status).To(Equal(originalReq.Conditions[0].Status)) + Expect(*result.Conditions[0].Reason).To(Equal(*originalReq.Conditions[0].Reason)) + Expect(*result.Conditions[0].Message).To(Equal(*originalReq.Conditions[0].Message)) + + // Verify data preserved + Expect(result.Data["section1"]["key"]).To(Equal(originalReq.Data["section1"]["key"])) + + // Verify metadata preserved + Expect(result.Metadata).ToNot(BeNil()) + Expect(*result.Metadata.JobName).To(Equal(*originalReq.Metadata.JobName)) +} diff --git a/pkg/api/presenters/cluster.go b/pkg/api/presenters/cluster.go index e1b2bc6..01a9507 100644 --- a/pkg/api/presenters/cluster.go +++ b/pkg/api/presenters/cluster.go @@ -1,35 +1,131 @@ package presenters import ( + "encoding/json" + "fmt" + "time" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" ) -// ConvertCluster converts openapi.Cluster to api.Cluster (GORM model) -func ConvertCluster(cluster openapi.Cluster) *api.Cluster { - // Use ClusterFromOpenAPICreate helper - createdBy := cluster.CreatedBy - if createdBy == "" { - createdBy = "system" +// ConvertCluster converts openapi.ClusterCreateRequest to api.Cluster (GORM model) +func ConvertCluster(req *openapi.ClusterCreateRequest, createdBy string) (*api.Cluster, error) { + // Marshal Spec + specJSON, err := json.Marshal(req.Spec) + if err != nil { + return nil, fmt.Errorf("failed to marshal cluster spec: %w", err) + } + + // Marshal Labels + labels := make(map[string]string) + if req.Labels != nil { + labels = *req.Labels + } + labelsJSON, err := json.Marshal(labels) + if err != nil { + return nil, fmt.Errorf("failed to marshal cluster labels: %w", err) } - req := &openapi.ClusterCreateRequest{ - Kind: cluster.Kind, - Name: cluster.Name, - Spec: cluster.Spec, - Labels: cluster.Labels, + // Marshal empty StatusConditions + statusConditionsJSON, err := json.Marshal([]api.ResourceCondition{}) + if err != nil { + return nil, fmt.Errorf("failed to marshal cluster status conditions: %w", err) } - return api.ClusterFromOpenAPICreate(req, createdBy) + return &api.Cluster{ + Kind: req.Kind, + Name: req.Name, + Spec: specJSON, + Labels: labelsJSON, + Generation: 1, + StatusPhase: "NotReady", + StatusObservedGeneration: 0, + StatusConditions: statusConditionsJSON, + CreatedBy: createdBy, + UpdatedBy: createdBy, + }, nil } // PresentCluster converts api.Cluster (GORM model) to openapi.Cluster func PresentCluster(cluster *api.Cluster) openapi.Cluster { - // Use the ToOpenAPI method we implemented - result := cluster.ToOpenAPI() - if result == nil { - // Return empty cluster if conversion fails - return openapi.Cluster{} + // Unmarshal Spec + var spec map[string]interface{} + if len(cluster.Spec) > 0 { + _ = json.Unmarshal(cluster.Spec, &spec) + } + + // Unmarshal Labels + var labels map[string]string + if len(cluster.Labels) > 0 { + _ = json.Unmarshal(cluster.Labels, &labels) + } + + // Unmarshal StatusConditions + var statusConditions []api.ResourceCondition + if len(cluster.StatusConditions) > 0 { + _ = json.Unmarshal(cluster.StatusConditions, &statusConditions) + } + + // Generate Href if not set (fallback) + href := cluster.Href + if href == "" { + href = "/api/hyperfleet/v1/clusters/" + cluster.ID + } + + result := openapi.Cluster{ + Id: &cluster.ID, + Kind: cluster.Kind, + Href: &href, + Name: cluster.Name, + Spec: spec, + Labels: &labels, + Generation: cluster.Generation, + CreatedTime: cluster.CreatedTime, + UpdatedTime: cluster.UpdatedTime, + CreatedBy: cluster.CreatedBy, + UpdatedBy: cluster.UpdatedBy, } - return *result + + // Build ClusterStatus - set required fields with defaults if nil + lastTransitionTime := time.Time{} + if cluster.StatusLastTransitionTime != nil { + lastTransitionTime = *cluster.StatusLastTransitionTime + } + + lastUpdatedTime := time.Time{} + if cluster.StatusLastUpdatedTime != nil { + lastUpdatedTime = *cluster.StatusLastUpdatedTime + } + + // Set phase, use NOT_READY as default if not set + phase := api.PhaseNotReady + if cluster.StatusPhase != "" { + phase = api.ResourcePhase(cluster.StatusPhase) + } + + // Convert domain ResourceConditions to openapi format + openapiConditions := make([]openapi.ResourceCondition, len(statusConditions)) + for i, cond := range statusConditions { + openapiConditions[i] = openapi.ResourceCondition{ + ObservedGeneration: cond.ObservedGeneration, + CreatedTime: cond.CreatedTime, + LastUpdatedTime: cond.LastUpdatedTime, + Type: cond.Type, + Status: openapi.ConditionStatus(string(cond.Status)), + Reason: cond.Reason, + Message: cond.Message, + LastTransitionTime: cond.LastTransitionTime, + } + } + + result.Status = openapi.ClusterStatus{ + Phase: openapi.ResourcePhase(string(phase)), + ObservedGeneration: cluster.StatusObservedGeneration, + Conditions: openapiConditions, + LastTransitionTime: lastTransitionTime, + LastUpdatedTime: lastUpdatedTime, + } + + return result } diff --git a/pkg/api/presenters/cluster_test.go b/pkg/api/presenters/cluster_test.go new file mode 100644 index 0000000..bd93699 --- /dev/null +++ b/pkg/api/presenters/cluster_test.go @@ -0,0 +1,400 @@ +package presenters + +import ( + "encoding/json" + "testing" + "time" + + . "github.com/onsi/gomega" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" +) + +// Helper function to create test ClusterCreateRequest +func createTestClusterRequest() *openapi.ClusterCreateRequest { + labels := map[string]string{"env": "test"} + + return &openapi.ClusterCreateRequest{ + Kind: "Cluster", + Name: "test-cluster", + Spec: map[string]interface{}{ + "region": "us-central1", + "provider": "gcp", + }, + Labels: &labels, + } +} + +// TestConvertCluster_Complete tests conversion with all fields populated +func TestConvertCluster_Complete(t *testing.T) { + RegisterTestingT(t) + + req := createTestClusterRequest() + createdBy := "user123" + + result, err := ConvertCluster(req, createdBy) + Expect(err).To(BeNil()) + + // Verify basic fields + Expect(result.Kind).To(Equal("Cluster")) + Expect(result.Name).To(Equal("test-cluster")) + Expect(result.CreatedBy).To(Equal("user123")) + Expect(result.UpdatedBy).To(Equal("user123")) + + // Verify defaults + Expect(result.Generation).To(Equal(int32(1))) + Expect(result.StatusPhase).To(Equal("NotReady")) + Expect(result.StatusObservedGeneration).To(Equal(int32(0))) + + // Verify Spec marshaled correctly + var spec map[string]interface{} + err = json.Unmarshal(result.Spec, &spec) + Expect(err).To(BeNil()) + Expect(spec["region"]).To(Equal("us-central1")) + Expect(spec["provider"]).To(Equal("gcp")) + + // Verify Labels marshaled correctly + var labels map[string]string + err = json.Unmarshal(result.Labels, &labels) + Expect(err).To(BeNil()) + Expect(labels["env"]).To(Equal("test")) + + // Verify StatusConditions initialized as empty array + var conditions []api.ResourceCondition + err = json.Unmarshal(result.StatusConditions, &conditions) + Expect(err).To(BeNil()) + Expect(len(conditions)).To(Equal(0)) +} + +// TestConvertCluster_WithLabels tests conversion with labels +func TestConvertCluster_WithLabels(t *testing.T) { + RegisterTestingT(t) + + labels := map[string]string{ + "env": "production", + "team": "platform", + } + + req := &openapi.ClusterCreateRequest{ + Kind: "Cluster", + Name: "labeled-cluster", + Spec: map[string]interface{}{"test": "spec"}, + Labels: &labels, + } + + result, err := ConvertCluster(req, "user456") + Expect(err).To(BeNil()) + + var resultLabels map[string]string + json.Unmarshal(result.Labels, &resultLabels) + Expect(resultLabels["env"]).To(Equal("production")) + Expect(resultLabels["team"]).To(Equal("platform")) +} + +// TestConvertCluster_WithoutLabels tests conversion with nil labels +func TestConvertCluster_WithoutLabels(t *testing.T) { + RegisterTestingT(t) + + req := &openapi.ClusterCreateRequest{ + Kind: "Cluster", + Name: "unlabeled-cluster", + Spec: map[string]interface{}{"test": "spec"}, + Labels: nil, // Nil labels + } + + result, err := ConvertCluster(req, "user789") + Expect(err).To(BeNil()) + + var resultLabels map[string]string + json.Unmarshal(result.Labels, &resultLabels) + Expect(len(resultLabels)).To(Equal(0)) // Empty map +} + +// TestConvertCluster_SpecMarshaling tests complex spec with nested objects +func TestConvertCluster_SpecMarshaling(t *testing.T) { + RegisterTestingT(t) + + complexSpec := map[string]interface{}{ + "provider": "gcp", + "region": "us-east1", + "config": map[string]interface{}{ + "nodes": 3, + "networking": map[string]interface{}{ + "cidr": "10.0.0.0/16", + }, + }, + "tags": []string{"production", "critical"}, + } + + req := &openapi.ClusterCreateRequest{ + Kind: "Cluster", + Name: "complex-cluster", + Spec: complexSpec, + } + + result, err := ConvertCluster(req, "user000") + Expect(err).To(BeNil()) + + var resultSpec map[string]interface{} + json.Unmarshal(result.Spec, &resultSpec) + Expect(resultSpec["provider"]).To(Equal("gcp")) + Expect(resultSpec["region"]).To(Equal("us-east1")) + + // Verify nested config + config := resultSpec["config"].(map[string]interface{}) + Expect(config["nodes"]).To(BeNumerically("==", 3)) + + networking := config["networking"].(map[string]interface{}) + Expect(networking["cidr"]).To(Equal("10.0.0.0/16")) + + // Verify tags array + tags := resultSpec["tags"].([]interface{}) + Expect(len(tags)).To(Equal(2)) +} + +// TestPresentCluster_Complete tests presentation with all fields +func TestPresentCluster_Complete(t *testing.T) { + RegisterTestingT(t) + + now := time.Now() + reason := "Ready" + message := "Cluster is ready" + + // Create domain ResourceCondition + conditions := []api.ResourceCondition{ + { + ObservedGeneration: 5, + CreatedTime: now, + LastUpdatedTime: now, + Type: "Available", + Status: api.ConditionTrue, + Reason: &reason, + Message: &message, + LastTransitionTime: now, + }, + } + conditionsJSON, _ := json.Marshal(conditions) + + spec := map[string]interface{}{"region": "us-west1"} + specJSON, _ := json.Marshal(spec) + + labels := map[string]string{"env": "staging"} + labelsJSON, _ := json.Marshal(labels) + + cluster := &api.Cluster{ + Kind: "Cluster", + Href: "/api/hyperfleet/v1/clusters/cluster-abc123", + Name: "presented-cluster", + Spec: specJSON, + Labels: labelsJSON, + Generation: 10, + StatusPhase: "Ready", + StatusObservedGeneration: 5, + StatusConditions: conditionsJSON, + StatusLastTransitionTime: &now, + StatusLastUpdatedTime: &now, + CreatedBy: "user123", + UpdatedBy: "user456", + } + cluster.ID = "cluster-abc123" + cluster.CreatedTime = now + cluster.UpdatedTime = now + + result := PresentCluster(cluster) + + // Verify basic fields + Expect(*result.Id).To(Equal("cluster-abc123")) + Expect(result.Kind).To(Equal("Cluster")) + Expect(*result.Href).To(Equal("/api/hyperfleet/v1/clusters/cluster-abc123")) + Expect(result.Name).To(Equal("presented-cluster")) + Expect(result.Generation).To(Equal(int32(10))) + Expect(result.CreatedBy).To(Equal("user123")) + Expect(result.UpdatedBy).To(Equal("user456")) + + // Verify Spec unmarshaled correctly + Expect(result.Spec["region"]).To(Equal("us-west1")) + + // Verify Labels unmarshaled correctly + Expect((*result.Labels)["env"]).To(Equal("staging")) + + // Verify Status + Expect(result.Status.Phase).To(Equal(openapi.READY)) + Expect(result.Status.ObservedGeneration).To(Equal(int32(5))) + Expect(len(result.Status.Conditions)).To(Equal(1)) + Expect(result.Status.Conditions[0].Type).To(Equal("Available")) + Expect(result.Status.Conditions[0].Status).To(Equal(openapi.TRUE)) + Expect(*result.Status.Conditions[0].Reason).To(Equal("Ready")) + + // Verify timestamps + Expect(result.CreatedTime.Unix()).To(Equal(now.Unix())) + Expect(result.UpdatedTime.Unix()).To(Equal(now.Unix())) + Expect(result.Status.LastTransitionTime.Unix()).To(Equal(now.Unix())) + Expect(result.Status.LastUpdatedTime.Unix()).To(Equal(now.Unix())) +} + +// TestPresentCluster_HrefGeneration tests that Href is generated if not set +func TestPresentCluster_HrefGeneration(t *testing.T) { + RegisterTestingT(t) + + cluster := &api.Cluster{ + Kind: "Cluster", + Href: "", // Empty Href + Name: "href-test", + Spec: []byte("{}"), + Labels: []byte("{}"), + StatusConditions: []byte("[]"), + } + cluster.ID = "cluster-xyz789" + + result := PresentCluster(cluster) + + Expect(*result.Href).To(Equal("/api/hyperfleet/v1/clusters/cluster-xyz789")) +} + +// TestPresentCluster_EmptyStatusPhase tests handling of empty StatusPhase +func TestPresentCluster_EmptyStatusPhase(t *testing.T) { + RegisterTestingT(t) + + cluster := &api.Cluster{ + Kind: "Cluster", + Name: "empty-phase-test", + Spec: []byte("{}"), + Labels: []byte("{}"), + StatusPhase: "", // Empty status phase + StatusConditions: []byte("[]"), + } + cluster.ID = "cluster-empty-phase" + + result := PresentCluster(cluster) + + // Should use NOT_READY as default + Expect(result.Status.Phase).To(Equal(openapi.NOT_READY)) +} + +// TestPresentCluster_NilStatusTimestamps tests handling of nil timestamps +func TestPresentCluster_NilStatusTimestamps(t *testing.T) { + RegisterTestingT(t) + + now := time.Now() + cluster := &api.Cluster{ + Kind: "Cluster", + Name: "nil-timestamps-test", + Spec: []byte("{}"), + Labels: []byte("{}"), + StatusConditions: []byte("[]"), + StatusLastTransitionTime: nil, // Nil timestamp + StatusLastUpdatedTime: nil, // Nil timestamp + } + cluster.ID = "cluster-nil-timestamps" + cluster.CreatedTime = now + cluster.UpdatedTime = now + + result := PresentCluster(cluster) + + // Verify zero time.Time is returned (not nil) + Expect(result.Status.LastTransitionTime.IsZero()).To(BeTrue()) + Expect(result.Status.LastUpdatedTime.IsZero()).To(BeTrue()) +} + +// TestPresentCluster_StatusConditionsConversion tests condition conversion +func TestPresentCluster_StatusConditionsConversion(t *testing.T) { + RegisterTestingT(t) + + now := time.Now() + reason1 := "Ready" + message1 := "All systems operational" + reason2 := "Degraded" + message2 := "Some components unavailable" + + // Create multiple domain ResourceConditions + conditions := []api.ResourceCondition{ + { + ObservedGeneration: 3, + CreatedTime: now, + LastUpdatedTime: now, + Type: "Available", + Status: api.ConditionTrue, + Reason: &reason1, + Message: &message1, + LastTransitionTime: now, + }, + { + ObservedGeneration: 3, + CreatedTime: now, + LastUpdatedTime: now, + Type: "Progressing", + Status: api.ConditionFalse, + Reason: &reason2, + Message: &message2, + LastTransitionTime: now, + }, + } + conditionsJSON, _ := json.Marshal(conditions) + + cluster := &api.Cluster{ + Kind: "Cluster", + Name: "multi-conditions-test", + Spec: []byte("{}"), + Labels: []byte("{}"), + StatusConditions: conditionsJSON, + } + cluster.ID = "cluster-multi-conditions" + cluster.CreatedTime = now + cluster.UpdatedTime = now + + result := PresentCluster(cluster) + + // Verify both conditions converted correctly + Expect(len(result.Status.Conditions)).To(Equal(2)) + + // First condition + Expect(result.Status.Conditions[0].Type).To(Equal("Available")) + Expect(result.Status.Conditions[0].Status).To(Equal(openapi.TRUE)) + Expect(*result.Status.Conditions[0].Reason).To(Equal("Ready")) + Expect(*result.Status.Conditions[0].Message).To(Equal("All systems operational")) + + // Second condition + Expect(result.Status.Conditions[1].Type).To(Equal("Progressing")) + Expect(result.Status.Conditions[1].Status).To(Equal(openapi.FALSE)) + Expect(*result.Status.Conditions[1].Reason).To(Equal("Degraded")) + Expect(*result.Status.Conditions[1].Message).To(Equal("Some components unavailable")) +} + +// TestConvertAndPresentCluster_RoundTrip tests data integrity through convert and present +func TestConvertAndPresentCluster_RoundTrip(t *testing.T) { + RegisterTestingT(t) + + originalReq := createTestClusterRequest() + + // Convert from OpenAPI request to domain + cluster, err := ConvertCluster(originalReq, "user999") + Expect(err).To(BeNil()) + + // Simulate database fields (ID, timestamps) + cluster.ID = "cluster-roundtrip-123" + now := time.Now() + cluster.CreatedTime = now + cluster.UpdatedTime = now + + // Present from domain back to OpenAPI + result := PresentCluster(cluster) + + // Verify data integrity + Expect(*result.Id).To(Equal("cluster-roundtrip-123")) + Expect(result.Kind).To(Equal(originalReq.Kind)) + Expect(result.Name).To(Equal(originalReq.Name)) + Expect(result.CreatedBy).To(Equal("user999")) + Expect(result.UpdatedBy).To(Equal("user999")) + + // Verify Spec preserved + Expect(result.Spec["region"]).To(Equal(originalReq.Spec["region"])) + Expect(result.Spec["provider"]).To(Equal(originalReq.Spec["provider"])) + + // Verify Labels preserved + Expect((*result.Labels)["env"]).To(Equal((*originalReq.Labels)["env"])) + + // Verify Status defaults + Expect(result.Status.Phase).To(Equal(openapi.NOT_READY)) + Expect(result.Status.ObservedGeneration).To(Equal(int32(0))) + Expect(len(result.Status.Conditions)).To(Equal(0)) +} diff --git a/pkg/api/presenters/node_pool.go b/pkg/api/presenters/node_pool.go index b33df17..f9f005f 100644 --- a/pkg/api/presenters/node_pool.go +++ b/pkg/api/presenters/node_pool.go @@ -1,40 +1,148 @@ package presenters import ( + "encoding/json" + "fmt" + "time" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" ) -// ConvertNodePool converts openapi.NodePool to api.NodePool (GORM model) -func ConvertNodePool(nodePool openapi.NodePool) *api.NodePool { - // Use NodePoolFromOpenAPICreate helper - createdBy := nodePool.CreatedBy - if createdBy == "" { - createdBy = "system" +// ConvertNodePool converts openapi.NodePoolCreateRequest to api.NodePool (GORM model) +func ConvertNodePool(req *openapi.NodePoolCreateRequest, ownerID, createdBy string) (*api.NodePool, error) { + // Marshal Spec + specJSON, err := json.Marshal(req.Spec) + if err != nil { + return nil, fmt.Errorf("failed to marshal nodepool spec: %w", err) } - ownerID := "" - if nodePool.OwnerReferences.Id != nil { - ownerID = *nodePool.OwnerReferences.Id + // Marshal Labels + labels := make(map[string]string) + if req.Labels != nil { + labels = *req.Labels + } + labelsJSON, err := json.Marshal(labels) + if err != nil { + return nil, fmt.Errorf("failed to marshal nodepool labels: %w", err) } - req := &openapi.NodePoolCreateRequest{ - Kind: nodePool.Kind, - Name: nodePool.Name, - Spec: nodePool.Spec, - Labels: nodePool.Labels, + // Marshal empty StatusConditions + statusConditionsJSON, err := json.Marshal([]api.ResourceCondition{}) + if err != nil { + return nil, fmt.Errorf("failed to marshal nodepool status conditions: %w", err) } - return api.NodePoolFromOpenAPICreate(req, ownerID, createdBy) + kind := "NodePool" + if req.Kind != nil { + kind = *req.Kind + } + + return &api.NodePool{ + Kind: kind, + Name: req.Name, + Spec: specJSON, + Labels: labelsJSON, + OwnerID: ownerID, + OwnerKind: "Cluster", + StatusPhase: "NotReady", + StatusObservedGeneration: 0, + StatusConditions: statusConditionsJSON, + CreatedBy: createdBy, + UpdatedBy: createdBy, + }, nil } // PresentNodePool converts api.NodePool (GORM model) to openapi.NodePool func PresentNodePool(nodePool *api.NodePool) openapi.NodePool { - // Use the ToOpenAPI method we implemented - result := nodePool.ToOpenAPI() - if result == nil { - // Return empty nodePool if conversion fails - return openapi.NodePool{} + // Unmarshal Spec + var spec map[string]interface{} + if len(nodePool.Spec) > 0 { + _ = json.Unmarshal(nodePool.Spec, &spec) + } + + // Unmarshal Labels + var labels map[string]string + if len(nodePool.Labels) > 0 { + _ = json.Unmarshal(nodePool.Labels, &labels) } - return *result + + // Unmarshal StatusConditions + var statusConditions []api.ResourceCondition + if len(nodePool.StatusConditions) > 0 { + _ = json.Unmarshal(nodePool.StatusConditions, &statusConditions) + } + + // Generate Href if not set (fallback) + href := nodePool.Href + if href == "" { + href = fmt.Sprintf("/api/hyperfleet/v1/clusters/%s/nodepools/%s", nodePool.OwnerID, nodePool.ID) + } + + // Generate OwnerHref if not set (fallback) + ownerHref := nodePool.OwnerHref + if ownerHref == "" { + ownerHref = "/api/hyperfleet/v1/clusters/" + nodePool.OwnerID + } + + kind := nodePool.Kind + result := openapi.NodePool{ + Id: &nodePool.ID, + Kind: &kind, + Href: &href, + Name: nodePool.Name, + Spec: spec, + Labels: &labels, + OwnerReferences: openapi.ObjectReference{ + Id: &nodePool.OwnerID, + Kind: &nodePool.OwnerKind, + Href: &ownerHref, + }, + CreatedTime: nodePool.CreatedTime, + UpdatedTime: nodePool.UpdatedTime, + CreatedBy: nodePool.CreatedBy, + UpdatedBy: nodePool.UpdatedBy, + } + + // Build NodePoolStatus - set required fields with defaults if nil + lastTransitionTime := time.Time{} + if nodePool.StatusLastTransitionTime != nil { + lastTransitionTime = *nodePool.StatusLastTransitionTime + } + + lastUpdatedTime := time.Time{} + if nodePool.StatusLastUpdatedTime != nil { + lastUpdatedTime = *nodePool.StatusLastUpdatedTime + } + + // Set phase, use NOT_READY as default if not set + phase := api.PhaseNotReady + if nodePool.StatusPhase != "" { + phase = api.ResourcePhase(nodePool.StatusPhase) + } + + // Convert domain ResourceConditions to openapi format + openapiConditions := make([]openapi.ResourceCondition, len(statusConditions)) + for i, cond := range statusConditions { + openapiConditions[i] = openapi.ResourceCondition{ + ObservedGeneration: cond.ObservedGeneration, + CreatedTime: cond.CreatedTime, + LastUpdatedTime: cond.LastUpdatedTime, + Type: cond.Type, + Status: openapi.ConditionStatus(string(cond.Status)), + Reason: cond.Reason, + Message: cond.Message, + LastTransitionTime: cond.LastTransitionTime, + } + } + + result.Status = openapi.NodePoolStatus{ + Phase: openapi.ResourcePhase(string(phase)), + ObservedGeneration: nodePool.StatusObservedGeneration, + Conditions: openapiConditions, + LastTransitionTime: lastTransitionTime, + LastUpdatedTime: lastUpdatedTime, + } + + return result } diff --git a/pkg/api/presenters/node_pool_test.go b/pkg/api/presenters/node_pool_test.go new file mode 100644 index 0000000..f1d1d2f --- /dev/null +++ b/pkg/api/presenters/node_pool_test.go @@ -0,0 +1,428 @@ +package presenters + +import ( + "encoding/json" + "testing" + "time" + + . "github.com/onsi/gomega" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" +) + +// Helper function to create test NodePoolCreateRequest +func createTestNodePoolRequest() *openapi.NodePoolCreateRequest { + labels := map[string]string{"env": "test"} + kind := "NodePool" + + return &openapi.NodePoolCreateRequest{ + Kind: &kind, + Name: "test-nodepool", + Spec: map[string]interface{}{ + "replicas": 3, + "instanceType": "n1-standard-4", + }, + Labels: &labels, + } +} + +// TestConvertNodePool_Complete tests conversion with all fields populated +func TestConvertNodePool_Complete(t *testing.T) { + RegisterTestingT(t) + + req := createTestNodePoolRequest() + ownerID := "cluster-owner-123" + createdBy := "user456" + + result, err := ConvertNodePool(req, ownerID, createdBy) + Expect(err).To(BeNil()) + + // Verify basic fields + Expect(result.Kind).To(Equal("NodePool")) + Expect(result.Name).To(Equal("test-nodepool")) + Expect(result.OwnerID).To(Equal("cluster-owner-123")) + Expect(result.OwnerKind).To(Equal("Cluster")) + Expect(result.CreatedBy).To(Equal("user456")) + Expect(result.UpdatedBy).To(Equal("user456")) + + // Verify defaults + Expect(result.StatusPhase).To(Equal("NotReady")) + Expect(result.StatusObservedGeneration).To(Equal(int32(0))) + + // Verify Spec marshaled correctly + var spec map[string]interface{} + err = json.Unmarshal(result.Spec, &spec) + Expect(err).To(BeNil()) + Expect(spec["replicas"]).To(BeNumerically("==", 3)) + Expect(spec["instanceType"]).To(Equal("n1-standard-4")) + + // Verify Labels marshaled correctly + var labels map[string]string + err = json.Unmarshal(result.Labels, &labels) + Expect(err).To(BeNil()) + Expect(labels["env"]).To(Equal("test")) + + // Verify StatusConditions initialized as empty array + var conditions []api.ResourceCondition + err = json.Unmarshal(result.StatusConditions, &conditions) + Expect(err).To(BeNil()) + Expect(len(conditions)).To(Equal(0)) +} + +// TestConvertNodePool_WithKind tests conversion with Kind specified +func TestConvertNodePool_WithKind(t *testing.T) { + RegisterTestingT(t) + + customKind := "CustomNodePool" + req := &openapi.NodePoolCreateRequest{ + Kind: &customKind, + Name: "custom-nodepool", + Spec: map[string]interface{}{"test": "spec"}, + Labels: nil, + } + + result, err := ConvertNodePool(req, "cluster-123", "user789") + Expect(err).To(BeNil()) + + Expect(result.Kind).To(Equal("CustomNodePool")) +} + +// TestConvertNodePool_WithoutKind tests conversion with nil Kind (uses default) +func TestConvertNodePool_WithoutKind(t *testing.T) { + RegisterTestingT(t) + + req := &openapi.NodePoolCreateRequest{ + Kind: nil, // Nil Kind + Name: "default-kind-nodepool", + Spec: map[string]interface{}{"test": "spec"}, + Labels: nil, + } + + result, err := ConvertNodePool(req, "cluster-456", "user000") + Expect(err).To(BeNil()) + + Expect(result.Kind).To(Equal("NodePool")) // Default value +} + +// TestConvertNodePool_WithLabels tests conversion with labels +func TestConvertNodePool_WithLabels(t *testing.T) { + RegisterTestingT(t) + + labels := map[string]string{ + "environment": "production", + "team": "platform", + "region": "us-east", + } + + req := &openapi.NodePoolCreateRequest{ + Name: "labeled-nodepool", + Spec: map[string]interface{}{"test": "spec"}, + Labels: &labels, + } + + result, err := ConvertNodePool(req, "cluster-789", "user111") + Expect(err).To(BeNil()) + + var resultLabels map[string]string + json.Unmarshal(result.Labels, &resultLabels) + Expect(resultLabels["environment"]).To(Equal("production")) + Expect(resultLabels["team"]).To(Equal("platform")) + Expect(resultLabels["region"]).To(Equal("us-east")) +} + +// TestConvertNodePool_WithoutLabels tests conversion with nil labels +func TestConvertNodePool_WithoutLabels(t *testing.T) { + RegisterTestingT(t) + + req := &openapi.NodePoolCreateRequest{ + Name: "unlabeled-nodepool", + Spec: map[string]interface{}{"test": "spec"}, + Labels: nil, // Nil labels + } + + result, err := ConvertNodePool(req, "cluster-xyz", "user222") + Expect(err).To(BeNil()) + + var resultLabels map[string]string + json.Unmarshal(result.Labels, &resultLabels) + Expect(len(resultLabels)).To(Equal(0)) // Empty map +} + +// TestPresentNodePool_Complete tests presentation with all fields +func TestPresentNodePool_Complete(t *testing.T) { + RegisterTestingT(t) + + now := time.Now() + reason := "Ready" + message := "NodePool is ready" + + // Create domain ResourceCondition + conditions := []api.ResourceCondition{ + { + ObservedGeneration: 5, + CreatedTime: now, + LastUpdatedTime: now, + Type: "Available", + Status: api.ConditionTrue, + Reason: &reason, + Message: &message, + LastTransitionTime: now, + }, + } + conditionsJSON, _ := json.Marshal(conditions) + + spec := map[string]interface{}{"replicas": 5} + specJSON, _ := json.Marshal(spec) + + labels := map[string]string{"env": "staging"} + labelsJSON, _ := json.Marshal(labels) + + nodePool := &api.NodePool{ + Kind: "NodePool", + Href: "/api/hyperfleet/v1/clusters/cluster-abc/nodepools/nodepool-xyz", + Name: "presented-nodepool", + Spec: specJSON, + Labels: labelsJSON, + OwnerID: "cluster-abc", + OwnerKind: "Cluster", + OwnerHref: "/api/hyperfleet/v1/clusters/cluster-abc", + StatusPhase: "Ready", + StatusObservedGeneration: 5, + StatusConditions: conditionsJSON, + StatusLastTransitionTime: &now, + StatusLastUpdatedTime: &now, + CreatedBy: "user123", + UpdatedBy: "user456", + } + nodePool.ID = "nodepool-xyz" + nodePool.CreatedTime = now + nodePool.UpdatedTime = now + + result := PresentNodePool(nodePool) + + // Verify basic fields + Expect(*result.Id).To(Equal("nodepool-xyz")) + Expect(*result.Kind).To(Equal("NodePool")) + Expect(*result.Href).To(Equal("/api/hyperfleet/v1/clusters/cluster-abc/nodepools/nodepool-xyz")) + Expect(result.Name).To(Equal("presented-nodepool")) + Expect(result.CreatedBy).To(Equal("user123")) + Expect(result.UpdatedBy).To(Equal("user456")) + + // Verify Spec unmarshaled correctly + Expect(result.Spec["replicas"]).To(BeNumerically("==", 5)) + + // Verify Labels unmarshaled correctly + Expect((*result.Labels)["env"]).To(Equal("staging")) + + // Verify OwnerReferences + Expect(*result.OwnerReferences.Id).To(Equal("cluster-abc")) + Expect(*result.OwnerReferences.Kind).To(Equal("Cluster")) + Expect(*result.OwnerReferences.Href).To(Equal("/api/hyperfleet/v1/clusters/cluster-abc")) + + // Verify Status + Expect(result.Status.Phase).To(Equal(openapi.READY)) + Expect(result.Status.ObservedGeneration).To(Equal(int32(5))) + Expect(len(result.Status.Conditions)).To(Equal(1)) + Expect(result.Status.Conditions[0].Type).To(Equal("Available")) + Expect(result.Status.Conditions[0].Status).To(Equal(openapi.TRUE)) + + // Verify timestamps + Expect(result.CreatedTime.Unix()).To(Equal(now.Unix())) + Expect(result.UpdatedTime.Unix()).To(Equal(now.Unix())) + Expect(result.Status.LastTransitionTime.Unix()).To(Equal(now.Unix())) + Expect(result.Status.LastUpdatedTime.Unix()).To(Equal(now.Unix())) +} + +// TestPresentNodePool_HrefGeneration tests that Href is generated if not set +func TestPresentNodePool_HrefGeneration(t *testing.T) { + RegisterTestingT(t) + + nodePool := &api.NodePool{ + Kind: "NodePool", + Href: "", // Empty Href + Name: "href-test", + Spec: []byte("{}"), + Labels: []byte("{}"), + OwnerID: "cluster-owner-456", + StatusConditions: []byte("[]"), + } + nodePool.ID = "nodepool-test-123" + + result := PresentNodePool(nodePool) + + Expect(*result.Href).To(Equal("/api/hyperfleet/v1/clusters/cluster-owner-456/nodepools/nodepool-test-123")) +} + +// TestPresentNodePool_OwnerHrefGeneration tests that OwnerHref is generated if not set +func TestPresentNodePool_OwnerHrefGeneration(t *testing.T) { + RegisterTestingT(t) + + nodePool := &api.NodePool{ + Kind: "NodePool", + Name: "owner-href-test", + Spec: []byte("{}"), + Labels: []byte("{}"), + OwnerID: "cluster-owner-789", + OwnerHref: "", // Empty OwnerHref + StatusConditions: []byte("[]"), + } + nodePool.ID = "nodepool-owner-test" + + result := PresentNodePool(nodePool) + + Expect(*result.OwnerReferences.Href).To(Equal("/api/hyperfleet/v1/clusters/cluster-owner-789")) +} + +// TestPresentNodePool_OwnerReferences tests OwnerReferences are set correctly +func TestPresentNodePool_OwnerReferences(t *testing.T) { + RegisterTestingT(t) + + nodePool := &api.NodePool{ + Kind: "NodePool", + Name: "owner-ref-test", + Spec: []byte("{}"), + Labels: []byte("{}"), + OwnerID: "cluster-ref-123", + OwnerKind: "Cluster", + StatusConditions: []byte("[]"), + } + nodePool.ID = "nodepool-ref-456" + + result := PresentNodePool(nodePool) + + Expect(result.OwnerReferences.Id).ToNot(BeNil()) + Expect(*result.OwnerReferences.Id).To(Equal("cluster-ref-123")) + Expect(result.OwnerReferences.Kind).ToNot(BeNil()) + Expect(*result.OwnerReferences.Kind).To(Equal("Cluster")) + Expect(result.OwnerReferences.Href).ToNot(BeNil()) +} + +// TestPresentNodePool_EmptyStatusPhase tests handling of empty StatusPhase +func TestPresentNodePool_EmptyStatusPhase(t *testing.T) { + RegisterTestingT(t) + + nodePool := &api.NodePool{ + Kind: "NodePool", + Name: "empty-phase-test", + Spec: []byte("{}"), + Labels: []byte("{}"), + OwnerID: "cluster-phase-test", + StatusPhase: "", // Empty status phase + StatusConditions: []byte("[]"), + } + nodePool.ID = "nodepool-empty-phase" + + result := PresentNodePool(nodePool) + + // Should use NOT_READY as default + Expect(result.Status.Phase).To(Equal(openapi.NOT_READY)) +} + +// TestPresentNodePool_StatusConditionsConversion tests condition conversion +func TestPresentNodePool_StatusConditionsConversion(t *testing.T) { + RegisterTestingT(t) + + now := time.Now() + reason1 := "Scaling" + message1 := "Scaling in progress" + reason2 := "Healthy" + message2 := "All nodes healthy" + + // Create multiple domain ResourceConditions + conditions := []api.ResourceCondition{ + { + ObservedGeneration: 2, + CreatedTime: now, + LastUpdatedTime: now, + Type: "Progressing", + Status: api.ConditionTrue, + Reason: &reason1, + Message: &message1, + LastTransitionTime: now, + }, + { + ObservedGeneration: 2, + CreatedTime: now, + LastUpdatedTime: now, + Type: "Healthy", + Status: api.ConditionTrue, + Reason: &reason2, + Message: &message2, + LastTransitionTime: now, + }, + } + conditionsJSON, _ := json.Marshal(conditions) + + nodePool := &api.NodePool{ + Kind: "NodePool", + Name: "multi-conditions-test", + Spec: []byte("{}"), + Labels: []byte("{}"), + OwnerID: "cluster-conditions", + StatusConditions: conditionsJSON, + } + nodePool.ID = "nodepool-multi-conditions" + nodePool.CreatedTime = now + nodePool.UpdatedTime = now + + result := PresentNodePool(nodePool) + + // Verify both conditions converted correctly + Expect(len(result.Status.Conditions)).To(Equal(2)) + + // First condition + Expect(result.Status.Conditions[0].Type).To(Equal("Progressing")) + Expect(result.Status.Conditions[0].Status).To(Equal(openapi.TRUE)) + Expect(*result.Status.Conditions[0].Reason).To(Equal("Scaling")) + Expect(*result.Status.Conditions[0].Message).To(Equal("Scaling in progress")) + + // Second condition + Expect(result.Status.Conditions[1].Type).To(Equal("Healthy")) + Expect(result.Status.Conditions[1].Status).To(Equal(openapi.TRUE)) + Expect(*result.Status.Conditions[1].Reason).To(Equal("Healthy")) + Expect(*result.Status.Conditions[1].Message).To(Equal("All nodes healthy")) +} + +// TestConvertAndPresentNodePool_RoundTrip tests data integrity through convert and present +func TestConvertAndPresentNodePool_RoundTrip(t *testing.T) { + RegisterTestingT(t) + + originalReq := createTestNodePoolRequest() + ownerID := "cluster-roundtrip-789" + + // Convert from OpenAPI request to domain + nodePool, err := ConvertNodePool(originalReq, ownerID, "user-roundtrip") + Expect(err).To(BeNil()) + + // Simulate database fields (ID, timestamps) + nodePool.ID = "nodepool-roundtrip-123" + now := time.Now() + nodePool.CreatedTime = now + nodePool.UpdatedTime = now + + // Present from domain back to OpenAPI + result := PresentNodePool(nodePool) + + // Verify data integrity + Expect(*result.Id).To(Equal("nodepool-roundtrip-123")) + Expect(*result.Kind).To(Equal(*originalReq.Kind)) + Expect(result.Name).To(Equal(originalReq.Name)) + Expect(result.CreatedBy).To(Equal("user-roundtrip")) + Expect(result.UpdatedBy).To(Equal("user-roundtrip")) + + // Verify Spec preserved + Expect(result.Spec["replicas"]).To(BeNumerically("==", originalReq.Spec["replicas"])) + Expect(result.Spec["instanceType"]).To(Equal(originalReq.Spec["instanceType"])) + + // Verify Labels preserved + Expect((*result.Labels)["env"]).To(Equal((*originalReq.Labels)["env"])) + + // Verify OwnerReferences set + Expect(*result.OwnerReferences.Id).To(Equal(ownerID)) + Expect(*result.OwnerReferences.Kind).To(Equal("Cluster")) + + // Verify Status defaults + Expect(result.Status.Phase).To(Equal(openapi.NOT_READY)) + Expect(result.Status.ObservedGeneration).To(Equal(int32(0))) + Expect(len(result.Status.Conditions)).To(Equal(0)) +} diff --git a/pkg/api/status_types.go b/pkg/api/status_types.go new file mode 100644 index 0000000..afd8ccf --- /dev/null +++ b/pkg/api/status_types.go @@ -0,0 +1,48 @@ +package api + +import "time" + +// ResourcePhase represents the lifecycle phase of a resource +// Domain equivalent of openapi.ResourcePhase +type ResourcePhase string + +const ( + PhaseNotReady ResourcePhase = "NotReady" // String value matches openapi.NOT_READY + PhaseReady ResourcePhase = "Ready" // String value matches openapi.READY + PhaseFailed ResourcePhase = "Failed" // String value matches openapi.FAILED +) + +// ConditionStatus represents the status of a condition +// Domain equivalent of openapi.ConditionStatus +type ConditionStatus string + +const ( + ConditionTrue ConditionStatus = "True" // String value matches openapi.TRUE + ConditionFalse ConditionStatus = "False" // String value matches openapi.FALSE + ConditionUnknown ConditionStatus = "Unknown" // String value matches openapi.UNKNOWN +) + +// ResourceCondition represents a condition of a resource +// Domain equivalent of openapi.ResourceCondition +// JSON tags match database JSONB structure +type ResourceCondition struct { + ObservedGeneration int32 `json:"observed_generation"` + CreatedTime time.Time `json:"created_time"` + LastUpdatedTime time.Time `json:"last_updated_time"` + Type string `json:"type"` + Status ConditionStatus `json:"status"` + Reason *string `json:"reason,omitempty"` + Message *string `json:"message,omitempty"` + LastTransitionTime time.Time `json:"last_transition_time"` +} + +// AdapterCondition represents a condition of an adapter +// Domain equivalent of openapi.AdapterCondition +// JSON tags match database JSONB structure +type AdapterCondition struct { + Type string `json:"type"` + Status ConditionStatus `json:"status"` + Reason *string `json:"reason,omitempty"` + Message *string `json:"message,omitempty"` + LastTransitionTime time.Time `json:"last_transition_time"` +} diff --git a/pkg/api/status_types_test.go b/pkg/api/status_types_test.go new file mode 100644 index 0000000..a7cfde4 --- /dev/null +++ b/pkg/api/status_types_test.go @@ -0,0 +1,336 @@ +package api + +import ( + "encoding/json" + "testing" + "time" + + . "github.com/onsi/gomega" +) + +// TestResourcePhase_Constants verifies that ResourcePhase constants match OpenAPI equivalents +func TestResourcePhase_Constants(t *testing.T) { + RegisterTestingT(t) + + // Verify constant values match expected strings + Expect(string(PhaseNotReady)).To(Equal("NotReady")) + Expect(string(PhaseReady)).To(Equal("Ready")) + Expect(string(PhaseFailed)).To(Equal("Failed")) + + // These values should match openapi.NOT_READY, openapi.READY, openapi.FAILED + // which are "NotReady", "Ready", "Failed" respectively +} + +// TestResourcePhase_StringConversion tests type casting to/from string +func TestResourcePhase_StringConversion(t *testing.T) { + RegisterTestingT(t) + + // Test converting string to ResourcePhase + phase := ResourcePhase("NotReady") + Expect(phase).To(Equal(PhaseNotReady)) + + // Test converting ResourcePhase to string + str := string(PhaseReady) + Expect(str).To(Equal("Ready")) +} + +// TestConditionStatus_Constants verifies that ConditionStatus constants match OpenAPI equivalents +func TestConditionStatus_Constants(t *testing.T) { + RegisterTestingT(t) + + // Verify constant values match expected strings + Expect(string(ConditionTrue)).To(Equal("True")) + Expect(string(ConditionFalse)).To(Equal("False")) + Expect(string(ConditionUnknown)).To(Equal("Unknown")) + + // These values should match openapi.TRUE, openapi.FALSE, openapi.UNKNOWN + // which are "True", "False", "Unknown" respectively +} + +// TestConditionStatus_StringConversion tests type casting to/from string +func TestConditionStatus_StringConversion(t *testing.T) { + RegisterTestingT(t) + + // Test converting string to ConditionStatus + status := ConditionStatus("True") + Expect(status).To(Equal(ConditionTrue)) + + // Test converting ConditionStatus to string + str := string(ConditionFalse) + Expect(str).To(Equal("False")) +} + +// TestResourceCondition_JSONSerialization tests marshaling ResourceCondition to JSON +func TestResourceCondition_JSONSerialization(t *testing.T) { + RegisterTestingT(t) + + now := time.Now().UTC().Truncate(time.Second) + reason := "TestReason" + message := "Test message" + + // Test full struct with all fields + fullCondition := ResourceCondition{ + ObservedGeneration: 5, + CreatedTime: now, + LastUpdatedTime: now, + Type: "Ready", + Status: ConditionTrue, + Reason: &reason, + Message: &message, + LastTransitionTime: now, + } + + jsonBytes, err := json.Marshal(fullCondition) + Expect(err).To(BeNil()) + + // Verify JSON contains expected fields + var jsonMap map[string]interface{} + err = json.Unmarshal(jsonBytes, &jsonMap) + Expect(err).To(BeNil()) + + Expect(jsonMap["observed_generation"]).To(BeNumerically("==", 5)) + Expect(jsonMap["type"]).To(Equal("Ready")) + Expect(jsonMap["status"]).To(Equal("True")) + Expect(jsonMap["reason"]).To(Equal("TestReason")) + Expect(jsonMap["message"]).To(Equal("Test message")) + + // Test struct with nil optional fields + minimalCondition := ResourceCondition{ + ObservedGeneration: 3, + CreatedTime: now, + LastUpdatedTime: now, + Type: "Available", + Status: ConditionFalse, + Reason: nil, + Message: nil, + LastTransitionTime: now, + } + + jsonBytes, err = json.Marshal(minimalCondition) + Expect(err).To(BeNil()) + + var minimalMap map[string]interface{} + err = json.Unmarshal(jsonBytes, &minimalMap) + Expect(err).To(BeNil()) + + // Verify optional fields are omitted + _, hasReason := minimalMap["reason"] + _, hasMessage := minimalMap["message"] + Expect(hasReason).To(BeFalse()) + Expect(hasMessage).To(BeFalse()) +} + +// TestResourceCondition_JSONDeserialization tests unmarshaling JSON to ResourceCondition +func TestResourceCondition_JSONDeserialization(t *testing.T) { + RegisterTestingT(t) + + // Test JSON with all fields + fullJSON := `{ + "observed_generation": 7, + "created_time": "2023-01-01T00:00:00Z", + "last_updated_time": "2023-01-01T01:00:00Z", + "type": "Validated", + "status": "True", + "reason": "Success", + "message": "Validation successful", + "last_transition_time": "2023-01-01T02:00:00Z" + }` + + var condition ResourceCondition + err := json.Unmarshal([]byte(fullJSON), &condition) + Expect(err).To(BeNil()) + + Expect(condition.ObservedGeneration).To(Equal(int32(7))) + Expect(condition.Type).To(Equal("Validated")) + Expect(condition.Status).To(Equal(ConditionTrue)) + Expect(condition.Reason).ToNot(BeNil()) + Expect(*condition.Reason).To(Equal("Success")) + Expect(condition.Message).ToNot(BeNil()) + Expect(*condition.Message).To(Equal("Validation successful")) + + // Test JSON with missing optional fields + minimalJSON := `{ + "observed_generation": 2, + "created_time": "2023-01-01T00:00:00Z", + "last_updated_time": "2023-01-01T01:00:00Z", + "type": "NotReady", + "status": "Unknown", + "last_transition_time": "2023-01-01T02:00:00Z" + }` + + var minimalCondition ResourceCondition + err = json.Unmarshal([]byte(minimalJSON), &minimalCondition) + Expect(err).To(BeNil()) + + Expect(minimalCondition.ObservedGeneration).To(Equal(int32(2))) + Expect(minimalCondition.Type).To(Equal("NotReady")) + Expect(minimalCondition.Status).To(Equal(ConditionUnknown)) + Expect(minimalCondition.Reason).To(BeNil()) + Expect(minimalCondition.Message).To(BeNil()) +} + +// TestResourceCondition_RoundTrip tests Marshal → Unmarshal to ensure no data loss +func TestResourceCondition_RoundTrip(t *testing.T) { + RegisterTestingT(t) + + now := time.Now().UTC().Truncate(time.Second) + reason := "RoundTripReason" + message := "Round trip message" + + original := ResourceCondition{ + ObservedGeneration: 10, + CreatedTime: now, + LastUpdatedTime: now, + Type: "HealthCheck", + Status: ConditionTrue, + Reason: &reason, + Message: &message, + LastTransitionTime: now, + } + + // Marshal + jsonBytes, err := json.Marshal(original) + Expect(err).To(BeNil()) + + // Unmarshal + var decoded ResourceCondition + err = json.Unmarshal(jsonBytes, &decoded) + Expect(err).To(BeNil()) + + // Verify all fields match + Expect(decoded.ObservedGeneration).To(Equal(original.ObservedGeneration)) + Expect(decoded.Type).To(Equal(original.Type)) + Expect(decoded.Status).To(Equal(original.Status)) + Expect(*decoded.Reason).To(Equal(*original.Reason)) + Expect(*decoded.Message).To(Equal(*original.Message)) + + // Compare timestamps (truncated to second precision to avoid nanosecond differences) + Expect(decoded.CreatedTime.Unix()).To(Equal(original.CreatedTime.Unix())) + Expect(decoded.LastUpdatedTime.Unix()).To(Equal(original.LastUpdatedTime.Unix())) + Expect(decoded.LastTransitionTime.Unix()).To(Equal(original.LastTransitionTime.Unix())) +} + +// TestAdapterCondition_JSONSerialization tests marshaling AdapterCondition to JSON +func TestAdapterCondition_JSONSerialization(t *testing.T) { + RegisterTestingT(t) + + now := time.Now().UTC().Truncate(time.Second) + reason := "AdapterReady" + message := "Adapter is ready" + + // Test with all fields + fullCondition := AdapterCondition{ + Type: "Connected", + Status: ConditionTrue, + Reason: &reason, + Message: &message, + LastTransitionTime: now, + } + + jsonBytes, err := json.Marshal(fullCondition) + Expect(err).To(BeNil()) + + var jsonMap map[string]interface{} + err = json.Unmarshal(jsonBytes, &jsonMap) + Expect(err).To(BeNil()) + + Expect(jsonMap["type"]).To(Equal("Connected")) + Expect(jsonMap["status"]).To(Equal("True")) + Expect(jsonMap["reason"]).To(Equal("AdapterReady")) + Expect(jsonMap["message"]).To(Equal("Adapter is ready")) + + // Test without optional fields + minimalCondition := AdapterCondition{ + Type: "Disconnected", + Status: ConditionFalse, + Reason: nil, + Message: nil, + LastTransitionTime: now, + } + + jsonBytes, err = json.Marshal(minimalCondition) + Expect(err).To(BeNil()) + + var minimalMap map[string]interface{} + err = json.Unmarshal(jsonBytes, &minimalMap) + Expect(err).To(BeNil()) + + _, hasReason := minimalMap["reason"] + _, hasMessage := minimalMap["message"] + Expect(hasReason).To(BeFalse()) + Expect(hasMessage).To(BeFalse()) +} + +// TestAdapterCondition_JSONDeserialization tests unmarshaling JSON to AdapterCondition +func TestAdapterCondition_JSONDeserialization(t *testing.T) { + RegisterTestingT(t) + + // Test JSON with all fields + fullJSON := `{ + "type": "Synced", + "status": "True", + "reason": "SyncSuccessful", + "message": "Data synchronized", + "last_transition_time": "2023-01-01T12:00:00Z" + }` + + var condition AdapterCondition + err := json.Unmarshal([]byte(fullJSON), &condition) + Expect(err).To(BeNil()) + + Expect(condition.Type).To(Equal("Synced")) + Expect(condition.Status).To(Equal(ConditionTrue)) + Expect(condition.Reason).ToNot(BeNil()) + Expect(*condition.Reason).To(Equal("SyncSuccessful")) + Expect(condition.Message).ToNot(BeNil()) + Expect(*condition.Message).To(Equal("Data synchronized")) + + // Test JSON without optional fields + minimalJSON := `{ + "type": "Error", + "status": "False", + "last_transition_time": "2023-01-01T12:00:00Z" + }` + + var minimalCondition AdapterCondition + err = json.Unmarshal([]byte(minimalJSON), &minimalCondition) + Expect(err).To(BeNil()) + + Expect(minimalCondition.Type).To(Equal("Error")) + Expect(minimalCondition.Status).To(Equal(ConditionFalse)) + Expect(minimalCondition.Reason).To(BeNil()) + Expect(minimalCondition.Message).To(BeNil()) +} + +// TestAdapterCondition_RoundTrip tests Marshal → Unmarshal to ensure no data loss +func TestAdapterCondition_RoundTrip(t *testing.T) { + RegisterTestingT(t) + + now := time.Now().UTC().Truncate(time.Second) + reason := "TestReason" + message := "Test message for round trip" + + original := AdapterCondition{ + Type: "Provisioned", + Status: ConditionTrue, + Reason: &reason, + Message: &message, + LastTransitionTime: now, + } + + // Marshal + jsonBytes, err := json.Marshal(original) + Expect(err).To(BeNil()) + + // Unmarshal + var decoded AdapterCondition + err = json.Unmarshal(jsonBytes, &decoded) + Expect(err).To(BeNil()) + + // Verify all fields match + Expect(decoded.Type).To(Equal(original.Type)) + Expect(decoded.Status).To(Equal(original.Status)) + Expect(*decoded.Reason).To(Equal(*original.Reason)) + Expect(*decoded.Message).To(Equal(*original.Message)) + Expect(decoded.LastTransitionTime.Unix()).To(Equal(original.LastTransitionTime.Unix())) +} diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index fc4da7c..d29fa7a 100755 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -64,6 +64,12 @@ type ServiceErrorCode int type ServiceErrors []ServiceError +// ValidationDetail represents a single field validation error +type ValidationDetail struct { + Field string `json:"field"` + Error string `json:"error"` +} + func Find(code ServiceErrorCode) (bool, *ServiceError) { for _, err := range Errors() { if err.Code == code { @@ -75,19 +81,19 @@ func Find(code ServiceErrorCode) (bool, *ServiceError) { func Errors() ServiceErrors { return ServiceErrors{ - ServiceError{ErrorInvalidToken, "Invalid token provided", http.StatusForbidden}, - ServiceError{ErrorForbidden, "Forbidden to perform this action", http.StatusForbidden}, - ServiceError{ErrorConflict, "An entity with the specified unique values already exists", http.StatusConflict}, - ServiceError{ErrorNotFound, "Resource not found", http.StatusNotFound}, - ServiceError{ErrorValidation, "General validation failure", http.StatusBadRequest}, - ServiceError{ErrorGeneral, "Unspecified error", http.StatusInternalServerError}, - ServiceError{ErrorNotImplemented, "HTTP Method not implemented for this endpoint", http.StatusMethodNotAllowed}, - ServiceError{ErrorUnauthorized, "Account is unauthorized to perform this action", http.StatusForbidden}, - ServiceError{ErrorUnauthenticated, "Account authentication could not be verified", http.StatusUnauthorized}, - ServiceError{ErrorMalformedRequest, "Unable to read request body", http.StatusBadRequest}, - ServiceError{ErrorBadRequest, "Bad request", http.StatusBadRequest}, - ServiceError{ErrorFailedToParseSearch, "Failed to parse search query", http.StatusBadRequest}, - ServiceError{ErrorDatabaseAdvisoryLock, "Database advisory lock error", http.StatusInternalServerError}, + {Code: ErrorInvalidToken, Reason: "Invalid token provided", HttpCode: http.StatusForbidden}, + {Code: ErrorForbidden, Reason: "Forbidden to perform this action", HttpCode: http.StatusForbidden}, + {Code: ErrorConflict, Reason: "An entity with the specified unique values already exists", HttpCode: http.StatusConflict}, + {Code: ErrorNotFound, Reason: "Resource not found", HttpCode: http.StatusNotFound}, + {Code: ErrorValidation, Reason: "General validation failure", HttpCode: http.StatusBadRequest}, + {Code: ErrorGeneral, Reason: "Unspecified error", HttpCode: http.StatusInternalServerError}, + {Code: ErrorNotImplemented, Reason: "HTTP Method not implemented for this endpoint", HttpCode: http.StatusMethodNotAllowed}, + {Code: ErrorUnauthorized, Reason: "Account is unauthorized to perform this action", HttpCode: http.StatusForbidden}, + {Code: ErrorUnauthenticated, Reason: "Account authentication could not be verified", HttpCode: http.StatusUnauthorized}, + {Code: ErrorMalformedRequest, Reason: "Unable to read request body", HttpCode: http.StatusBadRequest}, + {Code: ErrorBadRequest, Reason: "Bad request", HttpCode: http.StatusBadRequest}, + {Code: ErrorFailedToParseSearch, Reason: "Failed to parse search query", HttpCode: http.StatusBadRequest}, + {Code: ErrorDatabaseAdvisoryLock, Reason: "Database advisory lock error", HttpCode: http.StatusInternalServerError}, } } @@ -98,6 +104,8 @@ type ServiceError struct { Reason string // HttopCode is the HttpCode associated with the error when the error is returned as an API response HttpCode int + // Details contains field-level validation errors (optional) + Details []ValidationDetail } // New Reason can be a string with format verbs, which will be replace by the specified values @@ -107,7 +115,11 @@ func New(code ServiceErrorCode, reason string, values ...interface{}) *ServiceEr exists, err := Find(code) if !exists { glog.Errorf("Undefined error code used: %d", code) - err = &ServiceError{ErrorGeneral, "Unspecified error", 500} + err = &ServiceError{ + Code: ErrorGeneral, + Reason: "Unspecified error", + HttpCode: 500, + } } // If the reason is unspecified, use the default @@ -139,7 +151,7 @@ func (e *ServiceError) IsForbidden() bool { } func (e *ServiceError) AsOpenapiError(operationID string) openapi.Error { - return openapi.Error{ + openapiErr := openapi.Error{ Kind: openapi.PtrString("Error"), Id: openapi.PtrString(strconv.Itoa(int(e.Code))), Href: Href(e.Code), @@ -147,6 +159,20 @@ func (e *ServiceError) AsOpenapiError(operationID string) openapi.Error { Reason: openapi.PtrString(e.Reason), OperationId: openapi.PtrString(operationID), } + + // Add validation details if present + if len(e.Details) > 0 { + details := make([]openapi.ErrorDetailsInner, len(e.Details)) + for i, detail := range e.Details { + details[i] = openapi.ErrorDetailsInner{ + Field: openapi.PtrString(detail.Field), + Error: openapi.PtrString(detail.Error), + } + } + openapiErr.Details = details + } + + return openapiErr } func CodeStr(code ServiceErrorCode) *string { @@ -189,6 +215,13 @@ func Validation(reason string, values ...interface{}) *ServiceError { return New(ErrorValidation, reason, values...) } +// ValidationWithDetails creates a validation error with field-level details +func ValidationWithDetails(reason string, details []ValidationDetail) *ServiceError { + err := New(ErrorValidation, "%s", reason) + err.Details = details + return err +} + func MalformedRequest(reason string, values ...interface{}) *ServiceError { return New(ErrorMalformedRequest, reason, values...) } diff --git a/pkg/handlers/cluster.go b/pkg/handlers/cluster.go index 92d7c1c..4a4887a 100644 --- a/pkg/handlers/cluster.go +++ b/pkg/handlers/cluster.go @@ -36,11 +36,14 @@ func (h clusterHandler) Create(w http.ResponseWriter, r *http.Request) { }, func() (interface{}, *errors.ServiceError) { ctx := r.Context() - // Use the ClusterFromOpenAPICreate helper to convert the request - clusterModel := api.ClusterFromOpenAPICreate(&req, "system") - clusterModel, err := h.cluster.Create(ctx, clusterModel) + // Use the presenters.ConvertCluster helper to convert the request + clusterModel, err := presenters.ConvertCluster(&req, "system") if err != nil { - return nil, err + return nil, errors.GeneralError("Failed to convert cluster: %v", err) + } + clusterModel, svcErr := h.cluster.Create(ctx, clusterModel) + if svcErr != nil { + return nil, svcErr } return presenters.PresentCluster(clusterModel), nil }, diff --git a/pkg/handlers/cluster_nodepools.go b/pkg/handlers/cluster_nodepools.go index 9aa3a9d..3797946 100644 --- a/pkg/handlers/cluster_nodepools.go +++ b/pkg/handlers/cluster_nodepools.go @@ -139,8 +139,11 @@ func (h clusterNodePoolsHandler) Create(w http.ResponseWriter, r *http.Request) return nil, err } - // Use the NodePoolFromOpenAPICreate helper to convert the request - nodePoolModel := api.NodePoolFromOpenAPICreate(&req, cluster.ID, "system") + // Use the presenters.ConvertNodePool helper to convert the request + nodePoolModel, convErr := presenters.ConvertNodePool(&req, cluster.ID, "system") + if convErr != nil { + return nil, errors.GeneralError("Failed to convert nodepool: %v", convErr) + } // Create nodepool nodePoolModel, err = h.nodePoolService.Create(ctx, nodePoolModel) diff --git a/pkg/handlers/cluster_status.go b/pkg/handlers/cluster_status.go index 2d114ed..af294cd 100644 --- a/pkg/handlers/cluster_status.go +++ b/pkg/handlers/cluster_status.go @@ -5,8 +5,8 @@ import ( "github.com/gorilla/mux" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/presenters" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" @@ -41,7 +41,7 @@ func (h clusterStatusHandler) List(w http.ResponseWriter, r *http.Request) { // Convert to OpenAPI models items := make([]openapi.AdapterStatus, 0, len(adapterStatuses)) for _, as := range adapterStatuses { - items = append(items, *as.ToOpenAPI()) + items = append(items, presenters.PresentAdapterStatus(as)) } // Return list response with pagination metadata @@ -80,7 +80,10 @@ func (h clusterStatusHandler) Create(w http.ResponseWriter, r *http.Request) { } // Create adapter status from request - newStatus := api.AdapterStatusFromOpenAPICreate("Cluster", clusterID, &req) + newStatus, convErr := presenters.ConvertAdapterStatus("Cluster", clusterID, &req) + if convErr != nil { + return nil, errors.GeneralError("Failed to convert adapter status: %v", convErr) + } // Upsert (create or update based on resource_type + resource_id + adapter) adapterStatus, err := h.adapterStatusService.Upsert(ctx, newStatus) @@ -95,7 +98,8 @@ func (h clusterStatusHandler) Create(w http.ResponseWriter, r *http.Request) { log.Extra("cluster_id", clusterID).Extra("error", aggregateErr).Warning("Failed to aggregate cluster status") } - return adapterStatus.ToOpenAPI(), nil + status := presenters.PresentAdapterStatus(adapterStatus) + return &status, nil }, handleError, } diff --git a/pkg/handlers/node_pool.go b/pkg/handlers/node_pool.go index 9cb44e4..b2291f8 100644 --- a/pkg/handlers/node_pool.go +++ b/pkg/handlers/node_pool.go @@ -38,7 +38,10 @@ func (h nodePoolHandler) Create(w http.ResponseWriter, r *http.Request) { ctx := r.Context() // For standalone nodepools, owner_id would need to come from somewhere // This is likely not a supported use case, but using empty string for now - nodePoolModel := api.NodePoolFromOpenAPICreate(&req, "", "system") + nodePoolModel, convErr := presenters.ConvertNodePool(&req, "", "system") + if convErr != nil { + return nil, errors.GeneralError("Failed to convert nodepool: %v", convErr) + } nodePoolModel, err := h.nodePool.Create(ctx, nodePoolModel) if err != nil { return nil, err diff --git a/pkg/handlers/nodepool_status.go b/pkg/handlers/nodepool_status.go index 0710886..8033db3 100644 --- a/pkg/handlers/nodepool_status.go +++ b/pkg/handlers/nodepool_status.go @@ -5,8 +5,8 @@ import ( "github.com/gorilla/mux" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/presenters" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" @@ -41,7 +41,7 @@ func (h nodePoolStatusHandler) List(w http.ResponseWriter, r *http.Request) { // Convert to OpenAPI models items := make([]openapi.AdapterStatus, 0, len(adapterStatuses)) for _, as := range adapterStatuses { - items = append(items, *as.ToOpenAPI()) + items = append(items, presenters.PresentAdapterStatus(as)) } // Return list response with pagination metadata @@ -80,7 +80,10 @@ func (h nodePoolStatusHandler) Create(w http.ResponseWriter, r *http.Request) { } // Create adapter status from request - newStatus := api.AdapterStatusFromOpenAPICreate("NodePool", nodePoolID, &req) + newStatus, convErr := presenters.ConvertAdapterStatus("NodePool", nodePoolID, &req) + if convErr != nil { + return nil, errors.GeneralError("Failed to convert adapter status: %v", convErr) + } // Upsert (create or update based on resource_type + resource_id + adapter) adapterStatus, err := h.adapterStatusService.Upsert(ctx, newStatus) @@ -95,7 +98,8 @@ func (h nodePoolStatusHandler) Create(w http.ResponseWriter, r *http.Request) { log.Extra("nodepool_id", nodePoolID).Extra("error", aggregateErr).Warning("Failed to aggregate nodepool status") } - return adapterStatus.ToOpenAPI(), nil + status := presenters.PresentAdapterStatus(adapterStatus) + return &status, nil }, handleError, } diff --git a/pkg/middleware/schema_validation.go b/pkg/middleware/schema_validation.go new file mode 100644 index 0000000..286d7b9 --- /dev/null +++ b/pkg/middleware/schema_validation.go @@ -0,0 +1,121 @@ +package middleware + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "strings" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/validators" +) + +// handleValidationError writes validation error response +func handleValidationError(w http.ResponseWriter, r *http.Request, err *errors.ServiceError) { + log := logger.NewOCMLogger(r.Context()) + operationID := logger.GetOperationID(r.Context()) + + // Log validation errors as info (user error, not server error) + log.Infof(err.Error()) + + // Write JSON error response + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(err.HttpCode) + json.NewEncoder(w).Encode(err.AsOpenapiError(operationID)) +} + +// SchemaValidationMiddleware validates cluster and nodepool spec fields against OpenAPI schemas +func SchemaValidationMiddleware(validator *validators.SchemaValidator) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Check if the request requires spec validation + shouldValidate, resourceType := shouldValidateRequest(r.Method, r.URL.Path) + if !shouldValidate { + next.ServeHTTP(w, r) + return + } + + // Read and buffer the request body + bodyBytes, err := io.ReadAll(r.Body) + if err != nil { + serviceErr := errors.MalformedRequest("Failed to read request body") + handleValidationError(w, r, serviceErr) + return + } + r.Body.Close() + + // Restore the request body for the next handler + r.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) + + // Parse JSON to extract spec field + var requestData map[string]interface{} + if err := json.Unmarshal(bodyBytes, &requestData); err != nil { + serviceErr := errors.MalformedRequest("Invalid JSON in request body") + handleValidationError(w, r, serviceErr) + return + } + + // Extract spec field + spec, ok := requestData["spec"] + if !ok { + // If no spec field, skip validation (may be a request without spec) + next.ServeHTTP(w, r) + return + } + + // Convert spec to map[string]interface{} + specMap, ok := spec.(map[string]interface{}) + if !ok { + serviceErr := errors.Validation("spec field must be an object") + handleValidationError(w, r, serviceErr) + return + } + + // Validate spec using the resource type + // validator.Validate returns nil for unknown resource types + validationErr := validator.Validate(resourceType, specMap) + + // If validation failed, return 400 error + if validationErr != nil { + // Check if it's a ServiceError with details + if serviceErr, ok := validationErr.(*errors.ServiceError); ok { + handleValidationError(w, r, serviceErr) + return + } + // Fallback to generic validation error + serviceErr := errors.Validation("Spec validation failed: %v", validationErr) + handleValidationError(w, r, serviceErr) + return + } + + // Validation passed, continue to next handler + next.ServeHTTP(w, r) + }) + } +} + +// shouldValidateRequest determines if the request requires spec validation +// Returns (shouldValidate bool, resourceType string) +func shouldValidateRequest(method, path string) (bool, string) { + // Only validate POST and PATCH requests + if method != http.MethodPost && method != http.MethodPatch { + return false, "" + } + + // Check nodepools first (more specific path) + // POST /api/hyperfleet/v1/clusters/{cluster_id}/nodepools + // PATCH /api/hyperfleet/v1/clusters/{cluster_id}/nodepools/{nodepool_id} + if strings.Contains(path, "/nodepools") { + return true, "nodepool" + } + + // POST /api/hyperfleet/v1/clusters + // PATCH /api/hyperfleet/v1/clusters/{cluster_id} + if strings.HasSuffix(path, "/clusters") || strings.Contains(path, "/clusters/") { + return true, "cluster" + } + + return false, "" +} diff --git a/pkg/middleware/schema_validation_test.go b/pkg/middleware/schema_validation_test.go new file mode 100644 index 0000000..7a94202 --- /dev/null +++ b/pkg/middleware/schema_validation_test.go @@ -0,0 +1,393 @@ +package middleware + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + . "github.com/onsi/gomega" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/validators" +) + +const testSchema = ` +openapi: 3.0.0 +info: + title: Test Schema + version: 1.0.0 +paths: {} +components: + schemas: + ClusterSpec: + type: object + required: + - region + properties: + region: + type: string + enum: [us-central1, us-east1] + + NodePoolSpec: + type: object + required: + - replicas + properties: + replicas: + type: integer + minimum: 1 + maximum: 10 +` + +func TestSchemaValidationMiddleware_PostRequestValidation(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + middleware := SchemaValidationMiddleware(validator) + + // Create a valid cluster creation request + validRequest := map[string]interface{}{ + "name": "test-cluster", + "spec": map[string]interface{}{ + "region": "us-central1", + }, + } + + body, _ := json.Marshal(validRequest) + req := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + + nextHandlerCalled := false + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextHandlerCalled = true + w.WriteHeader(http.StatusCreated) + }) + + middleware(nextHandler).ServeHTTP(rr, req) + + // Verify next handler was called (validation passed) + Expect(nextHandlerCalled).To(BeTrue()) + Expect(rr.Code).To(Equal(http.StatusCreated)) +} + +func TestSchemaValidationMiddleware_PostRequestInvalidSpec(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + middleware := SchemaValidationMiddleware(validator) + + // Create an invalid cluster creation request (missing required field) + invalidRequest := map[string]interface{}{ + "name": "test-cluster", + "spec": map[string]interface{}{ + // missing "region" + }, + } + + body, _ := json.Marshal(invalidRequest) + req := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + + nextHandlerCalled := false + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextHandlerCalled = true + }) + + middleware(nextHandler).ServeHTTP(rr, req) + + // Verify next handler was NOT called (validation failed) + Expect(nextHandlerCalled).To(BeFalse()) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + + // Verify error response format + var errorResponse openapi.Error + err := json.Unmarshal(rr.Body.Bytes(), &errorResponse) + Expect(err).To(BeNil()) + Expect(errorResponse.Code).ToNot(BeNil()) + Expect(errorResponse.Reason).ToNot(BeNil()) +} + +func TestSchemaValidationMiddleware_PatchRequestValidation(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + middleware := SchemaValidationMiddleware(validator) + + // Create a valid cluster patch request + validRequest := map[string]interface{}{ + "spec": map[string]interface{}{ + "region": "us-east1", + }, + } + + body, _ := json.Marshal(validRequest) + req := httptest.NewRequest(http.MethodPatch, "/api/hyperfleet/v1/clusters/123", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + + nextHandlerCalled := false + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextHandlerCalled = true + w.WriteHeader(http.StatusOK) + }) + + middleware(nextHandler).ServeHTTP(rr, req) + + // Verify next handler was called + Expect(nextHandlerCalled).To(BeTrue()) + Expect(rr.Code).To(Equal(http.StatusOK)) +} + +func TestSchemaValidationMiddleware_GetRequestSkipped(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + middleware := SchemaValidationMiddleware(validator) + + // GET request should skip validation + req := httptest.NewRequest(http.MethodGet, "/api/hyperfleet/v1/clusters", nil) + rr := httptest.NewRecorder() + + nextHandlerCalled := false + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextHandlerCalled = true + w.WriteHeader(http.StatusOK) + }) + + middleware(nextHandler).ServeHTTP(rr, req) + + // Verify next handler was called (no validation) + Expect(nextHandlerCalled).To(BeTrue()) + Expect(rr.Code).To(Equal(http.StatusOK)) +} + +func TestSchemaValidationMiddleware_DeleteRequestSkipped(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + middleware := SchemaValidationMiddleware(validator) + + // DELETE request should skip validation + req := httptest.NewRequest(http.MethodDelete, "/api/hyperfleet/v1/clusters/123", nil) + rr := httptest.NewRecorder() + + nextHandlerCalled := false + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextHandlerCalled = true + w.WriteHeader(http.StatusNoContent) + }) + + middleware(nextHandler).ServeHTTP(rr, req) + + // Verify next handler was called + Expect(nextHandlerCalled).To(BeTrue()) + Expect(rr.Code).To(Equal(http.StatusNoContent)) +} + +func TestSchemaValidationMiddleware_NonClusterPath(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + middleware := SchemaValidationMiddleware(validator) + + // POST to non-cluster/nodepool path should skip validation + validRequest := map[string]interface{}{ + "name": "test", + "spec": map[string]interface{}{}, + } + + body, _ := json.Marshal(validRequest) + req := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/other", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + + nextHandlerCalled := false + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextHandlerCalled = true + w.WriteHeader(http.StatusOK) + }) + + middleware(nextHandler).ServeHTTP(rr, req) + + // Verify next handler was called (path not validated) + Expect(nextHandlerCalled).To(BeTrue()) + Expect(rr.Code).To(Equal(http.StatusOK)) +} + +func TestSchemaValidationMiddleware_NodePoolValidation(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + middleware := SchemaValidationMiddleware(validator) + + // Create a valid nodepool request + validRequest := map[string]interface{}{ + "name": "test-nodepool", + "spec": map[string]interface{}{ + "replicas": 3, + }, + } + + body, _ := json.Marshal(validRequest) + req := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters/123/nodepools", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + + nextHandlerCalled := false + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextHandlerCalled = true + w.WriteHeader(http.StatusCreated) + }) + + middleware(nextHandler).ServeHTTP(rr, req) + + // If failed, print response for debugging + if !nextHandlerCalled { + t.Logf("Response code: %d", rr.Code) + t.Logf("Response body: %s", rr.Body.String()) + } + + // Verify next handler was called + Expect(nextHandlerCalled).To(BeTrue()) + Expect(rr.Code).To(Equal(http.StatusCreated)) +} + +func TestSchemaValidationMiddleware_NodePoolInvalidSpec(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + middleware := SchemaValidationMiddleware(validator) + + // Create an invalid nodepool request (replicas out of range) + invalidRequest := map[string]interface{}{ + "name": "test-nodepool", + "spec": map[string]interface{}{ + "replicas": 100, // exceeds maximum: 10 + }, + } + + body, _ := json.Marshal(invalidRequest) + req := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters/123/nodepools", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + + nextHandlerCalled := false + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextHandlerCalled = true + }) + + middleware(nextHandler).ServeHTTP(rr, req) + + // Verify next handler was NOT called + Expect(nextHandlerCalled).To(BeFalse()) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) +} + +func TestSchemaValidationMiddleware_MissingSpecField(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + middleware := SchemaValidationMiddleware(validator) + + // Request without spec field should pass through (spec is optional in request) + requestWithoutSpec := map[string]interface{}{ + "name": "test-cluster", + } + + body, _ := json.Marshal(requestWithoutSpec) + req := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + + nextHandlerCalled := false + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextHandlerCalled = true + w.WriteHeader(http.StatusCreated) + }) + + middleware(nextHandler).ServeHTTP(rr, req) + + // Verify next handler was called (missing spec is ok) + Expect(nextHandlerCalled).To(BeTrue()) +} + +func TestSchemaValidationMiddleware_InvalidSpecType(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + middleware := SchemaValidationMiddleware(validator) + + // Spec field is not an object + invalidRequest := map[string]interface{}{ + "name": "test-cluster", + "spec": "invalid-string", + } + + body, _ := json.Marshal(invalidRequest) + req := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + + nextHandlerCalled := false + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextHandlerCalled = true + }) + + middleware(nextHandler).ServeHTTP(rr, req) + + // Verify next handler was NOT called + Expect(nextHandlerCalled).To(BeFalse()) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + + // Verify error message + var errorResponse openapi.Error + err := json.Unmarshal(rr.Body.Bytes(), &errorResponse) + Expect(err).To(BeNil()) + Expect(*errorResponse.Reason).To(ContainSubstring("spec field must be an object")) +} + +func TestSchemaValidationMiddleware_MalformedJSON(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + middleware := SchemaValidationMiddleware(validator) + + // Invalid JSON + invalidJSON := []byte(`{"name": "test", "spec": {invalid}`) + req := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters", bytes.NewBuffer(invalidJSON)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + + nextHandlerCalled := false + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + nextHandlerCalled = true + }) + + middleware(nextHandler).ServeHTTP(rr, req) + + // Verify next handler was NOT called + Expect(nextHandlerCalled).To(BeFalse()) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) +} + +// Helper function to setup test validator +func setupTestValidator(t *testing.T) *validators.SchemaValidator { + tmpDir := t.TempDir() + schemaPath := filepath.Join(tmpDir, "test-schema.yaml") + err := os.WriteFile(schemaPath, []byte(testSchema), 0644) + if err != nil { + t.Fatalf("Failed to create test schema: %v", err) + } + + validator, err := validators.NewSchemaValidator(schemaPath) + if err != nil { + t.Fatalf("Failed to create validator: %v", err) + } + + return validator +} diff --git a/pkg/services/cluster.go b/pkg/services/cluster.go index 1eae467..0d492eb 100644 --- a/pkg/services/cluster.go +++ b/pkg/services/cluster.go @@ -3,10 +3,10 @@ package services import ( "context" "encoding/json" + "math" "time" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/dao" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" @@ -131,19 +131,19 @@ func (s *sqlClusterService) UpdateClusterStatusFromAdapters(ctx context.Context, return nil, errors.GeneralError("Failed to get adapter statuses: %s", err) } - // Build the list of ResourceCondition from adapter Available conditions - adapters := []openapi.ResourceCondition{} - maxObservedGeneration := int32(0) + // Build the list of ResourceCondition + adapters := []api.ResourceCondition{} + minObservedGeneration := int32(math.MaxInt32) for _, adapterStatus := range adapterStatuses { // Unmarshal Conditions from JSONB - var conditions []openapi.AdapterCondition - if unmarshalErr := json.Unmarshal(adapterStatus.Conditions, &conditions); unmarshalErr != nil { + var conditions []api.AdapterCondition + if err := json.Unmarshal(adapterStatus.Conditions, &conditions); err != nil { continue // Skip if can't unmarshal } // Find the "Available" condition - var availableCondition *openapi.AdapterCondition + var availableCondition *api.AdapterCondition for i := range conditions { if conditions[i].Type == "Available" { availableCondition = &conditions[i] @@ -157,7 +157,7 @@ func (s *sqlClusterService) UpdateClusterStatusFromAdapters(ctx context.Context, } // Convert to ResourceCondition - condResource := openapi.ResourceCondition{ + condResource := api.ResourceCondition{ Type: MapAdapterToConditionType(adapterStatus.Adapter), Status: availableCondition.Status, Reason: availableCondition.Reason, @@ -178,9 +178,10 @@ func (s *sqlClusterService) UpdateClusterStatusFromAdapters(ctx context.Context, adapters = append(adapters, condResource) - // Track max observed generation - if adapterStatus.ObservedGeneration > maxObservedGeneration { - maxObservedGeneration = adapterStatus.ObservedGeneration + // Track min observed generation + // Use the LOWEST generation to ensure cluster status only advances when ALL adapters catch up + if adapterStatus.ObservedGeneration < minObservedGeneration { + minObservedGeneration = adapterStatus.ObservedGeneration } } @@ -204,7 +205,12 @@ func (s *sqlClusterService) UpdateClusterStatusFromAdapters(ctx context.Context, // Update cluster status fields now := time.Now() cluster.StatusPhase = newPhase - cluster.StatusObservedGeneration = maxObservedGeneration + // Set observed_generation to min across all adapters (0 if no adapters) + if len(adapterStatuses) == 0 { + cluster.StatusObservedGeneration = 0 + } else { + cluster.StatusObservedGeneration = minObservedGeneration + } // Marshal conditions to JSON conditionsJSON, err := json.Marshal(adapters) diff --git a/pkg/services/node_pool.go b/pkg/services/node_pool.go index 2a87ead..43bb74d 100644 --- a/pkg/services/node_pool.go +++ b/pkg/services/node_pool.go @@ -3,10 +3,10 @@ package services import ( "context" "encoding/json" + "math" "time" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/dao" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" @@ -132,18 +132,18 @@ func (s *sqlNodePoolService) UpdateNodePoolStatusFromAdapters(ctx context.Contex } // Build the list of ResourceCondition - adapters := []openapi.ResourceCondition{} - maxObservedGeneration := int32(0) + adapters := []api.ResourceCondition{} + minObservedGeneration := int32(math.MaxInt32) for _, adapterStatus := range adapterStatuses { // Unmarshal Conditions from JSONB - var conditions []openapi.AdapterCondition - if unmarshalErr := json.Unmarshal(adapterStatus.Conditions, &conditions); unmarshalErr != nil { + var conditions []api.AdapterCondition + if err := json.Unmarshal(adapterStatus.Conditions, &conditions); err != nil { continue // Skip if can't unmarshal } // Find the "Available" condition - var availableCondition *openapi.AdapterCondition + var availableCondition *api.AdapterCondition for i := range conditions { if conditions[i].Type == "Available" { availableCondition = &conditions[i] @@ -157,7 +157,7 @@ func (s *sqlNodePoolService) UpdateNodePoolStatusFromAdapters(ctx context.Contex } // Convert to ResourceCondition - condResource := openapi.ResourceCondition{ + condResource := api.ResourceCondition{ Type: MapAdapterToConditionType(adapterStatus.Adapter), Status: availableCondition.Status, Reason: availableCondition.Reason, @@ -178,9 +178,10 @@ func (s *sqlNodePoolService) UpdateNodePoolStatusFromAdapters(ctx context.Contex adapters = append(adapters, condResource) - // Track max observed generation - if adapterStatus.ObservedGeneration > maxObservedGeneration { - maxObservedGeneration = adapterStatus.ObservedGeneration + // Track min observed generation + // Use the LOWEST generation to ensure nodepool status only advances when ALL adapters catch up + if adapterStatus.ObservedGeneration < minObservedGeneration { + minObservedGeneration = adapterStatus.ObservedGeneration } } @@ -205,7 +206,12 @@ func (s *sqlNodePoolService) UpdateNodePoolStatusFromAdapters(ctx context.Contex // Update nodepool status fields now := time.Now() nodePool.StatusPhase = newPhase - nodePool.StatusObservedGeneration = maxObservedGeneration + // Set observed_generation to min across all adapters (0 if no adapters, matching DB default) + if len(adapterStatuses) == 0 { + nodePool.StatusObservedGeneration = 0 + } else { + nodePool.StatusObservedGeneration = minObservedGeneration + } // Marshal conditions to JSON conditionsJSON, err := json.Marshal(adapters) diff --git a/pkg/services/types.go b/pkg/services/types.go index 9a98f8c..9dac5c2 100755 --- a/pkg/services/types.go +++ b/pkg/services/types.go @@ -54,6 +54,12 @@ func NewListArguments(params url.Values) *ListArguments { if v := strings.Trim(params.Get("orderBy"), " "); v != "" { listArgs.OrderBy = strings.Split(v, ",") } + + // Set default sorting to created_time desc if orderBy not provided + if len(listArgs.OrderBy) == 0 { + listArgs.OrderBy = []string{"created_time desc"} + } + if v := strings.Trim(params.Get("fields"), " "); v != "" { fields := strings.Split(v, ",") idNotPresent := true diff --git a/pkg/services/types_test.go b/pkg/services/types_test.go new file mode 100644 index 0000000..228a98d --- /dev/null +++ b/pkg/services/types_test.go @@ -0,0 +1,206 @@ +package services + +import ( + "net/url" + "reflect" + "testing" + + . "github.com/onsi/gomega" +) + +func TestNewListArguments_OrderBy(t *testing.T) { + RegisterTestingT(t) + + tests := []struct { + name string + queryParams url.Values + expectedOrderBy []string + }{ + { + name: "no orderBy - should use default created_time desc", + queryParams: url.Values{}, + expectedOrderBy: []string{"created_time desc"}, + }, + { + name: "orderBy with asc direction", + queryParams: url.Values{"orderBy": []string{"name asc"}}, + expectedOrderBy: []string{"name asc"}, + }, + { + name: "orderBy with desc direction", + queryParams: url.Values{"orderBy": []string{"name desc"}}, + expectedOrderBy: []string{"name desc"}, + }, + { + name: "orderBy without direction", + queryParams: url.Values{"orderBy": []string{"name"}}, + expectedOrderBy: []string{"name"}, + }, + { + name: "multiple orderBy fields", + queryParams: url.Values{"orderBy": []string{"name asc,created_time desc"}}, + expectedOrderBy: []string{"name asc", "created_time desc"}, + }, + { + name: "orderBy with spaces should be trimmed", + queryParams: url.Values{"orderBy": []string{" name asc "}}, + expectedOrderBy: []string{"name asc"}, + }, + { + name: "empty orderBy string - should use default", + queryParams: url.Values{"orderBy": []string{""}}, + expectedOrderBy: []string{"created_time desc"}, + }, + { + name: "orderBy with whitespace only - should use default", + queryParams: url.Values{"orderBy": []string{" "}}, + expectedOrderBy: []string{"created_time desc"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + RegisterTestingT(t) + listArgs := NewListArguments(tt.queryParams) + Expect(listArgs.OrderBy).To(Equal(tt.expectedOrderBy), + "OrderBy mismatch for test case: %s", tt.name) + }) + } +} + +func TestNewListArguments_DefaultValues(t *testing.T) { + RegisterTestingT(t) + + listArgs := NewListArguments(url.Values{}) + + Expect(listArgs.Page).To(Equal(1), "Default page should be 1") + Expect(listArgs.Size).To(Equal(int64(100)), "Default size should be 100") + Expect(listArgs.Search).To(Equal(""), "Default search should be empty") + Expect(listArgs.OrderBy).To(Equal([]string{"created_time desc"}), "Default orderBy should be created_time desc") +} + +func TestNewListArguments_PageSize(t *testing.T) { + RegisterTestingT(t) + + tests := []struct { + name string + queryParams url.Values + expectedPage int + expectedSize int64 + }{ + { + name: "custom page and pageSize", + queryParams: url.Values{"page": []string{"2"}, "pageSize": []string{"50"}}, + expectedPage: 2, + expectedSize: 50, + }, + { + name: "custom page and size (legacy)", + queryParams: url.Values{"page": []string{"3"}, "size": []string{"25"}}, + expectedPage: 3, + expectedSize: 25, + }, + { + name: "pageSize takes precedence over size", + queryParams: url.Values{"pageSize": []string{"30"}, "size": []string{"60"}}, + expectedPage: 1, + expectedSize: 30, + }, + { + name: "negative size defaults to MaxListSize", + queryParams: url.Values{"pageSize": []string{"-1"}}, + expectedPage: 1, + expectedSize: MaxListSize, + }, + { + name: "size exceeding MaxListSize defaults to MaxListSize", + queryParams: url.Values{"pageSize": []string{"100000"}}, + expectedPage: 1, + expectedSize: MaxListSize, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + RegisterTestingT(t) + listArgs := NewListArguments(tt.queryParams) + Expect(listArgs.Page).To(Equal(tt.expectedPage), "Page mismatch") + Expect(listArgs.Size).To(Equal(tt.expectedSize), "Size mismatch") + }) + } +} + +func TestNewListArguments_Search(t *testing.T) { + RegisterTestingT(t) + + tests := []struct { + name string + queryParams url.Values + expectedSearch string + }{ + { + name: "no search parameter", + queryParams: url.Values{}, + expectedSearch: "", + }, + { + name: "search with value", + queryParams: url.Values{"search": []string{"name='test'"}}, + expectedSearch: "name='test'", + }, + { + name: "search with spaces should be trimmed", + queryParams: url.Values{"search": []string{" name='test' "}}, + expectedSearch: "name='test'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + RegisterTestingT(t) + listArgs := NewListArguments(tt.queryParams) + Expect(listArgs.Search).To(Equal(tt.expectedSearch), "Search mismatch") + }) + } +} + +func TestNewListArguments_Fields(t *testing.T) { + RegisterTestingT(t) + + tests := []struct { + name string + queryParams url.Values + expectedFields []string + }{ + { + name: "no fields parameter", + queryParams: url.Values{}, + expectedFields: nil, + }, + { + name: "fields without id - should add id automatically", + queryParams: url.Values{"fields": []string{"name,status"}}, + expectedFields: []string{"name", "status", "id"}, + }, + { + name: "fields with id - should not duplicate", + queryParams: url.Values{"fields": []string{"id,name,status"}}, + expectedFields: []string{"id", "name", "status"}, + }, + { + name: "fields with spaces and commas", + queryParams: url.Values{"fields": []string{" name , status , id "}}, + expectedFields: []string{"name", "status", "id"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + RegisterTestingT(t) + listArgs := NewListArguments(tt.queryParams) + if !reflect.DeepEqual(listArgs.Fields, tt.expectedFields) { + t.Errorf("Fields = %v, want %v", listArgs.Fields, tt.expectedFields) + } + }) + } +} diff --git a/pkg/validators/schema_validator.go b/pkg/validators/schema_validator.go new file mode 100644 index 0000000..d3886cc --- /dev/null +++ b/pkg/validators/schema_validator.go @@ -0,0 +1,153 @@ +package validators + +import ( + "context" + "fmt" + "strings" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" +) + +// ResourceSchema represents a validation schema for a specific resource type +type ResourceSchema struct { + TypeName string + Schema *openapi3.SchemaRef +} + +// SchemaValidator validates JSON objects against OpenAPI schemas +type SchemaValidator struct { + doc *openapi3.T + schemas map[string]*ResourceSchema +} + +// NewSchemaValidator creates a new schema validator by loading an OpenAPI spec from the given path +func NewSchemaValidator(schemaPath string) (*SchemaValidator, error) { + // Load OpenAPI spec + loader := openapi3.NewLoader() + doc, err := loader.LoadFromFile(schemaPath) + if err != nil { + return nil, fmt.Errorf("failed to load OpenAPI schema from %s: %w", schemaPath, err) + } + + // Validate the loaded document + if err := doc.Validate(context.Background()); err != nil { + return nil, fmt.Errorf("invalid OpenAPI schema: %w", err) + } + + // Extract ClusterSpec schema + clusterSpecSchema := doc.Components.Schemas["ClusterSpec"] + if clusterSpecSchema == nil { + return nil, fmt.Errorf("ClusterSpec schema not found in OpenAPI spec") + } + + // Extract NodePoolSpec schema + nodePoolSpecSchema := doc.Components.Schemas["NodePoolSpec"] + if nodePoolSpecSchema == nil { + return nil, fmt.Errorf("NodePoolSpec schema not found in OpenAPI spec") + } + + // Build schemas map + schemas := map[string]*ResourceSchema{ + "cluster": { + TypeName: "ClusterSpec", + Schema: clusterSpecSchema, + }, + "nodepool": { + TypeName: "NodePoolSpec", + Schema: nodePoolSpecSchema, + }, + } + + return &SchemaValidator{ + doc: doc, + schemas: schemas, + }, nil +} + +// Validate validates a spec for the given resource type +// Returns nil if resourceType is not found in schemas (allows graceful handling) +func (v *SchemaValidator) Validate(resourceType string, spec map[string]interface{}) error { + resourceSchema := v.schemas[resourceType] + if resourceSchema == nil { + // Unknown resource type, skip validation + return nil + } + + return v.validateSpec(spec, resourceSchema.Schema, resourceSchema.TypeName) +} + +// ValidateClusterSpec validates a cluster spec against the ClusterSpec schema +// Deprecated: Use Validate("cluster", spec) instead +func (v *SchemaValidator) ValidateClusterSpec(spec map[string]interface{}) error { + return v.Validate("cluster", spec) +} + +// ValidateNodePoolSpec validates a nodepool spec against the NodePoolSpec schema +// Deprecated: Use Validate("nodepool", spec) instead +func (v *SchemaValidator) ValidateNodePoolSpec(spec map[string]interface{}) error { + return v.Validate("nodepool", spec) +} + +// validateSpec performs the actual validation and converts errors to our error format +func (v *SchemaValidator) validateSpec(spec map[string]interface{}, schemaRef *openapi3.SchemaRef, specTypeName string) error { + // Cast spec to interface{} for VisitJSON + var specData interface{} = spec + + // Validate against schema + if err := schemaRef.Value.VisitJSON(specData); err != nil { + // Convert validation error to our error format with details + validationDetails := convertValidationError(err, "spec") + return errors.ValidationWithDetails( + fmt.Sprintf("Invalid %s", specTypeName), + validationDetails, + ) + } + + return nil +} + +// convertValidationError converts OpenAPI validation errors to our ValidationDetail format +func convertValidationError(err error, prefix string) []errors.ValidationDetail { + var details []errors.ValidationDetail + + switch e := err.(type) { + case openapi3.MultiError: + // Recursively process each sub-error + for _, subErr := range e { + subDetails := convertValidationError(subErr, prefix) + details = append(details, subDetails...) + } + case *openapi3.SchemaError: + // Extract field path from SchemaError + field := prefix + + // Use JSONPointer which contains the actual data path + // JSONPointer returns the path like ["platform", "gcp", "diskSize"] + if len(e.JSONPointer()) > 0 { + jsonPath := strings.Join(e.JSONPointer(), ".") + if jsonPath != "" { + field = prefix + "." + jsonPath + } + } + + // Use the error message (Reason) which already contains field information + // Examples: + // - "property 'region' is missing" + // - "property 'unknownField' is unsupported" + // - "number must be at least 10" + details = append(details, errors.ValidationDetail{ + Field: field, + Error: e.Reason, + }) + default: + // Fallback for unknown error types + // Error message already contains the full description + details = append(details, errors.ValidationDetail{ + Field: prefix, + Error: err.Error(), + }) + } + + return details +} diff --git a/pkg/validators/schema_validator_test.go b/pkg/validators/schema_validator_test.go new file mode 100644 index 0000000..34ca2de --- /dev/null +++ b/pkg/validators/schema_validator_test.go @@ -0,0 +1,357 @@ +package validators + +import ( + "os" + "path/filepath" + "testing" + + . "github.com/onsi/gomega" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" +) + +const testSchema = ` +openapi: 3.0.0 +info: + title: Test Schema + version: 1.0.0 +paths: {} +components: + schemas: + ClusterSpec: + type: object + required: + - region + - provider + properties: + region: + type: string + enum: [us-central1, us-east1, europe-west1] + provider: + type: string + enum: [gcp, aws] + network: + type: object + properties: + vpc_id: + type: string + minLength: 1 + + NodePoolSpec: + type: object + required: + - machine_type + - replicas + properties: + machine_type: + type: string + minLength: 1 + replicas: + type: integer + minimum: 1 + maximum: 100 + autoscaling: + type: boolean +` + +func TestNewSchemaValidator(t *testing.T) { + RegisterTestingT(t) + + // Create temporary schema file + tmpDir := t.TempDir() + schemaPath := filepath.Join(tmpDir, "test-schema.yaml") + err := os.WriteFile(schemaPath, []byte(testSchema), 0644) + Expect(err).To(BeNil()) + + // Test successful schema loading + validator, err := NewSchemaValidator(schemaPath) + Expect(err).To(BeNil()) + Expect(validator).ToNot(BeNil()) + Expect(validator.doc).ToNot(BeNil()) + Expect(validator.schemas).ToNot(BeNil()) + Expect(validator.schemas["cluster"]).ToNot(BeNil()) + Expect(validator.schemas["cluster"].Schema).ToNot(BeNil()) + Expect(validator.schemas["cluster"].TypeName).To(Equal("ClusterSpec")) + Expect(validator.schemas["nodepool"]).ToNot(BeNil()) + Expect(validator.schemas["nodepool"].Schema).ToNot(BeNil()) + Expect(validator.schemas["nodepool"].TypeName).To(Equal("NodePoolSpec")) +} + +func TestNewSchemaValidator_InvalidPath(t *testing.T) { + RegisterTestingT(t) + + // Test with non-existent file + _, err := NewSchemaValidator("/nonexistent/path/schema.yaml") + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("failed to load OpenAPI schema")) +} + +func TestNewSchemaValidator_MissingSchemas(t *testing.T) { + RegisterTestingT(t) + + // Schema without required components + invalidSchema := ` +openapi: 3.0.0 +info: + title: Invalid Schema + version: 1.0.0 +paths: {} +components: + schemas: + SomeOtherSchema: + type: object +` + + tmpDir := t.TempDir() + schemaPath := filepath.Join(tmpDir, "invalid-schema.yaml") + err := os.WriteFile(schemaPath, []byte(invalidSchema), 0644) + Expect(err).To(BeNil()) + + // Should fail because ClusterSpec is missing + _, err = NewSchemaValidator(schemaPath) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("ClusterSpec schema not found")) +} + +func TestValidateClusterSpec_Valid(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test valid cluster spec + validSpec := map[string]interface{}{ + "region": "us-central1", + "provider": "gcp", + } + + err := validator.ValidateClusterSpec(validSpec) + Expect(err).To(BeNil()) +} + +func TestValidateClusterSpec_ValidWithOptionalFields(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test valid cluster spec with optional network field + validSpec := map[string]interface{}{ + "region": "europe-west1", + "provider": "aws", + "network": map[string]interface{}{ + "vpc_id": "vpc-12345", + }, + } + + err := validator.ValidateClusterSpec(validSpec) + Expect(err).To(BeNil()) +} + +func TestValidateClusterSpec_MissingRequiredField(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test spec missing required field + invalidSpec := map[string]interface{}{ + "region": "us-central1", + // missing "provider" + } + + err := validator.ValidateClusterSpec(invalidSpec) + Expect(err).ToNot(BeNil()) + + // Check that error is a ServiceError with details + serviceErr := getServiceError(err) + Expect(serviceErr).ToNot(BeNil()) + Expect(serviceErr.Details).ToNot(BeEmpty()) +} + +func TestValidateClusterSpec_InvalidEnum(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test spec with invalid enum value + invalidSpec := map[string]interface{}{ + "region": "asia-southeast1", // not in enum + "provider": "gcp", + } + + err := validator.ValidateClusterSpec(invalidSpec) + Expect(err).ToNot(BeNil()) + + serviceErr := getServiceError(err) + Expect(serviceErr).ToNot(BeNil()) + Expect(serviceErr.Details).ToNot(BeEmpty()) + + // Verify we get validation details (field path extraction is tested separately) + Expect(serviceErr.Details[0].Error).ToNot(BeEmpty()) +} + +func TestValidateClusterSpec_InvalidNestedField(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test spec with invalid nested field (empty vpc_id) + invalidSpec := map[string]interface{}{ + "region": "us-central1", + "provider": "gcp", + "network": map[string]interface{}{ + "vpc_id": "", // violates minLength: 1 + }, + } + + err := validator.ValidateClusterSpec(invalidSpec) + Expect(err).ToNot(BeNil()) + + serviceErr := getServiceError(err) + Expect(serviceErr).ToNot(BeNil()) +} + +func TestValidateNodePoolSpec_Valid(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test valid nodepool spec + validSpec := map[string]interface{}{ + "machine_type": "n1-standard-4", + "replicas": 3, + } + + err := validator.ValidateNodePoolSpec(validSpec) + Expect(err).To(BeNil()) +} + +func TestValidateNodePoolSpec_ValidWithOptional(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test valid nodepool spec with optional autoscaling + validSpec := map[string]interface{}{ + "machine_type": "n1-standard-4", + "replicas": 5, + "autoscaling": true, + } + + err := validator.ValidateNodePoolSpec(validSpec) + Expect(err).To(BeNil()) +} + +func TestValidateNodePoolSpec_MissingRequiredField(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test spec missing required field + invalidSpec := map[string]interface{}{ + "machine_type": "n1-standard-4", + // missing "replicas" + } + + err := validator.ValidateNodePoolSpec(invalidSpec) + Expect(err).ToNot(BeNil()) + + serviceErr := getServiceError(err) + Expect(serviceErr).ToNot(BeNil()) + Expect(serviceErr.Details).ToNot(BeEmpty()) +} + +func TestValidateNodePoolSpec_InvalidType(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test spec with wrong type (replicas should be integer) + invalidSpec := map[string]interface{}{ + "machine_type": "n1-standard-4", + "replicas": "three", // should be integer + } + + err := validator.ValidateNodePoolSpec(invalidSpec) + Expect(err).ToNot(BeNil()) + + serviceErr := getServiceError(err) + Expect(serviceErr).ToNot(BeNil()) +} + +func TestValidateNodePoolSpec_OutOfRange(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test spec with value out of range + invalidSpec := map[string]interface{}{ + "machine_type": "n1-standard-4", + "replicas": 150, // exceeds maximum: 100 + } + + err := validator.ValidateNodePoolSpec(invalidSpec) + Expect(err).ToNot(BeNil()) + + serviceErr := getServiceError(err) + Expect(serviceErr).ToNot(BeNil()) +} + +func TestValidateNodePoolSpec_BelowMinimum(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test spec with value below minimum + invalidSpec := map[string]interface{}{ + "machine_type": "n1-standard-4", + "replicas": 0, // below minimum: 1 + } + + err := validator.ValidateNodePoolSpec(invalidSpec) + Expect(err).ToNot(BeNil()) +} + +func TestValidateNodePoolSpec_EmptyMachineType(t *testing.T) { + RegisterTestingT(t) + + validator := setupTestValidator(t) + + // Test spec with empty machine_type (violates minLength) + invalidSpec := map[string]interface{}{ + "machine_type": "", + "replicas": 3, + } + + err := validator.ValidateNodePoolSpec(invalidSpec) + Expect(err).ToNot(BeNil()) +} + +// Helper functions + +func setupTestValidator(t *testing.T) *SchemaValidator { + tmpDir := t.TempDir() + schemaPath := filepath.Join(tmpDir, "test-schema.yaml") + err := os.WriteFile(schemaPath, []byte(testSchema), 0644) + if err != nil { + t.Fatalf("Failed to create test schema: %v", err) + } + + validator, err := NewSchemaValidator(schemaPath) + if err != nil { + t.Fatalf("Failed to create validator: %v", err) + } + + return validator +} + +func getServiceError(err error) *errors.ServiceError { + if err == nil { + return nil + } + + // Import errors package at the top + // This is a type assertion to check if err is a *errors.ServiceError + if serviceErr, ok := err.(*errors.ServiceError); ok { + return serviceErr + } + + return nil +} diff --git a/test/integration/adapter_status_test.go b/test/integration/adapter_status_test.go index ab2215f..af6936a 100644 --- a/test/integration/adapter_status_test.go +++ b/test/integration/adapter_status_test.go @@ -220,7 +220,7 @@ func TestAdapterStatusIdempotency(t *testing.T) { Expect(err).NotTo(HaveOccurred()) Expect(resp.StatusCode).To(Equal(http.StatusCreated)) Expect(status1.Adapter).To(Equal("idempotency-test-adapter")) - Expect(status1.Conditions[0].Status).To(Equal("False")) + Expect(string(status1.Conditions[0].Status)).To(Equal(string(openapi.FALSE))) // Second POST: Update the same adapter with different conditions statusInput2 := openapi.AdapterStatusCreateRequest{ @@ -242,7 +242,7 @@ func TestAdapterStatusIdempotency(t *testing.T) { Expect(err).NotTo(HaveOccurred()) Expect(resp.StatusCode).To(Equal(http.StatusCreated)) Expect(status2.Adapter).To(Equal("idempotency-test-adapter")) - Expect(status2.Conditions[0].Status).To(Equal("True")) + Expect(status2.Conditions[0].Status).To(Equal(openapi.TRUE)) // GET all statuses - should have only ONE status for "idempotency-test-adapter" list, _, err := client.DefaultAPI.GetClusterStatuses(ctx, cluster.ID).Execute() @@ -260,7 +260,7 @@ func TestAdapterStatusIdempotency(t *testing.T) { // Verify: should have exactly ONE entry for this adapter (updated, not duplicated) Expect(adapterCount).To(Equal(1), "Adapter should be updated, not duplicated") - Expect(finalStatus.Conditions[0].Status).To(Equal("True"), "Conditions should be updated to latest") + Expect(finalStatus.Conditions[0].Status).To(Equal(openapi.TRUE), "Conditions should be updated to latest") } // TestAdapterStatusPagingEdgeCases tests edge cases in pagination diff --git a/test/integration/clusters_test.go b/test/integration/clusters_test.go index 02b6980..39ac38e 100644 --- a/test/integration/clusters_test.go +++ b/test/integration/clusters_test.go @@ -2,9 +2,14 @@ package integration import ( "context" + "encoding/json" "fmt" + "io" "net/http" + "os" + "strings" "testing" + "time" . "github.com/onsi/gomega" "gopkg.in/resty.v1" @@ -275,3 +280,345 @@ func TestClusterBoundaryValues(t *testing.T) { Expect(resp.StatusCode).To(Equal(http.StatusCreated)) Expect(unicodeNameCluster.Name).To(Equal("テスト-δοκιμή-🚀")) } + +// TestClusterSchemaValidation tests schema validation for cluster specs +// Note: This test validates against the base openapi.yaml schema which has an empty ClusterSpec +// The base schema accepts any JSON object, so this test mainly verifies the middleware is working +func TestClusterSchemaValidation(t *testing.T) { + RegisterTestingT(t) + h, client := test.RegisterIntegration(t) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + + // Test 1: Valid cluster spec (base schema accepts any object) + validSpec := map[string]interface{}{ + "region": "us-central1", + "provider": "gcp", + } + + validInput := openapi.ClusterCreateRequest{ + Kind: "Cluster", + Name: "schema-valid-test", + Spec: validSpec, + } + + cluster, resp, err := client.DefaultAPI.PostCluster(ctx).ClusterCreateRequest(validInput).Execute() + Expect(err).NotTo(HaveOccurred(), "Valid spec should be accepted") + Expect(resp.StatusCode).To(Equal(http.StatusCreated)) + Expect(*cluster.Id).NotTo(BeEmpty()) + + // Test 2: Invalid spec type (spec must be object, not string) + // This should fail even with base schema + // Can't use the generated struct because Spec is typed as map[string]interface{} + // So we send raw JSON request + invalidTypeJSON := `{ + "kind": "Cluster", + "name": "schema-invalid-type", + "spec": "invalid-string-spec" + }` + + jwtToken := ctx.Value(openapi.ContextAccessToken) + + resp2, err := resty.R(). + SetHeader("Content-Type", "application/json"). + SetHeader("Authorization", fmt.Sprintf("Bearer %s", jwtToken)). + SetBody(invalidTypeJSON). + Post(h.RestURL("/clusters")) + + if resp2.StatusCode() == http.StatusBadRequest { + t.Logf("Schema validation correctly rejected invalid spec type") + // Verify error response contains details + var errorResponse openapi.Error + json.Unmarshal(resp2.Body(), &errorResponse) + Expect(errorResponse.Code).ToNot(BeNil()) + Expect(errorResponse.Reason).ToNot(BeNil()) + } else { + t.Logf("Base schema may accept any spec type, status: %d", resp2.StatusCode()) + } + + // Test 3: Empty spec (should be valid as spec is optional in base schema) + emptySpecInput := openapi.ClusterCreateRequest{ + Kind: "Cluster", + Name: "schema-empty-spec", + Spec: map[string]interface{}{}, + } + + cluster3, resp3, err := client.DefaultAPI.PostCluster(ctx).ClusterCreateRequest(emptySpecInput).Execute() + Expect(err).NotTo(HaveOccurred(), "Empty spec should be accepted by base schema") + Expect(resp3.StatusCode).To(Equal(http.StatusCreated)) + Expect(*cluster3.Id).NotTo(BeEmpty()) +} + +// TestClusterSchemaValidationWithProviderSchema tests schema validation with a provider-specific schema +// This test will only work if OPENAPI_SCHEMA_PATH is set to a provider schema (e.g., gcp_openapi.yaml) +// When using the base schema, this test will be skipped +func TestClusterSchemaValidationWithProviderSchema(t *testing.T) { + RegisterTestingT(t) + + // Check if we're using a provider schema or base schema + // If base schema, skip detailed validation tests + schemaPath := os.Getenv("OPENAPI_SCHEMA_PATH") + if schemaPath == "" || strings.HasSuffix(schemaPath, "openapi/openapi.yaml") { + t.Skip("Skipping provider schema validation test - using base schema") + return + } + + h, client := test.RegisterIntegration(t) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + + // Test with provider-specific schema (assumes GCP schema for this example) + // If using a different provider, adjust the spec accordingly + + // Test 1: Invalid spec - missing required field + invalidSpec := map[string]interface{}{ + "gcp": map[string]interface{}{ + // Missing required "region" field + "zone": "us-central1-a", + }, + } + + invalidInput := openapi.ClusterCreateRequest{ + Kind: "Cluster", + Name: "provider-schema-invalid", + Spec: invalidSpec, + } + + _, resp, err := client.DefaultAPI.PostCluster(ctx).ClusterCreateRequest(invalidInput).Execute() + Expect(err).To(HaveOccurred(), "Should reject spec with missing required field") + Expect(resp.StatusCode).To(Equal(http.StatusBadRequest)) + defer resp.Body.Close() + + // Parse error response to verify field-level details + bodyBytes, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("failed to read response body: %v", err) + } + + var errorResponse openapi.Error + if err := json.Unmarshal(bodyBytes, &errorResponse); err != nil { + t.Fatalf("failed to unmarshal error response body: %v", err) + } + + Expect(errorResponse.Code).ToNot(BeNil()) + Expect(*errorResponse.Code).To(Equal("hyperfleet-8")) // Validation error code + Expect(errorResponse.Details).ToNot(BeEmpty(), "Should include field-level error details") + + // Verify details contain field path + foundRegionError := false + for _, detail := range errorResponse.Details { + if detail.Field != nil && strings.Contains(*detail.Field, "region") { + foundRegionError = true + break + } + } + Expect(foundRegionError).To(BeTrue(), "Error details should mention missing 'region' field") +} + +// TestClusterSchemaValidationErrorDetails tests that validation errors include detailed field information +func TestClusterSchemaValidationErrorDetails(t *testing.T) { + RegisterTestingT(t) + h, _ := test.RegisterIntegration(t) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + + // Send request with spec field as wrong type (not an object) + invalidTypeRequest := map[string]interface{}{ + "kind": "Cluster", + "name": "error-details-test", + "spec": "not-an-object", // Invalid type + } + + body, _ := json.Marshal(invalidTypeRequest) + jwtToken := ctx.Value(openapi.ContextAccessToken) + + resp, err := resty.R(). + SetHeader("Content-Type", "application/json"). + SetHeader("Authorization", fmt.Sprintf("Bearer %s", jwtToken)). + SetBody(body). + Post(h.RestURL("/clusters")) + + Expect(err).To(BeNil()) + + // Log response for debugging + t.Logf("Response status: %d, body: %s", resp.StatusCode(), string(resp.Body())) + + Expect(resp.StatusCode()).To(Equal(http.StatusBadRequest), "Should return 400 for invalid spec type") + + // Parse error response + var errorResponse openapi.Error + if err := json.Unmarshal(resp.Body(), &errorResponse); err != nil { + t.Fatalf("failed to unmarshal error response: %v, response body: %s", err, string(resp.Body())) + } + + // Verify error structure + Expect(errorResponse.Kind).ToNot(BeNil()) + Expect(*errorResponse.Kind).To(Equal("Error")) + + Expect(errorResponse.Code).ToNot(BeNil()) + // Both hyperfleet-8 (validation error) and hyperfleet-17 (invalid request format) are acceptable + // as they both indicate the spec field is invalid + validCodes := []string{"hyperfleet-8", "hyperfleet-17"} + Expect(validCodes).To(ContainElement(*errorResponse.Code), "Expected validation or format error code") + + Expect(errorResponse.Reason).ToNot(BeNil()) + Expect(*errorResponse.Reason).To(ContainSubstring("spec")) + + Expect(errorResponse.Href).ToNot(BeNil()) + Expect(errorResponse.OperationId).ToNot(BeNil()) + + t.Logf("Error response: code=%s, reason=%s", *errorResponse.Code, *errorResponse.Reason) +} + +// TestClusterList_DefaultSorting tests that clusters are sorted by created_time desc by default +func TestClusterList_DefaultSorting(t *testing.T) { + h, client := test.RegisterIntegration(t) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + + // Create 3 clusters with delays to ensure different timestamps + var createdClusters []openapi.Cluster + for i := 1; i <= 3; i++ { + clusterInput := openapi.ClusterCreateRequest{ + Kind: "Cluster", + Name: fmt.Sprintf("sort-test-%d-%s", i, h.NewID()), + Spec: map[string]interface{}{"test": fmt.Sprintf("value-%d", i)}, + } + + cluster, _, err := client.DefaultAPI.PostCluster(ctx).ClusterCreateRequest(clusterInput).Execute() + Expect(err).NotTo(HaveOccurred(), "Failed to create cluster %d", i) + createdClusters = append(createdClusters, *cluster) + + // Add 100ms delay to ensure different created_time + time.Sleep(100 * time.Millisecond) + } + + // List clusters without orderBy parameter - should default to created_time desc + list, _, err := client.DefaultAPI.GetClusters(ctx).Execute() + Expect(err).NotTo(HaveOccurred(), "Failed to list clusters") + Expect(len(list.Items)).To(BeNumerically(">=", 3), "Should have at least 3 clusters") + + // Find our test clusters in the response + var testClusters []openapi.Cluster + for _, item := range list.Items { + for _, created := range createdClusters { + if *item.Id == *created.Id { + testClusters = append(testClusters, item) + break + } + } + } + + Expect(len(testClusters)).To(Equal(3), "Should find all 3 test clusters") + + // Verify they are sorted by created_time desc (newest first) + // testClusters should be in reverse creation order + Expect(*testClusters[0].Id).To(Equal(*createdClusters[2].Id), "First cluster should be the last created") + Expect(*testClusters[1].Id).To(Equal(*createdClusters[1].Id), "Second cluster should be the middle created") + Expect(*testClusters[2].Id).To(Equal(*createdClusters[0].Id), "Third cluster should be the first created") + + t.Logf("✓ Default sorting works: clusters sorted by created_time desc") +} + +// TestClusterList_OrderByName tests custom sorting by name +func TestClusterList_OrderByName(t *testing.T) { + h, client := test.RegisterIntegration(t) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + + // Create clusters with names that will sort alphabetically + testPrefix := fmt.Sprintf("name-sort-%s", h.NewID()) + names := []string{ + fmt.Sprintf("%s-charlie", testPrefix), + fmt.Sprintf("%s-alpha", testPrefix), + fmt.Sprintf("%s-bravo", testPrefix), + } + + var createdClusters []openapi.Cluster + for _, name := range names { + clusterInput := openapi.ClusterCreateRequest{ + Kind: "Cluster", + Name: name, + Spec: map[string]interface{}{"test": "value"}, + } + + cluster, _, err := client.DefaultAPI.PostCluster(ctx).ClusterCreateRequest(clusterInput).Execute() + Expect(err).NotTo(HaveOccurred(), "Failed to create cluster %s", name) + createdClusters = append(createdClusters, *cluster) + } + + // List with orderBy=name asc + list, _, err := client.DefaultAPI.GetClusters(ctx).OrderBy("name asc").Execute() + Expect(err).NotTo(HaveOccurred(), "Failed to list clusters with orderBy") + Expect(len(list.Items)).To(BeNumerically(">=", 3), "Should have at least 3 clusters") + + // Find our test clusters in the response + var testClusters []openapi.Cluster + for _, item := range list.Items { + if strings.HasPrefix(item.Name, testPrefix) { + testClusters = append(testClusters, item) + } + } + + Expect(len(testClusters)).To(Equal(3), "Should find all 3 test clusters") + + // Verify they are sorted by name asc (alphabetically) + Expect(testClusters[0].Name).To(ContainSubstring("alpha"), "First should be alpha") + Expect(testClusters[1].Name).To(ContainSubstring("bravo"), "Second should be bravo") + Expect(testClusters[2].Name).To(ContainSubstring("charlie"), "Third should be charlie") + + t.Logf("✓ Custom sorting works: clusters sorted by name asc") +} + +// TestClusterList_OrderByNameDesc tests sorting by name descending +func TestClusterList_OrderByNameDesc(t *testing.T) { + h, client := test.RegisterIntegration(t) + + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + + // Create clusters with names that will sort alphabetically + testPrefix := fmt.Sprintf("desc-sort-%s", h.NewID()) + names := []string{ + fmt.Sprintf("%s-alpha", testPrefix), + fmt.Sprintf("%s-charlie", testPrefix), + fmt.Sprintf("%s-bravo", testPrefix), + } + + for _, name := range names { + clusterInput := openapi.ClusterCreateRequest{ + Kind: "Cluster", + Name: name, + Spec: map[string]interface{}{"test": "value"}, + } + + _, _, err := client.DefaultAPI.PostCluster(ctx).ClusterCreateRequest(clusterInput).Execute() + Expect(err).NotTo(HaveOccurred(), "Failed to create cluster %s", name) + } + + // List with orderBy=name desc + list, _, err := client.DefaultAPI.GetClusters(ctx).OrderBy("name desc").Execute() + Expect(err).NotTo(HaveOccurred(), "Failed to list clusters with orderBy desc") + + // Find our test clusters in the response + var testClusters []openapi.Cluster + for _, item := range list.Items { + if strings.HasPrefix(item.Name, testPrefix) { + testClusters = append(testClusters, item) + } + } + + Expect(len(testClusters)).To(Equal(3), "Should find all 3 test clusters") + + // Verify they are sorted by name desc (reverse alphabetically) + Expect(testClusters[0].Name).To(ContainSubstring("charlie"), "First should be charlie") + Expect(testClusters[1].Name).To(ContainSubstring("bravo"), "Second should be bravo") + Expect(testClusters[2].Name).To(ContainSubstring("alpha"), "Third should be alpha") + + t.Logf("✓ Descending sorting works: clusters sorted by name desc") +} diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index 096a8a3..e072c68 100755 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -3,6 +3,7 @@ package integration import ( "flag" "os" + "path/filepath" "runtime" "testing" @@ -14,6 +15,35 @@ import ( func TestMain(m *testing.M) { flag.Parse() glog.Infof("Starting integration test using go version %s", runtime.Version()) + + // Set OPENAPI_SCHEMA_PATH for integration tests if not already set + // This enables schema validation middleware during tests + if os.Getenv("OPENAPI_SCHEMA_PATH") == "" { + // Get the repo root directory (2 levels up from test/integration) + // Use runtime.Caller to find this file's path + _, filename, _, ok := runtime.Caller(0) + if !ok { + glog.Warningf("Failed to determine current file path via runtime.Caller, skipping OPENAPI_SCHEMA_PATH setup") + } else { + // filename is like: /path/to/repo/test/integration/integration_test.go + // Navigate up: integration_test.go -> integration -> test -> repo + integrationDir := filepath.Dir(filename) // /path/to/repo/test/integration + testDir := filepath.Dir(integrationDir) // /path/to/repo/test + repoRoot := filepath.Dir(testDir) // /path/to/repo + + // Build schema path using filepath.Join for cross-platform compatibility + schemaPath := filepath.Join(repoRoot, "openapi", "openapi.yaml") + + // Verify the schema file exists before setting the env var + if _, err := os.Stat(schemaPath); err != nil { + glog.Warningf("Schema file not found at %s: %v, skipping OPENAPI_SCHEMA_PATH setup", schemaPath, err) + } else { + os.Setenv("OPENAPI_SCHEMA_PATH", schemaPath) + glog.Infof("Set OPENAPI_SCHEMA_PATH=%s for integration tests", schemaPath) + } + } + } + helper := test.NewHelper(&testing.T{}) exitCode := m.Run() helper.Teardown()