Merge "[OOM-K8S-CERT-EXTERNAL-PROVIDER] Add check if cert should be updated"

4 files changed, 255 insertions, 3 deletions

diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go
index 51d13590..1032ee00 100644
--- a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go
+++ b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go
@@ -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 {
diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller_test.go b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller_test.go
index 24b6b89e..10f88c08 100644
--- a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller_test.go
+++ b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller_test.go
@@ -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
index 00000000..93746b82
--- /dev/null
+++ b/certServiceK8sExternalProvider/src/cmpv2controller/util/certificate_update_util.go
@@ -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
index 00000000..7dbbbe7a
--- /dev/null
+++ b/certServiceK8sExternalProvider/src/cmpv2controller/util/certificate_update_util_test.go
@@ -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",
+		},
+	}
+}