Repository: VictoriaMetrics/operator

Issue:
vmalert: notifier's tls assets missing from vmalert-tls-assets
After ugpgrading vmoperator from `v0.59.2` to `v0.60.1`, any notifiers referencing a secret for a tls cert is no longer copied into `tls-assets-vmalert`

Example config:
```
notifiers:
- tlsConfig:
    ca:
      secret:
        key: ca.crt
        name: vmalert-tls
    cert:
      secret:
        key: tls.crt
        name: vmalert-tls
    insecureSkipVerify: true
    keySecret:
      key: tls.key
      name: vmalert-tls
  url: https://alertmanager-0.example.com
```

Hints from issue comments:
@AndrewChubatiuk: fix is available in operator release 0.60.2, operator chart release 0.50.3 and k8s stack release 0.55.2

Solution patch:
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 6e195fe8..2c5b73e9 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -14,6 +14,7 @@ aliases:
 ## tip
 
 * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): clear `status.reason` when transitioning to `expanding` state. This makes less confusing as resource in `expanding` state is no longer affected by an error displayed in `status.reason`. See [#1426](https://github.com/VictoriaMetrics/operator/issues/1426).
+* BUGFIX: [vmalert](https://docs.victoriametrics.com/operator/resources/vmalert/): properly load TLS secrets into VMAlert secret. See [#1437](https://github.com/VictoriaMetrics/operator/issues/1437).
 
 ## [v0.60.1](https://github.com/VictoriaMetrics/operator/releases/tag/v0.60.1)
 
diff --git a/internal/controller/operator/factory/vmalert/vmalert.go b/internal/controller/operator/factory/vmalert/vmalert.go
index aa2241d3..9473a29e 100644
--- a/internal/controller/operator/factory/vmalert/vmalert.go
+++ b/internal/controller/operator/factory/vmalert/vmalert.go
@@ -379,32 +379,18 @@ func buildAuthArgs(args []string, namespace, flagPrefix string, cfg vmv1beta1.HT
 		args = append(args, fmt.Sprintf("-%s.basicAuth.username=%s", flagPrefix, secret))
 	}
 	if cfg.TLSConfig != nil {
-		if len(cfg.TLSConfig.CAFile) > 0 {
-			args = append(args, fmt.Sprintf("-%s.tlsCAFile=%s", flagPrefix, cfg.TLSConfig.CAFile))
-		} else if len(cfg.TLSConfig.CA.PrefixedName()) > 0 {
-			file, err := ac.LoadPathFromSecretOrConfigMap(build.TLSAssetsResourceKind, namespace, &cfg.TLSConfig.CA)
-			if err != nil {
-				return nil, err
-			}
-			args = append(args, fmt.Sprintf("-%s.tlsCAFile=%s", flagPrefix, file))
+		creds, err := ac.BuildTLSCreds(namespace, cfg.TLSConfig)
+		if err != nil {
+			return nil, fmt.Errorf("failed to load notifier TLS credentials: %w", err)
 		}
-		if len(cfg.TLSConfig.CertFile) > 0 {
-			args = append(args, fmt.Sprintf("-%s.tlsCertFile=%s", flagPrefix, cfg.TLSConfig.CertFile))
-		} else if len(cfg.TLSConfig.Cert.PrefixedName()) > 0 {
-			file, err := ac.LoadPathFromSecretOrConfigMap(build.TLSAssetsResourceKind, namespace, &cfg.TLSConfig.Cert)
-			if err != nil {
-				return nil, err
-			}
-			args = append(args, fmt.Sprintf("-%s.tlsCertFile=%s", flagPrefix, file))
+		if len(creds.CAFile) > 0 {
+			args = append(args, fmt.Sprintf("-%s.tlsCAFile=%s", flagPrefix, creds.CAFile))
 		}
-		if len(cfg.TLSConfig.KeyFile) > 0 {
-			args = append(args, fmt.Sprintf("-%s.tlsKeyFile=%s", flagPrefix, cfg.TLSConfig.KeyFile))
-		} else if cfg.TLSConfig.KeySecret != nil {
-			file, err := ac.LoadPathFromSecret(build.TLSAssetsResourceKind, namespace, cfg.TLSConfig.KeySecret)
-			if err != nil {
-				return nil, err
-			}
-			args = append(args, fmt.Sprintf("-%s.tlsKeyFile=%s", flagPrefix, file))
+		if len(creds.CertFile) > 0 {
+			args = append(args, fmt.Sprintf("-%s.tlsCertFile=%s", flagPrefix, creds.CertFile))
+		}
+		if len(creds.KeyFile) > 0 {
+			args = append(args, fmt.Sprintf("-%s.tlsKeyFile=%s", flagPrefix, creds.KeyFile))
 		}
 		if cfg.TLSConfig.ServerName != "" {
 			args = append(args, fmt.Sprintf("-%s.tlsServerName=%s", flagPrefix, cfg.TLSConfig.ServerName))
@@ -578,8 +564,6 @@ func buildNotifiersArgs(cr *vmv1beta1.VMAlert, ac *build.AssetsCache) ([]string,
 	oauth2Scopes := remoteFlag{flagSetting: "-notifier.oauth2.scopes="}
 	oauth2TokenURL := remoteFlag{flagSetting: "-notifier.oauth2.tokenUrl="}
 
-	pathPrefix := path.Join(tlsAssetsDir, cr.Namespace)
-
 	for _, nt := range notifierTargets {
 		url.flagSetting += fmt.Sprintf("%s,", nt.URL)
 
@@ -587,28 +571,20 @@ func buildNotifiersArgs(cr *vmv1beta1.VMAlert, ac *build.AssetsCache) ([]string,
 		var inSecure bool
 		ntTLS := nt.TLSConfig
 		if ntTLS != nil {
-			if ntTLS.CAFile != "" {
-				caPath = ntTLS.CAFile
-			} else if ntTLS.CA.PrefixedName() != "" {
-				caPath = ntTLS.BuildAssetPath(pathPrefix, ntTLS.CA.PrefixedName(), ntTLS.CA.Key())
+			creds, err := ac.BuildTLSCreds(cr.Namespace, ntTLS)
+			if err != nil {
+				return nil, fmt.Errorf("failed to load notifier TLS credentials: %w", err)
 			}
-			if caPath != "" {
+			if creds.CAFile != "" {
+				caPath = creds.CAFile
 				tlsCAs.isNotNull = true
 			}
-			if ntTLS.CertFile != "" {
-				certPath = ntTLS.CertFile
-			} else if ntTLS.Cert.PrefixedName() != "" {
-				certPath = ntTLS.BuildAssetPath(pathPrefix, ntTLS.Cert.PrefixedName(), ntTLS.Cert.Key())
-			}
-			if certPath != "" {
+			if creds.CertFile != "" {
+				certPath = creds.CertFile
 				tlsCerts.isNotNull = true
 			}
-			if ntTLS.KeyFile != "" {
-				keyPath = ntTLS.KeyFile
-			} else if ntTLS.KeySecret != nil {
-				keyPath = ntTLS.BuildAssetPath(pathPrefix, ntTLS.KeySecret.Name, ntTLS.KeySecret.Key)
-			}
-			if keyPath != "" {
+			if creds.KeyFile != "" {
+				keyPath = creds.KeyFile
 				tlsKeys.isNotNull = true
 			}
 			if ntTLS.InsecureSkipVerify {



Test patch:
diff --git a/internal/controller/operator/factory/vmalert/vmalert_test.go b/internal/controller/operator/factory/vmalert/vmalert_test.go
index bf5c3be3..5144fe3a 100644
--- a/internal/controller/operator/factory/vmalert/vmalert_test.go
+++ b/internal/controller/operator/factory/vmalert/vmalert_test.go
@@ -11,15 +11,16 @@ import (
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/utils/ptr"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1"
 	"github.com/VictoriaMetrics/operator/internal/config"
+	"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/build"
 	"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/k8stools"
 )
 
@@ -27,12 +28,10 @@ func TestCreateOrUpdate(t *testing.T) {
 	tests := []struct {
 		name              string
 		cr                *vmv1beta1.VMAlert
-		c                 *config.BaseOperatorConf
 		cmNames           []string
-		want              reconcile.Result
 		wantErr           bool
 		predefinedObjects []runtime.Object
-		validator         func(vma *appsv1.Deployment) error
+		validator         func(*appsv1.Deployment, *corev1.Secret) error
 	}{
 		{
 			name: "base-spec-gen",
@@ -50,7 +49,6 @@ func TestCreateOrUpdate(t *testing.T) {
 					},
 				},
 			},
-			c: config.MustGetBaseConfig(),
 			predefinedObjects: []runtime.Object{
 				&appsv1.Deployment{
 					ObjectMeta: metav1.ObjectMeta{
@@ -96,7 +94,6 @@ func TestCreateOrUpdate(t *testing.T) {
 					ExternalLabels: map[string]string{"label1": "value1", "label2": "value-2"},
 				},
 			},
-			c: config.MustGetBaseConfig(),
 			predefinedObjects: []runtime.Object{
 				&appsv1.Deployment{
 					ObjectMeta: metav1.ObjectMeta{
@@ -114,10 +111,9 @@ func TestCreateOrUpdate(t *testing.T) {
 					},
 				},
 			},
-
-			validator: func(vma *appsv1.Deployment) error {
+			validator: func(d *appsv1.Deployment, s *corev1.Secret) error {
 				var foundOk bool
-				for _, cnt := range vma.Spec.Template.Spec.Containers {
+				for _, cnt := range d.Spec.Template.Spec.Containers {
 					if cnt.Name == "vmalert" {
 						args := cnt.Args
 						for _, arg := range args {
@@ -197,7 +193,7 @@ func TestCreateOrUpdate(t *testing.T) {
 									ConfigMap: &corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "datasource-tls"}, Key: "ca"},
 								},
 								Cert: vmv1beta1.SecretOrConfigMap{
-									ConfigMap: &corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "datasource-tls"}, Key: "ca"},
+									ConfigMap: &corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "datasource-tls"}, Key: "cert"},
 								},
 								KeySecret: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "datasource-tls"}, Key: "key"},
 							},
@@ -205,7 +201,6 @@ func TestCreateOrUpdate(t *testing.T) {
 					},
 				},
 			},
-			c: config.MustGetBaseConfig(),
 			predefinedObjects: []runtime.Object{
 				&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}},
 				&corev1.Secret{
@@ -238,6 +233,18 @@ func TestCreateOrUpdate(t *testing.T) {
 					},
 				},
 			},
+			validator: func(d *appsv1.Deployment, s *corev1.Secret) error {
+				if _, ok := s.Data["default_configmap_datasource-tls_ca"]; !ok {
+					return fmt.Errorf("failed to find expected TLS CA")
+				}
+				if _, ok := s.Data["default_configmap_datasource-tls_cert"]; !ok {
+					return fmt.Errorf("failed to find expected TLS cert")
+				}
+				if _, ok := s.Data["default_datasource-tls_key"]; !ok {
+					return fmt.Errorf("failed to find TLS key")
+				}
+				return nil
+			},
 		},
 		{
 			name: "with-notifiers-tls",
@@ -289,7 +296,6 @@ func TestCreateOrUpdate(t *testing.T) {
 					},
 				},
 			},
-			c: config.MustGetBaseConfig(),
 			predefinedObjects: []runtime.Object{
 				&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}},
 				&corev1.Secret{
@@ -367,7 +373,6 @@ func TestCreateOrUpdate(t *testing.T) {
 					},
 				},
 			},
-			c: config.MustGetBaseConfig(),
 			predefinedObjects: []runtime.Object{
 				&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}},
 				&corev1.Secret{
@@ -420,7 +425,6 @@ func TestCreateOrUpdate(t *testing.T) {
 					},
 				},
 			},
-			c: config.MustGetBaseConfig(),
 			predefinedObjects: []runtime.Object{
 				&corev1.Secret{
 					ObjectMeta: metav1.ObjectMeta{
@@ -460,9 +464,16 @@ func TestCreateOrUpdate(t *testing.T) {
 			if tt.validator != nil {
 				var generatedDeploment appsv1.Deployment
 				if err := fclient.Get(ctx, types.NamespacedName{Namespace: tt.cr.Namespace, Name: tt.cr.PrefixedName()}, &generatedDeploment); err != nil {
-					t.Fatalf("cannot find generated deployment: %v, err: %v", tt.cr.PrefixedName(), err)
+					t.Fatalf("cannot find generated deployment=%q, err: %v", tt.cr.PrefixedName(), err)
+				}
+				var generatedTLSSecret corev1.Secret
+				tlsSecretName := build.ResourceName(build.TLSAssetsResourceKind, tt.cr)
+				if err := fclient.Get(ctx, types.NamespacedName{Namespace: tt.cr.Namespace, Name: tlsSecretName}, &generatedTLSSecret); err != nil {
+					if !errors.IsNotFound(err) {
+						t.Fatalf("unexpected error during attempt to get tls secret=%q, err: %v", build.ResourceName(build.TLSAssetsResourceKind, tt.cr), err)
+					}
 				}
-				if err := tt.validator(&generatedDeploment); err != nil {
+				if err := tt.validator(&generatedDeploment, &generatedTLSSecret); err != nil {
 					t.Fatalf("unexpected error at deployment validation: %v", err)
 				}
 			}