Repository: elastic/cloud-on-k8s

Issue:
Elastic operator renewing Elasticsearch internal certificates breaks Stack Monitoring
<!--

** Please read the guidelines below. **

1.   GitHub is reserved for bug reports and feature requests. The best place to
     ask a general question is at the Elastic [forums](https://discuss.elastic.co).
     GitHub is not the place for general questions.
    
2.  Please fill out EITHER the feature request block or the bug report block
    below, and delete the other block.

-->

<!-- Bug report -->
## Bug Report
**What did you do?**

We have deployed Elasticsearch on Kubernetes with ECK. For monitoring we have deployed a monitoring cluster and use stack monitoring with beats to monitor our production cluster with the monitoring cluster. This works fine before ECK renewed the internal certificates the Elasticsearch cluster uses for internal communication.

**What did you expect to see?**

logs and metrics from the production cluster before and after the certificate change in stack monitoring in the Monitoring cluster.

**What did you see instead? Under which circumstances?**

Only logs from the production cluster after the certificate change, metricbeat stopped sending metrics.

**Environment**

* ECK version: 1.9.0
* Elasticsearch version: 7.16.3

* Kubernetes information:

	* On premise
	* Kubernetes distribution: Rancher
	* Kubernetes version: v1.21.9

* Logs:
```
Elasticsearch nodes gave this message: 

`[2022-03-07T12:47:39,404][WARN ][o.e.h.AbstractHttpServerTransport] [elasticsearch-es-master-2] caught exception while handling client http traffic, closing connection Netty4HttpChannel{localAddress=/127.0.0.1:9200, remoteAddress=/127.0.0.1:38980}
io.netty.handler.codec.DecoderException: javax.net.ssl.SSLHandshakeException: Received fatal alert: bad_certificate`

Metricbeat gave this error:
`2022-03-07T15:33:38.709Z        ERROR   module/wrapper.go:259   Error fetching data for metricset elasticsearch.node_stats: error making http request: Get "https://localhost:9200/_nodes/_local/stats": x509: certificate signed by unknown authority (possibly because of "crypto/rsa: verification error" while trying to verify candidate authority certificate "elasticsearch-http")`

I saw that the new internal CA was mounted in the metricbeat container, but I had to restart the metricbeat container to fix this issue. Filebeats certificate verification mode is on [certificate](https://www.elastic.co/guide/en/beats/filebeat/current/configuration-ssl.html#client-verification-mode)
```


Discussion:
@thbkrkr: Yes, I see the problem. The new certificates (from the monitored cluster and from the monitoring cluster) are well propagated in the Metricbeat container. Metricbeat uses a persistent connection so as long as the connection is established, it works, even if the certificate has expired. As soon as the connection is closed, Metricbeat tries to reconnect with the old certificate without considering the new certificate and got the PKI error `x509: certificate signed by unknown authority`.

Temporary workaround: kill the Beat process to recreate the Beat container (`kubectl exec $esPod -c metricbeat -- kill 1`).
@milanage: @thbkrkr 
Issue still happens on ECK 2.6.1 + Stack 8.8.1.  It looks like https://github.com/elastic/beats/pull/34416 does not really help.
What's our plan to fix this issue? @VCCPlindsten: This still happens on ECK 2.10 & Stack 8.12.0
@KannappanSomu: +1@snvk2889: Hi Team,

This issue is seen in ECK 2.16.0 version + stack version 8.16.1.
However the workaround actually helped for now.

Solution patch:
diff --git a/pkg/controller/common/stackmon/config.go b/pkg/controller/common/stackmon/config.go
index 8c9c52a8a..a5fa19a62 100644
--- a/pkg/controller/common/stackmon/config.go
+++ b/pkg/controller/common/stackmon/config.go
@@ -24,6 +24,7 @@ import (
 	"github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/metadata"
 	"github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/settings"
 	"github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/stackmon/monitoring"
+	"github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/version"
 	"github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/volume"
 	"github.com/elastic/cloud-on-k8s/v3/pkg/utils/k8s"
 )
@@ -40,6 +41,7 @@ func newBeatConfig(
 	ctx context.Context,
 	client k8s.Client,
 	beatName string,
+	imageVersion string,
 	resource monitoring.HasMonitoring,
 	associations []commonv1.Association,
 	baseConfig string,
@@ -52,7 +54,7 @@ func newBeatConfig(
 	assoc := associations[0]
 
 	// build the output section of the beat configuration file
-	outputCfg, caVolume, err := buildOutputConfig(ctx, client, assoc)
+	outputCfg, caVolume, err := buildOutputConfig(ctx, client, assoc, imageVersion)
 	if err != nil {
 		return beatConfig{}, err
 	}
@@ -112,7 +114,7 @@ func newBeatConfig(
 	}, err
 }
 
-func buildOutputConfig(ctx context.Context, client k8s.Client, assoc commonv1.Association) (map[string]interface{}, volume.VolumeLike, error) {
+func buildOutputConfig(ctx context.Context, client k8s.Client, assoc commonv1.Association, imageVersion string) (map[string]interface{}, volume.VolumeLike, error) {
 	credentials, err := association.ElasticsearchAuthSettings(ctx, client, assoc)
 	if err != nil {
 		return nil, volume.SecretVolume{}, err
@@ -132,6 +134,19 @@ func buildOutputConfig(ctx context.Context, client k8s.Client, assoc commonv1.As
 	// and therefore not being valid for the internal URL.
 	outputConfig["ssl.verification_mode"] = "certificate"
 
+	v, err := version.Parse(imageVersion)
+	if err != nil {
+		return nil, nil, err
+	}
+	// Reloading of certificates is only supported for Beats >= 8.8.0.
+	if v.GE(version.MinFor(8, 8, 0)) {
+		// Allow beats to reload when the ssl certificate changes (renewals)
+		outputConfig["ssl.restart_on_cert_change"] = map[string]interface{}{
+			"enabled": true,
+			"period":  "1m",
+		}
+	}
+
 	caDirPath := fmt.Sprintf(
 		"/mnt/elastic-internal/%s-association/%s/%s/certs",
 		assoc.AssociationType(), assoc.AssociationRef().Namespace, assoc.AssociationRef().NameOrSecretName(),
diff --git a/pkg/controller/common/stackmon/sidecar.go b/pkg/controller/common/stackmon/sidecar.go
index ba3b94fd7..32bd7f0c5 100644
--- a/pkg/controller/common/stackmon/sidecar.go
+++ b/pkg/controller/common/stackmon/sidecar.go
@@ -38,7 +38,7 @@ func NewMetricBeatSidecar(
 	image := container.ImageRepository(container.MetricbeatImage, imageVersion)
 	// EmptyDir volume so that MetricBeat does not write in the container image, which allows ReadOnlyRootFilesystem: true
 	emptyDir := volume.NewEmptyDirVolume("metricbeat-data", "/usr/share/metricbeat/data")
-	return NewBeatSidecar(ctx, client, "metricbeat", image, resource, monitoring.GetMetricsAssociation(resource), baseConfig, meta, caVolume, emptyDir)
+	return NewBeatSidecar(ctx, client, "metricbeat", image, imageVersion.String(), resource, monitoring.GetMetricsAssociation(resource), baseConfig, meta, caVolume, emptyDir)
 }
 
 func NewFileBeatSidecar(
@@ -57,7 +57,7 @@ func NewFileBeatSidecar(
 	image := container.ImageRepository(container.FilebeatImage, v)
 	// EmptyDir volume so that FileBeat does not write in the container image, which allows ReadOnlyRootFilesystem: true
 	emptyDir := volume.NewEmptyDirVolume("filebeat-data", "/usr/share/filebeat/data")
-	return NewBeatSidecar(ctx, client, "filebeat", image, resource, monitoring.GetLogsAssociation(resource), baseConfig, meta, additionalVolume, emptyDir)
+	return NewBeatSidecar(ctx, client, "filebeat", image, imageVersion, resource, monitoring.GetLogsAssociation(resource), baseConfig, meta, additionalVolume, emptyDir)
 }
 
 // BeatSidecar helps with building a beat sidecar container to monitor an Elastic Stack application. It focuses on
@@ -69,11 +69,11 @@ type BeatSidecar struct {
 	Volumes      []corev1.Volume
 }
 
-func NewBeatSidecar(ctx context.Context, client k8s.Client, beatName string, image string, resource monitoring.HasMonitoring,
+func NewBeatSidecar(ctx context.Context, client k8s.Client, beatName string, image string, imageVersion string, resource monitoring.HasMonitoring,
 	associations []commonv1.Association, baseConfig string, meta metadata.Metadata, additionalVolumes ...volume.VolumeLike,
 ) (BeatSidecar, error) {
 	// build the beat config
-	config, err := newBeatConfig(ctx, client, beatName, resource, associations, baseConfig, meta)
+	config, err := newBeatConfig(ctx, client, beatName, imageVersion, resource, associations, baseConfig, meta)
 	if err != nil {
 		return BeatSidecar{}, err
 	}
diff --git a/pkg/controller/kibana/stackmon/__snapshots__/sidecar_test.snap b/pkg/controller/kibana/stackmon/__snapshots__/sidecar_test.snap
index 868717631..e134a70a8 100755
--- a/pkg/controller/kibana/stackmon/__snapshots__/sidecar_test.snap
+++ b/pkg/controller/kibana/stackmon/__snapshots__/sidecar_test.snap
@@ -849,231 +849,3 @@ processors:
     - add_host_metadata: null
 
 ---
-
-[TestWithMonitoring/with_metrics_and_logs_monitoring_and_read_only_root_filesystem - 1]
-{
- "metadata": {
-  "annotations": {
-   "kibana.k8s.elastic.co/monitoring-config-hash": "1902028541"
-  },
-  "creationTimestamp": null
- },
- "spec": {
-  "automountServiceAccountToken": false,
-  "containers": [
-   {
-    "name": "kibana",
-    "resources": {},
-    "volumeMounts": [
-     {
-      "mountPath": "/usr/share/kibana/logs",
-      "name": "kibana-logs"
-     }
-    ]
-   },
-   {
-    "args": [
-     "-c",
-     "/etc/metricbeat-config/metricbeat.yml",
-     "-e"
-    ],
-    "env": [
-     {
-      "name": "POD_IP",
-      "valueFrom": {
-       "fieldRef": {
-        "apiVersion": "v1",
-        "fieldPath": "status.podIP"
-       }
-      }
-     },
-     {
-      "name": "POD_NAME",
-      "valueFrom": {
-       "fieldRef": {
-        "apiVersion": "v1",
-        "fieldPath": "metadata.name"
-       }
-      }
-     },
-     {
-      "name": "NODE_NAME",
-      "valueFrom": {
-       "fieldRef": {
-        "apiVersion": "v1",
-        "fieldPath": "spec.nodeName"
-       }
-      }
-     },
-     {
-      "name": "NAMESPACE",
-      "valueFrom": {
-       "fieldRef": {
-        "apiVersion": "v1",
-        "fieldPath": "metadata.namespace"
-       }
-      }
-     }
-    ],
-    "image": "docker.elastic.co/beats/metricbeat:7.14.0",
-    "name": "metricbeat",
-    "resources": {},
-    "volumeMounts": [
-     {
-      "mountPath": "/etc/metricbeat-config",
-      "name": "sample-metricbeat-config",
-      "readOnly": true
-     },
-     {
-      "mountPath": "/mnt/elastic-internal/kb-monitoring-association/observability/monitoring/certs",
-      "name": "kb-monitoring-03f555-ca",
-      "readOnly": true
-     },
-     {
-      "mountPath": "/mnt/elastic-internal/kb-monitoring/aerospace/sample/certs",
-      "name": "kb-monitoring-local-ca",
-      "readOnly": true
-     },
-     {
-      "mountPath": "/usr/share/metricbeat/data",
-      "name": "metricbeat-data"
-     },
-     {
-      "mountPath": "/usr/share/metricbeat/logs",
-      "name": "metricbeat-logs"
-     }
-    ]
-   },
-   {
-    "args": [
-     "-c",
-     "/etc/filebeat-config/filebeat.yml",
-     "-e"
-    ],
-    "env": [
-     {
-      "name": "POD_IP",
-      "valueFrom": {
-       "fieldRef": {
-        "apiVersion": "v1",
-        "fieldPath": "status.podIP"
-       }
-      }
-     },
-     {
-      "name": "POD_NAME",
-      "valueFrom": {
-       "fieldRef": {
-        "apiVersion": "v1",
-        "fieldPath": "metadata.name"
-       }
-      }
-     },
-     {
-      "name": "NODE_NAME",
-      "valueFrom": {
-       "fieldRef": {
-        "apiVersion": "v1",
-        "fieldPath": "spec.nodeName"
-       }
-      }
-     },
-     {
-      "name": "NAMESPACE",
-      "valueFrom": {
-       "fieldRef": {
-        "apiVersion": "v1",
-        "fieldPath": "metadata.namespace"
-       }
-      }
-     }
-    ],
-    "image": "docker.elastic.co/beats/filebeat:7.14.0",
-    "name": "filebeat",
-    "resources": {},
-    "volumeMounts": [
-     {
-      "mountPath": "/etc/filebeat-config",
-      "name": "sample-filebeat-config",
-      "readOnly": true
-     },
-     {
-      "mountPath": "/mnt/elastic-internal/kb-monitoring-association/observability/logs/certs",
-      "name": "kb-monitoring-584a4d-ca",
-      "readOnly": true
-     },
-     {
-      "mountPath": "/usr/share/filebeat/data",
-      "name": "filebeat-data"
-     },
-     {
-      "mountPath": "/usr/share/filebeat/logs",
-      "name": "filebeat-logs"
-     },
-     {
-      "mountPath": "/usr/share/kibana/logs",
-      "name": "kibana-logs"
-     }
-    ]
-   }
-  ],
-  "volumes": [
-   {
-    "emptyDir": {},
-    "name": "filebeat-data"
-   },
-   {
-    "emptyDir": {},
-    "name": "filebeat-logs"
-   },
-   {
-    "name": "kb-monitoring-03f555-ca",
-    "secret": {
-     "optional": false,
-     "secretName": "sample-es-monitoring-observability-monitoring-ca"
-    }
-   },
-   {
-    "name": "kb-monitoring-584a4d-ca",
-    "secret": {
-     "optional": false,
-     "secretName": "sample-es-logs-observability-monitoring-ca"
-    }
-   },
-   {
-    "name": "kb-monitoring-local-ca",
-    "secret": {
-     "optional": false,
-     "secretName": "sample-kb-http-certs-public"
-    }
-   },
-   {
-    "emptyDir": {},
-    "name": "kibana-logs"
-   },
-   {
-    "emptyDir": {},
-    "name": "metricbeat-data"
-   },
-   {
-    "emptyDir": {},
-    "name": "metricbeat-logs"
-   },
-   {
-    "name": "sample-filebeat-config",
-    "secret": {
-     "optional": false,
-     "secretName": "sample-kb-monitoring-filebeat-config"
-    }
-   },
-   {
-    "name": "sample-metricbeat-config",
-    "secret": {
-     "optional": false,
-     "secretName": "sample-kb-monitoring-metricbeat-config"
-    }
-   }
-  ]
- }
-}
----



Test patch:
diff --git a/pkg/controller/beat/common/stackmon/stackmon_test.go b/pkg/controller/beat/common/stackmon/stackmon_test.go
index 7f18039de..6c273a393 100644
--- a/pkg/controller/beat/common/stackmon/stackmon_test.go
+++ b/pkg/controller/beat/common/stackmon/stackmon_test.go
@@ -117,6 +117,34 @@ output:
         ssl:
             verification_mode: certificate
         username: es-user
+`
+	beatYmlGT88 := `http:
+    enabled: false
+metricbeat:
+    modules:
+        - hosts:
+            - http+unix:///var/shared/metricbeat-test-beat.sock
+          metricsets:
+            - stats
+            - state
+          module: beat
+          period: 10s
+          xpack:
+            enabled: true
+monitoring:
+    cluster_uuid: %s
+    enabled: false
+output:
+    elasticsearch:
+        hosts:
+            - %s
+        password: es-password
+        ssl:
+            restart_on_cert_change:
+                enabled: true
+                period: 1m
+            verification_mode: certificate
+        username: es-user
 `
 	standaloneBeatYML := `http:
     enabled: false
@@ -299,6 +327,22 @@ output:
 			want:    beatSidecarFixture(standaloneBeatYML),
 			wantErr: false,
 		},
+		{
+			name: "beat > 8.8 with stack monitoring enabled and valid elasticsearchRef returns properly configured sidecar",
+			args: args{
+				client: k8s.NewFakeClient(&beatFixture, &esFixture, &monitoringEsFixture, &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{Name: "es-secret-name", Namespace: "test"},
+					Data:       map[string][]byte{"es-user": []byte("es-password")},
+				}),
+				beat: func() *v1beta1.Beat {
+					beat := beatFixture.DeepCopy()
+					beat.Spec.Version = "8.8.0"
+					return beat
+				},
+			},
+			want:    beatSidecarFixture(fmt.Sprintf(beatYmlGT88, "abcd1234", "es-metrics-monitoring-url")),
+			wantErr: false,
+		},
 		{
 			name: "Beat with stack monitoring enabled and remote elasticsearchRef",
 			args: args{
@@ -349,8 +393,8 @@ output:
 				t.Errorf("MetricBeat() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
-			if !cmp.Equal(got, tt.want, cmpopts.IgnoreFields(stackmon.BeatSidecar{}, "ConfigHash")) {
-				t.Errorf("MetricBeat() = diff: %s", cmp.Diff(got, tt.want, cmpopts.IgnoreFields(stackmon.BeatSidecar{}, "ConfigHash")))
+			if !cmp.Equal(got, tt.want, cmpopts.IgnoreFields(stackmon.BeatSidecar{}, "ConfigHash"), cmpopts.IgnoreFields(corev1.Container{}, "Image")) {
+				t.Errorf("MetricBeat() = diff: %s", cmp.Diff(got, tt.want, cmpopts.IgnoreFields(stackmon.BeatSidecar{}, "ConfigHash"), cmpopts.IgnoreFields(corev1.Container{}, "Image")))
 			}
 		})
 	}
diff --git a/pkg/controller/common/stackmon/config_test.go b/pkg/controller/common/stackmon/config_test.go
index b67974ab6..71e78fbc6 100644
--- a/pkg/controller/common/stackmon/config_test.go
+++ b/pkg/controller/common/stackmon/config_test.go
@@ -25,6 +25,7 @@ func Test_newBeatConfig(t *testing.T) {
 		initObjects []client.Object
 		beatName    string
 		baseConfig  string
+		image       string
 		associated  commonv1.Associated
 	}
 	tests := []struct {
@@ -52,6 +53,7 @@ param2: value2
 					},
 				},
 				beatName: "metricbeat",
+				image:    "8.8.0",
 				associated: &esv1.Elasticsearch{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "monitored",
@@ -91,6 +93,9 @@ param2: value2
         ssl:
             certificate_authorities:
                 - /mnt/elastic-internal/es-monitoring-association/default/monitoring/certs/ca.crt
+            restart_on_cert_change:
+                enabled: true
+                period: 1m
             verification_mode: certificate
         username: default-monitored-default-monitoring-beat-es-mon-user
 param1: value1
@@ -112,6 +117,7 @@ param2: value2
 				context.Background(),
 				fakeClient,
 				tt.args.beatName,
+				tt.args.image,
 				hasMonitoring,
 				tt.args.associated.GetAssociations(),
 				tt.args.baseConfig,