From 0248a0d93f5ec88a980e6449d78b6f01d25f3464 Mon Sep 17 00:00:00 2001 From: karishma-chawla <74574173+karishma-chawla@users.noreply.github.com> Date: Mon, 30 Oct 2023 14:53:24 -0700 Subject: [PATCH 1/2] Initialize tfexec.Terraform once per request Signed-off-by: Karishma Chawla --- pkg/cli/cmd/recipe/show/show_test.go | 18 ++++----- pkg/recipes/terraform/execute.go | 55 +++++++++++++++----------- pkg/recipes/terraform/execute_test.go | 30 +++----------- pkg/recipes/terraform/module.go | 10 ++--- pkg/recipes/terraform/module_test.go | 12 ------ pkg/recipes/terraform/types_test.go | 57 +++++++++++++++++++++++++++ 6 files changed, 108 insertions(+), 74 deletions(-) create mode 100644 pkg/recipes/terraform/types_test.go diff --git a/pkg/cli/cmd/recipe/show/show_test.go b/pkg/cli/cmd/recipe/show/show_test.go index 0b8495dcd6..250dd9aa61 100644 --- a/pkg/cli/cmd/recipe/show/show_test.go +++ b/pkg/cli/cmd/recipe/show/show_test.go @@ -30,7 +30,7 @@ import ( "github.com/radius-project/radius/pkg/cli/output" "github.com/radius-project/radius/pkg/cli/workspaces" "github.com/radius-project/radius/pkg/corerp/api/v20231001preview" - datastorerp "github.com/radius-project/radius/pkg/datastoresrp/frontend/controller" + datastoresrp "github.com/radius-project/radius/pkg/datastoresrp/frontend/controller" "github.com/radius-project/radius/pkg/recipes" "github.com/radius-project/radius/pkg/to" "github.com/radius-project/radius/test/radcli" @@ -46,7 +46,7 @@ func Test_Validate(t *testing.T) { testcases := []radcli.ValidateInput{ { Name: "Valid Show Command", - Input: []string{"recipeName", "--resource-type", datastorerp.RedisCachesResourceType}, + Input: []string{"recipeName", "--resource-type", datastoresrp.RedisCachesResourceType}, ExpectedValid: true, ConfigHolder: framework.ConfigHolder{ ConfigFilePath: "", @@ -55,7 +55,7 @@ func Test_Validate(t *testing.T) { }, { Name: "Show Command with incorrect fallback workspace", - Input: []string{"-e", "my-env", "-g", "my-env", "recipeName", "--resource-type", datastorerp.RedisCachesResourceType}, + Input: []string{"-e", "my-env", "-g", "my-env", "recipeName", "--resource-type", datastoresrp.RedisCachesResourceType}, ExpectedValid: false, ConfigHolder: framework.ConfigHolder{ ConfigFilePath: "", @@ -64,7 +64,7 @@ func Test_Validate(t *testing.T) { }, { Name: "Show Command with too many positional args", - Input: []string{"recipeName", "arg2", "--resource-type", datastorerp.RedisCachesResourceType}, + Input: []string{"recipeName", "arg2", "--resource-type", datastoresrp.RedisCachesResourceType}, ExpectedValid: false, ConfigHolder: framework.ConfigHolder{ ConfigFilePath: "", @@ -73,7 +73,7 @@ func Test_Validate(t *testing.T) { }, { Name: "Show Command with fallback workspace", - Input: []string{"-e", "my-env", "-w", "test-workspace", "recipeName", "--resource-type", datastorerp.RedisCachesResourceType}, + Input: []string{"-e", "my-env", "-w", "test-workspace", "recipeName", "--resource-type", datastoresrp.RedisCachesResourceType}, ExpectedValid: true, ConfigHolder: framework.ConfigHolder{ ConfigFilePath: "", @@ -111,7 +111,7 @@ func Test_Run(t *testing.T) { } recipe := types.EnvironmentRecipe{ Name: "cosmosDB", - ResourceType: datastorerp.MongoDatabasesResourceType, + ResourceType: datastoresrp.MongoDatabasesResourceType, TemplateKind: recipes.TemplateKindBicep, TemplatePath: "ghcr.io/testpublicrecipe/bicep/modules/mongodatabases:v1", } @@ -145,7 +145,7 @@ func Test_Run(t *testing.T) { Workspace: &workspaces.Workspace{}, Format: "table", RecipeName: "cosmosDB", - ResourceType: datastorerp.MongoDatabasesResourceType, + ResourceType: datastoresrp.MongoDatabasesResourceType, } err := runner.Run(context.Background()) @@ -187,7 +187,7 @@ func Test_Run(t *testing.T) { } recipe := types.EnvironmentRecipe{ Name: "cosmosDB", - ResourceType: datastorerp.MongoDatabasesResourceType, + ResourceType: datastoresrp.MongoDatabasesResourceType, TemplateKind: recipes.TemplateKindTerraform, TemplatePath: "Azure/cosmosdb/azurerm", TemplateVersion: "1.1.0", @@ -222,7 +222,7 @@ func Test_Run(t *testing.T) { Workspace: &workspaces.Workspace{}, Format: "table", RecipeName: "cosmosDB", - ResourceType: datastorerp.MongoDatabasesResourceType, + ResourceType: datastoresrp.MongoDatabasesResourceType, } err := runner.Run(context.Background()) diff --git a/pkg/recipes/terraform/execute.go b/pkg/recipes/terraform/execute.go index 4a3954b601..ec177c9f1c 100644 --- a/pkg/recipes/terraform/execute.go +++ b/pkg/recipes/terraform/execute.go @@ -26,6 +26,7 @@ import ( "time" install "github.com/hashicorp/hc-install" + "github.com/hashicorp/terraform-exec/tfexec" tfjson "github.com/hashicorp/terraform-json" "github.com/radius-project/radius/pkg/metrics" "github.com/radius-project/radius/pkg/recipes" @@ -95,14 +96,20 @@ func (e *executor) Deploy(ctx context.Context, options Options) (*tfjson.State, return nil, err } + // Create a new instance of tfexec.Terraform with current the Terraform installation path + tf, err := NewTerraform(ctx, workingDir, execPath) + if err != nil { + return nil, err + } + // Create Terraform config in the working directory - kubernetesBackendSuffix, err := e.generateConfig(ctx, workingDir, execPath, options) + kubernetesBackendSuffix, err := e.generateConfig(ctx, tf, workingDir, options) if err != nil { return nil, err } // Run TF Init and Apply in the working directory - state, err := initAndApply(ctx, workingDir, execPath) + state, err := initAndApply(ctx, tf, workingDir) if err != nil { return nil, err } @@ -144,8 +151,14 @@ func (e *executor) Delete(ctx context.Context, options Options) error { return err } + // Create a new instance of tfexec.Terraform with current the Terraform installation path + tf, err := NewTerraform(ctx, workingDir, execPath) + if err != nil { + return err + } + // Create Terraform config in the working directory - kubernetesBackendSuffix, err := e.generateConfig(ctx, workingDir, execPath, options) + kubernetesBackendSuffix, err := e.generateConfig(ctx, tf, workingDir, options) if err != nil { return err } @@ -165,7 +178,7 @@ func (e *executor) Delete(ctx context.Context, options Options) error { } // Run TF Destroy in the working directory to delete the resources deployed by the recipe - err = initAndDestroy(ctx, workingDir, execPath) + err = initAndDestroy(ctx, tf) if err != nil { return err } @@ -204,12 +217,18 @@ func (e *executor) GetRecipeMetadata(ctx context.Context, options Options) (map[ return nil, err } + // Create a new instance of tfexec.Terraform with current the Terraform installation path + tf, err := NewTerraform(ctx, workingDir, execPath) + if err != nil { + return nil, err + } + _, err = getTerraformConfig(ctx, workingDir, options) if err != nil { return nil, err } - result, err := downloadAndInspect(ctx, workingDir, execPath, options) + result, err := downloadAndInspect(ctx, tf, workingDir, options) if err != nil { return nil, err } @@ -232,7 +251,7 @@ func createWorkingDir(ctx context.Context, tfDir string) (string, error) { } // generateConfig generates Terraform configuration with required inputs for the module, providers and backend to be initialized and applied. -func (e *executor) generateConfig(ctx context.Context, workingDir, execPath string, options Options) (string, error) { +func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, workingDir string, options Options) (string, error) { logger := ucplog.FromContextOrDiscard(ctx) tfConfig, err := getTerraformConfig(ctx, workingDir, options) @@ -240,7 +259,7 @@ func (e *executor) generateConfig(ctx context.Context, workingDir, execPath stri return "", err } - loadedModule, err := downloadAndInspect(ctx, workingDir, execPath, options) + loadedModule, err := downloadAndInspect(ctx, tf, workingDir, options) if err != nil { return "", err } @@ -298,13 +317,13 @@ func (e *executor) generateConfig(ctx context.Context, workingDir, execPath stri } // downloadAndInspect handles downloading the TF module and retrieving the necessary information -func downloadAndInspect(ctx context.Context, workingDir string, execPath string, options Options) (*moduleInspectResult, error) { +func downloadAndInspect(ctx context.Context, tf *tfexec.Terraform, workingDir string, options Options) (*moduleInspectResult, error) { logger := ucplog.FromContextOrDiscard(ctx) // Download the Terraform module to the working directory. logger.Info(fmt.Sprintf("Downloading Terraform module: %s", options.EnvRecipe.TemplatePath)) downloadStartTime := time.Now() - if err := downloadModule(ctx, workingDir, execPath, options.EnvRecipe.TemplatePath); err != nil { + if err := downloadModule(ctx, tf, options.EnvRecipe.TemplatePath); err != nil { metrics.DefaultRecipeEngineMetrics.RecordRecipeDownloadDuration(ctx, downloadStartTime, metrics.NewRecipeAttributes(metrics.RecipeEngineOperationDownloadRecipe, options.EnvRecipe.Name, options.EnvRecipe, recipes.RecipeDownloadFailed)) @@ -350,16 +369,11 @@ func getTerraformConfig(ctx context.Context, workingDir string, options Options) } // initAndApply runs Terraform init and apply in the provided working directory. -func initAndApply(ctx context.Context, workingDir, execPath string) (*tfjson.State, error) { +func initAndApply(ctx context.Context, tf *tfexec.Terraform, workingDir string) (*tfjson.State, error) { logger := ucplog.FromContextOrDiscard(ctx) - tf, err := NewTerraform(ctx, workingDir, execPath) - if err != nil { - return nil, err - } // Initialize Terraform logger.Info("Initializing Terraform") - terraformInitStartTime := time.Now() if err := tf.Init(ctx); err != nil { metrics.DefaultRecipeEngineMetrics.RecordTerraformInitializationDuration(ctx, terraformInitStartTime, @@ -382,19 +396,16 @@ func initAndApply(ctx context.Context, workingDir, execPath string) (*tfjson.Sta } // initAndDestroy runs Terraform init and destroy in the provided working directory. -func initAndDestroy(ctx context.Context, workingDir, execPath string) error { +func initAndDestroy(ctx context.Context, tf *tfexec.Terraform) error { logger := ucplog.FromContextOrDiscard(ctx) - tf, err := NewTerraform(ctx, workingDir, execPath) - if err != nil { - return err - } - // Initialize Terraform logger.Info("Initializing Terraform") - terraformInitStartTime := time.Now() if err := tf.Init(ctx); err != nil { + metrics.DefaultRecipeEngineMetrics.RecordTerraformInitializationDuration(ctx, terraformInitStartTime, + []attribute.KeyValue{metrics.OperationStateAttrKey.String(metrics.FailedOperationState)}) + return fmt.Errorf("terraform init failure: %w", err) } metrics.DefaultRecipeEngineMetrics.RecordTerraformInitializationDuration(ctx, terraformInitStartTime, nil) diff --git a/pkg/recipes/terraform/execute_test.go b/pkg/recipes/terraform/execute_test.go index c38c8d939f..b430d57de4 100644 --- a/pkg/recipes/terraform/execute_test.go +++ b/pkg/recipes/terraform/execute_test.go @@ -21,6 +21,7 @@ import ( "path/filepath" "testing" + "github.com/hashicorp/terraform-exec/tfexec" "github.com/radius-project/radius/pkg/recipes" "github.com/radius-project/radius/pkg/recipes/terraform/config" "github.com/radius-project/radius/test/testcontext" @@ -57,17 +58,7 @@ func TestCreateWorkingDir_Error(t *testing.T) { require.Contains(t, err.Error(), "failed to create working directory") } -func TestInitAndApply_EmptyWorkingDirPath(t *testing.T) { - // Create a temporary directory for testing. - testDir := t.TempDir() - execPath := filepath.Join(testDir, "terraform") - - _, err := initAndApply(testcontext.New(t), "", execPath) - require.Error(t, err) - require.Contains(t, err.Error(), "Terraform cannot be initialised with empty workdir") -} - -func TestGeneratedConfig(t *testing.T) { +func TestGenerateConfig(t *testing.T) { configTests := []struct { name string workingDir string @@ -75,7 +66,7 @@ func TestGeneratedConfig(t *testing.T) { err string }{ { - name: "empty recipe name", + name: "empty recipe name error", opts: Options{ EnvRecipe: &recipes.EnvironmentDefinition{ TemplatePath: "test/module/source", @@ -93,16 +84,6 @@ func TestGeneratedConfig(t *testing.T) { ResourceRecipe: &recipes.ResourceMetadata{}, }, err: "error creating file: open /invalid-dir/main.tf.json", - }, { - name: "invalid exec path", - opts: Options{ - EnvRecipe: &recipes.EnvironmentDefinition{ - Name: "test-recipe", - TemplatePath: "test/module/source", - }, - ResourceRecipe: &recipes.ResourceMetadata{}, - }, - err: "/terraform: no such file or directory", }, } @@ -112,9 +93,10 @@ func TestGeneratedConfig(t *testing.T) { if tc.workingDir == "" { tc.workingDir = t.TempDir() } - execPath := filepath.Join(tc.workingDir, "terraform") + tf, _ := tfexec.NewTerraform(tc.workingDir, filepath.Join(tc.workingDir, "terraform")) + e := executor{} - _, err := e.generateConfig(ctx, tc.workingDir, execPath, tc.opts) + _, err := e.generateConfig(ctx, tf, tc.workingDir, tc.opts) require.Error(t, err) require.ErrorContains(t, err, tc.err) }) diff --git a/pkg/recipes/terraform/module.go b/pkg/recipes/terraform/module.go index 930b7bc672..373eee0f47 100644 --- a/pkg/recipes/terraform/module.go +++ b/pkg/recipes/terraform/module.go @@ -22,6 +22,7 @@ import ( "path/filepath" "github.com/hashicorp/terraform-config-inspect/tfconfig" + "github.com/hashicorp/terraform-exec/tfexec" "github.com/radius-project/radius/pkg/recipes" "github.com/radius-project/radius/pkg/recipes/recipecontext" ) @@ -98,13 +99,8 @@ func inspectModule(workingDir, localModuleName string) (*moduleInspectResult, er // downloadModule downloads the module to the workingDir from the module source specified in the Terraform configuration. // It uses Terraform's Get command to download the module using the Terraform executable available at execPath. // An error is returned if the module could not be downloaded. -func downloadModule(ctx context.Context, workingDir, execPath, templatePath string) error { - tf, err := NewTerraform(ctx, workingDir, execPath) - if err != nil { - return err - } - - if err = tf.Get(ctx); err != nil { +func downloadModule(ctx context.Context, tf *tfexec.Terraform, templatePath string) error { + if err := tf.Get(ctx); err != nil { return fmt.Errorf("failed to run terraform get to download the module from source %q: %w", templatePath, err) } diff --git a/pkg/recipes/terraform/module_test.go b/pkg/recipes/terraform/module_test.go index bb9ad75065..4606cc932e 100644 --- a/pkg/recipes/terraform/module_test.go +++ b/pkg/recipes/terraform/module_test.go @@ -17,11 +17,9 @@ limitations under the License. package terraform import ( - "path/filepath" "testing" "github.com/hashicorp/terraform-config-inspect/tfconfig" - "github.com/radius-project/radius/test/testcontext" "github.com/stretchr/testify/require" ) @@ -89,13 +87,3 @@ func Test_InspectTFModuleConfig(t *testing.T) { }) } } - -func Test_DownloadModule_EmptyWorkingDirPath_Error(t *testing.T) { - // Create a temporary test directory. - testDir := t.TempDir() - execPath := filepath.Join(testDir, "terraform") - - err := downloadModule(testcontext.New(t), "", execPath, "test/module/source") - require.Error(t, err) - require.Contains(t, err.Error(), "Terraform cannot be initialised with empty workdir") -} diff --git a/pkg/recipes/terraform/types_test.go b/pkg/recipes/terraform/types_test.go new file mode 100644 index 0000000000..76b6ce3ebe --- /dev/null +++ b/pkg/recipes/terraform/types_test.go @@ -0,0 +1,57 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package terraform + +import ( + "path/filepath" + "testing" + + "github.com/radius-project/radius/test/testcontext" + "github.com/stretchr/testify/require" +) + +func TestNewTerraform_EmptyWorkingDirPath(t *testing.T) { + // Create a temporary directory for testing. + testDir := t.TempDir() + execPath := filepath.Join(testDir, "terraform") + + // Call NewTerraform with an empty working directory path. + _, err := NewTerraform(testcontext.New(t), "", execPath) + require.Error(t, err) + require.Contains(t, err.Error(), "Terraform cannot be initialised with empty workdir") +} + +func TestNewTerraform_InvalidWorkingDirPath(t *testing.T) { + // Create a temporary directory for testing. + testDir := t.TempDir() + execPath := filepath.Join(testDir, "terraform") + + // Call NewTerraform with an empty working directory path. + _, err := NewTerraform(testcontext.New(t), "/invalid-dir", execPath) + require.Error(t, err) + require.Equal(t, err.Error(), "failed to initialize Terraform: error initialising Terraform with workdir /invalid-dir: stat /invalid-dir: no such file or directory") +} + +func TestNewTerraform_EmptyExecPath(t *testing.T) { + // Create a temporary directory for testing. + testDir := t.TempDir() + + // Call NewTerraform with an empty working directory path. + _, err := NewTerraform(testcontext.New(t), testDir, "") + require.Error(t, err) + require.Contains(t, err.Error(), "failed to initialize Terraform: no suitable terraform binary could be found") +} From 2dde25c6cfac1c54a5fe024c6a71f14d8420da5a Mon Sep 17 00:00:00 2001 From: karishma-chawla <74574173+karishma-chawla@users.noreply.github.com> Date: Wed, 1 Nov 2023 18:17:55 -0700 Subject: [PATCH 2/2] Move working directory creation to newTerraform Signed-off-by: Karishma Chawla --- pkg/recipes/terraform/execute.go | 71 +++++++-------------------- pkg/recipes/terraform/execute_test.go | 60 +++------------------- pkg/recipes/terraform/log.go | 56 +++++++++++++++++++++ pkg/recipes/terraform/types.go | 50 +++++++++---------- pkg/recipes/terraform/types_test.go | 56 +++++++++++++++++---- 5 files changed, 148 insertions(+), 145 deletions(-) create mode 100644 pkg/recipes/terraform/log.go diff --git a/pkg/recipes/terraform/execute.go b/pkg/recipes/terraform/execute.go index ec177c9f1c..d6da965835 100644 --- a/pkg/recipes/terraform/execute.go +++ b/pkg/recipes/terraform/execute.go @@ -20,9 +20,6 @@ import ( "context" "errors" "fmt" - "io/fs" - "os" - "path/filepath" "time" install "github.com/hashicorp/hc-install" @@ -43,11 +40,6 @@ import ( "k8s.io/client-go/kubernetes" ) -const ( - executionSubDir = "deploy" - workingDirFileMode fs.FileMode = 0700 -) - var ( // ErrRecipeNameEmpty is the error when the recipe name is empty. ErrRecipeNameEmpty = errors.New("recipe name cannot be empty") @@ -90,26 +82,20 @@ func (e *executor) Deploy(ctx context.Context, options Options) (*tfjson.State, return nil, err } - // Create Working Directory - workingDir, err := createWorkingDir(ctx, options.RootDir) - if err != nil { - return nil, err - } - - // Create a new instance of tfexec.Terraform with current the Terraform installation path - tf, err := NewTerraform(ctx, workingDir, execPath) + // Create a new instance of tfexec.Terraform with current Terraform installation path + tf, err := NewTerraform(ctx, options.RootDir, execPath) if err != nil { return nil, err } // Create Terraform config in the working directory - kubernetesBackendSuffix, err := e.generateConfig(ctx, tf, workingDir, options) + kubernetesBackendSuffix, err := e.generateConfig(ctx, tf, options) if err != nil { return nil, err } // Run TF Init and Apply in the working directory - state, err := initAndApply(ctx, tf, workingDir) + state, err := initAndApply(ctx, tf) if err != nil { return nil, err } @@ -145,20 +131,14 @@ func (e *executor) Delete(ctx context.Context, options Options) error { return err } - // Create Working Directory - workingDir, err := createWorkingDir(ctx, options.RootDir) - if err != nil { - return err - } - - // Create a new instance of tfexec.Terraform with current the Terraform installation path - tf, err := NewTerraform(ctx, workingDir, execPath) + // Create a new instance of tfexec.Terraform with current Terraform installation path + tf, err := NewTerraform(ctx, options.RootDir, execPath) if err != nil { return err } // Create Terraform config in the working directory - kubernetesBackendSuffix, err := e.generateConfig(ctx, tf, workingDir, options) + kubernetesBackendSuffix, err := e.generateConfig(ctx, tf, options) if err != nil { return err } @@ -211,24 +191,18 @@ func (e *executor) GetRecipeMetadata(ctx context.Context, options Options) (map[ return nil, err } - // Create Working Directory - workingDir, err := createWorkingDir(ctx, options.RootDir) + // Create a new instance of tfexec.Terraform with current Terraform installation path + tf, err := NewTerraform(ctx, options.RootDir, execPath) if err != nil { return nil, err } - // Create a new instance of tfexec.Terraform with current the Terraform installation path - tf, err := NewTerraform(ctx, workingDir, execPath) + _, err = getTerraformConfig(ctx, tf.WorkingDir(), options) if err != nil { return nil, err } - _, err = getTerraformConfig(ctx, workingDir, options) - if err != nil { - return nil, err - } - - result, err := downloadAndInspect(ctx, tf, workingDir, options) + result, err := downloadAndInspect(ctx, tf, options) if err != nil { return nil, err } @@ -238,28 +212,17 @@ func (e *executor) GetRecipeMetadata(ctx context.Context, options Options) (map[ }, nil } -func createWorkingDir(ctx context.Context, tfDir string) (string, error) { - logger := ucplog.FromContextOrDiscard(ctx) - - workingDir := filepath.Join(tfDir, executionSubDir) - logger.Info(fmt.Sprintf("Creating Terraform working directory: %q", workingDir)) - if err := os.MkdirAll(workingDir, workingDirFileMode); err != nil { - return "", fmt.Errorf("failed to create working directory for terraform execution: %w", err) - } - - return workingDir, nil -} - // generateConfig generates Terraform configuration with required inputs for the module, providers and backend to be initialized and applied. -func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, workingDir string, options Options) (string, error) { +func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, options Options) (string, error) { logger := ucplog.FromContextOrDiscard(ctx) + workingDir := tf.WorkingDir() tfConfig, err := getTerraformConfig(ctx, workingDir, options) if err != nil { return "", err } - loadedModule, err := downloadAndInspect(ctx, tf, workingDir, options) + loadedModule, err := downloadAndInspect(ctx, tf, options) if err != nil { return "", err } @@ -317,7 +280,7 @@ func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, wor } // downloadAndInspect handles downloading the TF module and retrieving the necessary information -func downloadAndInspect(ctx context.Context, tf *tfexec.Terraform, workingDir string, options Options) (*moduleInspectResult, error) { +func downloadAndInspect(ctx context.Context, tf *tfexec.Terraform, options Options) (*moduleInspectResult, error) { logger := ucplog.FromContextOrDiscard(ctx) // Download the Terraform module to the working directory. @@ -336,7 +299,7 @@ func downloadAndInspect(ctx context.Context, tf *tfexec.Terraform, workingDir st // Load the downloaded module to retrieve providers and variables required by the module. // This is needed to add the appropriate providers config and populate the value of recipe context variable. logger.Info(fmt.Sprintf("Inspecting the downloaded Terraform module: %s", options.EnvRecipe.TemplatePath)) - loadedModule, err := inspectModule(workingDir, options.EnvRecipe.Name) + loadedModule, err := inspectModule(tf.WorkingDir(), options.EnvRecipe.Name) if err != nil { return nil, err } @@ -369,7 +332,7 @@ func getTerraformConfig(ctx context.Context, workingDir string, options Options) } // initAndApply runs Terraform init and apply in the provided working directory. -func initAndApply(ctx context.Context, tf *tfexec.Terraform, workingDir string) (*tfjson.State, error) { +func initAndApply(ctx context.Context, tf *tfexec.Terraform) (*tfjson.State, error) { logger := ucplog.FromContextOrDiscard(ctx) // Initialize Terraform diff --git a/pkg/recipes/terraform/execute_test.go b/pkg/recipes/terraform/execute_test.go index b430d57de4..87e54e8e37 100644 --- a/pkg/recipes/terraform/execute_test.go +++ b/pkg/recipes/terraform/execute_test.go @@ -17,7 +17,6 @@ limitations under the License. package terraform import ( - "os" "path/filepath" "testing" @@ -28,36 +27,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestCreateWorkingDir_Created(t *testing.T) { - // Create a temporary directory for testing. - testDir := t.TempDir() - - expectedWorkingDir := filepath.Join(testDir, executionSubDir) - workingDir, err := createWorkingDir(testcontext.New(t), testDir) - require.NoError(t, err) - require.Equal(t, expectedWorkingDir, workingDir) - - // Assert that the working directory was created. - _, err = os.Stat(workingDir) - require.NoError(t, err) -} - -func TestCreateWorkingDir_Error(t *testing.T) { - // Create a temporary directory for testing. - testDir := t.TempDir() - // Create a read-only directory within the temporary directory. - readOnlyDir := filepath.Join(testDir, "read-only-dir") - err := os.MkdirAll(readOnlyDir, 0555) - require.NoError(t, err) - - // Call createWorkingDir with the read-only directory. - _, err = createWorkingDir(testcontext.New(t), readOnlyDir) - - // Assert that createWorkingDir returns an error. - require.Error(t, err) - require.Contains(t, err.Error(), "failed to create working directory") -} - func TestGenerateConfig(t *testing.T) { configTests := []struct { name string @@ -73,17 +42,6 @@ func TestGenerateConfig(t *testing.T) { }, }, err: ErrRecipeNameEmpty.Error(), - }, { - name: "invalid working dir", - workingDir: "/invalid-dir", - opts: Options{ - EnvRecipe: &recipes.EnvironmentDefinition{ - Name: "test-recipe", - TemplatePath: "test/module/source", - }, - ResourceRecipe: &recipes.ResourceMetadata{}, - }, - err: "error creating file: open /invalid-dir/main.tf.json", }, } @@ -93,10 +51,11 @@ func TestGenerateConfig(t *testing.T) { if tc.workingDir == "" { tc.workingDir = t.TempDir() } - tf, _ := tfexec.NewTerraform(tc.workingDir, filepath.Join(tc.workingDir, "terraform")) + tf, err := tfexec.NewTerraform(tc.workingDir, filepath.Join(tc.workingDir, "terraform")) + require.NoError(t, err) e := executor{} - _, err := e.generateConfig(ctx, tf, tc.workingDir, tc.opts) + _, err = e.generateConfig(ctx, tf, tc.opts) require.Error(t, err) require.ErrorContains(t, err, tc.err) }) @@ -107,8 +66,6 @@ func Test_GetTerraformConfig(t *testing.T) { // Create a temporary directory for testing. testDir := t.TempDir() - workingDir, err := createWorkingDir(testcontext.New(t), testDir) - require.NoError(t, err) options := Options{ EnvRecipe: &recipes.EnvironmentDefinition{ Name: "test-recipe", @@ -121,7 +78,7 @@ func Test_GetTerraformConfig(t *testing.T) { Module: map[string]config.TFModuleConfig{ "test-recipe": {"source": "test/module/source"}}, } - tfConfig, err := getTerraformConfig(testcontext.New(t), workingDir, options) + tfConfig, err := getTerraformConfig(testcontext.New(t), testDir, options) require.NoError(t, err) require.Equal(t, &expectedConfig, tfConfig) } @@ -130,8 +87,6 @@ func Test_GetTerraformConfig_EmptyRecipeName(t *testing.T) { // Create a temporary directory for testing. testDir := t.TempDir() - workingDir, err := createWorkingDir(testcontext.New(t), testDir) - require.NoError(t, err) options := Options{ EnvRecipe: &recipes.EnvironmentDefinition{ Name: "", @@ -140,14 +95,13 @@ func Test_GetTerraformConfig_EmptyRecipeName(t *testing.T) { ResourceRecipe: &recipes.ResourceMetadata{}, } - _, err = getTerraformConfig(testcontext.New(t), workingDir, options) + _, err := getTerraformConfig(testcontext.New(t), testDir, options) require.Error(t, err) require.Equal(t, err, ErrRecipeNameEmpty) } func Test_GetTerraformConfig_InvalidDirectory(t *testing.T) { - // Create a temporary directory for testing. - workingDir := "invalid directory" + workingDir := "invalid-directory" options := Options{ EnvRecipe: &recipes.EnvironmentDefinition{ Name: "test-recipe", @@ -158,5 +112,5 @@ func Test_GetTerraformConfig_InvalidDirectory(t *testing.T) { _, err := getTerraformConfig(testcontext.New(t), workingDir, options) require.Error(t, err) - require.Contains(t, err.Error(), "error creating file: open invalid directory/main.tf.json: no such file or directory") + require.Contains(t, err.Error(), "error creating file: open invalid-directory/main.tf.json: no such file or directory") } diff --git a/pkg/recipes/terraform/log.go b/pkg/recipes/terraform/log.go new file mode 100644 index 0000000000..a236990f3f --- /dev/null +++ b/pkg/recipes/terraform/log.go @@ -0,0 +1,56 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package terraform + +import ( + "context" + + "github.com/go-logr/logr" + "github.com/hashicorp/terraform-exec/tfexec" + "github.com/radius-project/radius/pkg/ucp/ucplog" +) + +// tfLogWrapper is a wrapper around the Terraform logger to stream the logs to the Radius logger. +type tfLogWrapper struct { + logger logr.Logger + isStdErr bool +} + +// Write implements the io.Writer interface to stream the Terraform logs to the Radius logger. +func (w *tfLogWrapper) Write(p []byte) (n int, err error) { + if w.isStdErr { + w.logger.Error(nil, string(p)) + } else { + w.logger.Info(string(p)) + } + + return len(p), nil +} + +// configureTerraformLogs configures the Terraform logs to be streamed to the Radius logs. +func configureTerraformLogs(ctx context.Context, tf *tfexec.Terraform) { + logger := ucplog.FromContextOrDiscard(ctx) + + err := tf.SetLog("TRACE") + if err != nil { + logger.Error(err, "Failed to set log level for Terraform") + return + } + + tf.SetStdout(&tfLogWrapper{logger: logger}) + tf.SetStderr(&tfLogWrapper{logger: logger, isStdErr: true}) +} diff --git a/pkg/recipes/terraform/types.go b/pkg/recipes/terraform/types.go index 64ebe7a610..b69ef26bdf 100644 --- a/pkg/recipes/terraform/types.go +++ b/pkg/recipes/terraform/types.go @@ -19,16 +19,22 @@ package terraform import ( "context" "fmt" + "io/fs" + "os" + "path/filepath" - "github.com/go-logr/logr" "github.com/hashicorp/terraform-exec/tfexec" tfjson "github.com/hashicorp/terraform-json" "github.com/radius-project/radius/pkg/recipes" "github.com/radius-project/radius/pkg/ucp/ucplog" ) -//go:generate mockgen -destination=./mock_executor.go -package=terraform -self_package github.com/radius-project/radius/pkg/recipes/terraform github.com/radius-project/radius/pkg/recipes/terraform TerraformExecutor +const ( + executionSubDir = "deploy" + workingDirFileMode fs.FileMode = 0700 +) +//go:generate mockgen -destination=./mock_executor.go -package=terraform -self_package github.com/radius-project/radius/pkg/recipes/terraform github.com/radius-project/radius/pkg/recipes/terraform TerraformExecutor type TerraformExecutor interface { // Deploy installs terraform and runs terraform init and apply on the terraform module referenced by the recipe using terraform-exec. Deploy(ctx context.Context, options Options) (*tfjson.State, error) @@ -56,8 +62,13 @@ type Options struct { ResourceRecipe *recipes.ResourceMetadata } -// NewTerraform creates a new Terraform executor with Terraform logs enabled. -func NewTerraform(ctx context.Context, workingDir, execPath string) (*tfexec.Terraform, error) { +// NewTerraform creates a working directory for Terraform execution and new Terraform executor with Terraform logs enabled. +func NewTerraform(ctx context.Context, tfRootDir, execPath string) (*tfexec.Terraform, error) { + workingDir, err := createWorkingDir(ctx, tfRootDir) + if err != nil { + return nil, err + } + tf, err := tfexec.NewTerraform(workingDir, execPath) if err != nil { return nil, fmt.Errorf("failed to initialize Terraform: %w", err) @@ -68,32 +79,15 @@ func NewTerraform(ctx context.Context, workingDir, execPath string) (*tfexec.Ter return tf, nil } -// tfLogWrapper is a wrapper around the Terraform logger to stream the logs to the Radius logger. -type tfLogWrapper struct { - logger logr.Logger - isStdErr bool -} - -// Write implements the io.Writer interface to stream the Terraform logs to the Radius logger. -func (w *tfLogWrapper) Write(p []byte) (n int, err error) { - if w.isStdErr { - w.logger.Error(nil, string(p)) - } else { - w.logger.Info(string(p)) - } - return len(p), nil -} - -// configureTerraformLogs configures the Terraform logs to be streamed to the Radius logs. -func configureTerraformLogs(ctx context.Context, tf *tfexec.Terraform) { +// createWorkingDir creates a working directory for Terraform execution. +func createWorkingDir(ctx context.Context, tfDir string) (string, error) { logger := ucplog.FromContextOrDiscard(ctx) - err := tf.SetLog("TRACE") - if err != nil { - logger.Error(err, "Failed to set log level for Terraform") - return + workingDir := filepath.Join(tfDir, executionSubDir) + logger.Info(fmt.Sprintf("Creating Terraform working directory: %q", workingDir)) + if err := os.MkdirAll(workingDir, workingDirFileMode); err != nil { + return "", fmt.Errorf("failed to create working directory for terraform execution: %w", err) } - tf.SetStdout(&tfLogWrapper{logger: logger}) - tf.SetStderr(&tfLogWrapper{logger: logger, isStdErr: true}) + return workingDir, nil } diff --git a/pkg/recipes/terraform/types_test.go b/pkg/recipes/terraform/types_test.go index 76b6ce3ebe..5a497f4ef4 100644 --- a/pkg/recipes/terraform/types_test.go +++ b/pkg/recipes/terraform/types_test.go @@ -17,6 +17,7 @@ limitations under the License. package terraform import ( + "os" "path/filepath" "testing" @@ -24,34 +25,69 @@ import ( "github.com/stretchr/testify/require" ) -func TestNewTerraform_EmptyWorkingDirPath(t *testing.T) { +func TestNewTerraform_Success(t *testing.T) { // Create a temporary directory for testing. testDir := t.TempDir() execPath := filepath.Join(testDir, "terraform") + expectedWorkingDir := filepath.Join(testDir, executionSubDir) - // Call NewTerraform with an empty working directory path. - _, err := NewTerraform(testcontext.New(t), "", execPath) - require.Error(t, err) - require.Contains(t, err.Error(), "Terraform cannot be initialised with empty workdir") + tf, err := NewTerraform(testcontext.New(t), testDir, execPath) + require.NoError(t, err) + require.Equal(t, expectedWorkingDir, tf.WorkingDir()) } -func TestNewTerraform_InvalidWorkingDirPath(t *testing.T) { +func TestNewTerraform_InvalidDir(t *testing.T) { // Create a temporary directory for testing. testDir := t.TempDir() + // Create a read-only directory within the temporary directory. + readOnlyDir := filepath.Join(testDir, "read-only-dir") + err := os.MkdirAll(readOnlyDir, 0555) + require.NoError(t, err) + execPath := filepath.Join(testDir, "terraform") - // Call NewTerraform with an empty working directory path. - _, err := NewTerraform(testcontext.New(t), "/invalid-dir", execPath) + // Call NewTerraform with read only root directory. + _, err = NewTerraform(testcontext.New(t), readOnlyDir, execPath) require.Error(t, err) - require.Equal(t, err.Error(), "failed to initialize Terraform: error initialising Terraform with workdir /invalid-dir: stat /invalid-dir: no such file or directory") + require.Contains(t, err.Error(), "failed to create working directory for terraform execution") } func TestNewTerraform_EmptyExecPath(t *testing.T) { // Create a temporary directory for testing. testDir := t.TempDir() - // Call NewTerraform with an empty working directory path. + // Call NewTerraform with an empty exec path. _, err := NewTerraform(testcontext.New(t), testDir, "") require.Error(t, err) require.Contains(t, err.Error(), "failed to initialize Terraform: no suitable terraform binary could be found") } + +func TestCreateWorkingDir_Created(t *testing.T) { + // Create a temporary directory for testing. + testDir := t.TempDir() + + expectedWorkingDir := filepath.Join(testDir, executionSubDir) + workingDir, err := createWorkingDir(testcontext.New(t), testDir) + require.NoError(t, err) + require.Equal(t, expectedWorkingDir, workingDir) + + // Assert that the working directory was created. + _, err = os.Stat(workingDir) + require.NoError(t, err) +} + +func TestCreateWorkingDir_Error(t *testing.T) { + // Create a temporary directory for testing. + testDir := t.TempDir() + // Create a read-only directory within the temporary directory. + readOnlyDir := filepath.Join(testDir, "read-only-dir") + err := os.MkdirAll(readOnlyDir, 0555) + require.NoError(t, err) + + // Call createWorkingDir with the read-only directory. + _, err = createWorkingDir(testcontext.New(t), readOnlyDir) + + // Assert that createWorkingDir returns an error. + require.Error(t, err) + require.Contains(t, err.Error(), "failed to create working directory") +}