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

Added ability to specify rules directory #816

Merged
merged 9 commits into from
Apr 19, 2022
Merged

Added ability to specify rules directory #816

merged 9 commits into from
Apr 19, 2022

Conversation

SBe
Copy link

@SBe SBe commented Apr 19, 2022

Description

Added ability to specify rules directory with defaulting to the old value.
This functionality makes it easier to customize template and skip creating of secret with custom Elastalert configuration.

Checklist

  • I have reviewed the contributing guidelines.
  • I have included unit tests for my changes or additions.
  • I have successfully run make test-docker with my changes.
  • I have manually tested all relevant modes of the change in this PR.
  • I have updated the documentation.
  • I have updated the changelog.

Questions or Comments

I have not added additional unit tests, due to the fact that there are no helm chart tests implemented in elastalert.
I did not add the documentation at elastaler2.read because there are no functional changes in this PR.

CHANGELOG.md Outdated Show resolved Hide resolved
@jertel
Copy link
Owner

jertel commented Apr 19, 2022

It would be helpful to also edit the README.md chart documentation to explain what scenarios would require this new rulesDirectory setting to be customized.

@@ -56,6 +56,7 @@ The command removes all the Kubernetes components associated with the chart and
| `command` | command override for container | `NULL` |
| `args` | args override for container | `NULL` |
| `replicaCount` | number of replicas to run | 1 |
| `rulesFolder` | Locaton of rules directory. Usefull when you have one docker image and different set on rules per environemnt. For example development can reside in `/opt/elastalert/develop` and production in `/opt/elastalert/production`. | /opt/elastalert/rules |
Copy link
Owner

Choose a reason for hiding this comment

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

Can you help me understand this a bit better, in case others ask about it in the future? Typically when I am deploying a different set of rules to alternate environments my override yaml file defines the rules specific to each environment. The jertel/elastalert2 Docker image remains the same across all environments. How does changing this directory name make a difference?

Copy link
Author

Choose a reason for hiding this comment

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

Of course.
As far as I know there are few ways to have this implemented right now.
We can (for example) define our own secret and mount it in the Pod.

But what we wanted to have is to have as little copy-pasting and being able to new rules locally with docker-compose.
So, we have prepared set of yml files, which contain rules definitions. They are different per environment.
During local development, devs just add files to appropriate directory and run docker-compose.
image

Our docker image just takes all those rules (both lab and prod) and mounts them in container (both set of rules are present in the container). And now I would like to be able, to tell ElastAlert that I want to use rules from lets say lab subdirectory.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for that detailed explanation, and thanks for the contribution!

@jertel jertel merged commit ec53a91 into jertel:master Apr 19, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants