Ensure Kibana encryption key is specified (#2278)

docs/kibana.asciidoc

@@ -180,10 +180,7 @@ spec:
 [id="{p}-kibana-scaling"]
 === Scale out a Kibana deployment
 
-You may want to deploy more than one instance of Kibana.
-In this case all the instances must share the same encryption key.
-
-This can be done by setting the `xpack.security.encryptionKey` property using a secure setting as described in the next section.
+You may want to deploy more than one instance of Kibana. In this case all the instances must share the same encryption key. If you do not set one, the operator will generate one for you. If you would like to set your own encryption key, this can be done by setting the `xpack.security.encryptionKey` property using a secure setting as described in the next section.
 
 Note that while most reconfigurations of your Kibana instances will be carried out in rolling upgrade fashion, all version upgrades will cause Kibana downtime. This is due to the link:https://www.elastic.co/guide/en/kibana/current/upgrade.html[requirement] to run only a single version of Kibana at any given time.
 

pkg/controller/common/settings/canonical_config.go

@@ -60,16 +60,26 @@ func MustCanonicalConfig(cfg interface{}) *CanonicalConfig {
 }
 
 // MustNewSingleValue creates a new config holding a single string value.
-// Convenience constructor, will panic in the unlikely event of errors.
+// It is NewSingleValue but panics rather than returning errors, largely used for convenience in tests
 func MustNewSingleValue(k string, v string) *CanonicalConfig {
-	cfg := fromConfig(ucfg.New())
+	cfg := NewCanonicalConfig()
 	err := cfg.asUCfg().SetString(k, -1, v, Options...)
 	if err != nil {
 		panic(err)
 	}
 	return cfg
 }
 
+// NewSingleValue creates a new config holding a single string value.
+func NewSingleValue(k string, v string) (*CanonicalConfig, error) {
+	cfg := fromConfig(ucfg.New())
+	err := cfg.asUCfg().SetString(k, -1, v, Options...)
+	if err != nil {
+		return nil, errors.WithStack(err)
+	}
+	return cfg, nil
+}
+
 // ParseConfig parses the given configuration content into a CanonicalConfig.
 // Expects content to be in YAML format.
 func ParseConfig(yml []byte) (*CanonicalConfig, error) {

pkg/controller/kibana/config/fields.go

@@ -11,6 +11,7 @@ const (
 	ElasticSearchHosts                             = "elasticsearch.hosts"
 	XpackMonitoringUiContainerElasticsearchEnabled = "xpack.monitoring.ui.container.elasticsearch.enabled"
 	XpackLicenseManagementUIEnabled                = "xpack.license_management.ui.enabled" // >= 7.6
+	XpackSecurityEncryptionKey                     = "xpack.security.encryptionKey"
 
 	ElasticsearchSslCertificateAuthorities = "elasticsearch.ssl.certificateAuthorities"
 	ElasticsearchSslVerificationMode       = "elasticsearch.ssl.verificationMode"

pkg/controller/kibana/config/settings.go

@@ -5,8 +5,6 @@
 package config
 
 import (
-	"path"
-
 	commonv1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1"
 	kbv1 "github.com/elastic/cloud-on-k8s/pkg/apis/kibana/v1"
 	"github.com/elastic/cloud-on-k8s/pkg/controller/common/association"
@@ -15,15 +13,24 @@ import (
 	"github.com/elastic/cloud-on-k8s/pkg/controller/common/settings"
 	"github.com/elastic/cloud-on-k8s/pkg/controller/kibana/es"
 	"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
+	"github.com/elastic/go-ucfg"
+	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/rand"
+	"path"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 const (
-	// Kibana configuration settings file
+	// SettingsFilename is the Kibana configuration settings file
 	SettingsFilename = "kibana.yml"
-	// Environment variable name for the Node options that can be used to increase the Kibana maximum memory limit
+	// EnvNodeOpts is the environment variable name for the Node options that can be used to increase the Kibana maximum memory limit
 	EnvNodeOpts = "NODE_OPTS"
 )
 
+var log = logf.Log.WithName("kibana-config")
 
 // CanonicalConfig contains configuration for Kibana ("kibana.yml"),
 // as a hierarchical key-value configuration.
@@ -33,6 +40,14 @@ type CanonicalConfig struct {
 
 // NewConfigSettings returns the Kibana configuration settings for the given Kibana resource.
 func NewConfigSettings(client k8s.Client, kb kbv1.Kibana, versionSpecificCfg *settings.CanonicalConfig) (CanonicalConfig, error) {
+	currentConfig, err := getExistingConfig(client, kb)
+	if err != nil {
+		return CanonicalConfig{}, err
+	}
+	filteredCurrCfg, err := filterExistingConfig(currentConfig)
+	if err != nil {
+		return CanonicalConfig{}, err
+	}
 	specConfig := kb.Spec.Config
 	if specConfig == nil {
 		specConfig = &commonv1.Config{}
@@ -47,7 +62,12 @@ func NewConfigSettings(client k8s.Client, kb kbv1.Kibana, versionSpecificCfg *se
 	kibanaTLSCfg := settings.MustCanonicalConfig(kibanaTLSSettings(kb))
 
 	if !kb.RequiresAssociation() {
-		if err := cfg.MergeWith(versionSpecificCfg, kibanaTLSCfg, userSettings); err != nil {
+		// merge the configuration with userSettings last so they take precedence
+		if err := cfg.MergeWith(
+			filteredCurrCfg,
+			versionSpecificCfg,
+			kibanaTLSCfg,
+			userSettings); err != nil {
 			return CanonicalConfig{}, err
 		}
 		return CanonicalConfig{cfg}, nil
@@ -60,6 +80,7 @@ func NewConfigSettings(client k8s.Client, kb kbv1.Kibana, versionSpecificCfg *se
 
 	// merge the configuration with userSettings last so they take precedence
 	err = cfg.MergeWith(
+		filteredCurrCfg,
 		versionSpecificCfg,
 		kibanaTLSCfg,
 		settings.MustCanonicalConfig(elasticsearchTLSSettings(kb)),
@@ -78,12 +99,58 @@ func NewConfigSettings(client k8s.Client, kb kbv1.Kibana, versionSpecificCfg *se
 	return CanonicalConfig{cfg}, nil
 }
 
+// getExistingConfig retrieves the canonical config for a given Kibana, if one exists
+func getExistingConfig(client k8s.Client, kb kbv1.Kibana) (*settings.CanonicalConfig, error) {
+	var secret corev1.Secret
+	err := client.Get(types.NamespacedName{Name: SecretName(kb), Namespace: kb.Namespace}, &secret)
+	if err != nil && apierrors.IsNotFound(err) {
+		log.V(1).Info("Kibana config secret does not exist", "namespace", kb.Namespace, "kibana_name", kb.Name)
+		return nil, nil
+	} else if err != nil {
+		log.Error(err, "Error retrieving kibana config secret", "namespace", kb.Namespace, "kibana_name", kb.Name)
+		return nil, err
+	}
+	rawCfg, exists := secret.Data[SettingsFilename]
+	if !exists {
+		err = errors.New("Kibana config secret exists but missing config file key")
+		log.Error(err, "", "namespace", secret.Namespace, "secret_name", secret.Name, "key", SettingsFilename)
+		return nil, err
+	}
+	cfg, err := settings.ParseConfig(rawCfg)
+	if err != nil {
+		log.Error(err, "Error parsing existing kibana config in secret", "namespace", secret.Namespace, "secret_name", secret.Name, "key", SettingsFilename)
+		return nil, err
+	}
+	return cfg, nil
+}
+
+// filterExistingConfig filters an existing config for only items we want to preserve between spec changes
+// because they cannot be generated deterministically, e.g. encryption keys
+func filterExistingConfig(cfg *settings.CanonicalConfig) (*settings.CanonicalConfig, error) {
+	if cfg == nil {
+		return nil, nil
+	}
+	val, err := (*ucfg.Config)(cfg).String(XpackSecurityEncryptionKey, -1, settings.Options...)
+	if err != nil {
+		log.V(1).Info("Current config does not contain key", "key", XpackSecurityEncryptionKey, "error", err)
+		return nil, nil
+	}
+	filteredCfg, err := settings.NewSingleValue(XpackSecurityEncryptionKey, val)
+	if err != nil {
+		log.Error(err, "Error filtering current config")
+		return nil, err
+	}
+	return filteredCfg, nil
+}
+
 func baseSettings(kb kbv1.Kibana) map[string]interface{} {
 	return map[string]interface{}{
 		ServerName:         kb.Name,
 		ServerHost:         "0",
 		ElasticSearchHosts: []string{kb.AssociationConf().GetURL()},
 		XpackMonitoringUiContainerElasticsearchEnabled: true,
+		// this will get overriden if one already exists or is specified by the user
+		XpackSecurityEncryptionKey: rand.String(64),
 	}
 }