1
0
mirror of https://github.com/coreos/prometheus-operator.git synced 2026-02-05 15:46:31 +01:00

fix: handle duplicate sources (#6006)

* fix: handle duplicate sources

Skip appending sources with the same key for ConfigMaps and Secrets in
the AlertmanagerConfiguration.

Fixes: #5905

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>

* fixup! fix: handle duplicate sources

---------

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
This commit is contained in:
Pranshu Srivastava
2023-11-25 01:21:39 +05:30
committed by GitHub
parent 88eca6a97b
commit 9e9a7536a8
3 changed files with 154 additions and 40 deletions

View File

@@ -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)
}

View File

@@ -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{

View File

@@ -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)