Merge branch 'main' into bp/fix-variable-tests

.github/workflows/docs.yml

@@ -15,5 +15,9 @@ jobs:
         with:
           go-version: '1.22'
       - run: go generate main.go
+
+      - name: Stage new and modified files in docs folder
+        run: git add docs/
+
       - name: Check for file changes
-        run: git diff --exit-code
+        run: git diff --cached --exit-code

docs/resources/artifactory_generic_feed.md

@@ -14,7 +14,7 @@ This resource manages a Artifactory Generic feed in Octopus Deploy.
 
 ```terraform
 resource "octopusdeploy_artifactory_generic_feed" "example" {
-  feed_uri                       = "https://example.jfrog.io/"
+  feed_uri                       = "https://example.jfrog.io"
   password                       = "test-password"
   name                           = "Test Artifactory Generic Feed (OK to Delete)"
   username                       = "test-username"

docs/resources/lifecycle.md

@@ -34,7 +34,7 @@ resource "octopusdeploy_lifecycle" "example" {
 
     release_retention_policy {
       quantity_to_keep    = 1
-      should_keep_forever = true
+      should_keep_forever = false
       unit                = "Days"
     }
 

examples/resources/octopusdeploy_artifactory_generic_feed/resource.tf

@@ -1,5 +1,5 @@
 resource "octopusdeploy_artifactory_generic_feed" "example" {
-  feed_uri                       = "https://example.jfrog.io/"
+  feed_uri                       = "https://example.jfrog.io"
   password                       = "test-password"
   name                           = "Test Artifactory Generic Feed (OK to Delete)"
   username                       = "test-username"

examples/resources/octopusdeploy_lifecycle/resource.tf

@@ -20,7 +20,7 @@ resource "octopusdeploy_lifecycle" "example" {
 
     release_retention_policy {
       quantity_to_keep    = 1
-      should_keep_forever = true
+      should_keep_forever = false
       unit                = "Days"
     }
 

octopusdeploy_framework/resource_environment.go

@@ -123,6 +123,7 @@ func (r *environmentTypeResource) Update(ctx context.Context, req resource.Updat
 	updatedEnv.AllowDynamicInfrastructure = data.AllowDynamicInfrastructure.ValueBool()
 	updatedEnv.UseGuidedFailure = data.UseGuidedFailure.ValueBool()
 	updatedEnv.SortOrder = util.GetNumber(data.SortOrder)
+	updatedEnv.SpaceID = data.SpaceID.ValueString()
 	if len(data.JiraExtensionSettings.Elements()) > 0 {
 		jiraExtensionSettings := mapJiraExtensionSettings(data.JiraExtensionSettings)
 		if jiraExtensionSettings != nil {

octopusdeploy_framework/resource_library_variable_set_test.go

@@ -124,6 +124,77 @@ func TestAccOctopusDeployLibraryVariableSetWithUpdate(t *testing.T) {
 	})
 }
 
+func TestAccOctopusDeployLibraryVariableSetWithVariable(t *testing.T) {
+	localName := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha)
+	prefix := "octopusdeploy_library_variable_set." + localName
+
+	variableLocalName := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha)
+	variablePrefix := "octopusdeploy_variable." + variableLocalName
+
+	name := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha)
+	description := "Test variable set"
+	variableName := "Test.Variable " + name
+
+	resource.Test(t, resource.TestCase{
+		CheckDestroy:             testLibraryVariableSetDestroy,
+		PreCheck:                 func() { TestAccPreCheck(t) },
+		ProtoV6ProviderFactories: ProtoV6ProviderFactories(),
+		Steps: []resource.TestStep{
+			{
+				Config: testLibraryVariableSetWithVariable(localName, variableLocalName, name, description, variableName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckOctopusDeployLibraryVariableSetExists(prefix),
+					resource.TestCheckResourceAttr(prefix, "name", name),
+					resource.TestCheckResourceAttr(prefix, "description", description),
+					resource.TestCheckResourceAttr(variablePrefix, "name", variableName),
+					resource.TestCheckResourceAttr(variablePrefix, "type", "String"),
+					resource.TestCheckResourceAttr(variablePrefix, "description", "Test variable"),
+					resource.TestCheckResourceAttr(variablePrefix, "is_sensitive", "false"),
+					resource.TestCheckResourceAttr(variablePrefix, "is_editable", "true"),
+					resource.TestCheckResourceAttr(variablePrefix, "value", "True"),
+					resource.TestCheckResourceAttr(variablePrefix, "prompt.0.description", "test description"),
+					resource.TestCheckResourceAttr(variablePrefix, "prompt.0.label", "test label"),
+					resource.TestCheckResourceAttr(variablePrefix, "prompt.0.is_required", "true"),
+					resource.TestCheckResourceAttr(variablePrefix, "prompt.0.display_settings.0.control_type", "Select"),
+					resource.TestCheckResourceAttr(variablePrefix, "prompt.0.display_settings.0.select_option.0.display_name", "hi"),
+					resource.TestCheckResourceAttr(variablePrefix, "prompt.0.display_settings.0.select_option.0.value", "there"),
+				),
+			},
+		},
+	})
+}
+
+func testLibraryVariableSetWithVariable(localName, variableLocalName, name, description, variableName string) string {
+	return fmt.Sprintf(`
+resource "octopusdeploy_library_variable_set" "%s" {
+  name = "%s"
+  description = "%s"
+}
+
+resource "octopusdeploy_variable" "%s" {
+  name = "%s"
+  type = "String"
+  description = "Test variable"
+  is_sensitive = false
+  is_editable = true
+  owner_id = octopusdeploy_library_variable_set.%s.id
+  value = "True"
+
+  prompt {
+    description = "test description"
+    label       = "test label"
+    is_required = true
+    display_settings {
+      control_type = "Select"
+      select_option {
+        display_name = "hi"
+        value = "there"
+      }
+    }
+  }
+}
+`, localName, name, description, variableLocalName, variableName, localName)
+}
 func testLibraryVariableSetBasic(localName string, name string) string {
 	return fmt.Sprintf(`resource "octopusdeploy_library_variable_set" "%s" {
 		name = "%s"

octopusdeploy_framework/resource_space.go

@@ -3,6 +3,7 @@ package octopusdeploy_framework
 import (
 	"context"
 	"fmt"
+	"github.com/hashicorp/terraform-plugin-framework/attr"
 	"strings"
 
 	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/spaces"
@@ -109,11 +110,9 @@ func (s *spaceResource) Create(ctx context.Context, req resource.CreateRequest,
 		return
 	}
 
-	data.SpaceManagersTeams, diags = types.SetValueFrom(ctx, types.StringType, removeSpaceManagers(ctx, createdSpace.SpaceManagersTeams))
-	if diags.HasError() {
-		resp.Diagnostics.Append(diags...)
-		return
-	}
+	effectiveTeams := s.getEffectiveTeams(ctx, createdSpace)
+	data.SpaceManagersTeams = types.SetValueMust(types.StringType, effectiveTeams)
+
 	tflog.Debug(ctx, fmt.Sprintf("state space %#v", data))
 	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
 	util.Created(ctx, "space")
@@ -143,7 +142,9 @@ func (s *spaceResource) Read(ctx context.Context, req resource.ReadRequest, resp
 	data.IsTaskQueueStopped = types.BoolValue(spaceResult.TaskQueueStopped)
 	data.IsDefault = types.BoolValue(spaceResult.IsDefault)
 	data.SpaceManagersTeamMembers, _ = types.SetValueFrom(ctx, types.StringType, spaceResult.SpaceManagersTeamMembers)
-	data.SpaceManagersTeams, _ = types.SetValueFrom(ctx, types.StringType, removeSpaceManagers(ctx, spaceResult.SpaceManagersTeams))
+
+	effectiveTeams := s.getEffectiveTeams(ctx, spaceResult)
+	data.SpaceManagersTeams = types.SetValueMust(types.StringType, effectiveTeams)
 
 	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
 	tflog.Info(ctx, fmt.Sprintf("space read (%s)", data.ID))
@@ -209,12 +210,22 @@ func (s *spaceResource) Update(ctx context.Context, req resource.UpdateRequest,
 	plan.IsTaskQueueStopped = types.BoolValue(spaceResult.TaskQueueStopped)
 	plan.IsDefault = types.BoolValue(spaceResult.IsDefault)
 	plan.SpaceManagersTeamMembers, _ = types.SetValueFrom(ctx, types.StringType, spaceResult.SpaceManagersTeamMembers)
-	plan.SpaceManagersTeams, _ = types.SetValueFrom(ctx, types.StringType, removeSpaceManagers(ctx, updatedSpace.SpaceManagersTeams))
+
+	effectiveTeams := s.getEffectiveTeams(ctx, updatedSpace)
+	plan.SpaceManagersTeams = types.SetValueMust(types.StringType, effectiveTeams)
 
 	// save plan to state
 	resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...)
 }
 
+func (s *spaceResource) getEffectiveTeams(ctx context.Context, updatedSpace *spaces.Space) []attr.Value {
+	effectiveTeamMembers := make([]attr.Value, 0)
+	for _, t := range removeSpaceManagers(ctx, updatedSpace.SpaceManagersTeams) {
+		effectiveTeamMembers = append(effectiveTeamMembers, types.StringValue(t))
+	}
+	return effectiveTeamMembers
+}
+
 func (s *spaceResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
 	var data schemas.SpaceModel
 	resp.Diagnostics.Append(req.State.Get(ctx, &data)...)

octopusdeploy_framework/schemas/lifecycle.go

@@ -1,14 +1,19 @@
 package schemas
 
 import (
+	"context"
 	"github.com/OctopusDeploy/terraform-provider-octopusdeploy/octopusdeploy_framework/util"
+	"github.com/hashicorp/terraform-plugin-framework-validators/int64validator"
 	datasourceSchema "github.com/hashicorp/terraform-plugin-framework/datasource/schema"
 	resourceSchema "github.com/hashicorp/terraform-plugin-framework/resource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
+	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
+	"github.com/hashicorp/terraform-plugin-framework/tfsdk"
 	"github.com/hashicorp/terraform-plugin-framework/types"
+	"strings"
 )
 
 func GetResourceLifecycleSchema() resourceSchema.Schema {
@@ -28,6 +33,21 @@ func GetResourceLifecycleSchema() resourceSchema.Schema {
 	}
 }
 
+func GetDatasourceLifecycleSchema() datasourceSchema.Schema {
+	return datasourceSchema.Schema{
+		Description: "Provides information about existing lifecycles.",
+		Attributes: map[string]datasourceSchema.Attribute{
+			"id":           util.DataSourceString().Computed().Description("The ID of the lifecycle.").Build(),
+			"space_id":     util.DataSourceString().Optional().Description("The space ID associated with this lifecycle.").Build(),
+			"ids":          util.DataSourceList(types.StringType).Optional().Description("A list of lifecycle IDs to filter by.").Build(),
+			"partial_name": util.DataSourceString().Optional().Description("A partial name to filter lifecycles by.").Build(),
+			"skip":         util.DataSourceInt64().Optional().Description("A filter to specify the number of items to skip in the response.").Build(),
+			"take":         util.DataSourceInt64().Optional().Description("A filter to specify the number of items to take (or return) in the response.").Build(),
+			"lifecycles":   getLifecyclesAttribute(),
+		},
+	}
+}
+
 func getResourcePhaseBlockSchema() resourceSchema.ListNestedBlock {
 	return resourceSchema.ListNestedBlock{
 		Description: "Defines a phase in the lifecycle.",
@@ -70,6 +90,7 @@ func getResourceRetentionPolicyBlockSchema() resourceSchema.ListNestedBlock {
 				"quantity_to_keep": util.ResourceInt64().
 					Optional().Computed().
 					Default(int64default.StaticInt64(30)).
+					Validators(int64validator.AtLeast(0)).
 					Description("The number of days/releases to keep. The default value is 30. If 0 then all are kept.").
 					Build(),
 				"should_keep_forever": util.ResourceBool().
@@ -83,21 +104,9 @@ func getResourceRetentionPolicyBlockSchema() resourceSchema.ListNestedBlock {
 					Description("The unit of quantity to keep. Valid units are Days or Items. The default value is Days.").
 					Build(),
 			},
-		},
-	}
-}
-
-func GetDatasourceLifecycleSchema() datasourceSchema.Schema {
-	return datasourceSchema.Schema{
-		Description: "Provides information about existing lifecycles.",
-		Attributes: map[string]datasourceSchema.Attribute{
-			"id":           util.DataSourceString().Computed().Description("The ID of the lifecycle.").Build(),
-			"space_id":     util.DataSourceString().Optional().Description("The space ID associated with this lifecycle.").Build(),
-			"ids":          util.DataSourceList(types.StringType).Optional().Description("A list of lifecycle IDs to filter by.").Build(),
-			"partial_name": util.DataSourceString().Optional().Description("A partial name to filter lifecycles by.").Build(),
-			"skip":         util.DataSourceInt64().Optional().Description("A filter to specify the number of items to skip in the response.").Build(),
-			"take":         util.DataSourceInt64().Optional().Description("A filter to specify the number of items to take (or return) in the response.").Build(),
-			"lifecycles":   getLifecyclesAttribute(),
+			Validators: []validator.Object{
+				retentionPolicyValidator{},
+			},
 		},
 	}
 }
@@ -149,3 +158,57 @@ func getRetentionPolicyAttribute() datasourceSchema.ListNestedAttribute {
 		},
 	}
 }
+
+type retentionPolicyValidator struct{}
+
+func (v retentionPolicyValidator) Description(ctx context.Context) string {
+	return "validates that should_keep_forever is true only if quantity_to_keep is 0"
+}
+
+func (v retentionPolicyValidator) MarkdownDescription(ctx context.Context) string {
+	return v.Description(ctx)
+}
+
+func (v retentionPolicyValidator) ValidateObject(ctx context.Context, req validator.ObjectRequest, resp *validator.ObjectResponse) {
+	var retentionPolicy struct {
+		QuantityToKeep    types.Int64  `tfsdk:"quantity_to_keep"`
+		ShouldKeepForever types.Bool   `tfsdk:"should_keep_forever"`
+		Unit              types.String `tfsdk:"unit"`
+	}
+
+	diags := tfsdk.ValueAs(ctx, req.ConfigValue, &retentionPolicy)
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	if !retentionPolicy.QuantityToKeep.IsNull() && !retentionPolicy.ShouldKeepForever.IsNull() {
+		quantityToKeep := retentionPolicy.QuantityToKeep.ValueInt64()
+		shouldKeepForever := retentionPolicy.ShouldKeepForever.ValueBool()
+
+		if quantityToKeep == 0 && !shouldKeepForever {
+			resp.Diagnostics.AddAttributeError(
+				req.Path.AtName("should_keep_forever"),
+				"Invalid retention policy configuration",
+				"should_keep_forever must be true when quantity_to_keep is 0",
+			)
+		} else if quantityToKeep != 0 && shouldKeepForever {
+			resp.Diagnostics.AddAttributeError(
+				req.Path.AtName("should_keep_forever"),
+				"Invalid retention policy configuration",
+				"should_keep_forever must be false when quantity_to_keep is not 0",
+			)
+		}
+	}
+
+	if !retentionPolicy.Unit.IsNull() {
+		unit := retentionPolicy.Unit.ValueString()
+		if !strings.EqualFold(unit, "Days") && !strings.EqualFold(unit, "Items") {
+			resp.Diagnostics.AddAttributeError(
+				req.Path.AtName("unit"),
+				"Invalid retention policy unit",
+				"Unit must be either 'Days' or 'Items' (case insensitive)",
+			)
+		}
+	}
+}

octopusdeploy_framework/schemas/variable_prompt_options.go

@@ -117,10 +117,10 @@ func MapFromDisplaySettings(displaySettings *resources.DisplaySettings) attr.Val
 }
 
 func MapFromSelectOptions(selectOptions []*resources.SelectOption) []attr.Value {
-	options := make([]attr.Value, len(selectOptions))
+	options := make([]attr.Value, 0, len(selectOptions))
 	for _, option := range selectOptions {
 		options = append(options, types.ObjectValueMust(
-			VariableDisplaySettingsObjectType(),
+			VariableSelectOptionsObjectType(),
 			map[string]attr.Value{
 				VariableSchemaAttributeNames.Value:       types.StringValue(option.Value),
 				VariableSchemaAttributeNames.DisplayName: types.StringValue(option.DisplayName),