From fcc606e8e46d3d36a0dd49a9763a7fa24db72884 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Nov 2025 03:48:26 +0000 Subject: [PATCH 01/11] Initial plan From 668e37034fb96023f2da6279bf53d9bf5c5ed1f1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Nov 2025 04:00:52 +0000 Subject: [PATCH 02/11] Add required_versions attribute to fleet_agent_policy resource Co-authored-by: tobio <444668+tobio@users.noreply.github.com> --- internal/fleet/agent_policy/acc_test.go | 174 ++++++++++++++ internal/fleet/agent_policy/models.go | 139 +++++++++-- .../agent_policy/required_versions_test.go | 222 ++++++++++++++++++ .../agent_policy/required_versions_type.go | 68 ++++++ .../agent_policy/required_versions_value.go | 165 +++++++++++++ internal/fleet/agent_policy/schema.go | 20 ++ 6 files changed, 770 insertions(+), 18 deletions(-) create mode 100644 internal/fleet/agent_policy/required_versions_test.go create mode 100644 internal/fleet/agent_policy/required_versions_type.go create mode 100644 internal/fleet/agent_policy/required_versions_value.go diff --git a/internal/fleet/agent_policy/acc_test.go b/internal/fleet/agent_policy/acc_test.go index 22131dffc..2a4e3a230 100644 --- a/internal/fleet/agent_policy/acc_test.go +++ b/internal/fleet/agent_policy/acc_test.go @@ -799,3 +799,177 @@ resource "elasticstack_fleet_agent_policy" "test_policy" { } `, id) } + +func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { +policyName := sdkacctest.RandStringFromCharSet(22, sdkacctest.CharSetAlphaNum) + +resource.Test(t, resource.TestCase{ +PreCheck: func() { acctest.PreCheck(t) }, +CheckDestroy: checkResourceAgentPolicyDestroy, +ProtoV6ProviderFactories: acctest.Providers, +Steps: []resource.TestStep{ +{ +SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), +Config: testAccResourceAgentPolicyCreateWithRequiredVersions(policyName), +Check: resource.ComposeTestCheckFunc( +resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), +resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), +resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "1"), +resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ +"version": "8.15.0", +"percentage": "100", +}), +), +}, +{ +SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), +Config: testAccResourceAgentPolicyUpdateRequiredVersionsPercentage(policyName), +Check: resource.ComposeTestCheckFunc( +resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), +resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), +resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "1"), +resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ +"version": "8.15.0", +"percentage": "50", +}), +), +}, +{ +SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), +Config: testAccResourceAgentPolicyAddRequiredVersion(policyName), +Check: resource.ComposeTestCheckFunc( +resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), +resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), +resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "2"), +resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ +"version": "8.15.0", +"percentage": "50", +}), +resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ +"version": "8.16.0", +"percentage": "50", +}), +), +}, +{ +SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), +Config: testAccResourceAgentPolicyRemoveRequiredVersions(policyName), +Check: resource.ComposeTestCheckFunc( +resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), +resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), +resource.TestCheckNoResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions"), +), +}, +}, +}) +} + +func testAccResourceAgentPolicyCreateWithRequiredVersions(id string) string { +return fmt.Sprintf(` +provider "elasticstack" { + elasticsearch {} + kibana {} +} + +resource "elasticstack_fleet_agent_policy" "test_policy" { + name = "%s" + namespace = "default" + description = "Test Agent Policy with Required Versions" + monitor_logs = true + monitor_metrics = false + skip_destroy = false + required_versions = [ + { + version = "8.15.0" + percentage = 100 + } + ] +} + +data "elasticstack_fleet_enrollment_tokens" "test_policy" { + policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id +} +`, fmt.Sprintf("Policy %s", id)) +} + +func testAccResourceAgentPolicyUpdateRequiredVersionsPercentage(id string) string { +return fmt.Sprintf(` +provider "elasticstack" { + elasticsearch {} + kibana {} +} + +resource "elasticstack_fleet_agent_policy" "test_policy" { + name = "%s" + namespace = "default" + description = "Test Agent Policy with Required Versions - Updated Percentage" + monitor_logs = true + monitor_metrics = false + skip_destroy = false + required_versions = [ + { + version = "8.15.0" + percentage = 50 + } + ] +} + +data "elasticstack_fleet_enrollment_tokens" "test_policy" { + policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id +} +`, fmt.Sprintf("Policy %s", id)) +} + +func testAccResourceAgentPolicyAddRequiredVersion(id string) string { +return fmt.Sprintf(` +provider "elasticstack" { + elasticsearch {} + kibana {} +} + +resource "elasticstack_fleet_agent_policy" "test_policy" { + name = "%s" + namespace = "default" + description = "Test Agent Policy with Multiple Required Versions" + monitor_logs = true + monitor_metrics = false + skip_destroy = false + required_versions = [ + { + version = "8.15.0" + percentage = 50 + }, + { + version = "8.16.0" + percentage = 50 + } + ] +} + +data "elasticstack_fleet_enrollment_tokens" "test_policy" { + policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id +} +`, fmt.Sprintf("Policy %s", id)) +} + +func testAccResourceAgentPolicyRemoveRequiredVersions(id string) string { +return fmt.Sprintf(` +provider "elasticstack" { + elasticsearch {} + kibana {} +} + +resource "elasticstack_fleet_agent_policy" "test_policy" { + name = "%s" + namespace = "default" + description = "Test Agent Policy without Required Versions" + monitor_logs = true + monitor_metrics = false + skip_destroy = false +} + +data "elasticstack_fleet_enrollment_tokens" "test_policy" { + policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id +} +`, fmt.Sprintf("Policy %s", id)) +} diff --git a/internal/fleet/agent_policy/models.go b/internal/fleet/agent_policy/models.go index 24c854e55..780b8023a 100644 --- a/internal/fleet/agent_policy/models.go +++ b/internal/fleet/agent_policy/models.go @@ -30,24 +30,25 @@ type globalDataTagsItemModel struct { } type agentPolicyModel struct { - ID types.String `tfsdk:"id"` - PolicyID types.String `tfsdk:"policy_id"` - Name types.String `tfsdk:"name"` - Namespace types.String `tfsdk:"namespace"` - Description types.String `tfsdk:"description"` - DataOutputId types.String `tfsdk:"data_output_id"` - MonitoringOutputId types.String `tfsdk:"monitoring_output_id"` - FleetServerHostId types.String `tfsdk:"fleet_server_host_id"` - DownloadSourceId types.String `tfsdk:"download_source_id"` - MonitorLogs types.Bool `tfsdk:"monitor_logs"` - MonitorMetrics types.Bool `tfsdk:"monitor_metrics"` - SysMonitoring types.Bool `tfsdk:"sys_monitoring"` - SkipDestroy types.Bool `tfsdk:"skip_destroy"` - SupportsAgentless types.Bool `tfsdk:"supports_agentless"` - InactivityTimeout customtypes.Duration `tfsdk:"inactivity_timeout"` - UnenrollmentTimeout customtypes.Duration `tfsdk:"unenrollment_timeout"` - GlobalDataTags types.Map `tfsdk:"global_data_tags"` //> globalDataTagsModel - SpaceIds types.Set `tfsdk:"space_ids"` + ID types.String `tfsdk:"id"` + PolicyID types.String `tfsdk:"policy_id"` + Name types.String `tfsdk:"name"` + Namespace types.String `tfsdk:"namespace"` + Description types.String `tfsdk:"description"` + DataOutputId types.String `tfsdk:"data_output_id"` + MonitoringOutputId types.String `tfsdk:"monitoring_output_id"` + FleetServerHostId types.String `tfsdk:"fleet_server_host_id"` + DownloadSourceId types.String `tfsdk:"download_source_id"` + MonitorLogs types.Bool `tfsdk:"monitor_logs"` + MonitorMetrics types.Bool `tfsdk:"monitor_metrics"` + SysMonitoring types.Bool `tfsdk:"sys_monitoring"` + SkipDestroy types.Bool `tfsdk:"skip_destroy"` + SupportsAgentless types.Bool `tfsdk:"supports_agentless"` + InactivityTimeout customtypes.Duration `tfsdk:"inactivity_timeout"` + UnenrollmentTimeout customtypes.Duration `tfsdk:"unenrollment_timeout"` + GlobalDataTags types.Map `tfsdk:"global_data_tags"` //> globalDataTagsModel + SpaceIds types.Set `tfsdk:"space_ids"` + RequiredVersions RequiredVersionsValue `tfsdk:"required_versions"` } func (model *agentPolicyModel) populateFromAPI(ctx context.Context, data *kbapi.AgentPolicy) diag.Diagnostics { @@ -134,6 +135,37 @@ func (model *agentPolicyModel) populateFromAPI(ctx context.Context, data *kbapi. model.SpaceIds = types.SetNull(types.StringType) } + // Handle required_versions + if data.RequiredVersions != nil && len(*data.RequiredVersions) > 0 { + elemType := getRequiredVersionsElementType() + elements := make([]attr.Value, 0, len(*data.RequiredVersions)) + + for _, rv := range *data.RequiredVersions { + objValue, d := types.ObjectValue( + map[string]attr.Type{ + "version": types.StringType, + "percentage": types.Int32Type, + }, + map[string]attr.Value{ + "version": types.StringValue(rv.Version), + "percentage": types.Int32Value(int32(rv.Percentage)), + }, + ) + if d.HasError() { + return d + } + elements = append(elements, objValue) + } + + reqVersions, d := NewRequiredVersionsValue(elemType, elements) + if d.HasError() { + return d + } + model.RequiredVersions = reqVersions + } else { + model.RequiredVersions = NewRequiredVersionsValueNull(getRequiredVersionsElementType()) + } + return nil } @@ -186,6 +218,63 @@ func (model *agentPolicyModel) convertGlobalDataTags(ctx context.Context, feat f return &itemsList, diags } +// convertRequiredVersions converts the required versions from terraform model to API model +func (model *agentPolicyModel) convertRequiredVersions(ctx context.Context) (*[]struct { + Percentage float32 `json:"percentage"` + Version string `json:"version"` +}, diag.Diagnostics) { + var diags diag.Diagnostics + + if model.RequiredVersions.IsNull() || model.RequiredVersions.IsUnknown() { + return nil, diags + } + + elements := model.RequiredVersions.Elements() + if len(elements) == 0 { + return nil, diags + } + + result := make([]struct { + Percentage float32 `json:"percentage"` + Version string `json:"version"` + }, 0, len(elements)) + + for _, elem := range elements { + obj, ok := elem.(types.Object) + if !ok { + diags.AddError("required_versions conversion error", fmt.Sprintf("Expected ObjectValue, got %T", elem)) + continue + } + + attrs := obj.Attributes() + versionAttr := attrs["version"].(types.String) + percentageAttr := attrs["percentage"].(types.Int32) + + if versionAttr.IsNull() || versionAttr.IsUnknown() { + diags.AddError("required_versions validation error", "version cannot be null or unknown") + continue + } + if percentageAttr.IsNull() || percentageAttr.IsUnknown() { + diags.AddError("required_versions validation error", "percentage cannot be null or unknown") + continue + } + + result = append(result, struct { + Percentage float32 `json:"percentage"` + Version string `json:"version"` + }{ + Percentage: float32(percentageAttr.ValueInt32()), + Version: versionAttr.ValueString(), + }) + } + + if diags.HasError() { + return nil, diags + } + + return &result, diags +} + func (model *agentPolicyModel) toAPICreateModel(ctx context.Context, feat features) (kbapi.PostFleetAgentPoliciesJSONRequestBody, diag.Diagnostics) { monitoring := make([]kbapi.PostFleetAgentPoliciesJSONBodyMonitoringEnabled, 0, 2) @@ -282,6 +371,13 @@ func (model *agentPolicyModel) toAPICreateModel(ctx context.Context, feat featur body.SpaceIds = &spaceIds } + // Handle required_versions + requiredVersions, d := model.convertRequiredVersions(ctx) + if d.HasError() { + return kbapi.PostFleetAgentPoliciesJSONRequestBody{}, d + } + body.RequiredVersions = requiredVersions + return body, nil } @@ -379,5 +475,12 @@ func (model *agentPolicyModel) toAPIUpdateModel(ctx context.Context, feat featur body.SpaceIds = &spaceIds } + // Handle required_versions + requiredVersions, d := model.convertRequiredVersions(ctx) + if d.HasError() { + return kbapi.PutFleetAgentPoliciesAgentpolicyidJSONRequestBody{}, d + } + body.RequiredVersions = requiredVersions + return body, nil } diff --git a/internal/fleet/agent_policy/required_versions_test.go b/internal/fleet/agent_policy/required_versions_test.go new file mode 100644 index 000000000..6eb5eed0c --- /dev/null +++ b/internal/fleet/agent_policy/required_versions_test.go @@ -0,0 +1,222 @@ +package agent_policy + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRequiredVersionsValue_SetSemanticEquals(t *testing.T) { + ctx := context.Background() + elemType := getRequiredVersionsElementType() + + // Helper function to create a RequiredVersionsValue + createValue := func(versions []struct { + version string + percentage int32 + }) RequiredVersionsValue { + elements := make([]attr.Value, 0, len(versions)) + for _, v := range versions { + obj, diags := types.ObjectValue( + map[string]attr.Type{ + "version": types.StringType, + "percentage": types.Int32Type, + }, + map[string]attr.Value{ + "version": types.StringValue(v.version), + "percentage": types.Int32Value(v.percentage), + }, + ) + require.False(t, diags.HasError()) + elements = append(elements, obj) + } + val, diags := NewRequiredVersionsValue(elemType, elements) + require.False(t, diags.HasError()) + return val + } + + t.Run("equal when same versions regardless of percentage", func(t *testing.T) { + val1 := createValue([]struct { + version string + percentage int32 + }{ + {"8.15.0", 50}, + {"8.16.0", 50}, + }) + + val2 := createValue([]struct { + version string + percentage int32 + }{ + {"8.15.0", 100}, + {"8.16.0", 0}, + }) + + equal, diags := val1.SetSemanticEquals(ctx, val2) + require.False(t, diags.HasError()) + assert.True(t, equal, "Values should be semantically equal when versions match") + }) + + t.Run("not equal when different versions", func(t *testing.T) { + val1 := createValue([]struct { + version string + percentage int32 + }{ + {"8.15.0", 50}, + }) + + val2 := createValue([]struct { + version string + percentage int32 + }{ + {"8.16.0", 50}, + }) + + equal, diags := val1.SetSemanticEquals(ctx, val2) + require.False(t, diags.HasError()) + assert.False(t, equal, "Values should not be equal when versions differ") + }) + + t.Run("not equal when different number of versions", func(t *testing.T) { + val1 := createValue([]struct { + version string + percentage int32 + }{ + {"8.15.0", 50}, + }) + + val2 := createValue([]struct { + version string + percentage int32 + }{ + {"8.15.0", 50}, + {"8.16.0", 50}, + }) + + equal, diags := val1.SetSemanticEquals(ctx, val2) + require.False(t, diags.HasError()) + assert.False(t, equal, "Values should not be equal when number of versions differs") + }) + + t.Run("null values are equal", func(t *testing.T) { + val1 := NewRequiredVersionsValueNull(elemType) + val2 := NewRequiredVersionsValueNull(elemType) + + equal, diags := val1.SetSemanticEquals(ctx, val2) + require.False(t, diags.HasError()) + assert.True(t, equal, "Null values should be equal") + }) + + t.Run("null and non-null are not equal", func(t *testing.T) { + val1 := NewRequiredVersionsValueNull(elemType) + val2 := createValue([]struct { + version string + percentage int32 + }{ + {"8.15.0", 50}, + }) + + equal, diags := val1.SetSemanticEquals(ctx, val2) + require.False(t, diags.HasError()) + assert.False(t, equal, "Null and non-null values should not be equal") + }) + + t.Run("equal when versions in different order", func(t *testing.T) { + val1 := createValue([]struct { + version string + percentage int32 + }{ + {"8.15.0", 50}, + {"8.16.0", 50}, + }) + + val2 := createValue([]struct { + version string + percentage int32 + }{ + {"8.16.0", 100}, + {"8.15.0", 0}, + }) + + equal, diags := val1.SetSemanticEquals(ctx, val2) + require.False(t, diags.HasError()) + assert.True(t, equal, "Values should be semantically equal when versions match in different order") + }) +} + +func TestConvertRequiredVersions(t *testing.T) { + ctx := context.Background() + elemType := getRequiredVersionsElementType() + + t.Run("converts valid required versions", func(t *testing.T) { + elements := []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "version": types.StringType, + "percentage": types.Int32Type, + }, + map[string]attr.Value{ + "version": types.StringValue("8.15.0"), + "percentage": types.Int32Value(50), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "version": types.StringType, + "percentage": types.Int32Type, + }, + map[string]attr.Value{ + "version": types.StringValue("8.16.0"), + "percentage": types.Int32Value(50), + }, + ), + } + + reqVersions, diags := NewRequiredVersionsValue(elemType, elements) + require.False(t, diags.HasError()) + + model := &agentPolicyModel{ + RequiredVersions: reqVersions, + } + + result, diags := model.convertRequiredVersions(ctx) + require.False(t, diags.HasError()) + require.NotNil(t, result) + assert.Len(t, *result, 2) + + // Check that both versions are present + versions := make(map[string]float32) + for _, rv := range *result { + versions[rv.Version] = rv.Percentage + } + assert.Equal(t, float32(50), versions["8.15.0"]) + assert.Equal(t, float32(50), versions["8.16.0"]) + }) + + t.Run("returns nil for null required versions", func(t *testing.T) { + model := &agentPolicyModel{ + RequiredVersions: NewRequiredVersionsValueNull(elemType), + } + + result, diags := model.convertRequiredVersions(ctx) + require.False(t, diags.HasError()) + assert.Nil(t, result) + }) + + t.Run("returns nil for empty required versions", func(t *testing.T) { + reqVersions, diags := NewRequiredVersionsValue(elemType, []attr.Value{}) + require.False(t, diags.HasError()) + + model := &agentPolicyModel{ + RequiredVersions: reqVersions, + } + + result, diags := model.convertRequiredVersions(ctx) + require.False(t, diags.HasError()) + assert.Nil(t, result) + }) +} diff --git a/internal/fleet/agent_policy/required_versions_type.go b/internal/fleet/agent_policy/required_versions_type.go new file mode 100644 index 000000000..5fda38831 --- /dev/null +++ b/internal/fleet/agent_policy/required_versions_type.go @@ -0,0 +1,68 @@ +package agent_policy + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" + "github.com/hashicorp/terraform-plugin-go/tftypes" +) + +var ( + _ basetypes.SetTypable = (*RequiredVersionsType)(nil) +) + +// RequiredVersionsType is a custom set type that enforces uniqueness based on version only. +type RequiredVersionsType struct { + basetypes.SetType +} + +// String returns a human readable string of the type name. +func (t RequiredVersionsType) String() string { + return "agent_policy.RequiredVersionsType" +} + +// ValueType returns the Value type. +func (t RequiredVersionsType) ValueType(ctx context.Context) attr.Value { + return RequiredVersionsValue{ + SetValue: basetypes.NewSetUnknown(t.ElemType), + } +} + +// Equal returns true if the given type is equivalent. +func (t RequiredVersionsType) Equal(o attr.Type) bool { + other, ok := o.(RequiredVersionsType) + if !ok { + return false + } + return t.SetType.Equal(other.SetType) +} + +// ValueFromSet returns a SetValuable type given a SetValue. +func (t RequiredVersionsType) ValueFromSet(ctx context.Context, in basetypes.SetValue) (basetypes.SetValuable, diag.Diagnostics) { + return RequiredVersionsValue{ + SetValue: in, + }, nil +} + +// ValueFromTerraform returns a Value given a tftypes.Value. +func (t RequiredVersionsType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) { + attrValue, err := t.SetType.ValueFromTerraform(ctx, in) + if err != nil { + return nil, err + } + + setValue, ok := attrValue.(basetypes.SetValue) + if !ok { + return nil, fmt.Errorf("unexpected value type of %T", attrValue) + } + + setValuable, diags := t.ValueFromSet(ctx, setValue) + if diags.HasError() { + return nil, fmt.Errorf("unexpected error converting SetValue to SetValuable: %v", diags) + } + + return setValuable, nil +} diff --git a/internal/fleet/agent_policy/required_versions_value.go b/internal/fleet/agent_policy/required_versions_value.go new file mode 100644 index 000000000..ea5993ea0 --- /dev/null +++ b/internal/fleet/agent_policy/required_versions_value.go @@ -0,0 +1,165 @@ +package agent_policy + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" +) + +var ( + _ basetypes.SetValuable = (*RequiredVersionsValue)(nil) + _ basetypes.SetValuableWithSemanticEquals = (*RequiredVersionsValue)(nil) +) + +// RequiredVersionsValue is a custom set value that implements semantic equality based on version only. +type RequiredVersionsValue struct { + basetypes.SetValue +} + +// Type returns a RequiredVersionsType. +func (v RequiredVersionsValue) Type(ctx context.Context) attr.Type { + return RequiredVersionsType{ + SetType: basetypes.SetType{ + ElemType: v.SetValue.ElementType(ctx), + }, + } +} + +// Equal returns true if the given value is equivalent. +// This uses the standard SetValue equality which compares all fields. +func (v RequiredVersionsValue) Equal(o attr.Value) bool { + other, ok := o.(RequiredVersionsValue) + if !ok { + return false + } + return v.SetValue.Equal(other.SetValue) +} + +// SetSemanticEquals implements custom semantic equality that only compares version fields. +// This ensures that changes to percentage alone don't trigger recreation of resources. +func (v RequiredVersionsValue) SetSemanticEquals(ctx context.Context, newValuable basetypes.SetValuable) (bool, diag.Diagnostics) { + var diags diag.Diagnostics + + newValue, ok := newValuable.(RequiredVersionsValue) + if !ok { + diags.AddError( + "Semantic Equality Check Error", + "An unexpected value type was received while performing semantic equality checks. "+ + "Please report this to the provider developers.\n\n"+ + "Expected Value Type: "+fmt.Sprintf("%T", v)+"\n"+ + "Got Value Type: "+fmt.Sprintf("%T", newValuable), + ) + return false, diags + } + + // Handle null and unknown cases + if v.IsNull() { + return newValue.IsNull(), diags + } + if v.IsUnknown() { + return newValue.IsUnknown(), diags + } + + // Extract versions from both sets + oldVersions, d := extractVersions(ctx, v.SetValue) + diags.Append(d...) + if diags.HasError() { + return false, diags + } + + newVersions, d := extractVersions(ctx, newValue.SetValue) + diags.Append(d...) + if diags.HasError() { + return false, diags + } + + // Compare only the sets of versions + if len(oldVersions) != len(newVersions) { + return false, diags + } + + // Check if all versions in old set exist in new set + for version := range oldVersions { + if !newVersions[version] { + return false, diags + } + } + + return true, diags +} + +// extractVersions extracts version strings from a set of required version objects +func extractVersions(ctx context.Context, setValue basetypes.SetValue) (map[string]bool, diag.Diagnostics) { + var diags diag.Diagnostics + versions := make(map[string]bool) + + elements := setValue.Elements() + for _, elem := range elements { + obj, ok := elem.(basetypes.ObjectValue) + if !ok { + diags.AddError( + "Version Extraction Error", + fmt.Sprintf("Expected ObjectValue, got %T", elem), + ) + continue + } + + attrs := obj.Attributes() + versionAttr, ok := attrs["version"] + if !ok { + diags.AddError( + "Version Extraction Error", + "Required version object missing 'version' attribute", + ) + continue + } + + versionStr, ok := versionAttr.(types.String) + if !ok { + diags.AddError( + "Version Extraction Error", + fmt.Sprintf("Expected String for version, got %T", versionAttr), + ) + continue + } + + if !versionStr.IsNull() && !versionStr.IsUnknown() { + versions[versionStr.ValueString()] = true + } + } + + return versions, diags +} + +// NewRequiredVersionsValueNull creates a RequiredVersionsValue with a null value. +func NewRequiredVersionsValueNull(elemType attr.Type) RequiredVersionsValue { + return RequiredVersionsValue{ + SetValue: basetypes.NewSetNull(elemType), + } +} + +// NewRequiredVersionsValueUnknown creates a RequiredVersionsValue with an unknown value. +func NewRequiredVersionsValueUnknown(elemType attr.Type) RequiredVersionsValue { + return RequiredVersionsValue{ + SetValue: basetypes.NewSetUnknown(elemType), + } +} + +// NewRequiredVersionsValue creates a RequiredVersionsValue with a known value. +func NewRequiredVersionsValue(elemType attr.Type, elements []attr.Value) (RequiredVersionsValue, diag.Diagnostics) { + setValue, diags := basetypes.NewSetValue(elemType, elements) + return RequiredVersionsValue{ + SetValue: setValue, + }, diags +} + +// NewRequiredVersionsValueMust creates a RequiredVersionsValue with a known value and panics on error. +func NewRequiredVersionsValueMust(elemType attr.Type, elements []attr.Value) RequiredVersionsValue { + return RequiredVersionsValue{ + SetValue: basetypes.NewSetValueMust(elemType, elements), + } +} diff --git a/internal/fleet/agent_policy/schema.go b/internal/fleet/agent_policy/schema.go index e2291b0ec..229f67f0d 100644 --- a/internal/fleet/agent_policy/schema.go +++ b/internal/fleet/agent_policy/schema.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" ) func (r *agentPolicyResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { @@ -145,8 +146,27 @@ func getSchema() schema.Schema { Optional: true, Computed: true, }, + "required_versions": schema.SetAttribute{ + Description: "Specifies target agent versions for automatic upgrade. Each entry contains a version string and a percentage of agents to upgrade to that version. Multiple entries with the same version are not allowed.", + ElementType: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "version": types.StringType, + "percentage": types.Int32Type, + }, + }, + Optional: true, + CustomType: RequiredVersionsType{SetType: basetypes.SetType{ElemType: types.ObjectType{AttrTypes: map[string]attr.Type{"version": types.StringType, "percentage": types.Int32Type}}}}, + }, }} } func getGlobalDataTagsAttrTypes() attr.Type { return getSchema().Attributes["global_data_tags"].GetType() } +func getRequiredVersionsElementType() attr.Type { + return types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "version": types.StringType, + "percentage": types.Int32Type, + }, + } +} From bc8388ef994fd584d3b6fb1597eaf30fc136ca10 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Nov 2025 04:03:15 +0000 Subject: [PATCH 03/11] Fix linting issues and regenerate documentation Co-authored-by: tobio <444668+tobio@users.noreply.github.com> --- docs/resources/fleet_agent_policy.md | 10 ++ internal/fleet/agent_policy/acc_test.go | 130 +++++++++--------- internal/fleet/agent_policy/models.go | 4 +- .../agent_policy/required_versions_value.go | 2 +- 4 files changed, 78 insertions(+), 68 deletions(-) diff --git a/docs/resources/fleet_agent_policy.md b/docs/resources/fleet_agent_policy.md index 149ec5f8d..ec583a070 100644 --- a/docs/resources/fleet_agent_policy.md +++ b/docs/resources/fleet_agent_policy.md @@ -57,6 +57,7 @@ resource "elasticstack_fleet_agent_policy" "test_policy" { - `monitor_metrics` (Boolean) Enable collection of agent metrics. - `monitoring_output_id` (String) The identifier for monitoring output. - `policy_id` (String) Unique identifier of the agent policy. +- `required_versions` (Set of Object) Specifies target agent versions for automatic upgrade. Each entry contains a version string and a percentage of agents to upgrade to that version. Multiple entries with the same version are not allowed. (see [below for nested schema](#nestedatt--required_versions)) - `skip_destroy` (Boolean) Set to true if you do not wish the agent policy to be deleted at destroy time, and instead just remove the agent policy from the Terraform state. - `space_ids` (Set of String) The Kibana space IDs that this agent policy should be available in. When not specified, defaults to ["default"]. Note: The order of space IDs does not matter as this is a set. - `supports_agentless` (Boolean) Set to true to enable agentless data collection. @@ -75,6 +76,15 @@ Optional: - `number_value` (Number) Number value for the field. If this is set, string_value must not be defined. - `string_value` (String) String value for the field. If this is set, number_value must not be defined. + + +### Nested Schema for `required_versions` + +Optional: + +- `percentage` (Number) +- `version` (String) + ## Import Import is supported using the following syntax: diff --git a/internal/fleet/agent_policy/acc_test.go b/internal/fleet/agent_policy/acc_test.go index 2a4e3a230..6b6584046 100644 --- a/internal/fleet/agent_policy/acc_test.go +++ b/internal/fleet/agent_policy/acc_test.go @@ -801,71 +801,71 @@ resource "elasticstack_fleet_agent_policy" "test_policy" { } func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { -policyName := sdkacctest.RandStringFromCharSet(22, sdkacctest.CharSetAlphaNum) - -resource.Test(t, resource.TestCase{ -PreCheck: func() { acctest.PreCheck(t) }, -CheckDestroy: checkResourceAgentPolicyDestroy, -ProtoV6ProviderFactories: acctest.Providers, -Steps: []resource.TestStep{ -{ -SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), -Config: testAccResourceAgentPolicyCreateWithRequiredVersions(policyName), -Check: resource.ComposeTestCheckFunc( -resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), -resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), -resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "1"), -resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ -"version": "8.15.0", -"percentage": "100", -}), -), -}, -{ -SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), -Config: testAccResourceAgentPolicyUpdateRequiredVersionsPercentage(policyName), -Check: resource.ComposeTestCheckFunc( -resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), -resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), -resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "1"), -resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ -"version": "8.15.0", -"percentage": "50", -}), -), -}, -{ -SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), -Config: testAccResourceAgentPolicyAddRequiredVersion(policyName), -Check: resource.ComposeTestCheckFunc( -resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), -resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), -resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "2"), -resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ -"version": "8.15.0", -"percentage": "50", -}), -resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ -"version": "8.16.0", -"percentage": "50", -}), -), -}, -{ -SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), -Config: testAccResourceAgentPolicyRemoveRequiredVersions(policyName), -Check: resource.ComposeTestCheckFunc( -resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), -resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), -resource.TestCheckNoResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions"), -), -}, -}, -}) + policyName := sdkacctest.RandStringFromCharSet(22, sdkacctest.CharSetAlphaNum) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + CheckDestroy: checkResourceAgentPolicyDestroy, + ProtoV6ProviderFactories: acctest.Providers, + Steps: []resource.TestStep{ + { + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), + Config: testAccResourceAgentPolicyCreateWithRequiredVersions(policyName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ + "version": "8.15.0", + "percentage": "100", + }), + ), + }, + { + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), + Config: testAccResourceAgentPolicyUpdateRequiredVersionsPercentage(policyName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ + "version": "8.15.0", + "percentage": "50", + }), + ), + }, + { + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), + Config: testAccResourceAgentPolicyAddRequiredVersion(policyName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ + "version": "8.15.0", + "percentage": "50", + }), + resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ + "version": "8.16.0", + "percentage": "50", + }), + ), + }, + { + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), + Config: testAccResourceAgentPolicyRemoveRequiredVersions(policyName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), + resource.TestCheckNoResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions"), + ), + }, + }, + }) } func testAccResourceAgentPolicyCreateWithRequiredVersions(id string) string { -return fmt.Sprintf(` + return fmt.Sprintf(` provider "elasticstack" { elasticsearch {} kibana {} @@ -893,7 +893,7 @@ data "elasticstack_fleet_enrollment_tokens" "test_policy" { } func testAccResourceAgentPolicyUpdateRequiredVersionsPercentage(id string) string { -return fmt.Sprintf(` + return fmt.Sprintf(` provider "elasticstack" { elasticsearch {} kibana {} @@ -921,7 +921,7 @@ data "elasticstack_fleet_enrollment_tokens" "test_policy" { } func testAccResourceAgentPolicyAddRequiredVersion(id string) string { -return fmt.Sprintf(` + return fmt.Sprintf(` provider "elasticstack" { elasticsearch {} kibana {} @@ -953,7 +953,7 @@ data "elasticstack_fleet_enrollment_tokens" "test_policy" { } func testAccResourceAgentPolicyRemoveRequiredVersions(id string) string { -return fmt.Sprintf(` + return fmt.Sprintf(` provider "elasticstack" { elasticsearch {} kibana {} diff --git a/internal/fleet/agent_policy/models.go b/internal/fleet/agent_policy/models.go index 780b8023a..a26af7ddc 100644 --- a/internal/fleet/agent_policy/models.go +++ b/internal/fleet/agent_policy/models.go @@ -139,7 +139,7 @@ func (model *agentPolicyModel) populateFromAPI(ctx context.Context, data *kbapi. if data.RequiredVersions != nil && len(*data.RequiredVersions) > 0 { elemType := getRequiredVersionsElementType() elements := make([]attr.Value, 0, len(*data.RequiredVersions)) - + for _, rv := range *data.RequiredVersions { objValue, d := types.ObjectValue( map[string]attr.Type{ @@ -156,7 +156,7 @@ func (model *agentPolicyModel) populateFromAPI(ctx context.Context, data *kbapi. } elements = append(elements, objValue) } - + reqVersions, d := NewRequiredVersionsValue(elemType, elements) if d.HasError() { return d diff --git a/internal/fleet/agent_policy/required_versions_value.go b/internal/fleet/agent_policy/required_versions_value.go index ea5993ea0..bc68d8df7 100644 --- a/internal/fleet/agent_policy/required_versions_value.go +++ b/internal/fleet/agent_policy/required_versions_value.go @@ -24,7 +24,7 @@ type RequiredVersionsValue struct { func (v RequiredVersionsValue) Type(ctx context.Context) attr.Type { return RequiredVersionsType{ SetType: basetypes.SetType{ - ElemType: v.SetValue.ElementType(ctx), + ElemType: v.ElementType(ctx), }, } } From 7a367308353c985648a6289de9de3194553e51c2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Nov 2025 04:06:16 +0000 Subject: [PATCH 04/11] Add validator to prevent duplicate versions in required_versions Co-authored-by: tobio <444668+tobio@users.noreply.github.com> --- internal/fleet/agent_policy/acc_test.go | 48 +++++++ .../required_versions_validator.go | 75 ++++++++++ .../required_versions_validator_test.go | 129 ++++++++++++++++++ internal/fleet/agent_policy/schema.go | 3 + 4 files changed, 255 insertions(+) create mode 100644 internal/fleet/agent_policy/required_versions_validator.go create mode 100644 internal/fleet/agent_policy/required_versions_validator_test.go diff --git a/internal/fleet/agent_policy/acc_test.go b/internal/fleet/agent_policy/acc_test.go index 6b6584046..806c9ac25 100644 --- a/internal/fleet/agent_policy/acc_test.go +++ b/internal/fleet/agent_policy/acc_test.go @@ -973,3 +973,51 @@ data "elasticstack_fleet_enrollment_tokens" "test_policy" { } `, fmt.Sprintf("Policy %s", id)) } + +func TestAccResourceAgentPolicyWithDuplicateRequiredVersions(t *testing.T) { + policyName := sdkacctest.RandStringFromCharSet(22, sdkacctest.CharSetAlphaNum) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ProtoV6ProviderFactories: acctest.Providers, + Steps: []resource.TestStep{ + { + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), + Config: testAccResourceAgentPolicyCreateWithDuplicateRequiredVersions(policyName), + ExpectError: regexp.MustCompile(".*Duplicate Version.*8.15.0.*"), + }, + }, + }) +} + +func testAccResourceAgentPolicyCreateWithDuplicateRequiredVersions(id string) string { + return fmt.Sprintf(` +provider "elasticstack" { + elasticsearch {} + kibana {} +} + +resource "elasticstack_fleet_agent_policy" "test_policy" { + name = "%s" + namespace = "default" + description = "Test Agent Policy with Duplicate Required Versions" + monitor_logs = true + monitor_metrics = false + skip_destroy = false + required_versions = [ + { + version = "8.15.0" + percentage = 50 + }, + { + version = "8.15.0" + percentage = 100 + } + ] +} + +data "elasticstack_fleet_enrollment_tokens" "test_policy" { + policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id +} +`, fmt.Sprintf("Policy %s", id)) +} diff --git a/internal/fleet/agent_policy/required_versions_validator.go b/internal/fleet/agent_policy/required_versions_validator.go new file mode 100644 index 000000000..34d196e34 --- /dev/null +++ b/internal/fleet/agent_policy/required_versions_validator.go @@ -0,0 +1,75 @@ +package agent_policy + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +var _ validator.Set = (*uniqueVersionValidator)(nil) + +// uniqueVersionValidator validates that all required_versions have unique version strings +type uniqueVersionValidator struct{} + +func (v uniqueVersionValidator) Description(ctx context.Context) string { + return "Ensures that all required_versions entries have unique version strings" +} + +func (v uniqueVersionValidator) MarkdownDescription(ctx context.Context) string { + return "Ensures that all required_versions entries have unique version strings" +} + +func (v uniqueVersionValidator) ValidateSet(ctx context.Context, req validator.SetRequest, resp *validator.SetResponse) { + // If the value is unknown or null, validation should not be performed + if req.ConfigValue.IsUnknown() || req.ConfigValue.IsNull() { + return + } + + elements := req.ConfigValue.Elements() + versions := make(map[string]bool) + + for _, elem := range elements { + obj, ok := elem.(types.Object) + if !ok { + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid Type", + fmt.Sprintf("Expected object type in set, got %T", elem), + ) + continue + } + + attrs := obj.Attributes() + versionAttr, ok := attrs["version"] + if !ok { + continue + } + + versionStr, ok := versionAttr.(types.String) + if !ok { + continue + } + + if versionStr.IsNull() || versionStr.IsUnknown() { + continue + } + + version := versionStr.ValueString() + if versions[version] { + resp.Diagnostics.AddAttributeError( + req.Path, + "Duplicate Version", + fmt.Sprintf("The version '%s' appears multiple times in required_versions. Each version must be unique.", version), + ) + return + } + versions[version] = true + } +} + +// UniqueVersions returns a validator that ensures all required_versions have unique version strings +func UniqueVersions() validator.Set { + return uniqueVersionValidator{} +} diff --git a/internal/fleet/agent_policy/required_versions_validator_test.go b/internal/fleet/agent_policy/required_versions_validator_test.go new file mode 100644 index 000000000..32cdeaaec --- /dev/null +++ b/internal/fleet/agent_policy/required_versions_validator_test.go @@ -0,0 +1,129 @@ +package agent_policy + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/stretchr/testify/assert" +) + +func TestUniqueVersionsValidator(t *testing.T) { + ctx := context.Background() + elemType := getRequiredVersionsElementType() + + t.Run("valid - unique versions", func(t *testing.T) { + elements := []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "version": types.StringType, + "percentage": types.Int32Type, + }, + map[string]attr.Value{ + "version": types.StringValue("8.15.0"), + "percentage": types.Int32Value(50), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "version": types.StringType, + "percentage": types.Int32Type, + }, + map[string]attr.Value{ + "version": types.StringValue("8.16.0"), + "percentage": types.Int32Value(50), + }, + ), + } + + setValue := types.SetValueMust(elemType, elements) + + req := validator.SetRequest{ + Path: path.Root("required_versions"), + ConfigValue: setValue, + } + resp := &validator.SetResponse{ + Diagnostics: diag.Diagnostics{}, + } + + UniqueVersions().ValidateSet(ctx, req, resp) + + assert.False(t, resp.Diagnostics.HasError(), "Expected no errors for unique versions") + }) + + t.Run("invalid - duplicate versions", func(t *testing.T) { + elements := []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "version": types.StringType, + "percentage": types.Int32Type, + }, + map[string]attr.Value{ + "version": types.StringValue("8.15.0"), + "percentage": types.Int32Value(50), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "version": types.StringType, + "percentage": types.Int32Type, + }, + map[string]attr.Value{ + "version": types.StringValue("8.15.0"), + "percentage": types.Int32Value(100), + }, + ), + } + + setValue := types.SetValueMust(elemType, elements) + + req := validator.SetRequest{ + Path: path.Root("required_versions"), + ConfigValue: setValue, + } + resp := &validator.SetResponse{ + Diagnostics: diag.Diagnostics{}, + } + + UniqueVersions().ValidateSet(ctx, req, resp) + + assert.True(t, resp.Diagnostics.HasError(), "Expected error for duplicate versions") + assert.Contains(t, resp.Diagnostics.Errors()[0].Detail(), "8.15.0") + }) + + t.Run("valid - null value", func(t *testing.T) { + setValue := types.SetNull(elemType) + + req := validator.SetRequest{ + Path: path.Root("required_versions"), + ConfigValue: setValue, + } + resp := &validator.SetResponse{ + Diagnostics: diag.Diagnostics{}, + } + + UniqueVersions().ValidateSet(ctx, req, resp) + + assert.False(t, resp.Diagnostics.HasError(), "Expected no errors for null value") + }) + + t.Run("valid - empty set", func(t *testing.T) { + setValue := types.SetValueMust(elemType, []attr.Value{}) + + req := validator.SetRequest{ + Path: path.Root("required_versions"), + ConfigValue: setValue, + } + resp := &validator.SetResponse{ + Diagnostics: diag.Diagnostics{}, + } + + UniqueVersions().ValidateSet(ctx, req, resp) + + assert.False(t, resp.Diagnostics.HasError(), "Expected no errors for empty set") + }) +} diff --git a/internal/fleet/agent_policy/schema.go b/internal/fleet/agent_policy/schema.go index 229f67f0d..40432eae8 100644 --- a/internal/fleet/agent_policy/schema.go +++ b/internal/fleet/agent_policy/schema.go @@ -156,6 +156,9 @@ func getSchema() schema.Schema { }, Optional: true, CustomType: RequiredVersionsType{SetType: basetypes.SetType{ElemType: types.ObjectType{AttrTypes: map[string]attr.Type{"version": types.StringType, "percentage": types.Int32Type}}}}, + Validators: []validator.Set{ + UniqueVersions(), + }, }, }} } From 8f057058d798181d7de2c1fd31ebe5a17d12001a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Nov 2025 04:41:18 +0000 Subject: [PATCH 05/11] Simplify required_versions to use MapAttribute instead of custom Set type Co-authored-by: tobio <444668+tobio@users.noreply.github.com> --- docs/resources/fleet_agent_policy.md | 11 +- internal/fleet/agent_policy/acc_test.go | 106 ++------- internal/fleet/agent_policy/models.go | 80 +++---- .../agent_policy/required_versions_test.go | 222 ------------------ .../agent_policy/required_versions_type.go | 68 ------ .../required_versions_validator.go | 75 ------ .../required_versions_validator_test.go | 129 ---------- .../agent_policy/required_versions_value.go | 165 ------------- internal/fleet/agent_policy/schema.go | 26 +- 9 files changed, 51 insertions(+), 831 deletions(-) delete mode 100644 internal/fleet/agent_policy/required_versions_test.go delete mode 100644 internal/fleet/agent_policy/required_versions_type.go delete mode 100644 internal/fleet/agent_policy/required_versions_validator.go delete mode 100644 internal/fleet/agent_policy/required_versions_validator_test.go delete mode 100644 internal/fleet/agent_policy/required_versions_value.go diff --git a/docs/resources/fleet_agent_policy.md b/docs/resources/fleet_agent_policy.md index ec583a070..cf82b6b1b 100644 --- a/docs/resources/fleet_agent_policy.md +++ b/docs/resources/fleet_agent_policy.md @@ -57,7 +57,7 @@ resource "elasticstack_fleet_agent_policy" "test_policy" { - `monitor_metrics` (Boolean) Enable collection of agent metrics. - `monitoring_output_id` (String) The identifier for monitoring output. - `policy_id` (String) Unique identifier of the agent policy. -- `required_versions` (Set of Object) Specifies target agent versions for automatic upgrade. Each entry contains a version string and a percentage of agents to upgrade to that version. Multiple entries with the same version are not allowed. (see [below for nested schema](#nestedatt--required_versions)) +- `required_versions` (Map of Number) Map of agent versions to target percentages for automatic upgrade. The key is the target version and the value is the percentage of agents to upgrade to that version. - `skip_destroy` (Boolean) Set to true if you do not wish the agent policy to be deleted at destroy time, and instead just remove the agent policy from the Terraform state. - `space_ids` (Set of String) The Kibana space IDs that this agent policy should be available in. When not specified, defaults to ["default"]. Note: The order of space IDs does not matter as this is a set. - `supports_agentless` (Boolean) Set to true to enable agentless data collection. @@ -76,15 +76,6 @@ Optional: - `number_value` (Number) Number value for the field. If this is set, string_value must not be defined. - `string_value` (String) String value for the field. If this is set, number_value must not be defined. - - -### Nested Schema for `required_versions` - -Optional: - -- `percentage` (Number) -- `version` (String) - ## Import Import is supported using the following syntax: diff --git a/internal/fleet/agent_policy/acc_test.go b/internal/fleet/agent_policy/acc_test.go index 806c9ac25..6e46d78b6 100644 --- a/internal/fleet/agent_policy/acc_test.go +++ b/internal/fleet/agent_policy/acc_test.go @@ -814,11 +814,8 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), - resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ - "version": "8.15.0", - "percentage": "100", - }), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.%", "1"), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.8.15.0", "100"), ), }, { @@ -827,11 +824,8 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), - resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ - "version": "8.15.0", - "percentage": "50", - }), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.%", "1"), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.8.15.0", "50"), ), }, { @@ -840,15 +834,9 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), - resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.#", "2"), - resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ - "version": "8.15.0", - "percentage": "50", - }), - resource.TestCheckTypeSetElemNestedAttrs("elasticstack_fleet_agent_policy.test_policy", "required_versions.*", map[string]string{ - "version": "8.16.0", - "percentage": "50", - }), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.%", "2"), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.8.15.0", "50"), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.8.16.0", "50"), ), }, { @@ -878,12 +866,9 @@ resource "elasticstack_fleet_agent_policy" "test_policy" { monitor_logs = true monitor_metrics = false skip_destroy = false - required_versions = [ - { - version = "8.15.0" - percentage = 100 - } - ] + required_versions = { + "8.15.0" = 100 + } } data "elasticstack_fleet_enrollment_tokens" "test_policy" { @@ -906,12 +891,9 @@ resource "elasticstack_fleet_agent_policy" "test_policy" { monitor_logs = true monitor_metrics = false skip_destroy = false - required_versions = [ - { - version = "8.15.0" - percentage = 50 - } - ] + required_versions = { + "8.15.0" = 50 + } } data "elasticstack_fleet_enrollment_tokens" "test_policy" { @@ -934,16 +916,10 @@ resource "elasticstack_fleet_agent_policy" "test_policy" { monitor_logs = true monitor_metrics = false skip_destroy = false - required_versions = [ - { - version = "8.15.0" - percentage = 50 - }, - { - version = "8.16.0" - percentage = 50 - } - ] + required_versions = { + "8.15.0" = 50 + "8.16.0" = 50 + } } data "elasticstack_fleet_enrollment_tokens" "test_policy" { @@ -973,51 +949,3 @@ data "elasticstack_fleet_enrollment_tokens" "test_policy" { } `, fmt.Sprintf("Policy %s", id)) } - -func TestAccResourceAgentPolicyWithDuplicateRequiredVersions(t *testing.T) { - policyName := sdkacctest.RandStringFromCharSet(22, sdkacctest.CharSetAlphaNum) - - resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ProtoV6ProviderFactories: acctest.Providers, - Steps: []resource.TestStep{ - { - SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), - Config: testAccResourceAgentPolicyCreateWithDuplicateRequiredVersions(policyName), - ExpectError: regexp.MustCompile(".*Duplicate Version.*8.15.0.*"), - }, - }, - }) -} - -func testAccResourceAgentPolicyCreateWithDuplicateRequiredVersions(id string) string { - return fmt.Sprintf(` -provider "elasticstack" { - elasticsearch {} - kibana {} -} - -resource "elasticstack_fleet_agent_policy" "test_policy" { - name = "%s" - namespace = "default" - description = "Test Agent Policy with Duplicate Required Versions" - monitor_logs = true - monitor_metrics = false - skip_destroy = false - required_versions = [ - { - version = "8.15.0" - percentage = 50 - }, - { - version = "8.15.0" - percentage = 100 - } - ] -} - -data "elasticstack_fleet_enrollment_tokens" "test_policy" { - policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id -} -`, fmt.Sprintf("Policy %s", id)) -} diff --git a/internal/fleet/agent_policy/models.go b/internal/fleet/agent_policy/models.go index a26af7ddc..9c4d7018a 100644 --- a/internal/fleet/agent_policy/models.go +++ b/internal/fleet/agent_policy/models.go @@ -30,25 +30,25 @@ type globalDataTagsItemModel struct { } type agentPolicyModel struct { - ID types.String `tfsdk:"id"` - PolicyID types.String `tfsdk:"policy_id"` - Name types.String `tfsdk:"name"` - Namespace types.String `tfsdk:"namespace"` - Description types.String `tfsdk:"description"` - DataOutputId types.String `tfsdk:"data_output_id"` - MonitoringOutputId types.String `tfsdk:"monitoring_output_id"` - FleetServerHostId types.String `tfsdk:"fleet_server_host_id"` - DownloadSourceId types.String `tfsdk:"download_source_id"` - MonitorLogs types.Bool `tfsdk:"monitor_logs"` - MonitorMetrics types.Bool `tfsdk:"monitor_metrics"` - SysMonitoring types.Bool `tfsdk:"sys_monitoring"` - SkipDestroy types.Bool `tfsdk:"skip_destroy"` - SupportsAgentless types.Bool `tfsdk:"supports_agentless"` - InactivityTimeout customtypes.Duration `tfsdk:"inactivity_timeout"` - UnenrollmentTimeout customtypes.Duration `tfsdk:"unenrollment_timeout"` - GlobalDataTags types.Map `tfsdk:"global_data_tags"` //> globalDataTagsModel - SpaceIds types.Set `tfsdk:"space_ids"` - RequiredVersions RequiredVersionsValue `tfsdk:"required_versions"` + ID types.String `tfsdk:"id"` + PolicyID types.String `tfsdk:"policy_id"` + Name types.String `tfsdk:"name"` + Namespace types.String `tfsdk:"namespace"` + Description types.String `tfsdk:"description"` + DataOutputId types.String `tfsdk:"data_output_id"` + MonitoringOutputId types.String `tfsdk:"monitoring_output_id"` + FleetServerHostId types.String `tfsdk:"fleet_server_host_id"` + DownloadSourceId types.String `tfsdk:"download_source_id"` + MonitorLogs types.Bool `tfsdk:"monitor_logs"` + MonitorMetrics types.Bool `tfsdk:"monitor_metrics"` + SysMonitoring types.Bool `tfsdk:"sys_monitoring"` + SkipDestroy types.Bool `tfsdk:"skip_destroy"` + SupportsAgentless types.Bool `tfsdk:"supports_agentless"` + InactivityTimeout customtypes.Duration `tfsdk:"inactivity_timeout"` + UnenrollmentTimeout customtypes.Duration `tfsdk:"unenrollment_timeout"` + GlobalDataTags types.Map `tfsdk:"global_data_tags"` //> globalDataTagsModel + SpaceIds types.Set `tfsdk:"space_ids"` + RequiredVersions types.Map `tfsdk:"required_versions"` } func (model *agentPolicyModel) populateFromAPI(ctx context.Context, data *kbapi.AgentPolicy) diag.Diagnostics { @@ -137,33 +137,19 @@ func (model *agentPolicyModel) populateFromAPI(ctx context.Context, data *kbapi. // Handle required_versions if data.RequiredVersions != nil && len(*data.RequiredVersions) > 0 { - elemType := getRequiredVersionsElementType() - elements := make([]attr.Value, 0, len(*data.RequiredVersions)) + versionMap := make(map[string]attr.Value) for _, rv := range *data.RequiredVersions { - objValue, d := types.ObjectValue( - map[string]attr.Type{ - "version": types.StringType, - "percentage": types.Int32Type, - }, - map[string]attr.Value{ - "version": types.StringValue(rv.Version), - "percentage": types.Int32Value(int32(rv.Percentage)), - }, - ) - if d.HasError() { - return d - } - elements = append(elements, objValue) + versionMap[rv.Version] = types.Int32Value(int32(rv.Percentage)) } - reqVersions, d := NewRequiredVersionsValue(elemType, elements) + reqVersions, d := types.MapValue(types.Int32Type, versionMap) if d.HasError() { return d } model.RequiredVersions = reqVersions } else { - model.RequiredVersions = NewRequiredVersionsValueNull(getRequiredVersionsElementType()) + model.RequiredVersions = types.MapNull(types.Int32Type) } return nil @@ -239,22 +225,14 @@ func (model *agentPolicyModel) convertRequiredVersions(ctx context.Context) (*[] Version string `json:"version"` }, 0, len(elements)) - for _, elem := range elements { - obj, ok := elem.(types.Object) + for version, percentageVal := range elements { + percentageInt32, ok := percentageVal.(types.Int32) if !ok { - diags.AddError("required_versions conversion error", fmt.Sprintf("Expected ObjectValue, got %T", elem)) + diags.AddError("required_versions conversion error", fmt.Sprintf("Expected Int32 value, got %T", percentageVal)) continue } - attrs := obj.Attributes() - versionAttr := attrs["version"].(types.String) - percentageAttr := attrs["percentage"].(types.Int32) - - if versionAttr.IsNull() || versionAttr.IsUnknown() { - diags.AddError("required_versions validation error", "version cannot be null or unknown") - continue - } - if percentageAttr.IsNull() || percentageAttr.IsUnknown() { + if percentageInt32.IsNull() || percentageInt32.IsUnknown() { diags.AddError("required_versions validation error", "percentage cannot be null or unknown") continue } @@ -263,8 +241,8 @@ func (model *agentPolicyModel) convertRequiredVersions(ctx context.Context) (*[] Percentage float32 `json:"percentage"` Version string `json:"version"` }{ - Percentage: float32(percentageAttr.ValueInt32()), - Version: versionAttr.ValueString(), + Percentage: float32(percentageInt32.ValueInt32()), + Version: version, }) } diff --git a/internal/fleet/agent_policy/required_versions_test.go b/internal/fleet/agent_policy/required_versions_test.go deleted file mode 100644 index 6eb5eed0c..000000000 --- a/internal/fleet/agent_policy/required_versions_test.go +++ /dev/null @@ -1,222 +0,0 @@ -package agent_policy - -import ( - "context" - "testing" - - "github.com/hashicorp/terraform-plugin-framework/attr" - "github.com/hashicorp/terraform-plugin-framework/types" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestRequiredVersionsValue_SetSemanticEquals(t *testing.T) { - ctx := context.Background() - elemType := getRequiredVersionsElementType() - - // Helper function to create a RequiredVersionsValue - createValue := func(versions []struct { - version string - percentage int32 - }) RequiredVersionsValue { - elements := make([]attr.Value, 0, len(versions)) - for _, v := range versions { - obj, diags := types.ObjectValue( - map[string]attr.Type{ - "version": types.StringType, - "percentage": types.Int32Type, - }, - map[string]attr.Value{ - "version": types.StringValue(v.version), - "percentage": types.Int32Value(v.percentage), - }, - ) - require.False(t, diags.HasError()) - elements = append(elements, obj) - } - val, diags := NewRequiredVersionsValue(elemType, elements) - require.False(t, diags.HasError()) - return val - } - - t.Run("equal when same versions regardless of percentage", func(t *testing.T) { - val1 := createValue([]struct { - version string - percentage int32 - }{ - {"8.15.0", 50}, - {"8.16.0", 50}, - }) - - val2 := createValue([]struct { - version string - percentage int32 - }{ - {"8.15.0", 100}, - {"8.16.0", 0}, - }) - - equal, diags := val1.SetSemanticEquals(ctx, val2) - require.False(t, diags.HasError()) - assert.True(t, equal, "Values should be semantically equal when versions match") - }) - - t.Run("not equal when different versions", func(t *testing.T) { - val1 := createValue([]struct { - version string - percentage int32 - }{ - {"8.15.0", 50}, - }) - - val2 := createValue([]struct { - version string - percentage int32 - }{ - {"8.16.0", 50}, - }) - - equal, diags := val1.SetSemanticEquals(ctx, val2) - require.False(t, diags.HasError()) - assert.False(t, equal, "Values should not be equal when versions differ") - }) - - t.Run("not equal when different number of versions", func(t *testing.T) { - val1 := createValue([]struct { - version string - percentage int32 - }{ - {"8.15.0", 50}, - }) - - val2 := createValue([]struct { - version string - percentage int32 - }{ - {"8.15.0", 50}, - {"8.16.0", 50}, - }) - - equal, diags := val1.SetSemanticEquals(ctx, val2) - require.False(t, diags.HasError()) - assert.False(t, equal, "Values should not be equal when number of versions differs") - }) - - t.Run("null values are equal", func(t *testing.T) { - val1 := NewRequiredVersionsValueNull(elemType) - val2 := NewRequiredVersionsValueNull(elemType) - - equal, diags := val1.SetSemanticEquals(ctx, val2) - require.False(t, diags.HasError()) - assert.True(t, equal, "Null values should be equal") - }) - - t.Run("null and non-null are not equal", func(t *testing.T) { - val1 := NewRequiredVersionsValueNull(elemType) - val2 := createValue([]struct { - version string - percentage int32 - }{ - {"8.15.0", 50}, - }) - - equal, diags := val1.SetSemanticEquals(ctx, val2) - require.False(t, diags.HasError()) - assert.False(t, equal, "Null and non-null values should not be equal") - }) - - t.Run("equal when versions in different order", func(t *testing.T) { - val1 := createValue([]struct { - version string - percentage int32 - }{ - {"8.15.0", 50}, - {"8.16.0", 50}, - }) - - val2 := createValue([]struct { - version string - percentage int32 - }{ - {"8.16.0", 100}, - {"8.15.0", 0}, - }) - - equal, diags := val1.SetSemanticEquals(ctx, val2) - require.False(t, diags.HasError()) - assert.True(t, equal, "Values should be semantically equal when versions match in different order") - }) -} - -func TestConvertRequiredVersions(t *testing.T) { - ctx := context.Background() - elemType := getRequiredVersionsElementType() - - t.Run("converts valid required versions", func(t *testing.T) { - elements := []attr.Value{ - types.ObjectValueMust( - map[string]attr.Type{ - "version": types.StringType, - "percentage": types.Int32Type, - }, - map[string]attr.Value{ - "version": types.StringValue("8.15.0"), - "percentage": types.Int32Value(50), - }, - ), - types.ObjectValueMust( - map[string]attr.Type{ - "version": types.StringType, - "percentage": types.Int32Type, - }, - map[string]attr.Value{ - "version": types.StringValue("8.16.0"), - "percentage": types.Int32Value(50), - }, - ), - } - - reqVersions, diags := NewRequiredVersionsValue(elemType, elements) - require.False(t, diags.HasError()) - - model := &agentPolicyModel{ - RequiredVersions: reqVersions, - } - - result, diags := model.convertRequiredVersions(ctx) - require.False(t, diags.HasError()) - require.NotNil(t, result) - assert.Len(t, *result, 2) - - // Check that both versions are present - versions := make(map[string]float32) - for _, rv := range *result { - versions[rv.Version] = rv.Percentage - } - assert.Equal(t, float32(50), versions["8.15.0"]) - assert.Equal(t, float32(50), versions["8.16.0"]) - }) - - t.Run("returns nil for null required versions", func(t *testing.T) { - model := &agentPolicyModel{ - RequiredVersions: NewRequiredVersionsValueNull(elemType), - } - - result, diags := model.convertRequiredVersions(ctx) - require.False(t, diags.HasError()) - assert.Nil(t, result) - }) - - t.Run("returns nil for empty required versions", func(t *testing.T) { - reqVersions, diags := NewRequiredVersionsValue(elemType, []attr.Value{}) - require.False(t, diags.HasError()) - - model := &agentPolicyModel{ - RequiredVersions: reqVersions, - } - - result, diags := model.convertRequiredVersions(ctx) - require.False(t, diags.HasError()) - assert.Nil(t, result) - }) -} diff --git a/internal/fleet/agent_policy/required_versions_type.go b/internal/fleet/agent_policy/required_versions_type.go deleted file mode 100644 index 5fda38831..000000000 --- a/internal/fleet/agent_policy/required_versions_type.go +++ /dev/null @@ -1,68 +0,0 @@ -package agent_policy - -import ( - "context" - "fmt" - - "github.com/hashicorp/terraform-plugin-framework/attr" - "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/types/basetypes" - "github.com/hashicorp/terraform-plugin-go/tftypes" -) - -var ( - _ basetypes.SetTypable = (*RequiredVersionsType)(nil) -) - -// RequiredVersionsType is a custom set type that enforces uniqueness based on version only. -type RequiredVersionsType struct { - basetypes.SetType -} - -// String returns a human readable string of the type name. -func (t RequiredVersionsType) String() string { - return "agent_policy.RequiredVersionsType" -} - -// ValueType returns the Value type. -func (t RequiredVersionsType) ValueType(ctx context.Context) attr.Value { - return RequiredVersionsValue{ - SetValue: basetypes.NewSetUnknown(t.ElemType), - } -} - -// Equal returns true if the given type is equivalent. -func (t RequiredVersionsType) Equal(o attr.Type) bool { - other, ok := o.(RequiredVersionsType) - if !ok { - return false - } - return t.SetType.Equal(other.SetType) -} - -// ValueFromSet returns a SetValuable type given a SetValue. -func (t RequiredVersionsType) ValueFromSet(ctx context.Context, in basetypes.SetValue) (basetypes.SetValuable, diag.Diagnostics) { - return RequiredVersionsValue{ - SetValue: in, - }, nil -} - -// ValueFromTerraform returns a Value given a tftypes.Value. -func (t RequiredVersionsType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) { - attrValue, err := t.SetType.ValueFromTerraform(ctx, in) - if err != nil { - return nil, err - } - - setValue, ok := attrValue.(basetypes.SetValue) - if !ok { - return nil, fmt.Errorf("unexpected value type of %T", attrValue) - } - - setValuable, diags := t.ValueFromSet(ctx, setValue) - if diags.HasError() { - return nil, fmt.Errorf("unexpected error converting SetValue to SetValuable: %v", diags) - } - - return setValuable, nil -} diff --git a/internal/fleet/agent_policy/required_versions_validator.go b/internal/fleet/agent_policy/required_versions_validator.go deleted file mode 100644 index 34d196e34..000000000 --- a/internal/fleet/agent_policy/required_versions_validator.go +++ /dev/null @@ -1,75 +0,0 @@ -package agent_policy - -import ( - "context" - "fmt" - - "github.com/hashicorp/terraform-plugin-framework/schema/validator" - "github.com/hashicorp/terraform-plugin-framework/types" -) - -var _ validator.Set = (*uniqueVersionValidator)(nil) - -// uniqueVersionValidator validates that all required_versions have unique version strings -type uniqueVersionValidator struct{} - -func (v uniqueVersionValidator) Description(ctx context.Context) string { - return "Ensures that all required_versions entries have unique version strings" -} - -func (v uniqueVersionValidator) MarkdownDescription(ctx context.Context) string { - return "Ensures that all required_versions entries have unique version strings" -} - -func (v uniqueVersionValidator) ValidateSet(ctx context.Context, req validator.SetRequest, resp *validator.SetResponse) { - // If the value is unknown or null, validation should not be performed - if req.ConfigValue.IsUnknown() || req.ConfigValue.IsNull() { - return - } - - elements := req.ConfigValue.Elements() - versions := make(map[string]bool) - - for _, elem := range elements { - obj, ok := elem.(types.Object) - if !ok { - resp.Diagnostics.AddAttributeError( - req.Path, - "Invalid Type", - fmt.Sprintf("Expected object type in set, got %T", elem), - ) - continue - } - - attrs := obj.Attributes() - versionAttr, ok := attrs["version"] - if !ok { - continue - } - - versionStr, ok := versionAttr.(types.String) - if !ok { - continue - } - - if versionStr.IsNull() || versionStr.IsUnknown() { - continue - } - - version := versionStr.ValueString() - if versions[version] { - resp.Diagnostics.AddAttributeError( - req.Path, - "Duplicate Version", - fmt.Sprintf("The version '%s' appears multiple times in required_versions. Each version must be unique.", version), - ) - return - } - versions[version] = true - } -} - -// UniqueVersions returns a validator that ensures all required_versions have unique version strings -func UniqueVersions() validator.Set { - return uniqueVersionValidator{} -} diff --git a/internal/fleet/agent_policy/required_versions_validator_test.go b/internal/fleet/agent_policy/required_versions_validator_test.go deleted file mode 100644 index 32cdeaaec..000000000 --- a/internal/fleet/agent_policy/required_versions_validator_test.go +++ /dev/null @@ -1,129 +0,0 @@ -package agent_policy - -import ( - "context" - "testing" - - "github.com/hashicorp/terraform-plugin-framework/attr" - "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/path" - "github.com/hashicorp/terraform-plugin-framework/schema/validator" - "github.com/hashicorp/terraform-plugin-framework/types" - "github.com/stretchr/testify/assert" -) - -func TestUniqueVersionsValidator(t *testing.T) { - ctx := context.Background() - elemType := getRequiredVersionsElementType() - - t.Run("valid - unique versions", func(t *testing.T) { - elements := []attr.Value{ - types.ObjectValueMust( - map[string]attr.Type{ - "version": types.StringType, - "percentage": types.Int32Type, - }, - map[string]attr.Value{ - "version": types.StringValue("8.15.0"), - "percentage": types.Int32Value(50), - }, - ), - types.ObjectValueMust( - map[string]attr.Type{ - "version": types.StringType, - "percentage": types.Int32Type, - }, - map[string]attr.Value{ - "version": types.StringValue("8.16.0"), - "percentage": types.Int32Value(50), - }, - ), - } - - setValue := types.SetValueMust(elemType, elements) - - req := validator.SetRequest{ - Path: path.Root("required_versions"), - ConfigValue: setValue, - } - resp := &validator.SetResponse{ - Diagnostics: diag.Diagnostics{}, - } - - UniqueVersions().ValidateSet(ctx, req, resp) - - assert.False(t, resp.Diagnostics.HasError(), "Expected no errors for unique versions") - }) - - t.Run("invalid - duplicate versions", func(t *testing.T) { - elements := []attr.Value{ - types.ObjectValueMust( - map[string]attr.Type{ - "version": types.StringType, - "percentage": types.Int32Type, - }, - map[string]attr.Value{ - "version": types.StringValue("8.15.0"), - "percentage": types.Int32Value(50), - }, - ), - types.ObjectValueMust( - map[string]attr.Type{ - "version": types.StringType, - "percentage": types.Int32Type, - }, - map[string]attr.Value{ - "version": types.StringValue("8.15.0"), - "percentage": types.Int32Value(100), - }, - ), - } - - setValue := types.SetValueMust(elemType, elements) - - req := validator.SetRequest{ - Path: path.Root("required_versions"), - ConfigValue: setValue, - } - resp := &validator.SetResponse{ - Diagnostics: diag.Diagnostics{}, - } - - UniqueVersions().ValidateSet(ctx, req, resp) - - assert.True(t, resp.Diagnostics.HasError(), "Expected error for duplicate versions") - assert.Contains(t, resp.Diagnostics.Errors()[0].Detail(), "8.15.0") - }) - - t.Run("valid - null value", func(t *testing.T) { - setValue := types.SetNull(elemType) - - req := validator.SetRequest{ - Path: path.Root("required_versions"), - ConfigValue: setValue, - } - resp := &validator.SetResponse{ - Diagnostics: diag.Diagnostics{}, - } - - UniqueVersions().ValidateSet(ctx, req, resp) - - assert.False(t, resp.Diagnostics.HasError(), "Expected no errors for null value") - }) - - t.Run("valid - empty set", func(t *testing.T) { - setValue := types.SetValueMust(elemType, []attr.Value{}) - - req := validator.SetRequest{ - Path: path.Root("required_versions"), - ConfigValue: setValue, - } - resp := &validator.SetResponse{ - Diagnostics: diag.Diagnostics{}, - } - - UniqueVersions().ValidateSet(ctx, req, resp) - - assert.False(t, resp.Diagnostics.HasError(), "Expected no errors for empty set") - }) -} diff --git a/internal/fleet/agent_policy/required_versions_value.go b/internal/fleet/agent_policy/required_versions_value.go deleted file mode 100644 index bc68d8df7..000000000 --- a/internal/fleet/agent_policy/required_versions_value.go +++ /dev/null @@ -1,165 +0,0 @@ -package agent_policy - -import ( - "context" - "fmt" - - "github.com/hashicorp/terraform-plugin-framework/attr" - "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/types" - "github.com/hashicorp/terraform-plugin-framework/types/basetypes" -) - -var ( - _ basetypes.SetValuable = (*RequiredVersionsValue)(nil) - _ basetypes.SetValuableWithSemanticEquals = (*RequiredVersionsValue)(nil) -) - -// RequiredVersionsValue is a custom set value that implements semantic equality based on version only. -type RequiredVersionsValue struct { - basetypes.SetValue -} - -// Type returns a RequiredVersionsType. -func (v RequiredVersionsValue) Type(ctx context.Context) attr.Type { - return RequiredVersionsType{ - SetType: basetypes.SetType{ - ElemType: v.ElementType(ctx), - }, - } -} - -// Equal returns true if the given value is equivalent. -// This uses the standard SetValue equality which compares all fields. -func (v RequiredVersionsValue) Equal(o attr.Value) bool { - other, ok := o.(RequiredVersionsValue) - if !ok { - return false - } - return v.SetValue.Equal(other.SetValue) -} - -// SetSemanticEquals implements custom semantic equality that only compares version fields. -// This ensures that changes to percentage alone don't trigger recreation of resources. -func (v RequiredVersionsValue) SetSemanticEquals(ctx context.Context, newValuable basetypes.SetValuable) (bool, diag.Diagnostics) { - var diags diag.Diagnostics - - newValue, ok := newValuable.(RequiredVersionsValue) - if !ok { - diags.AddError( - "Semantic Equality Check Error", - "An unexpected value type was received while performing semantic equality checks. "+ - "Please report this to the provider developers.\n\n"+ - "Expected Value Type: "+fmt.Sprintf("%T", v)+"\n"+ - "Got Value Type: "+fmt.Sprintf("%T", newValuable), - ) - return false, diags - } - - // Handle null and unknown cases - if v.IsNull() { - return newValue.IsNull(), diags - } - if v.IsUnknown() { - return newValue.IsUnknown(), diags - } - - // Extract versions from both sets - oldVersions, d := extractVersions(ctx, v.SetValue) - diags.Append(d...) - if diags.HasError() { - return false, diags - } - - newVersions, d := extractVersions(ctx, newValue.SetValue) - diags.Append(d...) - if diags.HasError() { - return false, diags - } - - // Compare only the sets of versions - if len(oldVersions) != len(newVersions) { - return false, diags - } - - // Check if all versions in old set exist in new set - for version := range oldVersions { - if !newVersions[version] { - return false, diags - } - } - - return true, diags -} - -// extractVersions extracts version strings from a set of required version objects -func extractVersions(ctx context.Context, setValue basetypes.SetValue) (map[string]bool, diag.Diagnostics) { - var diags diag.Diagnostics - versions := make(map[string]bool) - - elements := setValue.Elements() - for _, elem := range elements { - obj, ok := elem.(basetypes.ObjectValue) - if !ok { - diags.AddError( - "Version Extraction Error", - fmt.Sprintf("Expected ObjectValue, got %T", elem), - ) - continue - } - - attrs := obj.Attributes() - versionAttr, ok := attrs["version"] - if !ok { - diags.AddError( - "Version Extraction Error", - "Required version object missing 'version' attribute", - ) - continue - } - - versionStr, ok := versionAttr.(types.String) - if !ok { - diags.AddError( - "Version Extraction Error", - fmt.Sprintf("Expected String for version, got %T", versionAttr), - ) - continue - } - - if !versionStr.IsNull() && !versionStr.IsUnknown() { - versions[versionStr.ValueString()] = true - } - } - - return versions, diags -} - -// NewRequiredVersionsValueNull creates a RequiredVersionsValue with a null value. -func NewRequiredVersionsValueNull(elemType attr.Type) RequiredVersionsValue { - return RequiredVersionsValue{ - SetValue: basetypes.NewSetNull(elemType), - } -} - -// NewRequiredVersionsValueUnknown creates a RequiredVersionsValue with an unknown value. -func NewRequiredVersionsValueUnknown(elemType attr.Type) RequiredVersionsValue { - return RequiredVersionsValue{ - SetValue: basetypes.NewSetUnknown(elemType), - } -} - -// NewRequiredVersionsValue creates a RequiredVersionsValue with a known value. -func NewRequiredVersionsValue(elemType attr.Type, elements []attr.Value) (RequiredVersionsValue, diag.Diagnostics) { - setValue, diags := basetypes.NewSetValue(elemType, elements) - return RequiredVersionsValue{ - SetValue: setValue, - }, diags -} - -// NewRequiredVersionsValueMust creates a RequiredVersionsValue with a known value and panics on error. -func NewRequiredVersionsValueMust(elemType attr.Type, elements []attr.Value) RequiredVersionsValue { - return RequiredVersionsValue{ - SetValue: basetypes.NewSetValueMust(elemType, elements), - } -} diff --git a/internal/fleet/agent_policy/schema.go b/internal/fleet/agent_policy/schema.go index 40432eae8..16414ae93 100644 --- a/internal/fleet/agent_policy/schema.go +++ b/internal/fleet/agent_policy/schema.go @@ -17,7 +17,6 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" - "github.com/hashicorp/terraform-plugin-framework/types/basetypes" ) func (r *agentPolicyResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { @@ -146,30 +145,13 @@ func getSchema() schema.Schema { Optional: true, Computed: true, }, - "required_versions": schema.SetAttribute{ - Description: "Specifies target agent versions for automatic upgrade. Each entry contains a version string and a percentage of agents to upgrade to that version. Multiple entries with the same version are not allowed.", - ElementType: types.ObjectType{ - AttrTypes: map[string]attr.Type{ - "version": types.StringType, - "percentage": types.Int32Type, - }, - }, - Optional: true, - CustomType: RequiredVersionsType{SetType: basetypes.SetType{ElemType: types.ObjectType{AttrTypes: map[string]attr.Type{"version": types.StringType, "percentage": types.Int32Type}}}}, - Validators: []validator.Set{ - UniqueVersions(), - }, + "required_versions": schema.MapAttribute{ + Description: "Map of agent versions to target percentages for automatic upgrade. The key is the target version and the value is the percentage of agents to upgrade to that version.", + ElementType: types.Int32Type, + Optional: true, }, }} } func getGlobalDataTagsAttrTypes() attr.Type { return getSchema().Attributes["global_data_tags"].GetType() } -func getRequiredVersionsElementType() attr.Type { - return types.ObjectType{ - AttrTypes: map[string]attr.Type{ - "version": types.StringType, - "percentage": types.Int32Type, - }, - } -} From 487e5910b76f1bd82840d8ce8c6bd736979b7c54 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 12 Nov 2025 17:07:13 +1100 Subject: [PATCH 06/11] Update internal/fleet/agent_policy/acc_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- internal/fleet/agent_policy/acc_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/fleet/agent_policy/acc_test.go b/internal/fleet/agent_policy/acc_test.go index 6e46d78b6..f17177f41 100644 --- a/internal/fleet/agent_policy/acc_test.go +++ b/internal/fleet/agent_policy/acc_test.go @@ -845,7 +845,7 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), - resource.TestCheckNoResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions"), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.%", "0"), ), }, }, From e858db40ee1c46a1e6b60cf88fcdd37965b53d28 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Nov 2025 06:36:18 +0000 Subject: [PATCH 07/11] Add version check for required_versions (9.1.0+) and fix empty map handling Co-authored-by: tobio <444668+tobio@users.noreply.github.com> --- internal/fleet/agent_policy/acc_test.go | 9 ++++---- internal/fleet/agent_policy/models.go | 30 ++++++++++++++++++++----- internal/fleet/agent_policy/resource.go | 7 ++++++ 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/internal/fleet/agent_policy/acc_test.go b/internal/fleet/agent_policy/acc_test.go index f17177f41..c6a653357 100644 --- a/internal/fleet/agent_policy/acc_test.go +++ b/internal/fleet/agent_policy/acc_test.go @@ -20,6 +20,7 @@ import ( ) var minVersionAgentPolicy = version.Must(version.NewVersion("8.6.0")) +var minVersionRequiredVersions = version.Must(version.NewVersion("9.1.0")) func TestAccResourceAgentPolicyFromSDK(t *testing.T) { policyName := sdkacctest.RandStringFromCharSet(22, sdkacctest.CharSetAlphaNum) @@ -809,7 +810,7 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { ProtoV6ProviderFactories: acctest.Providers, Steps: []resource.TestStep{ { - SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), Config: testAccResourceAgentPolicyCreateWithRequiredVersions(policyName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), @@ -819,7 +820,7 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { ), }, { - SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), Config: testAccResourceAgentPolicyUpdateRequiredVersionsPercentage(policyName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), @@ -829,7 +830,7 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { ), }, { - SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), Config: testAccResourceAgentPolicyAddRequiredVersion(policyName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), @@ -840,7 +841,7 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { ), }, { - SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionAgentPolicy), + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), Config: testAccResourceAgentPolicyRemoveRequiredVersions(policyName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), diff --git a/internal/fleet/agent_policy/models.go b/internal/fleet/agent_policy/models.go index 9c4d7018a..8ccb12811 100644 --- a/internal/fleet/agent_policy/models.go +++ b/internal/fleet/agent_policy/models.go @@ -22,6 +22,7 @@ type features struct { SupportsInactivityTimeout bool SupportsUnenrollmentTimeout bool SupportsSpaceIds bool + SupportsRequiredVersions bool } type globalDataTagsItemModel struct { @@ -140,7 +141,9 @@ func (model *agentPolicyModel) populateFromAPI(ctx context.Context, data *kbapi. versionMap := make(map[string]attr.Value) for _, rv := range *data.RequiredVersions { - versionMap[rv.Version] = types.Int32Value(int32(rv.Percentage)) + // Round the float32 percentage to nearest integer since we use Int32 in the schema + percentage := int32(rv.Percentage + 0.5) + versionMap[rv.Version] = types.Int32Value(percentage) } reqVersions, d := types.MapValue(types.Int32Type, versionMap) @@ -205,7 +208,7 @@ func (model *agentPolicyModel) convertGlobalDataTags(ctx context.Context, feat f } // convertRequiredVersions converts the required versions from terraform model to API model -func (model *agentPolicyModel) convertRequiredVersions(ctx context.Context) (*[]struct { +func (model *agentPolicyModel) convertRequiredVersions(ctx context.Context, feat features) (*[]struct { Percentage float32 `json:"percentage"` Version string `json:"version"` }, diag.Diagnostics) { @@ -215,9 +218,26 @@ func (model *agentPolicyModel) convertRequiredVersions(ctx context.Context) (*[] return nil, diags } + // Check if required_versions is supported + if !feat.SupportsRequiredVersions { + return nil, diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Root("required_versions"), + "Unsupported Elasticsearch version", + fmt.Sprintf("Required versions (automatic agent upgrades) are only supported in Elastic Stack %s and above", MinVersionRequiredVersions), + ), + } + } + elements := model.RequiredVersions.Elements() + + // If the map is empty (required_versions = {}), return an empty array to clear upgrades if len(elements) == 0 { - return nil, diags + emptyArray := make([]struct { + Percentage float32 `json:"percentage"` + Version string `json:"version"` + }, 0) + return &emptyArray, diags } result := make([]struct { @@ -350,7 +370,7 @@ func (model *agentPolicyModel) toAPICreateModel(ctx context.Context, feat featur } // Handle required_versions - requiredVersions, d := model.convertRequiredVersions(ctx) + requiredVersions, d := model.convertRequiredVersions(ctx, feat) if d.HasError() { return kbapi.PostFleetAgentPoliciesJSONRequestBody{}, d } @@ -454,7 +474,7 @@ func (model *agentPolicyModel) toAPIUpdateModel(ctx context.Context, feat featur } // Handle required_versions - requiredVersions, d := model.convertRequiredVersions(ctx) + requiredVersions, d := model.convertRequiredVersions(ctx, feat) if d.HasError() { return kbapi.PutFleetAgentPoliciesAgentpolicyidJSONRequestBody{}, d } diff --git a/internal/fleet/agent_policy/resource.go b/internal/fleet/agent_policy/resource.go index 5af42e7af..c0028d162 100644 --- a/internal/fleet/agent_policy/resource.go +++ b/internal/fleet/agent_policy/resource.go @@ -24,6 +24,7 @@ var ( MinVersionInactivityTimeout = version.Must(version.NewVersion("8.7.0")) MinVersionUnenrollmentTimeout = version.Must(version.NewVersion("8.15.0")) MinVersionSpaceIds = version.Must(version.NewVersion("9.1.0")) + MinVersionRequiredVersions = version.Must(version.NewVersion("9.1.0")) ) // NewResource is a helper function to simplify the provider implementation. @@ -75,11 +76,17 @@ func (r *agentPolicyResource) buildFeatures(ctx context.Context) (features, diag return features{}, diagutil.FrameworkDiagsFromSDK(diags) } + supportsRequiredVersions, diags := r.client.EnforceMinVersion(ctx, MinVersionRequiredVersions) + if diags.HasError() { + return features{}, diagutil.FrameworkDiagsFromSDK(diags) + } + return features{ SupportsGlobalDataTags: supportsGDT, SupportsSupportsAgentless: supportsSupportsAgentless, SupportsInactivityTimeout: supportsInactivityTimeout, SupportsUnenrollmentTimeout: supportsUnenrollmentTimeout, SupportsSpaceIds: supportsSpaceIds, + SupportsRequiredVersions: supportsRequiredVersions, }, nil } From 42771fcabbbbb758b065530314eaf86e3d95119b Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Thu, 13 Nov 2025 15:27:11 +1100 Subject: [PATCH 08/11] Restructure tests and fixup the null versions case --- internal/fleet/agent_policy/acc_test.go | 150 +++++------------- internal/fleet/agent_policy/models.go | 2 +- internal/fleet/agent_policy/schema.go | 5 + .../add_version/main.tf | 26 +++ .../create/main.tf | 25 +++ .../remove_versions/main.tf | 23 +++ .../unset_versions/main.tf | 22 +++ .../update_percentage/main.tf | 25 +++ 8 files changed, 168 insertions(+), 110 deletions(-) create mode 100644 internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/add_version/main.tf create mode 100644 internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/create/main.tf create mode 100644 internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/remove_versions/main.tf create mode 100644 internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/unset_versions/main.tf create mode 100644 internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/update_percentage/main.tf diff --git a/internal/fleet/agent_policy/acc_test.go b/internal/fleet/agent_policy/acc_test.go index a0321fb8c..d9d918243 100644 --- a/internal/fleet/agent_policy/acc_test.go +++ b/internal/fleet/agent_policy/acc_test.go @@ -530,13 +530,16 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { policyName := sdkacctest.RandStringFromCharSet(22, sdkacctest.CharSetAlphaNum) resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - CheckDestroy: checkResourceAgentPolicyDestroy, - ProtoV6ProviderFactories: acctest.Providers, + PreCheck: func() { acctest.PreCheck(t) }, + CheckDestroy: checkResourceAgentPolicyDestroy, Steps: []resource.TestStep{ { - SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), - Config: testAccResourceAgentPolicyCreateWithRequiredVersions(policyName), + ProtoV6ProviderFactories: acctest.Providers, + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), + ConfigDirectory: acctest.NamedTestCaseDirectory("create"), + ConfigVariables: config.Variables{ + "policy_name": config.StringVariable(fmt.Sprintf("Policy %s", policyName)), + }, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), @@ -545,8 +548,12 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { ), }, { - SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), - Config: testAccResourceAgentPolicyUpdateRequiredVersionsPercentage(policyName), + ProtoV6ProviderFactories: acctest.Providers, + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), + ConfigDirectory: acctest.NamedTestCaseDirectory("update_percentage"), + ConfigVariables: config.Variables{ + "policy_name": config.StringVariable(fmt.Sprintf("Policy %s", policyName)), + }, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), @@ -555,8 +562,27 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { ), }, { - SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), - Config: testAccResourceAgentPolicyAddRequiredVersion(policyName), + ProtoV6ProviderFactories: acctest.Providers, + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), + ConfigDirectory: acctest.NamedTestCaseDirectory("add_version"), + ConfigVariables: config.Variables{ + "policy_name": config.StringVariable(fmt.Sprintf("Policy %s", policyName)), + }, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.%", "2"), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.8.15.0", "50"), + resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "required_versions.8.16.0", "50"), + ), + }, + { + ProtoV6ProviderFactories: acctest.Providers, + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), + ConfigDirectory: acctest.NamedTestCaseDirectory("unset_versions"), + ConfigVariables: config.Variables{ + "policy_name": config.StringVariable(fmt.Sprintf("Policy %s", policyName)), + }, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), @@ -566,8 +592,12 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { ), }, { - SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), - Config: testAccResourceAgentPolicyRemoveRequiredVersions(policyName), + ProtoV6ProviderFactories: acctest.Providers, + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionRequiredVersions), + ConfigDirectory: acctest.NamedTestCaseDirectory("remove_versions"), + ConfigVariables: config.Variables{ + "policy_name": config.StringVariable(fmt.Sprintf("Policy %s", policyName)), + }, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "name", fmt.Sprintf("Policy %s", policyName)), resource.TestCheckResourceAttr("elasticstack_fleet_agent_policy.test_policy", "namespace", "default"), @@ -577,101 +607,3 @@ func TestAccResourceAgentPolicyWithRequiredVersions(t *testing.T) { }, }) } - -func testAccResourceAgentPolicyCreateWithRequiredVersions(id string) string { - return fmt.Sprintf(` -provider "elasticstack" { - elasticsearch {} - kibana {} -} - -resource "elasticstack_fleet_agent_policy" "test_policy" { - name = "%s" - namespace = "default" - description = "Test Agent Policy with Required Versions" - monitor_logs = true - monitor_metrics = false - skip_destroy = false - required_versions = { - "8.15.0" = 100 - } -} - -data "elasticstack_fleet_enrollment_tokens" "test_policy" { - policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id -} -`, fmt.Sprintf("Policy %s", id)) -} - -func testAccResourceAgentPolicyUpdateRequiredVersionsPercentage(id string) string { - return fmt.Sprintf(` -provider "elasticstack" { - elasticsearch {} - kibana {} -} - -resource "elasticstack_fleet_agent_policy" "test_policy" { - name = "%s" - namespace = "default" - description = "Test Agent Policy with Required Versions - Updated Percentage" - monitor_logs = true - monitor_metrics = false - skip_destroy = false - required_versions = { - "8.15.0" = 50 - } -} - -data "elasticstack_fleet_enrollment_tokens" "test_policy" { - policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id -} -`, fmt.Sprintf("Policy %s", id)) -} - -func testAccResourceAgentPolicyAddRequiredVersion(id string) string { - return fmt.Sprintf(` -provider "elasticstack" { - elasticsearch {} - kibana {} -} - -resource "elasticstack_fleet_agent_policy" "test_policy" { - name = "%s" - namespace = "default" - description = "Test Agent Policy with Multiple Required Versions" - monitor_logs = true - monitor_metrics = false - skip_destroy = false - required_versions = { - "8.15.0" = 50 - "8.16.0" = 50 - } -} - -data "elasticstack_fleet_enrollment_tokens" "test_policy" { - policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id -} -`, fmt.Sprintf("Policy %s", id)) -} - -func testAccResourceAgentPolicyRemoveRequiredVersions(id string) string { - return fmt.Sprintf(` -provider "elasticstack" { - elasticsearch {} - kibana {} -} - -resource "elasticstack_fleet_agent_policy" "test_policy" { - name = "%s" - namespace = "default" - description = "Test Agent Policy without Required Versions" - monitor_logs = true - monitor_metrics = false - skip_destroy = false -} - -data "elasticstack_fleet_enrollment_tokens" "test_policy" { - policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id -} -`, fmt.Sprintf("Policy %s", id)) -} diff --git a/internal/fleet/agent_policy/models.go b/internal/fleet/agent_policy/models.go index 8ccb12811..8e45db26d 100644 --- a/internal/fleet/agent_policy/models.go +++ b/internal/fleet/agent_policy/models.go @@ -137,7 +137,7 @@ func (model *agentPolicyModel) populateFromAPI(ctx context.Context, data *kbapi. } // Handle required_versions - if data.RequiredVersions != nil && len(*data.RequiredVersions) > 0 { + if data.RequiredVersions != nil { versionMap := make(map[string]attr.Value) for _, rv := range *data.RequiredVersions { diff --git a/internal/fleet/agent_policy/schema.go b/internal/fleet/agent_policy/schema.go index 16414ae93..76014c871 100644 --- a/internal/fleet/agent_policy/schema.go +++ b/internal/fleet/agent_policy/schema.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault" "github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/mapdefault" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/mapplanmodifier" "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" @@ -149,6 +150,10 @@ func getSchema() schema.Schema { Description: "Map of agent versions to target percentages for automatic upgrade. The key is the target version and the value is the percentage of agents to upgrade to that version.", ElementType: types.Int32Type, Optional: true, + Computed: true, + PlanModifiers: []planmodifier.Map{ + mapplanmodifier.UseStateForUnknown(), + }, }, }} } diff --git a/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/add_version/main.tf b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/add_version/main.tf new file mode 100644 index 000000000..8573493c8 --- /dev/null +++ b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/add_version/main.tf @@ -0,0 +1,26 @@ +variable "policy_name" { + type = string + description = "Name for the agent policy" +} + +provider "elasticstack" { + elasticsearch {} + kibana {} +} + +resource "elasticstack_fleet_agent_policy" "test_policy" { + name = var.policy_name + namespace = "default" + description = "Test Agent Policy with Multiple Required Versions" + monitor_logs = true + monitor_metrics = false + skip_destroy = false + required_versions = { + "8.15.0" = 50 + "8.16.0" = 50 + } +} + +data "elasticstack_fleet_enrollment_tokens" "test_policy" { + policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id +} diff --git a/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/create/main.tf b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/create/main.tf new file mode 100644 index 000000000..88b58460a --- /dev/null +++ b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/create/main.tf @@ -0,0 +1,25 @@ +variable "policy_name" { + type = string + description = "Name for the agent policy" +} + +provider "elasticstack" { + elasticsearch {} + kibana {} +} + +resource "elasticstack_fleet_agent_policy" "test_policy" { + name = var.policy_name + namespace = "default" + description = "Test Agent Policy with Required Versions" + monitor_logs = true + monitor_metrics = false + skip_destroy = false + required_versions = { + "8.15.0" = 100 + } +} + +data "elasticstack_fleet_enrollment_tokens" "test_policy" { + policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id +} diff --git a/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/remove_versions/main.tf b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/remove_versions/main.tf new file mode 100644 index 000000000..23886f854 --- /dev/null +++ b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/remove_versions/main.tf @@ -0,0 +1,23 @@ +variable "policy_name" { + type = string + description = "Name for the agent policy" +} + +provider "elasticstack" { + elasticsearch {} + kibana {} +} + +resource "elasticstack_fleet_agent_policy" "test_policy" { + name = var.policy_name + namespace = "default" + description = "Test Agent Policy without Required Versions" + monitor_logs = true + monitor_metrics = false + skip_destroy = false + required_versions = {} +} + +data "elasticstack_fleet_enrollment_tokens" "test_policy" { + policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id +} diff --git a/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/unset_versions/main.tf b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/unset_versions/main.tf new file mode 100644 index 000000000..2160d3333 --- /dev/null +++ b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/unset_versions/main.tf @@ -0,0 +1,22 @@ +variable "policy_name" { + type = string + description = "Name for the agent policy" +} + +provider "elasticstack" { + elasticsearch {} + kibana {} +} + +resource "elasticstack_fleet_agent_policy" "test_policy" { + name = var.policy_name + namespace = "default" + description = "Test Agent Policy without Required Versions" + monitor_logs = true + monitor_metrics = false + skip_destroy = false +} + +data "elasticstack_fleet_enrollment_tokens" "test_policy" { + policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id +} diff --git a/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/update_percentage/main.tf b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/update_percentage/main.tf new file mode 100644 index 000000000..fbe53d067 --- /dev/null +++ b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/update_percentage/main.tf @@ -0,0 +1,25 @@ +variable "policy_name" { + type = string + description = "Name for the agent policy" +} + +provider "elasticstack" { + elasticsearch {} + kibana {} +} + +resource "elasticstack_fleet_agent_policy" "test_policy" { + name = var.policy_name + namespace = "default" + description = "Test Agent Policy with Required Versions - Updated Percentage" + monitor_logs = true + monitor_metrics = false + skip_destroy = false + required_versions = { + "8.15.0" = 50 + } +} + +data "elasticstack_fleet_enrollment_tokens" "test_policy" { + policy_id = elasticstack_fleet_agent_policy.test_policy.policy_id +} From ab8f8abc8bf5bf66b790db3923b875391bc319a1 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Thu, 13 Nov 2025 15:30:33 +1100 Subject: [PATCH 09/11] make lint --- .../remove_versions/main.tf | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/remove_versions/main.tf b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/remove_versions/main.tf index 23886f854..792a938aa 100644 --- a/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/remove_versions/main.tf +++ b/internal/fleet/agent_policy/testdata/TestAccResourceAgentPolicyWithRequiredVersions/remove_versions/main.tf @@ -9,12 +9,12 @@ provider "elasticstack" { } resource "elasticstack_fleet_agent_policy" "test_policy" { - name = var.policy_name - namespace = "default" - description = "Test Agent Policy without Required Versions" - monitor_logs = true - monitor_metrics = false - skip_destroy = false + name = var.policy_name + namespace = "default" + description = "Test Agent Policy without Required Versions" + monitor_logs = true + monitor_metrics = false + skip_destroy = false required_versions = {} } From fc97622a75675765c269fcb49ee5fabec36cb316 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Nov 2025 04:53:38 +0000 Subject: [PATCH 10/11] Use math.Round() for explicit rounding semantics Co-authored-by: tobio <444668+tobio@users.noreply.github.com> --- internal/fleet/agent_policy/models.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/fleet/agent_policy/models.go b/internal/fleet/agent_policy/models.go index 8e45db26d..80614df3b 100644 --- a/internal/fleet/agent_policy/models.go +++ b/internal/fleet/agent_policy/models.go @@ -3,6 +3,7 @@ package agent_policy import ( "context" "fmt" + "math" "slices" "time" @@ -142,7 +143,7 @@ func (model *agentPolicyModel) populateFromAPI(ctx context.Context, data *kbapi. for _, rv := range *data.RequiredVersions { // Round the float32 percentage to nearest integer since we use Int32 in the schema - percentage := int32(rv.Percentage + 0.5) + percentage := int32(math.Round(float64(rv.Percentage))) versionMap[rv.Version] = types.Int32Value(percentage) } From ec49d5aff0eacf041621724d1b6f767cd1880e11 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Mon, 17 Nov 2025 14:57:06 +1100 Subject: [PATCH 11/11] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe238fb3c..e54c496af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## [Unreleased] +- Add `required_versions` to `elasticstack_fleet_agent_policy` ([#1436](https://github.com/elastic/terraform-provider-elasticstack/pull/1436)) - Fix `elasticstack_elasticsearch_snapshot_lifecycle` metadata type conversion causing terraform apply to fail ([#1409](https://github.com/elastic/terraform-provider-elasticstack/issues/1409)) - Add new `elasticstack_elasticsearch_ml_anomaly_detection_job` resource ([#1329](https://github.com/elastic/terraform-provider-elasticstack/pull/1329)) - Add new `elasticstack_elasticsearch_ml_datafeed` resource ([1340](https://github.com/elastic/terraform-provider-elasticstack/pull/1340))