Invalid AlertmanagerConfig passes validation and results in 'group_interval cannot be zero' errors

[leonardo-zorzi]

Is there an existing issue for this?

• I have searched the existing issues

What happened?

Description

AlertmanagerConfig CR with group_interval set to 0 passes validation and results in errors on the alertmanager instances itself.
ts=2024-05-15T12:04:48.725Z caller=coordinator.go:118 level=error component=configuration msg="Loading configuration file failed" file=/etc/alertmanager/config_out/alertmanager.env.yaml err="group_interval cannot be zero"

Steps to Reproduce

spec:
  route:
      groupBy: [ "..." ]
      groupWait: 0s
      groupInterval: 0s
      repeatInterval: 24h
      receiver: "a_receiver"
      routes:
        - matchers:
            - name: "alertname"
              value: "InfoInhibitor"
          receiver: "null"

I run the Operator on v0.67.1 (unfortunately the latest kube-prometheus) but the current master CRD seems to references still the same regex.

Expected Result

CR fails validation.

Actual Result

CR passed validation and gets added to the final config file.

Prometheus Operator Version

v0.67.1

Kubernetes Version

1.27

Kubernetes Cluster Type

GKE

How did you deploy Prometheus-Operator?

Other (please comment)

Manifests

kube-prometheus jsonnet

prometheus-operator log output

`ts=2024-05-15T12:04:48.725Z caller=coordinator.go:118 level=error component=configuration msg="Loading configuration file failed" file=/etc/alertmanager/config_out/alertmanager.env.yaml err="group_interval cannot be zero"`

Anything else?

No response

[codeknight03]

This one looks very similar to #6532. I have a PR open to fix this #6585.

I'll try to see if it is a similar case and if it is then I can take this up with another PR.

[codeknight03]

I missed that it is being validated unlike the case I referenced above but the regex being used is allowing 0 to be taken. The regex is pretty much used for all duration values. Might need to check for what all values the same case might occur.

prometheus-operator/pkg/alertmanager/validation/v1alpha1/validation.go

Lines 392 to 402 in 48e9858

	if r.GroupInterval != "" && !durationRe.MatchString(r.GroupInterval) {
	return fmt.Errorf("groupInterval %s does not match required regex: %s", r.GroupInterval, durationRe.String())
	}
	if r.GroupWait != "" && !durationRe.MatchString(r.GroupWait) {
	return fmt.Errorf("groupWait %s does not match required regex: %s", r.GroupWait, durationRe.String())
	}
	if r.RepeatInterval != "" && !durationRe.MatchString(r.RepeatInterval) {
	return fmt.Errorf("repeatInterval %s does not match required regex: %s", r.RepeatInterval, durationRe.String())
	}

[simonpasquier]

Thanks for the report! We'd need a different regex for durations that can't be zero.

[ArthurSens]

The fix would be in this regex:

prometheus-operator/pkg/apis/monitoring/v1/types.go

Line 40 in ff2896a

// +kubebuilder:validation:Pattern:="^(0|(([0-9]+)y)?(([0-9]+)w)?(([0-9]+)d)?(([0-9]+)h)?(([0-9]+)m)?(([0-9]+)s)?(([0-9]+)ms)?)$"

A test case showing that we reject configs that us 0s as duration would also be good :)

[codeknight03]

Will be adding a PR ASAP.

[simonpasquier]

To add to what @ArthurSens said and to expand my earlier comment, the existing pattern is correct for most duration fields except for groupInterval and repeatInterval which don't allow "zero" values. Hence we'd need to use a different type with a pattern that rejects "zero" durations (though it may be impossible to craft a regex covering all edge cases).