Skip to content

Commit

Permalink
Ensures last_known_envoy.txt doesn't reference an inconsistent version
Browse files Browse the repository at this point in the history
This changes the logic that verfies last_known_envoy.txt such that it
only expects updates when all supported platforms are available.

Before, this would update when the first platform was available, which
would break development (ex linux out before macOS).

Note: this doesn't address the similar problem at runtime, and a variant
of the problem in the e2e tests. This PR is standalone to highlight the
switch from using `jq` to go for linting.

See #393

Signed-off-by: Adrian Cole <adrian@tetrate.io>
  • Loading branch information
Adrian Cole committed Nov 1, 2021
1 parent 0097aa7 commit fd3ff6b
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 9 deletions.
4 changes: 1 addition & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,12 @@ format:
# lint is a PHONY target, so always runs. This allows skipping when sources didn't change.
build/lint: .golangci.yml $(all_sources)
@$(go) run $(golangci_lint) run --timeout 5m --config $< ./...
@$(go) test ./lint/...
@mkdir -p $(@D) && touch $@

lint:
@printf "$(ansi_format_dark)" lint "Running linters"
@$(MAKE) build/lint
@# this will taint if we are behind from latest binary. printf avoids adding a newline to the file
@curl -fsSL https://archive.tetratelabs.io/envoy/envoy-versions.json |jq -er .latestVersion|xargs printf "%s" \
>./internal/version/last_known_envoy.txt
@printf "$(ansi_format_bright)" lint "ok"

# CI blocks merge until this passes. If this fails, run "make check" locally and commit the difference.
Expand Down
10 changes: 5 additions & 5 deletions internal/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const debugSuffix = "_debug"
type Version interface {
// String allows access to the underlying representation. Ex. "1.18", "1.18_debug", "1.19.3_debug"
String() string
// ToMinor returns a variant used to look up the latest patch.
// ToMinor returns a variant used to look up the latest Patch.
ToMinor() MinorVersion
}

Expand Down Expand Up @@ -121,9 +121,9 @@ func (v PatchVersion) ToMinor() MinorVersion {
return MinorVersion(latestPatchFormat)
}

// patch attempts to parse a patch number from the Version.String.
// 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 {
func (v PatchVersion) Patch() int {
var matched []string
if matched = versionPattern.FindStringSubmatch(v.String()); matched == nil {
return 0 // impossible if created via NewVersion or NewPatchVersion
Expand All @@ -132,7 +132,7 @@ func (v PatchVersion) patch() int {
return i
}

// FindLatestPatchVersion finds the latest patch version for the given minor version or empty if not found.
// FindLatestPatchVersion finds the latest Patch version for the given minor version or empty if not found.
func FindLatestPatchVersion(patchVersions []PatchVersion, minorVersion MinorVersion) PatchVersion {
var latestVersion PatchVersion
var latestPatch int
Expand All @@ -141,7 +141,7 @@ func FindLatestPatchVersion(patchVersions []PatchVersion, minorVersion MinorVers
continue
}

if p := v.patch(); p >= latestPatch {
if p := v.Patch(); p >= latestPatch {
latestPatch = p
latestVersion = v
}
Expand Down
2 changes: 1 addition & 1 deletion internal/version/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func TestPatchVersion_ParsePatch(t *testing.T) {
for _, tt := range tests {
tc := tt
t.Run(tc.input.String(), func(t *testing.T) {
actual := tc.input.patch()
actual := tc.input.Patch()
require.Equal(t, tc.expected, actual)
})
}
Expand Down
76 changes: 76 additions & 0 deletions lint/last_known_envoy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Copyright 2021 Tetrate
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package lint

import (
"context"
"os"
"testing"

"github.com/stretchr/testify/require"

"github.com/tetratelabs/func-e/internal/envoy"
"github.com/tetratelabs/func-e/internal/globals"
"github.com/tetratelabs/func-e/internal/version"
)

const lastKnownEnvoyFile = "../internal/version/last_known_envoy.txt"

// TestLastKnownEnvoyAvailableOnAllPlatforms ensures that an inconsistent Envoy release doesn't end up being suggested,
// or used in unit tests. This passes only when all platforms are available. This is most frequently inconsistent due to
// Homebrew (macOS) being a version behind latest Linux.
//
// This issues a remote call to the versions server, so shouldn't be a normal unit test (as they must pass offline).
// This is invoked via `make lint`.

// requiredPlatforms are the platforms that Envoy is available on, which may differ than func-e.
// func-e's platforms are defined in the Makefile and are slightly wider due to the --platform flag.
var requiredPlatforms = []version.Platform{
"linux/amd64",
"linux/arm64",
"darwin/amd64",
// "darwin/arm64", TODO: https://github.com/envoyproxy/envoy/issues/1648
"windows/amd64",
// "windows/arm64", TODO: https://github.com/envoyproxy/envoy/issues/17572
}

func TestLastKnownEnvoyAvailableOnAllPlatforms(t *testing.T) {
// We know that the canonical json never has a debug version, so that doesn't need to be handled.
getEnvoyVersions := envoy.NewGetVersions(globals.DefaultEnvoyVersionsURL, globals.DefaultPlatform, "dev")
evs, err := getEnvoyVersions(context.Background())
require.NoError(t, err)

lastKnownEnvoy := version.PatchVersion("1.10.0")
version:
for v, r := range evs.Versions {
// Ensure all platforms are available, or skip the version
for _, p := range requiredPlatforms {
if _, ok := r.Tarballs[p]; !ok {
continue version
}
}

// 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 > lastKnownEnvoy.ToMinor() ||
m == lastKnownEnvoy.ToMinor() && v.Patch() > lastKnownEnvoy.Patch() {
lastKnownEnvoy = v
}
}

actual, err := os.ReadFile(lastKnownEnvoyFile)
require.NoError(t, err)
require.Equal(t, lastKnownEnvoy.String(), string(actual))
}

0 comments on commit fd3ff6b

Please sign in to comment.