From 511d89c15296e0fe3bc18a9d406cddd64d465790 Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Thu, 30 Aug 2018 09:54:56 -0700 Subject: [PATCH 1/3] *: test the case where only the application TLS asset exist --- pkg/tlsutil/tls.go | 2 +- test/e2e/tls_util_test.go | 112 ++++++++++++++++++++++++++------------ 2 files changed, 78 insertions(+), 36 deletions(-) diff --git a/pkg/tlsutil/tls.go b/pkg/tlsutil/tls.go index 4e70cffabf2..3cead2f8e21 100644 --- a/pkg/tlsutil/tls.go +++ b/pkg/tlsutil/tls.go @@ -167,7 +167,7 @@ func (scg *SDKCertGenerator) GenerateCert(cr runtime.Object, service *v1.Service if hasAppSecret && hasCASecretAndConfigMap { return appSecret, caConfigMap, caSecret, nil } else if hasAppSecret && !hasCASecretAndConfigMap { - // TODO + return nil, nil, nil, errors.New("ca secret and configMap are not found") } else if !hasAppSecret && hasCASecretAndConfigMap { // TODO } else { diff --git a/test/e2e/tls_util_test.go b/test/e2e/tls_util_test.go index 3f794d9cd16..4d3b23167ab 100644 --- a/test/e2e/tls_util_test.go +++ b/test/e2e/tls_util_test.go @@ -25,6 +25,35 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +var ( + // TLS test variables. + crKind = "Pod" + crName = "example-pod" + certName = "app-cert" + + caConfigMapAndSecretName = tlsutil.ToCASecretAndConfigMapName(crKind, crName) + caConfigMap = &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: caConfigMapAndSecretName, + }, + } + caSecret = &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: caConfigMapAndSecretName, + }, + } + + appSecret = &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: tlsutil.ToAppSecretName(crKind, crName, certName), + }, + } + + ccfg = &tlsutil.CertConfig{ + CertName: certName, + } +) + // TestBothAppAndCATLSAssetsExist ensures that when both application // and CA TLS assets exist in the k8s cluster for a given cr, // the GenerateCert() simply returns those to the caller. @@ -37,9 +66,23 @@ func TestBothAppAndCATLSAssetsExist(t *testing.T) { t.Fatal(err) } + appSecret, err := f.KubeClient.CoreV1().Secrets(namespace).Create(appSecret) + if err != nil { + t.Fatal(err) + } + + caConfigMap, err := f.KubeClient.CoreV1().ConfigMaps(namespace).Create(caConfigMap) + if err != nil { + t.Fatal(err) + } + + caSecret, err := f.KubeClient.CoreV1().Secrets(namespace).Create(caSecret) + if err != nil { + t.Fatal(err) + } + + cg := tlsutil.NewSDKCertGenerator(f.KubeClient) // Use Pod as a dummy runtime object for the CR input of GenerateCert(). - crKind := "Pod" - crName := "example-pod" mCR := &v1.Pod{ TypeMeta: metav1.TypeMeta{ Kind: crKind, @@ -49,55 +92,54 @@ func TestBothAppAndCATLSAssetsExist(t *testing.T) { Namespace: namespace, }, } - - certName := "app-cert" - appSecret := &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: tlsutil.ToAppSecretName(crKind, crName, certName), - }, - } - appSecret, err = f.KubeClient.CoreV1().Secrets(namespace).Create(appSecret) + actualAppSecret, actualCaConfigMap, actualCaSecret, err := cg.GenerateCert(mCR, nil, ccfg) if err != nil { t.Fatal(err) } - caConfigMapAndSecretName := tlsutil.ToCASecretAndConfigMapName(crKind, crName) - caConfigMap := &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: caConfigMapAndSecretName, - }, + if !reflect.DeepEqual(appSecret, actualAppSecret) { + t.Fatalf("expect %+v, but got %+v", appSecret, actualAppSecret) } - caConfigMap, err = f.KubeClient.CoreV1().ConfigMaps(namespace).Create(caConfigMap) - if err != nil { - t.Fatal(err) + if !reflect.DeepEqual(caConfigMap, actualCaConfigMap) { + t.Fatalf("expect %+v, but got %+v", caConfigMap, actualCaConfigMap) } - - caSecret := &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: caConfigMapAndSecretName, - }, + if !reflect.DeepEqual(caSecret, actualCaSecret) { + t.Fatalf("expect %+v, but got %+v", caSecret, actualCaSecret) } - caSecret, err = f.KubeClient.CoreV1().Secrets(namespace).Create(caSecret) +} + +// TestOnlyAppSecretExist tests a case where the application TLS asset exists but its correspoding CA asset doesn't. In this case, CertGenerator can't genereate a new CA because it won't verify the existing application TLS cert. Therefore, CertGenerator can't proceed and returns an error to the caller. +func TestOnlyAppSecretExist(t *testing.T) { + f := framework.Global + ctx := f.NewTestCtx(t) + defer ctx.Cleanup(t) + namespace, err := ctx.GetNamespace() if err != nil { t.Fatal(err) } - cg := tlsutil.NewSDKCertGenerator(f.KubeClient) - ccfg := &tlsutil.CertConfig{ - CertName: certName, - } - actualAppSecret, actualCaConfigMap, actualCaSecret, err := cg.GenerateCert(mCR, nil, ccfg) + _, err = f.KubeClient.CoreV1().Secrets(namespace).Create(appSecret) if err != nil { t.Fatal(err) } - if !reflect.DeepEqual(appSecret, actualAppSecret) { - t.Fatalf("expect %+v, got %+v", appSecret, actualAppSecret) + cg := tlsutil.NewSDKCertGenerator(f.KubeClient) + // Use Pod as a dummy runtime object for the CR input of GenerateCert(). + mCR := &v1.Pod{ + TypeMeta: metav1.TypeMeta{ + Kind: crKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: crName, + Namespace: namespace, + }, } - if !reflect.DeepEqual(caConfigMap, actualCaConfigMap) { - t.Fatalf("expect %+v, got %+v", caConfigMap, actualCaConfigMap) + _, _, _, err = cg.GenerateCert(mCR, nil, ccfg) + if err == nil { + t.Fatal("expect error, but got none") } - if !reflect.DeepEqual(caSecret, actualCaSecret) { - t.Fatalf("expect %+v, got %+v", caSecret, actualCaSecret) + expErrMsg := "ca secret and configMap are not found" + if err.Error() != expErrMsg { + t.Fatalf("expect %v, but got %v", expErrMsg, err.Error()) } } From 5bc7bc62ed87032d7018b0fecaf22b08db7718c4 Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Thu, 30 Aug 2018 10:21:51 -0700 Subject: [PATCH 2/3] pkg/tlsutil: don't return error if error is not found *: test the case when both app and CA TLS assets exist --- pkg/tlsutil/tls.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/tlsutil/tls.go b/pkg/tlsutil/tls.go index 3cead2f8e21..53002775194 100644 --- a/pkg/tlsutil/tls.go +++ b/pkg/tlsutil/tls.go @@ -196,12 +196,12 @@ func ToCASecretAndConfigMapName(kind, name string) string { func getAppSecretInCluster(kubeClient kubernetes.Interface, name, namespace string) (*v1.Secret, error) { se, err := kubeClient.CoreV1().Secrets(namespace).Get(name, metav1.GetOptions{}) + if err != nil && !apiErrors.IsNotFound(err) { + return nil, err + } if apiErrors.IsNotFound(err) { return nil, nil } - if err != nil { - return nil, err - } return se, nil } @@ -210,21 +210,21 @@ func getAppSecretInCluster(kubeClient kubernetes.Interface, name, namespace stri func getCASecretAndConfigMapInCluster(kubeClient kubernetes.Interface, name, namespace string) (*v1.Secret, *v1.ConfigMap, error) { hasConfigMap := true cm, err := kubeClient.CoreV1().ConfigMaps(namespace).Get(name, metav1.GetOptions{}) + if err != nil && !apiErrors.IsNotFound(err) { + return nil, nil, err + } if apiErrors.IsNotFound(err) { hasConfigMap = false } - if err != nil { - return nil, nil, err - } hasSecret := true se, err := kubeClient.CoreV1().Secrets(namespace).Get(name, metav1.GetOptions{}) + if err != nil && !apiErrors.IsNotFound(err) { + return nil, nil, err + } if apiErrors.IsNotFound(err) { hasSecret = false } - if err != nil { - return nil, nil, err - } if hasConfigMap != hasSecret { // TODO: this case can happen if creating CA configmap succeeds and creating CA secret failed. We need to handle this case properly. From 14957eb8776765c219687ef0016736ea596a5ccc Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Thu, 30 Aug 2018 12:28:57 -0700 Subject: [PATCH 3/3] *: define errors as constants in tlsutil/errors.go --- pkg/tlsutil/error.go | 22 ++++++++++++++++++++++ pkg/tlsutil/tls.go | 2 +- test/e2e/tls_util_test.go | 5 ++--- 3 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 pkg/tlsutil/error.go diff --git a/pkg/tlsutil/error.go b/pkg/tlsutil/error.go new file mode 100644 index 00000000000..f0cf6eadb59 --- /dev/null +++ b/pkg/tlsutil/error.go @@ -0,0 +1,22 @@ +// Copyright 2018 The Operator-SDK Authors +// +// 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. + +package tlsutil + +import "errors" + +var ( + ErrCANotFound = errors.New("ca secret and configMap are not found") + // TODO: add other tls util errors. +) diff --git a/pkg/tlsutil/tls.go b/pkg/tlsutil/tls.go index 53002775194..c2cb284a6b1 100644 --- a/pkg/tlsutil/tls.go +++ b/pkg/tlsutil/tls.go @@ -167,7 +167,7 @@ func (scg *SDKCertGenerator) GenerateCert(cr runtime.Object, service *v1.Service if hasAppSecret && hasCASecretAndConfigMap { return appSecret, caConfigMap, caSecret, nil } else if hasAppSecret && !hasCASecretAndConfigMap { - return nil, nil, nil, errors.New("ca secret and configMap are not found") + return nil, nil, nil, ErrCANotFound } else if !hasAppSecret && hasCASecretAndConfigMap { // TODO } else { diff --git a/test/e2e/tls_util_test.go b/test/e2e/tls_util_test.go index 4d3b23167ab..8d9584b8e0f 100644 --- a/test/e2e/tls_util_test.go +++ b/test/e2e/tls_util_test.go @@ -138,8 +138,7 @@ func TestOnlyAppSecretExist(t *testing.T) { if err == nil { t.Fatal("expect error, but got none") } - expErrMsg := "ca secret and configMap are not found" - if err.Error() != expErrMsg { - t.Fatalf("expect %v, but got %v", expErrMsg, err.Error()) + if err != tlsutil.ErrCANotFound { + t.Fatalf("expect %v, but got %v", tlsutil.ErrCANotFound.Error(), err.Error()) } }