From a4212d302097ede9cdeab961325f8979d6f881b9 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Mon, 27 Apr 2026 22:36:43 -0400 Subject: [PATCH 1/3] feat(do-plugin): expose http_port_protocol for gRPC services (F5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the canonical `http_port_protocol` config key for App Platform services and teach the historic `protocol` shorthand a `grpc` alias. Both flow into `godo.AppServiceSpec.Protocol` (godo v1.178.0 apps.gen.go:568) — `HTTP2` and `grpc` resolve to `SERVINGPROTOCOL_HTTP2`. When both keys are set, `http_port_protocol` wins. Co-authored-by: Claude --- CHANGELOG.md | 11 +++++ internal/drivers/app_platform_buildspec.go | 31 +++++++++---- .../drivers/app_platform_buildspec_test.go | 45 +++++++++++++++++++ 3 files changed, 79 insertions(+), 8 deletions(-) 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..1197482 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,32 @@ 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 canonical keys are accepted: +// +// - "http_port_protocol" — explicit, mirrors the DO App Platform API field +// name. Takes precedence when both keys are set. +// - "protocol" — historic shorthand. 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 { + // Explicit canonical key wins over the shorthand. + raw := strFromConfig(cfg, "http_port_protocol", "") + if raw == "" { + 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..46c397b 100644 --- a/internal/drivers/app_platform_buildspec_test.go +++ b/internal/drivers/app_platform_buildspec_test.go @@ -155,6 +155,51 @@ 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) + } +} + // ── BuildCommand / RunCommand / DockerfilePath / SourceDir ─────────────────── func TestBuildAppSpec_SourceFields(t *testing.T) { From 5e7a0736f3a3f98238a1f6259d6f57c73732e5bd Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Mon, 27 Apr 2026 22:50:36 -0400 Subject: [PATCH 2/3] fix(do-plugin): http_port_protocol precedence by key presence (F5 r2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decide precedence by KEY PRESENCE, not value emptiness, so an explicit `http_port_protocol: ""` is an opt-out instead of a silent fallthrough to the `protocol` shorthand. Previously `strFromConfig` collapsed empty to default and the function would proceed to read `protocol`, surfacing HTTP2 from a `protocol: grpc` shorthand the user thought they had overridden. New test (TDD, RED→GREEN): TestBuildAppSpec_HTTPPortProtocol_ExplicitEmpty_DoesNotFallThrough. Addresses code-review Finding #3 / Copilot inline comment, F5 round 2. Co-authored-by: Claude --- internal/drivers/app_platform_buildspec.go | 21 ++++++++++++++----- .../drivers/app_platform_buildspec_test.go | 21 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/internal/drivers/app_platform_buildspec.go b/internal/drivers/app_platform_buildspec.go index 1197482..a3121e7 100644 --- a/internal/drivers/app_platform_buildspec.go +++ b/internal/drivers/app_platform_buildspec.go @@ -124,16 +124,27 @@ func instanceSizeSlugFromConfig(cfg map[string]any) string { // Two canonical keys are accepted: // // - "http_port_protocol" — explicit, mirrors the DO App Platform API field -// name. Takes precedence when both keys are set. -// - "protocol" — historic shorthand. Recognized aliases: "grpc" → HTTP2 +// 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 { - // Explicit canonical key wins over the shorthand. - raw := strFromConfig(cfg, "http_port_protocol", "") - if raw == "" { + // 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) { diff --git a/internal/drivers/app_platform_buildspec_test.go b/internal/drivers/app_platform_buildspec_test.go index 46c397b..c283e7f 100644 --- a/internal/drivers/app_platform_buildspec_test.go +++ b/internal/drivers/app_platform_buildspec_test.go @@ -200,6 +200,27 @@ func TestBuildAppSpec_HTTPPortProtocol_OverridesProtocol(t *testing.T) { } } +// 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, `strFromConfig("http_port_protocol", "")` +// would return the empty default and the function would proceed to read +// `protocol`, surfacing HTTP2 — surprising and contrary to the documented +// precedence. (Code-review Finding #3 / Copilot inline comment, F5 round 2.) +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) { From bb524d0360fc2c6602a72d6812a0d835b1a54139 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Mon, 27 Apr 2026 23:18:03 -0400 Subject: [PATCH 3/3] docs(do-plugin): sanitize http_port_protocol docstrings (F5 r3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure docstring cleanup. No code, no test logic. - httpPortProtocolFromConfig: change "Two canonical keys" → "Two config keys" since `protocol` is a historic shorthand, not canonical. Mark http_port_protocol explicitly as the canonical key in its bullet. - TestBuildAppSpec_HTTPPortProtocol_ExplicitEmpty_DoesNotFallThrough: fix the strFromConfig signature reference (it takes cfg as first arg, not just key + default), and drop internal-process language from the trailing parenthetical per public-repo team conventions. Co-authored-by: Claude --- internal/drivers/app_platform_buildspec.go | 12 ++++++------ internal/drivers/app_platform_buildspec_test.go | 9 +++++---- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/internal/drivers/app_platform_buildspec.go b/internal/drivers/app_platform_buildspec.go index a3121e7..3226924 100644 --- a/internal/drivers/app_platform_buildspec.go +++ b/internal/drivers/app_platform_buildspec.go @@ -121,13 +121,13 @@ func instanceSizeSlugFromConfig(cfg map[string]any) string { // httpPortProtocolFromConfig maps the canonical port protocol to a godo.ServingProtocol // on godo.AppServiceSpec.Protocol (godo v1.178.0 apps.gen.go:568). // -// Two canonical keys are accepted: +// Two config keys are accepted: // -// - "http_port_protocol" — explicit, 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. +// - "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). diff --git a/internal/drivers/app_platform_buildspec_test.go b/internal/drivers/app_platform_buildspec_test.go index c283e7f..6c16c63 100644 --- a/internal/drivers/app_platform_buildspec_test.go +++ b/internal/drivers/app_platform_buildspec_test.go @@ -205,10 +205,11 @@ func TestBuildAppSpec_HTTPPortProtocol_OverridesProtocol(t *testing.T) { // 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, `strFromConfig("http_port_protocol", "")` -// would return the empty default and the function would proceed to read -// `protocol`, surfacing HTTP2 — surprising and contrary to the documented -// precedence. (Code-review Finding #3 / Copilot inline comment, F5 round 2.) +// 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",