Provide alternative to yaml comments for configuring linting rules

[mario-steinhoff-gcx]

Hey, just discovered this tool, awesome work!

We would like to use pint in watch mode for continous scanning of our prometheus rules, and use the owner feature so we can route alerts for problems to the right team.

Unfortunately, pint linting rules can only be configured via yaml comments, but we use jsonnet to generate our prometheus rules (serializing them to yaml via std.manifestYamlDoc()) and I couldn't find a way to add comments to the generated output. I am no jsonnet expert but it looks like the design of jsonnet doesn't even allow this.

Our primary motivation for jsonnet is that we can use the existing monitoring-mixins ecosystem and get an improved authoring experience over raw yaml - which works pretty well so far.

Some linters allow their rules to be configured in external config files, e.g. in PMD you can define rulesets - but I wouldn't use XML today :-) I havent thought about how this could look in detail but in general I think such a mechanism could solve our problem.

Would you be open to implement or accept a PR for such a feature?

[prymitive]

What kind of rules are you looking to add?

Comments are used mostly for rule level overrides, rules themselves have minimal configuration which is set in a config file. This is a common pattern among tools that are checking source code. pint is not unique here.

It's comments because this allows to place any overrides next to the source code they are for. Any other solution would require you to write your rule in one place and then add a pint override in another place, which will make maintenance of those rules much more difficult. There are some things that could be configured centrally, since they don't change much, like file ownership, but I wouldn't add any central mapping of everything that everyone can set, that's just gonna get messy. Maybe you should report a bug/request to jsonnet?

[mario-steinhoff-gcx]

What kind of rules are you looking to add?

I havent started integrating pint in our workflow yet, so I cant give you a comprehensive list. Most checks look useful though, so I would start by enabling all of them and see how far I get with this.

It's comments because this allows to place any overrides next to the source code they are for. Any other solution would require you to write your rule in one place and then add a pint override in another place, which will make maintenance of those rules much more difficult.

Sounds like a reasonable decision, but I'm not sure if it would be really that much more difficult to maintain overrides in .pint.hcl vs inline comments.

The configuration already provides a way to match rules to checks via rule/match/ignore blocks. It also allows to define multiple rule blocks and this gives a lot of flexibility, but the following won't work (contrived example to demonstrate the behavior).

Given a rules.yaml file:

groups:
  - name: "ingress-nginx.rules"
    rules:
      - alert: "NGINXConfigFailed"
        annotations:
          description: "bad ingress config - nginx config test failed"
        expr: "count(nginx_ingress_controller_config_last_reload_successful == 0) > 0"
        for: "1s"
        labels:
          severity: "critical"
      - alert: "NGINXCertificateExpiry"
        annotations:
          description: "ssl certificate(s) will expire in less then a week"
          summary: "renew expiring certificates to avoid downtime"
        expr: "(avg(nginx_ingress_controller_ssl_expire_time_seconds) by (host) - time()) < 604800"
        for: "1s"
        labels:
          severity: "critical"

and a .pint.hcl file:

rule {
  match {
    kind = "alerting"
    name = ".+"
  }

  annotation "summary" {
    required = true
  }
}

rule {
  match {
    kind = "alerting"
    name = "NGINXConfigFailed"
  }

  annotation "summary" {
    required = false
  }
}

Results in the following output:

> pint lint test.yaml
level=info msg="Loading configuration file" path=.pint.hcl
test.yaml:5-6: summary annotation is required (alerts/annotation)
 5 |         annotations:
 6 |           description: "bad ingress config - nginx config test failed"

level=info msg="Problems found" Warning=1

Where I would have expected that this passes, since there is a more specific rule block that overrides settings for the same annotation check.

This could be accompanied by allowing something like this:

rule {
  match {
    ...
  }

  checks {
    disabled = [...]
  }
}

To disable checks via matchers.

There are some things that could be configured centrally, since they don't change much, like file ownership, but I wouldn't add any central mapping of everything that everyone can set, that's just gonna get messy.

Having a central way to configure file ownership sounds really nice. Our files already follow a naming scheme that dictates ownership, so with the above suggestion in mind, ideally we could add config blocks like this:

rule {
  match {
    path = "team-abc-(.+)"
  }

  check "rule/owner" {
    owner = "team-abc"
  }
}

For the prom/series check, it seems that comments are the only way to configure min-age and ignore/label-value. While I'm not sure if we need to override min-age, the ignore/label-value is definitively something we would use sooner or later.

Maybe you should report a bug/request to jsonnet?

Hm yeah, might do. It seems that the jsonnet stdlib is implemented in jsonnet and the std.manifestYamlDoc function uses good ol' string concatenation to build the yaml structure. I dont see a technical reason that would prevent this from outputting comments, but it would require some special convention on the input data to std.manifestYamlDoc so it knows that a key should be printed as a comment, which feels kind of hacky.

[prymitive]

Where I would have expected that this passes, since there is a more specific rule block that overrides settings for the same annotation check.

If it matches it applies, it doesn't matter than there's another rule that also matches. If you want to exclude a subset of rules then add an ignore.

[LeoQuote]

It could be useful if it's possible to ignore an alert in annotations, we're using prometheus operator as well, the comment in yaml will disappear in the output prometheus rules.