Refactor security detection rule implementation based on code review feedback

Co-authored-by: nick-benoit <[email protected]>

Author: Copilot

internal/kibana/security_detection_rule/create.go

@@ -4,9 +4,9 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/elastic/terraform-provider-elasticstack/generated/kbapi"
-	"github.com/hashicorp/terraform-plugin-framework/diag"
+	"github.com/elastic/terraform-provider-elasticstack/internal/clients"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
+	"github.com/hashicorp/terraform-plugin-framework/types"
 )
 
 func (r *securityDetectionRuleResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
@@ -28,7 +28,7 @@ func (r *securityDetectionRuleResource) Create(ctx context.Context, req resource
 	}
 
 	// Build the create request
-	createProps, diags := r.buildCreateProps(ctx, data)
+	createProps, diags := data.toCreateProps(ctx)
 	resp.Diagnostics.Append(diags...)
 	if resp.Diagnostics.HasError() {
 		return
@@ -52,187 +52,32 @@ func (r *securityDetectionRuleResource) Create(ctx context.Context, req resource
 		return
 	}
 
-	// Parse the response
+	// Parse the response to get the ID, then use Read logic for consistency
 	ruleResponse, diags := r.parseRuleResponse(ctx, response.JSON200)
 	resp.Diagnostics.Append(diags...)
 	if resp.Diagnostics.HasError() {
 		return
 	}
 
-	// Update the data with response values
-	diags = r.updateDataFromRule(ctx, &data, ruleResponse)
-	resp.Diagnostics.Append(diags...)
-	if resp.Diagnostics.HasError() {
-		return
-	}
-
-	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
-}
-
-func (r *securityDetectionRuleResource) buildCreateProps(ctx context.Context, data SecurityDetectionRuleData) (kbapi.SecurityDetectionsAPIRuleCreateProps, diag.Diagnostics) {
-	var diags diag.Diagnostics
-	var createProps kbapi.SecurityDetectionsAPIRuleCreateProps
-
-	queryRuleQuery := kbapi.SecurityDetectionsAPIRuleQuery(data.Query.ValueString())
-	// Convert data to QueryRuleCreateProps since we're only supporting query rules initially
-	queryRule := kbapi.SecurityDetectionsAPIQueryRuleCreateProps{
-		Name:        kbapi.SecurityDetectionsAPIRuleName(data.Name.ValueString()),
-		Description: kbapi.SecurityDetectionsAPIRuleDescription(data.Description.ValueString()),
-		Type:        kbapi.SecurityDetectionsAPIQueryRuleCreatePropsType("query"),
-		Query:       &queryRuleQuery,
-		RiskScore:   kbapi.SecurityDetectionsAPIRiskScore(data.RiskScore.ValueInt64()),
-		Severity:    kbapi.SecurityDetectionsAPISeverity(data.Severity.ValueString()),
-	}
-
-	// Set optional rule_id if provided
-	if !data.RuleId.IsNull() && !data.RuleId.IsUnknown() {
-		ruleId := kbapi.SecurityDetectionsAPIRuleSignatureId(data.RuleId.ValueString())
-		queryRule.RuleId = &ruleId
-	}
-
-	// Set enabled status
-	if !data.Enabled.IsNull() {
-		enabled := kbapi.SecurityDetectionsAPIIsRuleEnabled(data.Enabled.ValueBool())
-		queryRule.Enabled = &enabled
-	}
-
-	// Set query language
-	if !data.Language.IsNull() {
-		var language kbapi.SecurityDetectionsAPIKqlQueryLanguage
-		switch data.Language.ValueString() {
-		case "kuery":
-			language = "kuery"
-		case "lucene":
-			language = "lucene"
-		default:
-			language = "kuery"
-		}
-		queryRule.Language = &language
-	}
-
-	// Set time range
-	if !data.From.IsNull() {
-		from := kbapi.SecurityDetectionsAPIRuleIntervalFrom(data.From.ValueString())
-		queryRule.From = &from
-	}
-
-	if !data.To.IsNull() {
-		to := kbapi.SecurityDetectionsAPIRuleIntervalTo(data.To.ValueString())
-		queryRule.To = &to
-	}
-
-	// Set interval
-	if !data.Interval.IsNull() {
-		interval := kbapi.SecurityDetectionsAPIRuleInterval(data.Interval.ValueString())
-		queryRule.Interval = &interval
-	}
-
-	// Set index patterns
-	if !data.Index.IsNull() && !data.Index.IsUnknown() {
-		var indexList []string
-		diags.Append(data.Index.ElementsAs(ctx, &indexList, false)...)
-		if !diags.HasError() && len(indexList) > 0 {
-			indexPatterns := make(kbapi.SecurityDetectionsAPIIndexPatternArray, len(indexList))
-			//nolint:staticcheck // Type conversion required, can't use copy()
-			for i, idx := range indexList {
-				indexPatterns[i] = idx
-			}
-			queryRule.Index = &indexPatterns
-		}
-	}
-
-	// Set author
-	if !data.Author.IsNull() && !data.Author.IsUnknown() {
-		var authorList []string
-		diags.Append(data.Author.ElementsAs(ctx, &authorList, false)...)
-		if !diags.HasError() && len(authorList) > 0 {
-			authorArray := make(kbapi.SecurityDetectionsAPIRuleAuthorArray, len(authorList))
-			//nolint:staticcheck // Type conversion required, can't use copy()
-			for i, author := range authorList {
-				authorArray[i] = author
-			}
-			queryRule.Author = &authorArray
-		}
-	}
-
-	// Set tags
-	if !data.Tags.IsNull() && !data.Tags.IsUnknown() {
-		var tagsList []string
-		diags.Append(data.Tags.ElementsAs(ctx, &tagsList, false)...)
-		if !diags.HasError() && len(tagsList) > 0 {
-			tagsArray := make(kbapi.SecurityDetectionsAPIRuleTagArray, len(tagsList))
-			//nolint:staticcheck // Type conversion required, can't use copy()
-			for i, tag := range tagsList {
-				tagsArray[i] = tag
-			}
-			queryRule.Tags = &tagsArray
-		}
-	}
-
-	// Set false positives
-	if !data.FalsePositives.IsNull() && !data.FalsePositives.IsUnknown() {
-		var fpList []string
-		diags.Append(data.FalsePositives.ElementsAs(ctx, &fpList, false)...)
-		if !diags.HasError() && len(fpList) > 0 {
-			fpArray := make(kbapi.SecurityDetectionsAPIRuleFalsePositiveArray, len(fpList))
-			//nolint:staticcheck // Type conversion required, can't use copy()
-			for i, fp := range fpList {
-				fpArray[i] = fp
-			}
-			queryRule.FalsePositives = &fpArray
-		}
-	}
-
-	// Set references
-	if !data.References.IsNull() && !data.References.IsUnknown() {
-		var refList []string
-		diags.Append(data.References.ElementsAs(ctx, &refList, false)...)
-		if !diags.HasError() && len(refList) > 0 {
-			refArray := make(kbapi.SecurityDetectionsAPIRuleReferenceArray, len(refList))
-			//nolint:staticcheck // Type conversion required, can't use copy()
-			for i, ref := range refList {
-				refArray[i] = ref
-			}
-			queryRule.References = &refArray
-		}
-	}
-
-	// Set optional string fields
-	if !data.License.IsNull() {
-		license := kbapi.SecurityDetectionsAPIRuleLicense(data.License.ValueString())
-		queryRule.License = &license
+	// Set the ID based on the created rule
+	compId := clients.CompositeId{
+		ClusterId:  data.SpaceId.ValueString(),
+		ResourceId: ruleResponse.Id.String(),
 	}
+	data.Id = types.StringValue(compId.String())
 
-	if !data.Note.IsNull() {
-		note := kbapi.SecurityDetectionsAPIInvestigationGuide(data.Note.ValueString())
-		queryRule.Note = &note
+	// Use Read logic to populate the state with fresh data from the API
+	readReq := resource.ReadRequest{
+		State: resp.State,
 	}
+	var readResp resource.ReadResponse
+	readReq.State.Set(ctx, &data)
+	r.Read(ctx, readReq, &readResp)
 
-	if !data.Setup.IsNull() {
-		setup := kbapi.SecurityDetectionsAPISetupGuide(data.Setup.ValueString())
-		queryRule.Setup = &setup
-	}
-
-	// Set max signals
-	if !data.MaxSignals.IsNull() {
-		maxSignals := kbapi.SecurityDetectionsAPIMaxSignals(data.MaxSignals.ValueInt64())
-		queryRule.MaxSignals = &maxSignals
-	}
-
-	// Set version
-	if !data.Version.IsNull() {
-		version := kbapi.SecurityDetectionsAPIRuleVersion(data.Version.ValueInt64())
-		queryRule.Version = &version
-	}
-
-	// Convert to union type
-	err := createProps.FromSecurityDetectionsAPIQueryRuleCreateProps(queryRule)
-	if err != nil {
-		diags.AddError(
-			"Error building create properties",
-			"Could not convert rule properties: "+err.Error(),
-		)
+	resp.Diagnostics.Append(readResp.Diagnostics...)
+	if resp.Diagnostics.HasError() {
+		return
 	}
 
-	return createProps, diags
+	resp.State = readResp.State
 }

internal/kibana/security_detection_rule/delete.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/elastic/terraform-provider-elasticstack/generated/kbapi"
+	"github.com/elastic/terraform-provider-elasticstack/internal/clients"
 	"github.com/google/uuid"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 )
@@ -18,7 +19,7 @@ func (r *securityDetectionRuleResource) Delete(ctx context.Context, req resource
 	}
 
 	// Parse ID to get space_id and rule_id
-	_, ruleId, diags := r.parseResourceId(data.Id.ValueString())
+	compId, diags := clients.CompositeIdFromStrFw(data.Id.ValueString())
 	resp.Diagnostics.Append(diags...)
 	if resp.Diagnostics.HasError() {
 		return
@@ -35,7 +36,12 @@ func (r *securityDetectionRuleResource) Delete(ctx context.Context, req resource
 	}
 
 	// Delete the rule
-	ruleObjectId := kbapi.SecurityDetectionsAPIRuleObjectId(uuid.MustParse(ruleId))
+	uid, err := uuid.Parse(compId.ResourceId)
+	if err != nil {
+		resp.Diagnostics.AddError("ID was not a valid UUID", err.Error())
+		return
+	}
+	ruleObjectId := kbapi.SecurityDetectionsAPIRuleObjectId(uid)
 	params := &kbapi.DeleteRuleParams{
 		Id: &ruleObjectId,
 	}