From ed7ed31dc810ef720ca8f514aa60f575ee7f1140 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 18:09:10 +0000 Subject: [PATCH 1/2] Initial plan From 60b5a50e6eb8abda369e6b3a82bcc1f19ea2482c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 18:23:12 +0000 Subject: [PATCH 2/2] fix: compute recipe context before terraform get to satisfy required argument validation Terraform 1.14.9 validates required module arguments during `terraform get`. The upstream kubernetes-redis module requires a `context` argument, but it was only being added AFTER the download step. This moves the recipe context computation to BEFORE the download, ensuring required arguments are present when terraform validates during module download. After download and inspection, if the module does not declare a context variable, it is removed from the config to avoid 'unexpected argument' errors during init/apply. Agent-Logs-Url: https://github.com/radius-project/radius/sessions/da1fcb4d-e5e9-4735-822f-cc86911c2544 Co-authored-by: sylvainsf <540991+sylvainsf@users.noreply.github.com> --- pkg/recipes/terraform/config/config.go | 8 +++ pkg/recipes/terraform/config/config_test.go | 38 +++++++++++ pkg/recipes/terraform/execute.go | 71 +++++++++++++-------- pkg/recipes/terraform/execute_test.go | 8 ++- 4 files changed, 95 insertions(+), 30 deletions(-) diff --git a/pkg/recipes/terraform/config/config.go b/pkg/recipes/terraform/config/config.go index 38c14dd5b35..36ce1ee43cd 100644 --- a/pkg/recipes/terraform/config/config.go +++ b/pkg/recipes/terraform/config/config.go @@ -199,6 +199,14 @@ func (cfg *TerraformConfig) AddRecipeContext(ctx context.Context, moduleName str return nil } +// RemoveRecipeContext removes the recipe context parameter from the module configuration. +// This is used when the downloaded module does not declare a context variable. +func (cfg *TerraformConfig) RemoveRecipeContext(moduleName string) { + if mod, ok := cfg.Module[moduleName]; ok { + delete(mod, recipecontext.RecipeContextParamKey) + } +} + // newModuleConfig creates a new TFModuleConfig object with the given module source and version // and also populates RecipeParams in TF module config. If same parameter key exists across params // then the last map specified gets precedence. diff --git a/pkg/recipes/terraform/config/config_test.go b/pkg/recipes/terraform/config/config_test.go index 35c32627d2d..40d48392cd0 100644 --- a/pkg/recipes/terraform/config/config_test.go +++ b/pkg/recipes/terraform/config/config_test.go @@ -302,6 +302,44 @@ func Test_AddRecipeContext(t *testing.T) { } } +func Test_RemoveRecipeContext(t *testing.T) { + ctx := testcontext.New(t) + + envdef := &recipes.EnvironmentDefinition{ + Name: testRecipeName, + TemplatePath: testTemplatePath, + TemplateVersion: testTemplateVersion, + Parameters: envParams, + } + metadata := &recipes.ResourceMetadata{ + Name: testRecipeName, + Parameters: resourceParams, + } + + tfconfig, err := New(context.Background(), testRecipeName, envdef, metadata) + require.NoError(t, err) + + // Add recipe context + err = tfconfig.AddRecipeContext(ctx, testRecipeName, getTestRecipeContext()) + require.NoError(t, err) + + // Verify context is present + mod := tfconfig.Module[testRecipeName] + _, hasContext := mod[recipecontext.RecipeContextParamKey] + require.True(t, hasContext) + + // Remove recipe context + tfconfig.RemoveRecipeContext(testRecipeName) + + // Verify context is removed + _, hasContext = mod[recipecontext.RecipeContextParamKey] + require.False(t, hasContext) + + // Verify other params are still present + _, hasSource := mod[moduleSourceKey] + require.True(t, hasSource) +} + func Test_AddProviders(t *testing.T) { mProvider, ucpConfiguredProviders, mBackend := setup(t) envRecipe, resourceRecipe := getTestInputs() diff --git a/pkg/recipes/terraform/execute.go b/pkg/recipes/terraform/execute.go index 7eabad6542b..f6f564d7c78 100644 --- a/pkg/recipes/terraform/execute.go +++ b/pkg/recipes/terraform/execute.go @@ -179,11 +179,28 @@ func (e *executor) GetRecipeMetadata(ctx context.Context, options Options) (map[ return nil, err } - _, err = getTerraformConfig(ctx, tf.WorkingDir(), options) + tfConfig, err := getTerraformConfig(ctx, tf.WorkingDir(), options) if err != nil { return nil, err } + // Pre-compute the recipe context and include it in the module config before downloading. + // Terraform validates required module arguments during "terraform get". + recipectx, err := recipecontext.New(options.ResourceRecipe, options.EnvConfig) + if err != nil { + return nil, err + } + if options.ResourceRecipe != nil { + recipectx.Resource.Connections = options.ResourceRecipe.ConnectedResourcesProperties + } + if err = tfConfig.AddRecipeContext(ctx, options.EnvRecipe.Name, recipectx); err != nil { + return nil, err + } + + if err := tfConfig.Save(ctx, tf.WorkingDir()); err != nil { + return nil, err + } + result, err := downloadAndInspect(ctx, tf, options) if err != nil { return nil, err @@ -260,11 +277,37 @@ func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, opt return "", err } + // Pre-compute the recipe context and include it in the module config before downloading. + // Terraform validates required module arguments during "terraform get", so the context + // must be present in the config before the module is downloaded. + // After download and inspection, the context will be removed if the module does not declare it. + recipectx, err := recipecontext.New(options.ResourceRecipe, options.EnvConfig) + if err != nil { + return "", err + } + if options.ResourceRecipe != nil { + recipectx.Resource.Connections = options.ResourceRecipe.ConnectedResourcesProperties + } + if err = tfConfig.AddRecipeContext(ctx, options.EnvRecipe.Name, recipectx); err != nil { + return "", err + } + + // Save the config with recipe context included before downloading the module. + if err := tfConfig.Save(ctx, workingDir); err != nil { + return "", err + } + loadedModule, err := downloadAndInspect(ctx, tf, options) if err != nil { return "", err } + // After inspection, remove the recipe context if the module does not declare the context variable. + // Terraform will reject unexpected arguments during init/apply. + if !loadedModule.ContextVarExists { + tfConfig.RemoveRecipeContext(options.EnvRecipe.Name) + } + // Generate Terraform providers configuration for required providers and add it to the Terraform configuration. logger.Info(fmt.Sprintf("Adding provider config for required providers %+v", loadedModule.RequiredProviders)) if err := tfConfig.AddProviders(ctx, loadedModule.RequiredProviders, providers.GetUCPConfiguredTerraformProviders(e.ucpConn, e.secretProvider), @@ -292,26 +335,6 @@ func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, opt } } - // Add recipe context parameter to the generated Terraform config's module parameters. - // This should only be added if the recipe context variable is declared in the downloaded module. - if loadedModule.ContextVarExists { - logger.Info("Adding recipe context module result") - - // Create the recipe context object to be passed to the recipe deployment - recipectx, err := recipecontext.New(options.ResourceRecipe, options.EnvConfig) - if err != nil { - return "", err - } - - //update the recipe context with connected resources properties - if options.ResourceRecipe != nil { - recipectx.Resource.Connections = options.ResourceRecipe.ConnectedResourcesProperties - } - - if err = tfConfig.AddRecipeContext(ctx, options.EnvRecipe.Name, recipectx); err != nil { - return "", err - } - } if loadedModule.ResultOutputExists { if err = tfConfig.AddOutputs(options.EnvRecipe.Name); err != nil { return "", err @@ -346,12 +369,6 @@ func getTerraformConfig(ctx context.Context, workingDir string, options Options) return nil, err } - // Before downloading the module, Teraform configuration needs to be persisted in the working directory. - // Terraform Get command uses this config file to download module from the source specified in the config. - if err := tfConfig.Save(ctx, workingDir); err != nil { - return nil, err - } - return tfConfig, nil } diff --git a/pkg/recipes/terraform/execute_test.go b/pkg/recipes/terraform/execute_test.go index df9a09d0051..324a134494d 100644 --- a/pkg/recipes/terraform/execute_test.go +++ b/pkg/recipes/terraform/execute_test.go @@ -115,6 +115,8 @@ func Test_GetTerraformConfig_EmptyRecipeName(t *testing.T) { } func Test_GetTerraformConfig_InvalidDirectory(t *testing.T) { + // getTerraformConfig no longer saves the file; it only creates the in-memory config. + // An invalid directory should not cause an error because saving happens separately. workingDir := "invalid-directory" options := Options{ EnvRecipe: &recipes.EnvironmentDefinition{ @@ -124,9 +126,9 @@ func Test_GetTerraformConfig_InvalidDirectory(t *testing.T) { ResourceRecipe: &recipes.ResourceMetadata{}, } - _, 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") + cfg, err := getTerraformConfig(testcontext.New(t), workingDir, options) + require.NoError(t, err) + require.NotNil(t, cfg) } func TestSetEnvironmentVariables(t *testing.T) {