From 9a4056ff4c8304f967aac1fd38675d3c7e074b8a Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Tue, 27 Mar 2018 12:11:57 +0200 Subject: [PATCH 1/3] Store hash of secret values, and detect changes --- pkg/apis/deployment/v1alpha/conditions.go | 4 + .../deployment/v1alpha/deployment_status.go | 4 + pkg/apis/deployment/v1alpha/secret_hashes.go | 37 ++++ .../v1alpha/zz_generated.deepcopy.go | 25 +++ pkg/deployment/deployment_inspector.go | 21 ++- pkg/deployment/resources/secret_hashes.go | 174 ++++++++++++++++++ pkg/util/k8sutil/events.go | 19 ++ 7 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 pkg/apis/deployment/v1alpha/secret_hashes.go create mode 100644 pkg/deployment/resources/secret_hashes.go diff --git a/pkg/apis/deployment/v1alpha/conditions.go b/pkg/apis/deployment/v1alpha/conditions.go index 9ecf645e3..3ba3d1dcb 100644 --- a/pkg/apis/deployment/v1alpha/conditions.go +++ b/pkg/apis/deployment/v1alpha/conditions.go @@ -37,6 +37,10 @@ const ( ConditionTypeTerminated ConditionType = "Terminated" // ConditionTypeAutoUpgrade indicates that the member has to be started with `--database.auto-upgrade` once. ConditionTypeAutoUpgrade ConditionType = "AutoUpgrade" + // ConditionTypeSecretsChanged indicates that the value of one of more secrets used by + // the deployment have changed. Once that is the case, the operator will no longer + // touch the deployment, until the original secrets have been restored. + ConditionTypeSecretsChanged ConditionType = "SecretsChanged" ) // Condition represents one current condition of a deployment or deployment member. diff --git a/pkg/apis/deployment/v1alpha/deployment_status.go b/pkg/apis/deployment/v1alpha/deployment_status.go index 4c9660aa8..31b2e6f34 100644 --- a/pkg/apis/deployment/v1alpha/deployment_status.go +++ b/pkg/apis/deployment/v1alpha/deployment_status.go @@ -50,4 +50,8 @@ type DeploymentStatus struct { // AcceptedSpec contains the last specification that was accepted by the operator. AcceptedSpec *DeploymentSpec `json:"accepted-spec,omitempty"` + + // SecretHashes keeps a sha256 hash of secret values, so we can + // detect changes in secret values. + SecretHashes *SecretHashes `json:"secret-hashes,omitempty"` } diff --git a/pkg/apis/deployment/v1alpha/secret_hashes.go b/pkg/apis/deployment/v1alpha/secret_hashes.go new file mode 100644 index 000000000..5860f363d --- /dev/null +++ b/pkg/apis/deployment/v1alpha/secret_hashes.go @@ -0,0 +1,37 @@ +// +// DISCLAIMER +// +// Copyright 2018 ArangoDB GmbH, Cologne, Germany +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Ewout Prangsma +// + +package v1alpha + +// SecretHashes keeps track of the value of secrets +// so we can detect changes. +// For each used secret, a sha256 hash is stored. +type SecretHashes struct { + // AuthJWT contains the hash of the auth.jwtSecretName secret + AuthJWT string `json:"auth-jwt,omitempty"` + // RocksDBEncryptionKey contains the hash of the rocksdb.encryption.keySecretName secret + RocksDBEncryptionKey string `json:"rocksdb-encryption-key,omitempty"` + // TLSCA contains the hash of the tls.caSecretName secret + TLSCA string `json:"tls-ca,omitempty"` + // SyncTLSCA contains the hash of the sync.tls.caSecretName secret + SyncTLSCA string `json:"sync-tls-ca,omitempty"` +} diff --git a/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go b/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go index b843241da..6b32aaa0d 100644 --- a/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go +++ b/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go @@ -265,6 +265,15 @@ func (in *DeploymentStatus) DeepCopyInto(out *DeploymentStatus) { (*in).DeepCopyInto(*out) } } + if in.SecretHashes != nil { + in, out := &in.SecretHashes, &out.SecretHashes + if *in == nil { + *out = nil + } else { + *out = new(SecretHashes) + **out = **in + } + } return } @@ -442,6 +451,22 @@ func (in *RocksDBSpec) DeepCopy() *RocksDBSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecretHashes) DeepCopyInto(out *SecretHashes) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretHashes. +func (in *SecretHashes) DeepCopy() *SecretHashes { + if in == nil { + return nil + } + out := new(SecretHashes) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServerGroupSpec) DeepCopyInto(out *ServerGroupSpec) { *out = *in diff --git a/pkg/deployment/deployment_inspector.go b/pkg/deployment/deployment_inspector.go index 7229aec6f..3b53ad951 100644 --- a/pkg/deployment/deployment_inspector.go +++ b/pkg/deployment/deployment_inspector.go @@ -26,6 +26,7 @@ import ( "context" "time" + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" ) @@ -37,12 +38,30 @@ import ( // - once in a while // Returns the delay until this function should be called again. func (d *Deployment) inspectDeployment(lastInterval time.Duration) time.Duration { - // log := d.deps.Log + log := d.deps.Log nextInterval := lastInterval hasError := false ctx := context.Background() + // Is the deployment in failed state, if so, give up. + if d.status.State == api.DeploymentStateFailed { + log.Debug().Msg("Deployment is in Failed state.") + return nextInterval + } + + // Inspect secret hashes + if err := d.resources.ValidateSecretHashes(); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Secret hash validation failed", err, d.apiObject)) + } + + // Is the deployment in a good state? + if d.status.Conditions.IsTrue(api.ConditionTypeSecretsChanged) { + log.Debug().Msg("Condition SecretsChanged is true. Revert secrets before we can continue") + return nextInterval + } + // Ensure we have image info if retrySoon, err := d.ensureImages(d.apiObject); err != nil { hasError = true diff --git a/pkg/deployment/resources/secret_hashes.go b/pkg/deployment/resources/secret_hashes.go new file mode 100644 index 000000000..868725612 --- /dev/null +++ b/pkg/deployment/resources/secret_hashes.go @@ -0,0 +1,174 @@ +// +// DISCLAIMER +// +// Copyright 2018 ArangoDB GmbH, Cologne, Germany +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Ewout Prangsma +// + +package resources + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "sort" + "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" +) + +// ValidateSecretHashes checks the hash of used secrets +// against the stored ones. +// If a hash is different, the deployment is marked +// with a SecretChangedCondition and the operator will no +// touch it until this is resolved. +func (r *Resources) ValidateSecretHashes() error { + // validate performs a secret hash comparison for a single secret. + // Return true if all is good, false when the SecretChanged condition + // must be set. + validate := func(secretName string, expectedHashRef *string, status *api.DeploymentStatus) (bool, error) { + log := r.log.With().Str("secret-name", secretName).Logger() + expectedHash := *expectedHashRef + hash, err := r.getSecretHash(secretName) + if expectedHash == "" { + // No hash set yet, try to fill it + if k8sutil.IsNotFound(err) { + // Secret does not (yet) exists, do nothing + return true, nil + } + if err != nil { + log.Warn().Err(err).Msg("Failed to get secret") + return true, nil // Since we do not yet have a hash, we let this go with only a warning. + } + // Hash fetched succesfully, store it + *expectedHashRef = hash + if r.context.UpdateStatus(*status); err != nil { + log.Debug().Msg("Failed to save secret hash") + return true, maskAny(err) + } + return true, nil + } + // Hash is set, it must match the current hash + if err != nil { + // Fetching error failed for other reason. + log.Debug().Err(err).Msg("Failed to fetch secret hash") + // This is not good, return false so SecretsChanged condition will be set. + return false, nil + } + if hash != expectedHash { + // Oops, hash has changed + log.Error().Msg("Secret has changed. You must revert it to the original value!") + // This is not good, return false so SecretsChanged condition will be set. + return false, nil + } + // All good + return true, nil + } + + spec := r.context.GetSpec() + log := r.log + var badSecretNames []string + status := r.context.GetStatus() + if status.SecretHashes == nil { + status.SecretHashes = &api.SecretHashes{} + } + hashes := status.SecretHashes + if spec.IsAuthenticated() { + secretName := spec.Authentication.GetJWTSecretName() + if hashOK, err := validate(secretName, &hashes.AuthJWT, &status); err != nil { + return maskAny(err) + } else if !hashOK { + badSecretNames = append(badSecretNames, secretName) + } + } + if spec.RocksDB.IsEncrypted() { + secretName := spec.RocksDB.Encryption.GetKeySecretName() + if hashOK, err := validate(secretName, &hashes.RocksDBEncryptionKey, &status); err != nil { + return maskAny(err) + } else if !hashOK { + badSecretNames = append(badSecretNames, secretName) + } + } + if spec.IsSecure() { + secretName := spec.TLS.GetCASecretName() + if hashOK, err := validate(secretName, &hashes.TLSCA, &status); err != nil { + return maskAny(err) + } else if !hashOK { + badSecretNames = append(badSecretNames, secretName) + } + } + if spec.Sync.IsEnabled() { + secretName := spec.Sync.TLS.GetCASecretName() + if hashOK, err := validate(secretName, &hashes.SyncTLSCA, &status); err != nil { + return maskAny(err) + } else if !hashOK { + badSecretNames = append(badSecretNames, secretName) + } + } + + if len(badSecretNames) > 0 { + // We have invalid hashes, set the SecretsChanged condition + if status.Conditions.Update(api.ConditionTypeSecretsChanged, true, + "Secrets have changed", fmt.Sprintf("Found %d changed secrets", len(badSecretNames))) { + log.Warn().Msgf("Found %d changed secrets. Settings SecretsChanged condition", len(badSecretNames)) + if err := r.context.UpdateStatus(status); err != nil { + log.Error().Err(err).Msg("Failed to save SecretsChanged condition") + return maskAny(err) + } + // Add an event about this + r.context.CreateEvent(k8sutil.NewSecretsChangedEvent(badSecretNames, r.context.GetAPIObject())) + } + } else { + // All good, we van remove the SecretsChanged condition + if status.Conditions.Remove(api.ConditionTypeSecretsChanged) { + log.Warn().Msg("Resetting SecretsChanged condition") + if err := r.context.UpdateStatus(status); err != nil { + log.Error().Err(err).Msg("Failed to save SecretsChanged condition") + return maskAny(err) + } + // Add an event about this + r.context.CreateEvent(k8sutil.NewSecretsRestoredEvent(r.context.GetAPIObject())) + } + } + + return nil +} + +// getSecretHash fetches a secret with given name and returns a hash over its value. +func (r *Resources) getSecretHash(secretName string) (string, error) { + kubecli := r.context.GetKubeCli() + ns := r.context.GetNamespace() + s, err := kubecli.CoreV1().Secrets(ns).Get(secretName, metav1.GetOptions{}) + if err != nil { + return "", maskAny(err) + } + // Create hash of value + rows := make([]string, 0, len(s.Data)) + for k, v := range s.Data { + rows = append(rows, k+"="+hex.EncodeToString(v)) + } + // Sort so we're not detecting order differences + sort.Strings(rows) + data := strings.Join(rows, "\n") + rawHash := sha256.Sum256([]byte(data)) + hash := fmt.Sprintf("%0x", rawHash) + return hash, nil +} diff --git a/pkg/util/k8sutil/events.go b/pkg/util/k8sutil/events.go index 568264d29..9b227d9bf 100644 --- a/pkg/util/k8sutil/events.go +++ b/pkg/util/k8sutil/events.go @@ -78,6 +78,25 @@ func NewImmutableFieldEvent(fieldName string, apiObject APIObject) *v1.Event { return event } +// NewSecretsChangedEvent creates an event indicating that one of more secrets have changed. +func NewSecretsChangedEvent(changedSecretNames []string, apiObject APIObject) *v1.Event { + event := newDeploymentEvent(apiObject) + event.Type = v1.EventTypeNormal + event.Reason = "Secrets changed" + event.Message = fmt.Sprintf("Found %d changed secrets. You must revert them before the operator can continue. Secrets: %v", len(changedSecretNames), changedSecretNames) + return event +} + +// NewSecretsRestoredEvent creates an event indicating that all secrets have been restored +// to their original values. +func NewSecretsRestoredEvent(apiObject APIObject) *v1.Event { + event := newDeploymentEvent(apiObject) + event.Type = v1.EventTypeNormal + event.Reason = "Secrets restored" + event.Message = "All secrets have been restored to their original value" + return event +} + // NewErrorEvent creates an even of type error. func NewErrorEvent(reason string, err error, apiObject APIObject) *v1.Event { event := newDeploymentEvent(apiObject) From 726ab7230315c2edea3e5e3aa5cf5190bc68e287 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Tue, 27 Mar 2018 12:55:37 +0200 Subject: [PATCH 2/3] Log improvements --- pkg/deployment/resources/secret_hashes.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/deployment/resources/secret_hashes.go b/pkg/deployment/resources/secret_hashes.go index 868725612..425ad1486 100644 --- a/pkg/deployment/resources/secret_hashes.go +++ b/pkg/deployment/resources/secret_hashes.go @@ -75,7 +75,10 @@ func (r *Resources) ValidateSecretHashes() error { } if hash != expectedHash { // Oops, hash has changed - log.Error().Msg("Secret has changed. You must revert it to the original value!") + log.Debug(). + Str("expected-hash", expectedHash). + Str("new-hash", hash). + Msg("Secret has changed.") // This is not good, return false so SecretsChanged condition will be set. return false, nil } @@ -139,7 +142,7 @@ func (r *Resources) ValidateSecretHashes() error { } else { // All good, we van remove the SecretsChanged condition if status.Conditions.Remove(api.ConditionTypeSecretsChanged) { - log.Warn().Msg("Resetting SecretsChanged condition") + log.Info().Msg("Resetting SecretsChanged condition") if err := r.context.UpdateStatus(status); err != nil { log.Error().Err(err).Msg("Failed to save SecretsChanged condition") return maskAny(err) From 9c07dcd1849be4bd137ddd75c0736a5969013c58 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Tue, 27 Mar 2018 12:55:51 +0200 Subject: [PATCH 3/3] Listen for secret changes --- pkg/deployment/deployment.go | 1 + pkg/deployment/informers.go | 43 ++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go index 25b80f2e5..7e75c8bb1 100644 --- a/pkg/deployment/deployment.go +++ b/pkg/deployment/deployment.go @@ -120,6 +120,7 @@ func New(config Config, deps Dependencies, apiObject *api.ArangoDeployment) (*De go d.run() go d.listenForPodEvents(d.stopCh) go d.listenForPVCEvents(d.stopCh) + go d.listenForSecretEvents(d.stopCh) go d.listenForServiceEvents(d.stopCh) if apiObject.Spec.GetMode() == api.DeploymentModeCluster { ci := newClusterScalingIntegration(d) diff --git a/pkg/deployment/informers.go b/pkg/deployment/informers.go index 2cc34ab63..5cf7da2a2 100644 --- a/pkg/deployment/informers.go +++ b/pkg/deployment/informers.go @@ -112,6 +112,49 @@ func (d *Deployment) listenForPVCEvents(stopCh <-chan struct{}) { informer.Run(stopCh) } +// listenForSecretEvents keep listening for changes in Secrets's until the given channel is closed. +func (d *Deployment) listenForSecretEvents(stopCh <-chan struct{}) { + source := cache.NewListWatchFromClient( + d.deps.KubeCli.CoreV1().RESTClient(), + "secrets", + d.apiObject.GetNamespace(), + fields.Everything()) + + getSecret := func(obj interface{}) (*v1.Secret, bool) { + secret, ok := obj.(*v1.Secret) + if !ok { + tombstone, ok := obj.(cache.DeletedFinalStateUnknown) + if !ok { + return nil, false + } + secret, ok = tombstone.Obj.(*v1.Secret) + return secret, ok + } + return secret, true + } + + _, informer := cache.NewIndexerInformer(source, &v1.Secret{}, 0, cache.ResourceEventHandlerFuncs{ + // Note: For secrets we look at all of them because they do not have to be owned by this deployment. + AddFunc: func(obj interface{}) { + if _, ok := getSecret(obj); ok { + d.triggerInspection() + } + }, + UpdateFunc: func(oldObj, newObj interface{}) { + if _, ok := getSecret(newObj); ok { + d.triggerInspection() + } + }, + DeleteFunc: func(obj interface{}) { + if _, ok := getSecret(obj); ok { + d.triggerInspection() + } + }, + }, cache.Indexers{}) + + informer.Run(stopCh) +} + // listenForServiceEvents keep listening for changes in Service's until the given channel is closed. func (d *Deployment) listenForServiceEvents(stopCh <-chan struct{}) { source := cache.NewListWatchFromClient(