From 8f188cb6b01bde05faa27a0699d23757042fce9a Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Fri, 1 Mar 2024 12:27:27 +0100 Subject: [PATCH 1/2] certrotation: update all secret types to `kubernetes.io/tls` preserving existing content Pre-4.7 clusters had `SecretTypeTLS` type set, so on 4.15 upgrade these clusters had the secret delete and recreated. This commit will ensure these are being created without cert and key being regenerated --- pkg/operator/certrotation/signer.go | 26 +++++++++++++++++++------- pkg/operator/certrotation/target.go | 26 +++++++++++++++++++------- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/pkg/operator/certrotation/signer.go b/pkg/operator/certrotation/signer.go index d143dc3056..ca30efb87e 100644 --- a/pkg/operator/certrotation/signer.go +++ b/pkg/operator/certrotation/signer.go @@ -62,24 +62,36 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (* signingCertKeyPairSecret := originalSigningCertKeyPairSecret.DeepCopy() if apierrors.IsNotFound(err) { // create an empty one - signingCertKeyPairSecret = &corev1.Secret{ObjectMeta: NewTLSArtifactObjectMeta( - c.Name, - c.Namespace, - c.AdditionalAnnotations, - )} + signingCertKeyPairSecret = &corev1.Secret{ + ObjectMeta: NewTLSArtifactObjectMeta( + c.Name, + c.Namespace, + c.AdditionalAnnotations, + ), + Type: corev1.SecretTypeTLS, + } } - signingCertKeyPairSecret.Type = corev1.SecretTypeTLS needsMetadataUpdate := false + // no ownerReference set if c.Owner != nil { needsMetadataUpdate = ensureOwnerReference(&signingCertKeyPairSecret.ObjectMeta, c.Owner) } + // ownership annotations not set needsMetadataUpdate = c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&signingCertKeyPairSecret.ObjectMeta) || needsMetadataUpdate + // convert outdated secret type (set pre 4.7) + if signingCertKeyPairSecret.Type != corev1.SecretTypeTLS { + signingCertKeyPairSecret.Type = corev1.SecretTypeTLS + needsMetadataUpdate = true + } + // apply changes (possibly via delete+recreate) if secret exists and requires metadata update + // this is done before content update to prevent unexpected rollouts if needsMetadataUpdate && len(signingCertKeyPairSecret.ResourceVersion) > 0 { - _, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, signingCertKeyPairSecret) + actualSigningCertKeyPairSecret, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, signingCertKeyPairSecret) if err != nil { return nil, err } + signingCertKeyPairSecret = actualSigningCertKeyPairSecret } if needed, reason := needNewSigningCertKeyPair(signingCertKeyPairSecret.Annotations, c.Refresh, c.RefreshOnlyWhenExpired); needed { diff --git a/pkg/operator/certrotation/target.go b/pkg/operator/certrotation/target.go index ad1caa6379..fb479e72b0 100644 --- a/pkg/operator/certrotation/target.go +++ b/pkg/operator/certrotation/target.go @@ -97,24 +97,36 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont targetCertKeyPairSecret := originalTargetCertKeyPairSecret.DeepCopy() if apierrors.IsNotFound(err) { // create an empty one - targetCertKeyPairSecret = &corev1.Secret{ObjectMeta: NewTLSArtifactObjectMeta( - c.Name, - c.Namespace, - c.AdditionalAnnotations, - )} + targetCertKeyPairSecret = &corev1.Secret{ + ObjectMeta: NewTLSArtifactObjectMeta( + c.Name, + c.Namespace, + c.AdditionalAnnotations, + ), + Type: corev1.SecretTypeTLS, + } } - targetCertKeyPairSecret.Type = corev1.SecretTypeTLS needsMetadataUpdate := false + // no ownerReference set if c.Owner != nil { needsMetadataUpdate = ensureOwnerReference(&targetCertKeyPairSecret.ObjectMeta, c.Owner) } + // ownership annotations not set needsMetadataUpdate = c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta) || needsMetadataUpdate + // convert outdated secret type (set pre 4.7) + if targetCertKeyPairSecret.Type != corev1.SecretTypeTLS { + targetCertKeyPairSecret.Type = corev1.SecretTypeTLS + needsMetadataUpdate = true + } + // apply changes (possibly via delete+recreate) if secret exists and requires metadata update + // this is done before content update to prevent unexpected rollouts if needsMetadataUpdate && len(targetCertKeyPairSecret.ResourceVersion) > 0 { - _, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, targetCertKeyPairSecret) + actualTargetCertKeyPairSecret, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, targetCertKeyPairSecret) if err != nil { return nil, err } + targetCertKeyPairSecret = actualTargetCertKeyPairSecret } if reason := c.CertCreator.NeedNewTargetCertKeyPair(targetCertKeyPairSecret.Annotations, signingCertKeyPair, caBundleCerts, c.Refresh, c.RefreshOnlyWhenExpired); len(reason) > 0 { From c1b3d395b296bf8fd0f4d77d2a88d4b06d844617 Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Fri, 1 Mar 2024 12:58:15 +0100 Subject: [PATCH 2/2] certrotation: use shared metadata update code for signer and target secrets --- pkg/operator/certrotation/metadata.go | 36 +++++ pkg/operator/certrotation/signer.go | 16 +-- pkg/operator/certrotation/signer_test.go | 143 ++++++++++++++++++- pkg/operator/certrotation/target.go | 16 +-- pkg/operator/certrotation/target_test.go | 166 ++++++++++++++++++++++- 5 files changed, 346 insertions(+), 31 deletions(-) create mode 100644 pkg/operator/certrotation/metadata.go diff --git a/pkg/operator/certrotation/metadata.go b/pkg/operator/certrotation/metadata.go new file mode 100644 index 0000000000..f64bde8fe0 --- /dev/null +++ b/pkg/operator/certrotation/metadata.go @@ -0,0 +1,36 @@ +package certrotation + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func ensureMetadataUpdate(secret *corev1.Secret, owner *metav1.OwnerReference, additionalAnnotations AdditionalAnnotations) bool { + needsMetadataUpdate := false + // no ownerReference set + if owner != nil { + needsMetadataUpdate = ensureOwnerReference(&secret.ObjectMeta, owner) + } + // ownership annotations not set + return additionalAnnotations.EnsureTLSMetadataUpdate(&secret.ObjectMeta) || needsMetadataUpdate +} + +func ensureSecretTLSTypeSet(secret *corev1.Secret) bool { + // Existing secret not found - no need to update metadata (will be done by needNewSigningCertKeyPair / NeedNewTargetCertKeyPair) + if len(secret.ResourceVersion) == 0 { + return false + } + + // convert outdated secret type (created by pre 4.7 installer) + if secret.Type != corev1.SecretTypeTLS { + secret.Type = corev1.SecretTypeTLS + // wipe secret contents if tls.crt and tls.key are missing + _, certExists := secret.Data[corev1.TLSCertKey] + _, keyExists := secret.Data[corev1.TLSPrivateKeyKey] + if !certExists || !keyExists { + secret.Data = map[string][]byte{} + } + return true + } + return false +} diff --git a/pkg/operator/certrotation/signer.go b/pkg/operator/certrotation/signer.go index ca30efb87e..59bf926d5a 100644 --- a/pkg/operator/certrotation/signer.go +++ b/pkg/operator/certrotation/signer.go @@ -72,21 +72,9 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (* } } - needsMetadataUpdate := false - // no ownerReference set - if c.Owner != nil { - needsMetadataUpdate = ensureOwnerReference(&signingCertKeyPairSecret.ObjectMeta, c.Owner) - } - // ownership annotations not set - needsMetadataUpdate = c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&signingCertKeyPairSecret.ObjectMeta) || needsMetadataUpdate - // convert outdated secret type (set pre 4.7) - if signingCertKeyPairSecret.Type != corev1.SecretTypeTLS { - signingCertKeyPairSecret.Type = corev1.SecretTypeTLS - needsMetadataUpdate = true - } - // apply changes (possibly via delete+recreate) if secret exists and requires metadata update + // apply necessary metadata (possibly via delete+recreate) if secret exists // this is done before content update to prevent unexpected rollouts - if needsMetadataUpdate && len(signingCertKeyPairSecret.ResourceVersion) > 0 { + if ensureMetadataUpdate(signingCertKeyPairSecret, c.Owner, c.AdditionalAnnotations) && ensureSecretTLSTypeSet(signingCertKeyPairSecret) { actualSigningCertKeyPairSecret, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, signingCertKeyPairSecret) if err != nil { return nil, err diff --git a/pkg/operator/certrotation/signer_test.go b/pkg/operator/certrotation/signer_test.go index f39e832e96..f58e8c41ba 100644 --- a/pkg/operator/certrotation/signer_test.go +++ b/pkg/operator/certrotation/signer_test.go @@ -15,6 +15,7 @@ import ( clienttesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" + "github.com/openshift/api/annotations" "github.com/openshift/library-go/pkg/operator/events" ) @@ -50,13 +51,30 @@ func TestEnsureSigningCertKeyPair(t *testing.T) { if len(actual.Data["tls.crt"]) == 0 || len(actual.Data["tls.key"]) == 0 { t.Error(actual.Data) } + if len(actual.Annotations) == 0 { + t.Errorf("expected certificates to be annotated") + } + ownershipValue, found := actual.Annotations[annotations.OpenShiftComponent] + if !found { + t.Errorf("expected secret to have ownership annotations, got: %v", actual.Annotations) + } + if ownershipValue != "test" { + t.Errorf("expected ownership annotation to be 'test', got: %v", ownershipValue) + } + if len(actual.OwnerReferences) != 1 { + t.Errorf("expected to have exactly one owner reference") + } + if actual.OwnerReferences[0].Name != "operator" { + t.Errorf("expected owner reference to be 'operator', got %v", actual.OwnerReferences[0].Name) + } }, }, { name: "update no annotations", initialSecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "signer"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "signer", ResourceVersion: "10"}, Type: corev1.SecretTypeTLS, + Data: map[string][]byte{"tls.crt": {}, "tls.key": {}}, }, verifyActions: func(t *testing.T, client *kubefake.Clientset) { t.Helper() @@ -65,10 +83,12 @@ func TestEnsureSigningCertKeyPair(t *testing.T) { t.Fatal(spew.Sdump(actions)) } + if !actions[0].Matches("get", "secrets") { + t.Error(actions[0]) + } if !actions[1].Matches("update", "secrets") { t.Error(actions[1]) } - actual := actions[1].(clienttesting.UpdateAction).GetObject().(*corev1.Secret) if certType, _ := CertificateTypeFromObject(actual); certType != CertificateTypeSigner { t.Errorf("expected certificate type 'signer', got: %v", certType) @@ -76,16 +96,33 @@ func TestEnsureSigningCertKeyPair(t *testing.T) { if len(actual.Data["tls.crt"]) == 0 || len(actual.Data["tls.key"]) == 0 { t.Error(actual.Data) } + ownershipValue, found := actual.Annotations[annotations.OpenShiftComponent] + if !found { + t.Errorf("expected secret to have ownership annotations, got: %v", actual.Annotations) + } + if ownershipValue != "test" { + t.Errorf("expected ownership annotation to be 'test', got: %v", ownershipValue) + } + if len(actual.OwnerReferences) != 1 { + t.Errorf("expected to have exactly one owner reference") + } + if actual.OwnerReferences[0].Name != "operator" { + t.Errorf("expected owner reference to be 'operator', got %v", actual.OwnerReferences[0].Name) + } }, }, { name: "update no work", initialSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "signer", + ResourceVersion: "10", Annotations: map[string]string{ "auth.openshift.io/certificate-not-after": "2108-09-08T22:47:31-07:00", "auth.openshift.io/certificate-not-before": "2108-09-08T20:47:31-07:00", + annotations.OpenShiftComponent: "test", }}, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{"tls.crt": {}, "tls.key": {}}, }, verifyActions: func(t *testing.T, client *kubefake.Clientset) { t.Helper() @@ -96,6 +133,102 @@ func TestEnsureSigningCertKeyPair(t *testing.T) { }, expectedError: "certFile missing", // this means we tried to read the cert from the existing secret. If we created one, we fail in the client check }, + { + name: "update SecretTLSType secrets", + initialSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "signer", + ResourceVersion: "10", + Annotations: map[string]string{ + "auth.openshift.io/certificate-not-after": "2108-09-08T22:47:31-07:00", + "auth.openshift.io/certificate-not-before": "2108-09-08T20:47:31-07:00", + }}, + Type: "SecretTypeTLS", + Data: map[string][]byte{"tls.crt": {}, "tls.key": {}}, + }, + verifyActions: func(t *testing.T, client *kubefake.Clientset) { + t.Helper() + actions := client.Actions() + if len(actions) != 3 { + t.Fatal(spew.Sdump(actions)) + } + + if !actions[0].Matches("get", "secrets") { + t.Error(actions[0]) + } + if !actions[1].Matches("delete", "secrets") { + t.Error(actions[1]) + } + if !actions[2].Matches("create", "secrets") { + t.Error(actions[2]) + } + actual := actions[2].(clienttesting.UpdateAction).GetObject().(*corev1.Secret) + if actual.Type != corev1.SecretTypeTLS { + t.Errorf("expected secret type to be kubernetes.io/tls, got: %v", actual.Type) + } + cert, found := actual.Data["tls.crt"] + if !found { + t.Errorf("expected to have tls.crt key") + } + if len(cert) != 0 { + t.Errorf("expected tls.crt to be empty, got %v", cert) + } + key, found := actual.Data["tls.key"] + if !found { + t.Errorf("expected to have tls.key key") + } + if len(key) != 0 { + t.Errorf("expected tls.key to be empty, got %v", key) + } + if len(actual.OwnerReferences) != 1 { + t.Errorf("expected to have exactly one owner reference") + } + if actual.OwnerReferences[0].Name != "operator" { + t.Errorf("expected owner reference to be 'operator', got %v", actual.OwnerReferences[0].Name) + } + }, + expectedError: "certFile missing", // this means we tried to read the cert from the existing secret. If we created one, we fail in the client check + }, + { + name: "recreate invalid type secrets", + initialSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "signer", + ResourceVersion: "10", + Annotations: map[string]string{ + "auth.openshift.io/certificate-not-after": "2108-09-08T22:47:31-07:00", + "auth.openshift.io/certificate-not-before": "2108-09-08T20:47:31-07:00", + }}, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{"foo": {}, "bar": {}}, + }, + verifyActions: func(t *testing.T, client *kubefake.Clientset) { + t.Helper() + actions := client.Actions() + if len(actions) != 3 { + t.Fatal(spew.Sdump(actions)) + } + + if !actions[0].Matches("get", "secrets") { + t.Error(actions[0]) + } + if !actions[1].Matches("delete", "secrets") { + t.Error(actions[1]) + } + if !actions[2].Matches("create", "secrets") { + t.Error(actions[2]) + } + actual := actions[2].(clienttesting.UpdateAction).GetObject().(*corev1.Secret) + if actual.Type != corev1.SecretTypeTLS { + t.Errorf("expected secret type to be kubernetes.io/tls, got: %v", actual.Type) + } + if len(actual.OwnerReferences) != 1 { + t.Errorf("expected to have exactly one owner reference") + } + if actual.OwnerReferences[0].Name != "operator" { + t.Errorf("expected owner reference to be 'operator', got %v", actual.OwnerReferences[0].Name) + } + }, + expectedError: "certFile missing", // this means we tried to read the cert from the existing secret. If we created one, we fail in the client check + }, } for _, test := range tests { @@ -116,6 +249,12 @@ func TestEnsureSigningCertKeyPair(t *testing.T) { Client: client.CoreV1(), Lister: corev1listers.NewSecretLister(indexer), EventRecorder: events.NewInMemoryRecorder("test"), + AdditionalAnnotations: AdditionalAnnotations{ + JiraComponent: "test", + }, + Owner: &metav1.OwnerReference{ + Name: "operator", + }, } _, err := c.EnsureSigningCertKeyPair(context.TODO()) diff --git a/pkg/operator/certrotation/target.go b/pkg/operator/certrotation/target.go index fb479e72b0..f7e37f4c81 100644 --- a/pkg/operator/certrotation/target.go +++ b/pkg/operator/certrotation/target.go @@ -107,21 +107,9 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont } } - needsMetadataUpdate := false - // no ownerReference set - if c.Owner != nil { - needsMetadataUpdate = ensureOwnerReference(&targetCertKeyPairSecret.ObjectMeta, c.Owner) - } - // ownership annotations not set - needsMetadataUpdate = c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta) || needsMetadataUpdate - // convert outdated secret type (set pre 4.7) - if targetCertKeyPairSecret.Type != corev1.SecretTypeTLS { - targetCertKeyPairSecret.Type = corev1.SecretTypeTLS - needsMetadataUpdate = true - } - // apply changes (possibly via delete+recreate) if secret exists and requires metadata update + // apply necessary metadata (possibly via delete+recreate) if secret exists // this is done before content update to prevent unexpected rollouts - if needsMetadataUpdate && len(targetCertKeyPairSecret.ResourceVersion) > 0 { + if ensureMetadataUpdate(targetCertKeyPairSecret, c.Owner, c.AdditionalAnnotations) && ensureSecretTLSTypeSet(targetCertKeyPairSecret) { actualTargetCertKeyPairSecret, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, targetCertKeyPairSecret) if err != nil { return nil, err diff --git a/pkg/operator/certrotation/target_test.go b/pkg/operator/certrotation/target_test.go index 01428669a8..16ad7c3c25 100644 --- a/pkg/operator/certrotation/target_test.go +++ b/pkg/operator/certrotation/target_test.go @@ -9,6 +9,7 @@ import ( "github.com/davecgh/go-spew/spew" + "github.com/openshift/api/annotations" "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/library-go/pkg/operator/events" @@ -161,9 +162,25 @@ func TestEnsureTargetCertKeyPair(t *testing.T) { } actual := actions[1].(clienttesting.CreateAction).GetObject().(*corev1.Secret) + if len(actual.Annotations) == 0 { + t.Errorf("expected certificates to be annotated") + } + ownershipValue, found := actual.Annotations[annotations.OpenShiftComponent] + if !found { + t.Errorf("expected secret to have ownership annotations, got: %v", actual.Annotations) + } + if ownershipValue != "test" { + t.Errorf("expected ownership annotation to be 'test', got: %v", ownershipValue) + } if len(actual.Data["tls.crt"]) == 0 || len(actual.Data["tls.key"]) == 0 { t.Error(actual.Data) } + if len(actual.OwnerReferences) != 1 { + t.Errorf("expected to have exactly one owner reference") + } + if actual.OwnerReferences[0].Name != "operator" { + t.Errorf("expected owner reference to be 'operator', got %v", actual.OwnerReferences[0].Name) + } }, }, { @@ -173,7 +190,7 @@ func TestEnsureTargetCertKeyPair(t *testing.T) { }, initialSecretFn: func() *corev1.Secret { caBundleSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "target-secret"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "target-secret", ResourceVersion: "10"}, Data: map[string][]byte{}, Type: corev1.SecretTypeTLS, } @@ -190,13 +207,154 @@ func TestEnsureTargetCertKeyPair(t *testing.T) { } actual := actions[1].(clienttesting.UpdateAction).GetObject().(*corev1.Secret) + if len(actual.Annotations) == 0 { + t.Errorf("expected certificates to be annotated") + } + ownershipValue, found := actual.Annotations[annotations.OpenShiftComponent] + if !found { + t.Errorf("expected secret to have ownership annotations, got: %v", actual.Annotations) + } + if ownershipValue != "test" { + t.Errorf("expected ownership annotation to be 'test', got: %v", ownershipValue) + } if len(actual.Data["tls.crt"]) == 0 || len(actual.Data["tls.key"]) == 0 { t.Error(actual.Data) } if actual.Annotations[CertificateHostnames] != "bar,foo" { t.Error(actual.Annotations[CertificateHostnames]) } + if len(actual.OwnerReferences) != 1 { + t.Errorf("expected to have exactly one owner reference") + } + if actual.OwnerReferences[0].Name != "operator" { + t.Errorf("expected owner reference to be 'operator', got %v", actual.OwnerReferences[0].Name) + } + }, + }, + { + name: "update SecretTLSType secrets", + caFn: func() (*crypto.CA, error) { + return newTestCACertificate(pkix.Name{CommonName: "signer-tests"}, int64(1), metav1.Duration{Duration: time.Hour * 24 * 60}, time.Now) + }, + initialSecretFn: func() *corev1.Secret { + caBundleSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "target-secret", ResourceVersion: "10"}, + Data: map[string][]byte{}, + Type: "SecretTypeTLS", + } + return caBundleSecret + }, + verifyActions: func(t *testing.T, client *kubefake.Clientset) { + actions := client.Actions() + if len(actions) != 5 { + t.Fatal(spew.Sdump(actions)) + } + + if !actions[0].Matches("get", "secrets") { + t.Error(actions[0]) + } + if !actions[1].Matches("delete", "secrets") { + t.Error(actions[1]) + } + if !actions[2].Matches("create", "secrets") { + t.Error(actions[2]) + } + if !actions[3].Matches("get", "secrets") { + t.Error(actions[3]) + } + if !actions[4].Matches("update", "secrets") { + t.Error(actions[4]) + } + actual := actions[4].(clienttesting.UpdateAction).GetObject().(*corev1.Secret) + if len(actual.Annotations) == 0 { + t.Errorf("expected certificates to be annotated") + } + ownershipValue, found := actual.Annotations[annotations.OpenShiftComponent] + if !found { + t.Errorf("expected secret to have ownership annotations, got: %v", actual.Annotations) + } + if ownershipValue != "test" { + t.Errorf("expected ownership annotation to be 'test', got: %v", ownershipValue) + } + if actual.Type != corev1.SecretTypeTLS { + t.Errorf("expected secret type to be kubernetes.io/tls, got: %v", actual.Type) + } + if len(actual.Data["tls.crt"]) == 0 || len(actual.Data["tls.key"]) == 0 { + t.Error(actual.Data) + } + if actual.Annotations[CertificateHostnames] != "bar,foo" { + t.Error(actual.Annotations[CertificateHostnames]) + } + if len(actual.OwnerReferences) != 1 { + t.Errorf("expected to have exactly one owner reference") + } + if actual.OwnerReferences[0].Name != "operator" { + t.Errorf("expected owner reference to be 'operator', got %v", actual.OwnerReferences[0].Name) + } + }, + }, + { + name: "recreate invalid secret type", + caFn: func() (*crypto.CA, error) { + return newTestCACertificate(pkix.Name{CommonName: "signer-tests"}, int64(1), metav1.Duration{Duration: time.Hour * 24 * 60}, time.Now) + }, + initialSecretFn: func() *corev1.Secret { + caBundleSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "target-secret", ResourceVersion: "10"}, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{"foo": {}, "bar": {}}, + } + return caBundleSecret + }, + verifyActions: func(t *testing.T, client *kubefake.Clientset) { + actions := client.Actions() + if len(actions) != 5 { + t.Fatal(spew.Sdump(actions)) + } + + if !actions[0].Matches("get", "secrets") { + t.Error(actions[0]) + } + if !actions[1].Matches("delete", "secrets") { + t.Error(actions[1]) + } + if !actions[2].Matches("create", "secrets") { + t.Error(actions[2]) + } + if !actions[3].Matches("get", "secrets") { + t.Error(actions[3]) + } + if !actions[4].Matches("update", "secrets") { + t.Error(actions[4]) + } + + actual := actions[4].(clienttesting.UpdateAction).GetObject().(*corev1.Secret) + if len(actual.Annotations) == 0 { + t.Errorf("expected certificates to be annotated") + } + ownershipValue, found := actual.Annotations[annotations.OpenShiftComponent] + if !found { + t.Errorf("expected secret to have ownership annotations, got: %v", actual.Annotations) + } + if ownershipValue != "test" { + t.Errorf("expected ownership annotation to be 'test', got: %v", ownershipValue) + } + if actual.Type != corev1.SecretTypeTLS { + t.Errorf("expected secret type to be kubernetes.io/tls, got: %v", actual.Type) + } + if len(actual.Data["tls.crt"]) == 0 || len(actual.Data["tls.key"]) == 0 { + t.Error(actual.Data) + } + if actual.Annotations[CertificateHostnames] != "bar,foo" { + t.Error(actual.Annotations[CertificateHostnames]) + } + if len(actual.OwnerReferences) != 1 { + t.Errorf("expected to have exactly one owner reference") + } + if actual.OwnerReferences[0].Name != "operator" { + t.Errorf("expected owner reference to be 'operator', got %v", actual.OwnerReferences[0].Name) + } }, }, } @@ -223,6 +381,12 @@ func TestEnsureTargetCertKeyPair(t *testing.T) { Client: client.CoreV1(), Lister: corev1listers.NewSecretLister(indexer), EventRecorder: events.NewInMemoryRecorder("test"), + AdditionalAnnotations: AdditionalAnnotations{ + JiraComponent: "test", + }, + Owner: &metav1.OwnerReference{ + Name: "operator", + }, } newCA, err := test.caFn()