Skip to content

Commit a2cbb0f

Browse files
johnstcnjohnstcn
authored
fix(enterprise/coderd): check provisionerd API version on connection (#12191)
1 parent f17149c commit a2cbb0f

File tree

11 files changed

+125
-7
lines changed

11 files changed

+125
-7
lines changed

‎coderd/util/apiversion/apiversion.go renamed to ‎apiversion/apiversion.go

File renamed without changes.

‎coderd/util/apiversion/apiversion_test.go renamed to ‎apiversion/apiversion_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55

66
"github.com/stretchr/testify/require"
77

8-
"github.com/coder/coder/v2/coderd/util/apiversion"
8+
"github.com/coder/coder/v2/apiversion"
99
)
1010

1111
func TestAPIVersionValidate(t *testing.T) {

‎coderd/healthcheck/provisioner.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import (
77

88
"golang.org/x/mod/semver"
99

10+
"github.com/coder/coder/v2/apiversion"
1011
"github.com/coder/coder/v2/buildinfo"
1112
"github.com/coder/coder/v2/coderd/database"
1213
"github.com/coder/coder/v2/coderd/database/db2sdk"
1314
"github.com/coder/coder/v2/coderd/database/dbauthz"
1415
"github.com/coder/coder/v2/coderd/database/dbtime"
1516
"github.com/coder/coder/v2/coderd/healthcheck/health"
1617
"github.com/coder/coder/v2/coderd/provisionerdserver"
17-
"github.com/coder/coder/v2/coderd/util/apiversion"
1818
"github.com/coder/coder/v2/coderd/util/ptr"
1919
"github.com/coder/coder/v2/codersdk"
2020
"github.com/coder/coder/v2/provisionersdk"

‎codersdk/provisionerdaemons.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/coder/coder/v2/codersdk/drpc"
1919
"github.com/coder/coder/v2/provisionerd/proto"
2020
"github.com/coder/coder/v2/provisionerd/runner"
21+
"github.com/coder/coder/v2/provisionersdk"
2122
)
2223

2324
type LogSource string
@@ -201,6 +202,8 @@ func (c *Client) ServeProvisionerDaemon(ctx context.Context, req ServeProvisione
201202
query := serverURL.Query()
202203
query.Add("id", req.ID.String())
203204
query.Add("name", req.Name)
205+
query.Add("version", provisionersdk.VersionCurrent.String())
206+
204207
for _, provisioner := range req.Provisioners {
205208
query.Add("provisioner", string(provisioner))
206209
}

‎enterprise/coderd/provisionerdaemons.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,16 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
239239
apiVersion = qv
240240
}
241241

242+
if err := provisionersdk.VersionCurrent.Validate(apiVersion); err != nil {
243+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
244+
Message: "Incompatible or unparsable version",
245+
Validations: []codersdk.ValidationError{
246+
{Field: "version", Detail: err.Error()},
247+
},
248+
})
249+
return
250+
}
251+
242252
// Create the daemon in the database.
243253
now := dbtime.Now()
244254
daemon, err := api.Database.UpsertProvisionerDaemon(authCtx, database.UpsertProvisionerDaemonParams{

‎enterprise/coderd/provisionerdaemons_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package coderd_test
33
import (
44
"bytes"
55
"context"
6+
"fmt"
7+
"io"
68
"net/http"
79
"testing"
810

@@ -12,6 +14,7 @@ import (
1214

1315
"cdr.dev/slog"
1416
"cdr.dev/slog/sloggers/slogtest"
17+
"github.com/coder/coder/v2/apiversion"
1518
"github.com/coder/coder/v2/buildinfo"
1619
"github.com/coder/coder/v2/coderd/coderdtest"
1720
"github.com/coder/coder/v2/coderd/database"
@@ -63,6 +66,108 @@ func TestProvisionerDaemonServe(t *testing.T) {
6366
}
6467
})
6568

69+
t.Run("NoVersion", func(t *testing.T) {
70+
t.Parallel()
71+
// In this test, we just send a HTTP request with minimal parameters to the provisionerdaemons
72+
// endpoint. We do not pass the required machinery to start a websocket connection, so we expect a
73+
// WebSocket protocol violation. This just means the pre-flight checks have passed though.
74+
75+
// Sending a HTTP request triggers an error log, which would otherwise fail the test.
76+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
77+
client, user := coderdenttest.New(t, &coderdenttest.Options{
78+
LicenseOptions: &coderdenttest.LicenseOptions{
79+
Features: license.Features{
80+
codersdk.FeatureExternalProvisionerDaemons: 1,
81+
},
82+
},
83+
ProvisionerDaemonPSK: "provisionersftw",
84+
Options: &coderdtest.Options{
85+
Logger: &logger,
86+
},
87+
})
88+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
89+
defer cancel()
90+
91+
// Formulate the correct URL for provisionerd server.
92+
srvURL, err := client.URL.Parse(fmt.Sprintf("/api/v2/organizations/%s/provisionerdaemons/serve", user.OrganizationID))
93+
require.NoError(t, err)
94+
q := srvURL.Query()
95+
// Set required query parameters.
96+
q.Add("provisioner", "echo")
97+
// Note: Explicitly not setting API version.
98+
q.Add("version", "")
99+
srvURL.RawQuery = q.Encode()
100+
101+
// Set PSK header for auth.
102+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, srvURL.String(), nil)
103+
require.NoError(t, err)
104+
req.Header.Set(codersdk.ProvisionerDaemonPSK, "provisionersftw")
105+
106+
// Do the request!
107+
resp, err := client.HTTPClient.Do(req)
108+
require.NoError(t, err)
109+
defer resp.Body.Close()
110+
b, err := io.ReadAll(resp.Body)
111+
require.NoError(t, err)
112+
// The below means that provisionerd tried to serve us!
113+
require.Contains(t, string(b), "Internal error accepting websocket connection.")
114+
115+
daemons, err := client.ProvisionerDaemons(ctx) //nolint:gocritic // Test assertion.
116+
require.NoError(t, err)
117+
if assert.Len(t, daemons, 1) {
118+
assert.Equal(t, "1.0", daemons[0].APIVersion) // The whole point of this test is here.
119+
}
120+
})
121+
122+
t.Run("OldVersion", func(t *testing.T) {
123+
t.Parallel()
124+
// In this test, we just send a HTTP request with minimal parameters to the provisionerdaemons
125+
// endpoint. We do not pass the required machinery to start a websocket connection, but we pass a
126+
// version header that should cause provisionerd to refuse to serve us, so no websocket for you!
127+
128+
// Sending a HTTP request triggers an error log, which would otherwise fail the test.
129+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
130+
client, user := coderdenttest.New(t, &coderdenttest.Options{
131+
LicenseOptions: &coderdenttest.LicenseOptions{
132+
Features: license.Features{
133+
codersdk.FeatureExternalProvisionerDaemons: 1,
134+
},
135+
},
136+
ProvisionerDaemonPSK: "provisionersftw",
137+
Options: &coderdtest.Options{
138+
Logger: &logger,
139+
},
140+
})
141+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
142+
defer cancel()
143+
144+
// Formulate the correct URL for provisionerd server.
145+
srvURL, err := client.URL.Parse(fmt.Sprintf("/api/v2/organizations/%s/provisionerdaemons/serve", user.OrganizationID))
146+
require.NoError(t, err)
147+
q := srvURL.Query()
148+
// Set required query parameters.
149+
q.Add("provisioner", "echo")
150+
151+
// Set a different (newer) version than the current.
152+
v := apiversion.New(provisionersdk.CurrentMajor+1, provisionersdk.CurrentMinor+1)
153+
q.Add("version", v.String())
154+
srvURL.RawQuery = q.Encode()
155+
156+
// Set PSK header for auth.
157+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, srvURL.String(), nil)
158+
require.NoError(t, err)
159+
req.Header.Set(codersdk.ProvisionerDaemonPSK, "provisionersftw")
160+
161+
// Do the request!
162+
resp, err := client.HTTPClient.Do(req)
163+
require.NoError(t, err)
164+
defer resp.Body.Close()
165+
b, err := io.ReadAll(resp.Body)
166+
require.NoError(t, err)
167+
// The below means that provisionerd tried to serve us, checked our api version, and said nope.
168+
require.Contains(t, string(b), "server is at version 1.0, behind requested major version 2.1")
169+
})
170+
66171
t.Run("NoLicense", func(t *testing.T) {
67172
t.Parallel()
68173
client, user := coderdenttest.New(t, &coderdenttest.Options{DontAddLicense: true})

‎enterprise/coderd/workspaceproxycoordinate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import (
66
"github.com/google/uuid"
77
"nhooyr.io/websocket"
88

9+
"github.com/coder/coder/v2/apiversion"
910
"github.com/coder/coder/v2/coderd/httpapi"
10-
"github.com/coder/coder/v2/coderd/util/apiversion"
1111
"github.com/coder/coder/v2/codersdk"
1212
"github.com/coder/coder/v2/tailnet/proto"
1313
)

‎enterprise/tailnet/workspaceproxy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
"tailscale.com/tailcfg"
1515

1616
"cdr.dev/slog"
17-
"github.com/coder/coder/v2/coderd/util/apiversion"
17+
"github.com/coder/coder/v2/apiversion"
1818
"github.com/coder/coder/v2/enterprise/wsproxy/wsproxysdk"
1919
agpl "github.com/coder/coder/v2/tailnet"
2020
)

‎provisionersdk/serve.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ import (
1616

1717
"cdr.dev/slog"
1818

19+
"github.com/coder/coder/v2/apiversion"
1920
"github.com/coder/coder/v2/coderd/tracing"
20-
"github.com/coder/coder/v2/coderd/util/apiversion"
2121
"github.com/coder/coder/v2/provisionersdk/proto"
2222
)
2323

‎tailnet/proto/version.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package proto
22

33
import (
4-
"github.com/coder/coder/v2/coderd/util/apiversion"
4+
"github.com/coder/coder/v2/apiversion"
55
)
66

77
const (

0 commit comments

Comments
 (0)