modified unsupported envoy version error (#16518)

- When an envoy version is out of a supported range, we now return the envoy version being used as `major.minor.x` to indicate that it is the minor version at most that is incompatible - When an envoy version is in the list of unsupported envoy versions we return back the envoy version in the error message as `major.minor.patch` as now the exact version matters.
2 years ago · c517f07eca
2 changed files with 78 additions and 24 deletions
--- a/command/connect/envoy/envoy.go
+++ b/command/connect/envoy/envoy.go
@ -503,15 +503,15 @@ func (c *cmd) run(args []string) int {
 			return 1
 		}

-		ok, err := checkEnvoyVersionCompatibility(v, xdscommon.UnsupportedEnvoyVersions)
+		ec, err := checkEnvoyVersionCompatibility(v, xdscommon.UnsupportedEnvoyVersions)

 		if err != nil {
 			c.UI.Warn("There was an error checking the compatibility of the envoy version: " + err.Error())
-		} else if !ok {
+		} else if !ec.isCompatible {
 			c.UI.Error(fmt.Sprintf("Envoy version %s is not supported. If there is a reason you need to use "+
 				"this version of envoy use the ignore-envoy-compatibility flag. Using an unsupported version of Envoy "+
 				"is not recommended and your experience may vary. For more information on compatibility "+
-				"see https://developer.hashicorp.com/consul/docs/connect/proxies/envoy#envoy-and-consul-client-agent", v))
+				"see https://developer.hashicorp.com/consul/docs/connect/proxies/envoy#envoy-and-consul-client-agent", ec.versionIncompatible))
 			return 1
 		}
 	}
