Ignores latestRelease in envoy-versions.json (#407)

This stops reading latestRelease in the envoy-versions.json as it is not platform-specific and causes usability and test glitches. Instead, the last available version is looked up based on the current platform. Notably, this allows linux to release ahead of macOS and without causing problems. Fixes #393 Signed-off-by: Adrian Cole <[email protected]>
tetratelabs · Nov 2, 2021 · 89a250f · 89a250f
1 parent 69d8caf
commit 89a250f
Show file tree

Hide file tree

Showing 7 changed files with 198 additions and 70 deletions.
diff --git a/internal/cmd/run.go b/internal/cmd/run.go
@@ -141,8 +141,10 @@ func setEnvoyVersion(ctx context.Context, o *globals.GlobalOpts) (err error) {
 	if evs, err = o.GetEnvoyVersions(ctx); err != nil {
 		return fmt.Errorf("couldn't lookup the latest Envoy version from %s: %w", o.EnvoyVersionsURL, err)
 	}
-	// TODO: LatestVersion may not be available for this platform #393
-	o.EnvoyVersion = evs.LatestVersion
+	o.EnvoyVersion = version.FindLatestVersion(versionsForPlatform(evs.Versions, o.Platform))
+	if o.EnvoyVersion == "" {
+		return fmt.Errorf("%s does not contain an Envoy release for platform %s", o.EnvoyVersionsURL, o.Platform)
+	}
 	// Persist it as a minor version, so that each invocation checks for the latest patch.
 	return envoy.WriteCurrentVersion(o.EnvoyVersion.ToMinor(), o.HomeDir)
 }
@@ -156,12 +158,7 @@ func ensurePatchVersion(ctx context.Context, o *globals.GlobalOpts, v version.Ve
 		evs, err := o.GetEnvoyVersions(ctx)
 		var patchVersions []version.PatchVersion
 		if err == nil {
-			// Filter versions available for this platform
-			for k, v := range evs.Versions {
-				if _, ok = v.Tarballs[o.Platform]; ok {
-					patchVersions = append(patchVersions, k)
-				}
-			}
+			patchVersions = versionsForPlatform(evs.Versions, o.Platform)
 			if pv := version.FindLatestPatchVersion(patchVersions, mv); pv != "" {
 				return pv, nil
 			}
@@ -182,3 +179,13 @@ func ensurePatchVersion(ctx context.Context, o *globals.GlobalOpts, v version.Ve
 	} // version.Version is a union type, so the only other option is a patch!
 	return v.(version.PatchVersion), nil
 }
+
+func versionsForPlatform(vs map[version.PatchVersion]version.Release, p version.Platform) []version.PatchVersion {
+	var patchVersions []version.PatchVersion
+	for k, v := range vs {
+		if _, ok := v.Tarballs[p]; ok {
+			patchVersions = append(patchVersions, k)
+		}
+	}
+	return patchVersions
+}
diff --git a/internal/cmd/run_test.go b/internal/cmd/run_test.go
@@ -92,21 +92,25 @@ func TestSetEnvoyVersion_ErrorReadingExistingVersion(t *testing.T) {
 	require.EqualError(t, err, moreos.ReplacePathSeparator(expectedErr))
 }
 
-// TODO: this is not platform-specific see #393
 func TestSetEnvoyVersion_UsesLatestVersionOnInitialRun(t *testing.T) {
 	o := &globals.GlobalOpts{
 		GetEnvoyVersions: func(context.Context) (*version.ReleaseVersions, error) {
-			return &version.ReleaseVersions{LatestVersion: "1.18.13"}, nil
+			return &version.ReleaseVersions{Versions: map[version.PatchVersion]version.Release{
+				"1.19.2": {Tarballs: map[version.Platform]version.TarballURL{globals.DefaultPlatform: ""}},
+				"1.18.3": {Tarballs: map[version.Platform]version.TarballURL{globals.DefaultPlatform: ""}},
+				"1.20.4": {Tarballs: map[version.Platform]version.TarballURL{"solaris/sparc64": ""}},
+			}}, nil
 		},
-		HomeDir: t.TempDir(),
-		Out:     new(bytes.Buffer), // we expect logging
+		HomeDir:  t.TempDir(),
+		Out:      new(bytes.Buffer), // we expect logging
+		Platform: globals.DefaultPlatform,
 	}
 
 	err := setEnvoyVersion(context.Background(), o)
 	require.NoError(t, err)
 
-	// The LatestVersion was set
-	require.Equal(t, version.PatchVersion("1.18.13"), o.EnvoyVersion)
+	// The highest version for this platform was set
+	require.Equal(t, version.PatchVersion("1.19.2"), o.EnvoyVersion)
 
 	// We notified the user about the remote lookup
 	require.Contains(t, o.Out.(*bytes.Buffer).String(), moreos.Sprintf("looking up the latest Envoy version\n"))
@@ -117,6 +121,27 @@ func TestSetEnvoyVersion_UsesLatestVersionOnInitialRun(t *testing.T) {
 	require.Equal(t, o.EnvoyVersion.ToMinor().String(), string(writtenVersion))
 }
 
+func TestSetEnvoyVersion_NotFound(t *testing.T) {
+	o := &globals.GlobalOpts{
+		GetEnvoyVersions: func(context.Context) (*version.ReleaseVersions, error) {
+			return &version.ReleaseVersions{Versions: map[version.PatchVersion]version.Release{
+				"1.18.14": {Tarballs: map[version.Platform]version.TarballURL{"solaris/sparc64": ""}},
+			}}, nil
+		},
+		EnvoyVersionsURL: "fake URL", // for logging
+		HomeDir:          t.TempDir(),
+		Out:              new(bytes.Buffer), // we expect logging
+		Platform:         globals.DefaultPlatform,
+	}
+
+	err := setEnvoyVersion(context.Background(), o)
+	expectedErr := fmt.Sprintf("fake URL does not contain an Envoy release for platform %s", o.Platform)
+	require.EqualError(t, err, expectedErr)
+
+	// We notified the user about the remote lookup
+	require.Contains(t, o.Out.(*bytes.Buffer).String(), moreos.Sprintf("looking up the latest Envoy version\n"))
+}
+
 func TestSetEnvoyVersion_ErrorLookingUpLatestVersionOnInitialRun(t *testing.T) {
 	o := &globals.GlobalOpts{
 		GetEnvoyVersions: func(context.Context) (*version.ReleaseVersions, error) {
@@ -223,8 +248,8 @@ func TestEnsurePatchVersion_FallbackSuccess(t *testing.T) {
 		},
 	}
 
-	for _, tc := range tests {
-		tc := tc // pin! see https://github.com/kyoh86/scopelint for why
+	for _, tt := range tests {
+		tc := tt // pin! see https://github.com/kyoh86/scopelint for why
 		t.Run(tc.name, func(t *testing.T) {
 
 			o := &globals.GlobalOpts{
@@ -263,3 +288,36 @@ func TestEnsurePatchVersion_FallbackFailure(t *testing.T) {
 	// We notified the user about the remote lookup
 	require.Contains(t, o.Out.(*bytes.Buffer).String(), moreos.Sprintf("looking up the latest patch for Envoy version 1.18\n"))
 }
+
+func TestVersionsForPlatform(t *testing.T) {
+	type testCase struct {
+		name     string
+		versions map[version.PatchVersion]version.Release
+		expected []version.PatchVersion
+	}
+	tests := []testCase{
+		{
+			name:     "empty",
+			versions: map[version.PatchVersion]version.Release{},
+		},
+		{
+			name: "skips other platform",
+			versions: map[version.PatchVersion]version.Release{
+				version.PatchVersion("1.18.3"):       {Tarballs: map[version.Platform]version.TarballURL{globals.DefaultPlatform: ""}},
+				version.PatchVersion("1.18.13"):      {Tarballs: map[version.Platform]version.TarballURL{globals.DefaultPlatform: ""}},
+				version.PatchVersion("1.18.14"):      {Tarballs: map[version.Platform]version.TarballURL{"solaris/sparc64": ""}},
+				version.PatchVersion("1.18.4"):       {Tarballs: map[version.Platform]version.TarballURL{globals.DefaultPlatform: ""}},
+				version.PatchVersion("1.18.4_debug"): {Tarballs: map[version.Platform]version.TarballURL{globals.DefaultPlatform: ""}},
+			},
+			expected: []version.PatchVersion{"1.18.3", "1.18.13", "1.18.4", "1.18.4_debug"},
+		},
+	}
+
+	for _, tt := range tests {
+		tc := tt // pin! see https://github.com/kyoh86/scopelint for why
+		t.Run(tc.name, func(t *testing.T) {
+			actual := versionsForPlatform(tc.versions, globals.DefaultPlatform)
+			require.ElementsMatch(t, tc.expected, actual)
+		})
+	}
+}
diff --git a/internal/envoy/versions_test.go b/internal/envoy/versions_test.go
@@ -31,6 +31,5 @@ func TestNewGetVersions(t *testing.T) {
 
 	evs, err := gv(context.Background())
 	require.NoError(t, err)
-	require.Equal(t, version.LastKnownEnvoy, evs.LatestVersion)
 	require.Contains(t, evs.Versions, version.LastKnownEnvoy)
 }
diff --git a/internal/test/server.go b/internal/test/server.go
@@ -50,7 +50,6 @@ func RequireEnvoyVersionsTestServer(t *testing.T, v version.PatchVersion) *httpt
 	s := &server{t: t}
 	h := httptest.NewServer(s)
 	s.versions = version.ReleaseVersions{
-		LatestVersion: v,
 		Versions: map[version.PatchVersion]version.Release{ // hard-code date so that tests don't drift
 			v: {ReleaseDate: FakeReleaseDate, Tarballs: map[version.Platform]version.TarballURL{
 				version.Platform(moreos.OSLinux + "/" + runtime.GOARCH):   TarballURL(h.URL, moreos.OSLinux, runtime.GOARCH, v),

diff --git a/internal/version/version.go b/internal/version/version.go
@@ -40,7 +40,7 @@ var LastKnownEnvoy = NewPatchVersion(lastKnownEnvoy)
 var (
 	// LastKnownEnvoyMinor is a convenience constant
 	LastKnownEnvoyMinor = LastKnownEnvoy.ToMinor()
-	versionPattern      = regexp.MustCompile(`^[1-9][0-9]*\.[0-9]+(\.[0-9]+)?(` + debugSuffix + `)?$`)
+	versionPattern      = regexp.MustCompile(`^([1-9][0-9]*\.[0-9]+)(\.[0-9]+)?(` + debugSuffix + `)?$`)
 )
 
 // debugSuffix is used to implement PatchVersion.ToMinor
@@ -61,7 +61,7 @@ type MinorVersion string
 
 // NewMinorVersion returns a MinorVersion for a valid input like "1.19" or empty if invalid.
 func NewMinorVersion(input string) MinorVersion {
-	if matched := versionPattern.FindStringSubmatch(input); len(matched) == 3 && matched[0] != "" && matched[1] == "" {
+	if matched := versionPattern.FindStringSubmatch(input); len(matched) == 4 && matched[0] != "" && matched[2] == "" {
 		return MinorVersion(input)
 	}
 	return ""
@@ -73,7 +73,7 @@ type PatchVersion string
 
 // NewPatchVersion returns a PatchVersion for a valid input like "1.19.1" or empty if invalid.
 func NewPatchVersion(input string) PatchVersion {
-	if matched := versionPattern.FindStringSubmatch(input); len(matched) == 3 && matched[0] != "" && matched[1] != "" {
+	if matched := versionPattern.FindStringSubmatch(input); len(matched) == 4 && matched[0] != "" && matched[2] != "" {
 		return PatchVersion(input)
 	}
 	return ""
@@ -110,25 +110,21 @@ func (v PatchVersion) String() string {
 
 // ToMinor satisfies Version.ToMinor
 func (v PatchVersion) ToMinor() MinorVersion {
-	splitDebug := strings.Split(string(v), debugSuffix)
-	splitVersion := strings.Split(splitDebug[0], ".")
-	latestPatchFormat := fmt.Sprintf("%s.%s", splitVersion[0], splitVersion[1])
-
-	if len(splitDebug) == 2 {
-		latestPatchFormat += debugSuffix
+	matched := versionPattern.FindStringSubmatch(v.String())
+	if matched == nil {
+		return "" // impossible if created via NewVersion or NewPatchVersion
 	}
-
-	return MinorVersion(latestPatchFormat)
+	return MinorVersion(matched[1] + matched[3]) // ex. "1.18" + ""  or "1.18" + "_debug"
 }
 
 // Patch attempts to parse a Patch number from the Version.String.
 // This will always succeed when created via NewVersion or NewPatchVersion
 func (v PatchVersion) Patch() int {
-	var matched []string
-	if matched = versionPattern.FindStringSubmatch(v.String()); matched == nil {
+	matched := versionPattern.FindStringSubmatch(v.String())
+	if matched == nil {
 		return 0 // impossible if created via NewVersion or NewPatchVersion
 	}
-	i, _ := strconv.Atoi(matched[1][1:]) // matched[1] will look like .1 or .10
+	i, _ := strconv.Atoi(matched[2][1:]) // matched[2] will look like .1 or .10
 	return i
 }
 
@@ -149,13 +145,29 @@ func FindLatestPatchVersion(patchVersions []PatchVersion, minorVersion MinorVers
 	return latestVersion
 }
 
+// FindLatestVersion finds the latest non-debug version or empty if the input is.
+func FindLatestVersion(patchVersions []PatchVersion) PatchVersion {
+	var latestVersion PatchVersion
+	for _, v := range patchVersions {
+		if strings.HasSuffix(v.String(), debugSuffix) {
+			continue
+		}
+
+		// Until Envoy 2.0.0, minor versions are double-digit and can be lexicographically compared.
+		// Patches have to be numerically compared, as they can be single or double-digit (albeit unlikely).
+		if m := v.ToMinor(); m > latestVersion.ToMinor() ||
+			m == latestVersion.ToMinor() && v.Patch() > latestVersion.Patch() {
+			latestVersion = v
+		}
+	}
+	return latestVersion
+}
+
 // GetReleaseVersions returns a version map from a remote URL. e.g. from globals.DefaultEnvoyVersionsURL
 type GetReleaseVersions func(ctx context.Context) (*ReleaseVersions, error)
 
 // ReleaseVersions primarily maps Version to TarballURL and tracks the LatestVersion
 type ReleaseVersions struct {
-	// LatestVersion is the latest stable Version
-	LatestVersion PatchVersion `json:"latestVersion"`
 	// Versions maps a Version to its Release
 	Versions map[PatchVersion]Release `json:"versions"`
 	// SHA256Sums maps a Tarball to its SHA256Sum

diff --git a/internal/version/version_test.go b/internal/version/version_test.go
@@ -70,7 +70,7 @@ func TestNewVersion(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tc := tt
+		tc := tt // pin! see https://github.com/kyoh86/scopelint for why
 		t.Run(tc.input, func(t *testing.T) {
 			actual, err := NewVersion("[version] argument", tc.input)
 			require.Equal(t, tc.expected, actual)
@@ -115,7 +115,7 @@ func TestNewMinorVersion(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tc := tt
+		tc := tt // pin! see https://github.com/kyoh86/scopelint for why
 		t.Run(tc.input, func(t *testing.T) {
 			actual := NewMinorVersion(tc.input)
 			require.Equal(t, tc.expected, actual)
@@ -155,15 +155,15 @@ func TestNewPatchVersion(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tc := tt
+		tc := tt // pin! see https://github.com/kyoh86/scopelint for why
 		t.Run(tc.input, func(t *testing.T) {
 			actual := NewPatchVersion(tc.input)
 			require.Equal(t, tc.expected, actual)
 		})
 	}
 }
 
-func TestPatchVersion_ParsePatch(t *testing.T) {
+func TestPatchVersion_Patch(t *testing.T) {
 	tests := []struct {
 		input    PatchVersion
 		expected int
@@ -199,7 +199,7 @@ func TestPatchVersion_ParsePatch(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tc := tt
+		tc := tt // pin! see https://github.com/kyoh86/scopelint for why
 		t.Run(tc.input.String(), func(t *testing.T) {
 			actual := tc.input.Patch()
 			require.Equal(t, tc.expected, actual)
@@ -230,8 +230,8 @@ func TestVersion_String(t *testing.T) {
 		},
 	}
 
-	for _, tt := range tests {
-		tc := tt
+	for _, tc := range tests {
+		tc := tc // pin! see https://github.com/kyoh86/scopelint for why
 		t.Run(tc.input.String(), func(t *testing.T) {
 			actual := tc.input.String()
 			require.Equal(t, tc.expected, actual)
@@ -263,7 +263,7 @@ func TestVersion_ToMinor(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tc := tt
+		tc := tt // pin! see https://github.com/kyoh86/scopelint for why
 		t.Run(tc.input.String(), func(t *testing.T) {
 			actual := tc.input.ToMinor()
 			require.Equal(t, tc.expected, actual)
@@ -320,10 +320,66 @@ func TestFindLatestPatch(t *testing.T) {
 		},
 	}
 
-	for _, tc := range tests {
+	for _, tt := range tests {
+		tc := tt // pin! see https://github.com/kyoh86/scopelint for why
 		t.Run(tc.name, func(t *testing.T) {
 			actual := FindLatestPatchVersion(tc.patchVersions, tc.minorVersion)
 			require.Equal(t, tc.expected, actual)
 		})
 	}
 }
+
+func TestFindLatestVersion(t *testing.T) {
+	type testCase struct {
+		name          string
+		patchVersions []PatchVersion
+		expected      PatchVersion
+	}
+
+	tests := []testCase{
+		{
+			name:          "empty",
+			patchVersions: []PatchVersion{},
+		},
+		{
+			name: "all debug is empty",
+			patchVersions: []PatchVersion{
+				PatchVersion("1.19.0_debug"),
+				PatchVersion("1.20.0_debug"),
+			},
+		},
+		{
+			name: "ignores debug",
+			patchVersions: []PatchVersion{
+				PatchVersion("1.20.0_debug"), // mixed debug and not is unlikely, but possible
+				PatchVersion("1.20.0"),
+			},
+			expected: PatchVersion("1.20.0"),
+		},
+		{
+			name: "latest patch",
+			patchVersions: []PatchVersion{
+				PatchVersion("1.18.1"),
+				PatchVersion("1.18.14"),
+				PatchVersion("1.18.4"),
+			},
+			expected: PatchVersion("1.18.14"),
+		},
+		{
+			name: "latest version",
+			patchVersions: []PatchVersion{
+				PatchVersion("1.20.1"),
+				PatchVersion("1.18.2"),
+			},
+			expected: PatchVersion("1.20.1"),
+		},
+	}
+
+	for _, tt := range tests {
+		tc := tt // pin! see https://github.com/kyoh86/scopelint for why
+		t.Run(tc.name, func(t *testing.T) {
+			actual := FindLatestVersion(tc.patchVersions)
+			require.Equal(t, tc.expected, actual)
+		})
+	}
+}