Replace gogo/protobuf with google/protobuf (#359)

**What changed?** gogo/protobuf has been replaced with Google's official go compiler. **Why?** gogo/protobuf has been deprecated for some time and the community is moving on, building new tools (like vtproto) atop google's v2 compiler. **How did you test it?** This branch is actively used by my [SDK](temporalio/sdk-go#1256) and [server](temporalio/temporal#5032) PRs so you can see it in use there **Potential risks** None I can think of. **Breaking changes for developers** - `*time.Time` in proto structs will now be [timestamppb.Timestamp](https://pkg.go.dev/google.golang.org/protobuf@v1.31.0/types/known/timestamppb#section-documentation) - `*time.Duration` will now be [durationpb.Duration](https://pkg.go.dev/google.golang.org/protobuf/types/known/durationpb) - V2-generated structs embed locks, so you cannot dereference them. `go vet` will scream at you about this. If you need a copy, use `proto.Clone`. - Proto objects, or objects embedding protos, cannot be compared using `reflect.DeepEqual` or _anything_ that uses it. This includes `testify` and `mock` equality testers! - You will need to use the `protorequire` ro `protoassert`packages instead. I've implemented proto-compatible assertions there for all cases I've encountered - If you need `reflect.DeepEqual` for any reason you can use `go.temporal.io/api/temporalproto.DeepEqual` instead
temporalio · Nov 22, 2023 · 6e7388c · 6e7388c
1 parent ca7959c
commit 6e7388c
Show file tree

Hide file tree

Showing 29 changed files with 208 additions and 4,999 deletions.
diff --git a/.github/workflows/all-docker-images.yaml b/.github/workflows/all-docker-images.yaml
@@ -7,6 +7,9 @@ on:
       go-ver:
         description: Go SDK ver to build. Skipped if not specified. Must start with v.
         type: string
+      go-repo-ref:
+        description: Go SDK repo ref to build. Mutually exclusive with go-ver.
+        type: string
       py-ver:
         description: Python SDK ver to build. Skipped if not specified. Must start with v.
         type: string
@@ -34,6 +37,9 @@ on:
       go-ver:
         description: Go SDK ver to build. Skipped if not specified. Must start with v.
         type: string
+      go-repo-ref:
+        description: Go SDK repo ref to build. Mutually exclusive with go-ver.
+        type: string
       py-ver:
         description: Python SDK ver to build. Skipped if not specified. Must start with v.
         type: string
@@ -55,15 +61,15 @@ on:
         type: boolean
         default: false
 
-
 jobs:
   build-go-docker-images:
-    if: inputs.go-ver
+    if: inputs.go-ver || inputs.go-repo-ref
     uses: ./.github/workflows/docker-images.yaml
     secrets: inherit
     with:
       lang: go
       sdk-version: ${{ inputs.go-ver }}
+      sdk-repo-ref: ${{ inputs.go-repo-ref }}
       semver-latest: major
       do-push: ${{ inputs.do-push }}
       skip-cloud: ${{ inputs.skip-cloud }}

diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -24,7 +24,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions/setup-go@v2
         with:
-          go-version: '^1.19'
+          go-version: '^1.20'
 
       - run: go build -o temporal-features
 
@@ -105,8 +105,9 @@ jobs:
   feature-tests-go:
     uses: ./.github/workflows/go.yaml
     with:
-      version: v1.25.0
-      version-is-repo-ref: false
+      # First commit that supports google/protobuf
+      version: f9d73bfdf7c8d3ec0311306140fbfafa7fb6f9cf
+      version-is-repo-ref: true
       features-repo-ref: ${{ github.head_ref }}
       features-repo-path: ${{ github.event.pull_request.head.repo.full_name }}
 
@@ -140,7 +141,7 @@ jobs:
     # TODO: Find some way to automatically upgrade to "latest"
     with:
       do-push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}
-      go-ver: 'v1.25.0'
+      go-repo-ref: 'f9d73bfdf7c8d3ec0311306140fbfafa7fb6f9cf'
       ts-ver: 'v1.5.2'
       java-ver: 'v1.22.0'
       py-ver: 'v1.4.0'

diff --git a/.github/workflows/docker-images.yaml b/.github/workflows/docker-images.yaml
@@ -40,12 +40,15 @@ jobs:
       TEMPORAL_CLOUD_NAMESPACE: sdk-ci.a2dd6
       REPO_URL: ${{ github.event.pull_request.head.repo.html_url }}
     steps:
+      - name: Fail if both sdk-version and sdk-repo-ref are present
+        if: inputs.sdk-version && inputs.sdk-repo-ref
+        run: exit 1
       - name: Print build information
         run: 'echo head_ref: "$GITHUB_HEAD_REF", ref: "$GITHUB_REF"'
       - uses: actions/checkout@v2
       - uses: actions/setup-go@v2
         with:
-          go-version: '^1.19'
+          go-version: '^1.20'
       - name: Lint dockerfile
         run: docker run --rm -i hadolint/hadolint < dockerfiles/${{inputs.lang}}.Dockerfile
 

diff --git a/.github/workflows/dotnet.yaml b/.github/workflows/dotnet.yaml
@@ -71,7 +71,7 @@ jobs:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
       - uses: actions/setup-go@v2
         with:
-          go-version: '^1.19'
+          go-version: '^1.20'
 
       - uses: Swatinem/rust-cache@v1
         if: ${{ inputs.version-is-repo-ref }}

diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml
@@ -52,6 +52,7 @@ jobs:
           path: features
           repository: ${{ inputs.features-repo-path }}
           ref: ${{ inputs.features-repo-ref }}
+
       - name: Checkout Go SDK repo
         if: ${{ inputs.version-is-repo-ref }}
         uses: actions/checkout@v2
@@ -63,7 +64,7 @@ jobs:
 
       - uses: actions/setup-go@v2
         with:
-          go-version: '^1.19'
+          go-version: '^1.20'
 
       - name: Start containerized server and dependencies
         if: inputs.docker-image-artifact-name

diff --git a/.github/workflows/java.yaml b/.github/workflows/java.yaml
@@ -70,7 +70,7 @@ jobs:
           java-version: '11'
       - uses: actions/setup-go@v2
         with:
-          go-version: '^1.19'
+          go-version: '^1.20'
 
       - name: Start containerized server and dependencies
         if: inputs.docker-image-artifact-name

diff --git a/.github/workflows/python.yaml b/.github/workflows/python.yaml
@@ -63,7 +63,7 @@ jobs:
 
       - uses: actions/setup-go@v2
         with:
-          go-version: '^1.19'
+          go-version: '^1.20'
       - uses: actions/setup-python@v1
         with:
           python-version: '3.10'

diff --git a/.github/workflows/typescript.yaml b/.github/workflows/typescript.yaml
@@ -73,7 +73,7 @@ jobs:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
       - uses: actions/setup-go@v2
         with:
-          go-version: '^1.19'
+          go-version: '^1.20'
 
       - uses: Swatinem/rust-cache@v1
         if: ${{ inputs.version-is-repo-ref }}

diff --git a/cmd/run_go.go b/cmd/run_go.go
@@ -19,7 +19,7 @@ func (p *Preparer) BuildGoProgram(ctx context.Context) (sdkbuild.Program, error)
 		Version: p.config.Version,
 		GoModContents: `module go.temporal.io/features-test
 
-go 1.17
+go 1.20
 
 require github.com/temporalio/features/features v1.0.0
 require github.com/temporalio/features/harness/go v1.0.0

diff --git a/dockerfiles/go.Dockerfile b/dockerfiles/go.Dockerfile
@@ -1,5 +1,5 @@
 # Build in a full featured container
-FROM golang:1.19 as build
+FROM golang:1.20 as build
 
 WORKDIR /app
 

diff --git a/features/data_converter/binary/feature.go b/features/data_converter/binary/feature.go
@@ -7,11 +7,12 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/gogo/protobuf/jsonpb"
 	"github.com/temporalio/features/harness/go/harness"
 	common "go.temporal.io/api/common/v1"
+	"go.temporal.io/api/temporalproto"
 	"go.temporal.io/sdk/client"
 	"go.temporal.io/sdk/workflow"
+	"google.golang.org/protobuf/proto"
 )
 
 var expectedResult = []byte{0xde, 0xad, 0xbe, 0xef}
@@ -50,19 +51,18 @@ func CheckResult(ctx context.Context, runner *harness.Runner, run client.Workflo
 	}
 
 	expectedPayload := &common.Payload{}
-	unmarshaler := jsonpb.Unmarshaler{}
-	err = unmarshaler.Unmarshal(file, expectedPayload)
-	if err != nil {
+	decoder := temporalproto.NewJSONDecoder(file, false)
+	if err := decoder.Decode(expectedPayload); err != nil {
 		return err
 	}
-	runner.Require.Equal(expectedPayload, payload)
+	runner.Require.True(proto.Equal(expectedPayload, payload))
 
 	payloadArg, err := harness.GetWorkflowArgumentPayload(ctx, runner.Client, run.GetID())
 	if err != nil {
 		return err
 	}
 
-	runner.Require.Equal(payload, payloadArg)
+	runner.Require.True(proto.Equal(payload, payloadArg))
 
 	return nil
 }
diff --git a/features/data_converter/binary_protobuf/feature.go b/features/data_converter/binary_protobuf/feature.go
@@ -3,7 +3,8 @@ package binary_protobuf
 import (
 	"context"
 
-	"github.com/gogo/protobuf/proto"
+	"google.golang.org/protobuf/proto"
+
 	"github.com/temporalio/features/harness/go/harness"
 	commonpb "go.temporal.io/api/common/v1"
 	"go.temporal.io/sdk/client"
@@ -24,22 +25,22 @@ var Feature = harness.Feature{
 		),
 	},
 	// ExecuteDefault does not support workflow arguments
-	Execute: harness.ExecuteWithArgs(Workflow, expectedResult),
+	Execute: harness.ExecuteWithArgs(Workflow, &expectedResult),
 }
 
 // An "echo" workflow
-func Workflow(ctx workflow.Context, res commonpb.DataBlob) (commonpb.DataBlob, error) {
+func Workflow(ctx workflow.Context, res *commonpb.DataBlob) (*commonpb.DataBlob, error) {
 	return res, nil
 }
 
 func CheckResult(ctx context.Context, runner *harness.Runner, run client.WorkflowRun) error {
 	// verify client result is DataBlob `0xdeadbeef`
-	result := commonpb.DataBlob{}
-	if err := run.Get(ctx, &result); err != nil {
+	result := &commonpb.DataBlob{}
+	if err := run.Get(ctx, result); err != nil {
 		return err
 	}
 
-	runner.Require.True(proto.Equal(&expectedResult, &result))
+	runner.Require.True(proto.Equal(&expectedResult, result))
 
 	payload, err := harness.GetWorkflowResultPayload(ctx, runner.Client, run.GetID())
 	if err != nil {
@@ -57,7 +58,7 @@ func CheckResult(ctx context.Context, runner *harness.Runner, run client.Workflo
 		return err
 	}
 
-	runner.Require.True(proto.Equal(&result, &resultInHistory))
+	runner.Require.True(proto.Equal(result, &resultInHistory))
 
 	payloadArg, err := harness.GetWorkflowArgumentPayload(ctx, runner.Client, run.GetID())
 	if err != nil {

diff --git a/features/data_converter/codec/feature.go b/features/data_converter/codec/feature.go
@@ -5,7 +5,8 @@ import (
 	"encoding/base64"
 	"encoding/json"
 
-	"github.com/gogo/protobuf/proto"
+	"google.golang.org/protobuf/proto"
+
 	"github.com/temporalio/features/harness/go/harness"
 	commonpb "go.temporal.io/api/common/v1"
 	"go.temporal.io/sdk/client"

diff --git a/features/data_converter/empty/feature.go b/features/data_converter/empty/feature.go
@@ -7,13 +7,14 @@ import (
 	"path"
 	"time"
 
-	"github.com/gogo/protobuf/jsonpb"
 	"github.com/temporalio/features/harness/go/harness"
 	"go.temporal.io/api/common/v1"
 	historyProto "go.temporal.io/api/history/v1"
+	"go.temporal.io/api/temporalproto"
 	"go.temporal.io/sdk/client"
 	"go.temporal.io/sdk/temporal"
 	"go.temporal.io/sdk/workflow"
+	"google.golang.org/protobuf/proto"
 )
 
 var Feature = harness.Feature{
@@ -71,11 +72,10 @@ func CheckResult(ctx context.Context, runner *harness.Runner, run client.Workflo
 	}
 
 	expectedPayload := &common.Payload{}
-	unmarshaler := jsonpb.Unmarshaler{}
-	err = unmarshaler.Unmarshal(file, expectedPayload)
-	if err != nil {
+	decoder := temporalproto.NewJSONDecoder(file, false)
+	if err := decoder.Decode(expectedPayload); err != nil {
 		return err
 	}
-	runner.Require.Equal(expectedPayload, payload)
+	runner.Require.True(proto.Equal(expectedPayload, payload))
 	return nil
 }
diff --git a/features/data_converter/json/feature.go b/features/data_converter/json/feature.go
@@ -4,7 +4,8 @@ import (
 	"context"
 	"encoding/json"
 
-	"github.com/gogo/protobuf/proto"
+	"google.golang.org/protobuf/proto"
+
 	"github.com/temporalio/features/harness/go/harness"
 	"go.temporal.io/sdk/client"
 	"go.temporal.io/sdk/workflow"

diff --git a/features/data_converter/json_protobuf/README.md b/features/data_converter/json_protobuf/README.md
@@ -10,7 +10,7 @@ with binary value `0xdeadbeef`
 - verify client result is [`DataBlob`] with value `0xdeadbeef`
 - get result payload of WorkflowExecutionCompleted event from workflow history
 - verify payload encoding is `json/protobuf`, unmarshall its data into a
-`DataBlob` using `jsonpb` library, and compare it to the client
+`DataBlob` using `protojson` library, and compare it to the client
 result
 - get argument payload of WorkflowExecutionStarted event from workflow history
 - verify that argument and result payloads are the same

diff --git a/features/data_converter/json_protobuf/feature.go b/features/data_converter/json_protobuf/feature.go
@@ -1,15 +1,14 @@
 package json_protobuf
 
 import (
-	"bytes"
 	"context"
 
-	"github.com/gogo/protobuf/jsonpb"
-	"github.com/gogo/protobuf/proto"
 	"github.com/temporalio/features/harness/go/harness"
 	commonpb "go.temporal.io/api/common/v1"
+	"go.temporal.io/api/temporalproto"
 	"go.temporal.io/sdk/client"
 	"go.temporal.io/sdk/workflow"
+	"google.golang.org/protobuf/proto"
 )
 
 var expectedResult = commonpb.DataBlob{Data: []byte{0xde, 0xad, 0xbe, 0xef}}
@@ -18,13 +17,13 @@ var Feature = harness.Feature{
 	Workflows:   Workflow,
 	CheckResult: CheckResult,
 	// ExecuteDefault does not support workflow arguments
-	Execute: harness.ExecuteWithArgs(Workflow, expectedResult),
+	Execute: harness.ExecuteWithArgs(Workflow, &expectedResult),
 	// No need of a custom data converter, the default one prioritizes
 	// ProtoJSONPayload over ProtoPayload
 }
 
 // An "echo" workflow
-func Workflow(ctx workflow.Context, res commonpb.DataBlob) (commonpb.DataBlob, error) {
+func Workflow(ctx workflow.Context, res *commonpb.DataBlob) (*commonpb.DataBlob, error) {
 	return res, nil
 }
 
@@ -48,8 +47,7 @@ func CheckResult(ctx context.Context, runner *harness.Runner, run client.Workflo
 	runner.Require.Equal("temporal.api.common.v1.DataBlob", messageType)
 
 	resultInHistory := commonpb.DataBlob{}
-	readerPayloadData := bytes.NewReader(payload.GetData())
-	if err := jsonpb.Unmarshal(readerPayloadData, &resultInHistory); err != nil {
+	if err := temporalproto.UnmarshalJSON(payload.GetData(), &resultInHistory); err != nil {
 		return err
 	}