From 410be6dde155c0e4deaf9770ff80280d76ec3000 Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Mon, 5 Dec 2022 18:00:44 -0800 Subject: [PATCH 01/14] Move terraform version listing into Terraform client --- .../terraform/mocks/mock_terraform_client.go | 46 +++++++++++++++++++ server/core/terraform/terraform_client.go | 11 +++++ server/events/project_command_builder.go | 10 ++++ .../project_command_builder_internal_test.go | 11 ++++- server/events/project_command_builder_test.go | 29 ++++++++++++ .../events/project_command_context_builder.go | 31 +++++++------ .../project_command_context_builder_test.go | 10 ++-- server/server.go | 1 + 8 files changed, 131 insertions(+), 18 deletions(-) diff --git a/server/core/terraform/mocks/mock_terraform_client.go b/server/core/terraform/mocks/mock_terraform_client.go index 746f39a0c7..655e6d40d9 100644 --- a/server/core/terraform/mocks/mock_terraform_client.go +++ b/server/core/terraform/mocks/mock_terraform_client.go @@ -61,6 +61,25 @@ func (mock *MockClient) EnsureVersion(log logging.SimpleLogging, v *go_version.V return ret0 } +func (mock *MockClient) ListAvailableVersions(log logging.SimpleLogging) ([]string, error) { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockClient().") + } + params := []pegomock.Param{log} + result := pegomock.GetGenericMockFrom(mock).Invoke("ListAvailableVersions", params, []reflect.Type{reflect.TypeOf((*[]string)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 []string + var ret1 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].([]string) + } + if result[1] != nil { + ret1 = result[1].(error) + } + } + return ret0, ret1 +} + func (mock *MockClient) VerifyWasCalledOnce() *VerifierMockClient { return &VerifierMockClient{ mock: mock, @@ -175,3 +194,30 @@ func (c *MockClient_EnsureVersion_OngoingVerification) GetAllCapturedArguments() } return } + +func (verifier *VerifierMockClient) ListAvailableVersions(log logging.SimpleLogging) *MockClient_ListAvailableVersions_OngoingVerification { + params := []pegomock.Param{log} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "ListAvailableVersions", params, verifier.timeout) + return &MockClient_ListAvailableVersions_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockClient_ListAvailableVersions_OngoingVerification struct { + mock *MockClient + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockClient_ListAvailableVersions_OngoingVerification) GetCapturedArguments() logging.SimpleLogging { + log := c.GetAllCapturedArguments() + return log[len(log)-1] +} + +func (c *MockClient_ListAvailableVersions_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(logging.SimpleLogging) + } + } + return +} diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index 3260b2d6d4..2914bad284 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -30,6 +30,7 @@ import ( "github.com/hashicorp/go-version" "github.com/mitchellh/go-homedir" "github.com/pkg/errors" + "github.com/warrensbox/terraform-switcher/lib" "github.com/runatlantis/atlantis/server/core/runtime/models" "github.com/runatlantis/atlantis/server/events/command" @@ -53,6 +54,8 @@ type Client interface { // EnsureVersion makes sure that terraform version `v` is available to use EnsureVersion(log logging.SimpleLogging, v *version.Version) error + + ListAvailableVersions(log logging.SimpleLogging) ([]string, error) } type DefaultClient struct { @@ -262,6 +265,14 @@ func (c *DefaultClient) TerraformBinDir() string { return c.binDir } +// ListAvailableVersions returns all available version of Terraform. If downloads are disabled, this will return an empty list. +func (c *DefaultClient) ListAvailableVersions(log logging.SimpleLogging) ([]string, error) { + url := fmt.Sprintf("%s/terraform", c.downloadBaseURL) + log.Debug("Listing Terraform versions available at: %s", url) + versions, err := lib.GetTFList(url, true) + return versions, err +} + // See Client.EnsureVersion. func (c *DefaultClient) EnsureVersion(log logging.SimpleLogging, v *version.Version) error { if v == nil { diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 6d58a79b96..64a6aa6a3b 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -10,6 +10,7 @@ import ( "github.com/uber-go/tally" "github.com/runatlantis/atlantis/server/core/config/valid" + "github.com/runatlantis/atlantis/server/core/terraform" "github.com/runatlantis/atlantis/server/logging" "github.com/runatlantis/atlantis/server/metrics" @@ -54,6 +55,7 @@ func NewInstrumentedProjectCommandBuilder( RestrictFileList bool, scope tally.Scope, logger logging.SimpleLogging, + terraformClient terraform.Client, ) *InstrumentedProjectCommandBuilder { scope = scope.SubScope("builder") @@ -79,6 +81,7 @@ func NewInstrumentedProjectCommandBuilder( RestrictFileList, scope, logger, + terraformClient, ), Logger: logger, scope: scope, @@ -102,6 +105,7 @@ func NewProjectCommandBuilder( RestrictFileList bool, scope tally.Scope, logger logging.SimpleLogging, + terraformClient terraform.Client, ) *DefaultProjectCommandBuilder { return &DefaultProjectCommandBuilder{ ParserValidator: parserValidator, @@ -121,6 +125,7 @@ func NewProjectCommandBuilder( commentBuilder, scope, ), + TerraformExecutor: terraformClient, } } @@ -181,6 +186,7 @@ type DefaultProjectCommandBuilder struct { AutoplanFileList string EnableDiffMarkdownFormat bool RestrictFileList bool + TerraformExecutor terraform.Client } // See ProjectCommandBuilder.BuildAutoplanCommands. @@ -330,6 +336,7 @@ func (p *DefaultProjectCommandBuilder) buildPlanAllCommands(ctx *command.Context repoCfg.ParallelApply, repoCfg.ParallelPlan, verbose, + p.TerraformExecutor, )...) } } else { @@ -367,6 +374,7 @@ func (p *DefaultProjectCommandBuilder) buildPlanAllCommands(ctx *command.Context DefaultParallelApplyEnabled, DefaultParallelPlanEnabled, verbose, + p.TerraformExecutor, )...) } } @@ -701,6 +709,7 @@ func (p *DefaultProjectCommandBuilder) buildProjectCommandCtx(ctx *command.Conte parallelApply, parallelPlan, verbose, + p.TerraformExecutor, )...) } } else { @@ -716,6 +725,7 @@ func (p *DefaultProjectCommandBuilder) buildProjectCommandCtx(ctx *command.Conte parallelApply, parallelPlan, verbose, + p.TerraformExecutor, )...) } diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index 62239521d9..d2e1487c82 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -7,9 +7,9 @@ import ( version "github.com/hashicorp/go-version" . "github.com/petergtz/pegomock" - "github.com/runatlantis/atlantis/server/core/config" "github.com/runatlantis/atlantis/server/core/config/valid" + "github.com/runatlantis/atlantis/server/core/terraform/mocks" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/matchers" "github.com/runatlantis/atlantis/server/events/models" @@ -621,6 +621,8 @@ projects: Ok(t, os.WriteFile(filepath.Join(tmp, "atlantis.yaml"), []byte(c.repoCfg), 0600)) } + terraformClient := mocks.NewMockClient() + builder := NewProjectCommandBuilder( false, parser, @@ -638,6 +640,7 @@ projects: false, statsScope, logger, + terraformClient, ) // We run a test for each type of command. @@ -825,6 +828,8 @@ projects: logger := logging.NewNoopLogger(t) statsScope, _, _ := metrics.NewLoggingScope(logging.NewNoopLogger(t), "atlantis") + terraformClient := mocks.NewMockClient() + builder := NewProjectCommandBuilder( false, parser, @@ -842,6 +847,7 @@ projects: false, statsScope, logger, + terraformClient, ) // We run a test for each type of command, again specific projects @@ -1058,6 +1064,8 @@ workflows: } statsScope, _, _ := metrics.NewLoggingScope(logging.NewNoopLogger(t), "atlantis") + terraformClient := mocks.NewMockClient() + builder := NewProjectCommandBuilder( true, parser, @@ -1075,6 +1083,7 @@ workflows: false, statsScope, logger, + terraformClient, ) cmd := command.PolicyCheck diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 9c3f8b8349..2d5e87413e 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -8,6 +8,7 @@ import ( "testing" . "github.com/petergtz/pegomock" + terraform_mocks "github.com/runatlantis/atlantis/server/core/terraform/mocks" "github.com/runatlantis/atlantis/server/core/config" "github.com/runatlantis/atlantis/server/core/config/valid" @@ -123,6 +124,8 @@ projects: logger := logging.NewNoopLogger(t) scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") + terraformClient := terraform_mocks.NewMockClient() + for _, c := range cases { t.Run(c.Description, func(t *testing.T) { RegisterMockTestingT(t) @@ -163,6 +166,7 @@ projects: false, scope, logger, + terraformClient, ) ctxs, err := builder.BuildAutoplanCommands(&command.Context{ @@ -415,6 +419,8 @@ projects: UnDivergedReq: false, } + terraformClient := terraform_mocks.NewMockClient() + builder := events.NewProjectCommandBuilder( false, &config.ParserValidator{}, @@ -432,6 +438,7 @@ projects: false, scope, logger, + terraformClient, ) var actCtxs []command.ProjectContext @@ -588,6 +595,8 @@ projects: UnDivergedReq: false, } + terraformClient := terraform_mocks.NewMockClient() + builder := events.NewProjectCommandBuilder( false, &config.ParserValidator{}, @@ -605,6 +614,7 @@ projects: true, scope, logger, + terraformClient, ) var actCtxs []command.ProjectContext @@ -758,6 +768,8 @@ projects: UnDivergedReq: false, } + terraformClient := terraform_mocks.NewMockClient() + builder := events.NewProjectCommandBuilder( false, &config.ParserValidator{}, @@ -775,6 +787,7 @@ projects: false, scope, logger, + terraformClient, ) ctxs, err := builder.BuildPlanCommands( @@ -850,6 +863,8 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) { } scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") + terraformClient := terraform_mocks.NewMockClient() + builder := events.NewProjectCommandBuilder( false, &config.ParserValidator{}, @@ -867,6 +882,7 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) { false, scope, logger, + terraformClient, ) ctxs, err := builder.BuildApplyCommands( @@ -935,6 +951,7 @@ projects: } logger := logging.NewNoopLogger(t) scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") + terraformClient := terraform_mocks.NewMockClient() builder := events.NewProjectCommandBuilder( false, @@ -953,6 +970,7 @@ projects: false, scope, logger, + terraformClient, ) ctx := &command.Context{ @@ -1016,6 +1034,8 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { UnDivergedReq: false, } + terraformClient := terraform_mocks.NewMockClient() + builder := events.NewProjectCommandBuilder( false, &config.ParserValidator{}, @@ -1033,6 +1053,7 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { false, scope, logger, + terraformClient, ) var actCtxs []command.ProjectContext @@ -1220,6 +1241,7 @@ projects: ApprovedReq: false, UnDivergedReq: false, } + terraformClient := terraform_mocks.NewMockClient() builder := events.NewProjectCommandBuilder( false, @@ -1238,6 +1260,7 @@ projects: false, scope, logger, + terraformClient, ) actCtxs, err := builder.BuildPlanCommands( @@ -1310,6 +1333,7 @@ parallel_plan: true`, UnDivergedReq: false, } scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") + terraformClient := terraform_mocks.NewMockClient() builder := events.NewProjectCommandBuilder( false, @@ -1328,6 +1352,7 @@ parallel_plan: true`, false, scope, logger, + terraformClient, ) var actCtxs []command.ProjectContext @@ -1370,6 +1395,7 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman } globalCfg := valid.NewGlobalCfgFromArgs(globalCfgArgs) + terraformClient := terraform_mocks.NewMockClient() builder := events.NewProjectCommandBuilder( true, @@ -1388,6 +1414,7 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman false, scope, logger, + terraformClient, ) ctxs, err := builder.BuildAutoplanCommands(&command.Context{ @@ -1453,6 +1480,7 @@ func TestDefaultProjectCommandBuilder_BuildVersionCommand(t *testing.T) { ApprovedReq: false, UnDivergedReq: false, } + terraformClient := terraform_mocks.NewMockClient() builder := events.NewProjectCommandBuilder( false, @@ -1471,6 +1499,7 @@ func TestDefaultProjectCommandBuilder_BuildVersionCommand(t *testing.T) { false, scope, logger, + terraformClient, ) ctxs, err := builder.BuildVersionCommands( diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 62b9adbf27..1162ed8668 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -7,17 +7,16 @@ import ( "github.com/Masterminds/semver" "github.com/google/uuid" "github.com/hashicorp/go-version" + "github.com/runatlantis/atlantis/server/core/terraform" + "github.com/warrensbox/terraform-switcher/lib" "github.com/hashicorp/terraform-config-inspect/tfconfig" "github.com/runatlantis/atlantis/server/core/config/valid" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" "github.com/uber-go/tally" - lib "github.com/warrensbox/terraform-switcher/lib" ) -var mirrorURL = "https://releases.hashicorp.com/terraform" - func NewProjectCommandContextBuilder(policyCheckEnabled bool, commentBuilder CommentBuilder, scope tally.Scope) ProjectCommandContextBuilder { projectCommandContextBuilder := &DefaultProjectCommandContextBuilder{ CommentBuilder: commentBuilder, @@ -44,7 +43,7 @@ type ProjectCommandContextBuilder interface { prjCfg valid.MergedProjectCfg, commentFlags []string, repoDir string, - automerge, parallelApply, parallelPlan, verbose bool, + automerge, parallelApply, parallelPlan, verbose bool, terraformClient terraform.Client, ) []command.ProjectContext } @@ -63,12 +62,12 @@ func (cb *CommandScopedStatsProjectCommandContextBuilder) BuildProjectContext( prjCfg valid.MergedProjectCfg, commentFlags []string, repoDir string, - automerge, parallelApply, parallelPlan, verbose bool, + automerge, parallelApply, parallelPlan, verbose bool, terraformClient terraform.Client, ) (projectCmds []command.ProjectContext) { cb.ProjectCounter.Inc(1) cmds := cb.ProjectCommandContextBuilder.BuildProjectContext( - ctx, cmdName, prjCfg, commentFlags, repoDir, automerge, parallelApply, parallelPlan, verbose, + ctx, cmdName, prjCfg, commentFlags, repoDir, automerge, parallelApply, parallelPlan, verbose, terraformClient, ) projectCmds = []command.ProjectContext{} @@ -95,7 +94,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( prjCfg valid.MergedProjectCfg, commentFlags []string, repoDir string, - automerge, parallelApply, parallelPlan, verbose bool, + automerge, parallelApply, parallelPlan, verbose bool, terraformClient terraform.Client, ) (projectCmds []command.ProjectContext) { ctx.Log.Debug("Building project command context for %s", cmdName) @@ -115,7 +114,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( // If TerraformVersion not defined in config file look for a // terraform.require_version block. if prjCfg.TerraformVersion == nil { - prjCfg.TerraformVersion = getTfVersion(ctx, filepath.Join(repoDir, prjCfg.RepoRelDir)) + prjCfg.TerraformVersion = getTfVersion(ctx, terraformClient, filepath.Join(repoDir, prjCfg.RepoRelDir)) } projectCmdContext := newProjectCommandContext( @@ -152,14 +151,14 @@ func (cb *PolicyCheckProjectCommandContextBuilder) BuildProjectContext( prjCfg valid.MergedProjectCfg, commentFlags []string, repoDir string, - automerge, parallelApply, parallelPlan, verbose bool, + automerge, parallelApply, parallelPlan, verbose bool, terraformClient terraform.Client, ) (projectCmds []command.ProjectContext) { ctx.Log.Debug("PolicyChecks are enabled") // If TerraformVersion not defined in config file look for a // terraform.require_version block. if prjCfg.TerraformVersion == nil { - prjCfg.TerraformVersion = getTfVersion(ctx, filepath.Join(repoDir, prjCfg.RepoRelDir)) + prjCfg.TerraformVersion = getTfVersion(ctx, terraformClient, filepath.Join(repoDir, prjCfg.RepoRelDir)) } projectCmds = cb.ProjectCommandContextBuilder.BuildProjectContext( @@ -172,6 +171,7 @@ func (cb *PolicyCheckProjectCommandContextBuilder) BuildProjectContext( parallelApply, parallelPlan, verbose, + terraformClient, ) if cmdName == command.Plan { @@ -285,7 +285,7 @@ func escapeArgs(args []string) []string { // Extracts required_version from Terraform configuration. // Returns nil if unable to determine version from configuration. -func getTfVersion(ctx *command.Context, absProjDir string) *version.Version { +func getTfVersion(ctx *command.Context, terraformClient terraform.Client, absProjDir string) *version.Version { module, diags := tfconfig.LoadModule(absProjDir) if diags.HasErrors() { ctx.Log.Err("trying to detect required version: %s", diags.Error()) @@ -298,11 +298,14 @@ func getTfVersion(ctx *command.Context, absProjDir string) *version.Version { requiredVersionSetting := module.RequiredCore[0] ctx.Log.Debug("found required_version setting of %q", requiredVersionSetting) - tflist, _ := lib.GetTFList(mirrorURL, true) + tfVersions, err := terraformClient.ListAvailableVersions(ctx.Log) + if err != nil { + ctx.Log.Err("Unable to list Terraform versions") + } constrains, _ := semver.NewConstraint(requiredVersionSetting) - versions := make([]*semver.Version, len(tflist)) + versions := make([]*semver.Version, len(tfVersions)) - for i, tfvals := range tflist { + for i, tfvals := range tfVersions { version, err := semver.NewVersion(tfvals) //NewVersion parses a given version and returns an instance of Version or an error if unable to parse the version. if err == nil { versions[i] = version diff --git a/server/events/project_command_context_builder_test.go b/server/events/project_command_context_builder_test.go index a9039e24ba..aa90dd90ee 100644 --- a/server/events/project_command_context_builder_test.go +++ b/server/events/project_command_context_builder_test.go @@ -5,6 +5,7 @@ import ( . "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/server/core/config/valid" + terraform_mocks "github.com/runatlantis/atlantis/server/core/terraform/mocks" "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/mocks" @@ -46,6 +47,9 @@ func TestProjectCommandContextBuilder_PullStatus(t *testing.T) { expectedApplyCmt := "Apply Comment" expectedPlanCmt := "Plan Comment" + terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(commandCtx.Log)) + t.Run("with project name defined", func(t *testing.T) { When(mockCommentBuilder.BuildPlanComment(projRepoRelDir, projWorkspace, projName, []string{})).ThenReturn(expectedPlanCmt) When(mockCommentBuilder.BuildApplyComment(projRepoRelDir, projWorkspace, projName, false)).ThenReturn(expectedApplyCmt) @@ -58,7 +62,7 @@ func TestProjectCommandContextBuilder_PullStatus(t *testing.T) { }, } - result := subject.BuildProjectContext(commandCtx, command.Plan, projCfg, []string{}, "some/dir", false, false, false, false) + result := subject.BuildProjectContext(commandCtx, command.Plan, projCfg, []string{}, "some/dir", false, false, false, false, terraformClient) assert.Equal(t, models.ErroredPolicyCheckStatus, result[0].ProjectPlanStatus) }) @@ -77,7 +81,7 @@ func TestProjectCommandContextBuilder_PullStatus(t *testing.T) { }, } - result := subject.BuildProjectContext(commandCtx, command.Plan, projCfg, []string{}, "some/dir", false, false, false, false) + result := subject.BuildProjectContext(commandCtx, command.Plan, projCfg, []string{}, "some/dir", false, false, false, false, terraformClient) assert.Equal(t, models.ErroredPolicyCheckStatus, result[0].ProjectPlanStatus) }) @@ -97,7 +101,7 @@ func TestProjectCommandContextBuilder_PullStatus(t *testing.T) { }, } - result := subject.BuildProjectContext(commandCtx, command.Plan, projCfg, []string{}, "some/dir", false, true, false, false) + result := subject.BuildProjectContext(commandCtx, command.Plan, projCfg, []string{}, "some/dir", false, true, false, false, terraformClient) assert.True(t, result[0].ParallelApplyEnabled) assert.False(t, result[0].ParallelPlanEnabled) diff --git a/server/server.go b/server/server.go index 2e84a37ea6..1da52bc150 100644 --- a/server/server.go +++ b/server/server.go @@ -557,6 +557,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { userConfig.RestrictFileList, statsScope, logger, + terraformClient, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTfVersion) From 08f91c9cb5f8b16e53810bb21ef753acc905abd3 Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Mon, 5 Dec 2022 18:02:27 -0800 Subject: [PATCH 02/14] Add flag to disable Terraform downloads --- server/core/terraform/terraform_client.go | 52 +++++++++++++------ .../core/terraform/terraform_client_test.go | 20 +++---- server/server.go | 1 + server/user_config.go | 1 + 4 files changed, 47 insertions(+), 27 deletions(-) diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index 2914bad284..5d9056513b 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -70,8 +70,9 @@ type DefaultClient struct { // with another binary, ex. echo. overrideTF string // downloader downloads terraform versions. - downloader Downloader - downloadBaseURL string + downloader Downloader + downloadBaseURL string + disableDownloads bool // versions maps from the string representation of a tf version (ex. 0.11.10) // to the absolute path of that binary on disk (if it exists). // Use versionsLock to control access. @@ -114,6 +115,7 @@ func NewClientWithDefaultVersion( defaultVersionFlagName string, tfDownloadURL string, tfDownloader Downloader, + disableDownloads bool, usePluginCache bool, fetchAsync bool, projectCmdOutputHandler jobs.ProjectCommandOutputHandler, @@ -150,7 +152,7 @@ func NewClientWithDefaultVersion( // Since ensureVersion might end up downloading terraform, // we call it asynchronously so as to not delay server startup. versionsLock.Lock() - _, err := ensureVersion(log, tfDownloader, versions, defaultVersion, binDir, tfDownloadURL) + _, err := ensureVersion(log, tfDownloader, versions, defaultVersion, binDir, tfDownloadURL, disableDownloads) versionsLock.Unlock() if err != nil { log.Err("could not download terraform %s: %s", defaultVersion.String(), err) @@ -180,6 +182,7 @@ func NewClientWithDefaultVersion( binDir: binDir, downloader: tfDownloader, downloadBaseURL: tfDownloadURL, + disableDownloads: disableDownloads, versionsLock: &versionsLock, versions: versions, usePluginCache: usePluginCache, @@ -198,6 +201,7 @@ func NewTestClient( defaultVersionFlagName string, tfDownloadURL string, tfDownloader Downloader, + disableDownloads bool, usePluginCache bool, projectCmdOutputHandler jobs.ProjectCommandOutputHandler, ) (*DefaultClient, error) { @@ -211,6 +215,7 @@ func NewTestClient( defaultVersionFlagName, tfDownloadURL, tfDownloader, + disableDownloads, usePluginCache, false, projectCmdOutputHandler, @@ -235,6 +240,7 @@ func NewClient( defaultVersionFlagName string, tfDownloadURL string, tfDownloader Downloader, + disableDownloads bool, usePluginCache bool, projectCmdOutputHandler jobs.ProjectCommandOutputHandler, ) (*DefaultClient, error) { @@ -248,6 +254,7 @@ func NewClient( defaultVersionFlagName, tfDownloadURL, tfDownloader, + disableDownloads, usePluginCache, true, projectCmdOutputHandler, @@ -268,6 +275,12 @@ func (c *DefaultClient) TerraformBinDir() string { // ListAvailableVersions returns all available version of Terraform. If downloads are disabled, this will return an empty list. func (c *DefaultClient) ListAvailableVersions(log logging.SimpleLogging) ([]string, error) { url := fmt.Sprintf("%s/terraform", c.downloadBaseURL) + + if c.disableDownloads { + log.Debug("Terraform downloads disabled. Won't list Terraform versions available at %s", url) + return []string{}, nil + } + log.Debug("Listing Terraform versions available at: %s", url) versions, err := lib.GetTFList(url, true) return versions, err @@ -281,7 +294,7 @@ func (c *DefaultClient) EnsureVersion(log logging.SimpleLogging, v *version.Vers var err error c.versionsLock.Lock() - _, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL) + _, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL, c.disableDownloads) c.versionsLock.Unlock() if err != nil { return err @@ -358,7 +371,7 @@ func (c *DefaultClient) prepCmd(log logging.SimpleLogging, v *version.Version, w } else { var err error c.versionsLock.Lock() - binPath, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL) + binPath, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL, c.disableDownloads) c.versionsLock.Unlock() if err != nil { return "", nil, err @@ -429,7 +442,7 @@ func MustConstraint(v string) version.Constraints { // ensureVersion returns the path to a terraform binary of version v. // It will download this version if we don't have it. -func ensureVersion(log logging.SimpleLogging, dl Downloader, versions map[string]string, v *version.Version, binDir string, downloadURL string) (string, error) { +func ensureVersion(log logging.SimpleLogging, dl Downloader, versions map[string]string, v *version.Version, binDir string, downloadURL string, downloadsDisabled bool) (string, error) { if binPath, ok := versions[v.String()]; ok { return binPath, nil } @@ -450,18 +463,23 @@ func ensureVersion(log logging.SimpleLogging, dl Downloader, versions map[string versions[v.String()] = dest return dest, nil } - log.Info("could not find terraform version %s in PATH or %s, downloading from %s", v.String(), binDir, downloadURL) - urlPrefix := fmt.Sprintf("%s/terraform/%s/terraform_%s", downloadURL, v.String(), v.String()) - binURL := fmt.Sprintf("%s_%s_%s.zip", urlPrefix, runtime.GOOS, runtime.GOARCH) - checksumURL := fmt.Sprintf("%s_SHA256SUMS", urlPrefix) - fullSrcURL := fmt.Sprintf("%s?checksum=file:%s", binURL, checksumURL) - if err := dl.GetFile(dest, fullSrcURL); err != nil { - return "", errors.Wrapf(err, "downloading terraform version %s at %q", v.String(), fullSrcURL) - } + if downloadsDisabled { + return "", fmt.Errorf("could not find terraform version %s in PATH or %s, and downloads are disabled", v.String(), binDir) + } else { - log.Info("downloaded terraform %s to %s", v.String(), dest) - versions[v.String()] = dest - return dest, nil + log.Info("could not find terraform version %s in PATH or %s, downloading from %s", v.String(), binDir, downloadURL) + urlPrefix := fmt.Sprintf("%s/terraform/%s/terraform_%s", downloadURL, v.String(), v.String()) + binURL := fmt.Sprintf("%s_%s_%s.zip", urlPrefix, runtime.GOOS, runtime.GOARCH) + checksumURL := fmt.Sprintf("%s_SHA256SUMS", urlPrefix) + fullSrcURL := fmt.Sprintf("%s?checksum=file:%s", binURL, checksumURL) + if err := dl.GetFile(dest, fullSrcURL); err != nil { + return "", errors.Wrapf(err, "downloading terraform version %s at %q", v.String(), fullSrcURL) + } + + log.Info("downloaded terraform %s to %s", v.String(), dest) + versions[v.String()] = dest + return dest, nil + } } // generateRCFile generates a .terraformrc file containing config for tfeToken diff --git a/server/core/terraform/terraform_client_test.go b/server/core/terraform/terraform_client_test.go index 2ce495f37e..139eb85422 100644 --- a/server/core/terraform/terraform_client_test.go +++ b/server/core/terraform/terraform_client_test.go @@ -76,7 +76,7 @@ is 0.11.13. You can update by downloading from developer.hashicorp.com/terraform Ok(t, err) defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))() - c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, projectCmdOutputHandler) + c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) Ok(t, err) Ok(t, err) @@ -110,7 +110,7 @@ is 0.11.13. You can update by downloading from developer.hashicorp.com/terraform Ok(t, err) defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))() - c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, projectCmdOutputHandler) + c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) Ok(t, err) Ok(t, err) @@ -131,8 +131,8 @@ func TestNewClient_NoTF(t *testing.T) { // Set PATH to only include our empty directory. defer tempSetEnv(t, "PATH", tmp)() - _, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, projectCmdOutputHandler) - ErrEquals(t, "terraform not found in $PATH. Set --default-tf-version or download terraform from https://developer.hashicorp.com/terraform/downloads", err) + _, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) + ErrEquals(t, "terraform not found in $PATH. Set --default-tf-version or download terraform from https://www.terraform.io/downloads.html", err) } // Test that if the default-tf flag is set and that binary is in our PATH @@ -154,7 +154,7 @@ func TestNewClient_DefaultTFFlagInPath(t *testing.T) { Ok(t, err) defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))() - c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, projectCmdOutputHandler) + c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) Ok(t, err) Ok(t, err) @@ -182,7 +182,7 @@ func TestNewClient_DefaultTFFlagInBinDir(t *testing.T) { Ok(t, err) defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))() - c, err := terraform.NewClient(logging.NewNoopLogger(t), binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, projectCmdOutputHandler) + c, err := terraform.NewClient(logging.NewNoopLogger(t), binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) Ok(t, err) Ok(t, err) @@ -214,7 +214,7 @@ func TestNewClient_DefaultTFFlagDownload(t *testing.T) { err := os.WriteFile(params[0].(string), []byte("#!/bin/sh\necho '\nTerraform v0.11.10\n'"), 0700) // #nosec G306 return []pegomock.ReturnValue{err} }) - c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, "https://my-mirror.releases.mycompany.com", mockDownloader, true, projectCmdOutputHandler) + c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, "https://my-mirror.releases.mycompany.com", mockDownloader, false, true, projectCmdOutputHandler) Ok(t, err) Ok(t, err) @@ -240,7 +240,7 @@ func TestNewClient_BadVersion(t *testing.T) { logger := logging.NewNoopLogger(t) _, binDir, cacheDir := mkSubDirs(t) projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler() - _, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "malformed", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, projectCmdOutputHandler) + _, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "malformed", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) ErrEquals(t, "Malformed version: malformed", err) } @@ -269,7 +269,7 @@ func TestRunCommandWithVersion_DLsTF(t *testing.T) { return []pegomock.ReturnValue{err} }) - c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, true, projectCmdOutputHandler) + c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, false, true, projectCmdOutputHandler) Ok(t, err) Equals(t, "0.11.10", c.DefaultVersion().String()) @@ -291,7 +291,7 @@ func TestEnsureVersion_downloaded(t *testing.T) { mockDownloader := mocks.NewMockDownloader() - c, err := terraform.NewTestClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, true, projectCmdOutputHandler) + c, err := terraform.NewTestClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, false, true, projectCmdOutputHandler) Ok(t, err) Equals(t, "0.11.10", c.DefaultVersion().String()) diff --git a/server/server.go b/server/server.go index 1da52bc150..7e40366613 100644 --- a/server/server.go +++ b/server/server.go @@ -393,6 +393,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { config.DefaultTFVersionFlag, userConfig.TFDownloadURL, &terraform.DefaultDownloader{}, + userConfig.TFDisableDownloads, true, projectCmdOutputHandler) // The flag.Lookup call is to detect if we're running in a unit test. If we diff --git a/server/user_config.go b/server/user_config.go index 771e89faf8..2656f930fb 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -100,6 +100,7 @@ type UserConfig struct { SSLKeyFile string `mapstructure:"ssl-key-file"` RestrictFileList bool `mapstructure:"restrict-file-list"` TFDownloadURL string `mapstructure:"tf-download-url"` + TFDisableDownloads bool `mapstructure:"tf-disable-downloads""` TFEHostname string `mapstructure:"tfe-hostname"` TFELocalExecutionMode bool `mapstructure:"tfe-local-execution-mode"` TFEToken string `mapstructure:"tfe-token"` From fceac9f3f2c5a36584ad7ff51410a25ac741b06e Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Tue, 20 Dec 2022 15:51:26 -0800 Subject: [PATCH 03/14] Fallback to exact version matching if unable to list TF versions --- .../events/project_command_context_builder.go | 45 +++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 1162ed8668..3aa0163ef2 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -2,6 +2,7 @@ package events import ( "path/filepath" + "regexp" "sort" "github.com/Masterminds/semver" @@ -283,6 +284,29 @@ func escapeArgs(args []string) []string { return escaped } +/* + if len(module.RequiredCore) != 1 { + ctx.Log.Info("cannot determine which version to use from terraform configuration, detected %d possibilities.", len(module.RequiredCore)) + return nil + } + requiredVersionSetting := module.RequiredCore[0] + + // We allow `= x.y.z`, `=x.y.z` or `x.y.z` where `x`, `y` and `z` are integers. + re := regexp.MustCompile(`^=?\s*([^\s]+)\s*$`) + matched := re.FindStringSubmatch(requiredVersionSetting) + if len(matched) == 0 { + ctx.Log.Debug("did not specify exact version in terraform configuration, found %q", requiredVersionSetting) + return nil + } + ctx.Log.Debug("found required_version setting of %q", requiredVersionSetting) + version, err := version.NewVersion(matched[1]) + if err != nil { + ctx.Log.Debug(err.Error()) + return nil + } + +*/ + // Extracts required_version from Terraform configuration. // Returns nil if unable to determine version from configuration. func getTfVersion(ctx *command.Context, terraformClient terraform.Client, absProjDir string) *version.Version { @@ -302,13 +326,26 @@ func getTfVersion(ctx *command.Context, terraformClient terraform.Client, absPro if err != nil { ctx.Log.Err("Unable to list Terraform versions") } - constrains, _ := semver.NewConstraint(requiredVersionSetting) + + if len(tfVersions) == 0 { + // Fall back to an exact required version string + // We allow `= x.y.z`, `=x.y.z` or `x.y.z` where `x`, `y` and `z` are integers. + re := regexp.MustCompile(`^=?\s*([^\s]+)\s*$`) + matched := re.FindStringSubmatch(requiredVersionSetting) + if len(matched) == 0 { + ctx.Log.Debug("Did not specify exact version in terraform configuration, found %q", requiredVersionSetting) + return nil + } + tfVersions = []string{matched[1]} + } + + constraint, _ := semver.NewConstraint(requiredVersionSetting) versions := make([]*semver.Version, len(tfVersions)) for i, tfvals := range tfVersions { - version, err := semver.NewVersion(tfvals) //NewVersion parses a given version and returns an instance of Version or an error if unable to parse the version. + newVersion, err := semver.NewVersion(tfvals) //NewVersion parses a given version and returns an instance of Version or an error if unable to parse the version. if err == nil { - versions[i] = version + versions[i] = newVersion } } @@ -320,7 +357,7 @@ func getTfVersion(ctx *command.Context, terraformClient terraform.Client, absPro sort.Sort(sort.Reverse(semver.Collection(versions))) for _, element := range versions { - if constrains.Check(element) { // Validate a version against a constraint + if constraint.Check(element) { // Validate a version against a constraint tfversionStr := element.String() if lib.ValidVersionFormat(tfversionStr) { //check if version format is correct tfversion, _ := version.NewVersion(tfversionStr) From c334cc4caba8a411011be305468c1b14812c6be3 Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Tue, 20 Dec 2022 15:34:34 -0800 Subject: [PATCH 04/14] Add new config option to documentation --- runatlantis.io/docs/server-configuration.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/runatlantis.io/docs/server-configuration.md b/runatlantis.io/docs/server-configuration.md index 2af2e8b188..9fa623505e 100644 --- a/runatlantis.io/docs/server-configuration.md +++ b/runatlantis.io/docs/server-configuration.md @@ -927,6 +927,13 @@ and set `--autoplan-modules` to `false`. environment where releases.hashicorp.com is not available. Directory structure of the custom endpoint should match that of releases.hashicorp.com. +### `--tf-disable-downloads` + ```bash + atlantis server --tf-disable-downloads + ``` + Prevent Atlantis from trying to list and download additional versions of Terraform. + Useful in an airgapped environment where a download mirror is not available. + ### `--tfe-hostname` ```bash atlantis server --tfe-hostname="my-terraform-enterprise.company.com" From e10ed6dbb9adbd86533d16ef334410def8063bba Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Tue, 20 Dec 2022 18:51:13 -0800 Subject: [PATCH 05/14] Fix and update tests --- .../events/events_controller_e2e_test.go | 3 +- .../core/terraform/terraform_client_test.go | 28 ++- server/events/project_command_builder_test.go | 232 ++++++++++++++++++ .../events/project_command_context_builder.go | 2 +- 4 files changed, 261 insertions(+), 4 deletions(-) diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 45d68de0ce..e5c7f8cc1a 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -914,7 +914,7 @@ func setupE2E(t *testing.T, repoDir, repoConfigFile string) (events_controllers. GitlabUser: "gitlab-user", ExecutableName: "atlantis", } - terraformClient, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", "default-tf-version", "https://releases.hashicorp.com", &NoopTFDownloader{}, false, projectCmdOutputHandler) + terraformClient, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", "default-tf-version", "https://releases.hashicorp.com", &NoopTFDownloader{}, false, false, projectCmdOutputHandler) Ok(t, err) boltdb, err := db.New(dataDir) Ok(t, err) @@ -1010,6 +1010,7 @@ func setupE2E(t *testing.T, repoDir, repoConfigFile string) (events_controllers. false, statsScope, logger, + terraformClient, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTFVersion) diff --git a/server/core/terraform/terraform_client_test.go b/server/core/terraform/terraform_client_test.go index 139eb85422..a238c0681f 100644 --- a/server/core/terraform/terraform_client_test.go +++ b/server/core/terraform/terraform_client_test.go @@ -132,7 +132,7 @@ func TestNewClient_NoTF(t *testing.T) { defer tempSetEnv(t, "PATH", tmp)() _, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) - ErrEquals(t, "terraform not found in $PATH. Set --default-tf-version or download terraform from https://www.terraform.io/downloads.html", err) + ErrEquals(t, "terraform not found in $PATH. Set --default-tf-version or download terraform from https://developer.hashicorp.com/terraform/downloads", err) } // Test that if the default-tf flag is set and that binary is in our PATH @@ -282,7 +282,7 @@ func TestRunCommandWithVersion_DLsTF(t *testing.T) { Equals(t, "\nTerraform v99.99.99\n\n", output) } -// Test the EnsureVersion downloads terraform. +// Test that EnsureVersion downloads terraform. func TestEnsureVersion_downloaded(t *testing.T) { logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) @@ -312,6 +312,30 @@ func TestEnsureVersion_downloaded(t *testing.T) { mockDownloader.VerifyWasCalledEventually(Once(), 2*time.Second).GetFile(filepath.Join(tmp, "bin", "terraform99.99.99"), expURL) } +// Test that EnsureVersion throws an error when downloads are disabled +func TestEnsureVersion_downloaded_downloadingDisabled(t *testing.T) { + logger := logging.NewNoopLogger(t) + RegisterMockTestingT(t) + _, binDir, cacheDir := mkSubDirs(t) + projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler() + + mockDownloader := mocks.NewMockDownloader() + + disableDownloads := true + c, err := terraform.NewTestClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, disableDownloads, true, projectCmdOutputHandler) + Ok(t, err) + + Equals(t, "0.11.10", c.DefaultVersion().String()) + + v, err := version.NewVersion("99.99.99") + Ok(t, err) + + err = c.EnsureVersion(logger, v) + ErrContains(t, "could not find terraform version", err) + ErrContains(t, "downloads are disabled", err) + mockDownloader.VerifyWasCalled(Never()) +} + // tempSetEnv sets env var key to value. It returns a function that when called // will reset the env var to its original value. func tempSetEnv(t *testing.T, key string, value string) func() { diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 2d5e87413e..319761489f 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -125,6 +125,7 @@ projects: scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) for _, c := range cases { t.Run(c.Description, func(t *testing.T) { @@ -420,6 +421,7 @@ projects: } terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) builder := events.NewProjectCommandBuilder( false, @@ -596,6 +598,7 @@ projects: } terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) builder := events.NewProjectCommandBuilder( false, @@ -769,6 +772,7 @@ projects: } terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) builder := events.NewProjectCommandBuilder( false, @@ -864,6 +868,7 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) { scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) builder := events.NewProjectCommandBuilder( false, @@ -952,6 +957,7 @@ projects: logger := logging.NewNoopLogger(t) scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) builder := events.NewProjectCommandBuilder( false, @@ -1035,6 +1041,7 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { } terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) builder := events.NewProjectCommandBuilder( false, @@ -1241,7 +1248,229 @@ projects: ApprovedReq: false, UnDivergedReq: false, } + + var versions []string + for i := 0; i < 32; i++ { + versions = append(versions, fmt.Sprintf("0.12.%d", i)) + } + terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn(versions, nil) + + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + valid.NewGlobalCfgFromArgs(globalCfgArgs), + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + false, + false, + "", + "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl,**/.terraform.lock.hcl", + false, + scope, + logger, + terraformClient, + ) + + actCtxs, err := builder.BuildPlanCommands( + &command.Context{ + Log: logger, + Scope: scope, + }, + &events.CommentCommand{ + RepoRelDir: "", + Flags: nil, + Name: command.Plan, + Verbose: false, + }) + + Ok(t, err) + Equals(t, len(testCase.Exp), len(actCtxs)) + for _, actCtx := range actCtxs { + if testCase.Exp[actCtx.RepoRelDir] != nil { + Assert(t, actCtx.TerraformVersion != nil, "TerraformVersion is nil.") + Equals(t, testCase.Exp[actCtx.RepoRelDir], actCtx.TerraformVersion.Segments()) + } else { + Assert(t, actCtx.TerraformVersion == nil, "TerraformVersion is supposed to be nil.") + } + } + }) + } +} + +// If TF downloads are disabled, test that terraform version is used when specified in terraform configuration only if an exact version +func TestDefaultProjectCommandBuilder_TerraformVersion_DownloadsDisabled(t *testing.T) { + // For the following tests: + // If terraform configuration is used, result should be `0.12.8`. + // If project configuration is used, result should be `0.12.6`. + // If an inexact version is used, the result should be `nil` + // If default is to be used, result should be `nil`. + + baseVersionConfig := ` +terraform { + required_version = "%s0.12.8" +} +` + + atlantisYamlContent := ` +version: 3 +projects: +- dir: project1 # project1 uses the defaults + terraform_version: v0.12.6 +` + + exactSymbols := []string{"", "="} + // Depending on when the tests are run, the > and >= matching versions will have to be increased. + // It's probably not worth testing the terraform-switcher version here so we only test <, <=, and ~>. + // One way to test this in the future is to mock tfswitcher.GetTFList() to return the highest + // version of 1.3.5. + // nonExactSymbols := []string{">", ">=", "<", "<=", "~>"} + nonExactSymbols := []string{"<", "<=", "~>"} + nonExactVersions := map[string]map[string][]int{ + // ">": { + // "project1": nil, + // }, + // ">=": { + // "project1": nil, + // }, + "<": { + "project1": nil, + }, + "<=": { + "project1": nil, + }, + "~>": { + "project1": nil, + }, + } + + type testCase struct { + DirStructure map[string]interface{} + AtlantisYAML string + ModifiedFiles []string + Exp map[string][]int + } + + testCases := make(map[string]testCase) + + for _, exactSymbol := range exactSymbols { + testCases[fmt.Sprintf("exact version in terraform config using \"%s\"", exactSymbol)] = testCase{ + DirStructure: map[string]interface{}{ + "project1": map[string]interface{}{ + "main.tf": fmt.Sprintf(baseVersionConfig, exactSymbol), + }, + }, + ModifiedFiles: []string{"project1/main.tf"}, + Exp: map[string][]int{ + "project1": {0, 12, 8}, + }, + } + } + + for _, nonExactSymbol := range nonExactSymbols { + testCases[fmt.Sprintf("non-exact version in terraform config using \"%s\"", nonExactSymbol)] = testCase{ + DirStructure: map[string]interface{}{ + "project1": map[string]interface{}{ + "main.tf": fmt.Sprintf(baseVersionConfig, nonExactSymbol), + }, + }, + ModifiedFiles: []string{"project1/main.tf"}, + Exp: nonExactVersions[nonExactSymbol], + } + } + + // atlantis.yaml should take precedence over terraform config + testCases["with project config and terraform config"] = testCase{ + DirStructure: map[string]interface{}{ + "project1": map[string]interface{}{ + "main.tf": fmt.Sprintf(baseVersionConfig, exactSymbols[0]), + }, + valid.DefaultAtlantisFile: atlantisYamlContent, + }, + ModifiedFiles: []string{"project1/main.tf", "project2/main.tf"}, + Exp: map[string][]int{ + "project1": {0, 12, 6}, + }, + } + + testCases["with project config only"] = testCase{ + DirStructure: map[string]interface{}{ + "project1": map[string]interface{}{ + "main.tf": nil, + }, + valid.DefaultAtlantisFile: atlantisYamlContent, + }, + ModifiedFiles: []string{"project1/main.tf"}, + Exp: map[string][]int{ + "project1": {0, 12, 6}, + }, + } + + testCases["neither project config or terraform config"] = testCase{ + DirStructure: map[string]interface{}{ + "project1": map[string]interface{}{ + "main.tf": nil, + }, + }, + ModifiedFiles: []string{"project1/main.tf", "project2/main.tf"}, + Exp: map[string][]int{ + "project1": nil, + }, + } + + testCases["project with different terraform config"] = testCase{ + DirStructure: map[string]interface{}{ + "project1": map[string]interface{}{ + "main.tf": fmt.Sprintf(baseVersionConfig, exactSymbols[0]), + }, + "project2": map[string]interface{}{ + "main.tf": strings.Replace(fmt.Sprintf(baseVersionConfig, exactSymbols[0]), "0.12.8", "0.12.9", -1), + }, + }, + ModifiedFiles: []string{"project1/main.tf", "project2/main.tf"}, + Exp: map[string][]int{ + "project1": {0, 12, 8}, + "project2": {0, 12, 9}, + }, + } + + logger := logging.NewNoopLogger(t) + scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") + + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + RegisterMockTestingT(t) + + tmpDir := DirStructure(t, testCase.DirStructure) + + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetModifiedFiles(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())).ThenReturn(testCase.ModifiedFiles, nil) + + workingDir := mocks.NewMockWorkingDir() + When(workingDir.Clone( + matchers.AnyPtrToLoggingSimpleLogger(), + matchers.AnyModelsRepo(), + matchers.AnyModelsPullRequest(), + AnyString())).ThenReturn(tmpDir, false, nil) + + When(workingDir.GetWorkingDir( + matchers.AnyModelsRepo(), + matchers.AnyModelsPullRequest(), + AnyString())).ThenReturn(tmpDir, nil) + + globalCfgArgs := valid.GlobalCfgArgs{ + AllowRepoCfg: true, + MergeableReq: false, + ApprovedReq: false, + UnDivergedReq: false, + } + terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) builder := events.NewProjectCommandBuilder( false, @@ -1334,6 +1563,7 @@ parallel_plan: true`, } scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) builder := events.NewProjectCommandBuilder( false, @@ -1396,6 +1626,7 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman globalCfg := valid.NewGlobalCfgFromArgs(globalCfgArgs) terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) builder := events.NewProjectCommandBuilder( true, @@ -1481,6 +1712,7 @@ func TestDefaultProjectCommandBuilder_BuildVersionCommand(t *testing.T) { UnDivergedReq: false, } terraformClient := terraform_mocks.NewMockClient() + When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) builder := events.NewProjectCommandBuilder( false, diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 3aa0163ef2..d2f669ed0e 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -330,7 +330,7 @@ func getTfVersion(ctx *command.Context, terraformClient terraform.Client, absPro if len(tfVersions) == 0 { // Fall back to an exact required version string // We allow `= x.y.z`, `=x.y.z` or `x.y.z` where `x`, `y` and `z` are integers. - re := regexp.MustCompile(`^=?\s*([^\s]+)\s*$`) + re := regexp.MustCompile(`^=?\s*([0-9.]+)\s*$`) matched := re.FindStringSubmatch(requiredVersionSetting) if len(matched) == 0 { ctx.Log.Debug("Did not specify exact version in terraform configuration, found %q", requiredVersionSetting) From 54947f659b475ba52647f54a03236fcabf6db990 Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Tue, 20 Dec 2022 18:57:42 -0800 Subject: [PATCH 06/14] Fix check-lint errors --- server/core/terraform/terraform_client.go | 25 +++++++++++------------ server/user_config.go | 2 +- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index 5d9056513b..a848f04940 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -465,21 +465,20 @@ func ensureVersion(log logging.SimpleLogging, dl Downloader, versions map[string } if downloadsDisabled { return "", fmt.Errorf("could not find terraform version %s in PATH or %s, and downloads are disabled", v.String(), binDir) - } else { - - log.Info("could not find terraform version %s in PATH or %s, downloading from %s", v.String(), binDir, downloadURL) - urlPrefix := fmt.Sprintf("%s/terraform/%s/terraform_%s", downloadURL, v.String(), v.String()) - binURL := fmt.Sprintf("%s_%s_%s.zip", urlPrefix, runtime.GOOS, runtime.GOARCH) - checksumURL := fmt.Sprintf("%s_SHA256SUMS", urlPrefix) - fullSrcURL := fmt.Sprintf("%s?checksum=file:%s", binURL, checksumURL) - if err := dl.GetFile(dest, fullSrcURL); err != nil { - return "", errors.Wrapf(err, "downloading terraform version %s at %q", v.String(), fullSrcURL) - } + } - log.Info("downloaded terraform %s to %s", v.String(), dest) - versions[v.String()] = dest - return dest, nil + log.Info("could not find terraform version %s in PATH or %s, downloading from %s", v.String(), binDir, downloadURL) + urlPrefix := fmt.Sprintf("%s/terraform/%s/terraform_%s", downloadURL, v.String(), v.String()) + binURL := fmt.Sprintf("%s_%s_%s.zip", urlPrefix, runtime.GOOS, runtime.GOARCH) + checksumURL := fmt.Sprintf("%s_SHA256SUMS", urlPrefix) + fullSrcURL := fmt.Sprintf("%s?checksum=file:%s", binURL, checksumURL) + if err := dl.GetFile(dest, fullSrcURL); err != nil { + return "", errors.Wrapf(err, "downloading terraform version %s at %q", v.String(), fullSrcURL) } + + log.Info("downloaded terraform %s to %s", v.String(), dest) + versions[v.String()] = dest + return dest, nil } // generateRCFile generates a .terraformrc file containing config for tfeToken diff --git a/server/user_config.go b/server/user_config.go index 2656f930fb..7398f0c3c8 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -100,7 +100,7 @@ type UserConfig struct { SSLKeyFile string `mapstructure:"ssl-key-file"` RestrictFileList bool `mapstructure:"restrict-file-list"` TFDownloadURL string `mapstructure:"tf-download-url"` - TFDisableDownloads bool `mapstructure:"tf-disable-downloads""` + TFDisableDownloads bool `mapstructure:"tf-disable-downloads"` TFEHostname string `mapstructure:"tfe-hostname"` TFELocalExecutionMode bool `mapstructure:"tfe-local-execution-mode"` TFEToken string `mapstructure:"tfe-token"` From 6ae09a25fd15c4e604fb4467f76dd91b10a9f91f Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Wed, 21 Dec 2022 09:41:09 -0800 Subject: [PATCH 07/14] Change option name --- cmd/server.go | 6 +++++ runatlantis.io/docs/server-configuration.md | 18 ++++++++----- .../events/events_controller_e2e_test.go | 2 +- server/core/terraform/terraform_client.go | 26 +++++++++---------- .../core/terraform/terraform_client_test.go | 22 ++++++++-------- server/server.go | 2 +- server/user_config.go | 2 +- 7 files changed, 44 insertions(+), 34 deletions(-) diff --git a/cmd/server.go b/cmd/server.go index 506b537608..ad07254e49 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -117,6 +117,7 @@ const ( SSLCertFileFlag = "ssl-cert-file" SSLKeyFileFlag = "ssl-key-file" RestrictFileList = "restrict-file-list" + TFDownloadFlag = "tf-download" TFDownloadURLFlag = "tf-download-url" VarFileAllowlistFlag = "var-file-allowlist" VCSStatusName = "vcs-status-name" @@ -151,6 +152,7 @@ const ( DefaultRedisTLSEnabled = false DefaultRedisInsecureSkipVerify = false DefaultTFDownloadURL = "https://releases.hashicorp.com" + DefaultTFDownload = true DefaultTFEHostname = "app.terraform.io" DefaultVCSStatusName = "atlantis" DefaultWebBasicAuth = false @@ -500,6 +502,10 @@ var boolFlags = map[string]boolFlag{ description: "Skips cloning the PR repo if there are no projects were changed in the PR.", defaultValue: false, }, + TFDownloadFlag: { + description: "Allow Atlantis to list & download Terraform versions. Setting this to false can be helpful in air-gapped environments.", + defaultValue: DefaultTFDownload, + }, TFELocalExecutionModeFlag: { description: "Enable if you're using local execution mode (instead of TFE/C's remote execution mode).", defaultValue: false, diff --git a/runatlantis.io/docs/server-configuration.md b/runatlantis.io/docs/server-configuration.md index 9fa623505e..8e2a1a97e5 100644 --- a/runatlantis.io/docs/server-configuration.md +++ b/runatlantis.io/docs/server-configuration.md @@ -917,6 +917,15 @@ and set `--autoplan-modules` to `false`. ``` Namespace for emitting stats/metrics. See [stats](stats.html) section. +### `--tf--download` + ```bash + atlantis server --tf-download=false + # or + ATLANTIS_TF_DOWNLOAD=false + ``` +Defaults to `true`. Allow Atlantis to list and download additional versions of Terraform. +Setting this to `false` can be useful in an air-gapped environment where a download mirror is not available. + ### `--tf-download-url` ```bash atlantis server --tf-download-url="https://releases.company.com" @@ -926,13 +935,8 @@ and set `--autoplan-modules` to `false`. An alternative URL to download Terraform versions if they are missing. Useful in an airgapped environment where releases.hashicorp.com is not available. Directory structure of the custom endpoint should match that of releases.hashicorp.com. - -### `--tf-disable-downloads` - ```bash - atlantis server --tf-disable-downloads - ``` - Prevent Atlantis from trying to list and download additional versions of Terraform. - Useful in an airgapped environment where a download mirror is not available. + + This has no impact if `--tf-download` is set to `false`. ### `--tfe-hostname` ```bash diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index e5c7f8cc1a..b6aeed6397 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -914,7 +914,7 @@ func setupE2E(t *testing.T, repoDir, repoConfigFile string) (events_controllers. GitlabUser: "gitlab-user", ExecutableName: "atlantis", } - terraformClient, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", "default-tf-version", "https://releases.hashicorp.com", &NoopTFDownloader{}, false, false, projectCmdOutputHandler) + terraformClient, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", "default-tf-version", "https://releases.hashicorp.com", &NoopTFDownloader{}, true, false, projectCmdOutputHandler) Ok(t, err) boltdb, err := db.New(dataDir) Ok(t, err) diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index a848f04940..33726f5234 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -72,7 +72,7 @@ type DefaultClient struct { // downloader downloads terraform versions. downloader Downloader downloadBaseURL string - disableDownloads bool + downloadsAllowed bool // versions maps from the string representation of a tf version (ex. 0.11.10) // to the absolute path of that binary on disk (if it exists). // Use versionsLock to control access. @@ -115,7 +115,7 @@ func NewClientWithDefaultVersion( defaultVersionFlagName string, tfDownloadURL string, tfDownloader Downloader, - disableDownloads bool, + allowDownloads bool, usePluginCache bool, fetchAsync bool, projectCmdOutputHandler jobs.ProjectCommandOutputHandler, @@ -152,7 +152,7 @@ func NewClientWithDefaultVersion( // Since ensureVersion might end up downloading terraform, // we call it asynchronously so as to not delay server startup. versionsLock.Lock() - _, err := ensureVersion(log, tfDownloader, versions, defaultVersion, binDir, tfDownloadURL, disableDownloads) + _, err := ensureVersion(log, tfDownloader, versions, defaultVersion, binDir, tfDownloadURL, allowDownloads) versionsLock.Unlock() if err != nil { log.Err("could not download terraform %s: %s", defaultVersion.String(), err) @@ -182,7 +182,7 @@ func NewClientWithDefaultVersion( binDir: binDir, downloader: tfDownloader, downloadBaseURL: tfDownloadURL, - disableDownloads: disableDownloads, + downloadsAllowed: allowDownloads, versionsLock: &versionsLock, versions: versions, usePluginCache: usePluginCache, @@ -201,7 +201,7 @@ func NewTestClient( defaultVersionFlagName string, tfDownloadURL string, tfDownloader Downloader, - disableDownloads bool, + downloadsAllowed bool, usePluginCache bool, projectCmdOutputHandler jobs.ProjectCommandOutputHandler, ) (*DefaultClient, error) { @@ -215,7 +215,7 @@ func NewTestClient( defaultVersionFlagName, tfDownloadURL, tfDownloader, - disableDownloads, + downloadsAllowed, usePluginCache, false, projectCmdOutputHandler, @@ -240,7 +240,7 @@ func NewClient( defaultVersionFlagName string, tfDownloadURL string, tfDownloader Downloader, - disableDownloads bool, + allowDownloads bool, usePluginCache bool, projectCmdOutputHandler jobs.ProjectCommandOutputHandler, ) (*DefaultClient, error) { @@ -254,7 +254,7 @@ func NewClient( defaultVersionFlagName, tfDownloadURL, tfDownloader, - disableDownloads, + allowDownloads, usePluginCache, true, projectCmdOutputHandler, @@ -276,7 +276,7 @@ func (c *DefaultClient) TerraformBinDir() string { func (c *DefaultClient) ListAvailableVersions(log logging.SimpleLogging) ([]string, error) { url := fmt.Sprintf("%s/terraform", c.downloadBaseURL) - if c.disableDownloads { + if !c.downloadsAllowed { log.Debug("Terraform downloads disabled. Won't list Terraform versions available at %s", url) return []string{}, nil } @@ -294,7 +294,7 @@ func (c *DefaultClient) EnsureVersion(log logging.SimpleLogging, v *version.Vers var err error c.versionsLock.Lock() - _, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL, c.disableDownloads) + _, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL, c.downloadsAllowed) c.versionsLock.Unlock() if err != nil { return err @@ -371,7 +371,7 @@ func (c *DefaultClient) prepCmd(log logging.SimpleLogging, v *version.Version, w } else { var err error c.versionsLock.Lock() - binPath, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL, c.disableDownloads) + binPath, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL, c.downloadsAllowed) c.versionsLock.Unlock() if err != nil { return "", nil, err @@ -442,7 +442,7 @@ func MustConstraint(v string) version.Constraints { // ensureVersion returns the path to a terraform binary of version v. // It will download this version if we don't have it. -func ensureVersion(log logging.SimpleLogging, dl Downloader, versions map[string]string, v *version.Version, binDir string, downloadURL string, downloadsDisabled bool) (string, error) { +func ensureVersion(log logging.SimpleLogging, dl Downloader, versions map[string]string, v *version.Version, binDir string, downloadURL string, downloadsAllowed bool) (string, error) { if binPath, ok := versions[v.String()]; ok { return binPath, nil } @@ -463,7 +463,7 @@ func ensureVersion(log logging.SimpleLogging, dl Downloader, versions map[string versions[v.String()] = dest return dest, nil } - if downloadsDisabled { + if !downloadsAllowed { return "", fmt.Errorf("could not find terraform version %s in PATH or %s, and downloads are disabled", v.String(), binDir) } diff --git a/server/core/terraform/terraform_client_test.go b/server/core/terraform/terraform_client_test.go index a238c0681f..0de0beb97a 100644 --- a/server/core/terraform/terraform_client_test.go +++ b/server/core/terraform/terraform_client_test.go @@ -76,7 +76,7 @@ is 0.11.13. You can update by downloading from developer.hashicorp.com/terraform Ok(t, err) defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))() - c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) + c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, true, projectCmdOutputHandler) Ok(t, err) Ok(t, err) @@ -110,7 +110,7 @@ is 0.11.13. You can update by downloading from developer.hashicorp.com/terraform Ok(t, err) defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))() - c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) + c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, true, projectCmdOutputHandler) Ok(t, err) Ok(t, err) @@ -131,7 +131,7 @@ func TestNewClient_NoTF(t *testing.T) { // Set PATH to only include our empty directory. defer tempSetEnv(t, "PATH", tmp)() - _, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) + _, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, true, projectCmdOutputHandler) ErrEquals(t, "terraform not found in $PATH. Set --default-tf-version or download terraform from https://developer.hashicorp.com/terraform/downloads", err) } @@ -182,7 +182,7 @@ func TestNewClient_DefaultTFFlagInBinDir(t *testing.T) { Ok(t, err) defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))() - c, err := terraform.NewClient(logging.NewNoopLogger(t), binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) + c, err := terraform.NewClient(logging.NewNoopLogger(t), binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, true, projectCmdOutputHandler) Ok(t, err) Ok(t, err) @@ -214,7 +214,7 @@ func TestNewClient_DefaultTFFlagDownload(t *testing.T) { err := os.WriteFile(params[0].(string), []byte("#!/bin/sh\necho '\nTerraform v0.11.10\n'"), 0700) // #nosec G306 return []pegomock.ReturnValue{err} }) - c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, "https://my-mirror.releases.mycompany.com", mockDownloader, false, true, projectCmdOutputHandler) + c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, "https://my-mirror.releases.mycompany.com", mockDownloader, true, true, projectCmdOutputHandler) Ok(t, err) Ok(t, err) @@ -240,7 +240,7 @@ func TestNewClient_BadVersion(t *testing.T) { logger := logging.NewNoopLogger(t) _, binDir, cacheDir := mkSubDirs(t) projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler() - _, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "malformed", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, false, true, projectCmdOutputHandler) + _, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "malformed", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, true, projectCmdOutputHandler) ErrEquals(t, "Malformed version: malformed", err) } @@ -269,7 +269,7 @@ func TestRunCommandWithVersion_DLsTF(t *testing.T) { return []pegomock.ReturnValue{err} }) - c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, false, true, projectCmdOutputHandler) + c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, true, true, projectCmdOutputHandler) Ok(t, err) Equals(t, "0.11.10", c.DefaultVersion().String()) @@ -290,8 +290,8 @@ func TestEnsureVersion_downloaded(t *testing.T) { projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler() mockDownloader := mocks.NewMockDownloader() - - c, err := terraform.NewTestClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, false, true, projectCmdOutputHandler) + downloadsAllowed := true + c, err := terraform.NewTestClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, downloadsAllowed, true, projectCmdOutputHandler) Ok(t, err) Equals(t, "0.11.10", c.DefaultVersion().String()) @@ -321,8 +321,8 @@ func TestEnsureVersion_downloaded_downloadingDisabled(t *testing.T) { mockDownloader := mocks.NewMockDownloader() - disableDownloads := true - c, err := terraform.NewTestClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, disableDownloads, true, projectCmdOutputHandler) + downloadsAllowed := false + c, err := terraform.NewTestClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, downloadsAllowed, true, projectCmdOutputHandler) Ok(t, err) Equals(t, "0.11.10", c.DefaultVersion().String()) diff --git a/server/server.go b/server/server.go index 7e40366613..c37328157a 100644 --- a/server/server.go +++ b/server/server.go @@ -393,7 +393,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { config.DefaultTFVersionFlag, userConfig.TFDownloadURL, &terraform.DefaultDownloader{}, - userConfig.TFDisableDownloads, + userConfig.TFDownload, true, projectCmdOutputHandler) // The flag.Lookup call is to detect if we're running in a unit test. If we diff --git a/server/user_config.go b/server/user_config.go index 7398f0c3c8..4203ca6139 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -99,8 +99,8 @@ type UserConfig struct { SSLCertFile string `mapstructure:"ssl-cert-file"` SSLKeyFile string `mapstructure:"ssl-key-file"` RestrictFileList bool `mapstructure:"restrict-file-list"` + TFDownload bool `mapstructure:"tf-download"` TFDownloadURL string `mapstructure:"tf-download-url"` - TFDisableDownloads bool `mapstructure:"tf-disable-downloads"` TFEHostname string `mapstructure:"tfe-hostname"` TFELocalExecutionMode bool `mapstructure:"tfe-local-execution-mode"` TFEToken string `mapstructure:"tfe-token"` From cf69a50bd6fbf3071e11039066a910b24f48556e Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Wed, 21 Dec 2022 09:53:44 -0800 Subject: [PATCH 08/14] Remove obsolete commented code --- .../events/project_command_context_builder.go | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index d2f669ed0e..677faa4c76 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -284,29 +284,6 @@ func escapeArgs(args []string) []string { return escaped } -/* - if len(module.RequiredCore) != 1 { - ctx.Log.Info("cannot determine which version to use from terraform configuration, detected %d possibilities.", len(module.RequiredCore)) - return nil - } - requiredVersionSetting := module.RequiredCore[0] - - // We allow `= x.y.z`, `=x.y.z` or `x.y.z` where `x`, `y` and `z` are integers. - re := regexp.MustCompile(`^=?\s*([^\s]+)\s*$`) - matched := re.FindStringSubmatch(requiredVersionSetting) - if len(matched) == 0 { - ctx.Log.Debug("did not specify exact version in terraform configuration, found %q", requiredVersionSetting) - return nil - } - ctx.Log.Debug("found required_version setting of %q", requiredVersionSetting) - version, err := version.NewVersion(matched[1]) - if err != nil { - ctx.Log.Debug(err.Error()) - return nil - } - -*/ - // Extracts required_version from Terraform configuration. // Returns nil if unable to determine version from configuration. func getTfVersion(ctx *command.Context, terraformClient terraform.Client, absProjDir string) *version.Version { From 2a3673833e4d4e6c32cc85e163f34b73ab29ca26 Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Wed, 21 Dec 2022 11:26:30 -0800 Subject: [PATCH 09/14] Migrate version detection logic into terraform client --- .../terraform/mocks/mock_terraform_client.go | 46 +++ server/core/terraform/terraform_client.go | 72 +++- .../core/terraform/terraform_client_test.go | 150 +++++++++ server/events/project_command_builder_test.go | 312 ++---------------- .../events/project_command_context_builder.go | 76 +---- 5 files changed, 295 insertions(+), 361 deletions(-) diff --git a/server/core/terraform/mocks/mock_terraform_client.go b/server/core/terraform/mocks/mock_terraform_client.go index 655e6d40d9..e6d7006d5f 100644 --- a/server/core/terraform/mocks/mock_terraform_client.go +++ b/server/core/terraform/mocks/mock_terraform_client.go @@ -80,6 +80,21 @@ func (mock *MockClient) ListAvailableVersions(log logging.SimpleLogging) ([]stri return ret0, ret1 } +func (mock *MockClient) DetectVersion(projectDirectory string, log logging.SimpleLogging) *go_version.Version { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockClient().") + } + params := []pegomock.Param{projectDirectory, log} + result := pegomock.GetGenericMockFrom(mock).Invoke("DetectVersion", params, []reflect.Type{reflect.TypeOf((**go_version.Version)(nil)).Elem()}) + var ret0 *go_version.Version + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(*go_version.Version) + } + } + return ret0 +} + func (mock *MockClient) VerifyWasCalledOnce() *VerifierMockClient { return &VerifierMockClient{ mock: mock, @@ -221,3 +236,34 @@ func (c *MockClient_ListAvailableVersions_OngoingVerification) GetAllCapturedArg } return } + +func (verifier *VerifierMockClient) DetectVersion(projectDirectory string, log logging.SimpleLogging) *MockClient_DetectVersion_OngoingVerification { + params := []pegomock.Param{projectDirectory, log} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DetectVersion", params, verifier.timeout) + return &MockClient_DetectVersion_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockClient_DetectVersion_OngoingVerification struct { + mock *MockClient + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockClient_DetectVersion_OngoingVerification) GetCapturedArguments() (string, logging.SimpleLogging) { + projectDirectory, log := c.GetAllCapturedArguments() + return projectDirectory[len(projectDirectory)-1], log[len(log)-1] +} + +func (c *MockClient_DetectVersion_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []logging.SimpleLogging) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]string, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(string) + } + _param1 = make([]logging.SimpleLogging, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(logging.SimpleLogging) + } + } + return +} diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index 33726f5234..5ffd0dff34 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -23,11 +23,14 @@ import ( "path/filepath" "regexp" "runtime" + "sort" "strings" "sync" + "github.com/Masterminds/semver" "github.com/hashicorp/go-getter" "github.com/hashicorp/go-version" + "github.com/hashicorp/terraform-config-inspect/tfconfig" "github.com/mitchellh/go-homedir" "github.com/pkg/errors" "github.com/warrensbox/terraform-switcher/lib" @@ -55,7 +58,11 @@ type Client interface { // EnsureVersion makes sure that terraform version `v` is available to use EnsureVersion(log logging.SimpleLogging, v *version.Version) error + // ListAvailableVersions returns all available version of Terraform, if available; otherwise this will return an empty list. ListAvailableVersions(log logging.SimpleLogging) ([]string, error) + + // DetectVersion Extracts required_version from Terraform configuration in the specified project directory. Returns nil if unable to determine the version. + DetectVersion(projectDirectory string, log logging.SimpleLogging) *version.Version } type DefaultClient struct { @@ -272,7 +279,7 @@ func (c *DefaultClient) TerraformBinDir() string { return c.binDir } -// ListAvailableVersions returns all available version of Terraform. If downloads are disabled, this will return an empty list. +// ListAvailableVersions returns all available version of Terraform. If downloads are not allowed, this will return an empty list. func (c *DefaultClient) ListAvailableVersions(log logging.SimpleLogging) ([]string, error) { url := fmt.Sprintf("%s/terraform", c.downloadBaseURL) @@ -286,6 +293,69 @@ func (c *DefaultClient) ListAvailableVersions(log logging.SimpleLogging) ([]stri return versions, err } +// DetectVersion Extracts required_version from Terraform configuration in the specified project directory. Returns nil if unable to determine the version. +// This will also try to intelligently evaluate non-exact matches by listing the available versions of Terraform and picking the best match. +func (c *DefaultClient) DetectVersion(projectDirectory string, log logging.SimpleLogging) *version.Version { + module, diags := tfconfig.LoadModule(projectDirectory) + if diags.HasErrors() { + log.Err("trying to detect required version: %s", diags.Error()) + } + + if len(module.RequiredCore) != 1 { + log.Info("cannot determine which version to use from terraform configuration, detected %d possibilities.", len(module.RequiredCore)) + return nil + } + requiredVersionSetting := module.RequiredCore[0] + log.Debug("found required_version setting of %q", requiredVersionSetting) + + tfVersions, err := c.ListAvailableVersions(log) + if err != nil { + log.Err("Unable to list Terraform versions, may fall back to default: %s", err) + } + + if len(tfVersions) == 0 { + // Fall back to an exact required version string + // We allow `= x.y.z`, `=x.y.z` or `x.y.z` where `x`, `y` and `z` are integers. + re := regexp.MustCompile(`^=?\s*([0-9.]+)\s*$`) + matched := re.FindStringSubmatch(requiredVersionSetting) + if len(matched) == 0 { + log.Debug("Did not specify exact version in terraform configuration, found %q", requiredVersionSetting) + return nil + } + tfVersions = []string{matched[1]} + } + + constraint, _ := semver.NewConstraint(requiredVersionSetting) + versions := make([]*semver.Version, len(tfVersions)) + + for i, tfvals := range tfVersions { + newVersion, err := semver.NewVersion(tfvals) //NewVersion parses a given version and returns an instance of Version or an error if unable to parse the version. + if err == nil { + versions[i] = newVersion + } + } + + if len(versions) == 0 { + log.Debug("did not specify exact valid version in terraform configuration, found %q", requiredVersionSetting) + return nil + } + + sort.Sort(sort.Reverse(semver.Collection(versions))) + + for _, element := range versions { + if constraint.Check(element) { // Validate a version against a constraint + tfversionStr := element.String() + if lib.ValidVersionFormat(tfversionStr) { //check if version format is correct + tfversion, _ := version.NewVersion(tfversionStr) + log.Info("detected module requires version: %s", tfversionStr) + return tfversion + } + } + } + log.Debug("could not match any valid terraform version with %q", requiredVersionSetting) + return nil +} + // See Client.EnsureVersion. func (c *DefaultClient) EnsureVersion(log logging.SimpleLogging, v *version.Version) error { if v == nil { diff --git a/server/core/terraform/terraform_client_test.go b/server/core/terraform/terraform_client_test.go index 0de0beb97a..5228198a3d 100644 --- a/server/core/terraform/terraform_client_test.go +++ b/server/core/terraform/terraform_client_test.go @@ -18,6 +18,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" "time" @@ -357,3 +358,152 @@ func mkSubDirs(t *testing.T) (string, string, string) { return tmp, binDir, cachedir } + +// If TF downloads are disabled, test that terraform version is used when specified in terraform configuration only if an exact version +func TestDefaultProjectCommandBuilder_TerraformVersion(t *testing.T) { + // For the following tests: + // If terraform configuration is used, result should be `0.12.8`. + // If project configuration is used, result should be `0.12.6`. + // If an inexact version is used, the result should be `nil` + // If default is to be used, result should be `nil`. + + baseVersionConfig := ` +terraform { + required_version = "%s0.12.8" +} +` + + exactSymbols := []string{"", "="} + // Depending on when the tests are run, the > and >= matching versions will have to be increased. + // It's probably not worth testing the terraform-switcher version here so we only test <, <=, and ~>. + // One way to test this in the future is to mock tfswitcher.GetTFList() to return the highest + // version of 1.3.5. + // nonExactSymbols := []string{">", ">=", "<", "<=", "~>"} + nonExactSymbols := []string{"<", "<=", "~>"} + nonExactVersions := map[string]map[string]string{ + // ">": { + // "project1": "1.3.5", + // }, + // ">=": { + // "project1": "1.3.5", + // }, + "<": { + "project1": "0.12.7", + }, + "<=": { + "project1": "0.12.8", + }, + "~>": { + "project1": "0.12.31", + }, + } + + type testCase struct { + DirStructure map[string]interface{} + Exp map[string]string + IsExact bool + } + + testCases := make(map[string]testCase) + + for _, exactSymbol := range exactSymbols { + testCases[fmt.Sprintf("exact version using \"%s\"", exactSymbol)] = testCase{ + DirStructure: map[string]interface{}{ + "project1": map[string]interface{}{ + "main.tf": fmt.Sprintf(baseVersionConfig, exactSymbol), + }, + }, + Exp: map[string]string{ + "project1": "0.12.8", + }, + IsExact: true, + } + } + + for _, nonExactSymbol := range nonExactSymbols { + testCases[fmt.Sprintf("non-exact version using \"%s\"", nonExactSymbol)] = testCase{ + DirStructure: map[string]interface{}{ + "project1": map[string]interface{}{ + "main.tf": fmt.Sprintf(baseVersionConfig, nonExactSymbol), + }, + }, + Exp: nonExactVersions[nonExactSymbol], + IsExact: false, + } + } + + testCases["no version specified"] = testCase{ + DirStructure: map[string]interface{}{ + "project1": map[string]interface{}{ + "main.tf": nil, + }, + }, + Exp: map[string]string{ + "project1": "", + }, + IsExact: true, + } + + testCases["projects with different terraform versions"] = testCase{ + DirStructure: map[string]interface{}{ + "project1": map[string]interface{}{ + "main.tf": fmt.Sprintf(baseVersionConfig, exactSymbols[0]), + }, + "project2": map[string]interface{}{ + "main.tf": strings.Replace(fmt.Sprintf(baseVersionConfig, exactSymbols[0]), "0.12.8", "0.12.9", -1), + }, + }, + Exp: map[string]string{ + "project1": "0.12.8", + "project2": "0.12.9", + }, + IsExact: true, + } + + runDetectVersionTestCase := func(t *testing.T, name string, testCase testCase, downloadsAllowed bool) bool { + return t.Run(name, func(t *testing.T) { + RegisterMockTestingT(t) + + logger := logging.NewNoopLogger(t) + RegisterMockTestingT(t) + _, binDir, cacheDir := mkSubDirs(t) + projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler() + + mockDownloader := mocks.NewMockDownloader() + c, err := terraform.NewTestClient(logger, + binDir, + cacheDir, + "", + "", + "", + cmd.DefaultTFVersionFlag, + cmd.DefaultTFDownloadURL, + mockDownloader, + downloadsAllowed, + true, + projectCmdOutputHandler) + Ok(t, err) + + tmpDir := DirStructure(t, testCase.DirStructure) + + for project, expectedVersion := range testCase.Exp { + detectedVersion := c.DetectVersion(filepath.Join(tmpDir, project), logger) + + expectNil := expectedVersion == "" || (!testCase.IsExact && !downloadsAllowed) + if expectNil { + Assert(t, detectedVersion == nil, "TerraformVersion is supposed to be nil.") + } else { + Assert(t, detectedVersion != nil, "TerraformVersion is nil.") + Ok(t, err) + Equals(t, expectedVersion, detectedVersion.String()) + } + } + + }) + } + + for name, testCase := range testCases { + runDetectVersionTestCase(t, name+": Downloads Allowed", testCase, true) + runDetectVersionTestCase(t, name+": Downloads Disabled", testCase, false) + } +} diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 319761489f..7681849490 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -1,12 +1,12 @@ package events_test import ( - "fmt" "os" "path/filepath" "strings" "testing" + "github.com/hashicorp/go-version" . "github.com/petergtz/pegomock" terraform_mocks "github.com/runatlantis/atlantis/server/core/terraform/mocks" @@ -1092,7 +1092,7 @@ func TestDefaultProjectCommandBuilder_TerraformVersion(t *testing.T) { baseVersionConfig := ` terraform { - required_version = "%s0.12.8" + required_version = "0.12.8" } ` @@ -1103,77 +1103,26 @@ projects: terraform_version: v0.12.6 ` - exactSymbols := []string{"", "="} - // Depending on when the tests are run, the > and >= matching versions will have to be increased. - // It's probably not worth testing the terraform-switcher version here so we only test <, <=, and ~>. - // One way to test this in the future is to mock tfswitcher.GetTFList() to return the highest - // version of 1.3.5. - // nonExactSymbols := []string{">", ">=", "<", "<=", "~>"} - nonExactSymbols := []string{"<", "<=", "~>"} - nonExactVersions := map[string]map[string][]int{ - // ">": { - // "project1": {1, 3, 5}, - // }, - // ">=": { - // "project1": {1, 3, 5}, - // }, - "<": { - "project1": {0, 12, 7}, - }, - "<=": { - "project1": {0, 12, 8}, - }, - "~>": { - "project1": {0, 12, 31}, - }, - } - type testCase struct { DirStructure map[string]interface{} AtlantisYAML string ModifiedFiles []string - Exp map[string][]int + Exp map[string]string } testCases := make(map[string]testCase) - for _, exactSymbol := range exactSymbols { - testCases[fmt.Sprintf("exact version in terraform config using \"%s\"", exactSymbol)] = testCase{ - DirStructure: map[string]interface{}{ - "project1": map[string]interface{}{ - "main.tf": fmt.Sprintf(baseVersionConfig, exactSymbol), - }, - }, - ModifiedFiles: []string{"project1/main.tf"}, - Exp: map[string][]int{ - "project1": {0, 12, 8}, - }, - } - } - - for _, nonExactSymbol := range nonExactSymbols { - testCases[fmt.Sprintf("non-exact version in terraform config using \"%s\"", nonExactSymbol)] = testCase{ - DirStructure: map[string]interface{}{ - "project1": map[string]interface{}{ - "main.tf": fmt.Sprintf(baseVersionConfig, nonExactSymbol), - }, - }, - ModifiedFiles: []string{"project1/main.tf"}, - Exp: nonExactVersions[nonExactSymbol], - } - } - // atlantis.yaml should take precedence over terraform config testCases["with project config and terraform config"] = testCase{ DirStructure: map[string]interface{}{ "project1": map[string]interface{}{ - "main.tf": fmt.Sprintf(baseVersionConfig, exactSymbols[0]), + "main.tf": baseVersionConfig, }, valid.DefaultAtlantisFile: atlantisYamlContent, }, ModifiedFiles: []string{"project1/main.tf", "project2/main.tf"}, - Exp: map[string][]int{ - "project1": {0, 12, 6}, + Exp: map[string]string{ + "project1": "0.12.6", }, } @@ -1185,8 +1134,8 @@ projects: valid.DefaultAtlantisFile: atlantisYamlContent, }, ModifiedFiles: []string{"project1/main.tf"}, - Exp: map[string][]int{ - "project1": {0, 12, 6}, + Exp: map[string]string{ + "project1": "0.12.6", }, } @@ -1197,24 +1146,24 @@ projects: }, }, ModifiedFiles: []string{"project1/main.tf", "project2/main.tf"}, - Exp: map[string][]int{ - "project1": nil, + Exp: map[string]string{ + "project1": "", }, } testCases["project with different terraform config"] = testCase{ DirStructure: map[string]interface{}{ "project1": map[string]interface{}{ - "main.tf": fmt.Sprintf(baseVersionConfig, exactSymbols[0]), + "main.tf": baseVersionConfig, }, "project2": map[string]interface{}{ - "main.tf": strings.Replace(fmt.Sprintf(baseVersionConfig, exactSymbols[0]), "0.12.8", "0.12.9", -1), + "main.tf": strings.Replace(baseVersionConfig, "0.12.8", "0.12.9", -1), }, }, ModifiedFiles: []string{"project1/main.tf", "project2/main.tf"}, - Exp: map[string][]int{ - "project1": {0, 12, 8}, - "project2": {0, 12, 9}, + Exp: map[string]string{ + "project1": "0.12.8", + "project2": "0.12.9", }, } @@ -1249,228 +1198,17 @@ projects: UnDivergedReq: false, } - var versions []string - for i := 0; i < 32; i++ { - versions = append(versions, fmt.Sprintf("0.12.%d", i)) - } terraformClient := terraform_mocks.NewMockClient() - When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn(versions, nil) - builder := events.NewProjectCommandBuilder( - false, - &config.ParserValidator{}, - &events.DefaultProjectFinder{}, - vcsClient, - workingDir, - events.NewDefaultWorkingDirLocker(), - valid.NewGlobalCfgFromArgs(globalCfgArgs), - &events.DefaultPendingPlanFinder{}, - &events.CommentParser{ExecutableName: "atlantis"}, - false, - false, - "", - "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl,**/.terraform.lock.hcl", - false, - scope, - logger, - terraformClient, - ) - - actCtxs, err := builder.BuildPlanCommands( - &command.Context{ - Log: logger, - Scope: scope, - }, - &events.CommentCommand{ - RepoRelDir: "", - Flags: nil, - Name: command.Plan, - Verbose: false, - }) - - Ok(t, err) - Equals(t, len(testCase.Exp), len(actCtxs)) - for _, actCtx := range actCtxs { - if testCase.Exp[actCtx.RepoRelDir] != nil { - Assert(t, actCtx.TerraformVersion != nil, "TerraformVersion is nil.") - Equals(t, testCase.Exp[actCtx.RepoRelDir], actCtx.TerraformVersion.Segments()) - } else { - Assert(t, actCtx.TerraformVersion == nil, "TerraformVersion is supposed to be nil.") + When(terraformClient.DetectVersion(AnyString(), matchers.AnyLoggingSimpleLogging())).Then(func(params []Param) ReturnValues { + projectName := filepath.Base(params[0].(string)) + testVersion := testCase.Exp[projectName] + if testVersion != "" { + v, _ := version.NewVersion(testVersion) + return []ReturnValue{v} } - } - }) - } -} - -// If TF downloads are disabled, test that terraform version is used when specified in terraform configuration only if an exact version -func TestDefaultProjectCommandBuilder_TerraformVersion_DownloadsDisabled(t *testing.T) { - // For the following tests: - // If terraform configuration is used, result should be `0.12.8`. - // If project configuration is used, result should be `0.12.6`. - // If an inexact version is used, the result should be `nil` - // If default is to be used, result should be `nil`. - - baseVersionConfig := ` -terraform { - required_version = "%s0.12.8" -} -` - - atlantisYamlContent := ` -version: 3 -projects: -- dir: project1 # project1 uses the defaults - terraform_version: v0.12.6 -` - - exactSymbols := []string{"", "="} - // Depending on when the tests are run, the > and >= matching versions will have to be increased. - // It's probably not worth testing the terraform-switcher version here so we only test <, <=, and ~>. - // One way to test this in the future is to mock tfswitcher.GetTFList() to return the highest - // version of 1.3.5. - // nonExactSymbols := []string{">", ">=", "<", "<=", "~>"} - nonExactSymbols := []string{"<", "<=", "~>"} - nonExactVersions := map[string]map[string][]int{ - // ">": { - // "project1": nil, - // }, - // ">=": { - // "project1": nil, - // }, - "<": { - "project1": nil, - }, - "<=": { - "project1": nil, - }, - "~>": { - "project1": nil, - }, - } - - type testCase struct { - DirStructure map[string]interface{} - AtlantisYAML string - ModifiedFiles []string - Exp map[string][]int - } - - testCases := make(map[string]testCase) - - for _, exactSymbol := range exactSymbols { - testCases[fmt.Sprintf("exact version in terraform config using \"%s\"", exactSymbol)] = testCase{ - DirStructure: map[string]interface{}{ - "project1": map[string]interface{}{ - "main.tf": fmt.Sprintf(baseVersionConfig, exactSymbol), - }, - }, - ModifiedFiles: []string{"project1/main.tf"}, - Exp: map[string][]int{ - "project1": {0, 12, 8}, - }, - } - } - - for _, nonExactSymbol := range nonExactSymbols { - testCases[fmt.Sprintf("non-exact version in terraform config using \"%s\"", nonExactSymbol)] = testCase{ - DirStructure: map[string]interface{}{ - "project1": map[string]interface{}{ - "main.tf": fmt.Sprintf(baseVersionConfig, nonExactSymbol), - }, - }, - ModifiedFiles: []string{"project1/main.tf"}, - Exp: nonExactVersions[nonExactSymbol], - } - } - - // atlantis.yaml should take precedence over terraform config - testCases["with project config and terraform config"] = testCase{ - DirStructure: map[string]interface{}{ - "project1": map[string]interface{}{ - "main.tf": fmt.Sprintf(baseVersionConfig, exactSymbols[0]), - }, - valid.DefaultAtlantisFile: atlantisYamlContent, - }, - ModifiedFiles: []string{"project1/main.tf", "project2/main.tf"}, - Exp: map[string][]int{ - "project1": {0, 12, 6}, - }, - } - - testCases["with project config only"] = testCase{ - DirStructure: map[string]interface{}{ - "project1": map[string]interface{}{ - "main.tf": nil, - }, - valid.DefaultAtlantisFile: atlantisYamlContent, - }, - ModifiedFiles: []string{"project1/main.tf"}, - Exp: map[string][]int{ - "project1": {0, 12, 6}, - }, - } - - testCases["neither project config or terraform config"] = testCase{ - DirStructure: map[string]interface{}{ - "project1": map[string]interface{}{ - "main.tf": nil, - }, - }, - ModifiedFiles: []string{"project1/main.tf", "project2/main.tf"}, - Exp: map[string][]int{ - "project1": nil, - }, - } - - testCases["project with different terraform config"] = testCase{ - DirStructure: map[string]interface{}{ - "project1": map[string]interface{}{ - "main.tf": fmt.Sprintf(baseVersionConfig, exactSymbols[0]), - }, - "project2": map[string]interface{}{ - "main.tf": strings.Replace(fmt.Sprintf(baseVersionConfig, exactSymbols[0]), "0.12.8", "0.12.9", -1), - }, - }, - ModifiedFiles: []string{"project1/main.tf", "project2/main.tf"}, - Exp: map[string][]int{ - "project1": {0, 12, 8}, - "project2": {0, 12, 9}, - }, - } - - logger := logging.NewNoopLogger(t) - scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") - - for name, testCase := range testCases { - t.Run(name, func(t *testing.T) { - RegisterMockTestingT(t) - - tmpDir := DirStructure(t, testCase.DirStructure) - - vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())).ThenReturn(testCase.ModifiedFiles, nil) - - workingDir := mocks.NewMockWorkingDir() - When(workingDir.Clone( - matchers.AnyPtrToLoggingSimpleLogger(), - matchers.AnyModelsRepo(), - matchers.AnyModelsPullRequest(), - AnyString())).ThenReturn(tmpDir, false, nil) - - When(workingDir.GetWorkingDir( - matchers.AnyModelsRepo(), - matchers.AnyModelsPullRequest(), - AnyString())).ThenReturn(tmpDir, nil) - - globalCfgArgs := valid.GlobalCfgArgs{ - AllowRepoCfg: true, - MergeableReq: false, - ApprovedReq: false, - UnDivergedReq: false, - } - - terraformClient := terraform_mocks.NewMockClient() - When(terraformClient.ListAvailableVersions(matchers.AnyLoggingSimpleLogging())).ThenReturn([]string{}, nil) + return nil + }) builder := events.NewProjectCommandBuilder( false, @@ -1507,9 +1245,9 @@ projects: Ok(t, err) Equals(t, len(testCase.Exp), len(actCtxs)) for _, actCtx := range actCtxs { - if testCase.Exp[actCtx.RepoRelDir] != nil { + if testCase.Exp[actCtx.RepoRelDir] != "" { Assert(t, actCtx.TerraformVersion != nil, "TerraformVersion is nil.") - Equals(t, testCase.Exp[actCtx.RepoRelDir], actCtx.TerraformVersion.Segments()) + Equals(t, testCase.Exp[actCtx.RepoRelDir], actCtx.TerraformVersion.String()) } else { Assert(t, actCtx.TerraformVersion == nil, "TerraformVersion is supposed to be nil.") } diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 677faa4c76..1a2a2dd50e 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -2,17 +2,10 @@ package events import ( "path/filepath" - "regexp" - "sort" - "github.com/Masterminds/semver" "github.com/google/uuid" - "github.com/hashicorp/go-version" - "github.com/runatlantis/atlantis/server/core/terraform" - "github.com/warrensbox/terraform-switcher/lib" - - "github.com/hashicorp/terraform-config-inspect/tfconfig" "github.com/runatlantis/atlantis/server/core/config/valid" + "github.com/runatlantis/atlantis/server/core/terraform" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" "github.com/uber-go/tally" @@ -115,7 +108,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( // If TerraformVersion not defined in config file look for a // terraform.require_version block. if prjCfg.TerraformVersion == nil { - prjCfg.TerraformVersion = getTfVersion(ctx, terraformClient, filepath.Join(repoDir, prjCfg.RepoRelDir)) + prjCfg.TerraformVersion = terraformClient.DetectVersion(filepath.Join(repoDir, prjCfg.RepoRelDir), ctx.Log) } projectCmdContext := newProjectCommandContext( @@ -159,7 +152,7 @@ func (cb *PolicyCheckProjectCommandContextBuilder) BuildProjectContext( // If TerraformVersion not defined in config file look for a // terraform.require_version block. if prjCfg.TerraformVersion == nil { - prjCfg.TerraformVersion = getTfVersion(ctx, terraformClient, filepath.Join(repoDir, prjCfg.RepoRelDir)) + prjCfg.TerraformVersion = terraformClient.DetectVersion(filepath.Join(repoDir, prjCfg.RepoRelDir), ctx.Log) } projectCmds = cb.ProjectCommandContextBuilder.BuildProjectContext( @@ -283,66 +276,3 @@ func escapeArgs(args []string) []string { } return escaped } - -// Extracts required_version from Terraform configuration. -// Returns nil if unable to determine version from configuration. -func getTfVersion(ctx *command.Context, terraformClient terraform.Client, absProjDir string) *version.Version { - module, diags := tfconfig.LoadModule(absProjDir) - if diags.HasErrors() { - ctx.Log.Err("trying to detect required version: %s", diags.Error()) - } - - if len(module.RequiredCore) != 1 { - ctx.Log.Info("cannot determine which version to use from terraform configuration, detected %d possibilities.", len(module.RequiredCore)) - return nil - } - requiredVersionSetting := module.RequiredCore[0] - ctx.Log.Debug("found required_version setting of %q", requiredVersionSetting) - - tfVersions, err := terraformClient.ListAvailableVersions(ctx.Log) - if err != nil { - ctx.Log.Err("Unable to list Terraform versions") - } - - if len(tfVersions) == 0 { - // Fall back to an exact required version string - // We allow `= x.y.z`, `=x.y.z` or `x.y.z` where `x`, `y` and `z` are integers. - re := regexp.MustCompile(`^=?\s*([0-9.]+)\s*$`) - matched := re.FindStringSubmatch(requiredVersionSetting) - if len(matched) == 0 { - ctx.Log.Debug("Did not specify exact version in terraform configuration, found %q", requiredVersionSetting) - return nil - } - tfVersions = []string{matched[1]} - } - - constraint, _ := semver.NewConstraint(requiredVersionSetting) - versions := make([]*semver.Version, len(tfVersions)) - - for i, tfvals := range tfVersions { - newVersion, err := semver.NewVersion(tfvals) //NewVersion parses a given version and returns an instance of Version or an error if unable to parse the version. - if err == nil { - versions[i] = newVersion - } - } - - if len(versions) == 0 { - ctx.Log.Debug("did not specify exact valid version in terraform configuration, found %q", requiredVersionSetting) - return nil - } - - sort.Sort(sort.Reverse(semver.Collection(versions))) - - for _, element := range versions { - if constraint.Check(element) { // Validate a version against a constraint - tfversionStr := element.String() - if lib.ValidVersionFormat(tfversionStr) { //check if version format is correct - tfversion, _ := version.NewVersion(tfversionStr) - ctx.Log.Info("detected module requires version: %s", tfversionStr) - return tfversion - } - } - } - ctx.Log.Debug("could not match any valid terraform version with %q", requiredVersionSetting) - return nil -} From 588cc9109d87be7c3c8a673c74efc9ff266349b4 Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Wed, 21 Dec 2022 18:20:17 -0800 Subject: [PATCH 10/14] Improve consistency in new function signatures & log statements --- .../terraform/mocks/mock_terraform_client.go | 2 +- server/core/terraform/terraform_client.go | 52 +++++++++---------- .../core/terraform/terraform_client_test.go | 2 +- server/events/project_command_builder_test.go | 4 +- .../events/project_command_context_builder.go | 4 +- 5 files changed, 32 insertions(+), 32 deletions(-) diff --git a/server/core/terraform/mocks/mock_terraform_client.go b/server/core/terraform/mocks/mock_terraform_client.go index e6d7006d5f..7b880e3467 100644 --- a/server/core/terraform/mocks/mock_terraform_client.go +++ b/server/core/terraform/mocks/mock_terraform_client.go @@ -80,7 +80,7 @@ func (mock *MockClient) ListAvailableVersions(log logging.SimpleLogging) ([]stri return ret0, ret1 } -func (mock *MockClient) DetectVersion(projectDirectory string, log logging.SimpleLogging) *go_version.Version { +func (mock *MockClient) DetectVersion(log logging.SimpleLogging, projectDirectory string) *go_version.Version { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClient().") } diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index 5ffd0dff34..d722fa8db3 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -62,7 +62,7 @@ type Client interface { ListAvailableVersions(log logging.SimpleLogging) ([]string, error) // DetectVersion Extracts required_version from Terraform configuration in the specified project directory. Returns nil if unable to determine the version. - DetectVersion(projectDirectory string, log logging.SimpleLogging) *version.Version + DetectVersion(log logging.SimpleLogging, projectDirectory string) *version.Version } type DefaultClient struct { @@ -77,9 +77,9 @@ type DefaultClient struct { // with another binary, ex. echo. overrideTF string // downloader downloads terraform versions. - downloader Downloader - downloadBaseURL string - downloadsAllowed bool + downloader Downloader + downloadBaseURL string + downloadAllowed bool // versions maps from the string representation of a tf version (ex. 0.11.10) // to the absolute path of that binary on disk (if it exists). // Use versionsLock to control access. @@ -122,7 +122,7 @@ func NewClientWithDefaultVersion( defaultVersionFlagName string, tfDownloadURL string, tfDownloader Downloader, - allowDownloads bool, + tfDownloadAllowed bool, usePluginCache bool, fetchAsync bool, projectCmdOutputHandler jobs.ProjectCommandOutputHandler, @@ -159,7 +159,7 @@ func NewClientWithDefaultVersion( // Since ensureVersion might end up downloading terraform, // we call it asynchronously so as to not delay server startup. versionsLock.Lock() - _, err := ensureVersion(log, tfDownloader, versions, defaultVersion, binDir, tfDownloadURL, allowDownloads) + _, err := ensureVersion(log, tfDownloader, versions, defaultVersion, binDir, tfDownloadURL, tfDownloadAllowed) versionsLock.Unlock() if err != nil { log.Err("could not download terraform %s: %s", defaultVersion.String(), err) @@ -189,7 +189,7 @@ func NewClientWithDefaultVersion( binDir: binDir, downloader: tfDownloader, downloadBaseURL: tfDownloadURL, - downloadsAllowed: allowDownloads, + downloadAllowed: tfDownloadAllowed, versionsLock: &versionsLock, versions: versions, usePluginCache: usePluginCache, @@ -208,7 +208,7 @@ func NewTestClient( defaultVersionFlagName string, tfDownloadURL string, tfDownloader Downloader, - downloadsAllowed bool, + tfDownloadAllowed bool, usePluginCache bool, projectCmdOutputHandler jobs.ProjectCommandOutputHandler, ) (*DefaultClient, error) { @@ -222,7 +222,7 @@ func NewTestClient( defaultVersionFlagName, tfDownloadURL, tfDownloader, - downloadsAllowed, + tfDownloadAllowed, usePluginCache, false, projectCmdOutputHandler, @@ -247,7 +247,7 @@ func NewClient( defaultVersionFlagName string, tfDownloadURL string, tfDownloader Downloader, - allowDownloads bool, + tfDownloadAllowed bool, usePluginCache bool, projectCmdOutputHandler jobs.ProjectCommandOutputHandler, ) (*DefaultClient, error) { @@ -261,7 +261,7 @@ func NewClient( defaultVersionFlagName, tfDownloadURL, tfDownloader, - allowDownloads, + tfDownloadAllowed, usePluginCache, true, projectCmdOutputHandler, @@ -283,7 +283,7 @@ func (c *DefaultClient) TerraformBinDir() string { func (c *DefaultClient) ListAvailableVersions(log logging.SimpleLogging) ([]string, error) { url := fmt.Sprintf("%s/terraform", c.downloadBaseURL) - if !c.downloadsAllowed { + if !c.downloadAllowed { log.Debug("Terraform downloads disabled. Won't list Terraform versions available at %s", url) return []string{}, nil } @@ -295,18 +295,18 @@ func (c *DefaultClient) ListAvailableVersions(log logging.SimpleLogging) ([]stri // DetectVersion Extracts required_version from Terraform configuration in the specified project directory. Returns nil if unable to determine the version. // This will also try to intelligently evaluate non-exact matches by listing the available versions of Terraform and picking the best match. -func (c *DefaultClient) DetectVersion(projectDirectory string, log logging.SimpleLogging) *version.Version { +func (c *DefaultClient) DetectVersion(log logging.SimpleLogging, projectDirectory string) *version.Version { module, diags := tfconfig.LoadModule(projectDirectory) if diags.HasErrors() { - log.Err("trying to detect required version: %s", diags.Error()) + log.Err("Trying to detect required version: %s", diags.Error()) } if len(module.RequiredCore) != 1 { - log.Info("cannot determine which version to use from terraform configuration, detected %d possibilities.", len(module.RequiredCore)) + log.Info("Cannot determine which version to use from terraform configuration, detected %d possibilities.", len(module.RequiredCore)) return nil } requiredVersionSetting := module.RequiredCore[0] - log.Debug("found required_version setting of %q", requiredVersionSetting) + log.Debug("Found required_version setting of %q", requiredVersionSetting) tfVersions, err := c.ListAvailableVersions(log) if err != nil { @@ -329,14 +329,14 @@ func (c *DefaultClient) DetectVersion(projectDirectory string, log logging.Simpl versions := make([]*semver.Version, len(tfVersions)) for i, tfvals := range tfVersions { - newVersion, err := semver.NewVersion(tfvals) //NewVersion parses a given version and returns an instance of Version or an error if unable to parse the version. + newVersion, err := semver.NewVersion(tfvals) if err == nil { versions[i] = newVersion } } if len(versions) == 0 { - log.Debug("did not specify exact valid version in terraform configuration, found %q", requiredVersionSetting) + log.Debug("Did not specify exact valid version in terraform configuration, found %q", requiredVersionSetting) return nil } @@ -347,12 +347,12 @@ func (c *DefaultClient) DetectVersion(projectDirectory string, log logging.Simpl tfversionStr := element.String() if lib.ValidVersionFormat(tfversionStr) { //check if version format is correct tfversion, _ := version.NewVersion(tfversionStr) - log.Info("detected module requires version: %s", tfversionStr) + log.Info("Detected module requires version: %s", tfversionStr) return tfversion } } } - log.Debug("could not match any valid terraform version with %q", requiredVersionSetting) + log.Debug("Could not match any valid terraform version with %q", requiredVersionSetting) return nil } @@ -364,7 +364,7 @@ func (c *DefaultClient) EnsureVersion(log logging.SimpleLogging, v *version.Vers var err error c.versionsLock.Lock() - _, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL, c.downloadsAllowed) + _, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL, c.downloadAllowed) c.versionsLock.Unlock() if err != nil { return err @@ -408,7 +408,7 @@ func (c *DefaultClient) RunCommandWithVersion(ctx command.ProjectContext, path s ctx.Log.Err(err.Error()) return ansi.Strip(string(out)), err } - ctx.Log.Info("successfully ran %q in %q", tfCmd, path) + ctx.Log.Info("Successfully ran %q in %q", tfCmd, path) return ansi.Strip(string(out)), nil } @@ -441,7 +441,7 @@ func (c *DefaultClient) prepCmd(log logging.SimpleLogging, v *version.Version, w } else { var err error c.versionsLock.Lock() - binPath, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL, c.downloadsAllowed) + binPath, err = ensureVersion(log, c.downloader, c.versions, v, c.binDir, c.downloadBaseURL, c.downloadAllowed) c.versionsLock.Unlock() if err != nil { return "", nil, err @@ -534,10 +534,10 @@ func ensureVersion(log logging.SimpleLogging, dl Downloader, versions map[string return dest, nil } if !downloadsAllowed { - return "", fmt.Errorf("could not find terraform version %s in PATH or %s, and downloads are disabled", v.String(), binDir) + return "", fmt.Errorf("Could not find terraform version %s in PATH or %s, and downloads are disabled", v.String(), binDir) } - log.Info("could not find terraform version %s in PATH or %s, downloading from %s", v.String(), binDir, downloadURL) + log.Info("Could not find terraform version %s in PATH or %s, downloading from %s", v.String(), binDir, downloadURL) urlPrefix := fmt.Sprintf("%s/terraform/%s/terraform_%s", downloadURL, v.String(), v.String()) binURL := fmt.Sprintf("%s_%s_%s.zip", urlPrefix, runtime.GOOS, runtime.GOARCH) checksumURL := fmt.Sprintf("%s_SHA256SUMS", urlPrefix) @@ -546,7 +546,7 @@ func ensureVersion(log logging.SimpleLogging, dl Downloader, versions map[string return "", errors.Wrapf(err, "downloading terraform version %s at %q", v.String(), fullSrcURL) } - log.Info("downloaded terraform %s to %s", v.String(), dest) + log.Info("Downloaded terraform %s to %s", v.String(), dest) versions[v.String()] = dest return dest, nil } diff --git a/server/core/terraform/terraform_client_test.go b/server/core/terraform/terraform_client_test.go index 5228198a3d..cfe13442de 100644 --- a/server/core/terraform/terraform_client_test.go +++ b/server/core/terraform/terraform_client_test.go @@ -487,7 +487,7 @@ terraform { tmpDir := DirStructure(t, testCase.DirStructure) for project, expectedVersion := range testCase.Exp { - detectedVersion := c.DetectVersion(filepath.Join(tmpDir, project), logger) + detectedVersion := c.DetectVersion(logger, filepath.Join(tmpDir, project)) expectNil := expectedVersion == "" || (!testCase.IsExact && !downloadsAllowed) if expectNil { diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 7681849490..1ea491b0da 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -1200,8 +1200,8 @@ projects: terraformClient := terraform_mocks.NewMockClient() - When(terraformClient.DetectVersion(AnyString(), matchers.AnyLoggingSimpleLogging())).Then(func(params []Param) ReturnValues { - projectName := filepath.Base(params[0].(string)) + When(terraformClient.DetectVersion(matchers.AnyLoggingSimpleLogging(), AnyString())).Then(func(params []Param) ReturnValues { + projectName := filepath.Base(params[1].(string)) testVersion := testCase.Exp[projectName] if testVersion != "" { v, _ := version.NewVersion(testVersion) diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 1a2a2dd50e..cf8fd506f8 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -108,7 +108,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( // If TerraformVersion not defined in config file look for a // terraform.require_version block. if prjCfg.TerraformVersion == nil { - prjCfg.TerraformVersion = terraformClient.DetectVersion(filepath.Join(repoDir, prjCfg.RepoRelDir), ctx.Log) + prjCfg.TerraformVersion = terraformClient.DetectVersion(ctx.Log, filepath.Join(repoDir, prjCfg.RepoRelDir)) } projectCmdContext := newProjectCommandContext( @@ -152,7 +152,7 @@ func (cb *PolicyCheckProjectCommandContextBuilder) BuildProjectContext( // If TerraformVersion not defined in config file look for a // terraform.require_version block. if prjCfg.TerraformVersion == nil { - prjCfg.TerraformVersion = terraformClient.DetectVersion(filepath.Join(repoDir, prjCfg.RepoRelDir), ctx.Log) + prjCfg.TerraformVersion = terraformClient.DetectVersion(ctx.Log, filepath.Join(repoDir, prjCfg.RepoRelDir)) } projectCmds = cb.ProjectCommandContextBuilder.BuildProjectContext( From 8295a97516e0fb71603958d73af492f530fba251 Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Wed, 21 Dec 2022 18:36:52 -0800 Subject: [PATCH 11/14] Test requests before calling terraform-switch, to prevent unnecessary crashes --- server/core/terraform/terraform_client.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index d722fa8db3..d47ce92ebb 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -18,6 +18,7 @@ package terraform import ( "fmt" + "net/http" "os" "os/exec" "path/filepath" @@ -289,6 +290,13 @@ func (c *DefaultClient) ListAvailableVersions(log logging.SimpleLogging) ([]stri } log.Debug("Listing Terraform versions available at: %s", url) + // terraform-switcher calls os.Exit(1) if it fails to successfully GET the configured URL. + // So, before calling it, test if we can connect. Then we can return an error instead if the request fails. + resp, err := http.Get(url) + if err != nil || resp.StatusCode != 200 { + return nil, fmt.Errorf("Unable to list Terraform versions: %s", err) + } + versions, err := lib.GetTFList(url, true) return versions, err } From 61b9a2e85afc697988b6cc96e899e62fac0e6972 Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Wed, 21 Dec 2022 19:44:08 -0800 Subject: [PATCH 12/14] Fix broken tests --- .../terraform/mocks/mock_terraform_client.go | 22 +++++++++---------- .../core/terraform/terraform_client_test.go | 2 +- server/events/project_command_builder_test.go | 3 +-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/server/core/terraform/mocks/mock_terraform_client.go b/server/core/terraform/mocks/mock_terraform_client.go index 7b880e3467..2f0488f42d 100644 --- a/server/core/terraform/mocks/mock_terraform_client.go +++ b/server/core/terraform/mocks/mock_terraform_client.go @@ -84,7 +84,7 @@ func (mock *MockClient) DetectVersion(log logging.SimpleLogging, projectDirector if mock == nil { panic("mock must not be nil. Use myMock := NewMockClient().") } - params := []pegomock.Param{projectDirectory, log} + params := []pegomock.Param{log, projectDirectory} result := pegomock.GetGenericMockFrom(mock).Invoke("DetectVersion", params, []reflect.Type{reflect.TypeOf((**go_version.Version)(nil)).Elem()}) var ret0 *go_version.Version if len(result) != 0 { @@ -237,8 +237,8 @@ func (c *MockClient_ListAvailableVersions_OngoingVerification) GetAllCapturedArg return } -func (verifier *VerifierMockClient) DetectVersion(projectDirectory string, log logging.SimpleLogging) *MockClient_DetectVersion_OngoingVerification { - params := []pegomock.Param{projectDirectory, log} +func (verifier *VerifierMockClient) DetectVersion(log logging.SimpleLogging, projectDirectory string) *MockClient_DetectVersion_OngoingVerification { + params := []pegomock.Param{log, projectDirectory} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DetectVersion", params, verifier.timeout) return &MockClient_DetectVersion_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -248,21 +248,21 @@ type MockClient_DetectVersion_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockClient_DetectVersion_OngoingVerification) GetCapturedArguments() (string, logging.SimpleLogging) { - projectDirectory, log := c.GetAllCapturedArguments() - return projectDirectory[len(projectDirectory)-1], log[len(log)-1] +func (c *MockClient_DetectVersion_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, string) { + log, projectDirectory := c.GetAllCapturedArguments() + return log[len(log)-1], projectDirectory[len(projectDirectory)-1] } -func (c *MockClient_DetectVersion_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []logging.SimpleLogging) { +func (c *MockClient_DetectVersion_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(string) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]logging.SimpleLogging, len(c.methodInvocations)) + _param1 = make([]string, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(logging.SimpleLogging) + _param1[u] = param.(string) } } return diff --git a/server/core/terraform/terraform_client_test.go b/server/core/terraform/terraform_client_test.go index cfe13442de..b4833b9b43 100644 --- a/server/core/terraform/terraform_client_test.go +++ b/server/core/terraform/terraform_client_test.go @@ -332,7 +332,7 @@ func TestEnsureVersion_downloaded_downloadingDisabled(t *testing.T) { Ok(t, err) err = c.EnsureVersion(logger, v) - ErrContains(t, "could not find terraform version", err) + ErrContains(t, "Could not find terraform version", err) ErrContains(t, "downloads are disabled", err) mockDownloader.VerifyWasCalled(Never()) } diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 1ea491b0da..7122eebb1f 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -1199,7 +1199,6 @@ projects: } terraformClient := terraform_mocks.NewMockClient() - When(terraformClient.DetectVersion(matchers.AnyLoggingSimpleLogging(), AnyString())).Then(func(params []Param) ReturnValues { projectName := filepath.Base(params[1].(string)) testVersion := testCase.Exp[projectName] @@ -1246,7 +1245,7 @@ projects: Equals(t, len(testCase.Exp), len(actCtxs)) for _, actCtx := range actCtxs { if testCase.Exp[actCtx.RepoRelDir] != "" { - Assert(t, actCtx.TerraformVersion != nil, "TerraformVersion is nil.") + Assert(t, actCtx.TerraformVersion != nil, "TerraformVersion is nil, not %s for %s", testCase.Exp[actCtx.RepoRelDir], actCtx.RepoRelDir) Equals(t, testCase.Exp[actCtx.RepoRelDir], actCtx.TerraformVersion.String()) } else { Assert(t, actCtx.TerraformVersion == nil, "TerraformVersion is supposed to be nil.") From c4c1815762f778b3c864238c1e8fcfa992dc1461 Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Wed, 21 Dec 2022 20:01:06 -0800 Subject: [PATCH 13/14] Silence gosec error --- server/core/terraform/terraform_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index d47ce92ebb..780481bb5d 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -292,7 +292,7 @@ func (c *DefaultClient) ListAvailableVersions(log logging.SimpleLogging) ([]stri log.Debug("Listing Terraform versions available at: %s", url) // terraform-switcher calls os.Exit(1) if it fails to successfully GET the configured URL. // So, before calling it, test if we can connect. Then we can return an error instead if the request fails. - resp, err := http.Get(url) + resp, err := http.Get(url) // #nosec G107 -- terraform-switch makes this same call below. Also, we don't process the response payload. if err != nil || resp.StatusCode != 200 { return nil, fmt.Errorf("Unable to list Terraform versions: %s", err) } From a21732f570273e016c0af476b69bb74fe1c91302 Mon Sep 17 00:00:00 2001 From: "adam.verigin" Date: Thu, 22 Dec 2022 07:57:38 -0800 Subject: [PATCH 14/14] Close response body --- server/core/terraform/terraform_client.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index 780481bb5d..199fc245b3 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -290,12 +290,18 @@ func (c *DefaultClient) ListAvailableVersions(log logging.SimpleLogging) ([]stri } log.Debug("Listing Terraform versions available at: %s", url) + // terraform-switcher calls os.Exit(1) if it fails to successfully GET the configured URL. // So, before calling it, test if we can connect. Then we can return an error instead if the request fails. resp, err := http.Get(url) // #nosec G107 -- terraform-switch makes this same call below. Also, we don't process the response payload. - if err != nil || resp.StatusCode != 200 { + if err != nil { return nil, fmt.Errorf("Unable to list Terraform versions: %s", err) } + defer resp.Body.Close() // nolint: errcheck + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("Unable to list Terraform versions: response code %d from %s", resp.StatusCode, url) + } versions, err := lib.GetTFList(url, true) return versions, err