Fix Stack Monitoring with custom certificate without CA (#5310) (#5319)

Stack Monitoring incorrectly assumed that if the monitored Elastic resource has TLS enabled, there is a CA to configure in the input section of the Metricbeat config. This overlooked that you can have a certificate issued by a well-known CA, so you don't always provide a CA when TLS is enabled. This is fixed by differentiating between isSSL/isTLS and HasCA.

Author: thbkrkr

pkg/controller/common/certificates/ca.go

@@ -5,6 +5,7 @@
 package certificates
 
 import (
+	"context"
 	"crypto"
 	cryptorand "crypto/rand"
 	"crypto/rsa"
@@ -14,6 +15,11 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
+
+	"github.com/elastic/cloud-on-k8s/pkg/controller/common/name"
+	"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
 )
 
 var (
@@ -124,3 +130,18 @@ func (c *CA) CreateCertificate(
 
 	return certData, err
 }
+
+// PublicCertsHasCACert returns true if an Elastic resource has a CA (ca.crt) in its public HTTP certs secret given its namer,
+// namespace and name.
+func PublicCertsHasCACert(client k8s.Client, namer name.Namer, namespace string, name string) (bool, error) {
+	certsNsn := types.NamespacedName{
+		Name:      PublicCertsSecretName(namer, name),
+		Namespace: namespace,
+	}
+	var certsSecret corev1.Secret
+	if err := client.Get(context.Background(), certsNsn, &certsSecret); err != nil {
+		return false, err
+	}
+	_, ok := certsSecret.Data[CAFileName]
+	return ok, nil
+}

pkg/controller/common/certificates/ca_test.go

@@ -15,6 +15,11 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1"
+	"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
 )
 
 func init() {
@@ -85,3 +90,59 @@ func TestNewSelfSignedCA(t *testing.T) {
 	require.Equal(t, testRSAPrivateKey, ca.PrivateKey)
 	require.True(t, ca.Cert.NotBefore.Before(time.Now().Add(2*time.Hour)))
 }
+
+func Test_PublicCertsHasCACert(t *testing.T) {
+	tests := []struct {
+		name    string
+		secret  corev1.Secret
+		wantErr bool
+		want   bool
+	}{
+		{
+			name: "Happy path: with ca.crt",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "ns",
+					Name:      "c1-es-http-certs-public",
+				},
+				Data: map[string][]byte{
+					CAFileName: []byte("..."),
+					CertFileName: []byte("..."),
+				},
+			},
+			wantErr: false,
+			want: true,
+		},
+		{
+			name: "Happy path, without ca.crt",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "ns",
+					Name:      "c1-es-http-certs-public",
+				},
+				Data: map[string][]byte{
+					CertFileName: []byte("..."),
+				},
+			},
+			wantErr: false,
+			want: false,
+		},
+		{
+			name: "Error if no certs secret",
+			wantErr: true,
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := PublicCertsHasCACert(k8s.NewFakeClient(&tt.secret), esv1.ESNamer, "ns", "c1")
+			if (err != nil) != tt.wantErr {
+				t.Errorf("PublicCertsHasCACert() error = %v, wantErr = %v", err, tt.wantErr)
+			}
+			if got != tt.want {
+				t.Errorf("PublicCertsHasCACert() got = %v, want = %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/controller/common/stackmon/config.go

@@ -160,8 +160,8 @@ type inputConfigData struct {
 	Username string
 	Password string
 	IsSSL    bool
-	SSLPath  string
-	SSLMode  string
+	HasCA    bool
+	CAPath   string
 }
 
 // buildMetricbeatBaseConfig builds the base configuration for Metricbeat with the Elasticsearch or Kibana modules used
@@ -181,23 +181,32 @@ func buildMetricbeatBaseConfig(
 		return "", nil, err
 	}
 
+	hasCA := false
+	if isTLS {
+		var err error
+		hasCA, err = certificates.PublicCertsHasCACert(client, namer, nsn.Namespace, nsn.Name)
+		if err != nil {
+			return "", nil, err
+		}
+	}
+
 	configData := inputConfigData{
-		URL:      url,
 		Username: user.MonitoringUserName,
 		Password: password,
-		IsSSL:    isTLS,
+		URL:      url,   // Metricbeat in the sidecar connects to the monitored resource using `localhost`
+		IsSSL:    isTLS, // enable SSL configuration based on whether the monitored resource has TLS enabled
+		HasCA:    hasCA, // the CA is optional to support custom certificate issued by a well-known CA, so without provided CA to configure
 	}
 
 	var caVolume volume.VolumeLike
-	if configData.IsSSL {
+	if configData.HasCA {
 		caVolume = volume.NewSecretVolumeWithMountPath(
 			certificates.PublicCertsSecretName(namer, nsn.Name),
 			fmt.Sprintf("%s-local-ca", string(associationType)),
 			fmt.Sprintf("/mnt/elastic-internal/%s/%s/%s/certs", string(associationType), nsn.Namespace, nsn.Name),
 		)
 
-		configData.SSLPath = filepath.Join(caVolume.VolumeMount().MountPath, certificates.CAFileName)
-		configData.SSLMode = "certificate"
+		configData.CAPath = filepath.Join(caVolume.VolumeMount().MountPath, certificates.CAFileName)
 	}
 
 	// render the config template with the config data

pkg/controller/common/stackmon/config_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/elastic/cloud-on-k8s/pkg/controller/common/name"
@@ -18,53 +19,86 @@ import (
 
 func TestBuildMetricbeatBaseConfig(t *testing.T) {
 	tests := []struct {
-		name       string
-		isTLS      bool
-		baseConfig string
+		name        string
+		isTLS       bool
+		certsSecret *corev1.Secret
+		hasCA       bool
+		baseConfig  string
 	}{
 		{
-			name:  "with tls",
+			name:  "with TLS and a CA",
 			isTLS: true,
+			certsSecret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "name-es-http-certs-public", Namespace: "namespace"},
+				Data: map[string][]byte{
+					"tls.crt": []byte("1234567890"),
+					"ca.crt":  []byte("1234567890"),
+				},
+			},
 			baseConfig: `
 				hosts: ["scheme://localhost:1234"]
 				username: elastic-internal-monitoring
 				password: 1234567890
-				ssl.certificate_authorities: ["/mnt/elastic-internal/xx-monitoring/namespace/name/certs/ca.crt"]
+				ssl.enabled: true
+				ssl.verification_mode: "certificate"
+				ssl.certificate_authorities: ["/mnt/elastic-internal/xx-monitoring/namespace/name/certs/ca.crt"]`,
+		},
+		{
+			name:  "with TLS and no CA",
+			isTLS: true,
+			certsSecret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "name-es-http-certs-public", Namespace: "namespace"},
+				Data: map[string][]byte{
+					"tls.crt": []byte("1234567890"),
+				},
+			},
+			baseConfig: `
+				hosts: ["scheme://localhost:1234"]
+				username: elastic-internal-monitoring
+				password: 1234567890
+				ssl.enabled: true
 				ssl.verification_mode: "certificate"`,
 		},
 		{
-			name:  "without tls",
+			name:  "without TLS",
 			isTLS: false,
 			baseConfig: `
 				hosts: ["scheme://localhost:1234"]
 				username: elastic-internal-monitoring
-				password: 1234567890`,
+				password: 1234567890
+				ssl.enabled: false
+				ssl.verification_mode: "certificate"`,
 		},
 	}
-
 	baseConfigTemplate := `
 				hosts: ["{{ .URL }}"]
 				username: {{ .Username }}
 				password: {{ .Password }}
-				{{- if .IsSSL }}
-				ssl.certificate_authorities: ["{{ .SSLPath }}"]
-				ssl.verification_mode: "{{ .SSLMode }}"
+				ssl.enabled: {{ .IsSSL }}
+				ssl.verification_mode: "certificate"
+				{{- if .HasCA }}
+				ssl.certificate_authorities: ["{{ .CAPath }}"]
 				{{- end }}`
-	sampleURL := "scheme://localhost:1234"
 
-	fakeClient := k8s.NewFakeClient(&corev1.Secret{
+	sampleURL := "scheme://localhost:1234"
+	internalUsersSecret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{Name: "name-es-internal-users", Namespace: "namespace"},
 		Data:       map[string][]byte{"elastic-internal-monitoring": []byte("1234567890")},
-	})
+	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
+			initObjects := []runtime.Object{internalUsersSecret}
+			if tc.certsSecret != nil {
+				initObjects = append(initObjects, tc.certsSecret)
+			}
+			fakeClient := k8s.NewFakeClient(initObjects...)
 			baseConfig, _, err := buildMetricbeatBaseConfig(
 				fakeClient,
 				"xx-monitoring",
 				types.NamespacedName{Namespace: "namespace", Name: "name"},
 				types.NamespacedName{Namespace: "namespace", Name: "name"},
-				name.NewNamer("xx"),
+				name.NewNamer("es"),
 				sampleURL,
 				tc.isTLS,
 				baseConfigTemplate,

pkg/controller/elasticsearch/stackmon/metricbeat.tpl.yml

@@ -17,9 +17,13 @@ metricbeat.modules:
     hosts: ["{{ .URL }}"]
     username: {{ .Username }}
     password: {{ .Password }}
-    {{- if .IsSSL }}
-    ssl.certificate_authorities: ["{{ .SSLPath }}"]
-    ssl.verification_mode: "{{ .SSLMode }}"
+    ssl.enabled: {{ .IsSSL }}
+    # The ssl verification_mode is set to `certificate` in the config template to verify that the certificate is signed by a trusted authority,
+    # but does not perform any hostname verification. This is used when SSL is enabled with or without CA, to support self-signed certificate
+    # with a custom CA or custom certificates with or without a CA that most likely are not issued for `localhost`.
+    ssl.verification_mode: "certificate"
+    {{- if .HasCA }}
+    ssl.certificate_authorities: ["{{ .CAPath }}"]
     {{- end }}
 
 processors:

pkg/controller/kibana/stackmon/metricbeat.tpl.yml

@@ -9,9 +9,13 @@ metricbeat.modules:
     hosts: ["{{ .URL }}"]
     username: {{ .Username }}
     password: {{ .Password }}
-    {{- if .IsSSL }}
-    ssl.certificate_authorities: ["{{ .SSLPath }}"]
-    ssl.verification_mode: "{{ .SSLMode }}"
+    ssl.enabled: {{ .IsSSL }}
+    # The ssl verification_mode is set to `certificate` in the config template to verify that the certificate is signed by a trusted authority,
+    # but does not perform any hostname verification. This is used when SSL is enabled with or without CA, to support self-signed certificate
+    # with a custom CA or custom certificates with or without a CA that most likely are not issued for `localhost`.
+    ssl.verification_mode: "certificate"
+    {{- if .HasCA }}
+    ssl.certificate_authorities: ["{{ .CAPath }}"]
     {{- end }}
 
 processors:

pkg/controller/kibana/stackmon/sidecar_test.go

@@ -48,9 +48,19 @@ func TestWithMonitoring(t *testing.T) {
 	}
 	fakeEsHTTPCertSecret := corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{Name: "sample-es-http-certs-public", Namespace: "aerospace"},
-		Data:       map[string][]byte{"ca.crt": []byte("7H1515N074r341C3r71F1C473")},
+		Data: map[string][]byte{
+			"tls.crt": []byte("7H1515N074r341C3r71F1C473"),
+			"ca.crt":  []byte("7H1515N074r341C3r71F1C473"),
+		},
+	}
+	fakeKbHTTPCertSecret := corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{Name: "sample-kb-http-certs-public", Namespace: "aerospace"},
+		Data: map[string][]byte{
+			"tls.crt": []byte("7H1515N074r341C3r71F1C473"),
+			"ca.crt":  []byte("7H1515N074r341C3r71F1C473"),
+		},
 	}
-	fakeClient := k8s.NewFakeClient(&fakeElasticUserSecret, &fakeMetricsBeatUserSecret, &fakeLogsBeatUserSecret, &fakeEsHTTPCertSecret)
+	fakeClient := k8s.NewFakeClient(&fakeElasticUserSecret, &fakeMetricsBeatUserSecret, &fakeLogsBeatUserSecret, &fakeEsHTTPCertSecret, &fakeKbHTTPCertSecret)
 
 	monitoringAssocConf := commonv1.AssociationConf{
 		AuthSecretName: "sample-observability-monitoring-beat-es-mon-user",