From 2f86349e2c5504d5b8b2134dddca65e4ed190bf5 Mon Sep 17 00:00:00 2001 From: rshoemaker Date: Fri, 3 Apr 2026 14:04:41 -0400 Subject: [PATCH] fix: make service config optional in API design Remove config from Required fields in the Goa DSL and add omitempty to the JSON tag. The custom validation layer already handles nil configs with sensible defaults, but the generated validation was rejecting requests before custom parsing could run. Normalize nil config to an empty map in the API conversion layer so downstream code never needs nil checks. --- api/apiv1/design/database.go | 4 +-- api/apiv1/gen/control_plane/service.go | 2 +- .../control_plane/client/encode_decode.go | 12 ++++---- .../gen/http/control_plane/client/types.go | 12 ++------ .../control_plane/server/encode_decode.go | 24 ++++++++------- .../gen/http/control_plane/server/types.go | 12 ++------ api/apiv1/gen/http/openapi.json | 3 +- api/apiv1/gen/http/openapi.yaml | 1 - api/apiv1/gen/http/openapi3.json | 21 +++++-------- api/apiv1/gen/http/openapi3.yaml | 7 ----- docs/services/managing.md | 2 +- server/internal/api/apiv1/convert.go | 11 ++++++- server/internal/api/apiv1/convert_test.go | 20 +++++++++++++ server/internal/api/apiv1/validate_test.go | 30 +++++++++++++++++++ 14 files changed, 99 insertions(+), 62 deletions(-) diff --git a/api/apiv1/design/database.go b/api/apiv1/design/database.go index 60799ada..75e03680 100644 --- a/api/apiv1/design/database.go +++ b/api/apiv1/design/database.go @@ -199,7 +199,7 @@ var ServiceSpec = g.Type("ServiceSpec", func() { "llm_model": "gpt-4", "openai_api_key": "sk-...", }) - g.Meta("struct:tag:json", "config") + g.Meta("struct:tag:json", "config,omitempty") }) g.Attribute("cpus", g.String, func() { g.Description("The number of CPUs to allocate for this service. It can include the SI suffix 'm', e.g. '500m' for 500 millicpus. Defaults to container defaults if unspecified.") @@ -225,7 +225,7 @@ var ServiceSpec = g.Type("ServiceSpec", func() { g.Meta("struct:tag:json", "database_connection,omitempty") }) - g.Required("service_id", "service_type", "version", "host_ids", "config") + g.Required("service_id", "service_type", "version", "host_ids") }) var BackupRepositorySpec = g.Type("BackupRepositorySpec", func() { diff --git a/api/apiv1/gen/control_plane/service.go b/api/apiv1/gen/control_plane/service.go index bdab95dd..c190a867 100644 --- a/api/apiv1/gen/control_plane/service.go +++ b/api/apiv1/gen/control_plane/service.go @@ -980,7 +980,7 @@ type ServiceSpec struct { Port *int `json:"port,omitempty"` // Service-specific configuration. For MCP services, this includes // llm_provider, llm_model, and provider-specific API keys. - Config map[string]any `json:"config"` + Config map[string]any `json:"config,omitempty"` // The number of CPUs to allocate for this service. It can include the SI // suffix 'm', e.g. '500m' for 500 millicpus. Defaults to container defaults if // unspecified. diff --git a/api/apiv1/gen/http/control_plane/client/encode_decode.go b/api/apiv1/gen/http/control_plane/client/encode_decode.go index 48cd9765..155e64f6 100644 --- a/api/apiv1/gen/http/control_plane/client/encode_decode.go +++ b/api/apiv1/gen/http/control_plane/client/encode_decode.go @@ -5724,11 +5724,13 @@ func unmarshalServiceSpecResponseBodyToControlplaneServiceSpec(v *ServiceSpecRes for i, val := range v.HostIds { res.HostIds[i] = controlplane.Identifier(val) } - res.Config = make(map[string]any, len(v.Config)) - for key, val := range v.Config { - tk := key - tv := val - res.Config[tk] = tv + if v.Config != nil { + res.Config = make(map[string]any, len(v.Config)) + for key, val := range v.Config { + tk := key + tv := val + res.Config[tk] = tv + } } if v.OrchestratorOpts != nil { res.OrchestratorOpts = unmarshalOrchestratorOptsResponseBodyToControlplaneOrchestratorOpts(v.OrchestratorOpts) diff --git a/api/apiv1/gen/http/control_plane/client/types.go b/api/apiv1/gen/http/control_plane/client/types.go index ccfe9b5b..c4dd22f3 100644 --- a/api/apiv1/gen/http/control_plane/client/types.go +++ b/api/apiv1/gen/http/control_plane/client/types.go @@ -2093,7 +2093,7 @@ type ServiceSpecRequestBody struct { Port *int `json:"port,omitempty"` // Service-specific configuration. For MCP services, this includes // llm_provider, llm_model, and provider-specific API keys. - Config map[string]any `json:"config"` + Config map[string]any `json:"config,omitempty"` // The number of CPUs to allocate for this service. It can include the SI // suffix 'm', e.g. '500m' for 500 millicpus. Defaults to container defaults if // unspecified. @@ -2493,7 +2493,7 @@ type ServiceSpecResponseBody struct { Port *int `json:"port,omitempty"` // Service-specific configuration. For MCP services, this includes // llm_provider, llm_model, and provider-specific API keys. - Config map[string]any `json:"config"` + Config map[string]any `json:"config,omitempty"` // The number of CPUs to allocate for this service. It can include the SI // suffix 'm', e.g. '500m' for 500 millicpus. Defaults to container defaults if // unspecified. @@ -2821,7 +2821,7 @@ type ServiceSpecRequestBodyRequestBody struct { Port *int `json:"port,omitempty"` // Service-specific configuration. For MCP services, this includes // llm_provider, llm_model, and provider-specific API keys. - Config map[string]any `json:"config"` + Config map[string]any `json:"config,omitempty"` // The number of CPUs to allocate for this service. It can include the SI // suffix 'm', e.g. '500m' for 500 millicpus. Defaults to container defaults if // unspecified. @@ -6241,9 +6241,6 @@ func ValidateServiceSpecRequestBody(body *ServiceSpecRequestBody) (err error) { if body.HostIds == nil { err = goa.MergeErrors(err, goa.MissingFieldError("host_ids", "body")) } - if body.Config == nil { - err = goa.MergeErrors(err, goa.MissingFieldError("config", "body")) - } if utf8.RuneCountInString(body.ServiceID) < 1 { err = goa.MergeErrors(err, goa.InvalidLengthError("body.service_id", body.ServiceID, utf8.RuneCountInString(body.ServiceID), 1, true)) } @@ -7005,9 +7002,6 @@ func ValidateServiceSpecRequestBodyRequestBody(body *ServiceSpecRequestBodyReque if body.HostIds == nil { err = goa.MergeErrors(err, goa.MissingFieldError("host_ids", "body")) } - if body.Config == nil { - err = goa.MergeErrors(err, goa.MissingFieldError("config", "body")) - } if utf8.RuneCountInString(body.ServiceID) < 1 { err = goa.MergeErrors(err, goa.InvalidLengthError("body.service_id", body.ServiceID, utf8.RuneCountInString(body.ServiceID), 1, true)) } diff --git a/api/apiv1/gen/http/control_plane/server/encode_decode.go b/api/apiv1/gen/http/control_plane/server/encode_decode.go index bdcb2b76..b8349a24 100644 --- a/api/apiv1/gen/http/control_plane/server/encode_decode.go +++ b/api/apiv1/gen/http/control_plane/server/encode_decode.go @@ -4083,11 +4083,13 @@ func unmarshalServiceSpecRequestBodyToControlplaneServiceSpec(v *ServiceSpecRequ for i, val := range v.HostIds { res.HostIds[i] = controlplane.Identifier(val) } - res.Config = make(map[string]any, len(v.Config)) - for key, val := range v.Config { - tk := key - tv := val - res.Config[tk] = tv + if v.Config != nil { + res.Config = make(map[string]any, len(v.Config)) + for key, val := range v.Config { + tk := key + tv := val + res.Config[tk] = tv + } } if v.OrchestratorOpts != nil { res.OrchestratorOpts = unmarshalOrchestratorOptsRequestBodyToControlplaneOrchestratorOpts(v.OrchestratorOpts) @@ -5087,11 +5089,13 @@ func unmarshalServiceSpecRequestBodyRequestBodyToControlplaneServiceSpec(v *Serv for i, val := range v.HostIds { res.HostIds[i] = controlplane.Identifier(val) } - res.Config = make(map[string]any, len(v.Config)) - for key, val := range v.Config { - tk := key - tv := val - res.Config[tk] = tv + if v.Config != nil { + res.Config = make(map[string]any, len(v.Config)) + for key, val := range v.Config { + tk := key + tv := val + res.Config[tk] = tv + } } if v.OrchestratorOpts != nil { res.OrchestratorOpts = unmarshalOrchestratorOptsRequestBodyRequestBodyToControlplaneOrchestratorOpts(v.OrchestratorOpts) diff --git a/api/apiv1/gen/http/control_plane/server/types.go b/api/apiv1/gen/http/control_plane/server/types.go index e8f68d2e..26fd1bdb 100644 --- a/api/apiv1/gen/http/control_plane/server/types.go +++ b/api/apiv1/gen/http/control_plane/server/types.go @@ -2177,7 +2177,7 @@ type ServiceSpecResponseBody struct { Port *int `json:"port,omitempty"` // Service-specific configuration. For MCP services, this includes // llm_provider, llm_model, and provider-specific API keys. - Config map[string]any `json:"config"` + Config map[string]any `json:"config,omitempty"` // The number of CPUs to allocate for this service. It can include the SI // suffix 'm', e.g. '500m' for 500 millicpus. Defaults to container defaults if // unspecified. @@ -2504,7 +2504,7 @@ type ServiceSpecRequestBody struct { Port *int `json:"port,omitempty"` // Service-specific configuration. For MCP services, this includes // llm_provider, llm_model, and provider-specific API keys. - Config map[string]any `json:"config"` + Config map[string]any `json:"config,omitempty"` // The number of CPUs to allocate for this service. It can include the SI // suffix 'm', e.g. '500m' for 500 millicpus. Defaults to container defaults if // unspecified. @@ -2831,7 +2831,7 @@ type ServiceSpecRequestBodyRequestBody struct { Port *int `json:"port,omitempty"` // Service-specific configuration. For MCP services, this includes // llm_provider, llm_model, and provider-specific API keys. - Config map[string]any `json:"config"` + Config map[string]any `json:"config,omitempty"` // The number of CPUs to allocate for this service. It can include the SI // suffix 'm', e.g. '500m' for 500 millicpus. Defaults to container defaults if // unspecified. @@ -5744,9 +5744,6 @@ func ValidateServiceSpecRequestBody(body *ServiceSpecRequestBody) (err error) { if body.HostIds == nil { err = goa.MergeErrors(err, goa.MissingFieldError("host_ids", "body")) } - if body.Config == nil { - err = goa.MergeErrors(err, goa.MissingFieldError("config", "body")) - } if body.ServiceID != nil { if utf8.RuneCountInString(*body.ServiceID) < 1 { err = goa.MergeErrors(err, goa.InvalidLengthError("body.service_id", *body.ServiceID, utf8.RuneCountInString(*body.ServiceID), 1, true)) @@ -6486,9 +6483,6 @@ func ValidateServiceSpecRequestBodyRequestBody(body *ServiceSpecRequestBodyReque if body.HostIds == nil { err = goa.MergeErrors(err, goa.MissingFieldError("host_ids", "body")) } - if body.Config == nil { - err = goa.MergeErrors(err, goa.MissingFieldError("config", "body")) - } if body.ServiceID != nil { if utf8.RuneCountInString(*body.ServiceID) < 1 { err = goa.MergeErrors(err, goa.InvalidLengthError("body.service_id", *body.ServiceID, utf8.RuneCountInString(*body.ServiceID), 1, true)) diff --git a/api/apiv1/gen/http/openapi.json b/api/apiv1/gen/http/openapi.json index 9c7508a0..66ec5162 100644 --- a/api/apiv1/gen/http/openapi.json +++ b/api/apiv1/gen/http/openapi.json @@ -8893,8 +8893,7 @@ "service_id", "service_type", "version", - "host_ids", - "config" + "host_ids" ] }, "StartInstanceResponse": { diff --git a/api/apiv1/gen/http/openapi.yaml b/api/apiv1/gen/http/openapi.yaml index b17cd5d4..98c4079d 100644 --- a/api/apiv1/gen/http/openapi.yaml +++ b/api/apiv1/gen/http/openapi.yaml @@ -6383,7 +6383,6 @@ definitions: - service_type - version - host_ids - - config StartInstanceResponse: title: StartInstanceResponse type: object diff --git a/api/apiv1/gen/http/openapi3.json b/api/apiv1/gen/http/openapi3.json index 5446d989..26b7a310 100644 --- a/api/apiv1/gen/http/openapi3.json +++ b/api/apiv1/gen/http/openapi3.json @@ -28158,8 +28158,7 @@ "service_id", "service_type", "version", - "host_ids", - "config" + "host_ids" ] }, "ServiceSpec2": { @@ -28323,8 +28322,7 @@ "service_id", "service_type", "version", - "host_ids", - "config" + "host_ids" ] }, "ServiceSpec3": { @@ -28488,8 +28486,7 @@ "service_id", "service_type", "version", - "host_ids", - "config" + "host_ids" ] }, "ServiceSpec4": { @@ -28653,8 +28650,7 @@ "service_id", "service_type", "version", - "host_ids", - "config" + "host_ids" ] }, "ServiceSpec5": { @@ -28818,8 +28814,7 @@ "service_id", "service_type", "version", - "host_ids", - "config" + "host_ids" ] }, "ServiceSpec6": { @@ -28984,8 +28979,7 @@ "service_id", "service_type", "version", - "host_ids", - "config" + "host_ids" ] }, "ServiceSpec7": { @@ -29147,8 +29141,7 @@ "service_id", "service_type", "version", - "host_ids", - "config" + "host_ids" ] }, "StartInstanceResponse": { diff --git a/api/apiv1/gen/http/openapi3.yaml b/api/apiv1/gen/http/openapi3.yaml index faa924e3..8bd8c25c 100644 --- a/api/apiv1/gen/http/openapi3.yaml +++ b/api/apiv1/gen/http/openapi3.yaml @@ -19929,7 +19929,6 @@ components: - service_type - version - host_ids - - config ServiceSpec2: type: object properties: @@ -20049,7 +20048,6 @@ components: - service_type - version - host_ids - - config ServiceSpec3: type: object properties: @@ -20169,7 +20167,6 @@ components: - service_type - version - host_ids - - config ServiceSpec4: type: object properties: @@ -20289,7 +20286,6 @@ components: - service_type - version - host_ids - - config ServiceSpec5: type: object properties: @@ -20409,7 +20405,6 @@ components: - service_type - version - host_ids - - config ServiceSpec6: type: object properties: @@ -20530,7 +20525,6 @@ components: - service_type - version - host_ids - - config ServiceSpec7: type: object properties: @@ -20648,7 +20642,6 @@ components: - service_type - version - host_ids - - config StartInstanceResponse: type: object properties: diff --git a/docs/services/managing.md b/docs/services/managing.md index cbb1dec4..d97a3307 100644 --- a/docs/services/managing.md +++ b/docs/services/managing.md @@ -16,7 +16,7 @@ The following table describes the fields in a service spec: | `service_type` | string | Yes | The type of service to run. One of: `mcp`, `rag`, `postgrest`. | | `version` | string | Yes | The service version in semver format (e.g., `1.0.0`) or the literal `latest`. | | `host_ids` | array | Yes | The IDs of the hosts to run this service on. One instance is created per host. | -| `config` | object | Yes | Service-type-specific configuration. See the page for your service type for valid fields. | +| `config` | object | No | Service-type-specific configuration. See the page for your service type for valid fields. When omitted, the service uses sensible defaults. | | `port` | integer | No | Host port to publish the service on. Set to `0` to let Docker assign a random port. When omitted, the service is not reachable from outside the Docker network. | | `cpus` | string | No | CPU limit for the service container. Accepts a decimal (e.g., `"0.5"`) or millicpu suffix (e.g., `"500m"`). Defaults to container defaults if unspecified. | | `memory` | string | No | Memory limit for the service container in SI or IEC notation (e.g., `"512M"`, `"1GiB"`). Defaults to container defaults if unspecified. | diff --git a/server/internal/api/apiv1/convert.go b/server/internal/api/apiv1/convert.go index 71c46b2f..f5545d15 100644 --- a/server/internal/api/apiv1/convert.go +++ b/server/internal/api/apiv1/convert.go @@ -44,6 +44,15 @@ func isSensitiveConfigKey(key string) bool { return false } +// normalizeConfig ensures a nil config map is converted to an empty map so +// downstream code never has to nil-check. +func normalizeConfig(config map[string]any) map[string]any { + if config == nil { + return map[string]any{} + } + return config +} + // scrubSensitiveConfig returns a copy of config with sensitive keys removed, // recursively descending into nested maps and slices. func scrubSensitiveConfig(config map[string]any) map[string]any { @@ -685,7 +694,7 @@ func apiToServiceSpec(apiSvc *api.ServiceSpec) (*database.ServiceSpec, error) { Version: apiSvc.Version, HostIDs: hostIDs, Port: apiSvc.Port, - Config: apiSvc.Config, + Config: normalizeConfig(apiSvc.Config), CPUs: cpus, MemoryBytes: memory, OrchestratorOpts: orchestratorOptsToDatabase(apiSvc.OrchestratorOpts), diff --git a/server/internal/api/apiv1/convert_test.go b/server/internal/api/apiv1/convert_test.go index 7386d77f..4e1df62b 100644 --- a/server/internal/api/apiv1/convert_test.go +++ b/server/internal/api/apiv1/convert_test.go @@ -31,3 +31,23 @@ func TestIsSensitiveConfigKey(t *testing.T) { } } } + +func TestNormalizeConfig(t *testing.T) { + t.Run("nil becomes empty map", func(t *testing.T) { + result := normalizeConfig(nil) + if result == nil { + t.Fatal("normalizeConfig(nil) returned nil, want empty map") + } + if len(result) != 0 { + t.Errorf("normalizeConfig(nil) returned map with %d entries, want 0", len(result)) + } + }) + + t.Run("non-nil map is returned as-is", func(t *testing.T) { + input := map[string]any{"key": "value"} + result := normalizeConfig(input) + if result["key"] != "value" { + t.Errorf("normalizeConfig did not preserve existing entries") + } + }) +} diff --git a/server/internal/api/apiv1/validate_test.go b/server/internal/api/apiv1/validate_test.go index e3980c3c..1c4c953b 100644 --- a/server/internal/api/apiv1/validate_test.go +++ b/server/internal/api/apiv1/validate_test.go @@ -933,6 +933,36 @@ func TestValidateServiceSpec(t *testing.T) { `service_type: unsupported service type "unknown"`, }, }, + { + name: "valid MCP service with nil config", + svc: &api.ServiceSpec{ + ServiceID: "mcp-server", + ServiceType: "mcp", + Version: "1.0.0", + HostIds: []api.Identifier{"host-1"}, + }, + }, + { + name: "valid postgrest with nil config", + svc: &api.ServiceSpec{ + ServiceID: "my-postgrest", + ServiceType: "postgrest", + Version: "latest", + HostIds: []api.Identifier{"host-1"}, + }, + }, + { + name: "RAG service with nil config requires pipelines", + svc: &api.ServiceSpec{ + ServiceID: "my-rag", + ServiceType: "rag", + Version: "1.0.0", + HostIds: []api.Identifier{"host-1"}, + }, + expected: []string{ + "pipelines is required", + }, + }, { name: "valid postgrest with defaults", svc: &api.ServiceSpec{