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

Proposed application.properties Defaults #17

Open
stackedsax opened this issue Dec 14, 2020 · 0 comments
Open

Proposed application.properties Defaults #17

stackedsax opened this issue Dec 14, 2020 · 0 comments
Labels
enhancement New feature or request

Comments

@stackedsax
Copy link
Contributor

stackedsax commented Dec 14, 2020

I'm hacking my way through creating helm charts for config-editor-ui and config-editor-rest and I thought I should write down some thoughts for improvements to the configuration system. I thought I should put my thoughts in the form of a proposal so we can all agree on what we're aiming for before going into the code and trying to make changes there.

Decent Defaults, Everything Customizable

I think that Siembol should default to a basic set of defaults and the user should only have to define the most minimal amount of configuration to get started. All of the customizations should still exist, but only if a user wants to configure custom locations for different repositories and directories.

Just the basics

Ultimately, I would like this as a minimal set of configuration options a user needs to set:

config-editor.service-types=alert,correlationalert,bdp-correlationalert,parserconfig,parsingapp,enrichment
config-editor.config-store.git-user-name=<my_username>
config-editor.config-store.store-repository-name=<my_repo>

Git defaults

Some of the git defaults that we could remove from application.properties would be:

config-editor.config-store.github-url=<default to https://github.com>
config-editor.config-store.git-password=<default_password_from_secret> ## deault this to a `kubectl create secret` name it ${GIT_PSWD}

Auth defaults

Additionally, I think authentication should default to:

config-editor-auth.type=disabled

That way, potential users aren't forced to set up authentication just to kick the tires.

However, not setting config-editor-auth.type throws an error right now. This should probably be a separate issue to make it not error.

Path defaults

For paths on the file system and in the repository, I would propose a couple of things:

  • default to one repository (for both *-repository-names)
  • default to one directory for the locally saved repository (for both *-repository-paths)
    • I'd love to find a more representative name for this option than "repository-name". It made me think that it was something to do with the remote repository in github for a while. It took me a second to realize that it meant the local file path where the repository was going to be checked out. It's a minor thing, but worth noting.
  • put that one default repository in one default "repository-path" that can contain multiple repositories if required. i.e.:
    • /<ephemeral_volume>/siembol-repositories/<repo_name>
  • default the subdirectories in each service-specific subdirectory to these three: "rules", "release", "testcases".
    • tiny question: should "release" be "releases" so make the pluralization of these directories match?
  • default to the name of each of the service types for the service-specific rules.
    • i.e., for the service type alert, the repository-path of /usr/local/siembol-repositories, and my personal repository of siembol-config-stackedsax, we assume that the directory structure is /usr/local/siembol-repositories/siembol-config-stackedsax/alert.

The end result should look like:

  • default remote repo: github://<repo_name>/<service_name>/['rules','release','testcases']
  • default local repo: /<ephemeral_volume>/siembol-repositories/<repo_name>/<service_name>/[’rules,‘release’,‘testcases’]
    • i.e. /usr/local/siembol-repositories/siembol-config-stackedsax/alert/rules

UI Config Filename

Lastly, I think the UI Config filename can be relative, not absolute, and default to:

config-editor.services.<service>.ui-config-file-name=ui-config/<service>-layout-config.json

Customizations

I still agree with all the customizations that are available to a user. Being able to specify all of these items per service seems like something that is good to anticipate:

  • github url
  • git username and password
  • store-repository-name
  • release-repository-name
  • store-repository-path
  • release-repository-path
  • store-directory
  • release-directory
  • test-case-directory

Eventually, someone will want to do something like this, so having the ability to override the defaults seems good. It looks like the ability to do this is already available, it's more just a matter of making that a custom, expert-level choice.

My expectation here is that you could choose any of these customizations a la carte. For example, you could choose to have different repositories for store and release, but you could still use the default repository-path and sub-directories without having to specify them, too.

Log Messages

While I worked through application.properties, I felt that the log messages could be improved for:

  • Incorrect directory structure
    • This does report an error that the directory could not be found, but it comes amidst a much larger and unnecessary stacktrace
  • Missing rules.json
    • I don't recall the stacktrace being very informative as to what was missing here
  • Incorrect format for rules.json
    • I only knew this was the issue because Jonathan had mentioned it to me
  • Incorrect/missing testcase format
    • I'm just guessing that this is the error I'm currently on
    • Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [uk.co.gresearch.siembol.configeditor.testcase.TestCaseEvaluator]: Factory method 'testCaseEvaluator' threw exception; nested exception is java.lang.NullPointerException

I can move these improvements to a separate issue -- I just wanted to make sure these ideas were captured.

Generate Skeleton Config

While we can suggest that people fork a siembol-config directory with the default structure as a way to get started, there may be plenty of people who don't follow that approach and instead create a bare config directory.

As an alternative approach, could we automatically create a PR on the initial startup that creates a skeleton rules repository? For example, if the default or the custom-specified directories don't exist when the repository if checked out locally, Siembol could create those directories and create a PR that offers to commit those directories in. Further, Siembol could add some default demo rules.json files. All a user would have to do is merge the PR and the next deployment of Siembol would work.

I want to make all these rules and directory creations as simple as possible for a new user, so any more ideas along these lines are welcome.

@mariannovotny mariannovotny added the enhancement New feature or request label May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants