From bc2fbe77b13721723f1025e117886251b5114392 Mon Sep 17 00:00:00 2001 From: bircni Date: Thu, 11 Jun 2026 11:18:31 +0200 Subject: [PATCH] refactor(actions): read runner capabilities from proto field (#38068) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [actions-proto-go v0.6.0](https://gitea.com/gitea/actions-proto-go) adds a `capabilities` field to `RegisterRequest` and `DeclareRequest`. This lets a runner advertise the transitional `cancelling` capability directly in the proto message instead of through the out-of-band mechanism we used while the proto bump was pending. This PR: - Bumps `gitea.dev/actions-proto-go` to `v0.6.0`. - Drops the forward-compat `capabilityGetter` type-assertion shim and the `runnerRequestHasCancellingCapability` helper, reading `GetCapabilities()` directly (now part of the `declareRequest` interface). - Removes the "capability state unknown → preserve existing value" branch. ## Why the behaviour change is correct The shim and the `(hasSupport, known)` two-value return only existed because the old proto had no `capabilities` field, so we couldn't tell "runner doesn't support it" from "we can't see the field." With v0.6.0 the field is always present. Since proto3 repeated fields have no presence, "no capabilities sent" now unambiguously means the runner does not advertise the capability, so a runner that omits `cancelling` is correctly recorded as `HasCancellingSupport = false`. There is no regression: prior to this bump Gitea was on `v0.5.0`, where the type assertion always failed and `HasCancellingSupport` was therefore never set from requests — so no runner relied on the preserved-unknown path. ## Compatibility The change is wire-compatible in both directions of version skew, because the new field uses a previously unused field number (8 on `RegisterRequest`, 3 on `DeclareRequest`) and the transport uses the binary protobuf codec: - **Old runner → new Gitea:** the runner omits the field; it decodes to an empty capability list. Registration/declaration succeed; the runner simply doesn't get the cancelling feature. - **New runner → old Gitea:** the runner sends the field; the old server's generated code doesn't know the field number and silently ignores it. Registration/declaration succeed. The feature only activates once both server and runner are on `v0.6.0`. --- go.mod | 2 +- go.sum | 4 +- routers/api/actions/runner/runner.go | 23 ++----- routers/api/actions/runner/runner_test.go | 83 ++++++++--------------- 4 files changed, 34 insertions(+), 78 deletions(-) diff --git a/go.mod b/go.mod index 25e319aefeb..faf89bb607c 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( gitea.com/go-chi/session v0.0.0-20251124165456-68e0254e989e gitea.com/lunny/dingtalk_webhook v0.0.0-20171025031554-e3534c89ef96 gitea.com/lunny/levelqueue v0.4.2-0.20230414023320-3c0159fe0fe4 - gitea.dev/actions-proto-go v0.5.0 + gitea.dev/actions-proto-go v0.6.0 gitea.dev/sdk v1.0.1 github.com/42wim/httpsig v1.2.4 github.com/42wim/sshsig v0.0.0-20260317195500-b9f38cf0d432 diff --git a/go.sum b/go.sum index 687b2458812..aff785fe47a 100644 --- a/go.sum +++ b/go.sum @@ -26,8 +26,8 @@ gitea.com/lunny/levelqueue v0.4.2-0.20230414023320-3c0159fe0fe4 h1:IFT+hup2xejHq gitea.com/lunny/levelqueue v0.4.2-0.20230414023320-3c0159fe0fe4/go.mod h1:HBqmLbz56JWpfEGG0prskAV97ATNRoj5LDmPicD22hU= gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a h1:lSA0F4e9A2NcQSqGqTOXqu2aRi/XEQxDCBwM8yJtE6s= gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a/go.mod h1:EXuID2Zs0pAQhH8yz+DNjUbjppKQzKFAn28TMYPB6IU= -gitea.dev/actions-proto-go v0.5.0 h1:Fc3DI4Fm3B3JBRXFUjegql+usoNAjjAw1cxMansfA2I= -gitea.dev/actions-proto-go v0.5.0/go.mod h1:p4RX+D9oqiEEzzkPMXscw2CmaGuYFPWFc6xIOmDNDqs= +gitea.dev/actions-proto-go v0.6.0 h1:gjllYQ5vmwlkqOeofTQu5qKTZpmf7kWsafoHvoPCSzY= +gitea.dev/actions-proto-go v0.6.0/go.mod h1:p4RX+D9oqiEEzzkPMXscw2CmaGuYFPWFc6xIOmDNDqs= gitea.dev/sdk v1.0.1 h1:CWXQUQvp2I6YKOWkhYo1Flx2sRNfMK1X9Op4oR2awXs= gitea.dev/sdk v1.0.1/go.mod h1:jCf5Uzz0Jkb61jxNgMxLOCWwle1J1B2nKdcRtxuK9rY= github.com/42wim/httpsig v1.2.4 h1:mI5bH0nm4xn7K18fo1K3okNDRq8CCJ0KbBYWyA6r8lU= diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go index 803b7c13a12..012f875ec70 100644 --- a/routers/api/actions/runner/runner.go +++ b/routers/api/actions/runner/runner.go @@ -69,7 +69,7 @@ func (s *Service) Register( } labels := req.Msg.Labels - hasCancellingSupport, _ := runnerRequestHasCancellingCapability(req.Msg) + hasCancellingSupport := slices.Contains(req.Msg.GetCapabilities(), runnerCapabilityCancelling) // create new runner name := util.EllipsisDisplayString(req.Msg.Name, 255) @@ -116,26 +116,11 @@ func (s *Service) Register( // state and will run post-step cleanup before finalizing the task. const runnerCapabilityCancelling = "cancelling" -type capabilityGetter interface { - GetCapabilities() []string -} - type declareRequest interface { proto.Message GetVersion() string GetLabels() []string -} - -func runnerRequestHasCancellingCapability(req proto.Message) (bool, bool) { - if req == nil { - return false, false - } - - if typedReq, ok := any(req).(capabilityGetter); ok { - return slices.Contains(typedReq.GetCapabilities(), runnerCapabilityCancelling), true - } - - return false, false + GetCapabilities() []string } func applyDeclareRequestToRunner(runner *actions_model.ActionRunner, req declareRequest) []string { @@ -143,8 +128,8 @@ func applyDeclareRequestToRunner(runner *actions_model.ActionRunner, req declare runner.Version = req.GetVersion() cols := []string{"agent_labels", "version"} - hasCancellingSupport, capabilityStateKnown := runnerRequestHasCancellingCapability(req) - if capabilityStateKnown && runner.HasCancellingSupport != hasCancellingSupport { + hasCancellingSupport := slices.Contains(req.GetCapabilities(), runnerCapabilityCancelling) + if runner.HasCancellingSupport != hasCancellingSupport { runner.HasCancellingSupport = hasCancellingSupport cols = append(cols, "has_cancelling_support") } diff --git a/routers/api/actions/runner/runner_test.go b/routers/api/actions/runner/runner_test.go index b2ed9290878..8a38ac70a23 100644 --- a/routers/api/actions/runner/runner_test.go +++ b/routers/api/actions/runner/runner_test.go @@ -12,47 +12,22 @@ import ( "github.com/stretchr/testify/assert" ) -type capabilityRegisterRequest struct { - *runnerv1.RegisterRequest - capabilities []string -} - -func (r *capabilityRegisterRequest) GetCapabilities() []string { - return r.capabilities -} - -type capabilityDeclareRequest struct { - *runnerv1.DeclareRequest - capabilities []string -} - -func (r *capabilityDeclareRequest) GetCapabilities() []string { - return r.capabilities -} - -func TestRunnerRequestHasCancellingCapabilityTypedAccessor(t *testing.T) { - registerReq := &capabilityRegisterRequest{ - RegisterRequest: &runnerv1.RegisterRequest{}, - capabilities: []string{runnerCapabilityCancelling, "other"}, +func TestApplyDeclareRequestToRunnerAdvertisedCapabilityEnablesCancelling(t *testing.T) { + runner := &actions_model.ActionRunner{} + req := &runnerv1.DeclareRequest{ + Version: "1.2.3", + Labels: []string{"linux"}, + Capabilities: []string{runnerCapabilityCancelling, "other"}, } - hasCapability, known := runnerRequestHasCancellingCapability(registerReq) - assert.True(t, hasCapability) - assert.True(t, known) - declareReq := &capabilityDeclareRequest{ - DeclareRequest: &runnerv1.DeclareRequest{}, - capabilities: nil, - } - hasCapability, known = runnerRequestHasCancellingCapability(declareReq) - assert.False(t, hasCapability) - assert.True(t, known) - - hasCapability, known = runnerRequestHasCancellingCapability(nil) - assert.False(t, hasCapability) - assert.False(t, known) + cols := applyDeclareRequestToRunner(runner, req) + assert.Equal(t, []string{"agent_labels", "version", "has_cancelling_support"}, cols) + assert.True(t, runner.HasCancellingSupport) + assert.Equal(t, "1.2.3", runner.Version) + assert.Equal(t, []string{"linux"}, runner.AgentLabels) } -func TestApplyDeclareRequestToRunnerPreservesUnknownCapabilityState(t *testing.T) { +func TestApplyDeclareRequestToRunnerMissingCapabilityDisablesCancelling(t *testing.T) { runner := &actions_model.ActionRunner{ HasCancellingSupport: true, } @@ -61,26 +36,22 @@ func TestApplyDeclareRequestToRunnerPreservesUnknownCapabilityState(t *testing.T Labels: []string{"linux"}, } - cols := applyDeclareRequestToRunner(runner, req) - assert.Equal(t, []string{"agent_labels", "version"}, cols) - assert.True(t, runner.HasCancellingSupport) - assert.Equal(t, "1.2.3", runner.Version) - assert.Equal(t, []string{"linux"}, runner.AgentLabels) -} - -func TestApplyDeclareRequestToRunnerUpdatesTypedCapabilityState(t *testing.T) { - runner := &actions_model.ActionRunner{ - HasCancellingSupport: true, - } - req := &capabilityDeclareRequest{ - DeclareRequest: &runnerv1.DeclareRequest{ - Version: "1.2.3", - Labels: []string{"linux"}, - }, - capabilities: []string{}, - } - cols := applyDeclareRequestToRunner(runner, req) assert.Equal(t, []string{"agent_labels", "version", "has_cancelling_support"}, cols) assert.False(t, runner.HasCancellingSupport) } + +func TestApplyDeclareRequestToRunnerUnchangedCapabilityOmitsColumn(t *testing.T) { + runner := &actions_model.ActionRunner{ + HasCancellingSupport: true, + } + req := &runnerv1.DeclareRequest{ + Version: "1.2.3", + Labels: []string{"linux"}, + Capabilities: []string{runnerCapabilityCancelling}, + } + + cols := applyDeclareRequestToRunner(runner, req) + assert.Equal(t, []string{"agent_labels", "version"}, cols) + assert.True(t, runner.HasCancellingSupport) +}