-
Notifications
You must be signed in to change notification settings - Fork 123
Add Security List Data Stream Resource #1525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a new resource elasticstack_kibana_security_list_data_streams to manage the lifecycle of .lists and .items data streams in Kibana, which are required before using security lists and exceptions that reference value lists. The implementation includes comprehensive test coverage for different list types (ip, keyword, ip_range) within security exception item entries.
Key Changes
- Added new
security_list_data_streamsresource for managing list data streams lifecycle - Enhanced test coverage for security exception items with list entry types
- Refactored acceptance tests to use the new resource instead of manual setup functions
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
provider/plugin_framework.go |
Registered the new security_list_data_streams resource |
internal/kibana/security_list_data_streams/*.go |
Implemented CRUD operations for the new resource |
internal/kibana/security_list_data_streams/testdata/**/*.tf |
Added test configurations for the new resource |
internal/kibana/security_list_item/acc_test.go |
Removed manual setup function in favor of resource-based approach |
internal/kibana/security_list_item/testdata/**/*.tf |
Updated test configs to use new data streams resource |
internal/kibana/security_list/acc_test.go |
Removed manual setup function in favor of resource-based approach |
internal/kibana/security_list/testdata/**/*.tf |
Updated test configs to use new data streams resource |
internal/kibana/security_exception_item/acc_test.go |
Enhanced test coverage for list entry types |
internal/kibana/security_exception_item/testdata/**/*.tf |
Added test configs for ip, keyword, and ip_range list types |
internal/kibana/security_exception_item/schema.go |
Added validation for list entries and list types |
internal/clients/kibana_oapi/security_lists.go |
Enhanced API functions with proper return values |
examples/resources/elasticstack_kibana_security_list_data_streams/resource.tf |
Added example usage |
docs/resources/elasticsearch_security_user.md |
Updated documentation formatting |
internal/kibana/security_list_item/testdata/TestAccResourceSecurityListItem/update/main.tf
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 3 comments.
internal/kibana/security_list_data_streams/resource-description.md
Outdated
Show resolved
Hide resolved
...ty_exception_item/testdata/TestAccResourceExceptionItemEntryType_List/list/exception_item.tf
Show resolved
Hide resolved
dcd89e2 to
a13f1fe
Compare
tobio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of small comments
| return | ||
| } | ||
|
|
||
| if !listIndex || !listItemIndex { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if !listIndex || !listItemIndex { | |
| if !listIndex && !listItemIndex { |
Should we only completely remove it if they're both missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we may want OR. If only one of them is false I don't think we would ever call the create API which I think is how we would get back to the desired end state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So another approach would be to have list_index and list_item_index set to true during planning (IIRC a default would work, otherwise via a plan modifier), which would then trigger an update if either of them were false in state. Which might be more informative for users, or give us more flexibility in the future.
I'm not fussed though, this approach works and we can adjust it if required.
internal/kibana/security_list_data_streams/resource-description.md
Outdated
Show resolved
Hide resolved
* origin/main: Fixup changelog Bump release disk size Prepare 0.13.0 release (#1532) Fix null value handling in Kibana connector config causing inconsistent apply state (#1524) Add Security List Data Stream Resource (#1525) chore(deps): update golang:1.25.5 docker digest to 0ece421 (#1531) Fleet agent policy host name format field (#1521) chore(deps): update kibana-openapi-spec digest to 6647f81 (#1528) chore(deps): update kibana-openapi-spec digest to bd3d07c (#1519)
Changes inspired by using all security exception adjacent resources together
elasticstack_kibana_security_list_data_streams) to manage lifecycle of create list data streamsRel: #1332