Skip to content

chore(coderd): extract api version validation to util package #11407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions coderd/util/apiversion/apiversion.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package apiversion

import (
"strconv"
"strings"

"golang.org/x/xerrors"
)

// New returns an *APIVersion with the given major.minor and
// additional supported major versions.
func New(maj, min int) *APIVersion {
v := &APIVersion{
supportedMajor: maj,
supportedMinor: min,
additionalMajors: make([]int, 0),
}
return v
}

type APIVersion struct {
supportedMajor int
supportedMinor int
additionalMajors []int
}

func (v *APIVersion) WithBackwardCompat(majs ...int) *APIVersion {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this.

v.additionalMajors = append(v.additionalMajors, majs[:]...)
return v
}

// Validate validates the given version against the given constraints:
// A given major.minor version is valid iff:
// 1. The requested major version is contained within v.supportedMajors
// 2. If the requested major version is the 'current major', then
// the requested minor version must be less than or equal to the supported
// minor version.
//
// For example, given majors {1, 2} and minor 2, then:
// - 0.x is not supported,
// - 1.x is supported,
// - 2.0, 2.1, and 2.2 are supported,
// - 2.3+ is not supported.
func (v *APIVersion) Validate(version string) error {
major, minor, err := Parse(version)
if err != nil {
return err
}
if major > v.supportedMajor {
return xerrors.Errorf("server is at version %d.%d, behind requested major version %s",
v.supportedMajor, v.supportedMinor, version)
}
if major == v.supportedMajor {
if minor > v.supportedMinor {
return xerrors.Errorf("server is at version %d.%d, behind requested minor version %s",
v.supportedMajor, v.supportedMinor, version)
}
return nil
}
for _, mjr := range v.additionalMajors {
if major == mjr {
return nil
}
}
return xerrors.Errorf("version %s is no longer supported", version)
}

// Parse parses a valid major.minor version string into (major, minor).
// Both major and minor must be valid integers separated by a period '.'.
func Parse(version string) (major int, minor int, err error) {
parts := strings.Split(version, ".")
if len(parts) != 2 {
return 0, 0, xerrors.Errorf("invalid version string: %s", version)
}
major, err = strconv.Atoi(parts[0])
if err != nil {
return 0, 0, xerrors.Errorf("invalid major version: %s", version)
}
minor, err = strconv.Atoi(parts[1])
if err != nil {
return 0, 0, xerrors.Errorf("invalid minor version: %s", version)
}
return major, minor, nil
}
90 changes: 90 additions & 0 deletions coderd/util/apiversion/apiversion_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package apiversion_test

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/coder/coder/v2/coderd/util/apiversion"
)

