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

feat: add config validate functionality without starting the server #110

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

DavidProdinger
Copy link

@DavidProdinger DavidProdinger commented Sep 27, 2024

New CLI-Flag

I have added the CLI flag -validate which checks the configuration and exits before starting the SPOA server.
Example call:

$ coraza-spoa -validate -config /etc/coraza-spoa/coraza.yaml
Loading 1 applications
Configuration is valid

Why?

When modifying rules, there can happen errors like wrong syntax or duplicate ids.
If they are unnoticed and you restart the coraza-spoa it will crash and the WAF isn't handling any requests anymore.
This flag enables you to check the configuration before running into such troubles.

This feature isn't solving the restart without downtime as in #19, but it minimizes the risk of bigger downtimes.

David Prodinger added 4 commits September 27, 2024 15:09
Added a flag to validate the configuration without restarting the server
main.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@fzipi fzipi changed the title Add config validate functionality without starting the server feat: add config validate functionality without starting the server Nov 25, 2024
Comment on lines 30 to +33
flag.StringVar(&configPath, "config", "", "configuration file")
flag.StringVar(&configPath, "f", "", "configuration file")
flag.BoolVar(&checkMode, "check", false, "check mode : only check config files and exit")
flag.BoolVar(&checkMode, "c", false, "check mode : only check config files and exit")
Copy link
Author

Choose a reason for hiding this comment

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

@fionera What do you think about having there only the short version of the parameters?

The rewrite is already a breaking change, so changing the parameter isn't that big of a deal.
Also having the same syntax as HAProxy makes it easier to remember...

But making the profiling options short, isn't necessary, since they are here for debugging and not for the run in normal operation. I would leave them long.

Suggested change
flag.StringVar(&configPath, "config", "", "configuration file")
flag.StringVar(&configPath, "f", "", "configuration file")
flag.BoolVar(&checkMode, "check", false, "check mode : only check config files and exit")
flag.BoolVar(&checkMode, "c", false, "check mode : only check config files and exit")
flag.StringVar(&configPath, "f", "", "configuration file")
flag.BoolVar(&checkMode, "c", false, "check mode : only check config files and exit")

Second consideration (or maybe other PR)
And add a version flag to print it from the binary, it gets it from the VCS at buildtime (since Go 1.18)

flag.BoolVar(&version, "v", false, "print version and exit")

// ...

if version {
    var buildInfo, _ = debug.ReadBuildInfo()
    fmt.Printf("Version: %s", buildInfo.Main.Version)
    os.Exit(0)
}

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