Update version parsing to work with build info (#12255)

Problem

The version checking that's used when checking if Linkerd is running the
latest version treats strings with the same version but different build
info as unqeual, when they should be considered equal. For instance, the
versions `edge-23.1.3` and `edge-23.1.3-arm` should be treated as equal.

Solution

Update channel version parsing to ignore build info in versions,
unless the build info is a numeric hotpatch, in which case the hotpatch
is preserved and the channel is updated to reflect that.

Validation

See the updated tests in the `version` package.

Signed-off-by: Kevin Ingelman <ki@buoyant.io>
This commit is contained in:
Kevin Ingelman 2024-03-15 11:56:25 -07:00 committed by GitHub
parent a82bf9cb16
commit 5a2aa55cd9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 175 additions and 66 deletions

View File

@ -55,7 +55,7 @@ func (c Channels) Match(actualVersion string) error {
}
for _, cv := range c.array {
if cv.channel == actual.channel {
if cv.updateChannel() == actual.updateChannel() {
return match(cv.String(), actualVersion)
}
}
@ -113,7 +113,7 @@ func getLatestVersions(ctx context.Context, client *http.Client, url string) (Ch
return Channels{}, fmt.Errorf("unexpected versioncheck response: %w", err)
}
if c != cv.channel {
if c != cv.updateChannel() {
return Channels{}, fmt.Errorf("unexpected versioncheck response: channel in %s does not match %s", cv, c)
}

View File

@ -3,7 +3,6 @@ package version
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/http/httptest"
@ -12,27 +11,33 @@ import (
)
func TestGetLatestVersions(t *testing.T) {
four := int64(4)
testCases := []struct {
name string
resp interface{}
err error
latest Channels
}{
{
"valid response",
map[string]string{
"foo": "foo-1.2.3",
"stable": "stable-2.1.0",
"edge": "edge-2.1.0",
"foo": "foo-1.2.3",
"fooHotpatch": "foo-1.2.3-4",
"stable": "stable-2.1.0",
"edge": "edge-2.1.0",
},
nil,
Channels{
[]channelVersion{
{"foo", "1.2.3"},
{"stable", "2.1.0"},
{"edge", "2.1.0"},
{"foo", "1.2.3", nil, "foo-1.2.3"},
{"foo", "1.2.3", &four, "foo-1.2.3-4"},
{"stable", "2.1.0", nil, "stable-2.1.0"},
{"edge", "2.1.0", nil, "edge-2.1.0"},
},
},
},
{
"channel version mismatch",
map[string]string{
"foo": "foo-1.2.3",
"stable": "stable-2.1.0",
@ -42,6 +47,7 @@ func TestGetLatestVersions(t *testing.T) {
Channels{},
},
{
"invalid version",
map[string]string{
"foo": "foo-1.2.3",
"stable": "badchannelversion",
@ -50,15 +56,16 @@ func TestGetLatestVersions(t *testing.T) {
Channels{},
},
{
"invalid JSON",
"bad response",
fmt.Errorf("json: cannot unmarshal string into Go value of type map[string]string"),
Channels{},
},
}
for i, tc := range testCases {
for _, tc := range testCases {
tc := tc // pin
t.Run(fmt.Sprintf("test %d GetLatestVersions(%s, %s)", i, tc.err, tc.latest), func(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
j, err := json.Marshal(tc.resp)
if err != nil {
t.Fatalf("JSON marshal failed with: %s", err)
@ -95,7 +102,7 @@ func channelsEqual(c1, c2 Channels) bool {
for _, cv1 := range c1.array {
found := false
for _, cv2 := range c2.array {
if cv1 == cv2 {
if cv1.channel == cv2.channel && cv1.version == cv2.version && cv1.hotpatchEqual(cv2) {
found = true
break
}
@ -109,50 +116,44 @@ func channelsEqual(c1, c2 Channels) bool {
}
func TestChannelsMatch(t *testing.T) {
four := int64(4)
channels := Channels{
[]channelVersion{
{"stable", "2.1.0", nil, "stable-2.1.0"},
{"foo", "1.2.3", nil, "foo-1.2.3"},
{"foo", "1.2.3", &four, "foo-1.2.3-4"},
{"version", "3.2.1", nil, "version-3.2.1"},
},
}
testCases := []struct {
actualVersion string
channels Channels
err error
}{
{"stable-2.1.0", nil},
{"stable-2.1.0-buildinfo", nil},
{"foo-1.2.3", nil},
{"foo-1.2.3-4", nil},
{"foo-1.2.3-4-buildinfo", nil},
{"version-3.2.1", nil},
{
"version-3.2.1-test",
Channels{
[]channelVersion{
{"stable", "2.1.0"},
{"foo", "1.2.3"},
{"version", "3.2.1-test"},
},
},
nil,
"foo-1.2.2",
fmt.Errorf("is running version 1.2.2 but the latest foo version is 1.2.3"),
},
{
"edge-older",
Channels{
[]channelVersion{
{"stable", "2.1.0"},
{"foo", "1.2.3"},
{"edge", "latest"},
},
},
errors.New("is running version older but the latest edge version is latest"),
"foo-1.2.3-3",
fmt.Errorf("is running version 1.2.3-3 but the latest foo version is 1.2.3-4"),
},
{
"unsupported-version-channel",
Channels{
[]channelVersion{
{"stable", "2.1.0"},
{"foo", "1.2.3"},
{"bar", "3.2.1"},
},
},
fmt.Errorf("unsupported version channel: %s", "unsupported-version-channel"),
"unsupportedChannel-1.2.3",
fmt.Errorf("unsupported version channel: unsupportedChannel-1.2.3"),
},
}
for i, tc := range testCases {
tc := tc // pin
t.Run(fmt.Sprintf("test %d ChannelsMatch(%s, %s)", i, tc.actualVersion, tc.err), func(t *testing.T) {
err := tc.channels.Match(tc.actualVersion)
err := channels.Match(tc.actualVersion)
if (err == nil && tc.err != nil) ||
(err != nil && tc.err == nil) ||
((err != nil && tc.err != nil) && (err.Error() != tc.err.Error())) {

View File

@ -2,30 +2,81 @@ package version
import (
"fmt"
"strconv"
"strings"
)
// channelVersion is a low-level struct for handling release channels in a
// structured way. It has no dependencies on the rest of the version package.
type channelVersion struct {
channel string
version string
channel string
version string
hotpatch *int64
original string
}
// String returns a string representation of a channelVersion, for example:
// { "channel": "version"} => "channel-version"
// hotpatchSuffix is the suffix applied to channel names to indicate that the
// version string includes a hotpatch number (e.g. dev-0.1.2-3)
const hotpatchSuffix = "Hotpatch"
func (cv channelVersion) String() string {
return fmt.Sprintf("%s-%s", cv.channel, cv.version)
return cv.original
}
func parseChannelVersion(cv string) (channelVersion, error) {
if parts := strings.SplitN(cv, "-", 2); len(parts) == 2 {
return channelVersion{
channel: parts[0],
version: parts[1],
}, nil
// updateChannel returns the channel name to check for updates. if there's no
// hotpatch number set, then it returns the channel name itself. otherwise it
// returns the channel name suffixed with "Hotpatch" to indicate that a separate
// update channel should be used.
func (cv channelVersion) updateChannel() string {
if cv.hotpatch != nil {
return cv.channel + hotpatchSuffix
}
return channelVersion{}, fmt.Errorf("unsupported version format: %s", cv)
return cv.channel
}
// versionWithHotpatch returns the version string, suffixed with the hotpatch
// number if it exists.
func (cv channelVersion) versionWithHotpatch() string {
if cv.hotpatch == nil {
return cv.version
}
return fmt.Sprintf("%s-%d", cv.version, *cv.hotpatch)
}
func (cv channelVersion) hotpatchEqual(other channelVersion) bool {
if cv.hotpatch == nil && other.hotpatch == nil {
return true
}
if cv.hotpatch == nil || other.hotpatch == nil {
return false
}
return *cv.hotpatch == *other.hotpatch
}
// parseChannelVersion parses a build string into a channelVersion struct. it
// expects the channel and version to be separated by a hyphen (e.g. dev-0.1.2).
// the version may additionally include a hotpatch number, which is separated
// from the base version by another hyphen (e.g. dev-0.1.2-3). if the version is
// suffixed with any other non-numeric build info strings (e.g. dev-0.1.2-foo),
// those strings are ignored.
func parseChannelVersion(cv string) (channelVersion, error) {
parts := strings.Split(cv, "-")
if len(parts) < 2 {
return channelVersion{}, fmt.Errorf("unsupported version format: %s", cv)
}
channel := parts[0]
version := parts[1]
var hotpatch *int64
for _, part := range parts[2:] {
if i, err := strconv.ParseInt(part, 10, 64); err == nil {
hotpatch = &i
break
}
}
return channelVersion{channel, version, hotpatch, cv}, nil
}
// IsReleaseChannel returns true if the channel of the version is "edge" or

View File

@ -45,8 +45,6 @@ func match(expectedVersion, actualVersion string) error {
return errors.New("expected version is empty")
} else if actualVersion == "" {
return errors.New("actual version is empty")
} else if actualVersion == expectedVersion {
return nil
}
actual, err := parseChannelVersion(actualVersion)
@ -63,6 +61,10 @@ func match(expectedVersion, actualVersion string) error {
actual, expected)
}
return fmt.Errorf("is running version %s but the latest %s version is %s",
actual.version, actual.channel, expected.version)
if actual.version != expected.version || !actual.hotpatchEqual(expected) {
return fmt.Errorf("is running version %s but the latest %s version is %s",
actual.versionWithHotpatch(), actual.channel, expected.versionWithHotpatch())
}
return nil
}

View File

@ -8,22 +8,77 @@ import (
func TestMatch(t *testing.T) {
testCases := []struct {
name string
expected string
actual string
err error
}{
{"dev-foo", "dev-foo", nil},
{"dev-foo-bar", "dev-foo-bar", nil},
{"dev-foo-bar", "dev-foo-baz", errors.New("is running version foo-baz but the latest dev version is foo-bar")},
{"dev-foo", "dev-bar", errors.New("is running version bar but the latest dev version is foo")},
{"dev-foo", "git-foo", errors.New("mismatched channels: running git-foo but retrieved dev-foo")},
{"badformat", "dev-foo", errors.New("failed to parse expected version: unsupported version format: badformat")},
{"dev-foo", "badformat", errors.New("failed to parse actual version: unsupported version format: badformat")},
{
name: "up-to-date",
expected: "dev-0.1.2",
actual: "dev-0.1.2",
},
{
name: "up-to-date with same build info",
expected: "dev-0.1.2-bar",
actual: "dev-0.1.2-bar",
},
{
name: "up-to-date with different build info",
expected: "dev-0.1.2-bar",
actual: "dev-0.1.2-baz",
},
{
name: "up-to-date with hotpatch",
expected: "dev-0.1.2-3",
actual: "dev-0.1.2-3",
},
{
name: "up-to-date with hotpatch and different build info",
expected: "dev-0.1.2-3-bar",
actual: "dev-0.1.2-3-baz",
},
{
name: "not up-to-date",
expected: "dev-0.1.2",
actual: "dev-0.1.1",
err: errors.New("is running version 0.1.1 but the latest dev version is 0.1.2"),
},
{
name: "not up-to-date but with same build info",
expected: "dev-0.1.2-bar",
actual: "dev-0.1.1-bar",
err: errors.New("is running version 0.1.1 but the latest dev version is 0.1.2"),
},
{
name: "not up-to-date with hotpatch",
expected: "dev-0.1.2-3",
actual: "dev-0.1.2-2",
err: errors.New("is running version 0.1.2-2 but the latest dev version is 0.1.2-3"),
},
{
name: "mismatched channels",
expected: "dev-0.1.2",
actual: "git-cb21f1bc",
err: errors.New("mismatched channels: running git-cb21f1bc but retrieved dev-0.1.2"),
},
{
name: "expected version malformed",
expected: "badformat",
actual: "dev-0.1.2",
err: errors.New("failed to parse expected version: unsupported version format: badformat"),
},
{
name: "actual version malformed",
expected: "dev-0.1.2",
actual: "badformat",
err: errors.New("failed to parse actual version: unsupported version format: badformat"),
},
}
for i, tc := range testCases {
for _, tc := range testCases {
tc := tc // pin
t.Run(fmt.Sprintf("test %d match(%s, %s)", i, tc.expected, tc.actual), func(t *testing.T) {
t.Run(fmt.Sprintf(tc.name), func(t *testing.T) {
err := match(tc.expected, tc.actual)
if (err == nil && tc.err != nil) ||
(err != nil && tc.err == nil) ||