From bb673873532b81336be86a3dba86fef50c83f699 Mon Sep 17 00:00:00 2001 From: Njal Karevoll Date: Thu, 6 Jun 2019 17:56:43 +0200 Subject: [PATCH] Allow additional labels to be set on the tranport cert secrets --- .../certificates/transport/pod_secret.go | 15 +++-- .../certificates/transport/pod_secret_test.go | 60 +++++++++++++++---- .../certificates/transport/reconcile_test.go | 6 ++ 3 files changed, 65 insertions(+), 16 deletions(-) diff --git a/operators/pkg/controller/elasticsearch/certificates/transport/pod_secret.go b/operators/pkg/controller/elasticsearch/certificates/transport/pod_secret.go index 7bd431165c..24e932eb52 100644 --- a/operators/pkg/controller/elasticsearch/certificates/transport/pod_secret.go +++ b/operators/pkg/controller/elasticsearch/certificates/transport/pod_secret.go @@ -5,8 +5,6 @@ package transport import ( - "reflect" - "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/reconciler" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/label" @@ -57,8 +55,17 @@ func EnsureTransportCertificateSecretExists( Expected: &expected, Reconciled: &reconciled, NeedsUpdate: func() bool { - // we only care about labels, not contents - return reflect.DeepEqual(expected.Labels, reconciled.Labels) + // we only care about labels, not contents at this point, and we can allow additional labels + if reconciled.Labels == nil { + return true + } + + for k, v := range expected.Labels { + if rv, ok := reconciled.Labels[k]; !ok || rv != v { + return true + } + } + return false }, UpdateReconciled: func() { if reconciled.Labels == nil { diff --git a/operators/pkg/controller/elasticsearch/certificates/transport/pod_secret_test.go b/operators/pkg/controller/elasticsearch/certificates/transport/pod_secret_test.go index b1e2f763da..19d1dfdbf2 100644 --- a/operators/pkg/controller/elasticsearch/certificates/transport/pod_secret_test.go +++ b/operators/pkg/controller/elasticsearch/certificates/transport/pod_secret_test.go @@ -8,30 +8,45 @@ import ( "testing" "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/label" "github.com/elastic/cloud-on-k8s/operators/pkg/utils/k8s" "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" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func TestEnsureTransportCertificateSecretExists(t *testing.T) { + es := v1alpha1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-es", + }, + } + pod := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "pod", }, } - preExistingSecret := &corev1.Secret{ + + defaultPodSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "pod-certs", + Labels: map[string]string{ + LabelCertificateType: LabelCertificateTypeTransport, + label.PodNameLabelName: pod.Name, + label.ClusterNameLabelName: es.Name, + }, }, } - es := v1alpha1.Elasticsearch{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-es", - }, + + defaultPodSecretWith := func(setter func(secret *corev1.Secret)) *corev1.Secret { + secret := defaultPodSecret.DeepCopy() + setter(secret) + return secret } type args struct { @@ -55,21 +70,42 @@ func TestEnsureTransportCertificateSecretExists(t *testing.T) { pod: pod, }, want: func(t *testing.T, secret *corev1.Secret) { - if assert.Contains(t, secret.Labels, LabelCertificateType) { - assert.Equal(t, secret.Labels[LabelCertificateType], LabelCertificateTypeTransport) - } - assert.Equal(t, "pod-certs", secret.Name) + // owner references are set upon creation, so ignore for comparison + expected := defaultPodSecretWith(func(s *corev1.Secret) { + s.OwnerReferences = secret.OwnerReferences + }) + assert.Equal(t, expected, secret) + }, + }, + { + name: "should update an existing secret", + args: args{ + c: k8s.WrapClient(fake.NewFakeClient(defaultPodSecretWith(func(secret *corev1.Secret) { + secret.ObjectMeta.UID = types.UID("42") + }))), + owner: es, + pod: pod, + }, + want: func(t *testing.T, secret *corev1.Secret) { + // UID should be kept the same + assert.Equal(t, defaultPodSecretWith(func(secret *corev1.Secret) { + secret.ObjectMeta.UID = types.UID("42") + }), secret) }, }, { - name: "should not create a new secret if it already exists", + name: "should allow additional labels in the secret", args: args{ - c: k8s.WrapClient(fake.NewFakeClient(preExistingSecret)), + c: k8s.WrapClient(fake.NewFakeClient(defaultPodSecretWith(func(secret *corev1.Secret) { + secret.ObjectMeta.Labels["foo"] = "bar" + }))), owner: es, pod: pod, }, want: func(t *testing.T, secret *corev1.Secret) { - assert.Equal(t, preExistingSecret, secret) + assert.Equal(t, defaultPodSecretWith(func(secret *corev1.Secret) { + secret.ObjectMeta.Labels["foo"] = "bar" + }), secret) }, }, } diff --git a/operators/pkg/controller/elasticsearch/certificates/transport/reconcile_test.go b/operators/pkg/controller/elasticsearch/certificates/transport/reconcile_test.go index 3a92502140..a101d40880 100644 --- a/operators/pkg/controller/elasticsearch/certificates/transport/reconcile_test.go +++ b/operators/pkg/controller/elasticsearch/certificates/transport/reconcile_test.go @@ -17,6 +17,7 @@ import ( "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/annotation" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/certificates" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/label" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/name" "github.com/elastic/cloud-on-k8s/operators/pkg/utils/k8s" "github.com/stretchr/testify/assert" @@ -241,6 +242,11 @@ func Test_doReconcileTransportCertificateSecret(t *testing.T) { objMeta := metav1.ObjectMeta{ Namespace: "namespace", Name: name.TransportCertsSecret(testPod.Name), + Labels: map[string]string{ + LabelCertificateType: LabelCertificateTypeTransport, + label.PodNameLabelName: testPod.Name, + label.ClusterNameLabelName: testCluster.Name, + }, } tests := []struct {