From a854242eeddd1d0aa08895352d50015ef70c24fa Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Wed, 27 Nov 2024 23:19:49 +0000 Subject: [PATCH] Update TeamCity diff checker tool (#12426) --- .../teamcity-services-diff-check-weekly.yml | 16 +- .../teamcity-services-diff-check.yml | 14 +- tools/teamcity-diff-check/go.mod | 3 + tools/teamcity-diff-check/main.go | 99 +++++++---- tools/teamcity-diff-check/main_test.go | 159 ++++++++++++++++++ .../test-fixtures/empty-files/ga-services.txt | 0 .../test-fixtures/empty-files/services_ga.kt | 0 .../everything-ok/ga-services.txt | 2 + .../everything-ok/services_ga.kt | 12 ++ .../mismatch-provider/ga-services.txt | 1 + .../mismatch-provider/services_ga.kt | 12 ++ .../mismatch-teamcity/ga-services.txt | 2 + .../mismatch-teamcity/services_ga.kt | 7 + 13 files changed, 283 insertions(+), 44 deletions(-) create mode 100644 tools/teamcity-diff-check/go.mod create mode 100644 tools/teamcity-diff-check/main_test.go create mode 100644 tools/teamcity-diff-check/test-fixtures/empty-files/ga-services.txt create mode 100644 tools/teamcity-diff-check/test-fixtures/empty-files/services_ga.kt create mode 100644 tools/teamcity-diff-check/test-fixtures/everything-ok/ga-services.txt create mode 100644 tools/teamcity-diff-check/test-fixtures/everything-ok/services_ga.kt create mode 100644 tools/teamcity-diff-check/test-fixtures/mismatch-provider/ga-services.txt create mode 100644 tools/teamcity-diff-check/test-fixtures/mismatch-provider/services_ga.kt create mode 100644 tools/teamcity-diff-check/test-fixtures/mismatch-teamcity/ga-services.txt create mode 100644 tools/teamcity-diff-check/test-fixtures/mismatch-teamcity/services_ga.kt diff --git a/.github/workflows/teamcity-services-diff-check-weekly.yml b/.github/workflows/teamcity-services-diff-check-weekly.yml index d9c257b1aca6..877c98844bfb 100644 --- a/.github/workflows/teamcity-services-diff-check-weekly.yml +++ b/.github/workflows/teamcity-services-diff-check-weekly.yml @@ -37,11 +37,13 @@ jobs: run: echo "GOOGLE_BETA_REPO_PATH=${{ env.OUTPUT_PATH}}" >> $GITHUB_ENV - name: Check that new services have been added to the TeamCity configuration code run: | - # Create lists of service packages in providers - ls ${{env.GOOGLE_REPO_PATH}}/google/services > tools/teamcity-diff-check/services_ga.txt - ls ${{env.GOOGLE_BETA_REPO_PATH}}/google-beta/services > tools/teamcity-diff-check/services_beta.txt - + # Create lists of service packages in providers. Need to cd into repos where go.mod is to do this command. + cd ${{env.GOOGLE_REPO_PATH}} + go list -f '{{.Name}}' ${{env.GOOGLE_REPO_PATH}}/google/services/... > $GITHUB_WORKSPACE/provider_services_ga.txt + cd ${{env.GOOGLE_BETA_REPO_PATH}} + go list -f '{{.Name}}' ${{env.GOOGLE_BETA_REPO_PATH}}/google-beta/services/... > $GITHUB_WORKSPACE/provider_services_beta.txt + # Run tool to compare service packages in the providers vs those listed in TeamCity config files - cd tools/teamcity-diff-check - go run main.go -service_file=services_ga - go run main.go -service_file=services_beta + cd $GITHUB_WORKSPACE + go run ./tools/teamcity-diff-check/main.go -version=ga -provider_services=./provider_services_ga.txt -teamcity_services=./mmv1/third_party/terraform/.teamcity/components/inputs/services_ga.kt + go run ./tools/teamcity-diff-check/main.go -version=beta -provider_services=./provider_services_beta.txt -teamcity_services=./mmv1/third_party/terraform/.teamcity/components/inputs/services_beta.kt diff --git a/.github/workflows/teamcity-services-diff-check.yml b/.github/workflows/teamcity-services-diff-check.yml index a5ff3e1bc3a7..ea059c466c4a 100644 --- a/.github/workflows/teamcity-services-diff-check.yml +++ b/.github/workflows/teamcity-services-diff-check.yml @@ -52,11 +52,13 @@ jobs: - name: Check that new services have been added to the TeamCity configuration code if: steps.generate.outcome == 'success' run: | - # Create lists of service packages in providers - ls ${{env.GOOGLE_REPO_PATH}}/google/services > tools/teamcity-diff-check/services_ga.txt - ls ${{env.GOOGLE_BETA_REPO_PATH}}/google-beta/services > tools/teamcity-diff-check/services_beta.txt + # Create lists of service packages in providers. Need to cd into repos where go.mod is to do this command. + cd ${{env.GOOGLE_REPO_PATH}} + go list -f '{{.Name}}' ${{env.GOOGLE_REPO_PATH}}/google/services/... > $GITHUB_WORKSPACE/provider_services_ga.txt + cd ${{env.GOOGLE_BETA_REPO_PATH}} + go list -f '{{.Name}}' ${{env.GOOGLE_BETA_REPO_PATH}}/google-beta/services/... > $GITHUB_WORKSPACE/provider_services_beta.txt # Run tool to compare service packages in the providers vs those listed in TeamCity config files - cd tools/teamcity-diff-check - go run main.go -service_file=services_ga - go run main.go -service_file=services_beta + cd $GITHUB_WORKSPACE + go run ./tools/teamcity-diff-check/main.go -version=ga -provider_services=./provider_services_ga.txt -teamcity_services=./mmv1/third_party/terraform/.teamcity/components/inputs/services_ga.kt + go run ./tools/teamcity-diff-check/main.go -version=beta -provider_services=./provider_services_beta.txt -teamcity_services=./mmv1/third_party/terraform/.teamcity/components/inputs/services_beta.kt diff --git a/tools/teamcity-diff-check/go.mod b/tools/teamcity-diff-check/go.mod new file mode 100644 index 000000000000..c0643ef9ac8c --- /dev/null +++ b/tools/teamcity-diff-check/go.mod @@ -0,0 +1,3 @@ +module github.com/GoogleCloudPlatform/magic-modules/tools/teamcity-diff-check + +go 1.23.2 diff --git a/tools/teamcity-diff-check/main.go b/tools/teamcity-diff-check/main.go index 301306f5b319..eba516ae27c9 100644 --- a/tools/teamcity-diff-check/main.go +++ b/tools/teamcity-diff-check/main.go @@ -9,31 +9,58 @@ import ( "regexp" ) -var serviceFile = flag.String("service_file", "services_ga", "kotlin service file to be parsed") +var version = flag.String("version", "", "the provider version under test. Must be `ga` or `beta`") +var teamcityServiceFile = flag.String("teamcity_services", "", "path to a kotlin service file to be parsed") +var providerServiceFile = flag.String("provider_services", "", "path to a .txt file listing all service packages in the provider") -func serviceDifference(gS, tS []string) []string { - t := make(map[string]struct{}, len(tS)) - for _, s := range tS { - t[s] = struct{}{} - } +// listDifference checks that all the items in list B are present in list A +func listDifference(listA, listB []string) error { + a := make(map[string]struct{}, len(listA)) + for _, s := range listA { + a[s] = struct{}{} + } var diff []string - for _, s := range gS { - if _, found := t[s]; !found { + for _, s := range listB { + if _, found := a[s]; !found { diff = append(diff, s) } } - return diff + if len(diff) > 0 { + return fmt.Errorf("%v", diff) + } + + return nil } func main() { flag.Parse() - file, err := os.Open(*serviceFile + ".txt") + ga := *version == "ga" + beta := *version == "beta" + if !ga && !beta { + fmt.Fprint(os.Stderr, "the flag `version` must be set to either `ga` or `beta`, and is case sensitive\n") + os.Exit(1) + } + + err := compareServices(*teamcityServiceFile, *providerServiceFile) + if err != nil { + fmt.Fprintf(os.Stderr, "Errors when inspecting the %s version of the Google provider\n", *version) + fmt.Fprintf(os.Stderr, "%s\n", err.Error()) + os.Exit(1) + } + + fmt.Fprintf(os.Stdout, "All services present in the %s provider codebase are present in TeamCity config, and vice versa\n", *version) +} + +// compareServices contains most of the logic of the main function, but is separated to make the code more testable +func compareServices(teamcityServiceFile, providerServiceFile string) error { + + // Get array of services from the provider service list file + file, err := os.Open(providerServiceFile) if err != nil { - fmt.Println(err) - return + return fmt.Errorf("error opening provider service list file: %w", err) } defer file.Close() @@ -42,31 +69,28 @@ func main() { for scanner.Scan() { googleServices = append(googleServices, scanner.Text()) } + if len(googleServices) == 0 { + return fmt.Errorf("could not find any services in the provider service list file %s", providerServiceFile) + } - //////////////////////////////////////////////////////////////////////////////// - - filePath := fmt.Sprintf("mmv1/third_party/terraform/.teamcity/components/inputs/%s.kt", *serviceFile) - f, err := os.Open(fmt.Sprintf("../../%s", filePath)) // Need to make path relative to where the script is called + // Get array of services from the TeamCity service list file + f, err := os.Open(teamcityServiceFile) if err != nil { - panic(err) + return fmt.Errorf("error opening TeamCity service list file: %w", err) } - // Get the file size stat, err := f.Stat() if err != nil { - fmt.Println(err) - return + return fmt.Errorf("error stating TeamCity service list file: %w", err) } - // Read the file into a byte slice bs := make([]byte, stat.Size()) _, err = bufio.NewReader(f).Read(bs) if err != nil && err != io.EOF { - fmt.Println(err) - return + return fmt.Errorf("error processing TeamCity service list file: %w", err) } - // Regex pattern captures "services" from *serviceFile. + // Regex pattern captures "services" from the Kotlin service list file. pattern := regexp.MustCompile(`(?m)"(?P\w+)"\sto\s+mapOf`) template := []byte("$service") @@ -74,20 +98,33 @@ func main() { dst := []byte{} teamcityServices := []string{} - // For each match of the regex in the content. for _, submatches := range pattern.FindAllSubmatchIndex(bs, -1) { service := pattern.Expand(dst, template, bs, submatches) teamcityServices = append(teamcityServices, string(service)) } if len(teamcityServices) == 0 { - fmt.Fprintf(os.Stderr, "error: script could not find any services listed in the file %s.kt .\n", filePath) - os.Exit(1) + return fmt.Errorf("could not find any services in the TeamCity service list file %s", teamcityServiceFile) } - if diff := serviceDifference(googleServices, teamcityServices); len(diff) != 0 { - fmt.Fprintf(os.Stderr, "error: missing services detected in %s\n", filePath) - fmt.Fprintf(os.Stderr, "Please update file to include these new services: %s\n", diff) - os.Exit(1) + // Determine diffs + errTeamCity := listDifference(teamcityServices, googleServices) + errProvider := listDifference(googleServices, teamcityServices) + + switch { + case errTeamCity != nil && errProvider != nil: + return fmt.Errorf(`mismatches detected: +TeamCity service file is missing services present in the provider: %s +Provider codebase is missing services present in the TeamCity service file: %s`, + errTeamCity, errProvider) + case errTeamCity != nil: + return fmt.Errorf(`mismatches detected: +TeamCity service file is missing services present in the provider: %s`, + errTeamCity) + case errProvider != nil: + return fmt.Errorf(`mismatches detected: +Provider codebase is missing services present in the TeamCity service file: %s`, + errProvider) } + return nil } diff --git a/tools/teamcity-diff-check/main_test.go b/tools/teamcity-diff-check/main_test.go new file mode 100644 index 000000000000..2166bf9acebb --- /dev/null +++ b/tools/teamcity-diff-check/main_test.go @@ -0,0 +1,159 @@ +package main + +import ( + "regexp" + "testing" +) + +func Test_main_happyPaths(t *testing.T) { + testCases := map[string]struct { + providerServiceFile string + teamcityServiceFile string + expectError bool + errorRegex *regexp.Regexp + missingServiceRegex *regexp.Regexp + }{ + "everything matches": { + providerServiceFile: "./test-fixtures/everything-ok/ga-services.txt", + teamcityServiceFile: "./test-fixtures/everything-ok/services_ga.kt", + }, + "something missing in TeamCity config present in provider code": { + providerServiceFile: "./test-fixtures/mismatch-teamcity/ga-services.txt", + teamcityServiceFile: "./test-fixtures/mismatch-teamcity/services_ga.kt", + expectError: true, + errorRegex: regexp.MustCompile("TeamCity service file is missing services present in the provider"), + missingServiceRegex: regexp.MustCompile("[pubsub]"), + }, + "something missing in provider code present in TeamCity config": { + providerServiceFile: "./test-fixtures/mismatch-provider/ga-services.txt", + teamcityServiceFile: "./test-fixtures/mismatch-provider/services_ga.kt", + expectError: true, + errorRegex: regexp.MustCompile("Provider codebase is missing services present in the TeamCity service file"), + missingServiceRegex: regexp.MustCompile("[compute]"), + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + err := compareServices(tc.teamcityServiceFile, tc.providerServiceFile) + if err != nil && !tc.expectError { + t.Fatalf("saw an unexpected error: %s", err) + } + if err == nil && tc.expectError { + t.Fatalf("expected an error but saw none") + } + + if err == nil { + // Stop handling of non-error test cases + return + } + + if !tc.errorRegex.MatchString(err.Error()) { + t.Fatalf("expected error to contain a match for regex `%s`, got error string: `%s`", tc.errorRegex.String(), err) + } + if !tc.missingServiceRegex.MatchString(err.Error()) { + t.Fatalf("expected error to contain a match for regex `%s`, got error string: `%s`", tc.errorRegex.String(), err) + } + }) + } +} + +func Test_main_unhappyPaths(t *testing.T) { + testCases := map[string]struct { + providerServiceFile string + teamcityServiceFile string + expectError bool + errorRegex *regexp.Regexp + }{ + "cannot find provider service file": { + providerServiceFile: "./test-fixtures/doesnt-exist.txt", + teamcityServiceFile: "./test-fixtures/everything-ok/services_ga.kt", + expectError: true, + errorRegex: regexp.MustCompile("error opening provider service list file: open ./test-fixtures/doesnt-exist.txt"), + }, + "cannot find TeamCity service file": { + providerServiceFile: "./test-fixtures/everything-ok/ga-services.txt", + teamcityServiceFile: "./test-fixtures/everything-ok/doesnt-exist.kt", + expectError: true, + errorRegex: regexp.MustCompile("error opening TeamCity service list file: open ./test-fixtures/everything-ok/doesnt-exist.kt"), + }, + "empty TeamCity service file": { + providerServiceFile: "./test-fixtures/everything-ok/ga-services.txt", + teamcityServiceFile: "./test-fixtures/empty-files/services_ga.kt", + expectError: true, + errorRegex: regexp.MustCompile("could not find any services in the TeamCity service list file ./test-fixtures/empty-files/services_ga.kt"), + }, + "empty provider service file": { + providerServiceFile: "./test-fixtures/empty-files/ga-services.txt", + teamcityServiceFile: "./test-fixtures/everything-ok/services_ga.kt", + expectError: true, + errorRegex: regexp.MustCompile("could not find any services in the provider service list file ./test-fixtures/empty-files/ga-services.txt"), + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + err := compareServices(tc.teamcityServiceFile, tc.providerServiceFile) + if err != nil && !tc.expectError { + t.Fatalf("saw an unexpected error: %s", err) + } + if err == nil && tc.expectError { + t.Fatalf("expected an error but saw none") + } + + if !tc.errorRegex.MatchString(err.Error()) { + t.Fatalf("expected error to contain a match for regex `%s`, got error string: `%s`", tc.errorRegex.String(), err) + } + }) + } +} + +func Test_listDifference(t *testing.T) { + testCases := map[string]struct { + a []string + b []string + expectDiff bool + errorRegex *regexp.Regexp + }{ + "detects when lists match": { + a: []string{"a", "c", "b"}, + b: []string{"a", "b", "c"}, + }, + "detects when items from list A is missing items present in list B - 1 missing": { + a: []string{"a", "b"}, + b: []string{"a", "c", "b"}, + expectDiff: true, + errorRegex: regexp.MustCompile("[c]"), + }, + "detects when items from list A is missing items present in list B - 2 missing": { + a: []string{"b"}, + b: []string{"a", "c", "b"}, + expectDiff: true, + errorRegex: regexp.MustCompile("[a, c]"), + }, + "doesn't detect differences if list A is a superset of list B": { + a: []string{"a", "b", "c"}, + b: []string{"a", "c"}, + expectDiff: false, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + err := listDifference(tc.a, tc.b) + if !tc.expectDiff && (err != nil) { + t.Fatalf("saw an unexpected diff error: %s", err) + } + if tc.expectDiff && (err == nil) { + t.Fatalf("expected a diff error but saw none") + } + if !tc.expectDiff && err == nil { + // Stop assertions in no error cases + return + } + if !tc.errorRegex.MatchString(err.Error()) { + t.Fatalf("expected diff error to contain a match for regex %s, error string: %s", tc.errorRegex.String(), err) + } + }) + } +} diff --git a/tools/teamcity-diff-check/test-fixtures/empty-files/ga-services.txt b/tools/teamcity-diff-check/test-fixtures/empty-files/ga-services.txt new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tools/teamcity-diff-check/test-fixtures/empty-files/services_ga.kt b/tools/teamcity-diff-check/test-fixtures/empty-files/services_ga.kt new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tools/teamcity-diff-check/test-fixtures/everything-ok/ga-services.txt b/tools/teamcity-diff-check/test-fixtures/everything-ok/ga-services.txt new file mode 100644 index 000000000000..62aec83f16e1 --- /dev/null +++ b/tools/teamcity-diff-check/test-fixtures/everything-ok/ga-services.txt @@ -0,0 +1,2 @@ +compute +pubsub \ No newline at end of file diff --git a/tools/teamcity-diff-check/test-fixtures/everything-ok/services_ga.kt b/tools/teamcity-diff-check/test-fixtures/everything-ok/services_ga.kt new file mode 100644 index 000000000000..b56973ecf0ea --- /dev/null +++ b/tools/teamcity-diff-check/test-fixtures/everything-ok/services_ga.kt @@ -0,0 +1,12 @@ +var ServicesListGa = mapOf( + "compute" to mapOf( + "name" to "compute", + "displayName" to "Compute", + "path" to "./google/services/compute" + ), + "pubsub" to mapOf( + "name" to "pubsub", + "displayName" to "PubSub", + "path" to "./google/services/pubsub" + ), +) \ No newline at end of file diff --git a/tools/teamcity-diff-check/test-fixtures/mismatch-provider/ga-services.txt b/tools/teamcity-diff-check/test-fixtures/mismatch-provider/ga-services.txt new file mode 100644 index 000000000000..a48da76e0d8c --- /dev/null +++ b/tools/teamcity-diff-check/test-fixtures/mismatch-provider/ga-services.txt @@ -0,0 +1 @@ +pubsub \ No newline at end of file diff --git a/tools/teamcity-diff-check/test-fixtures/mismatch-provider/services_ga.kt b/tools/teamcity-diff-check/test-fixtures/mismatch-provider/services_ga.kt new file mode 100644 index 000000000000..b56973ecf0ea --- /dev/null +++ b/tools/teamcity-diff-check/test-fixtures/mismatch-provider/services_ga.kt @@ -0,0 +1,12 @@ +var ServicesListGa = mapOf( + "compute" to mapOf( + "name" to "compute", + "displayName" to "Compute", + "path" to "./google/services/compute" + ), + "pubsub" to mapOf( + "name" to "pubsub", + "displayName" to "PubSub", + "path" to "./google/services/pubsub" + ), +) \ No newline at end of file diff --git a/tools/teamcity-diff-check/test-fixtures/mismatch-teamcity/ga-services.txt b/tools/teamcity-diff-check/test-fixtures/mismatch-teamcity/ga-services.txt new file mode 100644 index 000000000000..62aec83f16e1 --- /dev/null +++ b/tools/teamcity-diff-check/test-fixtures/mismatch-teamcity/ga-services.txt @@ -0,0 +1,2 @@ +compute +pubsub \ No newline at end of file diff --git a/tools/teamcity-diff-check/test-fixtures/mismatch-teamcity/services_ga.kt b/tools/teamcity-diff-check/test-fixtures/mismatch-teamcity/services_ga.kt new file mode 100644 index 000000000000..8023d6236096 --- /dev/null +++ b/tools/teamcity-diff-check/test-fixtures/mismatch-teamcity/services_ga.kt @@ -0,0 +1,7 @@ +var ServicesListGa = mapOf( + "compute" to mapOf( + "name" to "compute", + "displayName" to "Compute", + "path" to "./google/services/compute" + ), +) \ No newline at end of file