[OOM-K8S-CERT-EXTERNAL-PROVIDER] Add check if cert should be updated 51/122451/3
authorRemigiusz Janeczek <remigiusz.janeczek@nokia.com>
Tue, 6 Jul 2021 11:33:51 +0000 (13:33 +0200)
committerRemigiusz Janeczek <remigiusz.janeczek@nokia.com>
Thu, 8 Jul 2021 07:16:23 +0000 (09:16 +0200)
Issue-ID: OOM-2753
Signed-off-by: Remigiusz Janeczek <remigiusz.janeczek@nokia.com>
Change-Id: If0d7154b39c9ca7f9a7942f61b93725405b8f4e8

certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go
certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller_test.go
certServiceK8sExternalProvider/src/cmpv2controller/util/certificate_update_util.go [new file with mode: 0644]
certServiceK8sExternalProvider/src/cmpv2controller/util/certificate_update_util_test.go [new file with mode: 0644]

index 51d1359..1032ee0 100644 (file)
@@ -3,7 +3,7 @@
  * oom-certservice-k8s-external-provider
  * ================================================================================
  * Copyright 2019 The cert-manager authors.
- * Modifications copyright (C) 2020 Nokia. All rights reserved.
+ * Modifications copyright (C) 2020-2021 Nokia. All rights reserved.
  * ================================================================================
  * This source code was copied from the following git repository:
  * https://github.com/smallstep/step-issuer
