Skip to content

Commit

Permalink
api/vmalertmanagerconfig: rework config validation
Browse files Browse the repository at this point in the history
* adds more strict validation to VMAlertmanagerConfig
* skip broken configurations from VMAlertmanager during config build
* adds new status fields to the VMalertmanager config:
  * lastSyncError - that defines last error occured to the config during config generation
  * status - generic CRD status for health track
  * additional fields for sync error context, like last error timestamp and name of alertmanager
* adds stack trace only to the panic level errors. It should reduce log noise

#825
Signed-off-by: f41gh7 <[email protected]>
  • Loading branch information
f41gh7 committed Aug 5, 2024
1 parent 7f0d74d commit 7ba1d4c
Show file tree
Hide file tree
Showing 15 changed files with 2,645 additions and 406 deletions.
14 changes: 11 additions & 3 deletions api/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ require (
github.com/VictoriaMetrics/VictoriaMetrics v1.101.0
github.com/onsi/ginkgo/v2 v2.17.2
github.com/onsi/gomega v1.33.1
github.com/prometheus/alertmanager v0.27.0
github.com/stretchr/testify v1.9.0
gopkg.in/yaml.v2 v2.4.0
k8s.io/api v0.30.2
Expand All @@ -19,6 +20,7 @@ require (
require (
github.com/VictoriaMetrics/metrics v1.33.1 // indirect
github.com/VictoriaMetrics/metricsql v0.75.1 // indirect
github.com/aws/aws-sdk-go v1.51.23 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/bmatcuk/doublestar/v4 v4.6.1 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
Expand All @@ -27,11 +29,13 @@ require (
github.com/evanphx/json-patch v4.12.0+incompatible // indirect
github.com/evanphx/json-patch/v5 v5.9.0 // indirect
github.com/fsnotify/fsnotify v1.7.0 // indirect
github.com/go-kit/log v0.2.1 // indirect
github.com/go-logfmt/logfmt v0.6.0 // indirect
github.com/go-logr/logr v1.4.1 // indirect
github.com/go-logr/zapr v1.3.0 // indirect
github.com/go-openapi/jsonpointer v0.19.6 // indirect
github.com/go-openapi/jsonreference v0.20.2 // indirect
github.com/go-openapi/swag v0.22.3 // indirect
github.com/go-openapi/jsonpointer v0.20.2 // indirect
github.com/go-openapi/jsonreference v0.20.4 // indirect
github.com/go-openapi/swag v0.22.9 // indirect
github.com/go-task/slim-sprig/v3 v3.0.0 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
Expand All @@ -42,18 +46,22 @@ require (
github.com/google/pprof v0.0.0-20240424215950-a892ee059fd6 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/imdario/mergo v0.3.12 // indirect
github.com/jmespath/go-jmespath v0.4.0 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/jpillora/backoff v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/klauspost/compress v1.17.8 // indirect
github.com/mailru/easyjson v0.7.7 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/prometheus/client_golang v1.19.0 // indirect
github.com/prometheus/client_model v0.6.1 // indirect
github.com/prometheus/common v0.52.3 // indirect
github.com/prometheus/common/sigv4 v0.1.0 // indirect
github.com/prometheus/procfs v0.13.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/valyala/bytebufferpool v1.0.0 // indirect
Expand Down
443 changes: 428 additions & 15 deletions api/go.sum

Large diffs are not rendered by default.

34 changes: 34 additions & 0 deletions api/operator/v1beta1/common_scrapeparams.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"reflect"
"strings"

v1 "k8s.io/api/core/v1"
)
Expand Down Expand Up @@ -82,6 +83,24 @@ type OAuth2 struct {
EndpointParams map[string]string `json:"endpoint_params,omitempty"`
}

func (o *OAuth2) validate() error {
if o == nil {
return nil
}
if o.TokenURL == "" {
return fmt.Errorf("token_url field for oauth2 config must be set")
}

if o.ClientID == (SecretOrConfigMap{}) {
return fmt.Errorf("client_id field must be set")
}

if o.ClientID.Secret != nil && o.ClientID.ConfigMap != nil {
return fmt.Errorf("cannot specify both Secret and ConfigMap for client_id field")
}
return nil
}

// Authorization configures generic authorization params
type Authorization struct {
// Type of authorization, default to bearer
Expand All @@ -94,6 +113,21 @@ type Authorization struct {
CredentialsFile string `json:"credentialsFile,omitempty"`
}

func (ac *Authorization) validate() error {
if ac == nil {
return nil
}

if strings.ToLower(strings.TrimSpace(ac.Type)) == "basic" {
return fmt.Errorf("Authorization type cannot be set to 'basic', use 'basic_auth' instead`")
}

if ac.Credentials == nil && len(ac.CredentialsFile) == 0 {
return fmt.Errorf("at least `credentials` or `credentials_file` must be set")
}
return nil
}

// RelabelConfig allows dynamic rewriting of the label set
// More info: https://docs.victoriametrics.com/#relabeling
// +k8s:openapi-gen=true
Expand Down
5 changes: 5 additions & 0 deletions api/operator/v1beta1/vmalertmanager_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ func (r *VMAlertmanager) SetupWebhookWithManager(mgr ctrl.Manager) error {
var _ webhook.Validator = &VMAlertmanager{}

func (r *VMAlertmanager) sanityCheck() error {
if len(r.Spec.ConfigRawYaml) > 0 {
if err := ValidateAlertmanagerConfigSpec([]byte(r.Spec.ConfigRawYaml)); err != nil {
return fmt.Errorf("bad config syntax at spec.configRawYaml: %w", err)
}
}
if r.Spec.ConfigSecret == r.ConfigSecretName() {
return fmt.Errorf("spec.configSecret uses the same name as built-in config secret used by operator. Please change it's name")
}
Expand Down
46 changes: 34 additions & 12 deletions api/operator/v1beta1/vmalertmanager_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,52 @@ package v1beta1

import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var _ = Describe("VMAlertmanager Webhook", func() {

var am *VMAlertmanager
BeforeEach(func() {
am = &VMAlertmanager{
ObjectMeta: metav1.ObjectMeta{
Name: "test-suite",
Namespace: "test",
},
Spec: VMAlertmanagerSpec{},
}
})
Context("When creating VMAlertmanager under Validating Webhook", func() {
It("Should deny if a required field is empty", func() {

// TODO(user): Add your logic here

It("Should deny config file with bad syntax", func() {
am.Spec.ConfigRawYaml = `
global:
resolve_timeout: 10m
group_wait: 1s
`
Expect(am.sanityCheck()).NotTo(Succeed())
})

It("Should admit if all required fields are provided", func() {

// TODO(user): Add your logic here

It("Should allow with correct config syntax", func() {
am.Spec.ConfigRawYaml = `
global:
resolve_timeout: 5m
route:
group_wait: 10s
group_interval: 2m
group_by: ["alertgroup", "resource_id"]
repeat_interval: 12h
receiver: 'blackhole'
receivers:
# by default route to dev/null
- name: blackhole
`
Expect(am.sanityCheck()).To(Succeed())
})
})

Context("When creating VMAlertmanager under Conversion Webhook", func() {
It("Should get the converted version of VMAlertmanager", func() {

// TODO(user): Add your logic here

})
})

})
Loading

0 comments on commit 7ba1d4c

Please sign in to comment.