Skip to content

Commit

Permalink
fix: read the space id from the plan rather than the state for a vari…
Browse files Browse the repository at this point in the history
…able (#776)

chore!: removed references to redundant encryption attributes
  • Loading branch information
benPearce1 authored Sep 4, 2024
1 parent 7e1d38b commit 5aa239f
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 72 deletions.
3 changes: 0 additions & 3 deletions docs/resources/variable.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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)

<a id="nestedblock--prompt"></a>
### Nested Schema for `prompt`
Expand Down
43 changes: 21 additions & 22 deletions octopusdeploy_framework/resource_variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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())
}
66 changes: 43 additions & 23 deletions octopusdeploy_framework/resource_variable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand All @@ -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,
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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
Expand All @@ -112,6 +113,8 @@ func testVariableBasic(spaceID string, environmentLocalName string,
%s
%s
resource "octopusdeploy_variable" "%s" {
description = "%s"
is_sensitive = "%v"
Expand All @@ -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,
Expand All @@ -140,6 +145,7 @@ func testVariableBasic(spaceID string, environmentLocalName string,
value,
channelLocalName,
environmentLocalName,
spaceID,
)
}

Expand All @@ -150,57 +156,71 @@ 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 {
return fmt.Sprintf(`resource "octopusdeploy_project" "%s" {
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)
}

Expand Down
24 changes: 0 additions & 24 deletions octopusdeploy_framework/schemas/variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -36,9 +34,6 @@ var VariableSchemaAttributeNames = struct {
DisplayName string
IsRequired string
Label string
EncryptedValue string
KeyFingerprint string
PgpKey string
}{
Prompt: "prompt",
OwnerID: "owner_id",
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"`
Expand Down

0 comments on commit 5aa239f

Please sign in to comment.