@@ -40,6 +40,7 @@ import (
        "onap.org/oom-certservice/k8s-external-provider/src/cmpv2api"
        "onap.org/oom-certservice/k8s-external-provider/src/cmpv2controller/logger"
        "onap.org/oom-certservice/k8s-external-provider/src/cmpv2controller/updater"
+       "onap.org/oom-certservice/k8s-external-provider/src/cmpv2controller/util"
        provisioners "onap.org/oom-certservice/k8s-external-provider/src/cmpv2provisioner"
        "onap.org/oom-certservice/k8s-external-provider/src/leveledlogger"
        x509utils "onap.org/oom-certservice/k8s-external-provider/src/x509"
@@ -137,14 +138,22 @@ func (controller *CertificateRequestController) Reconcile(k8sRequest ctrl.Reques
        // 9. Log Certificate Request properties not supported or overridden by CertService API
        logger.LogCertRequestProperties(leveledlogger.GetLoggerWithName("CSR details:"), certificateRequest, csr)
 
-       // 10. Sign CertificateRequest
+       // 10. Check if CertificateRequest is an update request
+       isUpdateRevision, oldCertificate, oldPrivateKey := util.CheckIfCertificateUpdateAndRetrieveOldCertificateAndPk(
+               controller.Client, certificateRequest, ctx)
+       if isUpdateRevision {
+               log.Debug("Certificate will be updated.", "old-certificate", oldCertificate,
+                       "old-private-key", oldPrivateKey) //TODO: remove private key from logger
+       }
+
+       // 11. Sign CertificateRequest
        signedPEM, trustedCAs, err := provisioner.Sign(ctx, certificateRequest, privateKeyBytes)
        if err != nil {
                controller.handleErrorFailedToSignCertificate(certUpdater, log, err)
                return ctrl.Result{}, nil
        }
 
-       // 11. Store signed certificates in CertificateRequest
+       // 12. Store signed certificates in CertificateRequest
        certificateRequest.Status.Certificate = signedPEM
        certificateRequest.Status.CA = trustedCAs
        if err := certUpdater.UpdateCertificateRequestWithSignedCertificates(); err != nil {
index 24b6b89..10f88c0 100644 (file)
@@ -53,6 +53,7 @@ func Test_shouldSaveCorrectSignedPems_whenRequestReceived(t *testing.T) {
        createProvisioner(verifiedIssuer)
        fakeClient := fake.NewFakeClientWithScheme(testdata.GetScheme(), &verifiedIssuer,
                getValidCertificateRequest(), getValidPrivateKeySecret())
+
        fakeRecorder := record.NewFakeRecorder(recorderBufferSize)
        controller := getCertRequestController(fakeRecorder, fakeClient)
        fakeRequest := testdata.GetFakeRequest(certificateRequestName)
diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/util/certificate_update_util.go b/certServiceK8sExternalProvider/src/cmpv2controller/util/certificate_update_util.go
new file mode 100644 (file)
index 0000000..93746b8
--- /dev/null
@@ -0,0 +1,89 @@
+/*
+ * ============LICENSE_START=======================================================
+ * oom-certservice-k8s-external-provider
+ * ================================================================================
+ * Copyright (C) 2021 Nokia. All rights reserved.
+ * ================================================================================
+ * This source code was copied from the following git repository:
+ * https://github.com/smallstep/step-issuer
+ * The source code was modified for usage in the ONAP project.
+ * ================================================================================
+ * 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.
+ * ============LICENSE_END=========================================================
+ */
+
+package util
+
+import (
+       "context"
+       "encoding/base64"
+       "encoding/json"
+       "strconv"
+
+       cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
+       core "k8s.io/api/core/v1"
+       "k8s.io/apimachinery/pkg/types"
+       "sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+const (
+       revisionAnnotation                 = "cert-manager.io/certificate-revision"
+       certificateConfigurationAnnotation = "kubectl.kubernetes.io/last-applied-configuration"
+       oldCertificateSecretKey            = "tls.crt"
+       oldPrivateKeySecretKey             = "tls.key"
+)
+
+func CheckIfCertificateUpdateAndRetrieveOldCertificateAndPk(
+       k8sClient client.Client,
+       certificateRequest *cmapi.CertificateRequest,
+       ctx context.Context,
+) (bool, string, string) {
+       if !IsUpdateCertificateRevision(certificateRequest) {
+               return false, "", ""
+       }
+       certificate, privateKey := RetrieveOldCertificateAndPk(k8sClient, certificateRequest, ctx)
+       areCertAndPkPresent := certificate != "" && privateKey != ""
+       return areCertAndPkPresent, certificate, privateKey
+}
+
+func IsUpdateCertificateRevision(certificateRequest *cmapi.CertificateRequest) bool {
+       revision, err := strconv.Atoi(certificateRequest.ObjectMeta.Annotations[revisionAnnotation])
+       if err != nil {
+               return false
+       }
+       return revision > 1
+}
+
+func RetrieveOldCertificateAndPk(
+       k8sClient client.Client,
+       certificateRequest *cmapi.CertificateRequest,
+       ctx context.Context,
+) (string, string) {
+       certificateConfigString := certificateRequest.ObjectMeta.Annotations[certificateConfigurationAnnotation]
+       var certificateConfig cmapi.Certificate
+       if err := json.Unmarshal([]byte(certificateConfigString), &certificateConfig); err != nil {
+               return "", ""
+       }
+       oldCertificateSecretName := certificateConfig.Spec.SecretName
+       oldCertificateSecretNamespacedName := types.NamespacedName{
+               Namespace: certificateConfig.Namespace,
+               Name:      oldCertificateSecretName,
+       }
+       var oldCertificateSecret core.Secret
+       if err := k8sClient.Get(ctx, oldCertificateSecretNamespacedName, &oldCertificateSecret); err != nil {
+               return "", ""
+       }
+       oldCertificateString := base64.StdEncoding.EncodeToString(oldCertificateSecret.Data[oldCertificateSecretKey])
+       oldPrivateKeyString := base64.StdEncoding.EncodeToString(oldCertificateSecret.Data[oldPrivateKeySecretKey])
+       return oldCertificateString, oldPrivateKeyString
+}
diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/util/certificate_update_util_test.go b/certServiceK8sExternalProvider/src/cmpv2controller/util/certificate_update_util_test.go
new file mode 100644 (file)
index 0000000..7dbbbe7
--- /dev/null
@@ -0,0 +1,153 @@
+/*
+ * ============LICENSE_START=======================================================
+ * oom-certservice-k8s-external-provider
+ * ================================================================================
+ * Copyright (C) 2021 Nokia. All rights reserved.
+ * ================================================================================
+ * This source code was copied from the following git repository:
+ * https://github.com/smallstep/step-issuer
+ * The source code was modified for usage in the ONAP project.
+ * ================================================================================
+ * 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.
+ * ============LICENSE_END=========================================================
+ */
+
+package util
+
+import (
+       "encoding/base64"
+       "fmt"
+       "testing"
+
+       cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
+       "github.com/stretchr/testify/assert"
+       v1 "k8s.io/api/core/v1"
+       metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+       "onap.org/oom-certservice/k8s-external-provider/src/testdata"
+       "sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+const (
+       oldCertificateConfig = "{\"apiVersion\":\"cert-manager.io/v1\",\"kind\":\"Certificate\",\"metadata\":{\"annotations\":{},\"name\":\"cert-test\",\"namespace\":\"onap\"},\"spec\":{\"commonName\":\"certissuer.onap.org\",\"dnsNames\":[\"localhost\",\"certissuer.onap.org\"],\"emailAddresses\":[\"onap@onap.org\"],\"ipAddresses\":[\"127.0.0.1\"],\"issuerRef\":{\"group\":\"certmanager.onap.org\",\"kind\":\"CMPv2Issuer\",\"name\":\"cmpv2-issuer-onap\"},\"secretName\":\"cert-test-secret-name\",\"subject\":{\"countries\":[\"US\"],\"localities\":[\"San-Francisco\"],\"organizationalUnits\":[\"ONAP\"],\"organizations\":[\"Linux-Foundation\"],\"provinces\":[\"California\"]},\"uris\":[\"onap://cluster.local/\"]}}\n"
+       testPrivateKeyData   = "test-private-key"
+       testCertificateData  = "test-certificate"
+)
+
+func Test_CheckIfCertificateUpdateAndRetrieveOldCertificateAndPk_revisionOne(t *testing.T) {
+       request := new(cmapi.CertificateRequest)
+       request.ObjectMeta.Annotations = map[string]string{
+               revisionAnnotation: "2",
+       }
+       isUpdate, certificate, privateKey := CheckIfCertificateUpdateAndRetrieveOldCertificateAndPk(nil, request, nil)
+       assert.False(t, isUpdate)
+       assert.Equal(t, "", certificate)
+       assert.Equal(t, "", privateKey)
+}
+
+func Test_CheckIfCertificateUpdateAndRetrieveOldCertificateAndPk_revisionTwoSecretPresent(t *testing.T) {
+       request := new(cmapi.CertificateRequest)
+       request.ObjectMeta.Annotations = map[string]string{
+               revisionAnnotation:                 "2",
+               certificateConfigurationAnnotation: oldCertificateConfig,
+       }
+       fakeClient := fake.NewFakeClientWithScheme(testdata.GetScheme(), getValidCertificateSecret())
+       isUpdate, certificate, privateKey := CheckIfCertificateUpdateAndRetrieveOldCertificateAndPk(fakeClient, request, nil)
+       assert.True(t, isUpdate)
+       assert.Equal(t, base64.StdEncoding.EncodeToString([]byte(testCertificateData)), certificate)
+       assert.Equal(t, base64.StdEncoding.EncodeToString([]byte(testPrivateKeyData)), privateKey)
+}
+
+func Test_CheckIfCertificateUpdateAndRetrieveOldCertificateAndPk_revisionTwoSecretNotPresent(t *testing.T) {
+       request := new(cmapi.CertificateRequest)
+       request.ObjectMeta.Annotations = map[string]string{
+               revisionAnnotation:                 "2",
+               certificateConfigurationAnnotation: oldCertificateConfig,
+       }
+       fakeClient := fake.NewFakeClientWithScheme(testdata.GetScheme())
+       isUpdate, certificate, privateKey := CheckIfCertificateUpdateAndRetrieveOldCertificateAndPk(fakeClient, request, nil)
+       assert.False(t, isUpdate)
+       assert.Equal(t, "", certificate)
+       assert.Equal(t, "", privateKey)
+}
+
+func Test_IsUpdateCertificateRevision(t *testing.T) {
+       parameters := []struct {
+               revision string
+               expected bool
+       }{
+               {"1", false},
+               {"2", true},
+               {"invalid", false},
+       }
+
+       for _, parameter := range parameters {
+               testName := fmt.Sprintf("Expected:%v for revision=%v", parameter.expected, parameter.revision)
+               t.Run(testName, func(t *testing.T) {
+                       testIsUpdateCertificateRevision(t, parameter.revision, parameter.expected)
+               })
+       }
+}
+
+func testIsUpdateCertificateRevision(t *testing.T, revision string, expected bool) {
+       request := new(cmapi.CertificateRequest)
+       request.ObjectMeta.Annotations = map[string]string{
+               revisionAnnotation: revision,
+       }
+       assert.Equal(t, expected, IsUpdateCertificateRevision(request))
+}
+
+func Test_RetrieveOldCertificateAndPk_shouldSucceedWhenSecretPresent(t *testing.T) {
+       request := new(cmapi.CertificateRequest)
+       request.ObjectMeta.Annotations = map[string]string{
+               certificateConfigurationAnnotation: oldCertificateConfig,
+       }
+       fakeClient := fake.NewFakeClientWithScheme(testdata.GetScheme(), getValidCertificateSecret())
+       certificate, privateKey := RetrieveOldCertificateAndPk(fakeClient, request, nil)
+       assert.Equal(t, base64.StdEncoding.EncodeToString([]byte(testCertificateData)), certificate)
+       assert.Equal(t, base64.StdEncoding.EncodeToString([]byte(testPrivateKeyData)), privateKey)
+}
+
+func Test_RetrieveOldCertificateAndPk_shouldReturnEmptyStringsWhenSecretNotPresent(t *testing.T) {
+       request := new(cmapi.CertificateRequest)
+       request.ObjectMeta.Annotations = map[string]string{
+               certificateConfigurationAnnotation: oldCertificateConfig,
+       }
+       fakeClient := fake.NewFakeClientWithScheme(testdata.GetScheme())
+       certificate, privateKey := RetrieveOldCertificateAndPk(fakeClient, request, nil)
+       assert.Equal(t, "", certificate)
+       assert.Equal(t, "", privateKey)
+}
+
+func Test_RetrieveOldCertificateAndPk_shouldReturnEmptyStringsWhenOldCertificateCannotBeUnmarshalled(t *testing.T) {
+       request := new(cmapi.CertificateRequest)
+       fakeClient := fake.NewFakeClientWithScheme(testdata.GetScheme())
+       certificate, privateKey := RetrieveOldCertificateAndPk(fakeClient, request, nil)
+       assert.Equal(t, "", certificate)
+       assert.Equal(t, "", privateKey)
+}
+
+func getValidCertificateSecret() *v1.Secret {
+       const privateKeySecretKey = "tls.key"
+       const certificateSecretKey = "tls.crt"
+
+       return &v1.Secret{
+               Data: map[string][]byte{
+                       privateKeySecretKey:  []byte("test-private-key"),
+                       certificateSecretKey: []byte("test-certificate"),
+               },
+               ObjectMeta: metav1.ObjectMeta{
+                       Name:      "cert-test-secret-name",
+                       Namespace: "onap",
+               },
+       }
+}