helm-charts icon indicating copy to clipboard operation
helm-charts copied to clipboard

[victoria-metrics-k8s-stack] empty vmalert.spec.notifier creates bogus configuration.

Open b-a-t opened this issue 1 year ago • 4 comments

I'm using ArgoCD ApplicationSet to deploy VM to the clusters. Due to the way templating works, it's easier for me to specify an empty resource line, rather than skip the parameter entirely.

So, for certain clusters I got:

vmalert:
  spec:
    externalLabels:
       cluster: 'dev'
    notifier: { }
  ingress:
     enabled: true
...

The resulting command line is quite unexpected:

... -notifier.url=http://vmalertmanager-vm-stack.monitoring.svc:9093, -remoteRead.url=http://vmselect-vm-stack.monitoring.svc:8481/select/0/prometheus ...

Please note the comma after the first(default implicit) notifier. So, an empty map is creating an empty invisible URL, which is already not good.

But based on the following error message in the logs:

{"ts":"2023-03-22T01:58:13.861Z","level":"error","caller":"VictoriaMetrics/app/vmalert/group.go:318","msg":"group \"vm-health\": errors(1): rule \"TooManyLogs\": failed to send alerts to addr \"/api/v2/alerts\": Post \"/api/v2/alerts\": unsupported protocol scheme \"\""}

that empty non-existing URL is still taken into account as URL and an attempt to write to it is performed.

So to me, there are two errors in the processing of the empty notifier field:

  1. it is taken into account as a normal URL by YAML parsing code.
  2. comma in the list of -notifier.url parameters also happily generates empty URL configuration.

So, IMHO in both places we need an additional sanity checks.

b-a-t avatar Mar 22 '23 11:03 b-a-t