From 5aa239f0e219a67196d545d79794403f06c46815 Mon Sep 17 00:00:00 2001 From: Ben Pearce Date: Wed, 4 Sep 2024 17:25:37 +1000 Subject: [PATCH] fix: read the space id from the plan rather than the state for a variable (#776) chore!: removed references to redundant encryption attributes --- docs/resources/variable.md | 3 - octopusdeploy_framework/resource_variable.go | 43 ++++++------ .../resource_variable_test.go | 66 ++++++++++++------- octopusdeploy_framework/schemas/variable.go | 24 ------- 4 files changed, 64 insertions(+), 72 deletions(-) diff --git a/docs/resources/variable.md b/docs/resources/variable.md index 0ff293df1..6b4c2408b 100644 --- a/docs/resources/variable.md +++ b/docs/resources/variable.md @@ -104,7 +104,6 @@ resource "octopusdeploy_variable" "prompted_variable" { - `is_editable` (Boolean) Indicates whether or not this variable is considered editable. - `is_sensitive` (Boolean) Indicates whether or not this resource is considered sensitive and should be kept secret. - `owner_id` (String) -- `pgp_key` (String, Sensitive) - `project_id` (String, Deprecated) - `prompt` (Block List) (see [below for nested schema](#nestedblock--prompt)) - `scope` (Block List) (see [below for nested schema](#nestedblock--scope)) @@ -114,9 +113,7 @@ resource "octopusdeploy_variable" "prompted_variable" { ### Read-Only -- `encrypted_value` (String) - `id` (String) The unique ID for this resource. -- `key_fingerprint` (String) ### Nested Schema for `prompt` diff --git a/octopusdeploy_framework/resource_variable.go b/octopusdeploy_framework/resource_variable.go index 96a842f14..dd31c49fc 100644 --- a/octopusdeploy_framework/resource_variable.go +++ b/octopusdeploy_framework/resource_variable.go @@ -141,41 +141,41 @@ func (r *variableTypeResource) Update(ctx context.Context, req resource.UpdateRe internal.Mutex.Lock() defer internal.Mutex.Unlock() - var data, state schemas.VariableTypeResourceModel - resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...) + var plan, state schemas.VariableTypeResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) resp.Diagnostics.Append(req.State.Get(ctx, &state)...) if resp.Diagnostics.HasError() { return } - tflog.Info(ctx, fmt.Sprintf("updating variable (%s)", data.ID)) + tflog.Info(ctx, fmt.Sprintf("updating variable (%s)", plan.ID)) - variableOwnerId, err := getVariableOwnerID(&data) + variableOwnerId, err := getVariableOwnerID(&plan) if err != nil { resp.Diagnostics.AddError("invalid resource configuration", err.Error()) return } - name := data.Name.ValueString() + name := plan.Name.ValueString() updatedVariable := variables.NewVariable(name) - updatedVariable.Description = data.Description.ValueString() - updatedVariable.IsEditable = data.IsEditable.ValueBool() - updatedVariable.IsSensitive = data.IsSensitive.ValueBool() - updatedVariable.Type = data.Type.ValueString() - updatedVariable.Scope = schemas.MapToVariableScope(data.Scope) - updatedVariable.Prompt = schemas.MapToVariablePromptOptions(data.Prompt) - updatedVariable.SpaceID = state.SpaceID.ValueString() + updatedVariable.Description = plan.Description.ValueString() + updatedVariable.IsEditable = plan.IsEditable.ValueBool() + updatedVariable.IsSensitive = plan.IsSensitive.ValueBool() + updatedVariable.Type = plan.Type.ValueString() + updatedVariable.Scope = schemas.MapToVariableScope(plan.Scope) + updatedVariable.Prompt = schemas.MapToVariablePromptOptions(plan.Prompt) + updatedVariable.SpaceID = plan.SpaceID.ValueString() if updatedVariable.IsSensitive { updatedVariable.Type = schemas.VariableTypeNames.Sensitive - updatedVariable.Value = data.SensitiveValue.ValueString() + updatedVariable.Value = plan.SensitiveValue.ValueString() } else { - updatedVariable.Value = data.Value.ValueString() + updatedVariable.Value = plan.Value.ValueString() } updatedVariable.ID = state.ID.ValueString() - variableSet, err := variables.UpdateSingle(r.Config.Client, state.SpaceID.ValueString(), variableOwnerId.ValueString(), updatedVariable) + variableSet, err := variables.UpdateSingle(r.Config.Client, plan.SpaceID.ValueString(), variableOwnerId.ValueString(), updatedVariable) if err != nil { resp.Diagnostics.AddError("update variable failed", err.Error()) return @@ -187,10 +187,10 @@ func (r *variableTypeResource) Update(ctx context.Context, req resource.UpdateRe return } - tflog.Info(ctx, fmt.Sprintf("variable updated (%s)", data.ID)) + tflog.Info(ctx, fmt.Sprintf("variable updated (%s)", plan.ID)) - mapVariableToState(&data, updatedVariable) - resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) + mapVariableToState(&plan, updatedVariable) + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } func (r *variableTypeResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { @@ -296,15 +296,14 @@ func mapVariableToState(data *schemas.VariableTypeResourceModel, variable *varia ) } - if !data.Scope.IsNull() { + if variable.Scope.IsEmpty() { + data.Scope = types.ListNull(types.ObjectType{AttrTypes: schemas.VariableScopeObjectType()}) + } else { data.Scope = types.ListValueMust( types.ObjectType{AttrTypes: schemas.VariableScopeObjectType()}, []attr.Value{schemas.MapFromVariableScope(variable.Scope)}, ) } - data.EncryptedValue = types.StringNull() - data.KeyFingerprint = types.StringNull() - data.ID = types.StringValue(variable.GetID()) } diff --git a/octopusdeploy_framework/resource_variable_test.go b/octopusdeploy_framework/resource_variable_test.go index d71cc6dd8..6262f2708 100644 --- a/octopusdeploy_framework/resource_variable_test.go +++ b/octopusdeploy_framework/resource_variable_test.go @@ -2,13 +2,13 @@ package octopusdeploy_framework import ( "fmt" + internalTest "github.com/OctopusDeploy/terraform-provider-octopusdeploy/internal/test" "path/filepath" "strings" "testing" "time" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/variables" - internalTest "github.com/OctopusDeploy/terraform-provider-octopusdeploy/internal/test" "github.com/OctopusSolutionsEngineering/OctopusTerraformTestFramework/octoclient" "github.com/OctopusSolutionsEngineering/OctopusTerraformTestFramework/test" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" @@ -17,7 +17,7 @@ import ( ) func TestAccOctopusDeployVariableBasic(t *testing.T) { - internalTest.SkipCI(t, "Octopus API error: The resource you requested was not found. []") + internalTest.SkipCI(t, "After applying this test step, the refresh plan was not empty. scope.tenant_tags") localName := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha) prefix := "octopusdeploy_variable." + localName @@ -43,6 +43,7 @@ func TestAccOctopusDeployVariableBasic(t *testing.T) { projectName := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha) spaceID := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha) + spaceName := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha) resource.Test(t, resource.TestCase{ CheckDestroy: testVariableDestroy, @@ -58,10 +59,10 @@ func TestAccOctopusDeployVariableBasic(t *testing.T) { resource.TestCheckResourceAttr(prefix, "type", variableType), resource.TestCheckResourceAttr(prefix, "value", value), resource.TestCheckResourceAttr(prefix, "scope.#", "1"), - resource.TestCheckResourceAttr(prefix, "scope.0.%", "6"), + resource.TestCheckResourceAttr(prefix, "scope.0.%", "7"), resource.TestCheckResourceAttr(prefix, "scope.0.environments.#", "1"), ), - Config: testVariableBasic(spaceID, environmentLocalName, environmentName, lifecycleLocalName, lifecycleName, projectGroupLocalName, projectGroupName, projectLocalName, projectName, channelLocalName, channelName, localName, name, description, isSensitive, value, variableType), + Config: testVariableBasic(spaceID, spaceName, environmentLocalName, environmentName, lifecycleLocalName, lifecycleName, projectGroupLocalName, projectGroupName, projectLocalName, projectName, channelLocalName, channelName, localName, name, description, isSensitive, value, variableType), }, { Check: resource.ComposeTestCheckFunc( @@ -75,7 +76,7 @@ func TestAccOctopusDeployVariableBasic(t *testing.T) { resource.TestCheckResourceAttr(prefix, "scope.0.%", "6"), resource.TestCheckResourceAttr(prefix, "scope.0.environments.#", "1"), ), - Config: testVariableBasic(spaceID, environmentLocalName, environmentName, lifecycleLocalName, lifecycleName, projectGroupLocalName, projectGroupName, projectLocalName, projectName, channelLocalName, channelName, localName, name, description, isSensitive, newValue, variableType), + Config: testVariableBasic(spaceID, spaceName, environmentLocalName, environmentName, lifecycleLocalName, lifecycleName, projectGroupLocalName, projectGroupName, projectLocalName, projectName, channelLocalName, channelName, localName, name, description, isSensitive, newValue, variableType), }, { Check: resource.ComposeTestCheckFunc( @@ -90,16 +91,16 @@ func TestAccOctopusDeployVariableBasic(t *testing.T) { resource.TestCheckResourceAttr(prefix, "scope.0.environments.#", "1"), ), Config: fmt.Sprintf(`%s - -%s`, + + %s`, testGcpAccount(localName, name, acctest.RandStringFromCharSet(20, acctest.CharSetAlpha)), - testVariableBasic(spaceID, environmentLocalName, environmentName, lifecycleLocalName, lifecycleName, projectGroupLocalName, projectGroupName, projectLocalName, projectName, channelLocalName, channelName, localName, name, description, isSensitive, accountValue, accountVariableType)), + testVariableBasic(spaceID, spaceName, environmentLocalName, environmentName, lifecycleLocalName, lifecycleName, projectGroupLocalName, projectGroupName, projectLocalName, projectName, channelLocalName, channelName, localName, name, description, isSensitive, accountValue, accountVariableType)), }, }, }) } -func testVariableBasic(spaceID string, environmentLocalName string, +func testVariableBasic(spaceID string, spaceName string, environmentLocalName string, environmentName string, lifecycleLocalName string, lifecycleName string, projectGroupLocalName string, projectGroupName string, projectLocalName string, projectName string, channelLocalName string, channelName string, localName string, name string, description string, isSensitive bool, value string, variableType string) string { return fmt.Sprintf(`%s @@ -112,6 +113,8 @@ func testVariableBasic(spaceID string, environmentLocalName string, %s + %s + resource "octopusdeploy_variable" "%s" { description = "%s" is_sensitive = "%v" @@ -125,12 +128,14 @@ func testVariableBasic(spaceID string, environmentLocalName string, environments = [octopusdeploy_environment.%s.id] tenant_tags = [] } + space_id = octopusdeploy_space.%s.id }`, - createEnvironment(environmentLocalName, environmentName), - createLifecycle(lifecycleLocalName, lifecycleName), - createProjectGroup(projectGroupLocalName, projectGroupName), + createSpace(spaceID, spaceName), + createEnvironment(spaceID, environmentLocalName, environmentName), + createLifecycle(spaceID, lifecycleLocalName, lifecycleName), + createProjectGroup(spaceID, projectGroupLocalName, projectGroupName), createProject(spaceID, projectLocalName, projectName, lifecycleLocalName, projectGroupLocalName), - createChannel(channelLocalName, channelName, projectLocalName), + createChannel(spaceID, channelLocalName, channelName, projectLocalName), localName, description, isSensitive, @@ -140,6 +145,7 @@ func testVariableBasic(spaceID string, environmentLocalName string, value, channelLocalName, environmentLocalName, + spaceID, ) } @@ -150,22 +156,32 @@ func testGcpAccount(localName string, name string, jsonKey string) string { }`, localName, jsonKey, name) } -func createEnvironment(localName, name string) string { +func createSpace(localname string, name string) string { + return fmt.Sprintf(`resource "octopusdeploy_space" "%s" { + name = "%s" + space_managers_teams = ["teams-administrators"] +}`, localname, name) +} + +func createEnvironment(spaceId, localName, name string) string { return fmt.Sprintf(`resource "octopusdeploy_environment" "%s" { name = "%s" - }`, localName, name) + space_id = octopusdeploy_space.%s.id + }`, localName, name, spaceId) } -func createLifecycle(localName, name string) string { +func createLifecycle(spaceId, localName, name string) string { return fmt.Sprintf(`resource "octopusdeploy_lifecycle" "%s" { name = "%s" - }`, localName, name) + space_id = octopusdeploy_space.%s.id + }`, localName, name, spaceId) } -func createProjectGroup(localName, name string) string { +func createProjectGroup(spaceId, localName, name string) string { return fmt.Sprintf(`resource "octopusdeploy_project_group" "%s" { name = "%s" - }`, localName, name) + space_id = octopusdeploy_space.%s.id + }`, localName, name, spaceId) } func createProject(spaceID string, localName, name, lifecycleLocal, projectGroupLocal string) string { @@ -173,34 +189,38 @@ func createProject(spaceID string, localName, name, lifecycleLocal, projectGroup name = "%s" lifecycle_id = octopusdeploy_lifecycle.%s.id project_group_id = octopusdeploy_project_group.%s.id - space_id = "%s" + space_id = octopusdeploy_space.%s.id }`, localName, name, lifecycleLocal, projectGroupLocal, spaceID) } -func createChannel(localName, name, projectLocal string) string { +func createChannel(spaceId string, localName, name, projectLocal string) string { return fmt.Sprintf(`resource "octopusdeploy_channel" "%s" { name = "%s" project_id = octopusdeploy_project.%s.id - }`, localName, name, projectLocal) + space_id = octopusdeploy_space.%s.id + }`, localName, name, projectLocal, spaceId) } func testAccCheckVariableExists() resource.TestCheckFunc { return func(s *terraform.State) error { var ownerID string var variableID string + var spaceID string for _, r := range s.RootModule().Resources { if r.Type == "octopusdeploy_project" { ownerID = r.Primary.ID + spaceID = r.Primary.Attributes["space_id"] } if r.Type == "octopusdeploy_variable" { ownerID = r.Primary.Attributes["owner_id"] + spaceID = r.Primary.Attributes["space_id"] variableID = r.Primary.ID } } - if _, err := octoClient.Variables.GetByID(ownerID, variableID); err != nil { + if _, err := variables.GetByID(octoClient, spaceID, ownerID, variableID); err != nil { return fmt.Errorf("error retrieving variable %s", err) } diff --git a/octopusdeploy_framework/schemas/variable.go b/octopusdeploy_framework/schemas/variable.go index 9d2bb6cef..9d2ffb6c4 100644 --- a/octopusdeploy_framework/schemas/variable.go +++ b/octopusdeploy_framework/schemas/variable.go @@ -9,8 +9,6 @@ import ( datasourceSchema "github.com/hashicorp/terraform-plugin-framework/datasource/schema" "github.com/hashicorp/terraform-plugin-framework/path" resourceSchema "github.com/hashicorp/terraform-plugin-framework/resource/schema" - "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" - "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" ) @@ -36,9 +34,6 @@ var VariableSchemaAttributeNames = struct { DisplayName string IsRequired string Label string - EncryptedValue string - KeyFingerprint string - PgpKey string }{ Prompt: "prompt", OwnerID: "owner_id", @@ -55,9 +50,6 @@ var VariableSchemaAttributeNames = struct { DisplayName: "display_name", IsRequired: "is_required", Label: "label", - EncryptedValue: "encrypted_value", - KeyFingerprint: "key_fingerprint", - PgpKey: "pgp_key", } var VariableTypeNames = struct { @@ -173,19 +165,6 @@ func (v VariableSchema) GetResourceSchema() resourceSchema.Schema { "Indicates whether or not this resource is considered sensitive and should be kept secret.", false, true), - VariableSchemaAttributeNames.KeyFingerprint: resourceSchema.StringAttribute{ - Computed: true, - }, - VariableSchemaAttributeNames.PgpKey: resourceSchema.StringAttribute{ - Optional: true, - Sensitive: true, - PlanModifiers: []planmodifier.String{ - stringplanmodifier.RequiresReplace(), - }, - }, - VariableSchemaAttributeNames.EncryptedValue: resourceSchema.StringAttribute{ - Computed: true, - }, VariableSchemaAttributeNames.SensitiveValue: resourceSchema.StringAttribute{ Optional: true, Sensitive: true, @@ -226,9 +205,6 @@ type VariableTypeResourceModel struct { Type types.String `tfsdk:"type"` SensitiveValue types.String `tfsdk:"sensitive_value"` Value types.String `tfsdk:"value"` - PgpKey types.String `tfsdk:"pgp_key"` - KeyFingerprint types.String `tfsdk:"key_fingerprint"` - EncryptedValue types.String `tfsdk:"encrypted_value"` Prompt types.List `tfsdk:"prompt"` Scope types.List `tfsdk:"scope"` SpaceID types.String `tfsdk:"space_id"`