Fix regular expression issue in Golang Upgrade and remove release-14.0 from target branch

.github/workflows/update_golang_version.yml

@@ -15,7 +15,7 @@ jobs:
       pull-requests: write
     strategy:
       matrix:
-        branch: [ main, release-17.0, release-16.0, release-15.0, release-14.0 ]
+        branch: [ main, release-17.0, release-16.0, release-15.0 ]
     name: Update Golang Version
     runs-on: ubuntu-latest
     steps:

go/tools/go-upgrade/go-upgrade.go

@@ -17,6 +17,7 @@ limitations under the License.
 package main
 
 import (
+	"bufio"
 	"fmt"
 	"io"
 	"log"
@@ -36,6 +37,36 @@ import (
 
 const (
 	goDevAPI = "https://go.dev/dl/?mode=json"
+
+	// regexpFindBootstrapVersion greps the current bootstrap version from the Makefile. The bootstrap
+	// version is composed of either one or two numbers, for instance: 18.1 or 18.
+	// The expected format of the input is BOOTSTRAP_VERSION=18 or BOOTSTRAP_VERSION=18.1
+	regexpFindBootstrapVersion = "(?i).*BOOTSTRAP_VERSION[[:space:]]*=[[:space:]]*([0-9.]+).*"
+
+	// regexpFindGolangVersion greps all numbers separated by a . after the goversion_min function call
+	// This is used to understand what is the current version of Golang using either two or three numbers
+	// The major, minor and optional patch number of the Golang version
+	regexpFindGolangVersion = "(?i).*goversion_min[[:space:]]*([0-9.]+).*"
+
+	// regexpReplaceGoModGoVersion replaces the top-level golang version instruction in the go.mod file
+	// Example going from go1.20 to go1.20: `go 1.20` -> `go 1.21`
+	regexpReplaceGoModGoVersion = `go[[:space:]]([0-9.]+)\.([0-9.]+)`
+
+	// The regular expressions below match the entire bootstrap_version declaration in Dockerfiles and Makefile
+	// A bootstrap version declaration is usually: 'ARG bootstrap_version = 18' in Dockerfile, and
+	// 'BOOTSTRAP_VERSION=18' in the Makefile. Note that the value 18 can also be a float.
+	regexpReplaceDockerfileBootstrapVersion = "ARG[[:space:]]*bootstrap_version[[:space:]]*=[[:space:]]*[0-9.]+"
+	regexpReplaceMakefileBootstrapVersion   = "BOOTSTRAP_VERSION[[:space:]]*=[[:space:]]*[0-9.]+"
+
+	// The regular expression below matches the bootstrap_version we are using in the test.go file.
+	// In test.go, there is a flag named 'bootstrap-version' that has a default value. We are looking
+	// to match the entire flag name + the default value (being the current bootstrap version)
+	// Example input: "flag.String("bootstrap-version", "20", "the version identifier to use for the docker images")"
+	regexpReplaceTestGoBootstrapVersion = `\"bootstrap-version\",[[:space:]]*\"([0-9.]+)\"`
+
+	// regexpReplaceGolangVersionInWorkflow matches the golang version increment in the string `go-version: 1.20.5`
+	// which is used to replace the golang version we use inside our workflows
+	regexpReplaceGolangVersionInWorkflow = `go-version:[[:space:]]*([0-9.]+).*`
 )
 
 type (
@@ -186,7 +217,7 @@ func updateWorkflowFilesOnly(goTo string) error {
 
 	for _, fileToChange := range filesToChange {
 		err = replaceInFile(
-			[]*regexp.Regexp{regexp.MustCompile(`go-version:[[:space:]]*([0-9.]+).*`)},
+			[]*regexp.Regexp{regexp.MustCompile(regexpReplaceGolangVersionInWorkflow)},
 			[]string{"go-version: " + newV.String()},
 			fileToChange,
 		)
@@ -249,7 +280,7 @@ func currentGolangVersion() (*version.Version, error) {
 	}
 	content := string(contentRaw)
 
-	versre := regexp.MustCompile("(?i).*goversion_min[[:space:]]*([0-9.]+).*")
+	versre := regexp.MustCompile(regexpFindGolangVersion)
 	versionStr := versre.FindStringSubmatch(content)
 	if len(versionStr) != 2 {
 		return nil, fmt.Errorf("malformatted error, got: %v", versionStr)
@@ -264,7 +295,7 @@ func currentBootstrapVersion() (bootstrapVersion, error) {
 	}
 	content := string(contentRaw)
 
-	versre := regexp.MustCompile("(?i).*BOOTSTRAP_VERSION[[:space:]]*=[[:space:]]*([0-9.]+).*")
+	versre := regexp.MustCompile(regexpFindBootstrapVersion)
 	versionStr := versre.FindStringSubmatch(content)
 	if len(versionStr) != 2 {
 		return bootstrapVersion{}, fmt.Errorf("malformatted error, got: %v", versionStr)
@@ -372,6 +403,7 @@ func replaceGoVersionInCodebase(old, new *version.Version, workflowUpdate bool)
 	}
 
 	for _, fileToChange := range filesToChange {
+		// The regular expression below simply replace the old version string by the new golang version
 		err = replaceInFile(
 			[]*regexp.Regexp{regexp.MustCompile(fmt.Sprintf(`(%s)`, old.String()))},
 			[]string{new.String()},
@@ -384,7 +416,7 @@ func replaceGoVersionInCodebase(old, new *version.Version, workflowUpdate bool)
 
 	if !isSameMajorMinorVersion(old, new) {
 		err = replaceInFile(
-			[]*regexp.Regexp{regexp.MustCompile(`go[[:space:]]*([0-9.]+)`)},
+			[]*regexp.Regexp{regexp.MustCompile(regexpReplaceGoModGoVersion)},
 			[]string{fmt.Sprintf("go %d.%d", new.Segments()[0], new.Segments()[1])},
 			"./go.mod",
 		)
@@ -414,8 +446,8 @@ func updateBootstrapVersionInCodebase(old, new string, newGoVersion *version.Ver
 	for _, file := range files {
 		err = replaceInFile(
 			[]*regexp.Regexp{
-				regexp.MustCompile(`ARG[[:space:]]*bootstrap_version[[:space:]]*=[[:space:]]*[0-9.]+`), // Dockerfile
-				regexp.MustCompile(`BOOTSTRAP_VERSION[[:space:]]*=[[:space:]]*[0-9.]+`),                // Makefile
+				regexp.MustCompile(regexpReplaceDockerfileBootstrapVersion), // Dockerfile
+				regexp.MustCompile(regexpReplaceMakefileBootstrapVersion),   // Makefile
 			},
 			[]string{
 				fmt.Sprintf("ARG bootstrap_version=%s", new), // Dockerfile
@@ -429,7 +461,7 @@ func updateBootstrapVersionInCodebase(old, new string, newGoVersion *version.Ver
 	}
 
 	err = replaceInFile(
-		[]*regexp.Regexp{regexp.MustCompile(`\"bootstrap-version\",[[:space:]]*\"([0-9.]+)\"`)},
+		[]*regexp.Regexp{regexp.MustCompile(regexpReplaceTestGoBootstrapVersion)},
 		[]string{fmt.Sprintf("\"bootstrap-version\", \"%s\"", new)},
 		"./test.go",
 	)
@@ -510,17 +542,23 @@ func replaceInFile(oldexps []*regexp.Regexp, new []string, fileToChange string)
 	}
 	defer f.Close()
 
-	content, err := io.ReadAll(f)
-	if err != nil {
-		return err
-	}
-	contentStr := string(content)
-
-	for i, oldex := range oldexps {
-		contentStr = oldex.ReplaceAllString(contentStr, new[i])
+	var res []string
+	reader := bufio.NewReader(f)
+	for {
+		line, err := reader.ReadString('\n')
+		if err != nil {
+			if err == io.EOF {
+				break
+			}
+			panic(err)
+		}
+		for i, oldexp := range oldexps {
+			line = oldexp.ReplaceAllString(line, new[i])
+		}
+		res = append(res, line)
 	}
 
-	_, err = f.WriteAt([]byte(contentStr), 0)
+	_, err = f.WriteAt([]byte(strings.Join(res, "")), 0)
 	if err != nil {
 		return err
 	}

go/tools/go-upgrade/go-upgrade_test.go

@@ -0,0 +1,105 @@
+/*
+Copyright 2023 The Vitess Authors.
+
+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 main
+
+import (
+	"regexp"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestRegularExpressions(t *testing.T) {
+	lists := []struct {
+		regexp string
+		input  string
+		checkF func(t *testing.T, regexp *regexp.Regexp, input string)
+	}{
+		{
+			regexp: regexpFindBootstrapVersion,
+			input:  "BOOTSTRAP_VERSION=18.1",
+			checkF: func(t *testing.T, regexp *regexp.Regexp, input string) {
+				submatch := regexp.FindStringSubmatch(input)
+				require.Len(t, submatch, 2, "Should have two submatches in the regular expression")
+				require.Equal(t, "18.1", submatch[1])
+			},
+		},
+		{
+			regexp: regexpFindGolangVersion,
+			input:  `goversion_min 1.20.5 || echo "Go version reported`,
+			checkF: func(t *testing.T, regexp *regexp.Regexp, input string) {
+				submatch := regexp.FindStringSubmatch(input)
+				require.Len(t, submatch, 2, "Should have two submatches in the regular expression")
+				require.Equal(t, "1.20.5", submatch[1])
+			},
+		},
+		{
+			regexp: regexpReplaceGoModGoVersion,
+			input:  "go 1.20",
+			checkF: func(t *testing.T, regexp *regexp.Regexp, input string) {
+				res := regexp.ReplaceAllString(input, "go 1.21")
+				require.Equal(t, "go 1.21", res)
+			},
+		},
+		{
+			regexp: regexpReplaceGoModGoVersion,
+			input:  "go 1 20",
+			checkF: func(t *testing.T, regexp *regexp.Regexp, input string) {
+				res := regexp.ReplaceAllString(input, "go 1.21")
+				require.Equal(t, "go 1 20", res)
+			},
+		},
+		{
+			regexp: regexpReplaceDockerfileBootstrapVersion,
+			input:  "ARG bootstrap_version=18.1",
+			checkF: func(t *testing.T, regexp *regexp.Regexp, input string) {
+				res := regexp.ReplaceAllString(input, "ARG bootstrap_version=18.2")
+				require.Equal(t, "ARG bootstrap_version=18.2", res)
+			},
+		},
+		{
+			regexp: regexpReplaceMakefileBootstrapVersion,
+			input:  "BOOTSTRAP_VERSION=18.1",
+			checkF: func(t *testing.T, regexp *regexp.Regexp, input string) {
+				res := regexp.ReplaceAllString(input, "BOOTSTRAP_VERSION=18.2")
+				require.Equal(t, "BOOTSTRAP_VERSION=18.2", res)
+			},
+		},
+		{
+			regexp: regexpReplaceTestGoBootstrapVersion,
+			input:  `flag.String("bootstrap-version", "18.1", "the version identifier to use for the docker images")`,
+			checkF: func(t *testing.T, regexp *regexp.Regexp, input string) {
+				res := regexp.ReplaceAllString(input, "\"bootstrap-version\", \"18.2\"")
+				require.Equal(t, `flag.String("bootstrap-version", "18.2", "the version identifier to use for the docker images")`, res)
+			},
+		},
+		{
+			regexp: regexpReplaceGolangVersionInWorkflow,
+			input:  "go-version: 1.20.5",
+			checkF: func(t *testing.T, regexp *regexp.Regexp, input string) {
+				res := regexp.ReplaceAllString(input, "go-version: 1.20.6")
+				require.Equal(t, `go-version: 1.20.6`, res)
+			},
+		},
+	}
+
+	for _, list := range lists {
+		t.Run(list.regexp+" "+list.input, func(t *testing.T) {
+			list.checkF(t, regexp.MustCompile(list.regexp), list.input)
+		})
+	}
+}