Skip to content

Commit cf698bc

Browse files
committed
feat!: migrate protobuf codegen to protoc-gen-go and grpc-gateway v2
Replace gogo/protobuf codegen (protoc-gen-gogofast) with the official protoc-gen-go + protoc-gen-go-grpc. Migrate grpc-gateway from v1 to v2 and replace protoc-gen-swagger with protoc-gen-openapiv2. Drop go-grpc-middleware v1 in favor of grpc.ChainUnaryInterceptor. BREAKING CHANGE: the /api/v1/stream/events/{namespace} stream now wraps each event as {"type", "object"}; some OpenAPI definitions are renamed (e.g. WorkflowCreateRequest -> CreateWorkflowBody), renaming generated SDK classes; HTTP error bodies use google.rpc.Status. See docs/upgrading.md. Key changes: - Bump k8s.io/code-generator and proto deps to v0.35.4, aligned with go.mod's k8s.io libraries - protoc-gen-go generates .pb.go (messages), protoc-gen-go-grpc generates _grpc.pb.go (service stubs) - grpc-gateway v2 runtime in argoserver.go - New leaf package util/grpc/gateway holds the gateway glue shared by the server and the generated pkg/apiclient code: an SSE/fields-aware stream forwarder (reusing util/fields.Cleaner) and gateway.MessageV2Of, which bridges gogo-generated v1alpha1 types to grpc-gateway v2's proto.Message while preserving their encoding/json wire format - Vendor patch (hack/vendor-patches.sh) for K8s types losing ProtoMessage() in k8s 1.36+: changes panic to return nil in protoMessageV2Of (anchored to that one function, fails loudly if it no longer applies), letting aberrantLoadMessageDesc handle them; the kubernetes_protomessage_one_more_release build tag stays exported for builds that do not use the vendor tree - Add `make vendor` target (go mod vendor + vendor patches) wired as prerequisite to all proto/build/test targets - Wrap WatchEvents RPC in EventWatchEvent (matching the *WatchEvent naming of the other streams) and update the UI events panel for the new shape - HTTP request contexts carry the server logger; the gateway stream forwarder falls back to a default logger rather than panicking from its keepalive goroutine when one is missing - pkg/apiclient probes at startup that Kubernetes types can be marshalled, turning the unprotected-module-consumer panic into an immediate, actionable error (pkg/apiclient/protocompat.go) - HTTP round-trip test (bufconn gRPC + gateway mux + httptest, sharing the production mux configuration) pinning unary JSON bodies, unary and in-stream google.rpc.Status error shapes, SSE streaming/flush, the {"result": ...} envelope, and ?fields filtering - Update swaggify.sh for protoc-gen-openapiv2 naming conventions - Artifact endpoints in _.primary.swagger.json moved to the google.rpc.Status error model used everywhere else - Migrate the nix dev env (dev/nix/flake.nix, devenv.nix) to the new toolchain - Renovate managers for the codegen tool pins, including the nix copies (annotated, never automerged) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Alan Clucas <alan@clucas.org> Claude-Session: https://claude.ai/code/session_01HDe5nzNL4WzD4CTAWZT5Et
1 parent f016ad8 commit cf698bc

280 files changed

