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

Default prometheusRules.rules should be an empty list, not map #823

Closed
wants to merge 1 commit into from

Conversation

vitaliyf
Copy link
Contributor

Support for prometheus-operator was added in #772 and a default empty set of rules was defined as an empty map {}. However, as evidenced by the commented out rule examples below that very same values.yaml, this is expected to be a list, so rules: value should be set to an empty list [].

Because of this Helm issues a warning if you actually define rules and pass them with a --values :

coalesce.go:220: warning: cannot overwrite table with non table for vault.serverTelemetry.prometheusRules.rules (map[])

Support for prometheus-operator was added in hashicorp#772 and a default empty set of rules was defined as an empty map `{}`. However, as evidenced by the commented out rule examples below that very same values.yaml, this is expected to be a list, so `rules:` value should be set to an empty list `[]`.

Because of this Helm issues a warning if you actually define rules and pass them with a --values <filename>:

```
coalesce.go:220: warning: cannot overwrite table with non table for vault.serverTelemetry.prometheusRules.rules (map[])
```
@hashicorp-cla
Copy link

hashicorp-cla commented Dec 23, 2022

CLA assistant check
All committers have signed the CLA.

@tvoran tvoran added bug Something isn't working vault-server Area: operation and usage of vault server in k8s labels Jan 12, 2023
@tvoran
Copy link
Member

tvoran commented Jan 12, 2023

Hi @vitaliyf, good spot! It looks like you're right, since PrometheusRule.spec.groups.rules is defined as a list in the operator.

We should update the tests in test/unit/prometheus-prometheusrules.bats, and also update the json schema to ensure that only an array is accepted for vault.serverTelemetry.prometheusRules.rules.

@tvoran
Copy link
Member

tvoran commented May 18, 2023

Superseded by #886.

@tvoran tvoran closed this May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working vault-server Area: operation and usage of vault server in k8s
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants