Skip to content

Commit 4b2ae00

Browse files
committed
cleanup
1 parent 6245a90 commit 4b2ae00

File tree

4 files changed

+214
-289
lines changed

4 files changed

+214
-289
lines changed

octopusdeploy_framework/resource_tenant_common_variable.go

Lines changed: 55 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/variables"
1212
"github.com/OctopusDeploy/terraform-provider-octopusdeploy/octopusdeploy_framework/schemas"
1313
"github.com/OctopusDeploy/terraform-provider-octopusdeploy/octopusdeploy_framework/util"
14+
"github.com/hashicorp/terraform-plugin-framework/diag"
1415
"github.com/hashicorp/terraform-plugin-framework/path"
1516
"github.com/hashicorp/terraform-plugin-framework/resource"
1617
"github.com/hashicorp/terraform-plugin-framework/types"
@@ -89,6 +90,55 @@ func isTemplateControlTypeSensitive(displaySettings map[string]string) bool {
8990
return displaySettings["Octopus.ControlType"] == "Sensitive"
9091
}
9192

93+
func (t *tenantCommonVariableResource) validateScopeSupport(planScope []tenantCommonVariableScopeModel, diags *diag.Diagnostics) bool {
94+
if len(planScope) > 0 && !t.supportsV2() {
95+
diags.AddError(
96+
"Scope block is not supported",
97+
"The 'scope' block requires V2 API support. Your Octopus Server does not support this feature.",
98+
)
99+
return false
100+
}
101+
return true
102+
}
103+
104+
func commonVariableMatchesPlan(variable variables.TenantCommonVariable, planLibrarySetID, planTemplateID string, planScope []tenantCommonVariableScopeModel) bool {
105+
if variable.LibraryVariableSetId != planLibrarySetID || variable.TemplateID != planTemplateID {
106+
return false
107+
}
108+
109+
if len(planScope) == 0 {
110+
return len(variable.Scope.EnvironmentIds) == 0
111+
}
112+
113+
return scopesMatch(planScope[0].EnvironmentIDs, variable.Scope.EnvironmentIds)
114+
}
115+
116+
func scopesMatch(planEnvIDs types.Set, serverEnvIDs []string) bool {
117+
if planEnvIDs.IsNull() || planEnvIDs.IsUnknown() {
118+
return len(serverEnvIDs) == 0
119+
}
120+
121+
planEnvironments := make([]types.String, 0, len(planEnvIDs.Elements()))
122+
planEnvIDs.ElementsAs(context.Background(), &planEnvironments, false)
123+
124+
if len(planEnvironments) != len(serverEnvIDs) {
125+
return false
126+
}
127+
128+
planEnvSet := make(map[string]bool)
129+
for _, e := range planEnvironments {
130+
planEnvSet[e.ValueString()] = true
131+
}
132+
133+
for _, serverEnv := range serverEnvIDs {
134+
if !planEnvSet[serverEnv] {
135+
return false
136+
}
137+
}
138+
139+
return true
140+
}
141+
92142
func (t *tenantCommonVariableResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
93143
var plan tenantCommonVariableResourceModel
94144
resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
@@ -99,11 +149,7 @@ func (t *tenantCommonVariableResource) Create(ctx context.Context, req resource.
99149
internal.KeyedMutex.Lock(plan.TenantID.ValueString())
100150
defer internal.KeyedMutex.Unlock(plan.TenantID.ValueString())
101151

102-
if len(plan.Scope) > 0 && !t.supportsV2() {
103-
resp.Diagnostics.AddError(
104-
"Scoped tenant variables are not supported",
105-
"The scope block requires V2 API support (CommonVariableScopingFeatureToggle). Your Octopus Server does not support this feature.",
106-
)
152+
if !t.validateScopeSupport(plan.Scope, &resp.Diagnostics) {
107153
return
108154
}
109155

@@ -244,39 +290,11 @@ func (t *tenantCommonVariableResource) createV2(ctx context.Context, plan *tenan
244290
return
245291
}
246292

247-
// Find the created variable and get its ID
248293
var createdID string
249294
for _, v := range updateResp.Variables {
250-
if v.LibraryVariableSetId == plan.LibraryVariableSetID.ValueString() && v.TemplateID == plan.TemplateID.ValueString() {
251-
// Also match on scope to handle multiple variables with same template but different scopes
252-
if len(plan.Scope) > 0 {
253-
if len(v.Scope.EnvironmentIds) > 0 {
254-
planEnvs := plan.Scope[0].EnvironmentIDs.Elements()
255-
if len(planEnvs) == len(v.Scope.EnvironmentIds) {
256-
match := true
257-
planEnvSet := make(map[string]bool)
258-
for _, e := range planEnvs {
259-
planEnvSet[e.String()] = true
260-
}
261-
for _, serverEnv := range v.Scope.EnvironmentIds {
262-
if !planEnvSet["\""+serverEnv+"\""] {
263-
match = false
264-
break
265-
}
266-
}
267-
if match {
268-
createdID = v.GetID()
269-
break
270-
}
271-
}
272-
}
273-
} else {
274-
// No scope in plan, match unscoped variable
275-
if len(v.Scope.EnvironmentIds) == 0 {
276-
createdID = v.GetID()
277-
break
278-
}
279-
}
295+
if commonVariableMatchesPlan(v, plan.LibraryVariableSetID.ValueString(), plan.TemplateID.ValueString(), plan.Scope) {
296+
createdID = v.GetID()
297+
break
280298
}
281299
}
282300

@@ -314,9 +332,7 @@ func (t *tenantCommonVariableResource) Read(ctx context.Context, req resource.Re
314332
spaceID = tenant.SpaceID
315333
}
316334

317-
// Determine which API version to use based on ID format
318335
isV1ID := isCompositeID(state.ID.ValueString())
319-
320336
if isV1ID && t.supportsV2() {
321337
t.migrateV1ToV2OnRead(ctx, &state, spaceID, resp)
322338
} else if !isV1ID {
@@ -415,22 +431,18 @@ func (t *tenantCommonVariableResource) migrateV1ToV2OnRead(ctx context.Context,
415431
return
416432
}
417433

418-
// Find the variable
419434
var found bool
420435
for _, v := range getResp.Variables {
421436
if v.LibraryVariableSetId == state.LibraryVariableSetID.ValueString() && v.TemplateID == state.TemplateID.ValueString() {
422-
// Migrate to V2 ID
423437
state.ID = types.StringValue(v.GetID())
424438

425-
// Update scope
426439
if len(v.Scope.EnvironmentIds) > 0 {
427440
envSet := util.BuildStringSetOrEmpty(v.Scope.EnvironmentIds)
428441
state.Scope = []tenantCommonVariableScopeModel{{EnvironmentIDs: envSet}}
429442
} else {
430443
state.Scope = nil
431444
}
432445

433-
// Update value if not sensitive
434446
isSensitive := isTemplateControlTypeSensitive(v.Template.DisplaySettings)
435447
if !isSensitive {
436448
state.Value = types.StringValue(v.Value.Value)
@@ -457,12 +469,7 @@ func (t *tenantCommonVariableResource) Update(ctx context.Context, req resource.
457469
internal.KeyedMutex.Lock(plan.TenantID.ValueString())
458470
defer internal.KeyedMutex.Unlock(plan.TenantID.ValueString())
459471

460-
// Validate scope block usage on unsupported servers
461-
if len(plan.Scope) > 0 && !t.supportsV2() {
462-
resp.Diagnostics.AddError(
463-
"V2 API not supported",
464-
"The 'scope' block requires V2 API support (CommonVariableScopingFeatureToggle). Your Octopus Server version does not support this feature. Please upgrade your server or remove the scope block.",
465-
)
472+
if !t.validateScopeSupport(plan.Scope, &resp.Diagnostics) {
466473
return
467474
}
468475

octopusdeploy_framework/resource_tenant_common_variable_test.go

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,6 @@ func testTenantCommonVariableExists(resourceName string) resource.TestCheckFunc
127127
return fmt.Errorf("Library variable ID is not set")
128128
}
129129

130-
// Check if this is a V2 ID (no colons) or V1 ID (has colons)
131130
if !strings.Contains(rs.Primary.ID, ":") {
132131
tenantID := rs.Primary.Attributes["tenant_id"]
133132
spaceID := rs.Primary.Attributes["space_id"]
@@ -144,7 +143,6 @@ func testTenantCommonVariableExists(resourceName string) resource.TestCheckFunc
144143
return fmt.Errorf("Error retrieving tenant common variables: %s", err.Error())
145144
}
146145

147-
// Search for the variable by ID
148146
for _, v := range getResp.Variables {
149147
if v.GetID() == rs.Primary.ID {
150148
return nil
@@ -154,7 +152,6 @@ func testTenantCommonVariableExists(resourceName string) resource.TestCheckFunc
154152
return fmt.Errorf("Tenant common variable with ID %s not found via V2 API", rs.Primary.ID)
155153
}
156154

157-
// V1 API - use composite ID
158155
importStrings := strings.Split(rs.Primary.ID, ":")
159156
if len(importStrings) != 3 {
160157
return fmt.Errorf("octopusdeploy_tenant_common_variable import must be in the form of TenantID:LibraryVariableSetID:VariableID (e.g. Tenants-123:LibraryVariableSets-456:6c9f2ba3-3ccd-407f-bbdf-6618e4fd0a0c")
@@ -190,9 +187,7 @@ func testAccTenantCommonVariableCheckDestroy(s *terraform.State) error {
190187
continue
191188
}
192189

193-
// Check if this is a V2 ID (no colons) or V1 ID (has colons)
194190
if !strings.Contains(rs.Primary.ID, ":") {
195-
// V2 API - use real ID
196191
tenantID := rs.Primary.Attributes["tenant_id"]
197192
spaceID := rs.Primary.Attributes["space_id"]
198193

@@ -205,11 +200,9 @@ func testAccTenantCommonVariableCheckDestroy(s *terraform.State) error {
205200

206201
getResp, err := tenants.GetCommonVariables(client, query)
207202
if err != nil {
208-
// If we can't get the variables, assume they're gone
209203
return nil
210204
}
211205

212-
// Search for the variable by ID
213206
for _, v := range getResp.Variables {
214207
if v.GetID() == rs.Primary.ID {
215208
return fmt.Errorf("Tenant common variable (%s) still exists", rs.Primary.ID)
@@ -219,7 +212,6 @@ func testAccTenantCommonVariableCheckDestroy(s *terraform.State) error {
219212
continue
220213
}
221214

222-
// V1 API - use composite ID
223215
importStrings := strings.Split(rs.Primary.ID, ":")
224216
if len(importStrings) != 3 {
225217
return fmt.Errorf("octopusdeploy_tenant_common_variable import must be in the form of TenantID:LibraryVariableSetID:VariableID (e.g. Tenants-123:LibraryVariableSets-456:6c9f2ba3-3ccd-407f-bbdf-6618e4fd0a0c")
@@ -276,7 +268,6 @@ func TestAccTenantCommonVariableMigration(t *testing.T) {
276268
PreCheck: func() { TestAccPreCheck(t) },
277269
Steps: []resource.TestStep{
278270
{
279-
// Step 1: Create with V1 API (force V2 API off)
280271
ProtoV6ProviderFactories: ProtoV6ProviderFactoriesWithFeatureToggleOverrides(map[string]bool{
281272
"CommonVariableScopingFeatureToggle": false,
282273
}),
@@ -286,7 +277,6 @@ func TestAccTenantCommonVariableMigration(t *testing.T) {
286277
resource.TestCheckNoResourceAttr(resourceName, "scope.#"),
287278
func(s *terraform.State) error {
288279
rs := s.RootModule().Resources[resourceName]
289-
// Verify it's a V1 composite ID (has colons)
290280
if !strings.Contains(rs.Primary.ID, ":") {
291281
return fmt.Errorf("Expected V1 composite ID with colons, got: %s", rs.Primary.ID)
292282
}
@@ -296,7 +286,6 @@ func TestAccTenantCommonVariableMigration(t *testing.T) {
296286
Config: testAccTenantCommonVariableMigrationV1(lifecycleLocalName, lifecycleName, projectGroupLocalName, projectGroupName, projectLocalName, projectName, env1LocalName, env1Name, env2LocalName, env2Name, tenantLocalName, tenantName, tenantVariablesLocalName, value),
297287
},
298288
{
299-
// Step 2: Migrate to V2 API by adding scope block (enable V2 API)
300289
ProtoV6ProviderFactories: ProtoV6ProviderFactoriesWithFeatureToggleOverrides(map[string]bool{
301290
"CommonVariableScopingFeatureToggle": true,
302291
}),
@@ -307,7 +296,6 @@ func TestAccTenantCommonVariableMigration(t *testing.T) {
307296
resource.TestCheckResourceAttr(resourceName, "scope.0.environment_ids.#", "2"),
308297
func(s *terraform.State) error {
309298
rs := s.RootModule().Resources[resourceName]
310-
// Verify it's a V2 real ID (no colons)
311299
if strings.Contains(rs.Primary.ID, ":") {
312300
return fmt.Errorf("Expected V2 real ID without colons, got: %s", rs.Primary.ID)
313301
}
@@ -317,7 +305,6 @@ func TestAccTenantCommonVariableMigration(t *testing.T) {
317305
Config: testAccTenantCommonVariableMigrationV2(lifecycleLocalName, lifecycleName, projectGroupLocalName, projectGroupName, projectLocalName, projectName, env1LocalName, env1Name, env2LocalName, env2Name, tenantLocalName, tenantName, tenantVariablesLocalName, newValue),
318306
},
319307
{
320-
// Step 3: Update value again to verify it works after migration
321308
ProtoV6ProviderFactories: ProtoV6ProviderFactoriesWithFeatureToggleOverrides(map[string]bool{
322309
"CommonVariableScopingFeatureToggle": true,
323310
}),
@@ -328,7 +315,6 @@ func TestAccTenantCommonVariableMigration(t *testing.T) {
328315
resource.TestCheckResourceAttr(resourceName, "scope.0.environment_ids.#", "2"),
329316
func(s *terraform.State) error {
330317
rs := s.RootModule().Resources[resourceName]
331-
// Verify still using V2 ID
332318
if strings.Contains(rs.Primary.ID, ":") {
333319
return fmt.Errorf("Expected V2 real ID without colons, got: %s", rs.Primary.ID)
334320
}
@@ -476,7 +462,6 @@ func TestAccTenantCommonVariableWithScope(t *testing.T) {
476462
ProtoV6ProviderFactories: ProtoV6ProviderFactories(),
477463
Steps: []resource.TestStep{
478464
{
479-
// Create with scope
480465
Check: resource.ComposeTestCheckFunc(
481466
testTenantCommonVariableExistsV2(resourceName),
482467
resource.TestCheckResourceAttr(resourceName, "value", value),
@@ -486,7 +471,6 @@ func TestAccTenantCommonVariableWithScope(t *testing.T) {
486471
Config: testAccTenantCommonVariableWithScope(lifecycleLocalName, lifecycleName, projectGroupLocalName, projectGroupName, projectLocalName, projectName, env1LocalName, env1Name, env2LocalName, env2Name, tenantLocalName, tenantName, tenantVariablesLocalName, value),
487472
},
488473
{
489-
// Update value
490474
Check: resource.ComposeTestCheckFunc(
491475
testTenantCommonVariableExistsV2(resourceName),
492476
resource.TestCheckResourceAttr(resourceName, "value", newValue),
@@ -566,12 +550,10 @@ func testTenantCommonVariableExistsV2(resourceName string) resource.TestCheckFun
566550
return fmt.Errorf("Tenant common variable ID is not set")
567551
}
568552

569-
// V2 uses real IDs (e.g., TenantVariables-123), not composite IDs with colons
570553
if strings.Contains(rs.Primary.ID, ":") {
571554
return fmt.Errorf("Expected V2 ID (e.g., TenantVariables-123) but got V1 composite ID: %s", rs.Primary.ID)
572555
}
573556

574-
// Use V2 API to verify the variable exists
575557
tenantID := rs.Primary.Attributes["tenant_id"]
576558
spaceID := rs.Primary.Attributes["space_id"]
577559

@@ -587,7 +569,6 @@ func testTenantCommonVariableExistsV2(resourceName string) resource.TestCheckFun
587569
return fmt.Errorf("Error retrieving tenant common variables: %s", err.Error())
588570
}
589571

590-
// Search for the variable by ID
591572
for _, v := range getResp.Variables {
592573
if v.GetID() == rs.Primary.ID {
593574
return nil
@@ -605,9 +586,7 @@ func testAccTenantCommonVariableCheckDestroyV2(s *terraform.State) error {
605586
continue
606587
}
607588

608-
// V2 uses real IDs, not composite IDs
609589
if strings.Contains(rs.Primary.ID, ":") {
610-
// This is a V1 ID, use V1 destroy check
611590
continue
612591
}
613592

@@ -623,11 +602,9 @@ func testAccTenantCommonVariableCheckDestroyV2(s *terraform.State) error {
623602

624603
getResp, err := tenants.GetCommonVariables(client, query)
625604
if err != nil {
626-
// If we can't get the variables, assume they're gone
627605
return nil
628606
}
629607

630-
// Search for the variable by ID
631608
for _, v := range getResp.Variables {
632609
if v.GetID() == rs.Primary.ID {
633610
return fmt.Errorf("Tenant common variable (%s) still exists", rs.Primary.ID)

0 commit comments

Comments
 (0)