Fix handling of HTTP CA (#1742)

* Improve certificate handling

* Revert to old lint config

* Update docs

* Add missing period to doc

docs/accessing-services.asciidoc

@@ -144,12 +144,13 @@ You can bring your own certificate to configure TLS to ensure that communication
 
 Create a Kubernetes secret with:
 
-- `tls.crt`: the certificate (or a chain).
+- `ca.crt`: CA certificate (optional if `tls.crt` was issued by a well-known CA).
+- `tls.crt`: the certificate.
 - `tls.key`: the private key to the first certificate in the certificate chain.
 
 [source,sh]
 ----
-kubectl create secret tls my-cert --cert tls.crt --key tls.key
+kubectl create secret generic my-cert --from-file=ca.crt=tls.crt --from-file=tls.crt=tls.crt --from-file=tls.key=tls.key
 ----
 
 Then you just have to reference the secret name in the `http.tls.certificate` section of the resource manifest.

docs/api-docs.asciidoc

@@ -220,6 +220,10 @@ _string_
 _string_
 |
 ---
+| *`caCertProvided`* +
+_bool_
+|
+---
 | *`caSecretName`* +
 _string_
 |

docs/elasticsearch-spec.asciidoc

@@ -215,8 +215,9 @@ spec:
 === Custom HTTP certificate
 
 You can provide your own CA and certificates instead of the self-signed certificate to connect to Elasticsearch via HTTPS using a Kubernetes secret.
+The certificate must be stored under `tls.crt` and the private key must be stored under `tls.key`. If your certificate was not issued by a well-known CA, you must include the trust chain under `ca.crt` as well.
 
-You need to reference the name of a secret that contains a TLS private key and a certificate (or a chain), in the `spec.http.tls.certificate` section.
+You need to reference the name of a secret that contains a TLS private key and a certificate (and optionally, a trust chain), in the `spec.http.tls.certificate` section.
 
 [source,yaml]
 ----
@@ -231,8 +232,8 @@ This is an example on how create a Kubernetes TLS secret with a self-signed cert
 
 [source,sh]
 ----
-$ openssl req -x509 -newkey rsa:4096 -keyout tls.key -out tls.crt -days 365 -nodes
-$ kubectl create secret tls my-cert --cert tls.crt --key tls.key
+$ openssl req -x509 -sha256 -nodes -newkey rsa:4096 -days 365 -subj "/CN=quickstart-es-http" -addext "subjectAltName=DNS:quickstart-es-http.default.svc" -keyout tls.key -out tls.crt
+$ kubectl create secret generic my-cert --from-file=ca.crt=tls.crt --from-file=tls.crt=tls.crt --from-file=tls.key=tls.key
 ----
 
 [id="{p}-es-secure-settings"]

pkg/apis/common/v1alpha1/association.go

@@ -41,63 +41,71 @@ type Associator interface {
 type AssociationConf struct {
 	AuthSecretName string `json:"authSecretName"`
 	AuthSecretKey  string `json:"authSecretKey"`
+	CACertProvided bool   `json:"caCertProvided"`
 	CASecretName   string `json:"caSecretName"`
 	URL            string `json:"url"`
 }
 
 // IsConfigured returns true if all the fields are set.
-func (esac *AssociationConf) IsConfigured() bool {
-	return esac.AuthIsConfigured() && esac.CAIsConfigured() && esac.URLIsConfigured()
+func (ac *AssociationConf) IsConfigured() bool {
+	return ac.AuthIsConfigured() && ac.CAIsConfigured() && ac.URLIsConfigured()
 }
 
 // AuthIsConfigured returns true if all the auth fields are set.
-func (esac *AssociationConf) AuthIsConfigured() bool {
-	if esac == nil {
+func (ac *AssociationConf) AuthIsConfigured() bool {
+	if ac == nil {
 		return false
 	}
-	return esac.AuthSecretName != "" && esac.AuthSecretKey != ""
+	return ac.AuthSecretName != "" && ac.AuthSecretKey != ""
 }
 
 // CAIsConfigured returns true if the CA field is set.
-func (esac *AssociationConf) CAIsConfigured() bool {
-	if esac == nil {
+func (ac *AssociationConf) CAIsConfigured() bool {
+	if ac == nil {
 		return false
 	}
-	return esac.CASecretName != ""
+	return ac.CASecretName != ""
 }
 
 // URLIsConfigured returns true if the URL field is set.
-func (esac *AssociationConf) URLIsConfigured() bool {
-	if esac == nil {
+func (ac *AssociationConf) URLIsConfigured() bool {
+	if ac == nil {
 		return false
 	}
-	return esac.URL != ""
+	return ac.URL != ""
 }
 
-func (esac *AssociationConf) GetAuthSecretName() string {
-	if esac == nil {
+func (ac *AssociationConf) GetAuthSecretName() string {
+	if ac == nil {
 		return ""
 	}
-	return esac.AuthSecretName
+	return ac.AuthSecretName
 }
 
-func (esac *AssociationConf) GetAuthSecretKey() string {
-	if esac == nil {
+func (ac *AssociationConf) GetAuthSecretKey() string {
+	if ac == nil {
 		return ""
 	}
-	return esac.AuthSecretKey
+	return ac.AuthSecretKey
 }
 
-func (esac *AssociationConf) GetCASecretName() string {
-	if esac == nil {
+func (ac *AssociationConf) GetCACertProvided() bool {
+	if ac == nil {
+		return false
+	}
+	return ac.CACertProvided
+}
+
+func (ac *AssociationConf) GetCASecretName() string {
+	if ac == nil {
 		return ""
 	}
-	return esac.CASecretName
+	return ac.CASecretName
 }
 
-func (esac *AssociationConf) GetURL() string {
-	if esac == nil {
+func (ac *AssociationConf) GetURL() string {
+	if ac == nil {
 		return ""
 	}
-	return esac.URL
+	return ac.URL
 }

pkg/controller/apmserver/config/config.go

@@ -51,15 +51,17 @@ func NewConfigFromSpec(c k8s.Client, as *v1alpha1.ApmServer) (*settings.Canonica
 		if err != nil {
 			return nil, err
 		}
-		outputCfg = settings.MustCanonicalConfig(
-			map[string]interface{}{
-				"output.elasticsearch.hosts":                       []string{as.AssociationConf().GetURL()},
-				"output.elasticsearch.username":                    username,
-				"output.elasticsearch.password":                    password,
-				"output.elasticsearch.ssl.certificate_authorities": []string{filepath.Join(CertificatesDir, certificates.CertFileName)},
-			},
-		)
 
+		tmpOutputCfg := map[string]interface{}{
+			"output.elasticsearch.hosts":    []string{as.AssociationConf().GetURL()},
+			"output.elasticsearch.username": username,
+			"output.elasticsearch.password": password,
+		}
+		if as.AssociationConf().GetCACertProvided() {
+			tmpOutputCfg["output.elasticsearch.ssl.certificate_authorities"] = []string{filepath.Join(CertificatesDir, certificates.CAFileName)}
+		}
+
+		outputCfg = settings.MustCanonicalConfig(tmpOutputCfg)
 	}
 
 	// Create a base configuration.

pkg/controller/apmserver/config/config_test.go

@@ -0,0 +1,144 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License;
+// you may not use this file except in compliance with the Elastic License.
+
+package config
+
+import (
+	"testing"
+
+	"github.com/elastic/cloud-on-k8s/pkg/apis/apm/v1alpha1"
+	commonv1alpha1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1alpha1"
+	"github.com/elastic/cloud-on-k8s/pkg/controller/common/settings"
+	"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
+	"github.com/stretchr/testify/require"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+func TestNewConfigFromSpec(t *testing.T) {
+	testCases := []struct {
+		name            string
+		configOverrides map[string]interface{}
+		assocConf       *commonv1alpha1.AssociationConf
+		wantConf        map[string]interface{}
+		wantErr         bool
+	}{
+		{
+			name: "default config",
+		},
+		{
+			name: "with overridden config",
+			configOverrides: map[string]interface{}{
+				"apm-server.secret_token": "MYSECRET",
+			},
+			wantConf: map[string]interface{}{
+				"apm-server.secret_token": "MYSECRET",
+			},
+		},
+		{
+			name: "without Elasticsearch CA cert",
+			assocConf: &commonv1alpha1.AssociationConf{
+				AuthSecretName: "test-es-elastic-user",
+				AuthSecretKey:  "elastic",
+				CASecretName:   "test-es-http-ca-public",
+				CACertProvided: false,
+				URL:            "https://test-es-http.default.svc:9200",
+			},
+			wantConf: map[string]interface{}{
+				"output.elasticsearch.hosts":    []string{"https://test-es-http.default.svc:9200"},
+				"output.elasticsearch.username": "elastic",
+				"output.elasticsearch.password": "password",
+			},
+		},
+		{
+			name: "with Elasticsearch CA cert",
+			assocConf: &commonv1alpha1.AssociationConf{
+				AuthSecretName: "test-es-elastic-user",
+				AuthSecretKey:  "elastic",
+				CASecretName:   "test-es-http-ca-public",
+				CACertProvided: true,
+				URL:            "https://test-es-http.default.svc:9200",
+			},
+			wantConf: map[string]interface{}{
+				"output.elasticsearch.hosts":                       []string{"https://test-es-http.default.svc:9200"},
+				"output.elasticsearch.username":                    "elastic",
+				"output.elasticsearch.password":                    "password",
+				"output.elasticsearch.ssl.certificate_authorities": []string{"config/elasticsearch-certs/ca.crt"},
+			},
+		},
+		{
+			name: "missing auth secret",
+			assocConf: &commonv1alpha1.AssociationConf{
+				AuthSecretName: "wrong-secret",
+				AuthSecretKey:  "elastic",
+				CASecretName:   "test-es-http-ca-public",
+				CACertProvided: true,
+				URL:            "https://test-es-http.default.svc:9200",
+			},
+			wantErr: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			client := k8s.WrapClient(fake.NewFakeClient(mkAuthSecret()))
+			apmServer := mkAPMServer(tc.configOverrides, tc.assocConf)
+			gotConf, err := NewConfigFromSpec(client, apmServer)
+			if tc.wantErr {
+				require.Error(t, err)
+				return
+			}
+
+			require.NoError(t, err)
+
+			wantConf := mkConf(t, tc.wantConf)
+			diff := wantConf.Diff(gotConf, nil)
+			require.Len(t, diff, 0)
+		})
+	}
+}
+
+func mkAPMServer(config map[string]interface{}, assocConf *commonv1alpha1.AssociationConf) *v1alpha1.ApmServer {
+	apmServer := &v1alpha1.ApmServer{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "apm-server",
+		},
+		Spec: v1alpha1.ApmServerSpec{
+			Config: &commonv1alpha1.Config{Data: config},
+		},
+	}
+
+	apmServer.SetAssociationConf(assocConf)
+	return apmServer
+}
+
+func mkAuthSecret() *v1.Secret {
+	return &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-es-elastic-user",
+		},
+		Data: map[string][]byte{
+			"elastic": []byte("password"),
+		},
+	}
+}
+
+func mkConf(t *testing.T, overrides map[string]interface{}) *settings.CanonicalConfig {
+	t.Helper()
+	cfg, err := settings.NewCanonicalConfigFrom(map[string]interface{}{
+		"apm-server.host":            ":8200",
+		"apm-server.secret_token":    "${SECRET_TOKEN}",
+		"apm-server.ssl.certificate": "/mnt/elastic-internal/http-certs/tls.crt",
+		"apm-server.ssl.enabled":     true,
+		"apm-server.ssl.key":         "/mnt/elastic-internal/http-certs/tls.key",
+	})
+	require.NoError(t, err)
+
+	overriddenCfg, err := settings.NewCanonicalConfigFrom(overrides)
+	require.NoError(t, err)
+
+	require.NoError(t, cfg.MergeWith(overriddenCfg))
+	return cfg
+}

pkg/controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller.go

@@ -278,7 +278,7 @@ func (r *ReconcileApmServerElasticsearchAssociation) reconcileInternal(apmServer
 		return commonv1alpha1.AssociationPending, err
 	}
 
-	caSecretName, err := r.reconcileElasticsearchCA(apmServer, elasticsearchRef.NamespacedName())
+	caSecret, err := r.reconcileElasticsearchCA(apmServer, elasticsearchRef.NamespacedName())
 	if err != nil {
 		return commonv1alpha1.AssociationPending, err // maybe not created yet
 	}
@@ -288,7 +288,8 @@ func (r *ReconcileApmServerElasticsearchAssociation) reconcileInternal(apmServer
 	expectedAssocConf := &commonv1alpha1.AssociationConf{
 		AuthSecretName: authSecretRef.Name,
 		AuthSecretKey:  authSecretRef.Key,
-		CASecretName:   caSecretName,
+		CACertProvided: caSecret.CACertProvided,
+		CASecretName:   caSecret.Name,
 		URL:            services.ExternalServiceURL(es),
 	}
 
@@ -311,15 +312,15 @@ func (r *ReconcileApmServerElasticsearchAssociation) reconcileInternal(apmServer
 	return commonv1alpha1.AssociationEstablished, nil
 }
 
-func (r *ReconcileApmServerElasticsearchAssociation) reconcileElasticsearchCA(apm *apmtype.ApmServer, es types.NamespacedName) (string, error) {
+func (r *ReconcileApmServerElasticsearchAssociation) reconcileElasticsearchCA(apm *apmtype.ApmServer, es types.NamespacedName) (association.CASecret, error) {
 	apmKey := k8s.ExtractNamespacedName(apm)
 	// watch ES CA secret to reconcile on any change
 	if err := r.watches.Secrets.AddHandler(watches.NamedWatch{
 		Name:    esCAWatchName(apmKey),
 		Watched: []types.NamespacedName{http.PublicCertsSecretRef(esname.ESNamer, es)},
 		Watcher: apmKey,
 	}); err != nil {
-		return "", err
+		return association.CASecret{}, err
 	}
 	// Build the labels applied on the secret
 	labels := labels.NewLabels(apm.Name)