@ -976,34 +976,73 @@ Usage: consul connect envoy [options] [-- pass-through options]
 `
 )

-func checkEnvoyVersionCompatibility(envoyVersion string, unsupportedList []string) (bool, error) {
-	// Now compare the versions to the list of supported versions
+type envoyCompat struct {
+	isCompatible        bool
+	versionIncompatible string
+}
+
+func checkEnvoyVersionCompatibility(envoyVersion string, unsupportedList []string) (envoyCompat, error) {
 	v, err := version.NewVersion(envoyVersion)
 	if err != nil {
-		return false, err
+		return envoyCompat{}, err
 	}

 	var cs strings.Builder

-	// Add one to the max minor version so that we accept all patches
+	// If there is a list of unsupported versions, build the constraint string,
+	// this will detect exactly unsupported versions
+	if len(unsupportedList) > 0 {
+		for i, s := range unsupportedList {
+			if i == 0 {
+				cs.WriteString(fmt.Sprintf("!= %s", s))
+			} else {
+				cs.WriteString(fmt.Sprintf(", != %s", s))
+			}
+		}
+
+		constraints, err := version.NewConstraint(cs.String())
+		if err != nil {
+			return envoyCompat{}, err
+		}
+
+		if c := constraints.Check(v); !c {
+			return envoyCompat{
+				isCompatible:        c,
+				versionIncompatible: envoyVersion,
+			}, nil
+		}
+	}
+
+	// Next build the constraint string using the bounds, make sure that we are less than but not equal to
+	// maxSupported since we will add 1. Need to add one to the max minor version so that we accept all patches
 	splitS := strings.Split(xdscommon.GetMaxEnvoyMinorVersion(), ".")
 	minor, err := strconv.Atoi(splitS[1])
 	if err != nil {
-		return false, err
+		return envoyCompat{}, err
 	}
 	minor++
 	maxSupported := fmt.Sprintf("%s.%d", splitS[0], minor)

-	// Build the constraint string, make sure that we are less than but not equal to maxSupported since we added 1
+	cs.Reset()
 	cs.WriteString(fmt.Sprintf(">= %s, < %s", xdscommon.GetMinEnvoyMinorVersion(), maxSupported))
-	for _, s := range unsupportedList {
-		cs.WriteString(fmt.Sprintf(", != %s", s))
-	}
-
 	constraints, err := version.NewConstraint(cs.String())
 	if err != nil {
-		return false, err
+		return envoyCompat{}, err
 	}

-	return constraints.Check(v), nil
+	if c := constraints.Check(v); !c {
+		return envoyCompat{
+			isCompatible:        c,
+			versionIncompatible: replacePatchVersionWithX(envoyVersion),
+		}, nil
+	}
+
+	return envoyCompat{isCompatible: true}, nil
+}
+
+func replacePatchVersionWithX(version string) string {
+	// Strip off the patch and append x to convey that the constraint is on the minor version and not the patch
+	// itself
+	a := strings.Split(version, ".")
+	return fmt.Sprintf("%s.%s.x", a[0], a[1])
 }
--- a/command/connect/envoy/envoy_test.go
+++ b/command/connect/envoy/envoy_test.go
@ -1696,50 +1696,65 @@ func TestCheckEnvoyVersionCompatibility(t *testing.T) {
 		name            string
 		envoyVersion    string
 		unsupportedList []string
-		expectedSupport bool
+		expectedCompat  envoyCompat
 		isErrorExpected bool
 	}{
 		{
 			name:            "supported-using-proxy-support-defined",
 			envoyVersion:    xdscommon.EnvoyVersions[1],
 			unsupportedList: xdscommon.UnsupportedEnvoyVersions,
-			expectedSupport: true,
+			expectedCompat: envoyCompat{
+				isCompatible: true,
+			},
 		},
 		{
 			name:            "supported-at-max",
 			envoyVersion:    xdscommon.GetMaxEnvoyMinorVersion(),
 			unsupportedList: xdscommon.UnsupportedEnvoyVersions,
-			expectedSupport: true,
+			expectedCompat: envoyCompat{
+				isCompatible: true,
+			},
 		},
 		{
 			name:            "supported-patch-higher",
 			envoyVersion:    addNPatchVersion(xdscommon.EnvoyVersions[0], 1),
 			unsupportedList: xdscommon.UnsupportedEnvoyVersions,
-			expectedSupport: true,
+			expectedCompat: envoyCompat{
+				isCompatible: true,
+			},
 		},
 		{
 			name:            "not-supported-minor-higher",
 			envoyVersion:    addNMinorVersion(xdscommon.EnvoyVersions[0], 1),
 			unsupportedList: xdscommon.UnsupportedEnvoyVersions,
-			expectedSupport: false,
+			expectedCompat: envoyCompat{
+				isCompatible:        false,
+				versionIncompatible: replacePatchVersionWithX(addNMinorVersion(xdscommon.EnvoyVersions[0], 1)),
+			},
 		},
 		{
 			name:            "not-supported-minor-lower",
 			envoyVersion:    addNMinorVersion(xdscommon.EnvoyVersions[len(xdscommon.EnvoyVersions)-1], -1),
 			unsupportedList: xdscommon.UnsupportedEnvoyVersions,
-			expectedSupport: false,
+			expectedCompat: envoyCompat{
+				isCompatible:        false,
+				versionIncompatible: replacePatchVersionWithX(addNMinorVersion(xdscommon.EnvoyVersions[len(xdscommon.EnvoyVersions)-1], -1)),
+			},
 		},
 		{
 			name:            "not-supported-explicitly-unsupported-version",
 			envoyVersion:    addNPatchVersion(xdscommon.EnvoyVersions[0], 1),
 			unsupportedList: []string{"1.23.1", addNPatchVersion(xdscommon.EnvoyVersions[0], 1)},
-			expectedSupport: false,
+			expectedCompat: envoyCompat{
+				isCompatible:        false,
+				versionIncompatible: addNPatchVersion(xdscommon.EnvoyVersions[0], 1),
+			},
 		},
 		{
 			name:            "error-bad-input",
 			envoyVersion:    "1.abc.3",
 			unsupportedList: xdscommon.UnsupportedEnvoyVersions,
-			expectedSupport: false,
+			expectedCompat:  envoyCompat{},
 			isErrorExpected: true,
 		},
 	}
@ -1752,7 +1767,7 @@ func TestCheckEnvoyVersionCompatibility(t *testing.T) {
 			} else {
 				assert.NoError(t, err)
 			}
-			assert.Equal(t, tc.expectedSupport, actual)
+			assert.Equal(t, tc.expectedCompat, actual)
 		})
 	}
 }