CBG-3445 prevent N-1 downgrades by stamping db version into gateway r…

…egistry (#6486) * CBG-3445 stamp db version into gateway registry - update gateway registry on startup to include new version numbers - ensure that a database is not loaded if is more than a minor version upgrade Improvements to ComparableVersion - make copyable so that you can't pass a nil version by mistake - change String() method to be a non pointer method to avoid mishaps if you don't print a pointer.
couchbase · Oct 5, 2023 · 7908d3c · 7908d3c
1 parent ce51e97
commit 7908d3c
Show file tree

Hide file tree

Showing 10 changed files with 346 additions and 37 deletions.
diff --git a/base/dcp_sharded.go b/base/dcp_sharded.go
@@ -404,7 +404,7 @@ func getMinNodeVersion(cfg cbgt.Cfg) (*ComparableVersion, error) {
 			return nil, fmt.Errorf("failed to get version of node %v: %w", MD(node.HostPort).Redact(), err)
 		}
 		if nodeVersion == nil {
-			nodeVersion = zeroComparableVersion
+			nodeVersion = zeroComparableVersion()
 		}
 		if minVersion == nil || nodeVersion.Less(minVersion) {
 			minVersion = nodeVersion

diff --git a/base/version_comparable.go b/base/version_comparable.go
@@ -9,11 +9,9 @@
 package base
 
 import (
-	"errors"
 	"fmt"
 	"strconv"
 	"strings"
-	"sync"
 )
 
 const (
@@ -28,18 +26,21 @@ type ComparableVersion struct {
 	epoch, major, minor, patch, other uint8
 	build                             uint16
 	edition                           productEdition
-	strOnce                           sync.Once
 	str                               string
 }
 
-var zeroComparableVersion = &ComparableVersion{
-	epoch:   0,
-	major:   0,
-	minor:   0,
-	patch:   0,
-	other:   0,
-	build:   0,
-	edition: "",
+func zeroComparableVersion() *ComparableVersion {
+	v := &ComparableVersion{
+		epoch:   0,
+		major:   0,
+		minor:   0,
+		patch:   0,
+		other:   0,
+		build:   0,
+		edition: "",
+	}
+	v.str = v.formatComparableVersion()
+	return v
 }
 
 // NewComparableVersionFromString parses a ComparableVersion from the given version string.
@@ -49,31 +50,38 @@ func NewComparableVersionFromString(version string) (*ComparableVersion, error)
 	if err != nil {
 		return nil, err
 	}
-	return &ComparableVersion{
+	v := &ComparableVersion{
 		epoch:   epoch,
 		major:   major,
 		minor:   minor,
 		patch:   patch,
 		other:   other,
 		build:   build,
 		edition: edition,
-	}, nil
+	}
+	v.str = v.formatComparableVersion()
+	if v.str != version {
+		return nil, fmt.Errorf("version string %q is not equal to formatted version string %q", version, v.str)
+	}
+	return v, nil
 }
 
 func NewComparableVersion(majorStr, minorStr, patchStr, otherStr, buildStr, editionStr string) (*ComparableVersion, error) {
 	_, major, minor, patch, other, build, edition, err := parseComparableVersionComponents("", majorStr, minorStr, patchStr, otherStr, buildStr, editionStr)
 	if err != nil {
 		return nil, err
 	}
-	return &ComparableVersion{
+	v := &ComparableVersion{
 		epoch:   comparableVersionEpoch,
 		major:   major,
 		minor:   minor,
 		patch:   patch,
 		other:   other,
 		build:   build,
 		edition: edition,
-	}, nil
+	}
+	v.str = v.formatComparableVersion()
+	return v, nil
 }
 
 // Equal returns true if pv is equal to b
@@ -129,10 +137,18 @@ func (a *ComparableVersion) Less(b *ComparableVersion) bool {
 	return false
 }
 
-func (pv *ComparableVersion) String() string {
-	pv.strOnce.Do(func() {
-		pv.str = pv.formatComparableVersion()
-	})
+// AtLeastMinorDowngrade returns true there is a major or minor downgrade from a to b.
+func (a *ComparableVersion) AtLeastMinorDowngrade(b *ComparableVersion) bool {
+	if a.epoch != b.epoch {
+		return a.epoch > b.epoch
+	}
+	if a.major != b.major {
+		return a.major > b.major
+	}
+	return a.minor > b.minor
+}
+
+func (pv ComparableVersion) String() string {
 	return pv.str
 }
 
@@ -147,7 +163,11 @@ func (pv *ComparableVersion) UnmarshalJSON(val []byte) error {
 	if err != nil {
 		return err
 	}
-	pv.epoch, pv.major, pv.minor, pv.patch, pv.other, pv.build, pv.edition, err = parseComparableVersion(strVal)
+	if strVal != "" {
+		pv.epoch, pv.major, pv.minor, pv.patch, pv.other, pv.build, pv.edition, err = parseComparableVersion(strVal)
+	}
+
+	pv.str = pv.formatComparableVersion()
 	return err
 }
 
@@ -290,7 +310,7 @@ func extractComparableVersionComponents(version string) (epoch, major, minor, pa
 	}
 
 	if major == "" || minor == "" || patch == "" {
-		return "", "", "", "", "", "", "", errors.New("version requires at least major.minor.patch components")
+		return "", "", "", "", "", "", "", fmt.Errorf("version %q requires at least major.minor.patch components", version)
 	}
 
 	return epoch, major, minor, patch, other, build, edition, nil

diff --git a/base/version_comparable_test.go b/base/version_comparable_test.go
@@ -1,5 +1,4 @@
 // Copyright 2022-Present Couchbase, Inc.
-//
 // Use of this software is governed by the Business Source License included
 // in the file licenses/BSL-Couchbase.txt.  As of the Change Date specified
 // in that file, in accordance with the Business Source License, use of this
@@ -9,6 +8,7 @@
 package base
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -115,6 +115,125 @@ func TestInvalidComparableVersion(t *testing.T) {
 	}
 }
 
+func TestComparableVersionJSONRoundTrip(t *testing.T) {
+	json, err := JSONMarshal(ProductVersion)
+	require.NoError(t, err)
+	var version ComparableVersion
+	err = JSONUnmarshal(json, &version)
+	require.NoError(t, err)
+	require.True(t, ProductVersion.Equal(&version))
+	require.Equal(t, ProductVersion.String(), version.String())
+}
+
+func TestComparableVersionEmptyStringJSON(t *testing.T) {
+	var version ComparableVersion
+	err := JSONUnmarshal([]byte(`""`), &version)
+	require.NoError(t, err)
+	require.True(t, zeroComparableVersion().Equal(&version))
+	require.Equal(t, "0.0.0", zeroComparableVersion().String())
+	require.Equal(t, "0.0.0", version.String())
+}
+
+func TestAtLeastMinorDowngradeVersion(t *testing.T) {
+	testCases := []struct {
+		versionA       string
+		versionB       string
+		minorDowngrade bool
+	}{
+		{
+			versionA:       "1.0.0",
+			versionB:       "1.0.0",
+			minorDowngrade: false,
+		},
+		{
+			versionA:       "1.0.0",
+			versionB:       "2.0.0",
+			minorDowngrade: false,
+		},
+		{
+			versionA:       "2.0.0",
+			versionB:       "1.0.0",
+			minorDowngrade: true,
+		},
+		{
+			versionA:       "1.0.0",
+			versionB:       "1.0.1",
+			minorDowngrade: false,
+		},
+		{
+			versionA:       "1.0.1",
+			versionB:       "1.0.0",
+			minorDowngrade: false,
+		},
+		{
+			versionA:       "1.1.0",
+			versionB:       "1.0.0",
+			minorDowngrade: true,
+		},
+		{
+			versionA:       "1.0.0",
+			versionB:       "1.1.0",
+			minorDowngrade: false,
+		},
+		{
+			versionA:       "1.0.0",
+			versionB:       "1.0.0.1",
+			minorDowngrade: false,
+		},
+		{
+			versionA:       "1.0.0.1",
+			versionB:       "1.0.0",
+			minorDowngrade: false,
+		},
+		{
+			versionA:       "1.0.0.1",
+			versionB:       "1.0.0.2",
+			minorDowngrade: false,
+		},
+		{
+			versionA:       "1.0.0.2",
+			versionB:       "1.0.0.1",
+			minorDowngrade: false,
+		},
+		{
+			versionA:       "1.0.0-EE",
+			versionB:       "1.1.0-CE",
+			minorDowngrade: false,
+		},
+		{
+			versionA:       "2.2.0",
+			versionB:       "1.1.0",
+			minorDowngrade: true,
+		},
+		{
+			versionA:       "1.1.0",
+			versionB:       "2.2.0",
+			minorDowngrade: false,
+		},
+		{
+			versionA:       "2.1.0",
+			versionB:       "1.2.0",
+			minorDowngrade: true,
+		},
+		{
+			versionA:       "1.2.0",
+			versionB:       "2.1.0",
+			minorDowngrade: false,
+		},
+	}
+
+	for _, test := range testCases {
+		t.Run(fmt.Sprintf("%s->%s", test.versionA, test.versionB), func(t *testing.T) {
+			versionA, err := NewComparableVersionFromString(test.versionA)
+			require.NoError(t, err)
+
+			versionB, err := NewComparableVersionFromString(test.versionB)
+			require.NoError(t, err)
+			require.Equal(t, test.minorDowngrade, versionA.AtLeastMinorDowngrade(versionB))
+		})
+	}
+}
+
 func BenchmarkComparableVersion(b *testing.B) {
 	const str = "8:7.6.5.4@3-EE"
 

diff --git a/rest/config.go b/rest/config.go
@@ -1764,6 +1764,13 @@ func (sc *ServerContext) applyConfigs(ctx context.Context, dbNameConfigs map[str
 
 // _applyConfig loads the given database, failFast=true will not attempt to retry connecting/loading
 func (sc *ServerContext) _applyConfig(nonContextStruct base.NonCancellableContext, cnf DatabaseConfig, failFast, isInitialStartup bool) (applied bool, err error) {
+	ctx := nonContextStruct.Ctx
+
+	nodeSGVersion := sc.BootstrapContext.sgVersion
+	err = sc.BootstrapContext.CheckMinorDowngrade(ctx, *cnf.Bucket, nodeSGVersion)
+	if err != nil {
+		return false, err
+	}
 	// 3.0.0 doesn't write a SGVersion, but everything else will
 	configSGVersionStr := "3.0.0"
 	if cnf.SGVersion != "" {
@@ -1777,9 +1784,8 @@ func (sc *ServerContext) _applyConfig(nonContextStruct base.NonCancellableContex
 
 	if !isInitialStartup {
 		// Skip applying if the config is from a newer SG version than this node and we're not just starting up
-		nodeSGVersion := base.ProductVersion
 		if nodeSGVersion.Less(configSGVersion) {
-			base.WarnfCtx(nonContextStruct.Ctx, "Cannot apply config update from server for db %q, this SG version is older than config's SG version (%s < %s)", cnf.Name, nodeSGVersion.String(), configSGVersion.String())
+			base.WarnfCtx(ctx, "Cannot apply config update from server for db %q, this SG version is older than config's SG version (%s < %s)", cnf.Name, nodeSGVersion.String(), configSGVersion.String())
 			return false, nil
 		}
 	}
@@ -1795,27 +1801,32 @@ func (sc *ServerContext) _applyConfig(nonContextStruct base.NonCancellableContex
 	if exists {
 		if cnf.cfgCas == 0 {
 			// force an update when the new config's cas was set to zero prior to load
-			base.InfofCtx(nonContextStruct.Ctx, base.KeyConfig, "Forcing update of config for database %q bucket %q", cnf.Name, *cnf.Bucket)
+			base.InfofCtx(ctx, base.KeyConfig, "Forcing update of config for database %q bucket %q", cnf.Name, *cnf.Bucket)
 		} else {
 			if sc.dbConfigs[cnf.Name].cfgCas >= cnf.cfgCas {
-				base.DebugfCtx(nonContextStruct.Ctx, base.KeyConfig, "Database %q bucket %q config has not changed since last update", cnf.Name, *cnf.Bucket)
+				base.DebugfCtx(ctx, base.KeyConfig, "Database %q bucket %q config has not changed since last update", cnf.Name, *cnf.Bucket)
 				return false, nil
 			}
-			base.InfofCtx(nonContextStruct.Ctx, base.KeyConfig, "Updating database %q for bucket %q with new config from bucket", cnf.Name, *cnf.Bucket)
+			base.InfofCtx(ctx, base.KeyConfig, "Updating database %q for bucket %q with new config from bucket", cnf.Name, *cnf.Bucket)
 		}
 	}
 
 	// Strip out version as we have no use for this locally and we want to prevent it being stored and being returned
 	// by any output
 	cnf.Version = ""
 
+	err = sc.BootstrapContext.SetSGVersion(ctx, *cnf.Bucket, nodeSGVersion)
+	if err != nil {
+		return false, nil
+	}
+
 	// Prevent database from being unsuspended when it is suspended
 	if sc._isDatabaseSuspended(cnf.Name) {
 		return true, nil
 	}
 
 	// TODO: Dynamic update instead of reload
-	if err := sc._reloadDatabaseWithConfig(nonContextStruct.Ctx, cnf, failFast, false); err != nil {
+	if err := sc._reloadDatabaseWithConfig(ctx, cnf, failFast, false); err != nil {
 		// remove these entries we just created above if the database hasn't loaded properly
 		return false, fmt.Errorf("couldn't reload database: %w", err)
 	}