diff --git a/pkg/alertmanager/operator.go b/pkg/alertmanager/operator.go index bf4004c22..7f043d48d 100644 --- a/pkg/alertmanager/operator.go +++ b/pkg/alertmanager/operator.go @@ -700,7 +700,7 @@ func (c *Operator) sync(ctx context.Context, key string) error { return err } - sset, err := makeStatefulSet(am, c.config, newSSetInputHash, tlsAssets.ShardNames()) + sset, err := makeStatefulSet(logger, am, c.config, newSSetInputHash, tlsAssets.ShardNames()) if err != nil { return fmt.Errorf("failed to generate statefulset: %w", err) } diff --git a/pkg/alertmanager/statefulset.go b/pkg/alertmanager/statefulset.go index d0520caad..d6367873e 100644 --- a/pkg/alertmanager/statefulset.go +++ b/pkg/alertmanager/statefulset.go @@ -21,11 +21,14 @@ import ( "strings" "github.com/blang/semver/v4" + "github.com/go-kit/log" + "github.com/go-kit/log/level" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" "github.com/prometheus-operator/prometheus-operator/pkg/k8sutil" @@ -64,7 +67,7 @@ var ( probeTimeoutSeconds int32 = 3 ) -func makeStatefulSet(am *monitoringv1.Alertmanager, config Config, inputHash string, tlsAssetSecrets []string) (*appsv1.StatefulSet, error) { +func makeStatefulSet(logger log.Logger, am *monitoringv1.Alertmanager, config Config, inputHash string, tlsAssetSecrets []string) (*appsv1.StatefulSet, error) { // TODO(fabxc): is this the right point to inject defaults? // Ideally we would do it before storing but that's currently not possible. // Potentially an update handler on first insertion. @@ -90,7 +93,7 @@ func makeStatefulSet(am *monitoringv1.Alertmanager, config Config, inputHash str am.Spec.Resources.Requests[v1.ResourceMemory] = resource.MustParse("200Mi") } - spec, err := makeStatefulSetSpec(am, config, tlsAssetSecrets) + spec, err := makeStatefulSetSpec(logger, am, config, tlsAssetSecrets) if err != nil { return nil, err } @@ -224,7 +227,7 @@ func makeStatefulSetService(p *monitoringv1.Alertmanager, config Config) *v1.Ser return svc } -func makeStatefulSetSpec(a *monitoringv1.Alertmanager, config Config, tlsAssetSecrets []string) (*appsv1.StatefulSetSpec, error) { +func makeStatefulSetSpec(logger log.Logger, a *monitoringv1.Alertmanager, config Config, tlsAssetSecrets []string) (*appsv1.StatefulSetSpec, error) { amVersion := operator.StringValOrDefault(a.Spec.Version, operator.DefaultAlertmanagerVersion) amImagePath, err := operator.BuildImagePath( operator.StringPtrValOrDefault(a.Spec.Image, ""), @@ -531,8 +534,13 @@ func makeStatefulSetSpec(a *monitoringv1.Alertmanager, config Config, tlsAssetSe amCfg := a.Spec.AlertmanagerConfiguration if amCfg != nil && len(amCfg.Templates) > 0 { sources := []v1.VolumeProjection{} + keys := sets.Set[string]{} for _, v := range amCfg.Templates { if v.ConfigMap != nil { + if keys.Has(v.ConfigMap.Key) { + level.Debug(logger).Log("msg", fmt.Sprintf("skipping %q due to duplicate key %q", v.ConfigMap.Key, v.ConfigMap.Name)) + continue + } sources = append(sources, v1.VolumeProjection{ ConfigMap: &v1.ConfigMapProjection{ LocalObjectReference: v1.LocalObjectReference{ @@ -544,9 +552,13 @@ func makeStatefulSetSpec(a *monitoringv1.Alertmanager, config Config, tlsAssetSe }}, }, }) - + keys.Insert(v.ConfigMap.Key) } if v.Secret != nil { + if keys.Has(v.Secret.Key) { + level.Debug(logger).Log("msg", fmt.Sprintf("skipping %q due to duplicate key %q", v.Secret.Key, v.Secret.Name)) + continue + } sources = append(sources, v1.VolumeProjection{ Secret: &v1.SecretProjection{ LocalObjectReference: v1.LocalObjectReference{ @@ -558,6 +570,7 @@ func makeStatefulSetSpec(a *monitoringv1.Alertmanager, config Config, tlsAssetSe }}, }, }) + keys.Insert(v.Secret.Key) } } volumes = append(volumes, v1.Volume{ diff --git a/pkg/alertmanager/statefulset_test.go b/pkg/alertmanager/statefulset_test.go index e5167628d..59ecc4313 100644 --- a/pkg/alertmanager/statefulset_test.go +++ b/pkg/alertmanager/statefulset_test.go @@ -20,6 +20,7 @@ import ( "strings" "testing" + "github.com/go-kit/log" "github.com/kylelemons/godebug/pretty" "github.com/stretchr/testify/require" "golang.org/x/exp/slices" @@ -69,7 +70,7 @@ func TestStatefulSetLabelingAndAnnotations(t *testing.T) { "app.kubernetes.io/instance": "", } - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ ObjectMeta: metav1.ObjectMeta{ Labels: labels, Annotations: annotations, @@ -101,7 +102,7 @@ func TestStatefulSetStoragePath(t *testing.T) { annotations := map[string]string{ "testannotation": "testannotationvalue", } - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ ObjectMeta: metav1.ObjectMeta{ Labels: labels, Annotations: annotations, @@ -131,7 +132,7 @@ func TestPodLabelsAnnotations(t *testing.T) { labels := map[string]string{ "testlabel": "testvalue", } - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ ObjectMeta: metav1.ObjectMeta{}, Spec: monitoringv1.AlertmanagerSpec{ PodMetadata: &monitoringv1.EmbeddedObjectMetadata{ @@ -153,7 +154,7 @@ func TestPodLabelsShouldNotBeSelectorLabels(t *testing.T) { labels := map[string]string{ "testlabel": "testvalue", } - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ ObjectMeta: metav1.ObjectMeta{}, Spec: monitoringv1.AlertmanagerSpec{ PodMetadata: &monitoringv1.EmbeddedObjectMetadata{ @@ -189,7 +190,7 @@ func TestStatefulSetPVC(t *testing.T) { }, } - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ ObjectMeta: metav1.ObjectMeta{ Labels: labels, Annotations: annotations, @@ -220,7 +221,7 @@ func TestStatefulEmptyDir(t *testing.T) { Medium: v1.StorageMediumMemory, } - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ ObjectMeta: metav1.ObjectMeta{ Labels: labels, Annotations: annotations, @@ -258,7 +259,7 @@ func TestStatefulSetEphemeral(t *testing.T) { }, } - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ ObjectMeta: metav1.ObjectMeta{ Labels: labels, Annotations: annotations, @@ -279,7 +280,7 @@ func TestStatefulSetEphemeral(t *testing.T) { } func TestListenLocal(t *testing.T) { - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ Spec: monitoringv1.AlertmanagerSpec{ ListenLocal: true, }, @@ -313,7 +314,7 @@ func TestListenLocal(t *testing.T) { } func TestListenTLS(t *testing.T) { - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ Spec: monitoringv1.AlertmanagerSpec{ Web: &monitoringv1.AlertmanagerWebSpec{ WebConfigFileFields: monitoringv1.WebConfigFileFields{ @@ -418,7 +419,7 @@ func TestMakeStatefulSetSpecSingleDoubleDashedArgs(t *testing.T) { replicas := int32(3) a.Spec.Replicas = &replicas - statefulSet, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + statefulSet, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -439,7 +440,7 @@ func TestMakeStatefulSetSpecWebRoutePrefix(t *testing.T) { a.Spec.Version = operator.DefaultAlertmanagerVersion a.Spec.Replicas = &replicas - statefulSet, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + statefulSet, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -496,7 +497,7 @@ func TestMakeStatefulSetSpecWebTimeout(t *testing.T) { a.Spec.Version = ts.version a.Spec.Web = ts.web - ss, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + ss, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -548,7 +549,7 @@ func TestMakeStatefulSetSpecWebConcurrency(t *testing.T) { a.Spec.Version = ts.version a.Spec.Web = ts.web - ss, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + ss, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -575,7 +576,7 @@ func TestMakeStatefulSetSpecPeersWithoutClusterDomain(t *testing.T) { }, } - statefulSet, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + statefulSet, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -610,7 +611,7 @@ func TestMakeStatefulSetSpecPeersWithClusterDomain(t *testing.T) { configWithClusterDomain := defaultTestConfig configWithClusterDomain.ClusterDomain = "custom.cluster" - statefulSet, err := makeStatefulSetSpec(&a, configWithClusterDomain, nil) + statefulSet, err := makeStatefulSetSpec(nil, &a, configWithClusterDomain, nil) if err != nil { t.Fatal(err) } @@ -636,7 +637,7 @@ func TestMakeStatefulSetSpecAdditionalPeers(t *testing.T) { a.Spec.Replicas = &replicas a.Spec.AdditionalPeers = []string{"example.com"} - statefulSet, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + statefulSet, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -673,7 +674,7 @@ func TestMakeStatefulSetSpecNotificationTemplates(t *testing.T) { }, }, } - statefulSet, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + statefulSet, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -703,7 +704,7 @@ func TestMakeStatefulSetSpecNotificationTemplates(t *testing.T) { func TestAdditionalSecretsMounted(t *testing.T) { secrets := []string{"secret1", "secret2"} - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ ObjectMeta: metav1.ObjectMeta{}, Spec: monitoringv1.AlertmanagerSpec{ Secrets: secrets, @@ -757,7 +758,7 @@ func TestAlertManagerDefaultBaseImageFlag(t *testing.T) { "testannotation": "testannotationvalue", } - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ ObjectMeta: metav1.ObjectMeta{ Labels: labels, Annotations: annotations, @@ -775,7 +776,7 @@ func TestAlertManagerDefaultBaseImageFlag(t *testing.T) { func TestSHAAndTagAndVersion(t *testing.T) { { - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ Spec: monitoringv1.AlertmanagerSpec{ Tag: "my-unrelated-tag", Version: "v0.15.3", @@ -792,7 +793,7 @@ func TestSHAAndTagAndVersion(t *testing.T) { } } { - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ Spec: monitoringv1.AlertmanagerSpec{ SHA: "7384a79f4b4991bf8269e7452390249b7c70bcdd10509c8c1c6c6e30e32fb324", Tag: "my-unrelated-tag", @@ -811,7 +812,7 @@ func TestSHAAndTagAndVersion(t *testing.T) { } { image := "my-registry/alertmanager:latest" - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ Spec: monitoringv1.AlertmanagerSpec{ SHA: "7384a79f4b4991bf8269e7452390249b7c70bcdd10509c8c1c6c6e30e32fb324", Tag: "my-unrelated-tag", @@ -841,7 +842,7 @@ func TestRetention(t *testing.T) { } for _, test := range tests { - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ Spec: monitoringv1.AlertmanagerSpec{ Retention: test.specRetention, }, @@ -867,7 +868,7 @@ func TestRetention(t *testing.T) { } func TestAdditionalConfigMap(t *testing.T) { - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ Spec: monitoringv1.AlertmanagerSpec{ ConfigMaps: []string{"test-cm1"}, }, @@ -909,14 +910,14 @@ func TestSidecarResources(t *testing.T) { Spec: monitoringv1.AlertmanagerSpec{}, } - sset, err := makeStatefulSet(am, testConfig, "", nil) + sset, err := makeStatefulSet(nil, am, testConfig, "", nil) require.NoError(t, err) return sset }) } func TestTerminationPolicy(t *testing.T) { - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ Spec: monitoringv1.AlertmanagerSpec{}, }, defaultTestConfig, "", nil) if err != nil { @@ -936,7 +937,7 @@ func TestClusterListenAddressForSingleReplica(t *testing.T) { a.Spec.Version = operator.DefaultAlertmanagerVersion a.Spec.Replicas = &replicas - statefulSet, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + statefulSet, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -963,7 +964,7 @@ func TestClusterListenAddressForSingleReplicaWithForceEnableClusterMode(t *testi a.Spec.Replicas = &replicas a.Spec.ForceEnableClusterMode = true - statefulSet, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + statefulSet, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -989,7 +990,7 @@ func TestClusterListenAddressForMultiReplica(t *testing.T) { a.Spec.Version = operator.DefaultAlertmanagerVersion a.Spec.Replicas = &replicas - statefulSet, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + statefulSet, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -1016,7 +1017,7 @@ func TestExpectStatefulSetMinReadySeconds(t *testing.T) { a.Spec.Replicas = &replicas // assert defaults to zero if nil - statefulSet, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + statefulSet, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -1027,7 +1028,7 @@ func TestExpectStatefulSetMinReadySeconds(t *testing.T) { // assert set correctly if not nil var expect uint32 = 5 a.Spec.MinReadySeconds = &expect - statefulSet, err = makeStatefulSetSpec(&a, defaultTestConfig, nil) + statefulSet, err = makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) if err != nil { t.Fatal(err) } @@ -1079,7 +1080,7 @@ func TestPodTemplateConfig(t *testing.T) { } imagePullPolicy := v1.PullAlways - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ ObjectMeta: metav1.ObjectMeta{}, Spec: monitoringv1.AlertmanagerSpec{ NodeSelector: nodeSelector, @@ -1134,7 +1135,7 @@ func TestPodTemplateConfig(t *testing.T) { } func TestConfigReloader(t *testing.T) { - baseSet, err := makeStatefulSet(&monitoringv1.Alertmanager{}, defaultTestConfig, "", nil) + baseSet, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{}, defaultTestConfig, "", nil) require.NoError(t, err) expectedArgsConfigReloader := []string{ @@ -1173,7 +1174,7 @@ func TestConfigReloader(t *testing.T) { func TestAutomountServiceAccountToken(t *testing.T) { for i := range []int{0, 1} { automountServiceAccountToken := (i == 0) - sset, err := makeStatefulSet(&monitoringv1.Alertmanager{ + sset, err := makeStatefulSet(nil, &monitoringv1.Alertmanager{ Spec: monitoringv1.AlertmanagerSpec{ AutomountServiceAccountToken: &automountServiceAccountToken, }, @@ -1215,7 +1216,7 @@ func TestClusterLabel(t *testing.T) { }, } - ss, err := makeStatefulSetSpec(&a, defaultTestConfig, nil) + ss, err := makeStatefulSetSpec(nil, &a, defaultTestConfig, nil) require.NoError(t, err) args := ss.Template.Spec.Containers[0].Args @@ -1228,6 +1229,106 @@ func TestClusterLabel(t *testing.T) { } } +func TestMakeStatefulSetSpecTemplatesUniqueness(t *testing.T) { + replicas := int32(1) + tt := []struct { + a monitoringv1.Alertmanager + expectedSourceCount int + }{ + { + a: monitoringv1.Alertmanager{ + Spec: monitoringv1.AlertmanagerSpec{ + Replicas: &replicas, + AlertmanagerConfiguration: &monitoringv1.AlertmanagerConfiguration{ + Templates: []monitoringv1.SecretOrConfigMap{ + { + Secret: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "template1", + }, + Key: "template1.tmpl", + }, + }, + { + Secret: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "template2", + }, + Key: "template1.tmpl", + }, + }, + }, + }, + }, + }, + expectedSourceCount: 1, + }, + { + a: monitoringv1.Alertmanager{ + Spec: monitoringv1.AlertmanagerSpec{ + Replicas: &replicas, + AlertmanagerConfiguration: &monitoringv1.AlertmanagerConfiguration{ + Templates: []monitoringv1.SecretOrConfigMap{ + { + Secret: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "template1", + }, + Key: "template1.tmpl", + }, + }, + { + Secret: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "template2", + }, + Key: "template2.tmpl", + }, + }, + }, + }, + }, + }, + expectedSourceCount: 2, + }, + { + a: monitoringv1.Alertmanager{ + Spec: monitoringv1.AlertmanagerSpec{ + Replicas: &replicas, + AlertmanagerConfiguration: &monitoringv1.AlertmanagerConfiguration{ + Templates: []monitoringv1.SecretOrConfigMap{ + { + Secret: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "template1", + }, + Key: "template1.tmpl", + }, + }, + }, + }, + }, + }, + expectedSourceCount: 1, + }, + } + + for _, test := range tt { + statefulSpec, err := makeStatefulSetSpec(log.NewNopLogger(), &test.a, defaultTestConfig, nil) + if err != nil { + t.Fatal(err) + } + volumes := statefulSpec.Template.Spec.Volumes + for _, volume := range volumes { + if volume.Name == "notification-templates" { + if len(volume.VolumeSource.Projected.Sources) != test.expectedSourceCount { + t.Fatalf("expected %d sources, got %d", test.expectedSourceCount, len(volume.VolumeSource.Projected.Sources)) + } + } + } + } +} + func containsString(sub string) func(string) bool { return func(x string) bool { return strings.Contains(x, sub)