From e5cf2682c132dec24f906369ed762af1aaceef71 Mon Sep 17 00:00:00 2001 From: Remigiusz Janeczek Date: Tue, 6 Jul 2021 13:33:51 +0200 Subject: [OOM-K8S-CERT-EXTERNAL-PROVIDER] Add check if cert should be updated Issue-ID: OOM-2753 Signed-off-by: Remigiusz Janeczek Change-Id: If0d7154b39c9ca7f9a7942f61b93725405b8f4e8 --- .../certificate_request_controller.go | 15 +- .../certificate_request_controller_test.go | 1 + .../util/certificate_update_util.go | 89 ++++++++++++ .../util/certificate_update_util_test.go | 153 +++++++++++++++++++++ 4 files changed, 255 insertions(+), 3 deletions(-) create mode 100644 certServiceK8sExternalProvider/src/cmpv2controller/util/certificate_update_util.go create mode 100644 certServiceK8sExternalProvider/src/cmpv2controller/util/certificate_update_util_test.go 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", + }, + } +} -- cgit 1.2.3-korg