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

allow to disable/enable ids logs #98

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

GrigoriyMikhalkin
Copy link
Contributor

Changes to allow enabling/disabling IDS logs via firewall resource. By default logs are disabled. Depends on metal-stack/metal-networker#59

@GrigoriyMikhalkin
Copy link
Contributor Author

GrigoriyMikhalkin commented Jun 29, 2021

@majst01 One question is left, how should enableIDS flag and resource field work together? At the moment flag is considered as default value(true) and overwritten by resource field(called DisableSuricataIDS) value if it differs.

@@ -79,6 +79,8 @@ type Data struct {
EgressRules []EgressRuleSNAT `json:"egressRules,omitempty"`
// FirewallNetworks holds the networks known at the metal-api for this firewall machine
FirewallNetworks []FirewallNetwork `json:"firewallNetworks,omitempty"`
// DisableSuricataIDS specifies if we need to enable IDS on the firewall machine
DisableSuricataIDS bool `json:"disableSuricataIDS,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not put any product names into the firewall resources, also it should be disabled by default if nothing was set, so EnableIDS would be more approriate, WDYT ?

Copy link
Contributor Author

@GrigoriyMikhalkin GrigoriyMikhalkin Jun 30, 2021

Choose a reason for hiding this comment

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

Yep, i also thought so initially. But currently we have a flag enable-IDS that is set to true by default. It's used for writing IDS stats to Firewall resource. Should we rid of this flag or change it's default value?(potentially, breaking backwards compatibility)

Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove the flag from main.go, and only rely on the Flag set in the firewall resource. The service will be started by systemd even if a unknown flag is given (needs to be tested).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

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.

2 participants