refactor: migrate to connect-go v2 (#1325)

Migrates the runner to `connectrpc.com/connect/v2` by pinning actionslib v1.5.0, which carries https://gitea.com/gitea/actionslib/pulls/28. The wire behaviour towards Gitea is unchanged and works against Gitea servers on connect v1 and v2. The client keeps v1's unlimited response size, and the error codes and messages the runner checks stay the same.

Reviewed-on: https://gitea.com/gitea/runner/pulls/1325
Reviewed-by: bircni <[email protected]>
Co-authored-by: silverwind <[email protected]>

internal/app/poll/poller.go

@@ -17,7 +17,7 @@ import (
 	"gitea.com/gitea/runner/internal/pkg/config"
 	"gitea.com/gitea/runner/internal/pkg/metrics"
 
-	"connectrpc.com/connect"
+	"connectrpc.com/connect/v2"
 	runnerv1 "gitea.dev/actionslib/runner/v1"
 )
 
@@ -355,9 +355,9 @@ func (p *Poller) fetchTask(ctx context.Context, s *workerState) (*runnerv1.Task,
 	// Load the version value that was in the cache when the request was sent.
 	v := p.tasksVersion.Load()
 	start := time.Now()
-	resp, err := p.client.FetchTask(reqCtx, connect.NewRequest(&runnerv1.FetchTaskRequest{
+	resp, err := p.client.FetchTask(reqCtx, &runnerv1.FetchTaskRequest{
 		TasksVersion: v,
-	}))
+	})
 
 	// Our own deadline proves nothing either way: today Gitea answers immediately,
 	// so it means a slow server, and once it holds the request open it is an idle
@@ -405,27 +405,27 @@ func (p *Poller) fetchTask(ctx context.Context, s *workerState) (*runnerv1.Task,
 	// Successful response — reset error counter.
 	s.consecutiveErrors = 0
 
-	if resp == nil || resp.Msg == nil {
+	if resp == nil {
 		s.consecutiveEmpty++
 		metrics.PollFetchTotal.WithLabelValues(metrics.LabelResultEmpty).Inc()
 		return nil, false
 	}
 
-	if resp.Msg.TasksVersion > v {
-		p.tasksVersion.CompareAndSwap(v, resp.Msg.TasksVersion)
+	if resp.TasksVersion > v {
+		p.tasksVersion.CompareAndSwap(v, resp.TasksVersion)
 	}
 
-	if resp.Msg.Task == nil {
+	if resp.Task == nil {
 		s.consecutiveEmpty++
 		metrics.PollFetchTotal.WithLabelValues(metrics.LabelResultEmpty).Inc()
 		return nil, false
 	}
 
 	// got a task, set `tasksVersion` to zero to force query db in next request.
-	p.tasksVersion.CompareAndSwap(resp.Msg.TasksVersion, 0)
+	p.tasksVersion.CompareAndSwap(resp.TasksVersion, 0)
 
 	metrics.PollFetchTotal.WithLabelValues(metrics.LabelResultTask).Inc()
-	return resp.Msg.Task, true
+	return resp.Task, true
 }
 
 func (p *Poller) markHealthyPoll() {

internal/app/poll/poller_test.go

@@ -15,7 +15,7 @@ import (
 	"gitea.com/gitea/runner/internal/pkg/client/mocks"
 	"gitea.com/gitea/runner/internal/pkg/config"
 
-	connect_go "connectrpc.com/connect"
+	"connectrpc.com/connect/v2"
 	runnerv1 "gitea.dev/actionslib/runner/v1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -28,9 +28,9 @@ import (
 func TestPoller_WorkerStateCounters(t *testing.T) {
 	client := mocks.NewClient(t)
 	client.On("FetchTask", mock.Anything, mock.Anything).Return(
-		func(_ context.Context, _ *connect_go.Request[runnerv1.FetchTaskRequest]) (*connect_go.Response[runnerv1.FetchTaskResponse], error) {
+		func(_ context.Context, _ *runnerv1.FetchTaskRequest) (*runnerv1.FetchTaskResponse, error) {
 			// Always return an empty response.
-			return connect_go.NewResponse(&runnerv1.FetchTaskResponse{}), nil
+			return &runnerv1.FetchTaskResponse{}, nil
 		},
 	)
 
@@ -109,8 +109,8 @@ func TestPoller_FetchUnauthenticatedStopsPolling(t *testing.T) {
 		fetchErr error
 		rejected bool
 	}{
-		"proxy": {connect_go.NewError(connect_go.CodeUnauthenticated, errors.New("401 Unauthorized")), false},
-		"gitea": {connect_go.NewWireError(connect_go.CodeUnknown, errors.New("rpc error: code = Unauthenticated desc = unregistered runner")), true},
+		"proxy": {connect.NewError(connect.CodeUnauthenticated, "401 Unauthorized"), false},
+		"gitea": {connect.NewError(connect.CodeUnknown, "rpc error: code = Unauthenticated desc = unregistered runner").WithRemote(), true},
 	} {
 		t.Run(name, func(t *testing.T) {
 			client := mocks.NewClient(t)
@@ -242,7 +242,7 @@ func TestPollerPausesAndResumesForLocalAvailability(t *testing.T) {
 	cli := mocks.NewClient(t)
 	cli.On("FetchTask", mock.Anything, mock.Anything).Maybe().Run(func(mock.Arguments) {
 		fetches.Add(1)
-	}).Return(connect_go.NewResponse(&runnerv1.FetchTaskResponse{}), nil)
+	}).Return(&runnerv1.FetchTaskResponse{}, nil)
 
 	cfg, err := config.LoadDefault("")
 	require.NoError(t, err)
@@ -294,8 +294,8 @@ func (m *idleAwareRunner) OnIdle(_ context.Context) {
 func TestPollerPollCallsOnIdle(t *testing.T) {
 	cli := mocks.NewClient(t)
 	cli.On("FetchTask", mock.Anything, mock.Anything).Return(
-		func(_ context.Context, _ *connect_go.Request[runnerv1.FetchTaskRequest]) (*connect_go.Response[runnerv1.FetchTaskResponse], error) {
-			return connect_go.NewResponse(&runnerv1.FetchTaskResponse{}), nil
+		func(_ context.Context, _ *runnerv1.FetchTaskRequest) (*runnerv1.FetchTaskResponse, error) {
+			return &runnerv1.FetchTaskResponse{}, nil
 		},
 	)
 
@@ -324,8 +324,8 @@ func TestPollerPollCallsOnIdle(t *testing.T) {
 func TestPollerPollOnceCallsOnIdle(t *testing.T) {
 	cli := mocks.NewClient(t)
 	cli.On("FetchTask", mock.Anything, mock.Anything).Return(
-		func(_ context.Context, _ *connect_go.Request[runnerv1.FetchTaskRequest]) (*connect_go.Response[runnerv1.FetchTaskResponse], error) {
-			return connect_go.NewResponse(&runnerv1.FetchTaskResponse{}), nil
+		func(_ context.Context, _ *runnerv1.FetchTaskRequest) (*runnerv1.FetchTaskResponse, error) {
+			return &runnerv1.FetchTaskResponse{}, nil
 		},
 	)
 
@@ -352,7 +352,7 @@ func TestPollerPollOnceCallsOnIdle(t *testing.T) {
 
 func TestPollerPollOnceCallsOnTaskFetchedBeforeRunningIt(t *testing.T) {
 	cli := mocks.NewClient(t)
-	cli.On("FetchTask", mock.Anything, mock.Anything).Return(connect_go.NewResponse(&runnerv1.FetchTaskResponse{Task: &runnerv1.Task{Id: 1}}), nil)
+	cli.On("FetchTask", mock.Anything, mock.Anything).Return(&runnerv1.FetchTaskResponse{Task: &runnerv1.Task{Id: 1}}, nil)
 	cfg, err := config.LoadDefault("")
 	require.NoError(t, err)
 	runner := &mockRunner{}
@@ -380,17 +380,17 @@ func TestPoller_ConcurrencyLimitedByCapacity(t *testing.T) {
 
 	cli := mocks.NewClient(t)
 	cli.On("FetchTask", mock.Anything, mock.Anything).Return(
-		func(_ context.Context, _ *connect_go.Request[runnerv1.FetchTaskRequest]) (*connect_go.Response[runnerv1.FetchTaskResponse], error) {
+		func(_ context.Context, _ *runnerv1.FetchTaskRequest) (*runnerv1.FetchTaskResponse, error) {
 			atomicMax(&maxFetchConcur, fetchConcur.Add(1))
 			defer fetchConcur.Add(-1)
 
 			n := tasksReturned.Add(1)
 			if n <= totalTasks {
-				return connect_go.NewResponse(&runnerv1.FetchTaskResponse{
+				return &runnerv1.FetchTaskResponse{
 					Task: &runnerv1.Task{Id: n},
-				}), nil
+				}, nil
 			}
-			return connect_go.NewResponse(&runnerv1.FetchTaskResponse{}), nil
+			return &runnerv1.FetchTaskResponse{}, nil
 		},
 	)
 
@@ -438,13 +438,13 @@ func TestPoller_ShutdownForcesJobsOnTimeout(t *testing.T) {
 	var served atomic.Bool
 	cli := mocks.NewClient(t)
 	cli.On("FetchTask", mock.Anything, mock.Anything).Return(
-		func(_ context.Context, _ *connect_go.Request[runnerv1.FetchTaskRequest]) (*connect_go.Response[runnerv1.FetchTaskResponse], error) {
+		func(_ context.Context, _ *runnerv1.FetchTaskRequest) (*runnerv1.FetchTaskResponse, error) {
 			if served.CompareAndSwap(false, true) {
-				return connect_go.NewResponse(&runnerv1.FetchTaskResponse{
+				return &runnerv1.FetchTaskResponse{
 					Task: &runnerv1.Task{Id: 1},
-				}), nil
+				}, nil
 			}
-			return connect_go.NewResponse(&runnerv1.FetchTaskResponse{}), nil
+			return &runnerv1.FetchTaskResponse{}, nil
 		},
 	)