diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d93d0b..b98052c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ All notable changes to workflow-plugin-digitalocean are documented here. +## [Unreleased] + +### Added + +- **`http_port_protocol` canonical key + `protocol: grpc` alias (P-2.F5)** — + App Platform services now honor an explicit `http_port_protocol` config key + that maps to `godo.AppServiceSpec.Protocol` (godo v1.178.0 `apps.gen.go:568`). + The historic `protocol` shorthand still works and gains a `grpc` alias that + resolves to `HTTP2` (gRPC requires HTTP/2 with prior knowledge per DO docs). + When both keys are set, `http_port_protocol` takes precedence. + ## [v0.7.9] - 2026-04-24 ### Added diff --git a/internal/drivers/app_platform_buildspec.go b/internal/drivers/app_platform_buildspec.go index 4bbc497..3226924 100644 --- a/internal/drivers/app_platform_buildspec.go +++ b/internal/drivers/app_platform_buildspec.go @@ -38,7 +38,7 @@ func buildAppSpec(name string, cfg map[string]any, region string) (*godo.AppSpec DockerfilePath: strFromConfig(cfg, "dockerfile_path", ""), SourceDir: strFromConfig(cfg, "source_dir", ""), InstanceSizeSlug: instanceSizeSlugFromConfig(cfg), - Protocol: servingProtocolFromConfig(cfg), + Protocol: httpPortProtocolFromConfig(cfg), InternalPorts: internalPortsFromConfig(cfg), Routes: routesFromConfig(cfg), HealthCheck: serviceHealthCheckFromConfig(cfg), @@ -118,17 +118,43 @@ func instanceSizeSlugFromConfig(cfg map[string]any) string { return "" } -// servingProtocolFromConfig maps canonical "protocol" to a godo.ServingProtocol. -// DO supports HTTP and HTTP2; unknown values are passed through for forward compatibility. -func servingProtocolFromConfig(cfg map[string]any) godo.ServingProtocol { - proto := strings.ToUpper(strFromConfig(cfg, "protocol", "")) - switch proto { - case "HTTP2": +// httpPortProtocolFromConfig maps the canonical port protocol to a godo.ServingProtocol +// on godo.AppServiceSpec.Protocol (godo v1.178.0 apps.gen.go:568). +// +// Two config keys are accepted: +// +// - "http_port_protocol" — the canonical key, mirrors the DO App Platform +// API field name. Takes precedence by KEY PRESENCE: if the key is set in +// cfg (even to an empty string), its value is honored and the shorthand +// is NOT consulted. This makes `http_port_protocol: ""` an explicit +// opt-out rather than a silent fallthrough. +// - "protocol" — historic shorthand. Consulted only when +// "http_port_protocol" is absent. Recognized aliases: "grpc" → HTTP2 +// (gRPC requires HTTP/2 with prior knowledge per DO docs). +// +// DO recognizes HTTP and HTTP2; unknown values pass through for forward +// compatibility with future godo releases. +func httpPortProtocolFromConfig(cfg map[string]any) godo.ServingProtocol { + // Key presence — not value emptiness — decides precedence, so that an + // explicit `http_port_protocol: ""` does NOT fall through to `protocol`. + var raw string + if v, ok := cfg["http_port_protocol"]; ok { + // If present but not a string (or explicitly empty), raw stays "" — + // which the switch below treats as "no protocol override". + if s, ok := v.(string); ok { + raw = s + } + } else { + raw = strFromConfig(cfg, "protocol", "") + } + switch strings.ToUpper(raw) { + case "HTTP2", "GRPC": + // gRPC over App Platform is served as HTTP/2 with prior knowledge. return godo.SERVINGPROTOCOL_HTTP2 case "HTTP", "": return "" // omit — DO defaults to HTTP default: - return godo.ServingProtocol(proto) + return godo.ServingProtocol(strings.ToUpper(raw)) } } diff --git a/internal/drivers/app_platform_buildspec_test.go b/internal/drivers/app_platform_buildspec_test.go index ba2062d..6c16c63 100644 --- a/internal/drivers/app_platform_buildspec_test.go +++ b/internal/drivers/app_platform_buildspec_test.go @@ -155,6 +155,73 @@ func TestBuildAppSpec_Protocol_Default(t *testing.T) { } } +// TestBuildAppSpec_HTTPPortProtocol_HTTP2 exercises the canonical +// `http_port_protocol: HTTP2` key — the official spelling that mirrors the +// DO App Platform API field semantics. It must reach godo.AppServiceSpec.Protocol. +func TestBuildAppSpec_HTTPPortProtocol_HTTP2(t *testing.T) { + cfg := map[string]any{ + "image": "registry.digitalocean.com/myrepo/myapp:v1", + "http_port_protocol": "HTTP2", + } + spec := buildSpecViaCreate(t, cfg) + if got := spec.Services[0].Protocol; got != godo.SERVINGPROTOCOL_HTTP2 { + t.Errorf("Protocol = %q, want HTTP2 (from http_port_protocol)", got) + } +} + +// TestBuildAppSpec_Protocol_GRPC_AliasesHTTP2 exercises the shorthand +// `protocol: grpc` alias. gRPC requires HTTP/2 transport (DO docs note HTTP2 +// "needs to be implemented in the service by serving HTTP/2 with prior +// knowledge"), so the alias must resolve to godo.SERVINGPROTOCOL_HTTP2. +func TestBuildAppSpec_Protocol_GRPC_AliasesHTTP2(t *testing.T) { + cfg := map[string]any{ + "image": "registry.digitalocean.com/myrepo/myapp:v1", + "protocol": "grpc", + } + spec := buildSpecViaCreate(t, cfg) + if got := spec.Services[0].Protocol; got != godo.SERVINGPROTOCOL_HTTP2 { + t.Errorf("Protocol = %q, want HTTP2 (gRPC alias)", got) + } +} + +// TestBuildAppSpec_HTTPPortProtocol_OverridesProtocol verifies precedence: +// when both `http_port_protocol` and `protocol` are set, the explicit +// `http_port_protocol` wins. This protects callers who mix the two during +// migration from the shorthand to the canonical key. +func TestBuildAppSpec_HTTPPortProtocol_OverridesProtocol(t *testing.T) { + cfg := map[string]any{ + "image": "registry.digitalocean.com/myrepo/myapp:v1", + "http_port_protocol": "HTTP2", + "protocol": "HTTP", + } + spec := buildSpecViaCreate(t, cfg) + if got := spec.Services[0].Protocol; got != godo.SERVINGPROTOCOL_HTTP2 { + t.Errorf("Protocol = %q, want HTTP2 (http_port_protocol takes precedence)", got) + } +} + +// TestBuildAppSpec_HTTPPortProtocol_ExplicitEmpty_DoesNotFallThrough verifies +// that the precedence rule is decided by KEY PRESENCE, not value emptiness. +// An explicit `http_port_protocol: ""` is the user opting out of any protocol +// override; it must NOT silently fall back to a `protocol: grpc` shorthand. +// +// Without the key-presence check, the function would treat +// `http_port_protocol: ""` as absent (because `strFromConfig`'s +// default-on-empty logic conflates explicit-empty with omitted) and consult +// `protocol` instead, surfacing HTTP2 — surprising and contrary to the +// documented precedence. +func TestBuildAppSpec_HTTPPortProtocol_ExplicitEmpty_DoesNotFallThrough(t *testing.T) { + cfg := map[string]any{ + "image": "registry.digitalocean.com/myrepo/myapp:v1", + "http_port_protocol": "", // explicit empty — opt out + "protocol": "grpc", + } + spec := buildSpecViaCreate(t, cfg) + if got := spec.Services[0].Protocol; got != "" { + t.Errorf("Protocol = %q, want empty (explicit empty http_port_protocol must not fall through to protocol: grpc)", got) + } +} + // ── BuildCommand / RunCommand / DockerfilePath / SourceDir ─────────────────── func TestBuildAppSpec_SourceFields(t *testing.T) {