Add check for version guards in erb templates (GoogleCloudPlatform#10297)

Author: roaks3

tools/template-check/go.mod

@@ -0,0 +1,3 @@
+module github.com/GoogleCloudPlatform/magic-modules/tools/template-check
+
+go 1.21

tools/template-check/main.go

@@ -0,0 +1,66 @@
+package main
+
+import (
+	"bufio"
+	"flag"
+	"fmt"
+	"os"
+
+	"github.com/GoogleCloudPlatform/magic-modules/tools/template-check/ruby"
+)
+
+func isValidTemplate(filename string) (bool, error) {
+	results, err := ruby.CheckVersionGuardsForFile(filename)
+	if err != nil {
+		return false, err
+	}
+
+	if len(results) > 0 {
+		fmt.Fprintf(os.Stderr, "error: invalid version checks found in %s:\n", filename)
+		for _, result := range results {
+			fmt.Fprintf(os.Stderr, "  %s\n", result)
+		}
+		return false, nil
+	}
+
+	return true, nil
+}
+
+func checkTemplate(filename string) bool {
+	valid, err := isValidTemplate(filename)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err.Error())
+		return false
+	}
+	return valid
+}
+
+func main() {
+	flag.Usage = func() {
+		fmt.Fprintf(flag.CommandLine.Output(), "template-check - check that a template file is valid\n    template-check [file]\n")
+	}
+
+	flag.Parse()
+
+	// Handle file as a positional argument
+	if flag.Arg(0) != "" {
+		if !checkTemplate(flag.Arg(0)) {
+			os.Exit(1)
+		}
+		os.Exit(0)
+	}
+
+	// Handle files coming from a linux pipe
+	fileInfo, _ := os.Stdin.Stat()
+	if fileInfo.Mode()&os.ModeCharDevice == 0 {
+		exitStatus := 0
+		scanner := bufio.NewScanner(bufio.NewReader(os.Stdin))
+		for scanner.Scan() {
+			if !checkTemplate(scanner.Text()) {
+				exitStatus = 1
+			}
+		}
+
+		os.Exit(exitStatus)
+	}
+}

tools/template-check/ruby/ruby.go

@@ -0,0 +1,66 @@
+package ruby
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"os"
+	"regexp"
+	"strings"
+)
+
+// Note: this is allowlisted to combat other issues like `=` instead of `==`, but it is possible we
+// need to add more options to this list in the future, like `private`. The main thing we want to
+// prevent is targeting `beta` in version guards, because it mishandles either `ga` or `private`.
+var allowedGuards = []string{
+	"<% unless version == 'ga' -%>",
+	"<% if version == 'ga' -%>",
+	"<% unless version == \"ga\" -%>",
+	"<% if version == \"ga\" -%>",
+}
+
+// Note: this does not account for _every_ possible use of a version guard (for example, those
+// starting with `version.nil?`), because the logic would start to get more complicated. Instead,
+// the goal is to capture (and validate) all "standard" version guards that would be added for new
+// resources/fields.
+func isVersionGuard(line string) bool {
+	re := regexp.MustCompile("<% [a-z]+ version ")
+	return re.MatchString(line)
+}
+
+func isValidVersionGuard(line string) bool {
+	for _, g := range allowedGuards {
+		if strings.Contains(line, g) {
+			return true
+		}
+	}
+	return false
+}
+
+// CheckVersionGuards scans the input for version guards, and checks that those version guards are
+// valid. Invalid version guards are returned along with the line number in which they occurred.
+func CheckVersionGuards(r io.Reader) []string {
+	scanner := bufio.NewScanner(r)
+	lineNum := 1
+	var invalidGuards []string
+	for scanner.Scan() {
+		if isVersionGuard(scanner.Text()) && !isValidVersionGuard(scanner.Text()) {
+			invalidGuards = append(invalidGuards, fmt.Sprintf("%d: %s", lineNum, scanner.Text()))
+		}
+		lineNum++
+	}
+	return invalidGuards
+}
+
+// CheckVersionGuardsForFile scans the file for version guards, and checks that those version
+// guards are valid. Invalid version guards are returned along with the line number in which they
+// occurred.
+func CheckVersionGuardsForFile(filename string) ([]string, error) {
+	file, err := os.Open(filename)
+	if err != nil {
+		return nil, err
+	}
+	defer file.Close()
+
+	return CheckVersionGuards(file), nil
+}

tools/template-check/ruby/ruby_test.go

@@ -0,0 +1,59 @@
+package ruby
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestCheckVersionGuards(t *testing.T) {
+	cases := map[string]struct {
+		fileText        string
+		expectedResults []string
+	}{
+		"allow standard format targeting ga": {
+			fileText:        "some text\n<% unless version == 'ga' -%>\nmore text",
+			expectedResults: nil,
+		},
+		"disallow targeting beta": {
+			fileText:        "some text\n<% unless version == 'beta' -%>\nmore text",
+			expectedResults: []string{"2: <% unless version == 'beta' -%>"},
+		},
+		"disallow 'if not'": {
+			fileText:        "some text\n<% if version != 'ga' -%>\nmore text",
+			expectedResults: []string{"2: <% if version != 'ga' -%>"},
+		},
+		"disallow single '='": {
+			fileText:        "some text\n<% unless version = 'ga' -%>\nmore text",
+			expectedResults: []string{"2: <% unless version = 'ga' -%>"},
+		},
+		"disallow leaving trailing line break": {
+			fileText:        "some text\n<% unless version == 'ga' %>\nmore text",
+			expectedResults: []string{"2: <% unless version == 'ga' %>"},
+		},
+		"one valid, one invalid": {
+			fileText:        "some text\n<% unless version == 'beta' -%>\nmore text\n<% unless version == 'ga' -%>",
+			expectedResults: []string{"2: <% unless version == 'beta' -%>"},
+		},
+		"multiple invalid": {
+			fileText:        "some text\n<% unless version == 'beta' -%>\nmore text\n\n\n<% if version == \"beta\" -%>",
+			expectedResults: []string{"2: <% unless version == 'beta' -%>", "6: <% if version == \"beta\" -%>"},
+		},
+	}
+
+	for tn, tc := range cases {
+		tc := tc
+		t.Run(tn, func(t *testing.T) {
+			t.Parallel()
+			results := CheckVersionGuards(strings.NewReader(tc.fileText))
+			if len(results) != len(tc.expectedResults) {
+				t.Errorf("Expected length %d, got %d", len(tc.expectedResults), len(results))
+				return
+			}
+			for i, result := range results {
+				if result != tc.expectedResults[i] {
+					t.Errorf("Expected %v, got %v", tc.expectedResults[i], result)
+				}
+			}
+		})
+	}
+}