chore(coderd): extract api version validation to util package #11407
Conversation
coderd/util/apiversion/apiversion.go
Outdated
| type APIVersion struct { | ||
| supportedMajors []int | ||
| supportedMinor int | ||
| } |
There was a problem hiding this comment.
Does this mean if you do
{
supportedMajors: []int{1,2}
supportedMinor : 2
}That v2.1 is not supported? Because the minor only supports 1 args, so all majors must be >= minor?
Feels odd because you could say support, v1.8 and v2.0, but in this function you cannot do that without supporting v1.0
There was a problem hiding this comment.
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.
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.
OK, that makes sense. Thanks for clarifying! I added the above to the doc comment for Validate().
coderd/util/apiversion/apiversion.go
Outdated
|
|
||
| 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.
|
|
||
| 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
| additionalMajors []int | ||
| } | ||
|
|
||
| func (v *APIVersion) WithBackwardCompat(majs ...int) *APIVersion { |
Extracts API version logic to a util package.