From 227a05e4bff831c81bafe734f282f755b9b3fb1c Mon Sep 17 00:00:00 2001 From: Thomas Rodgers Date: Thu, 26 Sep 2024 18:27:00 +0000 Subject: [PATCH] Further refactoring of magician VCR (#11647) Co-authored-by: Stephen Lewis (Burrows) --- .ci/gcb-pr-downstream-generation-and-test.yml | 2 +- .ci/magician/cmd/check_cassettes.go | 2 +- .ci/magician/cmd/generate_comment.go | 4 +- .ci/magician/cmd/test_terraform_vcr.go | 30 ++++---- .ci/magician/cmd/vcr_cassette_update.go | 2 +- .ci/magician/cmd/vcr_cassette_update_test.go | 2 +- .ci/magician/provider/version.go | 7 +- .ci/magician/vcr/tester.go | 68 ++++++++++++++----- magician-vcr-eap | 1 + 9 files changed, 80 insertions(+), 38 deletions(-) create mode 160000 magician-vcr-eap diff --git a/.ci/gcb-pr-downstream-generation-and-test.yml b/.ci/gcb-pr-downstream-generation-and-test.yml index 9699417a831f..9e753b8d5174 100644 --- a/.ci/gcb-pr-downstream-generation-and-test.yml +++ b/.ci/gcb-pr-downstream-generation-and-test.yml @@ -264,7 +264,7 @@ steps: - $COMMIT_SHA - $BUILD_ID - $PROJECT_ID - - "22" # Build step + - "23" # Build step - name: 'gcr.io/graphite-docker-images/go-plus' entrypoint: '/workspace/.ci/scripts/go-plus/magician/exec.sh' diff --git a/.ci/magician/cmd/check_cassettes.go b/.ci/magician/cmd/check_cassettes.go index b1b2de4aaf55..e149dd370844 100644 --- a/.ci/magician/cmd/check_cassettes.go +++ b/.ci/magician/cmd/check_cassettes.go @@ -64,7 +64,7 @@ var checkCassettesCmd = &cobra.Command{ ctlr := source.NewController(env["GOPATH"], "modular-magician", githubToken, rnr) - vt, err := vcr.NewTester(env, "vcr-check-cassettes", "ci-vcr-cassettes", rnr) + vt, err := vcr.NewTester(env, "ci-vcr-cassettes", "vcr-check-cassettes", rnr) if err != nil { return fmt.Errorf("error creating VCR tester: %w", err) } diff --git a/.ci/magician/cmd/generate_comment.go b/.ci/magician/cmd/generate_comment.go index 0962a0f71e19..20a5d41b5d2d 100644 --- a/.ci/magician/cmd/generate_comment.go +++ b/.ci/magician/cmd/generate_comment.go @@ -504,9 +504,9 @@ func detectMissingTests(diffProcessorPath, tpgbLocalPath string, rnr ExecRunner) } func formatDiffComment(data diffCommentData) (string, error) { - tmpl, err := template.New("DIFF_COMMENT.md").Parse(diffComment) + tmpl, err := template.New("DIFF_COMMENT.md.tmpl").Parse(diffComment) if err != nil { - panic(fmt.Sprintf("Unable to parse DIFF_COMMENT.md: %s", err)) + return "", fmt.Errorf("unable to parse template DIFF_COMMENT.md.tmpl: %s", err) } sb := new(strings.Builder) err = tmpl.Execute(sb, data) diff --git a/.ci/magician/cmd/test_terraform_vcr.go b/.ci/magician/cmd/test_terraform_vcr.go index f59db6c0cbe3..db1902730213 100644 --- a/.ci/magician/cmd/test_terraform_vcr.go +++ b/.ci/magician/cmd/test_terraform_vcr.go @@ -120,7 +120,7 @@ var testTerraformVCRCmd = &cobra.Command{ } ctlr := source.NewController(env["GOPATH"], "modular-magician", env["GITHUB_TOKEN_DOWNSTREAMS"], rnr) - vt, err := vcr.NewTester(env, "ci-vcr-logs", "ci-vcr-cassettes", rnr) + vt, err := vcr.NewTester(env, "ci-vcr-cassettes", "ci-vcr-logs", rnr) if err != nil { return fmt.Errorf("error creating VCR tester: %w", err) } @@ -180,7 +180,7 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, } fmt.Println("Running tests: Go files or test fixtures changed") - if err := vt.FetchCassettes(provider.Beta, baseBranch, prNumber); err != nil { + if err := vt.FetchCassettes(provider.Beta, baseBranch, newBranch); err != nil { return fmt.Errorf("error fetching cassettes: %w", err) } @@ -196,10 +196,10 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, } if err := vt.UploadLogs(vcr.UploadLogsOptions{ - PRNumber: prNumber, - BuildID: buildID, - Mode: vcr.Replaying, - Version: provider.Beta, + Head: newBranch, + BuildID: buildID, + Mode: vcr.Replaying, + Version: provider.Beta, }); err != nil { return fmt.Errorf("error uploading replaying logs: %w", err) } @@ -261,12 +261,12 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, testState = "success" } - if err := vt.UploadCassettes(prNumber, provider.Beta); err != nil { + if err := vt.UploadCassettes(newBranch, provider.Beta); err != nil { return fmt.Errorf("error uploading cassettes: %w", err) } if err := vt.UploadLogs(vcr.UploadLogsOptions{ - PRNumber: prNumber, + Head: newBranch, BuildID: buildID, Parallel: true, Mode: vcr.Recording, @@ -295,10 +295,10 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, } if err := vt.UploadLogs(vcr.UploadLogsOptions{ - PRNumber: prNumber, + Head: newBranch, BuildID: buildID, - Parallel: true, AfterRecording: true, + Parallel: true, Mode: vcr.Replaying, Version: provider.Beta, }); err != nil { @@ -491,21 +491,21 @@ func formatComment(fileName string, tmplText string, data any) (string, error) { } func formatTestsAnalytics(data analytics) (string, error) { - return formatComment("test_terraform_vcr_test_analytics.tmpl", testsAnalyticsTmplText, data) + return formatComment("test_analytics.tmpl", testsAnalyticsTmplText, data) } func formatNonExercisedTests(data nonExercisedTests) (string, error) { - return formatComment("test_terraform_vcr_recording_mode_results.tmpl", nonExercisedTestsTmplText, data) + return formatComment("non_exercised_tests.tmpl", nonExercisedTestsTmplText, data) } func formatWithReplayFailedTests(data withReplayFailedTests) (string, error) { - return formatComment("test_terraform_vcr_with_replay_failed_tests.tmpl", withReplayFailedTestsTmplText, data) + return formatComment("with_replay_failed_tests.tmpl", withReplayFailedTestsTmplText, data) } func formatWithoutReplayFailedTests(data withoutReplayFailedTests) (string, error) { - return formatComment("test_terraform_vcr_without_replay_failed_tests.tmpl", withoutReplayFailedTestsTmplText, data) + return formatComment("without_replay_failed_tests.tmpl", withoutReplayFailedTestsTmplText, data) } func formatRecordReplay(data recordReplay) (string, error) { - return formatComment("test_terraform_vcr_record_replay.tmpl", recordReplayTmplText, data) + return formatComment("record_replay.tmpl", recordReplayTmplText, data) } diff --git a/.ci/magician/cmd/vcr_cassette_update.go b/.ci/magician/cmd/vcr_cassette_update.go index ac10d7dafe15..91588e676195 100644 --- a/.ci/magician/cmd/vcr_cassette_update.go +++ b/.ci/magician/cmd/vcr_cassette_update.go @@ -90,7 +90,7 @@ var vcrCassetteUpdateCmd = &cobra.Command{ } ctlr := source.NewController(env["GOPATH"], "hashicorp", env["GITHUB_TOKEN_CLASSIC"], rnr) - vt, err := vcr.NewTester(env, "", "ci-vcr-cassettes", rnr) + vt, err := vcr.NewTester(env, "ci-vcr-cassettes", "", rnr) if err != nil { return fmt.Errorf("error creating VCR tester: %w", err) } diff --git a/.ci/magician/cmd/vcr_cassette_update_test.go b/.ci/magician/cmd/vcr_cassette_update_test.go index fad17ce4c584..beb96423869f 100644 --- a/.ci/magician/cmd/vcr_cassette_update_test.go +++ b/.ci/magician/cmd/vcr_cassette_update_test.go @@ -401,7 +401,7 @@ func TestExecVCRCassetteUpdate(t *testing.T) { ctlr := source.NewController("gopath", "hashicorp", "token", rnr) vt, err := vcr.NewTester(map[string]string{ "SA_KEY": "sa_key", - }, "", "ci-vcr-cassettes", rnr) + }, "ci-vcr-cassettes", "", rnr) if err != nil { t.Fatalf("Failed to create new tester: %v", err) } diff --git a/.ci/magician/provider/version.go b/.ci/magician/provider/version.go index c8c65fc8e82b..6372ff7fd297 100644 --- a/.ci/magician/provider/version.go +++ b/.ci/magician/provider/version.go @@ -6,9 +6,10 @@ const ( None Version = iota GA Beta + Alpha ) -const NumVersions = 2 +const NumVersions = 3 func (v Version) String() string { switch v { @@ -16,6 +17,8 @@ func (v Version) String() string { return "ga" case Beta: return "beta" + case Alpha: + return "alpha" } return "unknown" } @@ -33,6 +36,8 @@ func (v Version) RepoName() string { return "terraform-provider-google" case Beta: return "terraform-provider-google-beta" + case Alpha: + return "terraform-next" } return "unknown" } diff --git a/.ci/magician/vcr/tester.go b/.ci/magician/vcr/tester.go index 47a1ff968685..471f4448bbcd 100644 --- a/.ci/magician/vcr/tester.go +++ b/.ci/magician/vcr/tester.go @@ -50,8 +50,8 @@ type logKey struct { type Tester struct { env map[string]string // shared environment variables for running tests rnr ExecRunner // for running commands and manipulating files - logBucket string // GCS bucket name to store logs - cassetteBucket string // GCS bucket name to store cassettes + cassetteBucket string // name of GCS bucket to store cassettes + logBucket string // name of GCS bucket to store logs baseDir string // the directory in which this tester was created saKeyPath string // where sa_key.json is relative to baseDir cassettePaths map[provider.Version]string // where cassettes are relative to baseDir by version @@ -68,8 +68,46 @@ var testResultsExpression = regexp.MustCompile(`(?m:^--- (PASS|FAIL|SKIP): (Test var testPanicExpression = regexp.MustCompile(`^panic: .*`) +var safeToLog = map[string]bool{ + "ACCTEST_PARALLELISM": true, + "COMMIT_SHA": true, + "GITHUB_TOKEN": false, + "GITHUB_TOKEN_CLASSIC": false, + "GITHUB_TOKEN_DOWNSTREAMS": false, + "GITHUB_TOKEN_MAGIC_MODULES": false, + "GOCACHE": true, + "GOOGLE_APPLICATION_CREDENTIALS": false, + "GOOGLE_BILLING_ACCOUNT": false, + "GOOGLE_CREDENTIALS": false, + "GOOGLE_CUST_ID": true, + "GOOGLE_IDENTITY_USER": true, + "GOOGLE_MASTER_BILLING_ACCOUNT": false, + "GOOGLE_ORG": true, + "GOOGLE_ORG_2": true, + "GOOGLE_ORG_DOMAIN": true, + "GOOGLE_PROJECT": true, + "GOOGLE_PROJECT_NUMBER": true, + "GOOGLE_PUBLIC_AVERTISED_PREFIX_DESCRIPTION": true, + "GOOGLE_REGION": true, + "GOOGLE_SERVICE_ACCOUNT": true, + "GOOGLE_TEST_DIRECTORY": true, + "GOOGLE_ZONE": true, + "GOPATH": true, + "HOME": true, + "PATH": true, + "SA_KEY": false, + "TF_ACC": true, + "TF_LOG": true, + "TF_LOG_PATH_MASK": true, + "TF_LOG_SDK_FRAMEWORK": true, + "TF_SCHEMA_PANIC_ON_ERROR": true, + "USER": true, + "VCR_MODE": true, + "VCR_PATH": true, +} // true if shown, false if hidden (default false) + // Create a new tester in the current working directory and write the service account key file. -func NewTester(env map[string]string, logBucket, cassetteBucket string, rnr ExecRunner) (*Tester, error) { +func NewTester(env map[string]string, cassetteBucket, logBucket string, rnr ExecRunner) (*Tester, error) { var saKeyPath string if saKeyVal, ok := env["SA_KEY"]; ok { saKeyPath = "sa_key.json" @@ -80,8 +118,8 @@ func NewTester(env map[string]string, logBucket, cassetteBucket string, rnr Exec return &Tester{ env: env, rnr: rnr, - logBucket: logBucket, cassetteBucket: cassetteBucket, + logBucket: logBucket, baseDir: rnr.GetCWD(), saKeyPath: saKeyPath, cassettePaths: make(map[provider.Version]string, provider.NumVersions), @@ -96,7 +134,7 @@ func (vt *Tester) SetRepoPath(version provider.Version, repoPath string) { // Fetch the cassettes for the current version if not already fetched. // Should be run from the base dir. -func (vt *Tester) FetchCassettes(version provider.Version, baseBranch, prNumber string) error { +func (vt *Tester) FetchCassettes(version provider.Version, baseBranch, head string) error { _, ok := vt.cassettePaths[version] if ok { return nil @@ -116,8 +154,8 @@ func (vt *Tester) FetchCassettes(version provider.Version, baseBranch, prNumber fmt.Println("Error fetching cassettes: ", err) } } - if prNumber != "" { - bucketPath := fmt.Sprintf("gs://%s/%srefs/heads/auto-pr-%s/fixtures/*", vt.cassetteBucket, version.BucketPath(), prNumber) + if head != "" { + bucketPath := fmt.Sprintf("gs://%s/%srefs/heads/%s/fixtures/*", vt.cassetteBucket, version.BucketPath(), head) if err := vt.fetchBucketPath(bucketPath, cassettePath); err != nil { fmt.Println("Error fetching cassettes: ", err) } @@ -131,7 +169,7 @@ func (vt *Tester) fetchBucketPath(bucketPath, cassettePath string) error { args := []string{"-m", "-q", "cp", bucketPath, cassettePath} fmt.Println("Fetching cassettes:\n", "gsutil", strings.Join(args, " ")) if _, err := vt.rnr.Run("gsutil", args, nil); err != nil { - return err + return fmt.Errorf("error running gsutil: %v", err) } return nil } @@ -225,7 +263,7 @@ func (vt *Tester) Run(opt RunOptions) (Result, error) { } var printedEnv string for ev, val := range env { - if ev == "SA_KEY" || ev == "GOOGLE_CREDENTIALS" || strings.HasPrefix(ev, "GITHUB_TOKEN") { + if !safeToLog[ev] { val = "{hidden}" } printedEnv += fmt.Sprintf("%s=%s\n", ev, val) @@ -403,9 +441,8 @@ func (vt *Tester) getLogPath(mode Mode, version provider.Version) (string, error return logPath, nil } -// UploadLogsOptions defines options for uploading logs. type UploadLogsOptions struct { - PRNumber string + Head string BuildID string Parallel bool AfterRecording bool @@ -413,11 +450,10 @@ type UploadLogsOptions struct { Version provider.Version } -// UploadLogs uploads logs to Google Cloud Storage. func (vt *Tester) UploadLogs(opts UploadLogsOptions) error { bucketPath := fmt.Sprintf("gs://%s/%s/", vt.logBucket, opts.Version) - if opts.PRNumber != "" { - bucketPath += fmt.Sprintf("refs/heads/auto-pr-%s/", opts.PRNumber) + if opts.Head != "" { + bucketPath += fmt.Sprintf("refs/heads/%s/", opts.Head) } if opts.BuildID != "" { bucketPath += fmt.Sprintf("artifacts/%s/", opts.BuildID) @@ -478,7 +514,7 @@ func (vt *Tester) UploadLogs(opts UploadLogsOptions) error { return nil } -func (vt *Tester) UploadCassettes(prNumber string, version provider.Version) error { +func (vt *Tester) UploadCassettes(head string, version provider.Version) error { cassettePath, ok := vt.cassettePaths[version] if !ok { return fmt.Errorf("no cassettes found for version %s", version) @@ -488,7 +524,7 @@ func (vt *Tester) UploadCassettes(prNumber string, version provider.Version) err "-q", "cp", filepath.Join(cassettePath, "*"), - fmt.Sprintf("gs://%s/%s/refs/heads/auto-pr-%s/fixtures/", vt.cassetteBucket, version, prNumber), + fmt.Sprintf("gs://%s/%s/refs/heads/%s/fixtures/", vt.cassetteBucket, version, head), } fmt.Println("Uploading cassettes:\n", "gsutil", strings.Join(args, " ")) if _, err := vt.rnr.Run("gsutil", args, nil); err != nil { diff --git a/magician-vcr-eap b/magician-vcr-eap new file mode 160000 index 000000000000..489059b3c38f --- /dev/null +++ b/magician-vcr-eap @@ -0,0 +1 @@ +Subproject commit 489059b3c38f038cd99d3b1d570fa4b09db4275c