Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 8, 2025

PR Description: Implement Kibana Security Detection Rule Resource

Overview

This PR implements the elasticstack_kibana_security_detection_rule resource, addressing issue #523. This adds support for managing Kibana security detection rules through Terraform, enabling users to create, update, and manage various types of security rules including query, EQL, ESQL, machine learning, new terms, saved query, threat match, and threshold rules.

See examples/resources/elasticstack_kibana_security_detection_rule/resource.tf as a usage example. More widespread examples for each rule type can be found in the acceptance tests (security_detection_rule/acc_test.go)

File Structure

internal/kibana/security_detection_rule/
├── create.go                                          # Resource creation logic
├── delete.go                                          # Resource deletion logic  
├── read.go                                             # Resource read logic
├── update.go                                         # Resource update logic
├── resource.go                                      # Resource interface implementation
├── schema.go                                        # Terraform schema definition
├── models.go                                         # Terraform data models
├── rule_processor.go                            # Rule type processor interface - A rule type processor is implemented for each rule type
├── models_<rule_tyoe>.go                  # Rule type specific handling
├── models_to_api_type_utils.go         # Utilities to convert Terraform Types -> Api Models
├── models_from_api_type_utils.go    # Utilities to convert  Api Modles -> Terraform Types

Notes

While all rule types are currently supported by this resource there are some dependent resources that are not yet supported. Notably:

Support for both of should be included in the next milestone release however.

Copy link

cla-checker-service bot commented Sep 8, 2025

💚 CLA has been signed

Copilot AI and others added 2 commits September 8, 2025 22:23
Co-authored-by: nick-benoit <163016768+nick-benoit@users.noreply.github.com>
…examples

Co-authored-by: nick-benoit <163016768+nick-benoit@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Add Support for Kibana Security Detection rule Add Support for Kibana Security Detection rule Sep 8, 2025
@Copilot Copilot AI requested a review from nick-benoit September 8, 2025 22:33
Copilot finished work on behalf of nick-benoit September 8, 2025 22:33
@nick-benoit
Copy link

The trickiest part of this PR is sorting out rule_id vs id in the update case: https://www.elastic.co/docs/api/doc/kibana/operation/operation-updaterule

Update a detection rule using the rule_id or id field. The original rule is replaced, and all unspecified fields are deleted.

The api gives back this error if both are included:

│ API returned status 400: {"message":["both \"id\" and \"rule_id\" cannot exist, choose one or the other"],"status_code":400}```

If provided we will use the `rule_id` and unset the `id`. Otherwise we use the `id`.

@nick-benoit nick-benoit marked this pull request as ready for review September 9, 2025 21:23
Copy link
Member

@tobio tobio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty excellent first pass here, added a bunch of comments around using some shared utils and how we've generally tried to structure things but this is looking good.

IMO we should cover all the rule types in this PR, but I could be convinced they can be added in a follow up. We do need to cover them to consider the issue 'done' though.

@tobio
Copy link
Member

tobio commented Sep 10, 2025

Looking at the acceptance test failures:

The 7.17 test is failing because an expected index doesn't exist yet. I guess it wouldn't be hard to create another 7.x specific test which also first created the index, but I'm not sure it's worthwhile given it's 7.x.

The 8.10 test looks like we're getting a different response from Kibana in that version. Definitely worth investigating, but given 8.10 is 2 years old I don't think it's unreasonable to skip the test in 8.10 and mention in the docs that the resource isn't compatible with 8.10.

…feedback

Co-authored-by: nick-benoit <163016768+nick-benoit@users.noreply.github.com>
Copilot finished work on behalf of nick-benoit September 10, 2025 18:07
@nick-benoit
Copy link

@copilot Please support all detection rule types defined in https://www.elastic.co/docs/api/doc/kibana/group/endpoint-security-detections-api

This will require adding updating the schema in internal/kibana/security_detection_rule/schema.go and corresponding methods in internal/kibana/security_detection_rule/models.go (eg toCreateProps, etc). Additionally instead of always using SecurityDetectionsAPIQueryRuleCreateProps to create and SecurityDetectionsAPIQueryRuleUpdateProps to update we will need choose the appropriate query rule type based on the type of rules we are using.

Copy link
Contributor Author

Copilot AI commented Sep 10, 2025

@copilot Please support all detection rule types defined in https://www.elastic.co/docs/api/doc/kibana/group/endpoint-security-detections-api

This will require adding updating the schema in `intern...

I'm implementing support for all detection rule types. I've updated the schema to support all 8 rule types (query, eql, esql, machine_learning, new_terms, saved_query, threat_match, threshold) with their specific fields, and I'm working on updating the models to handle creation and updates for each type. This is a significant change that requires updating the create, update, and read logic for all rule types. I'll commit the progress once the implementation is complete and tested.

Co-authored-by: nick-benoit <163016768+nick-benoit@users.noreply.github.com>
tobio
tobio previously approved these changes Oct 2, 2025
Copy link
Member

@tobio tobio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one minor nit. Happy to get those addressed in a follow up PR though if you wanted to get this merged/released.

}

// Helper function to update data view ID from API response
func (d *SecurityDetectionRuleData) updateDataViewIdFromApi(ctx context.Context, dataViewId *kbapi.SecurityDetectionsAPIDataViewId) diag.Diagnostics {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following few functions return diags that will always be empty. We should likely just remove the return value altogether.

Copy link
Member

@tobio tobio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM post meta removal

@nick-benoit nick-benoit merged commit 1ae6c3e into main Oct 6, 2025
54 checks passed
@nick-benoit nick-benoit deleted the copilot/fix-1290-2 branch October 6, 2025 15:01
tobio added a commit to orangecola/terraform-provider-elasticstack that referenced this pull request Oct 7, 2025
* origin/main:
  Add Support for Multiple Kibana Security Detection Rule Types (elastic#1292)
  chore(deps): update module github.com/golangci/golangci-lint to v2.5.0 (elastic#1348)
  chore(deps): update golang:1.25.1 docker digest to d709837 (elastic#1347)
  fix(deps): update module github.com/hashicorp/terraform-plugin-mux to v0.21.0 (elastic#1349)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants