From eb442a06d818f4bfaf02548a817e25e72dd4d7ca Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Mon, 20 Oct 2025 10:35:24 -0600 Subject: [PATCH 01/14] Add requireValue to conditionalRequirement --- internal/utils/validators/conditional.go | 30 ++++++++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/internal/utils/validators/conditional.go b/internal/utils/validators/conditional.go index a13d400a5..4cf0ebddc 100644 --- a/internal/utils/validators/conditional.go +++ b/internal/utils/validators/conditional.go @@ -18,6 +18,7 @@ import ( type conditionalRequirement struct { dependentPath path.Path allowedValues []string + requireValue bool } // Description describes the validation in plain text formatting. @@ -34,13 +35,27 @@ func (v conditionalRequirement) MarkdownDescription(ctx context.Context) string } func (v conditionalRequirement) validate(ctx context.Context, config tfsdk.Config, val attr.Value, p path.Path) diag.Diagnostics { - if val.IsNull() || val.IsUnknown() { - return nil - } // Get the value at the dependent path var dependentValue types.String diags := config.GetAttribute(ctx, v.dependentPath, &dependentValue) + + isEmpty := val.IsNull() || val.IsUnknown() + if isEmpty && v.requireValue { + diags.AddAttributeError(p, "Invalid Configuration", + fmt.Sprintf("Attribute %s must be set when %s equals %q", + p, + v.dependentPath, + v.allowedValues[0], + ), + ) + return diags + } + + if isEmpty { + return nil + } + if diags.HasError() { return diags } @@ -131,14 +146,19 @@ func StringConditionalRequirementSingle(dependentPath path.Path, requiredValue s return StringConditionalRequirement(dependentPath, []string{requiredValue}) } -func Int64ConditionalRequirement(dependentPath path.Path, allowedValues []string) validator.Int64 { +func Int64ConditionalRequirement(dependentPath path.Path, allowedValues []string, requireValue bool) validator.Int64 { return conditionalRequirement{ dependentPath: dependentPath, allowedValues: allowedValues, + requireValue: requireValue, } } // Int64ConditionalRequirementSingle is a convenience function for when there's only one allowed value. func Int64ConditionalRequirementSingle(dependentPath path.Path, requiredValue string) validator.Int64 { - return Int64ConditionalRequirement(dependentPath, []string{requiredValue}) + return Int64ConditionalRequirement(dependentPath, []string{requiredValue}, false) +} + +func Int64ConditionalRequirementInclusionSingle(dependentPath path.Path, requiredValue string) validator.Int64 { + return Int64ConditionalRequirement(dependentPath, []string{requiredValue}, true) } From f61132e72782099f3f9c12e1907b696a9504f5fa Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Tue, 21 Oct 2025 11:39:36 -0600 Subject: [PATCH 02/14] Add validations for valueAllowed, valueRequired, valueForbidden --- internal/utils/validators/conditional.go | 453 +++++++++++++--- internal/utils/validators/conditional_test.go | 497 +++++++++++++++++- 2 files changed, 879 insertions(+), 71 deletions(-) diff --git a/internal/utils/validators/conditional.go b/internal/utils/validators/conditional.go index 4cf0ebddc..a56622e98 100644 --- a/internal/utils/validators/conditional.go +++ b/internal/utils/validators/conditional.go @@ -12,94 +12,208 @@ import ( "github.com/hashicorp/terraform-plugin-framework/types" ) -// conditionalRequirement represents a validator which ensures that an attribute -// can only be set if another attribute at a specified path equals one of the specified values. -// This is a shared implementation that can be used for both string and float64 validators. -type conditionalRequirement struct { +// condition represents a validation rule that enforces conditional requirements +// based on the value of a dependent field. It contains the path to the field +// that this condition depends on and a list of allowed values for that field. +// When the dependent field matches one of the allowed values, additional +// validation logic can be applied to the current field. +type condition struct { dependentPath path.Path allowedValues []string - requireValue bool +} + +// conditionalValidation represents a validation rule that applies different constraints +// based on a specified condition. It controls whether a value is required, allowed, +// or forbidden when the condition is met. This struct is used to implement +// conditional validation logic where the validation behavior depends on the +// evaluation of a particular condition. +type conditionalValidation struct { + condition condition + valueRequired bool + valueAllowed bool + valueForbidden bool } // Description describes the validation in plain text formatting. -func (v conditionalRequirement) Description(_ context.Context) string { - if len(v.allowedValues) == 1 { - return fmt.Sprintf("value can only be set when %s equals %q", v.dependentPath, v.allowedValues[0]) +func (v conditionalValidation) Description(_ context.Context) string { + if v.valueForbidden { + return fmt.Sprintf("Value cannot be set when %s equals %q", + v.condition.dependentPath, + v.condition.allowedValues[0], + ) + } else if v.valueRequired { + return fmt.Sprintf("Value must be set when %s equals %q", + v.condition.dependentPath, + v.condition.allowedValues[0], + ) + } else if v.valueAllowed { + if len(v.condition.allowedValues) == 1 { + return fmt.Sprintf("value can only be set when %s equals %q", v.condition.dependentPath, v.condition.allowedValues[0]) + } + return fmt.Sprintf("value can only be set when %s is one of %v", v.condition.dependentPath, v.condition.allowedValues) + } else { + return fmt.Sprintf("Unknown validation for %s equals %q", + v.condition.dependentPath, + v.condition.allowedValues[0], + ) } - return fmt.Sprintf("value can only be set when %s is one of %v", v.dependentPath, v.allowedValues) } // MarkdownDescription describes the validation in Markdown formatting. -func (v conditionalRequirement) MarkdownDescription(ctx context.Context) string { +func (v conditionalValidation) MarkdownDescription(ctx context.Context) string { return v.Description(ctx) } -func (v conditionalRequirement) validate(ctx context.Context, config tfsdk.Config, val attr.Value, p path.Path) diag.Diagnostics { +// Description describes the validation in plain text formatting. +func (v condition) Description(_ context.Context) string { + return fmt.Sprintf("Attribute '%v' is not one of %s", + v.dependentPath, + v.allowedValues, + ) +} + +// MarkdownDescription describes the validation in Markdown formatting. +func (v condition) MarkdownDescription(ctx context.Context) string { + return v.Description(ctx) +} - // Get the value at the dependent path +// isConditionMet checks if the condition is satisfied by evaluating the dependent field's value +func (v condition) isConditionMet(ctx context.Context, config tfsdk.Config) (bool, string, diag.Diagnostics) { var dependentValue types.String diags := config.GetAttribute(ctx, v.dependentPath, &dependentValue) - isEmpty := val.IsNull() || val.IsUnknown() - if isEmpty && v.requireValue { - diags.AddAttributeError(p, "Invalid Configuration", - fmt.Sprintf("Attribute %s must be set when %s equals %q", - p, - v.dependentPath, - v.allowedValues[0], - ), - ) - return diags - } - - if isEmpty { - return nil - } - if diags.HasError() { - return diags + return false, "", diags } // If dependent value is null, unknown, or doesn't match any allowed values, - // then the current attribute should not be set + // then the condition is not met dependentValueStr := dependentValue.ValueString() - isAllowed := false + conditionMet := false if !dependentValue.IsNull() && !dependentValue.IsUnknown() { for _, allowedValue := range v.allowedValues { if dependentValueStr == allowedValue { - isAllowed = true + conditionMet = true break } } } - if !isAllowed { - if len(v.allowedValues) == 1 { + return conditionMet, dependentValueStr, nil +} + +func (v condition) validate(ctx context.Context, config tfsdk.Config, val attr.Value, p path.Path) diag.Diagnostics { + conditionMet, dependentValueStr, diags := v.isConditionMet(ctx, config) + if diags.HasError() { + return diags + } + + if !conditionMet { + diags.AddAttributeError(p, "Invalid Configuration", fmt.Sprintf("Attribute '%s' is not one of %v, %s is %q", + v.dependentPath, + v.allowedValues, + v.dependentPath, + dependentValueStr, + )) + + return diags + + } + return nil +} + +func (v conditionalValidation) validateValueForbidden(conditionMet bool, dependentValueStr string, val attr.Value, p path.Path) diag.Diagnostics { + var diags diag.Diagnostics + + if !conditionMet { + return diags + } + + isEmpty := val.IsNull() || val.IsUnknown() + isSet := !isEmpty + if isSet { + diags.AddAttributeError(p, "Invalid Configuration", + fmt.Sprintf("Attribute %s cannot be set when %s equals %q", + p, + v.condition.dependentPath, + v.condition.allowedValues[0], + ), + ) + } + return diags +} + +func (v conditionalValidation) validateValueRequired(conditionMet bool, dependentValueStr string, val attr.Value, p path.Path) diag.Diagnostics { + var diags diag.Diagnostics + isEmpty := val.IsNull() || val.IsUnknown() + + if !conditionMet { + return diags + } + + if isEmpty { + diags.AddAttributeError(p, "Invalid Configuration", + fmt.Sprintf("Attribute %s must be set when %s equals %q", + p, + v.condition.dependentPath, + v.condition.allowedValues[0], + ), + ) + } + return diags +} + +func (v conditionalValidation) validateValueAllowed(conditionMet bool, dependentValueStr string, val attr.Value, p path.Path) diag.Diagnostics { + var diags diag.Diagnostics + isEmpty := val.IsNull() || val.IsUnknown() + isSet := !isEmpty + + if conditionMet { + return diags + } + + if isSet { + if len(v.condition.allowedValues) == 1 { diags.AddAttributeError(p, "Invalid Configuration", fmt.Sprintf("Attribute %s can only be set when %s equals %q, but %s is %q", p, - v.dependentPath, - v.allowedValues[0], - v.dependentPath, + v.condition.dependentPath, + v.condition.allowedValues[0], + v.condition.dependentPath, dependentValueStr, ), ) - return diags } else { diags.AddAttributeError(p, "Invalid Configuration", fmt.Sprintf("Attribute %s can only be set when %s is one of %v, but %s is %q", p, - v.dependentPath, - v.allowedValues, - v.dependentPath, + v.condition.dependentPath, + v.condition.allowedValues, + v.condition.dependentPath, dependentValueStr, ), ) - return diags } } + return diags +} + +func (v conditionalValidation) validate(ctx context.Context, config tfsdk.Config, val attr.Value, p path.Path) diag.Diagnostics { + conditionMet, dependentValueStr, diags := v.condition.isConditionMet(ctx, config) + if diags.HasError() { + return diags + } + + if v.valueForbidden { + return v.validateValueForbidden(conditionMet, dependentValueStr, val, p) + } else if v.valueRequired { + return v.validateValueRequired(conditionMet, dependentValueStr, val, p) + } else if v.valueAllowed { + return v.validateValueAllowed(conditionMet, dependentValueStr, val, p) + } + return nil } @@ -107,16 +221,97 @@ func (v conditionalRequirement) validate(ctx context.Context, config tfsdk.Confi // The validation logic is implemented directly in ValidateString and ValidateFloat64 methods // ValidateString performs the validation for string attributes. -func (v conditionalRequirement) ValidateString(ctx context.Context, request validator.StringRequest, response *validator.StringResponse) { +func (v conditionalValidation) ValidateString(ctx context.Context, request validator.StringRequest, response *validator.StringResponse) { response.Diagnostics.Append(v.validate(ctx, request.Config, request.ConfigValue, request.Path)...) } // ValidateInt64 performs the validation for int64 attributes. -func (v conditionalRequirement) ValidateInt64(ctx context.Context, request validator.Int64Request, response *validator.Int64Response) { +func (v conditionalValidation) ValidateInt64(ctx context.Context, request validator.Int64Request, response *validator.Int64Response) { + response.Diagnostics.Append(v.validate(ctx, request.Config, request.ConfigValue, request.Path)...) +} + +// ValidateInt64 performs the validation for List attributes. +func (v conditionalValidation) ValidateList(ctx context.Context, request validator.ListRequest, response *validator.ListResponse) { + response.Diagnostics.Append(v.validate(ctx, request.Config, request.ConfigValue, request.Path)...) +} +func (v condition) ValidateString(ctx context.Context, request validator.StringRequest, response *validator.StringResponse) { + response.Diagnostics.Append(v.validate(ctx, request.Config, request.ConfigValue, request.Path)...) +} + +func (v condition) ValidateList(ctx context.Context, request validator.ListRequest, response *validator.ListResponse) { response.Diagnostics.Append(v.validate(ctx, request.Config, request.ConfigValue, request.Path)...) } -// StringConditionalRequirement returns a validator which ensures that a string attribute +func (v condition) ValidateInt64(ctx context.Context, request validator.Int64Request, response *validator.Int64Response) { + response.Diagnostics.Append(v.validate(ctx, request.Config, request.ConfigValue, request.Path)...) +} + +// Assertion validations validate that the path matches one of the provided values and fail otherwise +// eg using Any to skip validation for some cases +// stringvalidator.Any( +// stringvalidator.ExactlyOneOf(path.MatchRoot("index"), path.MatchRoot("data_view_id")), +// validators.StringAssert( +// path.Root("type"), +// []string{"machine_learning", "esql"}, +// ), +// ) +// ------------------------------------------------------------------------------ + +// StringAssert returns a validator that ensures a string attribute's value is within +// the allowedValues slice when the attribute at dependentPath meets certain conditions. +// This conditional validator is typically used to enforce in combination with other validations +// eg composing with validator.Any to skip validation for certain cases +// +// Parameters: +// - dependentPath: The path to the attribute that this validation depends on +// - allowedValues: The slice of string values that are considered valid for assertion +// +// Returns a validator.String that can be used in schema attribute validation. +func StringAssert(dependentPath path.Path, allowedValues []string) validator.String { + return condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + } +} + +// ListAssert creates a list validator that conditionally validates list values based on another attribute. +// It returns a validator that checks if the list contains only values from the allowedValues slice +// when the attribute at dependentPath meets certain conditions. +// +// Parameters: +// - dependentPath: The path to the attribute that this validator depends on +// - allowedValues: A slice of strings representing the valid values for the list +// +// Returns a validator.List that can be used to validate list attributes conditionally. +func ListAssert(dependentPath path.Path, allowedValues []string) validator.List { + return condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + } +} + +// Int64Assert creates a conditional validator for int64 values that checks if the current +// attribute is valid based on the value of another attribute specified by dependentPath. +// The validation passes when the dependent attribute's value is one of the allowedValues. +// This is useful for implementing conditional validation logic where an int64 field +// should only be validated or have certain constraints when another field has specific values. +// +// Parameters: +// - dependentPath: The path to the attribute whose value determines if validation should occur +// - allowedValues: A slice of string values that the dependent attribute must match for validation to pass +// +// Returns a validator.Int64 that can be used in Terraform schema validation. +func Int64Assert(dependentPath path.Path, allowedValues []string) validator.Int64 { + return condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + } +} + +// Allowance validations validate that the value is only set when the condition is met +// ---------------------------------------------------------------------------------- + +// StringConditionalAllowance returns a validator which ensures that a string attribute // can only be set if another attribute at the specified path equals one of the specified values. // // The dependentPath parameter should use path.Root() to specify the attribute path. @@ -127,38 +322,174 @@ func (v conditionalRequirement) ValidateInt64(ctx context.Context, request valid // "connection_type": schema.StringAttribute{ // Optional: true, // Validators: []validator.String{ -// validators.StringConditionalRequirement( +// validators.StringConditionalAllowance( // path.Root("auth_type"), // []string{"none"}, -// "connection_type can only be set when auth_type is 'none'", // ), // }, // }, +func StringConditionalAllowance(dependentPath path.Path, allowedValues []string) validator.String { + return conditionalValidation{ + condition: condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + }, + valueAllowed: true, + } +} + +// StringConditionalAllowanceSingle is a convenience function for when there's only one allowed value. +func StringConditionalAllowanceSingle(dependentPath path.Path, requiredValue string) validator.String { + return StringConditionalAllowance(dependentPath, []string{requiredValue}) +} + +// Int64ConditionalAllowanceSingle is a convenience function for when there's only one allowed value. +func Int64ConditionalAllowanceSingle(dependentPath path.Path, requiredValue string) validator.Int64 { + return conditionalValidation{ + condition: condition{ + dependentPath: dependentPath, + allowedValues: []string{requiredValue}, + }, + valueAllowed: true, + } +} + +// Int64ConditionalAllowance returns a validator which ensures that an int64 attribute +// can only be set if another attribute at the specified path equals one of the specified values. +// +// The dependentPath parameter should use path.Root() to specify the attribute path. +// For example: path.Root("auth_type") +// +// Example usage: +// +// "connection_timeout": schema.Int64Attribute{ +// Optional: true, +// Validators: []validator.Int64{ +// validators.Int64ConditionalAllowance( +// path.Root("auth_type"), +// []string{"basic", "oauth"}, +// ), +// }, +// }, +func Int64ConditionalAllowance(dependentPath path.Path, requiredValues []string) validator.Int64 { + return conditionalValidation{ + condition: condition{ + dependentPath: dependentPath, + allowedValues: requiredValues, + }, + valueAllowed: true, + } +} + +// Requirement validations validate that the value is set when the condition is met +// ---------------------------------------------------------------------------------- + +// Int64ConditionalRequirementSingle is a convenience function for when there's only one required value. +func Int64ConditionalRequirementSingle(dependentPath path.Path, requiredValue string) validator.Int64 { + return conditionalValidation{ + condition: condition{ + dependentPath: dependentPath, + allowedValues: []string{requiredValue}, + }, + valueRequired: true, + } +} + +// Int64ConditionalRequirement returns a validator that requires an int64 value to be present +// when the field at the specified dependentPath contains one of the allowedValues. +// +// Parameters: +// - dependentPath: The path to the field whose value determines if this field is required +// - allowedValues: A slice of string values that trigger the requirement when found in the dependent field +// +// Returns: +// - validator.Int64: A validator that enforces the conditional requirement rule +func Int64ConditionalRequirement(dependentPath path.Path, allowedValues []string) validator.Int64 { + return conditionalValidation{ + condition: condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + }, + valueRequired: true, + } +} + +// StringConditionalRequirement returns a validator that requires a string value to be present +// when the field at the specified dependentPath contains one of the allowedValues. +// +// Parameters: +// - dependentPath: The path to the field whose value determines if this field is required +// - allowedValues: A slice of string values that trigger the requirement when found in the dependent field +// +// Returns: +// - validator.String: A validator that enforces the conditional requirement rule func StringConditionalRequirement(dependentPath path.Path, allowedValues []string) validator.String { - return conditionalRequirement{ - dependentPath: dependentPath, - allowedValues: allowedValues, + return conditionalValidation{ + condition: condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + }, + valueRequired: true, } } -// StringConditionalRequirementSingle is a convenience function for when there's only one allowed value. +// StringConditionalRequirementSingle creates a string validator that requires this field to be set +// when the dependent field at dependentPath equals the specified requiredValue. +// This is a convenience wrapper around StringConditionalRequirement for single value conditions. func StringConditionalRequirementSingle(dependentPath path.Path, requiredValue string) validator.String { return StringConditionalRequirement(dependentPath, []string{requiredValue}) } -func Int64ConditionalRequirement(dependentPath path.Path, allowedValues []string, requireValue bool) validator.Int64 { - return conditionalRequirement{ - dependentPath: dependentPath, - allowedValues: allowedValues, - requireValue: requireValue, +// Forbidden validate that the value is NOT set when the condition is met +// ------------------------------------------------------------------------------- + +// ListConditionalForbidden returns a validator that restricts a list attribute from having any values +// when a dependent attribute at the specified path contains one of the allowed values. +// When the dependent path's value matches any of the allowedValues, this validator will +// produce an error if the list attribute being validated is not empty. +// +// Parameters: +// - dependentPath: The path to the attribute whose value determines the validation behavior +// - allowedValues: The values that, when present in the dependent attribute, trigger the restriction +// +// Returns a List validator that enforces the conditional restriction rule. +func ListConditionalForbidden(dependentPath path.Path, allowedValues []string) validator.List { + return conditionalValidation{ + condition: condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + }, + valueForbidden: true, } } -// Int64ConditionalRequirementSingle is a convenience function for when there's only one allowed value. -func Int64ConditionalRequirementSingle(dependentPath path.Path, requiredValue string) validator.Int64 { - return Int64ConditionalRequirement(dependentPath, []string{requiredValue}, false) +// StringConditionalForbidden returns a string validator that restricts the current attribute +// from being set when the dependent attribute at dependentPath contains one of the +// allowedValues. This validator enforces that certain string attributes are forbidden +// when specific conditions are met on related attributes. +// +// Parameters: +// - dependentPath: The path to the attribute whose value determines the restriction +// - allowedValues: List of values that, when present in the dependent attribute, +// will forbid setting the current attribute +// +// Returns a validator.String that can be used in schema validation. +func StringConditionalForbidden(dependentPath path.Path, allowedValues []string) validator.String { + return conditionalValidation{ + condition: condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + }, + valueForbidden: true, + } } -func Int64ConditionalRequirementInclusionSingle(dependentPath path.Path, requiredValue string) validator.Int64 { - return Int64ConditionalRequirement(dependentPath, []string{requiredValue}, true) +func Int64ConditionalForbidden(dependentPath path.Path, allowedValues []string) validator.Int64 { + return conditionalValidation{ + condition: condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + }, + valueForbidden: true, + } } diff --git a/internal/utils/validators/conditional_test.go b/internal/utils/validators/conditional_test.go index faca2f8e5..981e11a1b 100644 --- a/internal/utils/validators/conditional_test.go +++ b/internal/utils/validators/conditional_test.go @@ -12,7 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" ) -func TestStringConditionalRequirement(t *testing.T) { +func TestStringConditionalAllowance(t *testing.T) { t.Parallel() type testCase struct { @@ -109,7 +109,7 @@ func TestStringConditionalRequirement(t *testing.T) { } // Create validator - v := StringConditionalRequirement( + v := StringConditionalAllowance( path.Root("auth_type"), []string{"none"}, ) @@ -139,8 +139,8 @@ func TestStringConditionalRequirement(t *testing.T) { } } -func TestStringConditionalRequirement_Description(t *testing.T) { - v := StringConditionalRequirement( +func TestStringConditionalAllowance_Description(t *testing.T) { + v := StringConditionalAllowance( path.Root("auth_type"), []string{"none"}, ) @@ -153,7 +153,482 @@ func TestStringConditionalRequirement_Description(t *testing.T) { } } -func TestInt64ConditionalRequirement(t *testing.T) { +func TestStringConditionalForbidden(t *testing.T) { + t.Parallel() + + type testCase struct { + name string + currentValue types.String + dependentValue types.String + expectedError bool + } + + testCases := []testCase{ + { + name: "valid - current null, dependent matches forbidden value", + currentValue: types.StringNull(), + dependentValue: types.StringValue("https"), + expectedError: false, + }, + { + name: "valid - current unknown, dependent matches forbidden value", + currentValue: types.StringUnknown(), + dependentValue: types.StringValue("https"), + expectedError: false, + }, + { + name: "valid - current set, dependent doesn't match forbidden value", + currentValue: types.StringValue("custom_cert"), + dependentValue: types.StringValue("http"), + expectedError: false, + }, + { + name: "invalid - current set, dependent matches forbidden value", + currentValue: types.StringValue("custom_cert"), + dependentValue: types.StringValue("https"), + expectedError: true, + }, + { + name: "invalid - current set, dependent matches one of multiple forbidden values", + currentValue: types.StringValue("custom_cert"), + dependentValue: types.StringValue("tls"), + expectedError: true, + }, + { + name: "valid - current set, dependent is null", + currentValue: types.StringValue("custom_cert"), + dependentValue: types.StringNull(), + expectedError: false, + }, + { + name: "valid - current set, dependent is unknown", + currentValue: types.StringValue("custom_cert"), + dependentValue: types.StringUnknown(), + expectedError: false, + }, + { + name: "valid - current null, dependent is null", + currentValue: types.StringNull(), + dependentValue: types.StringNull(), + expectedError: false, + }, + { + name: "valid - current null, dependent is unknown", + currentValue: types.StringNull(), + dependentValue: types.StringUnknown(), + expectedError: false, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + // Create a simple schema for testing + testSchema := schema.Schema{ + Attributes: map[string]schema.Attribute{ + "custom_cert": schema.StringAttribute{ + Optional: true, + }, + "protocol": schema.StringAttribute{ + Optional: true, + }, + }, + } + + // Create raw config values + currentTfValue, err := testCase.currentValue.ToTerraformValue(context.Background()) + if err != nil { + t.Fatalf("Error converting current value: %v", err) + } + dependentTfValue, err := testCase.dependentValue.ToTerraformValue(context.Background()) + if err != nil { + t.Fatalf("Error converting dependent value: %v", err) + } + + rawConfigValues := map[string]tftypes.Value{ + "custom_cert": currentTfValue, + "protocol": dependentTfValue, + } + + rawConfig := tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "custom_cert": tftypes.String, + "protocol": tftypes.String, + }, + }, + rawConfigValues, + ) + + config := tfsdk.Config{ + Raw: rawConfig, + Schema: testSchema, + } + + // Create validator - StringConditionalForbidden forbids the field when dependent matches forbidden values + v := StringConditionalForbidden( + path.Root("protocol"), + []string{"https", "tls"}, + ) + + // Create validation request + request := validator.StringRequest{ + Path: path.Root("custom_cert"), + ConfigValue: testCase.currentValue, + Config: config, + } + + // Run validation + response := &validator.StringResponse{} + v.ValidateString(context.Background(), request, response) + + // Check result + if testCase.expectedError { + if !response.Diagnostics.HasError() { + t.Errorf("Expected validation error but got none") + } + } else { + if response.Diagnostics.HasError() { + t.Errorf("Expected no validation error but got: %v", response.Diagnostics.Errors()) + } + } + }) + } +} + +func TestStringConditionalForbidden_Description(t *testing.T) { + v := StringConditionalForbidden( + path.Root("protocol"), + []string{"https", "tls"}, + ) + + description := v.Description(context.Background()) + // Note: Currently the Description method doesn't differentiate between allowed and forbidden + // This matches the current implementation behavior + expected := "Value cannot be set when protocol equals \"https\"" + + if description != expected { + t.Errorf("Expected description %q, got %q", expected, description) + } +} + +func TestStringConditionalRequirement(t *testing.T) { + t.Parallel() + + type testCase struct { + name string + currentValue types.String + dependentValue types.String + expectedError bool + } + + testCases := []testCase{ + { + name: "valid - current set, dependent matches required value", + currentValue: types.StringValue("some_value"), + dependentValue: types.StringValue("ssl"), + expectedError: false, + }, + { + name: "valid - current null, dependent doesn't match required value", + currentValue: types.StringNull(), + dependentValue: types.StringValue("none"), + expectedError: false, + }, + { + name: "valid - current unknown, dependent doesn't match required value", + currentValue: types.StringUnknown(), + dependentValue: types.StringValue("basic"), + expectedError: false, + }, + { + name: "valid - current set, dependent matches one of multiple allowed values", + currentValue: types.StringValue("certificate_path"), + dependentValue: types.StringValue("tls"), + expectedError: false, + }, + { + name: "invalid - current null, dependent matches required value", + currentValue: types.StringNull(), + dependentValue: types.StringValue("ssl"), + expectedError: true, + }, + { + name: "invalid - current unknown, dependent matches required value", + currentValue: types.StringUnknown(), + dependentValue: types.StringValue("tls"), + expectedError: true, + }, + { + name: "valid - current null, dependent is null", + currentValue: types.StringNull(), + dependentValue: types.StringNull(), + expectedError: false, + }, + { + name: "valid - current null, dependent is unknown", + currentValue: types.StringNull(), + dependentValue: types.StringUnknown(), + expectedError: false, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + // Create a simple schema for testing + testSchema := schema.Schema{ + Attributes: map[string]schema.Attribute{ + "ssl_cert": schema.StringAttribute{ + Optional: true, + }, + "security_mode": schema.StringAttribute{ + Optional: true, + }, + }, + } + + // Create raw config values + currentTfValue, err := testCase.currentValue.ToTerraformValue(context.Background()) + if err != nil { + t.Fatalf("Error converting current value: %v", err) + } + dependentTfValue, err := testCase.dependentValue.ToTerraformValue(context.Background()) + if err != nil { + t.Fatalf("Error converting dependent value: %v", err) + } + + rawConfigValues := map[string]tftypes.Value{ + "ssl_cert": currentTfValue, + "security_mode": dependentTfValue, + } + + rawConfig := tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "ssl_cert": tftypes.String, + "security_mode": tftypes.String, + }, + }, + rawConfigValues, + ) + + config := tfsdk.Config{ + Raw: rawConfig, + Schema: testSchema, + } + + // Create validator - StringConditionalRequirement requires the field when dependent matches allowed values + v := StringConditionalRequirement( + path.Root("security_mode"), + []string{"ssl", "tls"}, + ) + + // Create validation request + request := validator.StringRequest{ + Path: path.Root("ssl_cert"), + ConfigValue: testCase.currentValue, + Config: config, + } + + // Run validation + response := &validator.StringResponse{} + v.ValidateString(context.Background(), request, response) + + // Check result + if testCase.expectedError { + if !response.Diagnostics.HasError() { + t.Errorf("Expected validation error but got none") + } + } else { + if response.Diagnostics.HasError() { + t.Errorf("Expected no validation error but got: %v", response.Diagnostics.Errors()) + } + } + }) + } +} + +func TestStringConditionalRequirement_Description(t *testing.T) { + v := StringConditionalRequirement( + path.Root("security_mode"), + []string{"ssl", "tls"}, + ) + + description := v.Description(context.Background()) + expected := "Value must be set when security_mode equals \"ssl\"" + + if description != expected { + t.Errorf("Expected description %q, got %q", expected, description) + } +} + +func TestStringConditionalRequirementSingle_Description(t *testing.T) { + v := StringConditionalRequirementSingle( + path.Root("auth_type"), + "oauth", + ) + + description := v.Description(context.Background()) + expected := "Value must be set when auth_type equals \"oauth\"" + + if description != expected { + t.Errorf("Expected description %q, got %q", expected, description) + } +} + +func TestStringAssert(t *testing.T) { + t.Parallel() + + type testCase struct { + name string + currentValue types.String + dependentValue types.String + expectedError bool + } + + testCases := []testCase{ + { + name: "valid - current null, dependent matches allowed value", + currentValue: types.StringNull(), + dependentValue: types.StringValue("machine_learning"), + expectedError: false, + }, + { + name: "valid - current unknown, dependent matches allowed value", + currentValue: types.StringUnknown(), + dependentValue: types.StringValue("esql"), + expectedError: false, + }, + { + name: "valid - current set, dependent matches required value", + currentValue: types.StringValue("some_value"), + dependentValue: types.StringValue("machine_learning"), + expectedError: false, + }, + { + name: "invalid - current null, dependent doesn't match required value", + currentValue: types.StringNull(), + dependentValue: types.StringValue("other_type"), + expectedError: true, + }, + { + name: "invalid - current set, dependent doesn't match required value", + currentValue: types.StringValue("some_value"), + dependentValue: types.StringValue("other_type"), + expectedError: true, + }, + { + name: "invalid - current set, dependent is null", + currentValue: types.StringValue("some_value"), + dependentValue: types.StringNull(), + expectedError: true, + }, + { + name: "invalid - current set, dependent is unknown", + currentValue: types.StringValue("some_value"), + dependentValue: types.StringUnknown(), + expectedError: true, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + // Create a simple schema for testing + testSchema := schema.Schema{ + Attributes: map[string]schema.Attribute{ + "some_field": schema.StringAttribute{ + Optional: true, + }, + "type": schema.StringAttribute{ + Optional: true, + }, + }, + } + + // Create raw config values + currentTfValue, err := testCase.currentValue.ToTerraformValue(context.Background()) + if err != nil { + t.Fatalf("Error converting current value: %v", err) + } + dependentTfValue, err := testCase.dependentValue.ToTerraformValue(context.Background()) + if err != nil { + t.Fatalf("Error converting dependent value: %v", err) + } + + rawConfigValues := map[string]tftypes.Value{ + "some_field": currentTfValue, + "type": dependentTfValue, + } + + rawConfig := tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "some_field": tftypes.String, + "type": tftypes.String, + }, + }, + rawConfigValues, + ) + + config := tfsdk.Config{ + Raw: rawConfig, + Schema: testSchema, + } + + // Create validator - StringAssert validates that the dependent field matches allowed values + v := StringAssert( + path.Root("type"), + []string{"machine_learning", "esql"}, + ) + + // Create validation request + request := validator.StringRequest{ + Path: path.Root("some_field"), + ConfigValue: testCase.currentValue, + Config: config, + } + + // Run validation + response := &validator.StringResponse{} + v.ValidateString(context.Background(), request, response) + + // Check result + if testCase.expectedError { + if !response.Diagnostics.HasError() { + t.Errorf("Expected validation error but got none") + } + } else { + if response.Diagnostics.HasError() { + t.Errorf("Expected no validation error but got: %v", response.Diagnostics.Errors()) + } + } + }) + } +} + +func TestStringAssert_Description(t *testing.T) { + v := StringAssert( + path.Root("type"), + []string{"machine_learning", "esql"}, + ) + + description := v.Description(context.Background()) + expected := "Attribute 'type' is not one of [machine_learning esql]" + + if description != expected { + t.Errorf("Expected description %q, got %q", expected, description) + } +} + +func TestInt64ConditionalAllowance(t *testing.T) { t.Parallel() type testCase struct { @@ -249,11 +724,13 @@ func TestInt64ConditionalRequirement(t *testing.T) { Schema: testSchema, } - // Create validator - v := Int64ConditionalRequirement( - path.Root("compression"), - []string{"gzip"}, - ) + v := conditionalValidation{ + condition: condition{ + dependentPath: path.Root("compression"), + allowedValues: []string{"gzip"}, + }, + valueAllowed: true, + } // Create validation request request := validator.Int64Request{ From aeeac0ad0680ab7a5b0b3df304c639686f51326f Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Tue, 21 Oct 2025 11:42:10 -0600 Subject: [PATCH 03/14] Add validations to schema --- .../kibana/security_detection_rule/schema.go | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/internal/kibana/security_detection_rule/schema.go b/internal/kibana/security_detection_rule/schema.go index c131f78fc..ae3de62b7 100644 --- a/internal/kibana/security_detection_rule/schema.go +++ b/internal/kibana/security_detection_rule/schema.go @@ -5,10 +5,13 @@ import ( "regexp" "github.com/elastic/terraform-provider-elasticstack/internal/utils/customtypes" + "github.com/elastic/terraform-provider-elasticstack/internal/utils/validators" "github.com/hashicorp/terraform-plugin-framework-jsontypes/jsontypes" "github.com/hashicorp/terraform-plugin-framework-validators/int64validator" + "github.com/hashicorp/terraform-plugin-framework-validators/listvalidator" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault" @@ -73,6 +76,21 @@ func GetSchema() schema.Schema { "data_view_id": schema.StringAttribute{ MarkdownDescription: "Data view ID for the rule. Not supported for esql and machine_learning rule types.", Optional: true, + Validators: []validator.String{ + stringvalidator.All( + stringvalidator.Any( // Either index or data_view_id must be set except for esql and machine_learning where neither can be set + stringvalidator.ExactlyOneOf(path.MatchRoot("index"), path.MatchRoot("data_view_id")), + validators.StringAssert( + path.Root("type"), + []string{"machine_learning", "esql"}, + ), + ), + validators.StringConditionalForbidden( + path.Root("type"), + []string{"machine_learning", "esql"}, + ), + ), + }, }, "namespace": schema.StringAttribute{ MarkdownDescription: "Alerts index namespace. Available for all rule types.", @@ -108,6 +126,21 @@ func GetSchema() schema.Schema { MarkdownDescription: "Indices on which the rule functions.", Optional: true, Computed: true, + Validators: []validator.List{ + listvalidator.All( + listvalidator.Any( // Either index or data_view_id must be set except for esql and machine_learning where neither can be set + listvalidator.ExactlyOneOf(path.MatchRoot("index"), path.MatchRoot("data_view_id")), + validators.ListAssert( + path.Root("type"), + []string{"machine_learning", "esql"}, + ), + ), + validators.ListConditionalForbidden( + path.Root("type"), + []string{"machine_learning", "esql"}, + ), + ), + }, }, "enabled": schema.BoolAttribute{ MarkdownDescription: "Determines whether the rule is enabled.", @@ -302,6 +335,12 @@ func GetSchema() schema.Schema { MarkdownDescription: "Query and filter context array to define alert conditions as JSON. Supports complex filter structures including bool queries, term filters, range filters, etc. Available for all rule types.", Optional: true, CustomType: jsontypes.NormalizedType{}, + Validators: []validator.String{ + validators.StringConditionalForbidden( + path.Root("type"), + []string{"machine_learning", "esql"}, + ), + }, }, "note": schema.StringAttribute{ MarkdownDescription: "Notes to help investigate alerts produced by the rule.", @@ -602,6 +641,10 @@ func GetSchema() schema.Schema { MarkdownDescription: "Anomaly score threshold above which the rule creates an alert. Valid values are from 0 to 100. Required for machine_learning rules.", Optional: true, Validators: []validator.Int64{ + validators.Int64ConditionalRequirementSingle( + path.Root("type"), + "machine_learning", + ), int64validator.Between(0, 100), }, }, From 9a7b1a5f8d7468b0288cc7a96d1299c186256fe8 Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Tue, 21 Oct 2025 11:43:31 -0600 Subject: [PATCH 04/14] Remove model specific validations --- .../security_detection_rule/models_esql.go | 32 ------------------- .../models_machine_learning.go | 21 ------------ 2 files changed, 53 deletions(-) diff --git a/internal/kibana/security_detection_rule/models_esql.go b/internal/kibana/security_detection_rule/models_esql.go index a55eaeb0f..aa51a7997 100644 --- a/internal/kibana/security_detection_rule/models_esql.go +++ b/internal/kibana/security_detection_rule/models_esql.go @@ -9,7 +9,6 @@ import ( "github.com/google/uuid" "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/types" ) @@ -59,35 +58,10 @@ func (e EsqlRuleProcessor) ExtractId(response any) (string, diag.Diagnostics) { return value.Id.String(), diags } -// applyEsqlValidations validates that ESQL-specific constraints are met -func (d SecurityDetectionRuleData) applyEsqlValidations(diags *diag.Diagnostics) { - if utils.IsKnown(d.Index) { - diags.AddAttributeError( - path.Root("index"), - "Invalid attribute 'index'", - "ESQL rules do not use index patterns. Please remove the 'index' attribute.", - ) - } - - if utils.IsKnown(d.Filters) { - diags.AddAttributeError( - path.Root("filters"), - "Invalid attribute 'filters'", - "ESQL rules do not support filters. Please remove the 'filters' attribute.", - ) - } -} - func (d SecurityDetectionRuleData) toEsqlRuleCreateProps(ctx context.Context, client clients.MinVersionEnforceable) (kbapi.SecurityDetectionsAPIRuleCreateProps, diag.Diagnostics) { var diags diag.Diagnostics var createProps kbapi.SecurityDetectionsAPIRuleCreateProps - // Apply ESQL-specific validations - d.applyEsqlValidations(&diags) - if diags.HasError() { - return createProps, diags - } - esqlRule := kbapi.SecurityDetectionsAPIEsqlRuleCreateProps{ Name: kbapi.SecurityDetectionsAPIRuleName(d.Name.ValueString()), Description: kbapi.SecurityDetectionsAPIRuleDescription(d.Description.ValueString()), @@ -153,12 +127,6 @@ func (d SecurityDetectionRuleData) toEsqlRuleUpdateProps(ctx context.Context, cl var diags diag.Diagnostics var updateProps kbapi.SecurityDetectionsAPIRuleUpdateProps - // Apply ESQL-specific validations - d.applyEsqlValidations(&diags) - if diags.HasError() { - return updateProps, diags - } - // Parse ID to get space_id and rule_id compId, resourceIdDiags := clients.CompositeIdFromStrFw(d.Id.ValueString()) diags.Append(resourceIdDiags...) diff --git a/internal/kibana/security_detection_rule/models_machine_learning.go b/internal/kibana/security_detection_rule/models_machine_learning.go index 783adbbbe..7b3cf5c21 100644 --- a/internal/kibana/security_detection_rule/models_machine_learning.go +++ b/internal/kibana/security_detection_rule/models_machine_learning.go @@ -59,26 +59,10 @@ func (m MachineLearningRuleProcessor) ExtractId(response any) (string, diag.Diag return value.Id.String(), diags } -// applyMachineLearningValidations validates that Machine learning-specific constraints are met -func (d SecurityDetectionRuleData) applyMachineLearningValidations(diags *diag.Diagnostics) { - if !utils.IsKnown(d.AnomalyThreshold) { - diags.AddAttributeError( - path.Root("anomaly_threshold"), - "Missing attribute 'anomaly_threshold'", - "Machine learning rules require an 'anomaly_threshold' attribute.", - ) - } -} - func (d SecurityDetectionRuleData) toMachineLearningRuleCreateProps(ctx context.Context, client clients.MinVersionEnforceable) (kbapi.SecurityDetectionsAPIRuleCreateProps, diag.Diagnostics) { var diags diag.Diagnostics var createProps kbapi.SecurityDetectionsAPIRuleCreateProps - d.applyMachineLearningValidations(&diags) - if diags.HasError() { - return createProps, diags - } - mlRule := kbapi.SecurityDetectionsAPIMachineLearningRuleCreateProps{ Name: kbapi.SecurityDetectionsAPIRuleName(d.Name.ValueString()), Description: kbapi.SecurityDetectionsAPIRuleDescription(d.Description.ValueString()), @@ -156,11 +140,6 @@ func (d SecurityDetectionRuleData) toMachineLearningRuleUpdateProps(ctx context. var diags diag.Diagnostics var updateProps kbapi.SecurityDetectionsAPIRuleUpdateProps - d.applyMachineLearningValidations(&diags) - if diags.HasError() { - return updateProps, diags - } - // Parse ID to get space_id and rule_id compId, resourceIdDiags := clients.CompositeIdFromStrFw(d.Id.ValueString()) diags.Append(resourceIdDiags...) From 8b9ab7b6a92f727aa2afb6e5b0f506e14112a9d7 Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Tue, 21 Oct 2025 12:02:35 -0600 Subject: [PATCH 05/14] Fix invalid security rule tests --- .../security_detection_rule/acc_test.go | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/internal/kibana/security_detection_rule/acc_test.go b/internal/kibana/security_detection_rule/acc_test.go index 3f4a21f56..e80c291ce 100644 --- a/internal/kibana/security_detection_rule/acc_test.go +++ b/internal/kibana/security_detection_rule/acc_test.go @@ -73,7 +73,6 @@ func TestAccResourceSecurityDetectionRule_Query(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "severity", "medium"), resource.TestCheckResourceAttr(resourceName, "risk_score", "50"), resource.TestCheckResourceAttr(resourceName, "index.0", "logs-*"), - resource.TestCheckResourceAttr(resourceName, "data_view_id", "test-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "test-namespace"), resource.TestCheckResourceAttr(resourceName, "rule_name_override", "Custom Query Rule Name"), resource.TestCheckResourceAttr(resourceName, "timestamp_override", "@timestamp"), @@ -151,7 +150,6 @@ func TestAccResourceSecurityDetectionRule_Query(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "description", "Updated test query security detection rule"), resource.TestCheckResourceAttr(resourceName, "severity", "high"), resource.TestCheckResourceAttr(resourceName, "risk_score", "75"), - resource.TestCheckResourceAttr(resourceName, "data_view_id", "updated-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "updated-namespace"), resource.TestCheckResourceAttr(resourceName, "rule_name_override", "Updated Custom Query Rule Name"), resource.TestCheckResourceAttr(resourceName, "timestamp_override", "event.ingested"), @@ -268,7 +266,6 @@ func TestAccResourceSecurityDetectionRule_EQL(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "description", "Test EQL security detection rule"), resource.TestCheckResourceAttr(resourceName, "severity", "high"), resource.TestCheckResourceAttr(resourceName, "risk_score", "70"), - resource.TestCheckResourceAttr(resourceName, "index.0", "winlogbeat-*"), resource.TestCheckResourceAttr(resourceName, "tiebreaker_field", "@timestamp"), resource.TestCheckResourceAttr(resourceName, "data_view_id", "eql-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "eql-namespace"), @@ -728,7 +725,6 @@ func TestAccResourceSecurityDetectionRule_NewTerms(t *testing.T) { checkResourceJSONAttr(resourceName, "filters", `[{"bool": {"should": [{"wildcard": {"user.domain": "*.internal"}}, {"term": {"user.type": "service_account"}}]}}]`), resource.TestCheckResourceAttr(resourceName, "history_window_start", "now-14d"), - resource.TestCheckResourceAttr(resourceName, "data_view_id", "new-terms-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "new-terms-namespace"), resource.TestCheckResourceAttr(resourceName, "rule_name_override", "Custom New Terms Rule Name"), resource.TestCheckResourceAttr(resourceName, "timestamp_override", "user.created"), @@ -868,7 +864,6 @@ func TestAccResourceSecurityDetectionRule_SavedQuery(t *testing.T) { // Check filters field checkResourceJSONAttr(resourceName, "filters", `[{"prefix": {"event.action": "user_"}}]`), - resource.TestCheckResourceAttr(resourceName, "index.0", "logs-*"), resource.TestCheckResourceAttr(resourceName, "data_view_id", "saved-query-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "saved-query-namespace"), resource.TestCheckResourceAttr(resourceName, "rule_name_override", "Custom Saved Query Rule Name"), @@ -943,8 +938,6 @@ func TestAccResourceSecurityDetectionRule_SavedQuery(t *testing.T) { // Check filters field (updated values) checkResourceJSONAttr(resourceName, "filters", `[{"script": {"script": {"source": "doc['event.severity'].value > 2"}}}]`), - resource.TestCheckResourceAttr(resourceName, "index.0", "logs-*"), - resource.TestCheckResourceAttr(resourceName, "index.1", "audit-*"), resource.TestCheckResourceAttr(resourceName, "data_view_id", "updated-saved-query-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "updated-saved-query-namespace"), resource.TestCheckResourceAttr(resourceName, "rule_name_override", "Updated Custom Saved Query Rule Name"), @@ -1033,7 +1026,6 @@ func TestAccResourceSecurityDetectionRule_ThreatMatch(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "severity", "high"), resource.TestCheckResourceAttr(resourceName, "risk_score", "80"), resource.TestCheckResourceAttr(resourceName, "index.0", "logs-*"), - resource.TestCheckResourceAttr(resourceName, "data_view_id", "threat-match-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "threat-match-namespace"), resource.TestCheckResourceAttr(resourceName, "rule_name_override", "Custom Threat Match Rule Name"), resource.TestCheckResourceAttr(resourceName, "timestamp_override", "threat.indicator.first_seen"), @@ -1115,7 +1107,6 @@ func TestAccResourceSecurityDetectionRule_ThreatMatch(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "risk_score", "95"), resource.TestCheckResourceAttr(resourceName, "index.0", "logs-*"), resource.TestCheckResourceAttr(resourceName, "index.1", "network-*"), - resource.TestCheckResourceAttr(resourceName, "data_view_id", "updated-threat-match-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "updated-threat-match-namespace"), resource.TestCheckResourceAttr(resourceName, "rule_name_override", "Updated Custom Threat Match Rule Name"), resource.TestCheckResourceAttr(resourceName, "timestamp_override", "threat.indicator.last_seen"), @@ -1206,7 +1197,6 @@ func TestAccResourceSecurityDetectionRule_Threshold(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "description", "Test threshold security detection rule"), resource.TestCheckResourceAttr(resourceName, "severity", "medium"), resource.TestCheckResourceAttr(resourceName, "risk_score", "55"), - resource.TestCheckResourceAttr(resourceName, "index.0", "logs-*"), resource.TestCheckResourceAttr(resourceName, "data_view_id", "threshold-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "threshold-namespace"), resource.TestCheckResourceAttr(resourceName, "rule_name_override", "Custom Threshold Rule Name"), @@ -1277,8 +1267,6 @@ func TestAccResourceSecurityDetectionRule_Threshold(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "description", "Updated test threshold security detection rule"), resource.TestCheckResourceAttr(resourceName, "severity", "high"), resource.TestCheckResourceAttr(resourceName, "risk_score", "75"), - resource.TestCheckResourceAttr(resourceName, "index.0", "logs-*"), - resource.TestCheckResourceAttr(resourceName, "index.1", "audit-*"), resource.TestCheckResourceAttr(resourceName, "data_view_id", "updated-threshold-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "updated-threshold-namespace"), resource.TestCheckResourceAttr(resourceName, "rule_name_override", "Updated Custom Threshold Rule Name"), @@ -1437,7 +1425,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { to = "now" interval = "5m" index = ["logs-*"] - data_view_id = "test-data-view-id" namespace = "test-namespace" rule_name_override = "Custom Query Rule Name" timestamp_override = "@timestamp" @@ -1555,7 +1542,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { author = ["Test Author"] tags = ["test", "automation"] license = "Elastic License v2" - data_view_id = "updated-data-view-id" namespace = "updated-namespace" rule_name_override = "Updated Custom Query Rule Name" timestamp_override = "event.ingested" @@ -1695,7 +1681,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { from = "now-6m" to = "now" interval = "5m" - index = ["winlogbeat-*"] tiebreaker_field = "@timestamp" data_view_id = "eql-data-view-id" namespace = "eql-namespace" @@ -2273,7 +2258,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { index = ["logs-*"] new_terms_fields = ["user.name"] history_window_start = "now-14d" - data_view_id = "new-terms-data-view-id" namespace = "new-terms-namespace" rule_name_override = "Custom New Terms Rule Name" timestamp_override = "user.created" @@ -2492,7 +2476,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { from = "now-6m" to = "now" interval = "5m" - index = ["logs-*"] saved_id = "test-saved-query-id" data_view_id = "saved-query-data-view-id" namespace = "saved-query-namespace" @@ -2588,7 +2571,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { from = "now-6m" to = "now" interval = "5m" - index = ["logs-*", "audit-*"] saved_id = "test-saved-query-id-updated" data_view_id = "updated-saved-query-data-view-id" namespace = "updated-saved-query-namespace" @@ -2705,7 +2687,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { to = "now" interval = "5m" index = ["logs-*"] - data_view_id = "threat-match-data-view-id" namespace = "threat-match-namespace" rule_name_override = "Custom Threat Match Rule Name" timestamp_override = "threat.indicator.first_seen" @@ -2828,7 +2809,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { to = "now" interval = "5m" index = ["logs-*", "network-*"] - data_view_id = "updated-threat-match-data-view-id" namespace = "updated-threat-match-namespace" threat_index = ["threat-intel-*", "ioc-*"] threat_query = "threat.indicator.type:(ip OR domain)" @@ -2958,7 +2938,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { from = "now-6m" to = "now" interval = "5m" - index = ["logs-*"] data_view_id = "threshold-data-view-id" namespace = "threshold-namespace" rule_name_override = "Custom Threshold Rule Name" @@ -3065,7 +3044,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { from = "now-6m" to = "now" interval = "5m" - index = ["logs-*", "audit-*"] data_view_id = "updated-threshold-data-view-id" namespace = "updated-threshold-namespace" author = ["Test Author"] @@ -3206,7 +3184,6 @@ func TestAccResourceSecurityDetectionRule_WithConnectorAction(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "severity", "medium"), resource.TestCheckResourceAttr(resourceName, "risk_score", "50"), resource.TestCheckResourceAttr(resourceName, "index.0", "logs-*"), - resource.TestCheckResourceAttr(resourceName, "data_view_id", "connector-action-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "connector-action-namespace"), // Check risk score mapping @@ -3241,7 +3218,6 @@ func TestAccResourceSecurityDetectionRule_WithConnectorAction(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "description", "Updated test security detection rule with connector action"), resource.TestCheckResourceAttr(resourceName, "severity", "high"), resource.TestCheckResourceAttr(resourceName, "risk_score", "75"), - resource.TestCheckResourceAttr(resourceName, "data_view_id", "updated-connector-action-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "updated-connector-action-namespace"), resource.TestCheckResourceAttr(resourceName, "tags.#", "2"), resource.TestCheckResourceAttr(resourceName, "tags.0", "test"), @@ -3310,7 +3286,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { to = "now" interval = "5m" index = ["logs-*"] - data_view_id = "connector-action-data-view-id" namespace = "connector-action-namespace" risk_score_mapping = [ @@ -3381,7 +3356,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { to = "now" interval = "5m" index = ["logs-*"] - data_view_id = "updated-connector-action-data-view-id" namespace = "updated-connector-action-namespace" tags = ["test", "terraform"] @@ -3444,7 +3418,6 @@ func TestAccResourceSecurityDetectionRule_BuildingBlockType(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "severity", "low"), resource.TestCheckResourceAttr(resourceName, "risk_score", "21"), resource.TestCheckResourceAttr(resourceName, "index.0", "logs-*"), - resource.TestCheckResourceAttr(resourceName, "data_view_id", "building-block-data-view-id"), resource.TestCheckResourceAttr(resourceName, "namespace", "building-block-namespace"), resource.TestCheckResourceAttr(resourceName, "building_block_type", "default"), @@ -3503,7 +3476,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { to = "now" interval = "5m" index = ["logs-*"] - data_view_id = "building-block-data-view-id" namespace = "building-block-namespace" building_block_type = "default" } @@ -3528,7 +3500,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { from = "now-6m" to = "now" interval = "5m" - index = ["logs-*"] data_view_id = "updated-building-block-data-view-id" namespace = "updated-building-block-namespace" building_block_type = "default" @@ -3557,7 +3528,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { from = "now-6m" to = "now" interval = "5m" - index = ["logs-*"] data_view_id = "no-building-block-data-view-id" namespace = "no-building-block-namespace" } @@ -3716,7 +3686,6 @@ resource "elasticstack_kibana_security_detection_rule" "test" { to = "now" interval = "5m" index = ["logs-*"] - data_view_id = "no-filters-data-view-id" namespace = "no-filters-namespace" # Note: No filters field specified - this tests removing filters from a rule From 88162ca0bd77a5bbdda2f2847985598a08d5ab00 Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Tue, 21 Oct 2025 12:03:08 -0600 Subject: [PATCH 06/14] Update validation usage --- internal/fleet/output/schema.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/fleet/output/schema.go b/internal/fleet/output/schema.go index 590374c21..6a9669fad 100644 --- a/internal/fleet/output/schema.go +++ b/internal/fleet/output/schema.go @@ -152,10 +152,7 @@ func getSchema() schema.Schema { int64planmodifier.UseStateForUnknown(), }, Validators: []validator.Int64{ - validators.Int64ConditionalRequirement( - path.Root("kafka").AtName("compression"), - []string{"gzip"}, - ), + validators.Int64ConditionalAllowance(path.Root("kafka").AtName("compression"), []string{"gzip"}), }, }, "connection_type": schema.StringAttribute{ @@ -163,7 +160,7 @@ func getSchema() schema.Schema { Optional: true, Validators: []validator.String{ stringvalidator.OneOf("plaintext", "encryption"), - validators.StringConditionalRequirementSingle( + validators.StringConditionalAllowanceSingle( path.Root("kafka").AtName("auth_type"), "none", ), From 17a76dc65fac6b84417bcbe12282057da848f280 Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Tue, 21 Oct 2025 12:07:19 -0600 Subject: [PATCH 07/14] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 341ea9d58..14383e621 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## [Unreleased] - Fix regression restricting the characters in an `elasticstack_elasticsearch_role_mapping` `name`. ([#1373](https://github.com/elastic/terraform-provider-elasticstack/pull/1373)) +- Add schema validations to require either (but not both) `index` and `data_view_id` is set for relevant Security Detection Rules ## [0.12.0] - 2025-10-15 From 4b3c796ff9f0d1c5bd7374bf40482e51ce10738d Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Tue, 21 Oct 2025 14:16:54 -0600 Subject: [PATCH 08/14] Add comments for index/data_view_id validations --- internal/kibana/security_detection_rule/schema.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/kibana/security_detection_rule/schema.go b/internal/kibana/security_detection_rule/schema.go index ae3de62b7..345df33e6 100644 --- a/internal/kibana/security_detection_rule/schema.go +++ b/internal/kibana/security_detection_rule/schema.go @@ -78,13 +78,16 @@ func GetSchema() schema.Schema { Optional: true, Validators: []validator.String{ stringvalidator.All( - stringvalidator.Any( // Either index or data_view_id must be set except for esql and machine_learning where neither can be set + // Enforce that one of index or data_view_id are set if the rule type is not ml or esql + stringvalidator.Any( stringvalidator.ExactlyOneOf(path.MatchRoot("index"), path.MatchRoot("data_view_id")), validators.StringAssert( path.Root("type"), []string{"machine_learning", "esql"}, ), ), + + // Enforce that index is not set if the rule type is ml or esql validators.StringConditionalForbidden( path.Root("type"), []string{"machine_learning", "esql"}, @@ -128,13 +131,15 @@ func GetSchema() schema.Schema { Computed: true, Validators: []validator.List{ listvalidator.All( - listvalidator.Any( // Either index or data_view_id must be set except for esql and machine_learning where neither can be set + // Enforce that one of index or data_view_id are set if the rule type is not ml or esql + listvalidator.Any( listvalidator.ExactlyOneOf(path.MatchRoot("index"), path.MatchRoot("data_view_id")), validators.ListAssert( path.Root("type"), []string{"machine_learning", "esql"}, ), ), + // Enforce that index is not set if the rule type is ml or esql validators.ListConditionalForbidden( path.Root("type"), []string{"machine_learning", "esql"}, From 2c4fdd65a108d84d3cf0a059bb698c76180f3c19 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 22 Oct 2025 21:07:57 +1100 Subject: [PATCH 09/14] POC alternative conditional validation --- internal/fleet/output/schema.go | 4 +- .../kibana/security_detection_rule/schema.go | 12 +- internal/utils/validators/conditional.go | 446 +++++------------- internal/utils/validators/conditional_test.go | 182 +------ 4 files changed, 157 insertions(+), 487 deletions(-) diff --git a/internal/fleet/output/schema.go b/internal/fleet/output/schema.go index 6a9669fad..b0c25d986 100644 --- a/internal/fleet/output/schema.go +++ b/internal/fleet/output/schema.go @@ -152,7 +152,7 @@ func getSchema() schema.Schema { int64planmodifier.UseStateForUnknown(), }, Validators: []validator.Int64{ - validators.Int64ConditionalAllowance(path.Root("kafka").AtName("compression"), []string{"gzip"}), + validators.AllowedIfDependentPathEquals(path.Root("kafka").AtName("compression"), "gzip"), }, }, "connection_type": schema.StringAttribute{ @@ -160,7 +160,7 @@ func getSchema() schema.Schema { Optional: true, Validators: []validator.String{ stringvalidator.OneOf("plaintext", "encryption"), - validators.StringConditionalAllowanceSingle( + validators.AllowedIfDependentPathEquals( path.Root("kafka").AtName("auth_type"), "none", ), diff --git a/internal/kibana/security_detection_rule/schema.go b/internal/kibana/security_detection_rule/schema.go index 345df33e6..7f0cd41ec 100644 --- a/internal/kibana/security_detection_rule/schema.go +++ b/internal/kibana/security_detection_rule/schema.go @@ -81,14 +81,14 @@ func GetSchema() schema.Schema { // Enforce that one of index or data_view_id are set if the rule type is not ml or esql stringvalidator.Any( stringvalidator.ExactlyOneOf(path.MatchRoot("index"), path.MatchRoot("data_view_id")), - validators.StringAssert( + validators.DependantPathOneOf( path.Root("type"), []string{"machine_learning", "esql"}, ), ), // Enforce that index is not set if the rule type is ml or esql - validators.StringConditionalForbidden( + validators.ForbiddenIfDependentPathOneOf( path.Root("type"), []string{"machine_learning", "esql"}, ), @@ -134,13 +134,13 @@ func GetSchema() schema.Schema { // Enforce that one of index or data_view_id are set if the rule type is not ml or esql listvalidator.Any( listvalidator.ExactlyOneOf(path.MatchRoot("index"), path.MatchRoot("data_view_id")), - validators.ListAssert( + validators.DependantPathOneOf( path.Root("type"), []string{"machine_learning", "esql"}, ), ), // Enforce that index is not set if the rule type is ml or esql - validators.ListConditionalForbidden( + validators.ForbiddenIfDependentPathOneOf( path.Root("type"), []string{"machine_learning", "esql"}, ), @@ -341,7 +341,7 @@ func GetSchema() schema.Schema { Optional: true, CustomType: jsontypes.NormalizedType{}, Validators: []validator.String{ - validators.StringConditionalForbidden( + validators.ForbiddenIfDependentPathOneOf( path.Root("type"), []string{"machine_learning", "esql"}, ), @@ -646,7 +646,7 @@ func GetSchema() schema.Schema { MarkdownDescription: "Anomaly score threshold above which the rule creates an alert. Valid values are from 0 to 100. Required for machine_learning rules.", Optional: true, Validators: []validator.Int64{ - validators.Int64ConditionalRequirementSingle( + validators.RequiredIfDependentPathEquals( path.Root("type"), "machine_learning", ), diff --git a/internal/utils/validators/conditional.go b/internal/utils/validators/conditional.go index a56622e98..7714634dc 100644 --- a/internal/utils/validators/conditional.go +++ b/internal/utils/validators/conditional.go @@ -12,69 +12,28 @@ import ( "github.com/hashicorp/terraform-plugin-framework/types" ) +type valueValidator func(dependentFieldHasAllowedValue bool, dependentValueStr string, val attr.Value, p path.Path) diag.Diagnostics + // condition represents a validation rule that enforces conditional requirements // based on the value of a dependent field. It contains the path to the field // that this condition depends on and a list of allowed values for that field. // When the dependent field matches one of the allowed values, additional // validation logic can be applied to the current field. type condition struct { + description func() string dependentPath path.Path allowedValues []string -} - -// conditionalValidation represents a validation rule that applies different constraints -// based on a specified condition. It controls whether a value is required, allowed, -// or forbidden when the condition is met. This struct is used to implement -// conditional validation logic where the validation behavior depends on the -// evaluation of a particular condition. -type conditionalValidation struct { - condition condition - valueRequired bool - valueAllowed bool - valueForbidden bool -} - -// Description describes the validation in plain text formatting. -func (v conditionalValidation) Description(_ context.Context) string { - if v.valueForbidden { - return fmt.Sprintf("Value cannot be set when %s equals %q", - v.condition.dependentPath, - v.condition.allowedValues[0], - ) - } else if v.valueRequired { - return fmt.Sprintf("Value must be set when %s equals %q", - v.condition.dependentPath, - v.condition.allowedValues[0], - ) - } else if v.valueAllowed { - if len(v.condition.allowedValues) == 1 { - return fmt.Sprintf("value can only be set when %s equals %q", v.condition.dependentPath, v.condition.allowedValues[0]) - } - return fmt.Sprintf("value can only be set when %s is one of %v", v.condition.dependentPath, v.condition.allowedValues) - } else { - return fmt.Sprintf("Unknown validation for %s equals %q", - v.condition.dependentPath, - v.condition.allowedValues[0], - ) - } -} - -// MarkdownDescription describes the validation in Markdown formatting. -func (v conditionalValidation) MarkdownDescription(ctx context.Context) string { - return v.Description(ctx) + validateValue valueValidator } // Description describes the validation in plain text formatting. -func (v condition) Description(_ context.Context) string { - return fmt.Sprintf("Attribute '%v' is not one of %s", - v.dependentPath, - v.allowedValues, - ) +func (v condition) Description(ctx context.Context) string { + return v.description() } // MarkdownDescription describes the validation in Markdown formatting. func (v condition) MarkdownDescription(ctx context.Context) string { - return v.Description(ctx) + return v.description() } // isConditionMet checks if the condition is satisfied by evaluating the dependent field's value @@ -109,131 +68,9 @@ func (v condition) validate(ctx context.Context, config tfsdk.Config, val attr.V return diags } - if !conditionMet { - diags.AddAttributeError(p, "Invalid Configuration", fmt.Sprintf("Attribute '%s' is not one of %v, %s is %q", - v.dependentPath, - v.allowedValues, - v.dependentPath, - dependentValueStr, - )) - - return diags - - } - return nil -} - -func (v conditionalValidation) validateValueForbidden(conditionMet bool, dependentValueStr string, val attr.Value, p path.Path) diag.Diagnostics { - var diags diag.Diagnostics - - if !conditionMet { - return diags - } - - isEmpty := val.IsNull() || val.IsUnknown() - isSet := !isEmpty - if isSet { - diags.AddAttributeError(p, "Invalid Configuration", - fmt.Sprintf("Attribute %s cannot be set when %s equals %q", - p, - v.condition.dependentPath, - v.condition.allowedValues[0], - ), - ) - } - return diags -} - -func (v conditionalValidation) validateValueRequired(conditionMet bool, dependentValueStr string, val attr.Value, p path.Path) diag.Diagnostics { - var diags diag.Diagnostics - isEmpty := val.IsNull() || val.IsUnknown() - - if !conditionMet { - return diags - } - - if isEmpty { - diags.AddAttributeError(p, "Invalid Configuration", - fmt.Sprintf("Attribute %s must be set when %s equals %q", - p, - v.condition.dependentPath, - v.condition.allowedValues[0], - ), - ) - } - return diags -} - -func (v conditionalValidation) validateValueAllowed(conditionMet bool, dependentValueStr string, val attr.Value, p path.Path) diag.Diagnostics { - var diags diag.Diagnostics - isEmpty := val.IsNull() || val.IsUnknown() - isSet := !isEmpty - - if conditionMet { - return diags - } - - if isSet { - if len(v.condition.allowedValues) == 1 { - diags.AddAttributeError(p, "Invalid Configuration", - fmt.Sprintf("Attribute %s can only be set when %s equals %q, but %s is %q", - p, - v.condition.dependentPath, - v.condition.allowedValues[0], - v.condition.dependentPath, - dependentValueStr, - ), - ) - } else { - diags.AddAttributeError(p, "Invalid Configuration", - fmt.Sprintf("Attribute %s can only be set when %s is one of %v, but %s is %q", - p, - v.condition.dependentPath, - v.condition.allowedValues, - v.condition.dependentPath, - dependentValueStr, - ), - ) - } - } - - return diags -} - -func (v conditionalValidation) validate(ctx context.Context, config tfsdk.Config, val attr.Value, p path.Path) diag.Diagnostics { - conditionMet, dependentValueStr, diags := v.condition.isConditionMet(ctx, config) - if diags.HasError() { - return diags - } - - if v.valueForbidden { - return v.validateValueForbidden(conditionMet, dependentValueStr, val, p) - } else if v.valueRequired { - return v.validateValueRequired(conditionMet, dependentValueStr, val, p) - } else if v.valueAllowed { - return v.validateValueAllowed(conditionMet, dependentValueStr, val, p) - } - - return nil + return v.validateValue(conditionMet, dependentValueStr, val, p) } -// validateConditionalRequirement was an attempt at shared logic but is not used -// The validation logic is implemented directly in ValidateString and ValidateFloat64 methods - -// ValidateString performs the validation for string attributes. -func (v conditionalValidation) ValidateString(ctx context.Context, request validator.StringRequest, response *validator.StringResponse) { - response.Diagnostics.Append(v.validate(ctx, request.Config, request.ConfigValue, request.Path)...) -} - -// ValidateInt64 performs the validation for int64 attributes. -func (v conditionalValidation) ValidateInt64(ctx context.Context, request validator.Int64Request, response *validator.Int64Response) { - response.Diagnostics.Append(v.validate(ctx, request.Config, request.ConfigValue, request.Path)...) -} - -// ValidateInt64 performs the validation for List attributes. -func (v conditionalValidation) ValidateList(ctx context.Context, request validator.ListRequest, response *validator.ListResponse) { - response.Diagnostics.Append(v.validate(ctx, request.Config, request.ConfigValue, request.Path)...) -} func (v condition) ValidateString(ctx context.Context, request validator.StringRequest, response *validator.StringResponse) { response.Diagnostics.Append(v.validate(ctx, request.Config, request.ConfigValue, request.Path)...) } @@ -267,44 +104,31 @@ func (v condition) ValidateInt64(ctx context.Context, request validator.Int64Req // - allowedValues: The slice of string values that are considered valid for assertion // // Returns a validator.String that can be used in schema attribute validation. -func StringAssert(dependentPath path.Path, allowedValues []string) validator.String { +func DependantPathOneOf(dependentPath path.Path, allowedValues []string) condition { return condition{ dependentPath: dependentPath, allowedValues: allowedValues, - } -} + description: func() string { + return fmt.Sprintf("Attribute '%v' is not one of %s", + dependentPath, + allowedValues, + ) + }, + validateValue: func(dependentFieldHasAllowedValue bool, dependentValueStr string, val attr.Value, p path.Path) diag.Diagnostics { + if !dependentFieldHasAllowedValue { + var diags diag.Diagnostics + diags.AddAttributeError(p, "Invalid Configuration", fmt.Sprintf("Attribute '%s' is not one of %v, %s is %q", + dependentPath, + allowedValues, + dependentPath, + dependentValueStr, + )) -// ListAssert creates a list validator that conditionally validates list values based on another attribute. -// It returns a validator that checks if the list contains only values from the allowedValues slice -// when the attribute at dependentPath meets certain conditions. -// -// Parameters: -// - dependentPath: The path to the attribute that this validator depends on -// - allowedValues: A slice of strings representing the valid values for the list -// -// Returns a validator.List that can be used to validate list attributes conditionally. -func ListAssert(dependentPath path.Path, allowedValues []string) validator.List { - return condition{ - dependentPath: dependentPath, - allowedValues: allowedValues, - } -} + return diags + } -// Int64Assert creates a conditional validator for int64 values that checks if the current -// attribute is valid based on the value of another attribute specified by dependentPath. -// The validation passes when the dependent attribute's value is one of the allowedValues. -// This is useful for implementing conditional validation logic where an int64 field -// should only be validated or have certain constraints when another field has specific values. -// -// Parameters: -// - dependentPath: The path to the attribute whose value determines if validation should occur -// - allowedValues: A slice of string values that the dependent attribute must match for validation to pass -// -// Returns a validator.Int64 that can be used in Terraform schema validation. -func Int64Assert(dependentPath path.Path, allowedValues []string) validator.Int64 { - return condition{ - dependentPath: dependentPath, - allowedValues: allowedValues, + return nil + }, } } @@ -328,71 +152,65 @@ func Int64Assert(dependentPath path.Path, allowedValues []string) validator.Int6 // ), // }, // }, -func StringConditionalAllowance(dependentPath path.Path, allowedValues []string) validator.String { - return conditionalValidation{ - condition: condition{ - dependentPath: dependentPath, - allowedValues: allowedValues, +func AllowedIfDependentPathOneOf(dependentPath path.Path, allowedValues []string) condition { + return condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + description: func() string { + if len(allowedValues) == 1 { + return fmt.Sprintf("value can only be set when %s equals %q", dependentPath, allowedValues[0]) + } + return fmt.Sprintf("value can only be set when %s is one of %v", dependentPath, allowedValues) }, - valueAllowed: true, - } -} + validateValue: func(dependentFieldHasAllowedValue bool, dependentValueStr string, val attr.Value, p path.Path) diag.Diagnostics { + var diags diag.Diagnostics + isEmpty := val.IsNull() || val.IsUnknown() + isSet := !isEmpty -// StringConditionalAllowanceSingle is a convenience function for when there's only one allowed value. -func StringConditionalAllowanceSingle(dependentPath path.Path, requiredValue string) validator.String { - return StringConditionalAllowance(dependentPath, []string{requiredValue}) -} + if dependentFieldHasAllowedValue { + return diags + } + + if isSet { + if len(allowedValues) == 1 { + diags.AddAttributeError(p, "Invalid Configuration", + fmt.Sprintf("Attribute %s can only be set when %s equals %q, but %s is %q", + p, + dependentPath, + allowedValues[0], + dependentPath, + dependentValueStr, + ), + ) + } else { + diags.AddAttributeError(p, "Invalid Configuration", + fmt.Sprintf("Attribute %s can only be set when %s is one of %v, but %s is %q", + p, + dependentPath, + allowedValues, + dependentPath, + dependentValueStr, + ), + ) + } + } -// Int64ConditionalAllowanceSingle is a convenience function for when there's only one allowed value. -func Int64ConditionalAllowanceSingle(dependentPath path.Path, requiredValue string) validator.Int64 { - return conditionalValidation{ - condition: condition{ - dependentPath: dependentPath, - allowedValues: []string{requiredValue}, + return diags }, - valueAllowed: true, } } -// Int64ConditionalAllowance returns a validator which ensures that an int64 attribute -// can only be set if another attribute at the specified path equals one of the specified values. -// -// The dependentPath parameter should use path.Root() to specify the attribute path. -// For example: path.Root("auth_type") -// -// Example usage: -// -// "connection_timeout": schema.Int64Attribute{ -// Optional: true, -// Validators: []validator.Int64{ -// validators.Int64ConditionalAllowance( -// path.Root("auth_type"), -// []string{"basic", "oauth"}, -// ), -// }, -// }, -func Int64ConditionalAllowance(dependentPath path.Path, requiredValues []string) validator.Int64 { - return conditionalValidation{ - condition: condition{ - dependentPath: dependentPath, - allowedValues: requiredValues, - }, - valueAllowed: true, - } +// StringConditionalAllowanceSingle is a convenience function for when there's only one allowed value. +func AllowedIfDependentPathEquals(dependentPath path.Path, requiredValue string) condition { + return AllowedIfDependentPathOneOf(dependentPath, []string{requiredValue}) } // Requirement validations validate that the value is set when the condition is met // ---------------------------------------------------------------------------------- // Int64ConditionalRequirementSingle is a convenience function for when there's only one required value. -func Int64ConditionalRequirementSingle(dependentPath path.Path, requiredValue string) validator.Int64 { - return conditionalValidation{ - condition: condition{ - dependentPath: dependentPath, - allowedValues: []string{requiredValue}, - }, - valueRequired: true, - } +func RequiredIfDependentPathEquals(dependentPath path.Path, requiredValue string) condition { + return RequiredIfDependentPathOneOf(dependentPath, []string{requiredValue}) } // Int64ConditionalRequirement returns a validator that requires an int64 value to be present @@ -404,42 +222,38 @@ func Int64ConditionalRequirementSingle(dependentPath path.Path, requiredValue st // // Returns: // - validator.Int64: A validator that enforces the conditional requirement rule -func Int64ConditionalRequirement(dependentPath path.Path, allowedValues []string) validator.Int64 { - return conditionalValidation{ - condition: condition{ - dependentPath: dependentPath, - allowedValues: allowedValues, +func RequiredIfDependentPathOneOf(dependentPath path.Path, allowedValues []string) condition { + return condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + description: func() string { + if len(allowedValues) == 1 { + return fmt.Sprintf("value required when %s equals %q", dependentPath, allowedValues[0]) + } + return fmt.Sprintf("value required when %s is one of %v", dependentPath, allowedValues) }, - valueRequired: true, - } -} + validateValue: func(dependentFieldHasAllowedValue bool, dependentValueStr string, val attr.Value, p path.Path) diag.Diagnostics { + var diags diag.Diagnostics + isEmpty := val.IsNull() || val.IsUnknown() -// StringConditionalRequirement returns a validator that requires a string value to be present -// when the field at the specified dependentPath contains one of the allowedValues. -// -// Parameters: -// - dependentPath: The path to the field whose value determines if this field is required -// - allowedValues: A slice of string values that trigger the requirement when found in the dependent field -// -// Returns: -// - validator.String: A validator that enforces the conditional requirement rule -func StringConditionalRequirement(dependentPath path.Path, allowedValues []string) validator.String { - return conditionalValidation{ - condition: condition{ - dependentPath: dependentPath, - allowedValues: allowedValues, + if !dependentFieldHasAllowedValue { + return diags + } + + if isEmpty { + diags.AddAttributeError(p, "Invalid Configuration", + fmt.Sprintf("Attribute %s must be set when %s equals %q", + p, + dependentPath, + allowedValues[0], + ), + ) + } + return diags }, - valueRequired: true, } } -// StringConditionalRequirementSingle creates a string validator that requires this field to be set -// when the dependent field at dependentPath equals the specified requiredValue. -// This is a convenience wrapper around StringConditionalRequirement for single value conditions. -func StringConditionalRequirementSingle(dependentPath path.Path, requiredValue string) validator.String { - return StringConditionalRequirement(dependentPath, []string{requiredValue}) -} - // Forbidden validate that the value is NOT set when the condition is met // ------------------------------------------------------------------------------- @@ -453,43 +267,35 @@ func StringConditionalRequirementSingle(dependentPath path.Path, requiredValue s // - allowedValues: The values that, when present in the dependent attribute, trigger the restriction // // Returns a List validator that enforces the conditional restriction rule. -func ListConditionalForbidden(dependentPath path.Path, allowedValues []string) validator.List { - return conditionalValidation{ - condition: condition{ - dependentPath: dependentPath, - allowedValues: allowedValues, +func ForbiddenIfDependentPathOneOf(dependentPath path.Path, allowedValues []string) condition { + return condition{ + dependentPath: dependentPath, + allowedValues: allowedValues, + description: func() string { + if len(allowedValues) == 1 { + return fmt.Sprintf("value cannot be set when %s equals %q", dependentPath, allowedValues[0]) + } + return fmt.Sprintf("value cannot be set when %s is one of %v", dependentPath, allowedValues) }, - valueForbidden: true, - } -} + validateValue: func(dependentFieldHasAllowedValue bool, dependentValueStr string, val attr.Value, p path.Path) diag.Diagnostics { + var diags diag.Diagnostics -// StringConditionalForbidden returns a string validator that restricts the current attribute -// from being set when the dependent attribute at dependentPath contains one of the -// allowedValues. This validator enforces that certain string attributes are forbidden -// when specific conditions are met on related attributes. -// -// Parameters: -// - dependentPath: The path to the attribute whose value determines the restriction -// - allowedValues: List of values that, when present in the dependent attribute, -// will forbid setting the current attribute -// -// Returns a validator.String that can be used in schema validation. -func StringConditionalForbidden(dependentPath path.Path, allowedValues []string) validator.String { - return conditionalValidation{ - condition: condition{ - dependentPath: dependentPath, - allowedValues: allowedValues, - }, - valueForbidden: true, - } -} + if !dependentFieldHasAllowedValue { + return diags + } -func Int64ConditionalForbidden(dependentPath path.Path, allowedValues []string) validator.Int64 { - return conditionalValidation{ - condition: condition{ - dependentPath: dependentPath, - allowedValues: allowedValues, + isEmpty := val.IsNull() || val.IsUnknown() + isSet := !isEmpty + if isSet { + diags.AddAttributeError(p, "Invalid Configuration", + fmt.Sprintf("Attribute %s cannot be set when %s equals %q", + p, + dependentPath, + allowedValues[0], + ), + ) + } + return diags }, - valueForbidden: true, } } diff --git a/internal/utils/validators/conditional_test.go b/internal/utils/validators/conditional_test.go index 981e11a1b..f9e0c363e 100644 --- a/internal/utils/validators/conditional_test.go +++ b/internal/utils/validators/conditional_test.go @@ -10,9 +10,10 @@ import ( "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/stretchr/testify/require" ) -func TestStringConditionalAllowance(t *testing.T) { +func TestAllowedIfDependentPathOneOf(t *testing.T) { t.Parallel() type testCase struct { @@ -109,7 +110,7 @@ func TestStringConditionalAllowance(t *testing.T) { } // Create validator - v := StringConditionalAllowance( + v := AllowedIfDependentPathOneOf( path.Root("auth_type"), []string{"none"}, ) @@ -139,8 +140,8 @@ func TestStringConditionalAllowance(t *testing.T) { } } -func TestStringConditionalAllowance_Description(t *testing.T) { - v := StringConditionalAllowance( +func TestAllowedIfDependentPathOneOf_Description(t *testing.T) { + v := AllowedIfDependentPathOneOf( path.Root("auth_type"), []string{"none"}, ) @@ -153,7 +154,7 @@ func TestStringConditionalAllowance_Description(t *testing.T) { } } -func TestStringConditionalForbidden(t *testing.T) { +func TestForbiddenIfDependentPathOneOf(t *testing.T) { t.Parallel() type testCase struct { @@ -268,7 +269,7 @@ func TestStringConditionalForbidden(t *testing.T) { } // Create validator - StringConditionalForbidden forbids the field when dependent matches forbidden values - v := StringConditionalForbidden( + v := ForbiddenIfDependentPathOneOf( path.Root("protocol"), []string{"https", "tls"}, ) @@ -298,8 +299,8 @@ func TestStringConditionalForbidden(t *testing.T) { } } -func TestStringConditionalForbidden_Description(t *testing.T) { - v := StringConditionalForbidden( +func TestForbiddenIfDependentPathOneOf_Description(t *testing.T) { + v := ForbiddenIfDependentPathOneOf( path.Root("protocol"), []string{"https", "tls"}, ) @@ -307,14 +308,10 @@ func TestStringConditionalForbidden_Description(t *testing.T) { description := v.Description(context.Background()) // Note: Currently the Description method doesn't differentiate between allowed and forbidden // This matches the current implementation behavior - expected := "Value cannot be set when protocol equals \"https\"" - - if description != expected { - t.Errorf("Expected description %q, got %q", expected, description) - } + require.Equal(t, "value cannot be set when protocol is one of [https tls]", description) } -func TestStringConditionalRequirement(t *testing.T) { +func TestRequiredIfDependentPathOneOf(t *testing.T) { t.Parallel() type testCase struct { @@ -422,8 +419,8 @@ func TestStringConditionalRequirement(t *testing.T) { Schema: testSchema, } - // Create validator - StringConditionalRequirement requires the field when dependent matches allowed values - v := StringConditionalRequirement( + // Create validator - RequiredIfDependentPathOneOf requires the field when dependent matches allowed values + v := RequiredIfDependentPathOneOf( path.Root("security_mode"), []string{"ssl", "tls"}, ) @@ -453,35 +450,31 @@ func TestStringConditionalRequirement(t *testing.T) { } } -func TestStringConditionalRequirement_Description(t *testing.T) { - v := StringConditionalRequirement( +func TestRequiredIfDependentPathOneOf_Description(t *testing.T) { + v := RequiredIfDependentPathOneOf( path.Root("security_mode"), []string{"ssl", "tls"}, ) description := v.Description(context.Background()) - expected := "Value must be set when security_mode equals \"ssl\"" - - if description != expected { - t.Errorf("Expected description %q, got %q", expected, description) - } + require.Equal(t, "value required when security_mode is one of [ssl tls]", description) } -func TestStringConditionalRequirementSingle_Description(t *testing.T) { - v := StringConditionalRequirementSingle( +func TestRequiredIfDependentPathEquals_Description(t *testing.T) { + v := RequiredIfDependentPathEquals( path.Root("auth_type"), "oauth", ) description := v.Description(context.Background()) - expected := "Value must be set when auth_type equals \"oauth\"" + expected := "value required when auth_type equals \"oauth\"" if description != expected { t.Errorf("Expected description %q, got %q", expected, description) } } -func TestStringAssert(t *testing.T) { +func TestDependantPathOneOf(t *testing.T) { t.Parallel() type testCase struct { @@ -584,7 +577,7 @@ func TestStringAssert(t *testing.T) { } // Create validator - StringAssert validates that the dependent field matches allowed values - v := StringAssert( + v := DependantPathOneOf( path.Root("type"), []string{"machine_learning", "esql"}, ) @@ -614,8 +607,8 @@ func TestStringAssert(t *testing.T) { } } -func TestStringAssert_Description(t *testing.T) { - v := StringAssert( +func TestDependantPathOneOf_Description(t *testing.T) { + v := DependantPathOneOf( path.Root("type"), []string{"machine_learning", "esql"}, ) @@ -627,132 +620,3 @@ func TestStringAssert_Description(t *testing.T) { t.Errorf("Expected description %q, got %q", expected, description) } } - -func TestInt64ConditionalAllowance(t *testing.T) { - t.Parallel() - - type testCase struct { - name string - currentValue types.Int64 - dependentValue types.String - expectedError bool - } - - testCases := []testCase{ - { - name: "valid - current null, dependent any value", - currentValue: types.Int64Null(), - dependentValue: types.StringValue("none"), - expectedError: false, - }, - { - name: "valid - current unknown, dependent any value", - currentValue: types.Int64Unknown(), - dependentValue: types.StringValue("none"), - expectedError: false, - }, - { - name: "valid - current set, dependent matches required value", - currentValue: types.Int64Value(6), - dependentValue: types.StringValue("gzip"), - expectedError: false, - }, - { - name: "invalid - current set, dependent doesn't match required value", - currentValue: types.Int64Value(6), - dependentValue: types.StringValue("none"), - expectedError: true, - }, - { - name: "invalid - current set, dependent is null", - currentValue: types.Int64Value(6), - dependentValue: types.StringNull(), - expectedError: true, - }, - { - name: "invalid - current set, dependent is unknown", - currentValue: types.Int64Value(6), - dependentValue: types.StringUnknown(), - expectedError: true, - }, - } - - for _, testCase := range testCases { - testCase := testCase - t.Run(testCase.name, func(t *testing.T) { - t.Parallel() - - // Create a simple schema for testing - testSchema := schema.Schema{ - Attributes: map[string]schema.Attribute{ - "compression_level": schema.Float64Attribute{ - Optional: true, - }, - "compression": schema.StringAttribute{ - Optional: true, - }, - }, - } - - // Create raw config values - currentTfValue, err := testCase.currentValue.ToTerraformValue(context.Background()) - if err != nil { - t.Fatalf("Error converting current value: %v", err) - } - dependentTfValue, err := testCase.dependentValue.ToTerraformValue(context.Background()) - if err != nil { - t.Fatalf("Error converting dependent value: %v", err) - } - - rawConfigValues := map[string]tftypes.Value{ - "compression_level": currentTfValue, - "compression": dependentTfValue, - } - - rawConfig := tftypes.NewValue( - tftypes.Object{ - AttributeTypes: map[string]tftypes.Type{ - "compression_level": tftypes.Number, - "compression": tftypes.String, - }, - }, - rawConfigValues, - ) - - config := tfsdk.Config{ - Raw: rawConfig, - Schema: testSchema, - } - - v := conditionalValidation{ - condition: condition{ - dependentPath: path.Root("compression"), - allowedValues: []string{"gzip"}, - }, - valueAllowed: true, - } - - // Create validation request - request := validator.Int64Request{ - Path: path.Root("compression_level"), - ConfigValue: testCase.currentValue, - Config: config, - } - - // Run validation - response := &validator.Int64Response{} - v.ValidateInt64(context.Background(), request, response) - - // Check result - if testCase.expectedError { - if !response.Diagnostics.HasError() { - t.Errorf("Expected validation error but got none") - } - } else { - if response.Diagnostics.HasError() { - t.Errorf("Expected no validation error but got: %v", response.Diagnostics.Errors()) - } - } - }) - } -} From 861c68d02ce2164cb14ff393c661f81a50f41797 Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Wed, 22 Oct 2025 14:45:53 -0600 Subject: [PATCH 10/14] Cleanup conditional validations --- internal/utils/validators/conditional.go | 158 ++++++++++++++--------- 1 file changed, 96 insertions(+), 62 deletions(-) diff --git a/internal/utils/validators/conditional.go b/internal/utils/validators/conditional.go index 7714634dc..ec3e0362c 100644 --- a/internal/utils/validators/conditional.go +++ b/internal/utils/validators/conditional.go @@ -36,8 +36,20 @@ func (v condition) MarkdownDescription(ctx context.Context) string { return v.description() } -// isConditionMet checks if the condition is satisfied by evaluating the dependent field's value -func (v condition) isConditionMet(ctx context.Context, config tfsdk.Config) (bool, string, diag.Diagnostics) { +// dependentFieldHasAllowedValue checks if the dependent field specified by the condition's +// dependentPath has a value that matches one of the allowed values defined in the condition. +// It retrieves the dependent field's value from the provided configuration context and +// compares it against the condition's allowedValues slice. +// +// The method returns three values: +// - bool: true if the dependent field has a non-null, non-unknown value that matches +// one of the allowed values; false otherwise +// - string: the string representation of the dependent field's current value +// - diag.Diagnostics: any diagnostics encountered while retrieving the field value +// +// If the dependent field is null, unknown, or its value doesn't match any of the +// allowed values, the condition is considered not met and the method returns false. +func (v condition) dependentFieldHasAllowedValue(ctx context.Context, config tfsdk.Config) (bool, string, diag.Diagnostics) { var dependentValue types.String diags := config.GetAttribute(ctx, v.dependentPath, &dependentValue) @@ -48,27 +60,27 @@ func (v condition) isConditionMet(ctx context.Context, config tfsdk.Config) (boo // If dependent value is null, unknown, or doesn't match any allowed values, // then the condition is not met dependentValueStr := dependentValue.ValueString() - conditionMet := false + dependentFieldHasAllowedValue := false if !dependentValue.IsNull() && !dependentValue.IsUnknown() { for _, allowedValue := range v.allowedValues { if dependentValueStr == allowedValue { - conditionMet = true + dependentFieldHasAllowedValue = true break } } } - return conditionMet, dependentValueStr, nil + return dependentFieldHasAllowedValue, dependentValueStr, nil } func (v condition) validate(ctx context.Context, config tfsdk.Config, val attr.Value, p path.Path) diag.Diagnostics { - conditionMet, dependentValueStr, diags := v.isConditionMet(ctx, config) + dependentFieldHasAllowedValue, dependentValueStr, diags := v.dependentFieldHasAllowedValue(ctx, config) if diags.HasError() { return diags } - return v.validateValue(conditionMet, dependentValueStr, val, p) + return v.validateValue(dependentFieldHasAllowedValue, dependentValueStr, val, p) } func (v condition) ValidateString(ctx context.Context, request validator.StringRequest, response *validator.StringResponse) { @@ -83,27 +95,17 @@ func (v condition) ValidateInt64(ctx context.Context, request validator.Int64Req response.Diagnostics.Append(v.validate(ctx, request.Config, request.ConfigValue, request.Path)...) } -// Assertion validations validate that the path matches one of the provided values and fail otherwise -// eg using Any to skip validation for some cases -// stringvalidator.Any( -// stringvalidator.ExactlyOneOf(path.MatchRoot("index"), path.MatchRoot("data_view_id")), -// validators.StringAssert( -// path.Root("type"), -// []string{"machine_learning", "esql"}, -// ), -// ) -// ------------------------------------------------------------------------------ - -// StringAssert returns a validator that ensures a string attribute's value is within -// the allowedValues slice when the attribute at dependentPath meets certain conditions. -// This conditional validator is typically used to enforce in combination with other validations -// eg composing with validator.Any to skip validation for certain cases +// DependantPathOneOf creates a condition that validates a dependent path's value is one of the allowed values. +// It returns a condition that checks if the value at dependentPath matches any of the provided allowedValues. +// If the dependent field does not have an allowed value, it generates a diagnostic error indicating +// which values are permitted and what the current value is. // // Parameters: -// - dependentPath: The path to the attribute that this validation depends on -// - allowedValues: The slice of string values that are considered valid for assertion +// - dependentPath: The path to the attribute that must have one of the allowed values +// - allowedValues: A slice of strings representing the valid values for the dependent path // -// Returns a validator.String that can be used in schema attribute validation. +// Returns: +// - condition: A condition struct that can be used for validation func DependantPathOneOf(dependentPath path.Path, allowedValues []string) condition { return condition{ dependentPath: dependentPath, @@ -132,26 +134,20 @@ func DependantPathOneOf(dependentPath path.Path, allowedValues []string) conditi } } -// Allowance validations validate that the value is only set when the condition is met -// ---------------------------------------------------------------------------------- - -// StringConditionalAllowance returns a validator which ensures that a string attribute -// can only be set if another attribute at the specified path equals one of the specified values. +// AllowedIfDependentPathOneOf creates a validation condition that allows the current attribute +// to be set only when a dependent attribute at the specified path has one of the allowed values. // -// The dependentPath parameter should use path.Root() to specify the attribute path. -// For example: path.Root("auth_type") +// Parameters: +// - dependentPath: The path to the attribute that this validation depends on +// - allowedValues: A slice of string values that the dependent attribute must match // -// Example usage: +// Returns: +// - condition: A validation condition that can be used with conditional validators // -// "connection_type": schema.StringAttribute{ -// Optional: true, -// Validators: []validator.String{ -// validators.StringConditionalAllowance( -// path.Root("auth_type"), -// []string{"none"}, -// ), -// }, -// }, +// Example: +// +// // Only allow "ssl_cert" to be set when "protocol" is "https" +// AllowedIfDependentPathOneOf(path.Root("protocol"), []string{"https"}) func AllowedIfDependentPathOneOf(dependentPath path.Path, allowedValues []string) condition { return condition{ dependentPath: dependentPath, @@ -200,28 +196,57 @@ func AllowedIfDependentPathOneOf(dependentPath path.Path, allowedValues []string } } -// StringConditionalAllowanceSingle is a convenience function for when there's only one allowed value. +// AllowedIfDependentPathEquals returns a condition that allows a field to be set +// only if the value at the specified dependent path equals the required value. +// This is a convenience function that wraps AllowedIfDependentPathOneOf with a +// single value slice. +// +// Parameters: +// - dependentPath: The path to the field whose value determines if this field is allowed +// - requiredValue: The exact string value that the dependent field must equal +// +// Returns: +// - condition: A validation condition that enforces the dependency rule func AllowedIfDependentPathEquals(dependentPath path.Path, requiredValue string) condition { return AllowedIfDependentPathOneOf(dependentPath, []string{requiredValue}) } -// Requirement validations validate that the value is set when the condition is met -// ---------------------------------------------------------------------------------- - -// Int64ConditionalRequirementSingle is a convenience function for when there's only one required value. +// RequiredIfDependentPathEquals returns a condition that makes a field required +// when the value at the specified dependent path equals the given required value. +// This is a convenience function that wraps RequiredIfDependentPathOneOf with +// a single value slice. +// +// Parameters: +// - dependentPath: The path to the field whose value will be checked +// - requiredValue: The value that, when present at dependentPath, makes this field required +// +// Returns: +// - condition: A validation condition function func RequiredIfDependentPathEquals(dependentPath path.Path, requiredValue string) condition { return RequiredIfDependentPathOneOf(dependentPath, []string{requiredValue}) } -// Int64ConditionalRequirement returns a validator that requires an int64 value to be present -// when the field at the specified dependentPath contains one of the allowedValues. +// RequiredIfDependentPathOneOf returns a condition that validates an attribute is required +// when a dependent attribute's value matches one of the specified allowed values. +// +// The condition checks if the dependent attribute (specified by dependentPath) has a value +// that is present in the allowedValues slice. If the dependent attribute matches any of +// the allowed values, then the attribute being validated must not be null or unknown. // // Parameters: -// - dependentPath: The path to the field whose value determines if this field is required -// - allowedValues: A slice of string values that trigger the requirement when found in the dependent field +// - dependentPath: The path to the attribute whose value determines the requirement +// - allowedValues: A slice of string values that trigger the requirement when matched // // Returns: -// - validator.Int64: A validator that enforces the conditional requirement rule +// - condition: A validation condition that enforces the requirement rule +// +// Example usage: +// +// validator := RequiredIfDependentPathOneOf( +// path.Root("type"), +// []string{"custom", "advanced"}, +// ) +// // This would require the current attribute when "type" equals "custom" or "advanced" func RequiredIfDependentPathOneOf(dependentPath path.Path, allowedValues []string) condition { return condition{ dependentPath: dependentPath, @@ -254,19 +279,28 @@ func RequiredIfDependentPathOneOf(dependentPath path.Path, allowedValues []strin } } -// Forbidden validate that the value is NOT set when the condition is met -// ------------------------------------------------------------------------------- - -// ListConditionalForbidden returns a validator that restricts a list attribute from having any values -// when a dependent attribute at the specified path contains one of the allowed values. -// When the dependent path's value matches any of the allowedValues, this validator will -// produce an error if the list attribute being validated is not empty. +// ForbiddenIfDependentPathOneOf creates a validation condition that forbids setting a value +// when a dependent field matches one of the specified allowed values. +// +// This validator is useful for creating mutually exclusive configuration scenarios where +// certain attributes should not be set when another attribute has specific values. // // Parameters: -// - dependentPath: The path to the attribute whose value determines the validation behavior -// - allowedValues: The values that, when present in the dependent attribute, trigger the restriction +// - dependentPath: The path to the field whose value determines the validation behavior +// - allowedValues: A slice of string values that, when matched by the dependent field, +// will trigger the forbidden condition +// +// Returns: +// - condition: A validation condition that will generate an error if the current field +// is set while the dependent field matches any of the allowed values +// +// Example usage: // -// Returns a List validator that enforces the conditional restriction rule. +// validator := ForbiddenIfDependentPathOneOf( +// path.Root("type"), +// []string{"basic", "simple"}, +// ) +// // This will prevent setting the current attribute when "type" equals "basic" or "simple" func ForbiddenIfDependentPathOneOf(dependentPath path.Path, allowedValues []string) condition { return condition{ dependentPath: dependentPath, From a8d9de35e79f8e72e7d3adbeaa4f329ff9a7d110 Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Wed, 22 Oct 2025 15:04:01 -0600 Subject: [PATCH 11/14] Add resource level validation for index / data_view_id --- .../security_detection_rule/acc_test.go | 182 ++++++++++++++++++ .../kibana/security_detection_rule/schema.go | 83 +++++--- 2 files changed, 235 insertions(+), 30 deletions(-) diff --git a/internal/kibana/security_detection_rule/acc_test.go b/internal/kibana/security_detection_rule/acc_test.go index e80c291ce..80d9e53a0 100644 --- a/internal/kibana/security_detection_rule/acc_test.go +++ b/internal/kibana/security_detection_rule/acc_test.go @@ -3,6 +3,7 @@ package security_detection_rule_test import ( "context" "fmt" + "regexp" "strings" "testing" @@ -4740,3 +4741,184 @@ resource "elasticstack_kibana_security_detection_rule" "test" { } `, name) } + +// TestAccResourceSecurityDetectionRule_ValidateConfig tests the ValidateConfig method +// to ensure proper validation of index vs data_view_id configuration +func TestAccResourceSecurityDetectionRule_ValidateConfig(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ProtoV6ProviderFactories: acctest.Providers, + Steps: []resource.TestStep{ + // Test 1: Valid config with only index (should succeed) + { + Config: testAccSecurityDetectionRuleConfig_validationIndexOnly("test-validation-index-only"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "name", "test-validation-index-only"), + resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "index.0", "logs-*"), + resource.TestCheckNoResourceAttr("elasticstack_kibana_security_detection_rule.test", "data_view_id"), + ), + }, + // Test 2: Valid config with only data_view_id (should succeed) + { + Config: testAccSecurityDetectionRuleConfig_validationDataViewOnly("test-validation-dataview-only"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "name", "test-validation-dataview-only"), + resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "data_view_id", "test-data-view-id"), + resource.TestCheckNoResourceAttr("elasticstack_kibana_security_detection_rule.test", "index.0"), + ), + }, + // Test 3: Invalid config with both index and data_view_id (should fail) + { + Config: testAccSecurityDetectionRuleConfig_validationBothIndexAndDataView("test-validation-both"), + ExpectError: regexp.MustCompile("Both 'index' and 'data_view_id' cannot be set at the same time"), + PlanOnly: true, + }, + // Test 4: Invalid config with neither index nor data_view_id (should fail) + { + Config: testAccSecurityDetectionRuleConfig_validationNeither("test-validation-neither"), + ExpectError: regexp.MustCompile("One of 'index' or 'data_view_id' must be set"), + PlanOnly: true, + }, + // Test 5: ESQL rule type should skip validation (both index and data_view_id allowed to be unset) + { + Config: testAccSecurityDetectionRuleConfig_validationESQLType("test-validation-esql"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "name", "test-validation-esql"), + resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "type", "esql"), + resource.TestCheckNoResourceAttr("elasticstack_kibana_security_detection_rule.test", "index.0"), + resource.TestCheckNoResourceAttr("elasticstack_kibana_security_detection_rule.test", "data_view_id"), + ), + }, + // Test 6: Machine learning rule type should skip validation (both index and data_view_id allowed to be unset) + { + Config: testAccSecurityDetectionRuleConfig_validationMLType("test-validation-ml"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "name", "test-validation-ml"), + resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "type", "machine_learning"), + resource.TestCheckNoResourceAttr("elasticstack_kibana_security_detection_rule.test", "index.0"), + resource.TestCheckNoResourceAttr("elasticstack_kibana_security_detection_rule.test", "data_view_id"), + ), + }, + }, + }) +} + +// Helper function configurations for validation tests + +func testAccSecurityDetectionRuleConfig_validationIndexOnly(name string) string { + return fmt.Sprintf(` +provider "elasticstack" { + kibana {} +} + +resource "elasticstack_kibana_security_detection_rule" "test" { + name = "%s" + type = "query" + query = "*:*" + language = "kuery" + enabled = true + description = "Test validation with index only" + severity = "medium" + risk_score = 50 + index = ["logs-*"] +} +`, name) +} + +func testAccSecurityDetectionRuleConfig_validationDataViewOnly(name string) string { + return fmt.Sprintf(` +provider "elasticstack" { + kibana {} +} + +resource "elasticstack_kibana_security_detection_rule" "test" { + name = "%s" + type = "query" + query = "*:*" + language = "kuery" + enabled = true + description = "Test validation with data_view_id only" + severity = "medium" + risk_score = 50 + data_view_id = "test-data-view-id" +} +`, name) +} + +func testAccSecurityDetectionRuleConfig_validationBothIndexAndDataView(name string) string { + return fmt.Sprintf(` +provider "elasticstack" { + kibana {} +} + +resource "elasticstack_kibana_security_detection_rule" "test" { + name = "%s" + type = "query" + query = "*:*" + language = "kuery" + enabled = true + description = "Test validation with both index and data_view_id (should fail)" + severity = "medium" + risk_score = 50 + index = ["logs-*"] + data_view_id = "test-data-view-id" +} +`, name) +} + +func testAccSecurityDetectionRuleConfig_validationNeither(name string) string { + return fmt.Sprintf(` +provider "elasticstack" { + kibana {} +} + +resource "elasticstack_kibana_security_detection_rule" "test" { + name = "%s" + type = "query" + query = "*:*" + language = "kuery" + enabled = true + description = "Test validation with neither index nor data_view_id (should fail)" + severity = "medium" + risk_score = 50 +} +`, name) +} + +func testAccSecurityDetectionRuleConfig_validationESQLType(name string) string { + return fmt.Sprintf(` +provider "elasticstack" { + kibana {} +} + +resource "elasticstack_kibana_security_detection_rule" "test" { + name = "%s" + type = "esql" + query = "FROM logs-* | WHERE event.action == \"login\" | STATS count(*) BY user.name" + language = "esql" + enabled = true + description = "Test ESQL validation bypass - neither index nor data_view_id required" + severity = "medium" + risk_score = 50 +} +`, name) +} + +func testAccSecurityDetectionRuleConfig_validationMLType(name string) string { + return fmt.Sprintf(` +provider "elasticstack" { + kibana {} +} + +resource "elasticstack_kibana_security_detection_rule" "test" { + name = "%s" + type = "machine_learning" + enabled = true + description = "Test ML validation bypass - neither index nor data_view_id required" + severity = "medium" + risk_score = 50 + anomaly_threshold = 75 + machine_learning_job_id = ["test-ml-job"] +} +`, name) +} diff --git a/internal/kibana/security_detection_rule/schema.go b/internal/kibana/security_detection_rule/schema.go index 7f0cd41ec..688c52300 100644 --- a/internal/kibana/security_detection_rule/schema.go +++ b/internal/kibana/security_detection_rule/schema.go @@ -4,11 +4,11 @@ import ( "context" "regexp" + "github.com/elastic/terraform-provider-elasticstack/internal/utils" "github.com/elastic/terraform-provider-elasticstack/internal/utils/customtypes" "github.com/elastic/terraform-provider-elasticstack/internal/utils/validators" "github.com/hashicorp/terraform-plugin-framework-jsontypes/jsontypes" "github.com/hashicorp/terraform-plugin-framework-validators/int64validator" - "github.com/hashicorp/terraform-plugin-framework-validators/listvalidator" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/path" @@ -77,21 +77,10 @@ func GetSchema() schema.Schema { MarkdownDescription: "Data view ID for the rule. Not supported for esql and machine_learning rule types.", Optional: true, Validators: []validator.String{ - stringvalidator.All( - // Enforce that one of index or data_view_id are set if the rule type is not ml or esql - stringvalidator.Any( - stringvalidator.ExactlyOneOf(path.MatchRoot("index"), path.MatchRoot("data_view_id")), - validators.DependantPathOneOf( - path.Root("type"), - []string{"machine_learning", "esql"}, - ), - ), - - // Enforce that index is not set if the rule type is ml or esql - validators.ForbiddenIfDependentPathOneOf( - path.Root("type"), - []string{"machine_learning", "esql"}, - ), + // Enforce that data_view_id is not set if the rule type is ml or esql + validators.ForbiddenIfDependentPathOneOf( + path.Root("type"), + []string{"machine_learning", "esql"}, ), }, }, @@ -130,20 +119,10 @@ func GetSchema() schema.Schema { Optional: true, Computed: true, Validators: []validator.List{ - listvalidator.All( - // Enforce that one of index or data_view_id are set if the rule type is not ml or esql - listvalidator.Any( - listvalidator.ExactlyOneOf(path.MatchRoot("index"), path.MatchRoot("data_view_id")), - validators.DependantPathOneOf( - path.Root("type"), - []string{"machine_learning", "esql"}, - ), - ), - // Enforce that index is not set if the rule type is ml or esql - validators.ForbiddenIfDependentPathOneOf( - path.Root("type"), - []string{"machine_learning", "esql"}, - ), + // Enforce that index is not set if the rule type is ml or esql + validators.ForbiddenIfDependentPathOneOf( + path.Root("type"), + []string{"machine_learning", "esql"}, ), }, }, @@ -961,3 +940,47 @@ func getThreatSubtechniqueElementType() attr.Type { techniqueType := threatType.AttributeTypes()["technique"].(attr.TypeWithElementType).ElementType().(attr.TypeWithAttributeTypes) return techniqueType.AttributeTypes()["subtechnique"].(attr.TypeWithElementType).ElementType() } + +// ValidateConfig validates the configuration for a security detection rule resource. +// It ensures that the configuration meets the following requirements: +// +// - For rule types "esql" and "machine_learning", no additional validation is performed +// - For other rule types, exactly one of 'index' or 'data_view_id' must be specified +// - Both 'index' and 'data_view_id' cannot be set simultaneously +// +// The function adds appropriate error diagnostics if validation fails. +func (r securityDetectionRuleResource) ValidateConfig(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { + var data SecurityDetectionRuleData + + resp.Diagnostics.Append(req.Config.Get(ctx, &data)...) + + if resp.Diagnostics.HasError() { + return + } + + t, diag := data.Type.ToStringValue(ctx) + resp.Diagnostics.Append(diag...) + if resp.Diagnostics.HasError() { + return + } + + if t.ValueString() == "esql" || t.ValueString() == "machine_learning" { + return + } + + if utils.IsKnown(data.Index) && utils.IsKnown(data.DataViewId) { + resp.Diagnostics.AddError( + "Invalid Configuration", + "Both 'index' and 'data_view_id' cannot be set at the same time.", + ) + + } + + if !utils.IsKnown(data.Index) && !utils.IsKnown(data.DataViewId) { + resp.Diagnostics.AddError( + "Invalid Configuration", + "One of 'index' or 'data_view_id' must be set.", + ) + + } +} From d41b56440e18a9d17962adbed548fc37bc457ea0 Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Wed, 22 Oct 2025 15:13:55 -0600 Subject: [PATCH 12/14] Skip for irrelevant versions --- .../kibana/security_detection_rule/acc_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/kibana/security_detection_rule/acc_test.go b/internal/kibana/security_detection_rule/acc_test.go index 80d9e53a0..546fb8726 100644 --- a/internal/kibana/security_detection_rule/acc_test.go +++ b/internal/kibana/security_detection_rule/acc_test.go @@ -4751,7 +4751,8 @@ func TestAccResourceSecurityDetectionRule_ValidateConfig(t *testing.T) { Steps: []resource.TestStep{ // Test 1: Valid config with only index (should succeed) { - Config: testAccSecurityDetectionRuleConfig_validationIndexOnly("test-validation-index-only"), + Config: testAccSecurityDetectionRuleConfig_validationIndexOnly("test-validation-index-only"), + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionSupport), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "name", "test-validation-index-only"), resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "index.0", "logs-*"), @@ -4760,7 +4761,8 @@ func TestAccResourceSecurityDetectionRule_ValidateConfig(t *testing.T) { }, // Test 2: Valid config with only data_view_id (should succeed) { - Config: testAccSecurityDetectionRuleConfig_validationDataViewOnly("test-validation-dataview-only"), + Config: testAccSecurityDetectionRuleConfig_validationDataViewOnly("test-validation-dataview-only"), + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionSupport), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "name", "test-validation-dataview-only"), resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "data_view_id", "test-data-view-id"), @@ -4770,18 +4772,21 @@ func TestAccResourceSecurityDetectionRule_ValidateConfig(t *testing.T) { // Test 3: Invalid config with both index and data_view_id (should fail) { Config: testAccSecurityDetectionRuleConfig_validationBothIndexAndDataView("test-validation-both"), + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionSupport), ExpectError: regexp.MustCompile("Both 'index' and 'data_view_id' cannot be set at the same time"), PlanOnly: true, }, // Test 4: Invalid config with neither index nor data_view_id (should fail) { Config: testAccSecurityDetectionRuleConfig_validationNeither("test-validation-neither"), + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionSupport), ExpectError: regexp.MustCompile("One of 'index' or 'data_view_id' must be set"), PlanOnly: true, }, // Test 5: ESQL rule type should skip validation (both index and data_view_id allowed to be unset) { - Config: testAccSecurityDetectionRuleConfig_validationESQLType("test-validation-esql"), + Config: testAccSecurityDetectionRuleConfig_validationESQLType("test-validation-esql"), + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionSupport), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "name", "test-validation-esql"), resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "type", "esql"), @@ -4791,7 +4796,8 @@ func TestAccResourceSecurityDetectionRule_ValidateConfig(t *testing.T) { }, // Test 6: Machine learning rule type should skip validation (both index and data_view_id allowed to be unset) { - Config: testAccSecurityDetectionRuleConfig_validationMLType("test-validation-ml"), + Config: testAccSecurityDetectionRuleConfig_validationMLType("test-validation-ml"), + SkipFunc: versionutils.CheckIfVersionIsUnsupported(minVersionSupport), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "name", "test-validation-ml"), resource.TestCheckResourceAttr("elasticstack_kibana_security_detection_rule.test", "type", "machine_learning"), From 73d5cef2a75410654b617abc797126c52aa706a9 Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Wed, 22 Oct 2025 15:19:51 -0600 Subject: [PATCH 13/14] Remove outdated comment --- internal/utils/validators/conditional.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/utils/validators/conditional.go b/internal/utils/validators/conditional.go index ec3e0362c..df011533c 100644 --- a/internal/utils/validators/conditional.go +++ b/internal/utils/validators/conditional.go @@ -57,8 +57,6 @@ func (v condition) dependentFieldHasAllowedValue(ctx context.Context, config tfs return false, "", diags } - // If dependent value is null, unknown, or doesn't match any allowed values, - // then the condition is not met dependentValueStr := dependentValue.ValueString() dependentFieldHasAllowedValue := false From b2cce8e963b9ee0a1f6e4c127acca5958660e834 Mon Sep 17 00:00:00 2001 From: Nick Benoit Date: Wed, 22 Oct 2025 19:52:55 -0600 Subject: [PATCH 14/14] Simplify rule type check --- internal/kibana/security_detection_rule/schema.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/internal/kibana/security_detection_rule/schema.go b/internal/kibana/security_detection_rule/schema.go index 688c52300..0dab25047 100644 --- a/internal/kibana/security_detection_rule/schema.go +++ b/internal/kibana/security_detection_rule/schema.go @@ -958,13 +958,7 @@ func (r securityDetectionRuleResource) ValidateConfig(ctx context.Context, req r return } - t, diag := data.Type.ToStringValue(ctx) - resp.Diagnostics.Append(diag...) - if resp.Diagnostics.HasError() { - return - } - - if t.ValueString() == "esql" || t.ValueString() == "machine_learning" { + if data.Type.ValueString() == "esql" || data.Type.ValueString() == "machine_learning" { return }