func TestAPIVersionValidate(t *testing.T) {
t.Parallel()

// Given
v := apiversion.New(2, 1).WithBackwardCompat(1)

for _, tc := range []struct {
name string
version string
expectedError string
}{
{
name: "OK",
version: "2.1",
},
{
name: "MinorOK",
version: "2.0",
},
{
name: "MajorOK",
version: "1.0",
},
{
name: "TooNewMinor",
version: "2.2",
expectedError: "behind requested minor version",
},
{
name: "TooNewMajor",
version: "3.1",
expectedError: "behind requested major version",
},
{
name: "Malformed0",
version: "cats",
expectedError: "invalid version string",
},
{
name: "Malformed1",
version: "cats.dogs",
expectedError: "invalid major version",
},
{
name: "Malformed2",
version: "1.dogs",
expectedError: "invalid minor version",
},
{
name: "Malformed3",
version: "1.0.1",
expectedError: "invalid version string",
},
{
name: "Malformed4",
version: "11",
expectedError: "invalid version string",
},
{
name: "TooOld",
version: "0.8",
expectedError: "no longer supported",
},
} {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

// When
err := v.Validate(tc.version)

// Then
if tc.expectedError == "" {
require.NoError(t, err)
} else {
require.ErrorContains(t, err, tc.expectedError)
}
})
}
}
2 changes: 1 addition & 1 deletion coderd/workspaceagents.go
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,7 @@ func (api *API) workspaceAgentClientCoordinate(rw http.ResponseWriter, r *http.R
if qv != "" {
version = qv
}
if err := tailnet.ValidateVersion(version); err != nil {
if err := tailnet.CurrentVersion.Validate(version); err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Unknown or unsupported API version",
Validations: []codersdk.ValidationError{
Expand Down
47 changes: 3 additions & 44 deletions tailnet/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"context"
"io"
"net"
"strconv"
"strings"
"sync/atomic"
"time"

Expand All @@ -16,6 +14,7 @@ import (
"tailscale.com/tailcfg"

"cdr.dev/slog"
"github.com/coder/coder/v2/coderd/util/apiversion"
"github.com/coder/coder/v2/tailnet/proto"

"golang.org/x/xerrors"
Expand All @@ -26,47 +25,7 @@ const (
CurrentMinor = 0
)

var SupportedMajors = []int{2, 1}

func ValidateVersion(version string) error {
major, minor, err := parseVersion(version)
if err != nil {
return err
}
if major > CurrentMajor {
return xerrors.Errorf("server is at version %d.%d, behind requested version %s",
CurrentMajor, CurrentMinor, version)
}
if major == CurrentMajor {
if minor > CurrentMinor {
return xerrors.Errorf("server is at version %d.%d, behind requested version %s",
CurrentMajor, CurrentMinor, version)
}
return nil
}
for _, mjr := range SupportedMajors {
if major == mjr {
return nil
}
}
return xerrors.Errorf("version %s is no longer supported", version)
}

func parseVersion(version string) (major int, minor int, err error) {
parts := strings.Split(version, ".")
if len(parts) != 2 {
return 0, 0, xerrors.Errorf("invalid version string: %s", version)
}
major, err = strconv.Atoi(parts[0])
if err != nil {
return 0, 0, xerrors.Errorf("invalid major version: %s", version)
}
minor, err = strconv.Atoi(parts[1])
if err != nil {
return 0, 0, xerrors.Errorf("invalid minor version: %s", version)
}
return major, minor, nil
}
var CurrentVersion = apiversion.New(CurrentMajor, CurrentMinor).WithBackwardCompat(1)

type streamIDContextKey struct{}

Expand Down Expand Up @@ -127,7 +86,7 @@ func NewClientService(
}

func (s *ClientService) ServeClient(ctx context.Context, version string, conn net.Conn, id uuid.UUID, agent uuid.UUID) error {
major, _, err := parseVersion(version)
major, _, err := apiversion.Parse(version)
if err != nil {
s.logger.Warn(ctx, "serve client called with unparsable version", slog.Error(err))
return err
Expand Down
65 changes: 0 additions & 65 deletions tailnet/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package tailnet_test

import (
"context"
"fmt"
"io"
"net"
"net/http"
Expand All @@ -25,70 +24,6 @@ import (
"github.com/coder/coder/v2/tailnet"
)

func TestValidateVersion(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name string
version string
supported bool
}{
{
name: "Current",
version: fmt.Sprintf("%d.%d", tailnet.CurrentMajor, tailnet.CurrentMinor),
supported: true,
},
{
name: "TooNewMinor",
version: fmt.Sprintf("%d.%d", tailnet.CurrentMajor, tailnet.CurrentMinor+1),
},
{
name: "TooNewMajor",
version: fmt.Sprintf("%d.%d", tailnet.CurrentMajor+1, tailnet.CurrentMinor),
},
{
name: "1.0",
version: "1.0",
supported: true,
},
{
name: "2.0",
version: "2.0",
supported: true,
},
{
name: "Malformed0",
version: "cats",
},
{
name: "Malformed1",
version: "cats.dogs",
},
{
name: "Malformed2",
version: "1.0.1",
},
{
name: "Malformed3",
version: "11",
},
{
name: "TooOld",
version: "0.8",
},
} {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
err := tailnet.ValidateVersion(tc.version)
if tc.supported {
require.NoError(t, err)
} else {
require.Error(t, err)
}
})
}
}

func TestClientService_ServeClient_V2(t *testing.T) {
t.Parallel()
fCoord := newFakeCoordinator()
Expand Down