chore(coderd): extract api version validation to util package #11407
Next
Next commit
chore(coderd): extract api version validation to util package
- Loading branch information
commit 80b3dc85878c755ee33155bbac4247e53365c3ee
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| package apiversion | ||
|
|
||
| import ( | ||
| "slices" | ||
| "strconv" | ||
| "strings" | ||
|
|
||
| "golang.org/x/xerrors" | ||
| ) | ||
|
|
||
| func New(maj []int, min int) *APIVersion { | ||
| v := &APIVersion{ | ||
| supportedMajors: maj, | ||
| supportedMinor: min, | ||
| } | ||
| return v | ||
| } | ||
|
|
||
| type APIVersion struct { | ||
| supportedMajors []int | ||
| supportedMinor int | ||
| } | ||
|
|
||
| func (v *APIVersion) Validate(version string) error { | ||
| if len(v.supportedMajors) == 0 { | ||
| return xerrors.Errorf("developer error: SupportedMajors is empty") | ||
|
There was a problem hiding this comment. do this error checking on creation There was a problem hiding this comment. Modified function signature to make this impossible. |
||
| } | ||
| currentMajor := slices.Max(v.supportedMajors) | ||
| major, minor, err := Parse(version) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if major > currentMajor { | ||
| return xerrors.Errorf("server is at version %d.%d, behind requested major version %s", | ||
| currentMajor, v.supportedMinor, version) | ||
| } | ||
| if major == currentMajor { | ||
| if minor > v.supportedMinor { | ||
| return xerrors.Errorf("server is at version %d.%d, behind requested minor version %s", | ||
| currentMajor, v.supportedMinor, version) | ||
| } | ||
| return nil | ||
| } | ||
| for _, mjr := range v.supportedMajors { | ||
| if major == mjr { | ||
| return nil | ||
| } | ||
| } | ||
| return xerrors.Errorf("version %s is no longer supported", version) | ||
| } | ||
|
|
||
| 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 | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| package apiversion_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/coder/coder/v2/coderd/util/apiversion" | ||
| ) | ||
|
|
||
| func TestAPIVersionValidate(t *testing.T) { | ||
| // Given | ||
| v := apiversion.New([]int{2, 1}, 0) | ||
|
There was a problem hiding this comment. since this is now abstract, set the minor to 1 so you can also test that it works for minor < supportedMinor |
||
|
|
||
| t.Parallel() | ||
| for _, tc := range []struct { | ||
| name string | ||
| version string | ||
| expectedError string | ||
| }{ | ||
| { | ||
| name: "OK", | ||
| version: "2.0", | ||
| }, | ||
| { | ||
| name: "TooNewMinor", | ||
| version: "2.1", | ||
| expectedError: "behind requested minor version", | ||
| }, | ||
| { | ||
| name: "TooNewMajor", | ||
| version: "3.1", | ||
| expectedError: "behind requested major version", | ||
| }, | ||
| { | ||
| name: "1.0", | ||
| version: "1.0", | ||
| }, | ||
| { | ||
| name: "2.0", | ||
| version: "2.0", | ||
| }, | ||
| { | ||
| 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() | ||
| err := v.Validate(tc.version) | ||
| if tc.expectedError == "" { | ||
| require.NoError(t, err) | ||
| } else { | ||
| require.ErrorContains(t, err, tc.expectedError) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean if you do
{ supportedMajors: []int{1,2} supportedMinor : 2 }That v2.1 is not supported? Because the
minoronly supports 1 args, so all majors must be >= minor?Feels odd because you could say support,
v1.8andv2.0, but in this function you cannot do that without supportingv1.0There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I agree. We may need to change this logic when supporting multiple minor versions. This is just a refactor though and I don't want to change the existing logic, just move it around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, if you support majors {1,2} and minor 2, then you support 2.2, 2.1, 2.0 and 1.x
The minor support only applies to the highest major version.
That's because of the rules we have for how we bump major and minor versions.
If we make an API change that is back compatible, we bump the minor version.
If we make an API change that is non back compatible, we go to a new major version, and, ideally, keep support for the old major version for some time after. The old APIs we continue to support are feature-frozen, so we don't care about their minor versions any longer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, that makes sense. Thanks for clarifying! I added the above to the doc comment for
Validate().