upgrade: Fix insiders version string parsing (#48623)

cmd/frontend/graphqlbackend/site.go

@@ -253,54 +253,66 @@ type upgradeReadinessResolver struct {
 	initOnce sync.Once
 	initErr  error
 	runner   cliutil.Runner
-	version  oobmigration.Version
-	patch    int
+	version  string
 }
 
+var devSchemaFactory = cliutil.NewExpectedSchemaFactory(
+	"Local file",
+	[]cliutil.NamedRegexp{{Regexp: lazyregexp.New(`^dev$`)}},
+	func(filename, _ string) string { return filename },
+	cliutil.ReadSchemaFromFile,
+)
+
 var schemaFactories = append(
 	migratorshared.DefaultSchemaFactories,
 	// Special schema factory for dev environment.
-	cliutil.NewExpectedSchemaFactory(
-		"Local file",
-		[]cliutil.NamedRegexp{{Regexp: lazyregexp.New(`^dev$`)}},
-		func(filename, _ string) string { return filename },
-		cliutil.ReadSchemaFromFile,
-	),
+	devSchemaFactory,
 )
 
-func (r *upgradeReadinessResolver) init(ctx context.Context) (_ cliutil.Runner, _ oobmigration.Version, patch int, _ error) {
+var insidersVersionPattern = lazyregexp.New(`^[\w-]+_\d{4}-\d{2}-\d{2}_\d+\.\d+-(\w+)$`)
+
+func (r *upgradeReadinessResolver) init(ctx context.Context) (_ cliutil.Runner, version string, _ error) {
 	r.initOnce.Do(func() {
-		r.runner, r.version, r.patch, r.initErr = func() (_ cliutil.Runner, _ oobmigration.Version, patch int, _ error) {
+		r.runner, r.version, r.initErr = func() (cliutil.Runner, string, error) {
 			observationCtx := observation.NewContext(r.logger)
 			runner, err := migratorshared.NewRunnerWithSchemas(observationCtx, r.logger, schemas.SchemaNames, schemas.Schemas)
 			if err != nil {
-				return nil, oobmigration.Version{}, 0, errors.Wrap(err, "new runner")
+				return nil, "", errors.Wrap(err, "new runner")
 			}
 
-			version, patch, ok, err := cliutil.GetServiceVersion(ctx, runner)
+			versionStr, ok, err := cliutil.GetRawServiceVersion(ctx, runner)
 			if err != nil {
-				return nil, oobmigration.Version{}, 0, errors.Wrap(err, "get service version")
+				return nil, "", errors.Wrap(err, "get service version")
 			} else if !ok {
-				return nil, oobmigration.Version{}, 0, errors.New("invalid service version")
+				return nil, "", errors.New("invalid service version")
 			}
-			return runner, version, patch, nil
+
+			// Return abbreviated commit hash from insiders version
+			if matches := insidersVersionPattern.FindStringSubmatch(versionStr); len(matches) > 0 {
+				return runner, matches[1], nil
+			}
+
+			v, patch, ok := oobmigration.NewVersionAndPatchFromString(versionStr)
+			if !ok {
+				return nil, "", errors.Newf("cannot parse version: %q - expected [v]X.Y[.Z]", versionStr)
+			}
+
+			if v.Dev {
+				return runner, "dev", nil
+			}
+
+			return runner, v.GitTagWithPatch(patch), nil
 		}()
 	})
-	return r.runner, r.version, r.patch, r.initErr
+
+	return r.runner, r.version, r.initErr
 }
 
 func (r *upgradeReadinessResolver) SchemaDrift(ctx context.Context) (string, error) {
-	runner, v, patch, err := r.init(ctx)
+	runner, version, err := r.init(ctx)
 	if err != nil {
 		return "", err
 	}
-
-	var version string
-	if v.Dev {
-		version = "dev"
-	} else {
-		version = v.GitTagWithPatch(patch)
-	}
 	r.logger.Debug("schema drift", log.String("version", version))
 
 	var drift bytes.Buffer
@@ -325,9 +337,12 @@ func isRequiredOutOfBandMigration(version oobmigration.Version, m oobmigration.M
 
 func (r *upgradeReadinessResolver) RequiredOutOfBandMigrations(ctx context.Context) ([]*outOfBandMigrationResolver, error) {
 	updateStatus := updatecheck.Last()
-	if updateStatus == nil || !updateStatus.HasUpdate() {
+	if updateStatus == nil {
 		return nil, errors.New("no latest update version available (reload in a few seconds)")
 	}
+	if !updateStatus.HasUpdate() {
+		return nil, nil
+	}
 	version, _, ok := oobmigration.NewVersionAndPatchFromString(updateStatus.UpdateVersion)
 	if !ok {
 		return nil, errors.Errorf("invalid latest update version %q", updateStatus.UpdateVersion)

internal/database/migration/cliutil/drift_schema.go

@@ -35,10 +35,11 @@ func (r NamedRegexp) Example() string {
 }
 
 var (
-	versionBranchPattern = NamedRegexp{lazyregexp.New(`^\d+\.\d+$`), `4.1 (version branch)`}
-	tagPattern           = NamedRegexp{lazyregexp.New(`^v\d+\.\d+\.\d+$`), `v4.1.1 (tagged release)`}
-	commitPattern        = NamedRegexp{lazyregexp.New(`^[0-9A-Fa-f]{40}$`), `57b1f56787619464dc62f469127d64721b428b76 (40-character sha)`}
-	allPatterns          = []NamedRegexp{versionBranchPattern, tagPattern, commitPattern}
+	versionBranchPattern     = NamedRegexp{lazyregexp.New(`^\d+\.\d+$`), `4.1 (version branch)`}
+	tagPattern               = NamedRegexp{lazyregexp.New(`^v\d+\.\d+\.\d+$`), `v4.1.1 (tagged release)`}
+	commitPattern            = NamedRegexp{lazyregexp.New(`^[0-9A-Fa-f]{40}$`), `57b1f56787619464dc62f469127d64721b428b76 (40-character sha)`}
+	abbreviatedCommitPattern = NamedRegexp{lazyregexp.New(`^[0-9A-Fa-f]{12}$`), `57b1f5678761 (12-character sha)`}
+	allPatterns              = []NamedRegexp{versionBranchPattern, tagPattern, commitPattern, abbreviatedCommitPattern}
 )
 
 // GitHubExpectedSchemaFactory reads schema definitions from the sourcegraph/sourcegraph repository via GitHub's API.

internal/database/migration/cliutil/multiversion.go

@@ -270,21 +270,25 @@ func filterStitchedMigrationsForTags(tags []string) (map[string]shared.StitchedM
 	return filteredStitchedMigrationBySchemaName, nil
 }
 
-// GetServiceVersion returns the frontend service version information for the
-// given runner. Both of the return values `ok` and `error` should be checked to
-// ensure a valid version is returned.
-func GetServiceVersion(ctx context.Context, r Runner) (_ oobmigration.Version, patch int, ok bool, _ error) {
+// GetRawServiceVersion returns the frontend service version information for the given runner as a raw string.
+func GetRawServiceVersion(ctx context.Context, r Runner) (_ string, ok bool, _ error) {
 	db, err := extractDatabase(ctx, r)
 	if err != nil {
-		return oobmigration.Version{}, 0, false, err
+		return "", false, err
 	}
 
-	versionStr, ok, err := upgradestore.New(db).GetServiceVersion(ctx, "frontend")
+	return upgradestore.New(db).GetServiceVersion(ctx, "frontend")
+}
+
+// GetServiceVersion returns the frontend service version information for the given runner as a parsed version.
+// Both of the return values `ok` and `error` should be checked to ensure a valid version is returned.
+func GetServiceVersion(ctx context.Context, r Runner) (_ oobmigration.Version, patch int, ok bool, _ error) {
+	versionStr, ok, err := GetRawServiceVersion(ctx, r)
 	if err != nil {
 		return oobmigration.Version{}, 0, false, err
 	}
 	if !ok {
-		return oobmigration.Version{}, 0, false, nil
+		return oobmigration.Version{}, 0, false, err
 	}
 
 	version, patch, ok := oobmigration.NewVersionAndPatchFromString(versionStr)

internal/oobmigration/version.go

@@ -33,10 +33,7 @@ func newDevVersion(major, minor int) Version {
 	}
 }
 
-var (
-	versionPattern         = lazyregexp.New(`^v?(\d+)\.(\d+)(?:\.(\d+))?$`)
-	insidersVersionPattern = lazyregexp.New(`^[\w-]+_\d{4}-\d{2}-\d{2}_(\d+)\.(\d+)-\w+$`)
-)
+var versionPattern = lazyregexp.New(`^v?(\d+)\.(\d+)(?:\.(\d+))?$`)
 
 // NewVersionFromString parses the major and minor version from the given string. If
 // the string does not look like a parseable version, a false-valued flag is returned.
@@ -58,12 +55,7 @@ func NewVersionAndPatchFromString(v string) (Version, int, bool) {
 
 	matches := versionPattern.FindStringSubmatch(v)
 	if len(matches) < 3 {
-		matches = insidersVersionPattern.FindStringSubmatch(v)
-		if len(matches) < 3 {
-			return Version{}, 0, false
-		}
-
-		newVersion = newDevVersion
+		return Version{}, 0, false
 	}
 
 	major, _ := strconv.Atoi(matches[1])

internal/oobmigration/version_test.go

@@ -18,7 +18,6 @@ func TestNewVersionFromString(t *testing.T) {
 		{"v3.50", NewVersion(3, 50), 0, true},
 		{"3.50.3", NewVersion(3, 50), 3, true},
 		{"3.50.3+dev", newDevVersion(3, 50), 3, true},
-		{"202659_2023-03-01_4.5-dc2f6ca215c3", newDevVersion(4, 5), 0, true},
 		{"350", Version{}, 0, false},
 		{"350+dev", Version{}, 0, false},
 	}