Lines changed: 15557 additions & 38585 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
Description: Migrate protobuf codegen from gogo/protobuf to protoc-gen-go and grpc-gateway v2
2+
Authors: [Alan Clucas](https://github.com/Joibel)
3+
Component: Build and Development
4+
Issues: 7400 16595
5+
6+
The API client and server stubs under `pkg/apiclient` are now generated with the officially maintained `protoc-gen-go` and `protoc-gen-go-grpc` instead of the unmaintained gogo/protobuf fork.
7+
The HTTP gateway moved from grpc-gateway v1 to v2, and `protoc-gen-openapiv2` replaces `protoc-gen-swagger`.
8+
9+
This is largely internal, but has some API-visible effects:
10+
11+
- The `/api/v1/stream/events/{namespace}` stream now wraps each event as `{"result": {"type": ..., "object": ...}}` instead of `{"result": <Event>}`, matching the other watch streams.
12+
- Some OpenAPI definition names changed (for example `WorkflowCreateRequest` is now `CreateWorkflowBody`), which renames the corresponding generated SDK classes.
13+
- HTTP error bodies now use the standard `google.rpc.Status` shape instead of grpc-gateway v1's error types.
14+
15+
See the upgrading guide for details.

.github/workflows/ci-build.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,10 @@ jobs:
168168
- run: if (!(Test-Path "ui/dist/app/index.html")) { New-Item -ItemType Directory -Force -Path "ui/dist/app" | Out-Null; New-Item -ItemType File -Path "ui/dist/app/placeholder" | Out-Null }; go test -p 20 -covermode=atomic -coverprofile='coverage.out' $(go list ./... | select-string -Pattern 'github.com/argoproj/argo-workflows/v4/workflow/controller' , 'github.com/argoproj/argo-workflows/v4/server' -NotMatch)
169169
env:
170170
KUBECONFIG: /dev/null
171+
# This job runs go test directly (not through make), so it must set the
172+
# tag the Makefile normally exports: without it, Kubernetes v0.35 types
173+
# lack ProtoMessage() and pkg/apiclient's startup probe fails.
174+
GOFLAGS: -tags=kubernetes_protomessage_one_more_release
171175
- name: Upload coverage report
172176
# engineers just ignore this in PRs, so lets not even run it
173177
if: github.ref == 'refs/heads/main'

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ Per-directory guidance lives in nested AGENTS.md files: `workflow/controller/AGE
1414
- Local dev stack (k3d + Tilt, everything in-cluster with hot reload), ports, profiles, debugging: see docs/running-locally.md.
1515
- Lint: `make lint` (golangci-lint `--fix` + UI lint — commit what `--fix` changes; CI runs `git diff --exit-code`).
1616
- Codegen: `make codegen -B`. Pre-PR: `make pre-commit -B` (= codegen, lint, docs).
17+
- Vendoring: use `make vendor`, never plain `go mod vendor` — the make target also runs `hack/vendor-patches.sh`, which patches vendored `google.golang.org/protobuf` so messages embedding Kubernetes types keep marshalling once Kubernetes v1.36 removes `ProtoMessage()`.
18+
Builds driven through make are also covered by the `kubernetes_protomessage_one_more_release` build tag the Makefile exports, but plain `go build`/`go test`/gopls get no tag and rely on the patched vendor tree — plain `go mod vendor` silently reverts the patch, and the only runtime safety net is the startup probe in `pkg/apiclient/protocompat.go`.
1719

1820
## Conventions
1921

Makefile

Lines changed: 69 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
11
export SHELL:=bash
22
export SHELLOPTS:=$(if $(SHELLOPTS),$(SHELLOPTS):)pipefail:errexit
3-
# k8s v0.35 moved ProtoMessage() behind a build tag. We need it unconditionally
4-
# for gogo protobuf + grpc-gateway v1 compatibility (gRPC codec requires proto.Message).
3+
# Kubernetes v1.35 (k8s.io/* v0.35) moved ProtoMessage() behind this build tag;
4+
# k8s v1.36 removes it entirely. With the tag, k8s types keep satisfying
5+
# protoiface.MessageV1, so the protoadapt legacy bridge works. This export only
6+
# reaches builds driven through make — plain `go build`/`go test`/gopls get no
7+
# tag and instead rely on the patched vendor tree from hack/vendor-patches.sh
8+
# (which `make vendor` maintains; plain `go mod vendor` silently reverts it).
9+
# Module consumers of pkg/apiclient get neither: see the compatibility probe in
10+
# pkg/apiclient/protocompat.go and docs/upgrading.md.
11+
# Note: because Go takes the last -tags flag, a caller-supplied GOFLAGS=-tags=x
12+
# is overridden by this append for make-driven builds.
513
export GOFLAGS += -tags=kubernetes_protomessage_one_more_release
614

715
.PHONY: help
@@ -150,9 +158,10 @@ TOOL_MOCKERY := mockery
150158
TOOL_CONTROLLER_GEN := controller-gen
151159
TOOL_GO_TO_PROTOBUF := go-to-protobuf
152160
TOOL_PROTOC_GEN_GOGO := protoc-gen-gogo
153-
TOOL_PROTOC_GEN_GOGOFAST := protoc-gen-gogofast
161+
TOOL_PROTOC_GEN_GO := protoc-gen-go
162+
TOOL_PROTOC_GEN_GO_GRPC := protoc-gen-go-grpc
154163
TOOL_PROTOC_GEN_GRPC_GATEWAY:= protoc-gen-grpc-gateway
155-
TOOL_PROTOC_GEN_SWAGGER := protoc-gen-swagger
164+
TOOL_PROTOC_GEN_OPENAPIV2 := protoc-gen-openapiv2
156165
TOOL_OPENAPI_GEN := openapi-gen
157166
TOOL_SWAGGER := swagger
158167
TOOL_GOIMPORTS := goimports
@@ -164,9 +173,10 @@ TOOL_MOCKERY := $(GOPATH)/bin/mockery
164173
TOOL_CONTROLLER_GEN := $(GOPATH)/bin/controller-gen
165174
TOOL_GO_TO_PROTOBUF := $(GOPATH)/bin/go-to-protobuf
166175
TOOL_PROTOC_GEN_GOGO := $(GOPATH)/bin/protoc-gen-gogo
167-
TOOL_PROTOC_GEN_GOGOFAST := $(GOPATH)/bin/protoc-gen-gogofast
176+
TOOL_PROTOC_GEN_GO := $(GOPATH)/bin/protoc-gen-go
177+
TOOL_PROTOC_GEN_GO_GRPC := $(GOPATH)/bin/protoc-gen-go-grpc
168178
TOOL_PROTOC_GEN_GRPC_GATEWAY:= $(GOPATH)/bin/protoc-gen-grpc-gateway
169-
TOOL_PROTOC_GEN_SWAGGER := $(GOPATH)/bin/protoc-gen-swagger
179+
TOOL_PROTOC_GEN_OPENAPIV2 := $(GOPATH)/bin/protoc-gen-openapiv2
170180
TOOL_OPENAPI_GEN := $(GOPATH)/bin/openapi-gen
171181
TOOL_SWAGGER := $(GOPATH)/bin/swagger
172182
TOOL_GOIMPORTS := $(GOPATH)/bin/goimports
@@ -229,6 +239,11 @@ proto_vendor: argo-proto.yaml
229239

230240
.PHONY: proto-vendor
231241
proto-vendor: proto_vendor
242+
243+
.PHONY: vendor
244+
vendor:
245+
go mod vendor
246+
hack/vendor-patches.sh
232247
override LDFLAGS += \
233248
-X github.com/argoproj/argo-workflows/v4.version=$(VERSION) \
234249
-X github.com/argoproj/argo-workflows/v4.buildDate=$(BUILD_DATE) \
@@ -275,7 +290,7 @@ SWAGGER_FILES := pkg/apiclient/_.primary.swagger.json \
275290
pkg/apiclient/workflowarchive/workflow-archive.swagger.json \
276291
pkg/apiclient/workflowtemplate/workflow-template.swagger.json \
277292
pkg/apiclient/sync/sync.swagger.json
278-
PROTO_BINARIES := $(TOOL_PROTOC_GEN_GOGO) $(TOOL_PROTOC_GEN_GOGOFAST) $(TOOL_GOIMPORTS) $(TOOL_PROTOC_GEN_GRPC_GATEWAY) $(TOOL_PROTOC_GEN_SWAGGER) $(TOOL_BUF)
293+
PROTO_BINARIES := $(TOOL_PROTOC_GEN_GO) $(TOOL_PROTOC_GEN_GO_GRPC) $(TOOL_GOIMPORTS) $(TOOL_PROTOC_GEN_GRPC_GATEWAY) $(TOOL_PROTOC_GEN_OPENAPIV2) $(TOOL_BUF)
279294
ifneq ($(USE_NIX), true)
280295
pkg/apiclient/%.swagger.json: $(PROTO_BINARIES)
281296
endif
@@ -285,7 +300,7 @@ GENERATED_DOCS := $(QUICK_GENERATED_DOCS) docs/fields.md docs/cli/argo.md docs/w
285300
# `go mod vendor` rewrites vendor/modules.txt on every run
286301
# so depend on vendor/modules.txt in places where we want it up to date
287302
vendor/modules.txt: go.mod go.sum
288-
go mod vendor
303+
$(MAKE) vendor
289304
@touch $@
290305

291306
# Targets generated via $(call protoc) need a fresh vendor tree.
@@ -296,18 +311,37 @@ $(filter-out pkg/apiclient/_.%,$(SWAGGER_FILES)) pkg/apiclient/artifact/artifact
296311
define protoc
297312
# protoc $(1)
298313
[ -e ./proto_vendor ] || $(MAKE) proto-vendor
299-
mkdir -p $(GOPATH)/src github.com/argoproj
314+
mkdir -p github.com/argoproj
300315
[ -e github.com/argoproj/argo-workflows ] || ln -s ../.. github.com/argoproj/argo-workflows
301316
[ -e v4 ] || ln -s . v4
317+
# require_unimplemented_servers=false: production server structs implement
318+
# the full service interface explicitly instead of embedding
319+
# UnimplementedXServer, so adding an RPC is a deliberate compile break
320+
# rather than a silent 501. (Test fakes may still embed the stub.)
302321
protoc \
303322
-I /usr/local/include \
304323
-I $(CURDIR) \
305324
-I $(CURDIR)/proto_vendor \
306-
--gogofast_out=plugins=grpc:$(GOPATH)/src \
307-
--grpc-gateway_out=logtostderr=true:$(GOPATH)/src \
308-
--swagger_out=logtostderr=true,fqn_for_swagger_name=true:. \
325+
--go_out=paths=source_relative:. \
326+
--go-grpc_out=require_unimplemented_servers=false,paths=source_relative:. \
327+
--grpc-gateway_out=paths=source_relative:. \
328+
--openapiv2_out=openapi_naming_strategy=fqn:. \
309329
$(1)
310-
perl -i -pe 's|argoproj/argo-workflows/(?!v4/)|argoproj/argo-workflows/v4/|g' `echo "$(1)" | sed 's/proto/pb.go/g'`
330+
# Bridge gogo-generated v1alpha1 types (which lack ProtoReflect) to the
331+
# proto.Message return type grpc-gateway v2 requires. gateway.MessageV2Of also
332+
# preserves the encoding/json wire format of the original message — a bare
333+
# protoadapt wrapper has no exported fields and would serialize as {}.
334+
# The guards fail this rule loudly if a grpc-gateway upgrade changes the
335+
# generated code: every unary return must be wrapped (none left unwrapped),
336+
# and goimports keeps the injected import gofmt-clean.
337+
gw=`echo "$(1)" | sed 's/\.proto$$/.pb.gw.go/'` && \
338+
[ -f $$gw ] || { echo "$$gw was not generated — did the proto lose its google.api.http annotations?" >&2; exit 1; } && \
339+
perl -i -pe 's/return msg, metadata, err/return gateway.MessageV2Of(msg), metadata, err/g' $$gw && \
340+
grep -q 'gateway\.MessageV2Of(msg)' $$gw && \
341+
! grep -q 'return msg, metadata, err' $$gw && \
342+
perl -i -pe 's|"google.golang.org/protobuf/proto"|"google.golang.org/protobuf/proto"\n\t"github.com/argoproj/argo-workflows/v4/util/grpc/gateway"|' $$gw && \
343+
grep -q 'util/grpc/gateway' $$gw && \
344+
$(TOOL_GOIMPORTS) -w $$gw
311345
rm -rf github.com v4
312346
endef
313347

@@ -470,27 +504,36 @@ endif
470504
$(TOOL_GO_TO_PROTOBUF): Makefile
471505
# update this in Nix when upgrading it here
472506
ifneq ($(USE_NIX), true)
473-
go install k8s.io/code-generator/cmd/go-to-protobuf@v0.35.1
507+
go install k8s.io/code-generator/cmd/go-to-protobuf@v0.35.4
474508
endif
509+
# go-to-protobuf shells out to `protoc --gogo_out`, so protoc-gen-gogo is still
510+
# required to generate the (gogo-based) pkg/apis/workflow/v1alpha1 types, even
511+
# though pkg/apiclient codegen no longer uses gogo. gogo/protobuf is archived;
512+
# this pin is final.
475513
$(TOOL_PROTOC_GEN_GOGO): Makefile
476514
# update this in Nix when upgrading it here
477515
ifneq ($(USE_NIX), true)
478516
go install github.com/gogo/protobuf/protoc-gen-gogo@v1.3.2
479517
endif
480-
$(TOOL_PROTOC_GEN_GOGOFAST): Makefile
518+
$(TOOL_PROTOC_GEN_GO): Makefile
519+
# update this in Nix when upgrading it here
520+
ifneq ($(USE_NIX), true)
521+
go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.6
522+
endif
523+
$(TOOL_PROTOC_GEN_GO_GRPC): Makefile
481524
# update this in Nix when upgrading it here
482525
ifneq ($(USE_NIX), true)
483-
go install github.com/gogo/protobuf/protoc-gen-gogofast@v1.3.2
526+
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.5.1
484527
endif
485528
$(TOOL_PROTOC_GEN_GRPC_GATEWAY): Makefile
486529
# update this in Nix when upgrading it here
487530
ifneq ($(USE_NIX), true)
488-
go install github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway@v1.16.0
531+
go install github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway@v2.29.0
489532
endif
490-
$(TOOL_PROTOC_GEN_SWAGGER): Makefile
533+
$(TOOL_PROTOC_GEN_OPENAPIV2): Makefile
491534
# update this in Nix when upgrading it here
492535
ifneq ($(USE_NIX), true)
493-
go install github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger@v1.16.0
536+
go install github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2@v2.29.0
494537
endif
495538
$(TOOL_OPENAPI_GEN): Makefile
496539
# update this in Nix when upgrading it here
@@ -523,7 +566,7 @@ $(TOOL_EMBEDDOC): hack/embeddoc/main.go hack/embeddoc/go.mod
523566

524567
# go-to-protobuf fails with mysterious errors on code that doesn't compile
525568
ifneq ($(USE_NIX), true)
526-
pkg/apis/workflow/v1alpha1/generated.proto: $(TOOL_GO_TO_PROTOBUF) $(PROTO_BINARIES)
569+
pkg/apis/workflow/v1alpha1/generated.proto: $(TOOL_GO_TO_PROTOBUF) $(TOOL_PROTOC_GEN_GOGO) $(PROTO_BINARIES)
527570
endif
528571
pkg/apis/workflow/v1alpha1/generated.proto: $(TYPES) proto-vendor vendor/modules.txt
529572
# These files are generated on a v4/ folder by the tool. Link them to the root folder
@@ -544,10 +587,11 @@ pkg/apis/workflow/v1alpha1/generated.proto: $(TYPES) proto-vendor vendor/modules
544587
# behind a build tag. Strip it so codegen tools (mockery, etc.) can compile without
545588
# requiring the tag. Runtime builds use GOFLAGS for k8s vendor types instead.
546589
perl -i -ne 'print unless /kubernetes_protomessage_one_more_release/' pkg/apis/workflow/v1alpha1/generated.protomessage.pb.go
590+
! grep -q kubernetes_protomessage_one_more_release pkg/apis/workflow/v1alpha1/generated.protomessage.pb.go
547591
# Delete the link and created k8s.io directory
548592
rm -rf github.com v4 k8s.io
549593
# Restore vendor if go-to-protobuf deleted files
550-
go mod vendor
594+
$(MAKE) vendor
551595
touch $@
552596

553597
# this target will also create a .pb.go and a .pb.gw.go file, but in Make 3 we cannot use _grouped target_, instead we must choose
@@ -572,7 +616,6 @@ pkg/apiclient/sensor/sensor.swagger.json: $(TYPES) pkg/apiclient/sensor/sensor.p
572616

573617
pkg/apiclient/workflow/workflow.swagger.json: $(TYPES) pkg/apiclient/workflow/workflow.proto
574618
$(call protoc,pkg/apiclient/workflow/workflow.proto)
575-
perl -i -pe 's/return resp\.Recv\(\) \}, mux\.GetForwardResponseOptions\(\)\.\.\.\)/return wrapEventAsProtoMessage(resp.Recv()) }, mux.GetForwardResponseOptions()...)/ if /forward_WorkflowService_WatchEvents_0/' pkg/apiclient/workflow/workflow.pb.gw.go
576619

577620
pkg/apiclient/workflowarchive/workflow-archive.swagger.json: $(TYPES) pkg/apiclient/workflowarchive/workflow-archive.proto
578621
$(call protoc,pkg/apiclient/workflowarchive/workflow-archive.proto)
@@ -661,7 +704,7 @@ endif
661704
go mod tidy
662705
ifneq ($(USE_NIX), true)
663706
# Re-vendor if tidy changed go.mod or go.sum, so the lint below sees a consistent tree
664-
[ vendor/modules.txt -nt go.mod ] && [ vendor/modules.txt -nt go.sum ] || go mod vendor
707+
[ vendor/modules.txt -nt go.mod ] && [ vendor/modules.txt -nt go.sum ] || $(MAKE) vendor
665708
endif
666709
# Lint Go files (with auto-discovered build tags)
667710
$(TOOL_GOLANGCI_LINT) run --fix --verbose --build-tags="$(GO_BUILD_TAGS)"
@@ -679,7 +722,7 @@ test: $(TOOL_GOTESTSUM) $(TOOL_BUF)
679722
endif
680723
test: ui/dist/app/index.html $(JSON_TEST_OUTPUT) ## Run tests
681724
ifneq ($(USE_NIX), true)
682-
go mod vendor
725+
$(MAKE) vendor
683726
go build -mod=vendor ./...
684727
else
685728
go build ./...
@@ -885,7 +928,7 @@ ifneq ($(USE_NIX), true)
885928
pkg/apis/workflow/v1alpha1/zz_generated.deepcopy.go: $(TOOL_GO_TO_PROTOBUF)
886929
endif
887930
pkg/apis/workflow/v1alpha1/zz_generated.deepcopy.go: $(TYPES) vendor/modules.txt
888-
CODEGEN_DIR=$$(go list -mod=mod -m -f '{{.Dir}}' k8s.io/code-generator@v0.35.1); \
931+
CODEGEN_DIR=$$(go list -mod=mod -m -f '{{.Dir}}' k8s.io/code-generator@v0.35.4); \
889932
bash -c "source $$CODEGEN_DIR/kube_codegen.sh && \
890933
kube::codegen::gen_helpers \
891934
--boilerplate ./hack/custom-boilerplate.go.txt \
@@ -903,7 +946,7 @@ dist/kubernetes.swagger.json: Makefile
903946
@mkdir -p dist
904947
# recurl will only fetch if the file doesn't exist, so delete it
905948
rm -f $@
906-
./hack/recurl.sh $@ https://raw.githubusercontent.com/kubernetes/kubernetes/v1.35.1/api/openapi-spec/swagger.json
949+
./hack/recurl.sh $@ https://raw.githubusercontent.com/kubernetes/kubernetes/v1.35.4/api/openapi-spec/swagger.json
907950

908951
pkg/apiclient/_.secondary.swagger.json: hack/api/swagger/secondaryswaggergen.go pkg/apis/workflow/v1alpha1/openapi_generated.go dist/kubernetes.swagger.json
909952
# We have `hack/api/swagger` so that most hack script do not depend on the whole code base and are therefore slow.

0 commit comments

Comments
 (0)