Skip to content
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

Add kibana/security-rule asset type #142

Merged
merged 10 commits into from
Mar 18, 2021
Merged

Add kibana/security-rule asset type #142

merged 10 commits into from
Mar 18, 2021

Conversation

rw-access
Copy link
Contributor

@rw-access rw-access commented Feb 25, 2021

cc @FrankHassanabad, @spong, @brokensound77, @kevinlog

What does this PR do?

  • Add a security-rule asset type for rules that will be delivered to Security » Detection Engine » Detection rules part of Kibana.

Why is it important?

This is a dependency for:

Checklist

Note: I haven't added a test package yet, because I'm expecting requested changes. Once we converge on the spec, I think it makes sense to add a test package.

Related issues

No related public issues, just these PRs

@elasticmachine
Copy link

elasticmachine commented Feb 25, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #142 updated

  • Start Time: 2021-03-17T18:18:26.356+0000

  • Duration: 9 min 31 sec

  • Commit: ba23706

Trends 🧪

Image of Build Times

@mtojek mtojek self-requested a review March 1, 2021 09:03
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

Please make sure that the PR passes the CI verification. Quick check:

make update && make check && make test

@@ -65,3 +65,25 @@
type: file
contentMediaType: "application/json"
pattern: '^.+\.json$'
- description: Folder containing rules
Copy link
Contributor

Choose a reason for hiding this comment

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

Please create a sample package and add it to the test/packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this asset to test/packages/good

Copy link
Contributor Author

@rw-access rw-access Mar 17, 2021

Choose a reason for hiding this comment

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

Unless this should be its own test package

versions/1/kibana/spec.yml Outdated Show resolved Hide resolved
versions/1/manifest.spec.yml Outdated Show resolved Hide resolved
versions/1/kibana/spec.yml Outdated Show resolved Hide resolved
@ruflin
Copy link
Member

ruflin commented Mar 1, 2021

@rw-access Are rules and timeline tied together? Could a user just ship a rule or just a timeline?

versions/1/manifest.spec.yml Outdated Show resolved Hide resolved
versions/1/kibana/spec.yml Outdated Show resolved Hide resolved
versions/1/changelog.yml Outdated Show resolved Hide resolved
@rw-access rw-access changed the title Add kibana/rules for detection rules as JSON files Add kibana/security-rule asset type Mar 16, 2021
@ruflin
Copy link
Member

ruflin commented Mar 17, 2021

@rw-access Could you add a sample package for testing this?

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

For later: Should we do on the package side an validation of the rules file?

@ycombinator do we apply some automatic formatting to it?

- description: An individual rule file for the detection engine
type: file
contentMediaType: "application/json"
pattern: '^rule-.+\.json$'
Copy link
Member

Choose a reason for hiding this comment

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

Is the rule prefix in the name required?

Copy link
Contributor Author

@rw-access rw-access Mar 17, 2021

Choose a reason for hiding this comment

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

no, I can loosen this to just *.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just ^.+\.json$ as of ba23706

@ycombinator
Copy link
Contributor

For later: Should we do on the package side an validation of the rules file?

The package spec does allow for defining the structure of file contents but we have deliberately not specified structures for other Kibana saved objects like dashboards, etc. because we're treating those as Kibana implementation details (at least for the moment). The only basic validation is that the files match a certain mime type and are (potentially) named a certain way, both of which are being done in this PR.

@ycombinator do we apply some automatic formatting to it?

Automatic formatting to the JSON files? Yes, elastic-package format will take care of it.

Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

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.

None yet

5 participants