zegl / kube-score

Kubernetes object analysis with recommendations for improved reliability and security. kube-score actively prevents downtime and bugs in your Kubernetes YAML and Charts. Static code analysis for Kubernetes.
https://kube-score.com
MIT License
2.72k stars 174 forks source link

Feature request: a configuration file #384 #452

Closed kmarteaux closed 2 years ago

kmarteaux commented 2 years ago
RELNOTE:  

New kube-score configuration file capability.

### Configuration File Creation 

kube-score mkconfig action will create a configuration file from its registered checks. A user create a configuration file by running 

$ kube-score mkconfig --config /pathname/to/file

If the file has already been created, use the --force flag to overwrite the file. 

# Examples
$ ./kube-score mkconfig --config ./.kube-score.yml
File ./.kube-score.yml exists. Use --force flag to overwrite

$ ./kube-score mkconfig --config ./.kube-score.yml --force
Created kube-score configuration file  ./.kube-score.yml

To use the generated configuration file with kube-score 

$ kube-score score --config /pathname/to/config-file  /pathnname/to/<whatever>.yaml   

### Sample configuration files ###

A configuration can contain a list of checks to include and exclude. 

disable-all: true enable:

enable-all: true disable:

Running kube-score without a configuration file would be the same as

disable-all: false
enable-all: false
enable:  [] # all currently non-optional checks
disable: []
zegl commented 2 years ago

Hey! Sorry for not having the time to review this earlier (or participate in the design discussion)...

By default, kube-score will create a configuration file called .kube-score.yml

I think that it would be nice to not do this by default.

If kube-score is able to keep track of it's own defaults, as is needed to generate the configuration file in the first place, the configuration is not needed by default. So basically, if no configuration file could be found, don't generate one, and let the application run as normal.

This also means that this change would be fully backwards compatible (as it doesn't change any behaviour unless there exists a .kube-score.yaml file or --config is set).

I also don't want to risk breaking things by assuming that the filesystem is writeable.

What do you think?

kmarteaux commented 2 years ago

I agreed with your assessment and will modify the code accordingly.

kmarteaux commented 2 years ago

I will make all the recommended changes outlined and update the PR.

zegl commented 2 years ago

The code looks good, but after playing around with the configuration file, it's a bit confusing to me. The differences between a default and a optional check is extra confusing. I realise that this is lingo that comes from arguments already in kube-score, but together with the configuration file it does not make a lot of sense to me anymore.

Would it be possible for the configuration to only contain a list of checks to include and exclude?

disable-all: true # defaults to false
enable:
    - ingress-targets-service
    - cronjob-has-deadline
    - container-resources
    - container-image-tag

enable-all: true # defaults to false
disable:
    - statefulset-has-poddisruptionbudget
    - deployment-has-poddisruptionbudget
    - poddisruptionbudget-has-policy
    - pod-networkpolicy

I've been inspired by the configuration from golangci-lint, which has been a pleasure to use.

Running kube-score without a configuration file would be the same as

disable-all: false
enable-all: false
enable: # all currently non-optional checks
disable: []

Setting --enable-optional-test would be the same as adding the test to enable, and setting --ignore-test would be the same as adding it to disable.

When running score I propose that:

  1. Start with an empty enable and disable list
  2. If enable-all is true, add all tests to the list. If it's false, add only the non-optional tests to the list.
  3. If disable-all is true, add all tests to the disable list. If it's false, do nothing.
  4. If enable is set, use it as the enable list.
  5. If disable is set, use it as the disable list.
  6. If --enable-optional-test is set, add the test(s) to the enable list
  7. If --ignore-test is set, add the test(s) to the disable list

And when running, run a test if the test is on the enable list and not on the disable list.

As a bonus, we can add a --enable and --disable flags as aliases to --enable-optional-test and --ignore-test to simplify naming across the board.

What do you think?

kmarteaux commented 2 years ago

I like the proposed changes. At times I struggled myself with the -- "What's sensible?" -- question. The idea of including the default and optional checks was for the entire configuration to be self-documenting, but it does clutter things up. I will make the changes you've outlined.

kmarteaux commented 2 years ago

@zegl have a look at the revised implementation, hopefully it captures the feedback as intended. @zegl ?