From 46940d1d75b6af8fa690e35ff6d32543ac189fd6 Mon Sep 17 00:00:00 2001 From: loomt Date: Wed, 25 Sep 2024 00:06:43 +0800 Subject: [PATCH 1/3] check data field for user provided tls --- pkg/controller/plan/tls_utils.go | 4 ++-- pkg/controller/plan/tls_utils_test.go | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/pkg/controller/plan/tls_utils.go b/pkg/controller/plan/tls_utils.go index dc54213dc97..eab2aa9022d 100644 --- a/pkg/controller/plan/tls_utils.go +++ b/pkg/controller/plan/tls_utils.go @@ -109,12 +109,12 @@ func CheckTLSSecretRef(ctx context.Context, cli client.Reader, namespace string, if err := cli.Get(ctx, types.NamespacedName{Namespace: namespace, Name: secretRef.Name}, secret); err != nil { return err } - if secret.StringData == nil { + if secret.StringData == nil && secret.Data == nil { return errors.New("tls secret's data field shouldn't be nil") } keys := []string{secretRef.CA, secretRef.Cert, secretRef.Key} for _, key := range keys { - if _, ok := secret.StringData[key]; !ok { + if (secret.StringData != nil && secret.StringData[key] == "") || (secret.Data != nil && secret.Data[key] == nil) { return errors.Errorf("tls secret's data[%s] field shouldn't be empty", key) } } diff --git a/pkg/controller/plan/tls_utils_test.go b/pkg/controller/plan/tls_utils_test.go index db9cc87b2a9..8e32e9e61bc 100644 --- a/pkg/controller/plan/tls_utils_test.go +++ b/pkg/controller/plan/tls_utils_test.go @@ -115,7 +115,7 @@ var _ = Describe("TLSUtilsTest", func() { Expect(err).ShouldNot(BeNil()) Expect(err.Error()).Should(ContainSubstring(secretRef.CA)) - By("set everything ok") + By("stringData field is ok") k8sMock.EXPECT(). Get(gomock.Any(), gomock.Any(), &corev1.Secret{}, gomock.Any()). DoAndReturn(func(_ context.Context, objKey client.ObjectKey, obj *corev1.Secret, _ ...client.GetOption) error { @@ -130,6 +130,22 @@ var _ = Describe("TLSUtilsTest", func() { return nil }).Times(1) Expect(CheckTLSSecretRef(ctx, k8sMock, namespace, secretRef)).Should(Succeed()) + + By("data field is ok") + k8sMock.EXPECT(). + Get(gomock.Any(), gomock.Any(), &corev1.Secret{}, gomock.Any()). + DoAndReturn(func(_ context.Context, objKey client.ObjectKey, obj *corev1.Secret, _ ...client.GetOption) error { + Expect(obj).ShouldNot(BeNil()) + obj.Namespace = objKey.Namespace + obj.Name = objKey.Name + obj.Data = map[string][]byte{ + secretRef.Cert: []byte("foo"), + secretRef.Key: []byte("bar"), + secretRef.CA: []byte("ca"), + } + return nil + }).Times(1) + Expect(CheckTLSSecretRef(ctx, k8sMock, namespace, secretRef)).Should(Succeed()) }) Context("GetTLSKeyWord function", func() { From 6b0bdfabf34e2eda6161dc15e820ba865c588759 Mon Sep 17 00:00:00 2001 From: loomt Date: Wed, 25 Sep 2024 12:15:01 +0800 Subject: [PATCH 2/3] fix data check --- pkg/controller/plan/tls_utils.go | 2 +- pkg/controller/plan/tls_utils_test.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/pkg/controller/plan/tls_utils.go b/pkg/controller/plan/tls_utils.go index eab2aa9022d..500d4e9800e 100644 --- a/pkg/controller/plan/tls_utils.go +++ b/pkg/controller/plan/tls_utils.go @@ -114,7 +114,7 @@ func CheckTLSSecretRef(ctx context.Context, cli client.Reader, namespace string, } keys := []string{secretRef.CA, secretRef.Cert, secretRef.Key} for _, key := range keys { - if (secret.StringData != nil && secret.StringData[key] == "") || (secret.Data != nil && secret.Data[key] == nil) { + if (secret.StringData != nil && secret.StringData[key] == "") || (secret.Data != nil && len(secret.Data[key]) == 0) { return errors.Errorf("tls secret's data[%s] field shouldn't be empty", key) } } diff --git a/pkg/controller/plan/tls_utils_test.go b/pkg/controller/plan/tls_utils_test.go index 8e32e9e61bc..6b1b0d25d1e 100644 --- a/pkg/controller/plan/tls_utils_test.go +++ b/pkg/controller/plan/tls_utils_test.go @@ -115,6 +115,24 @@ var _ = Describe("TLSUtilsTest", func() { Expect(err).ShouldNot(BeNil()) Expect(err.Error()).Should(ContainSubstring(secretRef.CA)) + By("set empty CA in map Data") + k8sMock.EXPECT(). + Get(gomock.Any(), gomock.Any(), &corev1.Secret{}, gomock.Any()). + DoAndReturn(func(_ context.Context, objKey client.ObjectKey, obj *corev1.Secret, _ ...client.GetOption) error { + Expect(obj).ShouldNot(BeNil()) + obj.Namespace = objKey.Namespace + obj.Name = objKey.Name + obj.Data = map[string][]byte{ + secretRef.Cert: []byte("foo"), + secretRef.Key: []byte("bar"), + secretRef.CA: []byte(""), + } + return nil + }).Times(1) + err = CheckTLSSecretRef(ctx, k8sMock, namespace, secretRef) + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).Should(ContainSubstring(secretRef.CA)) + By("stringData field is ok") k8sMock.EXPECT(). Get(gomock.Any(), gomock.Any(), &corev1.Secret{}, gomock.Any()). From 985146606bc5871509f091626db472b0b2c090a7 Mon Sep 17 00:00:00 2001 From: loomt Date: Thu, 26 Sep 2024 10:33:38 +0800 Subject: [PATCH 3/3] del useless check --- pkg/controller/plan/tls_utils.go | 4 +-- pkg/controller/plan/tls_utils_test.go | 48 +-------------------------- 2 files changed, 3 insertions(+), 49 deletions(-) diff --git a/pkg/controller/plan/tls_utils.go b/pkg/controller/plan/tls_utils.go index 500d4e9800e..63f60dee1e2 100644 --- a/pkg/controller/plan/tls_utils.go +++ b/pkg/controller/plan/tls_utils.go @@ -109,12 +109,12 @@ func CheckTLSSecretRef(ctx context.Context, cli client.Reader, namespace string, if err := cli.Get(ctx, types.NamespacedName{Namespace: namespace, Name: secretRef.Name}, secret); err != nil { return err } - if secret.StringData == nil && secret.Data == nil { + if secret.Data == nil { return errors.New("tls secret's data field shouldn't be nil") } keys := []string{secretRef.CA, secretRef.Cert, secretRef.Key} for _, key := range keys { - if (secret.StringData != nil && secret.StringData[key] == "") || (secret.Data != nil && len(secret.Data[key]) == 0) { + if len(secret.Data[key]) == 0 { return errors.Errorf("tls secret's data[%s] field shouldn't be empty", key) } } diff --git a/pkg/controller/plan/tls_utils_test.go b/pkg/controller/plan/tls_utils_test.go index 6b1b0d25d1e..1ddea11b3a8 100644 --- a/pkg/controller/plan/tls_utils_test.go +++ b/pkg/controller/plan/tls_utils_test.go @@ -85,36 +85,6 @@ var _ = Describe("TLSUtilsTest", func() { err := CheckTLSSecretRef(ctx, k8sMock, namespace, secretRef) Expect(apierrors.IsNotFound(err)).Should(BeTrue()) - By("set stringData to nil") - k8sMock.EXPECT(). - Get(gomock.Any(), gomock.Any(), &corev1.Secret{}, gomock.Any()). - DoAndReturn(func(_ context.Context, objKey client.ObjectKey, obj *corev1.Secret, _ ...client.GetOption) error { - Expect(obj).ShouldNot(BeNil()) - obj.Namespace = objKey.Namespace - obj.Name = objKey.Name - return nil - }).Times(1) - err = CheckTLSSecretRef(ctx, k8sMock, namespace, secretRef) - Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(ContainSubstring("tls secret's data field shouldn't be nil")) - - By("set no CA key in map stringData") - k8sMock.EXPECT(). - Get(gomock.Any(), gomock.Any(), &corev1.Secret{}, gomock.Any()). - DoAndReturn(func(_ context.Context, objKey client.ObjectKey, obj *corev1.Secret, _ ...client.GetOption) error { - Expect(obj).ShouldNot(BeNil()) - obj.Namespace = objKey.Namespace - obj.Name = objKey.Name - obj.StringData = map[string]string{ - secretRef.Cert: "foo", - secretRef.Key: "bar", - } - return nil - }).Times(1) - err = CheckTLSSecretRef(ctx, k8sMock, namespace, secretRef) - Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(ContainSubstring(secretRef.CA)) - By("set empty CA in map Data") k8sMock.EXPECT(). Get(gomock.Any(), gomock.Any(), &corev1.Secret{}, gomock.Any()). @@ -133,23 +103,7 @@ var _ = Describe("TLSUtilsTest", func() { Expect(err).ShouldNot(BeNil()) Expect(err.Error()).Should(ContainSubstring(secretRef.CA)) - By("stringData field is ok") - k8sMock.EXPECT(). - Get(gomock.Any(), gomock.Any(), &corev1.Secret{}, gomock.Any()). - DoAndReturn(func(_ context.Context, objKey client.ObjectKey, obj *corev1.Secret, _ ...client.GetOption) error { - Expect(obj).ShouldNot(BeNil()) - obj.Namespace = objKey.Namespace - obj.Name = objKey.Name - obj.StringData = map[string]string{ - secretRef.Cert: "foo", - secretRef.Key: "bar", - secretRef.CA: "ca", - } - return nil - }).Times(1) - Expect(CheckTLSSecretRef(ctx, k8sMock, namespace, secretRef)).Should(Succeed()) - - By("data field is ok") + By("set everything ok") k8sMock.EXPECT(). Get(gomock.Any(), gomock.Any(), &corev1.Secret{}, gomock.Any()). DoAndReturn(func(_ context.Context, objKey client.ObjectKey, obj *corev1.Secret, _ ...client.GetOption) error {