From 311cb14d51f5f9b81c5761d815d5d7a5f9b63817 Mon Sep 17 00:00:00 2001 From: Jan Malkiewicz Date: Fri, 16 Oct 2020 10:42:57 +0200 Subject: [PATCH] [OOM-K8S-CERT-EXTERNAL-PROVIDER] Mock implementaion enhanced (part III) Code refactoring Added unit tests Issue-ID: OOM-2559 Signed-off-by: Jan Malkiewicz Change-Id: I3f3b7b39d739818fa82842993f621c6134816079 --- certServiceK8sExternalProvider/go.mod | 1 + certServiceK8sExternalProvider/go.sum | 4 + certServiceK8sExternalProvider/main.go | 62 +++++++--- certServiceK8sExternalProvider/main_test.go | 53 +++++++++ .../src/cmpv2api/cmpv2_groupversion_info_test.go | 36 ++++++ .../certificate_request_controller.go | 132 +++++++++++++-------- .../certificate_request_controller_test.go | 55 ++++++--- .../src/cmpv2controller/cmpv2_issuer_controller.go | 115 ++++++++++++------ .../cmpv2_issuer_controller_test.go | 66 +++++++++++ .../cmpv2controller/cmpv2_issuer_status_updater.go | 2 - .../src/cmpv2controller/status_reason.go | 28 +++++ certServiceK8sExternalProvider/src/exit_code.go | 15 +-- .../src/exit_code_test.go | 33 ++++++ 13 files changed, 473 insertions(+), 129 deletions(-) create mode 100644 certServiceK8sExternalProvider/main_test.go create mode 100644 certServiceK8sExternalProvider/src/cmpv2api/cmpv2_groupversion_info_test.go create mode 100644 certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller_test.go create mode 100644 certServiceK8sExternalProvider/src/cmpv2controller/status_reason.go create mode 100644 certServiceK8sExternalProvider/src/exit_code_test.go diff --git a/certServiceK8sExternalProvider/go.mod b/certServiceK8sExternalProvider/go.mod index 44a8b316..9bc7d725 100644 --- a/certServiceK8sExternalProvider/go.mod +++ b/certServiceK8sExternalProvider/go.mod @@ -30,6 +30,7 @@ require ( github.com/go-logr/logr v0.2.1 github.com/go-logr/zapr v0.2.0 // indirect github.com/jetstack/cert-manager v1.0.3 + github.com/stretchr/testify v1.6.1 gonum.org/v1/netlib v0.0.0-20190331212654-76723241ea4e // indirect k8s.io/api v0.19.0 k8s.io/apimachinery v0.19.0 diff --git a/certServiceK8sExternalProvider/go.sum b/certServiceK8sExternalProvider/go.sum index 830dbb30..9c356288 100644 --- a/certServiceK8sExternalProvider/go.sum +++ b/certServiceK8sExternalProvider/go.sum @@ -394,6 +394,7 @@ github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI= github.com/pquerna/cachecontrol v0.0.0-20171018203845-0dec1b30a021/go.mod h1:prYjPmNq4d1NPVmpShWobRqXY3q7Vp+80DqgxxUrUIA= @@ -456,11 +457,13 @@ github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DM github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= @@ -743,6 +746,7 @@ gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU= gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20190905181640-827449938966/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c h1:grhR+C34yXImVGp7EzNk+DTIk+323eIUWOmEevy6bDo= gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk= diff --git a/certServiceK8sExternalProvider/main.go b/certServiceK8sExternalProvider/main.go index 2fcbfaaf..8e5d36cb 100644 --- a/certServiceK8sExternalProvider/main.go +++ b/certServiceK8sExternalProvider/main.go @@ -39,6 +39,7 @@ import ( "os" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/manager" ) var ( @@ -50,24 +51,51 @@ func init() { _ = clientgoscheme.AddToScheme(scheme) _ = certmanager.AddToScheme(scheme) _ = certserviceapi.AddToScheme(scheme) + + ctrl.SetLogger(zap.New(zap.UseDevMode(true))) } func main() { + printVersionInfo() + + metricsAddr, enableLeaderElection := parseInputArguments() + + manager := createControllerManager(metricsAddr, enableLeaderElection) + + registerCMPv2IssuerController(manager) + registerCertificateRequestController(manager) + + startControllerManager(manager) + + setupLog.Info("Application is up and running.") +} + +func printVersionInfo() { fmt.Println() - fmt.Println(" *** Cert Service Provider v1.0.2 ***") + fmt.Println(" *** CMPv2 Provider v1.0.0 ***") fmt.Println() +} - setupLog.Info("Parsing arguments...") +func parseInputArguments() (string, bool) { + setupLog.Info("Parsing input arguments...") var metricsAddr string var enableLeaderElection bool flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.") flag.BoolVar(&enableLeaderElection, "enable-leader-election", false, "Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.") flag.Parse() + return metricsAddr, enableLeaderElection +} - ctrl.SetLogger(zap.New(zap.UseDevMode(true))) +func startControllerManager(manager manager.Manager) { + setupLog.Info("Starting CMPv2 controller manager...") + if err := manager.Start(ctrl.SetupSignalHandler()); err != nil { + exit(app.EXCEPTION_WHILE_RUNNING_CONTROLLER_MANAGER, err) + } +} - setupLog.Info("Creating k8s Manager...") +func createControllerManager(metricsAddr string, enableLeaderElection bool) manager.Manager { + setupLog.Info("Creating CMPv2 controller manager...") manager, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsAddr, @@ -76,32 +104,36 @@ func main() { if err != nil { exit(app.FAILED_TO_CREATE_CONTROLLER_MANAGER, err) } + return manager +} +func registerCMPv2IssuerController(manager manager.Manager) { setupLog.Info("Registering CMPv2IssuerController...") - if err = (&controllers.CMPv2IssuerController{ + + err := (&controllers.CMPv2IssuerController{ Client: manager.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("CMPv2Issuer"), Clock: clock.RealClock{}, Recorder: manager.GetEventRecorderFor("cmpv2-issuer-controller"), - }).SetupWithManager(manager); err != nil { + }).SetupWithManager(manager) + + if err != nil { exit(app.FAILED_TO_REGISTER_CMPv2_ISSUER_CONTROLLER, err) } +} +func registerCertificateRequestController(manager manager.Manager) { setupLog.Info("Registering CertificateRequestController...") - if err = (&controllers.CertificateRequestController{ + + err := (&controllers.CertificateRequestController{ Client: manager.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("CertificateRequest"), Recorder: manager.GetEventRecorderFor("certificate-requests-controller"), - }).SetupWithManager(manager); err != nil { - exit(app.FAILED_TO_REGISTER_CERT_REQUEST_CONTROLLER, err) - } + }).SetupWithManager(manager) - setupLog.Info("Starting k8s manager...") - if err := manager.Start(ctrl.SetupSignalHandler()); err != nil { - exit(app.EXCEPTION_WHILE_RUNNING_CONTROLLER_MANAGER, err) + if err != nil { + exit(app.FAILED_TO_REGISTER_CERT_REQUEST_CONTROLLER, err) } - setupLog.Info("Application is up and running.") - } func exit(exitCode app.ExitCode, err error) { diff --git a/certServiceK8sExternalProvider/main_test.go b/certServiceK8sExternalProvider/main_test.go new file mode 100644 index 00000000..d74fe0d3 --- /dev/null +++ b/certServiceK8sExternalProvider/main_test.go @@ -0,0 +1,53 @@ +/* + * ============LICENSE_START======================================================= + * oom-certservice-k8s-external-provider + * ================================================================================ + * Copyright (C) 2020 Nokia. All rights reserved. + * ================================================================================ + * 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 main + +import ( + "os" + "testing" + "github.com/stretchr/testify/assert" + "flag" +) + +func Test_shouldParseArguments_defaultValues(t *testing.T) { + os.Args = []string { + "first-arg-is-omitted-by-method-parse-arguments-so-this-only-a-placeholder"} + flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) + + metricsAddr, enableLeaderElection := parseInputArguments() + + assert.Equal(t, ":8080", metricsAddr) + assert.False(t, enableLeaderElection) +} + +func Test_shouldParseArguments_valuesFromCLI(t *testing.T) { + os.Args = []string { + "first-arg-is-omitted-by-method-parse-arguments-so-this-only-a-placeholder", + "--metrics-addr=127.0.0.1:555", + "--enable-leader-election=true" } + flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) + + metricsAddr, enableLeaderElection := parseInputArguments() + + assert.Equal(t, "127.0.0.1:555", metricsAddr) + assert.True(t, enableLeaderElection) + +} diff --git a/certServiceK8sExternalProvider/src/cmpv2api/cmpv2_groupversion_info_test.go b/certServiceK8sExternalProvider/src/cmpv2api/cmpv2_groupversion_info_test.go new file mode 100644 index 00000000..b95bded5 --- /dev/null +++ b/certServiceK8sExternalProvider/src/cmpv2api/cmpv2_groupversion_info_test.go @@ -0,0 +1,36 @@ +/* + * ============LICENSE_START======================================================= + * oom-certservice-k8s-external-provider + * ================================================================================ + * Copyright (C) 2020 Nokia. All rights reserved. + * ================================================================================ + * 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 cmpv2api + +import ( + "testing" + "github.com/stretchr/testify/assert" +) + +func Test_shouldHaveRightGroupVersion(t *testing.T) { + assert.Equal(t, "certmanager.onap.org", GroupVersion.Group) + assert.Equal(t, "v1", GroupVersion.Version) +} + +func Test_shouldRightIssuerKind(t *testing.T) { + assert.Equal(t, "CMPv2Issuer", CMPv2IssuerKind) +} + diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go index 1669c97f..38b5cdf3 100644 --- a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go +++ b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go @@ -53,91 +53,104 @@ type CertificateRequestController struct { // Reconcile will read and validate a CMPv2Issuer resource associated to the // CertificateRequest resource, and it will sign the CertificateRequest with the // provisioner in the CMPv2Issuer. -func (controller *CertificateRequestController) Reconcile(req ctrl.Request) (ctrl.Result, error) { +func (controller *CertificateRequestController) Reconcile(k8sRequest ctrl.Request) (ctrl.Result, error) { ctx := context.Background() - log := controller.Log.WithValues("certificate-request-controller", req.NamespacedName) + log := controller.Log.WithValues("certificate-request-controller", k8sRequest.NamespacedName) - // Fetch the CertificateRequest resource being reconciled. - // Just ignore the request if the certificate request has been deleted. + // 1. Fetch the CertificateRequest resource being reconciled. certificateRequest := new(cmapi.CertificateRequest) - if err := controller.Client.Get(ctx, req.NamespacedName, certificateRequest); err != nil { - if apierrors.IsNotFound(err) { - return ctrl.Result{}, nil - } - - log.Error(err, "failed to retrieve CertificateRequest resource") + if err := controller.Client.Get(ctx, k8sRequest.NamespacedName, certificateRequest); err != nil { + err = handleErrorResourceNotFound(log, err) return ctrl.Result{}, err } + // 2. Check if CertificateRequest is meant for CMPv2Issuer (if not ignore) if !isCMPv2CertificateRequest(certificateRequest) { - log.V(4).Info("certificate request is not CMPv2", + log.V(4).Info("Certificate request is not meant for CMPv2Issuer (ignoring)", "group", certificateRequest.Spec.IssuerRef.Group, "kind", certificateRequest.Spec.IssuerRef.Kind) return ctrl.Result{}, nil } - // If the certificate data is already set then we skip this request as it + // 3. If the certificate data is already set then we skip this request as it // has already been completed in the past. if len(certificateRequest.Status.Certificate) > 0 { - log.V(4).Info("existing certificate data found in status, skipping already completed CertificateRequest") + log.V(4).Info("Existing certificate data found in status, skipping already completed CertificateRequest") return ctrl.Result{}, nil } - // Fetch the CMPv2Issuer resource + // 4. Fetch the CMPv2Issuer resource issuer := cmpv2api.CMPv2Issuer{} issuerNamespaceName := types.NamespacedName{ - Namespace: req.Namespace, + Namespace: k8sRequest.Namespace, Name: certificateRequest.Spec.IssuerRef.Name, } if err := controller.Client.Get(ctx, issuerNamespaceName, &issuer); err != nil { - log.Error(err, "failed to retrieve CMPv2Issuer resource", "namespace", req.Namespace, "name", certificateRequest.Spec.IssuerRef.Name) - _ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Failed to retrieve CMPv2Issuer resource %s: %v", issuerNamespaceName, err) + controller.handleErrorGettingCMPv2Issuer(ctx, log, err, certificateRequest, issuerNamespaceName, k8sRequest) return ctrl.Result{}, err } - // Check if the CMPv2Issuer resource has been marked Ready - if !cmpv2IssuerHasCondition(issuer, cmpv2api.CMPv2IssuerCondition{Type: cmpv2api.ConditionReady, Status: cmpv2api.ConditionTrue}) { - err := fmt.Errorf("resource %s is not ready", issuerNamespaceName) - log.Error(err, "failed to retrieve CMPv2Issuer resource", "namespace", req.Namespace, "name", certificateRequest.Spec.IssuerRef.Name) - _ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "CMPv2Issuer resource %s is not Ready", issuerNamespaceName) + // 5. Check if CMPv2Issuer is ready to sing certificates + if !isCMPv2IssuerReady(issuer) { + err := controller.handleErrorCMPv2IssuerIsNotReady(ctx, log, issuerNamespaceName, certificateRequest, k8sRequest) return ctrl.Result{}, err } - // Load the provisioner that will sign the CertificateRequest + // 6. Load the provisioner that will sign the CertificateRequest provisioner, ok := provisioners.Load(issuerNamespaceName) if !ok { - err := fmt.Errorf("provisioner %s not found", issuerNamespaceName) - log.Error(err, "failed to provisioner for CMPv2Issuer resource") - _ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Failed to load provisioner for CMPv2Issuer resource %s", issuerNamespaceName) + err := controller.handleErrorCouldNotLoadCMPv2Provisioner(ctx, log, issuerNamespaceName, certificateRequest) return ctrl.Result{}, err } - // Sign CertificateRequest + // 7. Sign CertificateRequest signedPEM, trustedCAs, err := provisioner.Sign(ctx, certificateRequest) if err != nil { - log.Error(err, "failed to sign certificate request") - return ctrl.Result{}, controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "Failed to sign certificate request: %v", err) + controller.handleErrorFailedToSignCertificate(ctx, log, err, certificateRequest) + return ctrl.Result{}, err } + + // 8. Store signed certificates in CertificateRequest certificateRequest.Status.Certificate = signedPEM certificateRequest.Status.CA = trustedCAs + if err := controller.updateCertificateRequestWithSignedCerficates(ctx, certificateRequest); err != nil { + return ctrl.Result{}, err + } - return ctrl.Result{}, controller.setStatus(ctx, certificateRequest, cmmeta.ConditionTrue, cmapi.CertificateRequestReasonIssued, "Certificate issued") + return ctrl.Result{}, nil +} + +func (controller *CertificateRequestController) updateCertificateRequestWithSignedCerficates(ctx context.Context, certificateRequest *cmapi.CertificateRequest) error { + return controller.setStatus(ctx, certificateRequest, cmmeta.ConditionTrue, cmapi.CertificateRequestReasonIssued, "Certificate issued") } -// SetupWithManager initializes the CertificateRequest controller into the -// controller runtime. func (controller *CertificateRequestController) SetupWithManager(manager ctrl.Manager) error { return ctrl.NewControllerManagedBy(manager). For(&cmapi.CertificateRequest{}). Complete(controller) } -// cmpv2IssuerHasCondition will return true if the given CMPv2Issuer resource has -// a condition matching the provided CMPv2IssuerCondition. Only the Type and -// Status field will be used in the comparison, meaning that this function will -// return 'true' even if the Reason, Message and LastTransitionTime fields do -// not match. -func cmpv2IssuerHasCondition(issuer cmpv2api.CMPv2Issuer, condition cmpv2api.CMPv2IssuerCondition) bool { +func (controller *CertificateRequestController) setStatus(ctx context.Context, certificateRequest *cmapi.CertificateRequest, status cmmeta.ConditionStatus, reason, message string, args ...interface{}) error { + completeMessage := fmt.Sprintf(message, args...) + apiutil.SetCertificateRequestCondition(certificateRequest, cmapi.CertificateRequestConditionReady, status, reason, completeMessage) + + // Fire an Event to additionally inform users of the change + eventType := core.EventTypeNormal + if status == cmmeta.ConditionFalse { + eventType = core.EventTypeWarning + } + controller.Recorder.Event(certificateRequest, eventType, reason, completeMessage) + + return controller.Client.Status().Update(ctx, certificateRequest) +} + + +func isCMPv2IssuerReady(issuer cmpv2api.CMPv2Issuer) bool { + condition := cmpv2api.CMPv2IssuerCondition{Type: cmpv2api.ConditionReady, Status: cmpv2api.ConditionTrue} + return hasCondition(issuer, condition) +} + +func hasCondition(issuer cmpv2api.CMPv2Issuer, condition cmpv2api.CMPv2IssuerCondition) bool { existingConditions := issuer.Status.Conditions for _, cond := range existingConditions { if condition.Type == cond.Type && condition.Status == cond.Status { @@ -150,20 +163,41 @@ func cmpv2IssuerHasCondition(issuer cmpv2api.CMPv2Issuer, condition cmpv2api.CMP func isCMPv2CertificateRequest(certificateRequest *cmapi.CertificateRequest) bool { return certificateRequest.Spec.IssuerRef.Group != "" && certificateRequest.Spec.IssuerRef.Group == cmpv2api.GroupVersion.Group && - certificateRequest.Spec.IssuerRef.Kind == cmpv2api.CMPv2IssuerKind + certificateRequest.Spec.IssuerRef.Kind == cmpv2api.CMPv2IssuerKind } -func (controller *CertificateRequestController) setStatus(ctx context.Context, certificateRequest *cmapi.CertificateRequest, status cmmeta.ConditionStatus, reason, message string, args ...interface{}) error { - completeMessage := fmt.Sprintf(message, args...) - apiutil.SetCertificateRequestCondition(certificateRequest, cmapi.CertificateRequestConditionReady, status, reason, completeMessage) +// Error handling - // Fire an Event to additionally inform users of the change - eventType := core.EventTypeNormal - if status == cmmeta.ConditionFalse { - eventType = core.EventTypeWarning - } - controller.Recorder.Event(certificateRequest, eventType, reason, completeMessage) +func (controller *CertificateRequestController) handleErrorCouldNotLoadCMPv2Provisioner(ctx context.Context, log logr.Logger, issuerNamespaceName types.NamespacedName, certificateRequest *cmapi.CertificateRequest) error { + err := fmt.Errorf("provisioner %s not found", issuerNamespaceName) + log.Error(err, "Failed to load CMPv2 Provisioner resource") + _ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Failed to load provisioner for CMPv2Issuer resource %s", issuerNamespaceName) + return err +} - return controller.Client.Status().Update(ctx, certificateRequest) +func (controller *CertificateRequestController) handleErrorCMPv2IssuerIsNotReady(ctx context.Context, log logr.Logger, issuerNamespaceName types.NamespacedName, certificateRequest *cmapi.CertificateRequest, req ctrl.Request) error { + err := fmt.Errorf("resource %s is not ready", issuerNamespaceName) + log.Error(err, "CMPv2Issuer not ready", "namespace", req.Namespace, "name", certificateRequest.Spec.IssuerRef.Name) + _ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "CMPv2Issuer resource %s is not Ready", issuerNamespaceName) + return err +} + +func (controller *CertificateRequestController) handleErrorGettingCMPv2Issuer(ctx context.Context, log logr.Logger, err error, certificateRequest *cmapi.CertificateRequest, issuerNamespaceName types.NamespacedName, req ctrl.Request) { + log.Error(err, "Failed to retrieve CMPv2Issuer resource", "namespace", req.Namespace, "name", certificateRequest.Spec.IssuerRef.Name) + _ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Failed to retrieve CMPv2Issuer resource %s: %v", issuerNamespaceName, err) +} + +func (controller *CertificateRequestController) handleErrorFailedToSignCertificate(ctx context.Context, log logr.Logger, err error, certificateRequest *cmapi.CertificateRequest) { + log.Error(err, "Failed to sign certificate request") + _ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "Failed to sign certificate request: %v", err) +} + +func handleErrorResourceNotFound(log logr.Logger, err error) error { + if apierrors.IsNotFound(err) { + log.Error(err, "CertificateRequest resource not found") + } else { + log.Error(err, "Failed to retrieve CertificateRequest resource") + } + return err } diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller_test.go b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller_test.go index 36cfbc48..7e55f36f 100644 --- a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller_test.go +++ b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller_test.go @@ -1,35 +1,54 @@ +/* + * ============LICENSE_START======================================================= + * oom-certservice-k8s-external-provider + * ================================================================================ + * Copyright (C) 2020 Nokia. All rights reserved. + * ================================================================================ + * 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 cmpv2controller import ( cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" "testing" + "github.com/stretchr/testify/assert" + ) -func TestIsCMPv2CertificateRequest_notCMPv2Request(t *testing.T) { +const group = "certmanager.onap.org" + +func Test_shouldBeInvalidCMPv2CertificateRequest_whenEmpty(t *testing.T) { request := new(cmapi.CertificateRequest) - if isCMPv2CertificateRequest(request) { - t.Logf("CPMv2 request [NOK]") - t.FailNow() - } - request.Spec.IssuerRef.Group = "certmanager.onap.org" + assert.False(t, isCMPv2CertificateRequest(request)) +} + +func Test_shouldBeInvalidCMPv2CertificateRequest_whenKindIsCertificateRequest(t *testing.T) { + request := new(cmapi.CertificateRequest) + request.Spec.IssuerRef.Group = group request.Spec.IssuerRef.Kind = "CertificateRequest" - if isCMPv2CertificateRequest(request) { - t.Logf("CPMv2 request [NOK]") - t.FailNow() - } + + assert.False(t, isCMPv2CertificateRequest(request)) } -func TestIsCMPv2CertificateRequest_CMPvRequest(t *testing.T) { + +func Test_shouldBeValidCMPv2CertificateRequest_whenKindIsCMPvIssuer(t *testing.T) { request := new(cmapi.CertificateRequest) - request.Spec.IssuerRef.Group = "certmanager.onap.org" + request.Spec.IssuerRef.Group = group request.Spec.IssuerRef.Kind = "CMPv2Issuer" - if isCMPv2CertificateRequest(request) { - t.Logf("CPMv2 request [OK]") - } else { - t.Logf("Not a CPMv2 request [NOK]") - t.FailNow() - } + assert.True(t, isCMPv2CertificateRequest(request)) } diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller.go b/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller.go index e5b1b196..f57f5677 100644 --- a/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller.go +++ b/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller.go @@ -31,6 +31,7 @@ import ( "github.com/go-logr/logr" core "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "k8s.io/utils/clock" @@ -54,74 +55,120 @@ func (controller *CMPv2IssuerController) Reconcile(req ctrl.Request) (ctrl.Resul ctx := context.Background() log := controller.Log.WithValues("cmpv2-issuer-controller", req.NamespacedName) + // 1. Load CMPv2Issuer issuer := new(cmpv2api.CMPv2Issuer) - if err := controller.Client.Get(ctx, req.NamespacedName, issuer); err != nil { - log.Error(err, "failed to retrieve CMPv2Issuer resource") + if err := controller.loadResource(ctx, req.NamespacedName, issuer); err != nil { + handleErrorLoadingCMPv2Issuer(log, err) return ctrl.Result{}, client.IgnoreNotFound(err) } - log.Info("Issuer loaded: ", "issuer", issuer) + log.Info("CMPv2Issuer loaded: ", "issuer", issuer) + // 2. Validate CMPv2Issuer statusUpdater := newStatusUpdater(controller, issuer, log) - if err := validateCMPv2IssuerSpec(issuer.Spec); err != nil { - log.Error(err, "failed to validate CMPv2Issuer resource") - statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, "Validation", "Failed to validate resource: %v", err) + if err := validateCMPv2IssuerSpec(issuer.Spec, log); err != nil { + handleErrorCMPv2IssuerValidation(ctx, log, err, statusUpdater) return ctrl.Result{}, err } - log.Info("Issuer validated. ") - // Fetch the provisioner password + // 3. Load keystore and truststore information form k8s secret var secret core.Secret secretNamespaceName := types.NamespacedName{ Namespace: req.Namespace, Name: issuer.Spec.KeyRef.Name, } - if err := controller.Client.Get(ctx, secretNamespaceName, &secret); err != nil { - log.Error(err, "failed to retrieve CMPv2Issuer provisioner secret", "namespace", secretNamespaceName.Namespace, "name", secretNamespaceName.Name) - if apierrors.IsNotFound(err) { - statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, "NotFound", "Failed to retrieve provisioner secret: %v", err) - } else { - statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, "Error", "Failed to retrieve provisioner secret: %v", err) - } + if err := controller.loadResource(ctx, secretNamespaceName, &secret); err != nil { + handleErrorInvalidSecret(ctx, log, err, statusUpdater, secretNamespaceName) return ctrl.Result{}, err } password, ok := secret.Data[issuer.Spec.KeyRef.Key] if !ok { - err := fmt.Errorf("secret %s does not contain key %s", secret.Name, issuer.Spec.KeyRef.Key) - log.Error(err, "failed to retrieve CMPv2Issuer provisioner secret", "namespace", secretNamespaceName.Namespace, "name", secretNamespaceName.Name) - statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, "NotFound", "Failed to retrieve provisioner secret: %v", err) + err := handleErrorSecretNotFound(ctx, log, issuer, statusUpdater, secretNamespaceName, secret) return ctrl.Result{}, err } - // Initialize and store the provisioner + // 4. Create CMPv2 provisioner and store the instance for further use provisioner, err := provisioners.New(issuer, password) if err != nil { - log.Error(err, "failed to initialize provisioner") - statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, "Error", "failed initialize provisioner") + handleErrorProvisionerInitialization(ctx, log, err, statusUpdater) return ctrl.Result{}, err } provisioners.Store(req.NamespacedName, provisioner) - log.Info( "CMPv2Issuer verified. Updating status to Verified...") - return ctrl.Result{}, statusUpdater.Update(ctx, cmpv2api.ConditionTrue, "Verified", "CMPv2Issuer verified and ready to sign certificates") + // 5. Update the status of CMPv2Issuer to 'Validated' + if err := updateCMPv2IssuerStatusToVerified(statusUpdater, ctx, log); err != nil { + handleErrorUpdatingCMPv2IssuerStatus(log, err) + return ctrl.Result{}, err + } + + return ctrl.Result{}, nil } -// SetupWithManager initializes the CMPv2Issuer controller into the controller -// runtime. + func (controller *CMPv2IssuerController) SetupWithManager(manager ctrl.Manager) error { return ctrl.NewControllerManagedBy(manager). For(&cmpv2api.CMPv2Issuer{}). Complete(controller) } -func validateCMPv2IssuerSpec(issuerSpec cmpv2api.CMPv2IssuerSpec) error { +func (controller *CMPv2IssuerController) loadResource(ctx context.Context, key client.ObjectKey, obj runtime.Object) error { + return controller.Client.Get(ctx, key, obj) +} + + +func validateCMPv2IssuerSpec(issuerSpec cmpv2api.CMPv2IssuerSpec, log logr.Logger) error { switch { - case issuerSpec.URL == "": - return fmt.Errorf("spec.url cannot be empty") - case issuerSpec.KeyRef.Name == "": - return fmt.Errorf("spec.keyRef.name cannot be empty") - case issuerSpec.KeyRef.Key == "": - return fmt.Errorf("spec.keyRef.key cannot be empty") - default: - return nil + case issuerSpec.URL == "": + return fmt.Errorf("spec.url cannot be empty") + case issuerSpec.KeyRef.Name == "": + return fmt.Errorf("spec.keyRef.name cannot be empty") + case issuerSpec.KeyRef.Key == "": + return fmt.Errorf("spec.keyRef.key cannot be empty") + default: + log.Info("CMPv2Issuer validated. ") + return nil + } +} + +func updateCMPv2IssuerStatusToVerified(statusUpdater *CMPv2IssuerStatusUpdater, ctx context.Context, log logr.Logger) error { + log.Info("CMPv2 provisioner created -> updating status to of CMPv2Issuer resource to: Verified") + return statusUpdater.Update(ctx, cmpv2api.ConditionTrue, Verified, "CMPv2Issuer verified and ready to sign certificates") +} + + +// Error handling + +func handleErrorUpdatingCMPv2IssuerStatus(log logr.Logger, err error) { + log.Error(err, "Failed to update CMPv2Issuer status") +} + + +func handleErrorLoadingCMPv2Issuer(log logr.Logger, err error) { + log.Error(err, "Failed to retrieve CMPv2Issuer resource") +} + + +func handleErrorProvisionerInitialization(ctx context.Context, log logr.Logger, err error, statusUpdater *CMPv2IssuerStatusUpdater) { + log.Error(err, "Failed to initialize provisioner") + statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, Error, "Failed initialize provisioner") +} + +func handleErrorCMPv2IssuerValidation(ctx context.Context, log logr.Logger, err error, statusUpdater *CMPv2IssuerStatusUpdater) { + log.Error(err, "Failed to validate CMPv2Issuer resource") + statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, ValidationFailed, "Failed to validate resource: %v", err) +} + +func handleErrorSecretNotFound(ctx context.Context, log logr.Logger, issuer *cmpv2api.CMPv2Issuer, statusUpdater *CMPv2IssuerStatusUpdater, secretNamespaceName types.NamespacedName, secret core.Secret) error { + err := fmt.Errorf("secret %s does not contain key %s", secret.Name, issuer.Spec.KeyRef.Key) + log.Error(err, "Failed to retrieve CMPv2Issuer provisioner secret", "namespace", secretNamespaceName.Namespace, "name", secretNamespaceName.Name) + statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, NotFound, "Failed to retrieve provisioner secret: %v", err) + return err +} + +func handleErrorInvalidSecret(ctx context.Context, log logr.Logger, err error, statusUpdater *CMPv2IssuerStatusUpdater, secretNamespaceName types.NamespacedName) { + log.Error(err, "Failed to retrieve CMPv2Issuer provisioner secret", "namespace", secretNamespaceName.Namespace, "name", secretNamespaceName.Name) + if apierrors.IsNotFound(err) { + statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, NotFound, "Failed to retrieve provisioner secret: %v", err) + } else { + statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, Error, "Failed to retrieve provisioner secret: %v", err) } } diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller_test.go b/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller_test.go new file mode 100644 index 00000000..8409ea78 --- /dev/null +++ b/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller_test.go @@ -0,0 +1,66 @@ +/* + * ============LICENSE_START======================================================= + * oom-certservice-k8s-external-provider + * ================================================================================ + * Copyright (C) 2020 Nokia. All rights reserved. + * ================================================================================ + * 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 cmpv2controller + +import ( + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "onap.org/oom-certservice/k8s-external-provider/src/cmpv2api" + "testing" +) + +func Test_shouldBeInvalidCMPv2IssuerSpec_whenSpecIsEmpty(t *testing.T) { + spec := cmpv2api.CMPv2IssuerSpec{} + err := validateCMPv2IssuerSpec(spec, nil) + assert.NotNil(t, err) +} + +func Test_shouldBeInvalidCMPv2IssuerSpec_whenNotAllFieldsAreSet(t *testing.T) { + spec := cmpv2api.CMPv2IssuerSpec{} + spec.URL = "https://localhost" + spec.KeyRef = cmpv2api.SecretKeySelector{} + spec.KeyRef.Name = "secret-key" + + err := validateCMPv2IssuerSpec(spec, &MockLogger{}) + assert.NotNil(t, err) +} + +func Test_shouldBeValidCMPv2IssuerSpec_whenAllFieldsAreSet(t *testing.T) { + spec := cmpv2api.CMPv2IssuerSpec{} + spec.URL = "https://localhost" + spec.KeyRef = cmpv2api.SecretKeySelector{} + spec.KeyRef.Name = "secret-key" + spec.KeyRef.Key = "the-key" + + err := validateCMPv2IssuerSpec(spec, &MockLogger{}) + assert.Nil(t, err) +} + +type MockLogger struct { + mock.Mock +} +func (m *MockLogger) Info(msg string, keysAndValues ...interface{}) {} +func (m *MockLogger) Error(err error, msg string, keysAndValues ...interface{}) {} +func (m *MockLogger) Enabled() bool { return false } +func (m *MockLogger) V(level int) logr.Logger { return m } +func (m *MockLogger) WithValues(keysAndValues ...interface{}) logr.Logger { return m } +func (m *MockLogger) WithName(name string) logr.Logger { return m } diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_status_updater.go b/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_status_updater.go index f2ec5c5a..017e36a4 100644 --- a/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_status_updater.go +++ b/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_status_updater.go @@ -57,10 +57,8 @@ func (updater *CMPv2IssuerStatusUpdater) Update(ctx context.Context, status cmpv if status == cmpv2api.ConditionFalse { eventType = core.EventTypeWarning } - updater.logger.Info("Firing event: ", "issuer", updater.issuer, "eventtype", eventType, "reason", reason, "message", completeMessage) updater.Recorder.Event(updater.issuer, eventType, reason, completeMessage) - updater.logger.Info("Updating issuer... ") return updater.Client.Update(ctx, updater.issuer) } diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/status_reason.go b/certServiceK8sExternalProvider/src/cmpv2controller/status_reason.go new file mode 100644 index 00000000..d41712d3 --- /dev/null +++ b/certServiceK8sExternalProvider/src/cmpv2controller/status_reason.go @@ -0,0 +1,28 @@ +/* + * ============LICENSE_START======================================================= + * oom-certservice-k8s-external-provider + * ================================================================================ + * Copyright (C) 2020 Nokia. All rights reserved. + * ================================================================================ + * 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 cmpv2controller + +const ( + NotFound = "NotFound" + ValidationFailed = "ValidationFailed" + Error = "Error" + Verified = "Verified" +) diff --git a/certServiceK8sExternalProvider/src/exit_code.go b/certServiceK8sExternalProvider/src/exit_code.go index dea56c83..7435c64f 100644 --- a/certServiceK8sExternalProvider/src/exit_code.go +++ b/certServiceK8sExternalProvider/src/exit_code.go @@ -5,16 +5,9 @@ type ExitCode struct { Message string } -func newExitCode(code int, message string) *ExitCode{ - exitCode := new (ExitCode) - exitCode.Code = code - exitCode.Message = message - return exitCode -} - var ( - FAILED_TO_CREATE_CONTROLLER_MANAGER = ExitCode{1, "unable to create k8s controller manager"} - FAILED_TO_REGISTER_CMPv2_ISSUER_CONTROLLER = ExitCode{2, "unable to register CMPv2Issuer controller"} - FAILED_TO_REGISTER_CERT_REQUEST_CONTROLLER = ExitCode{3, "unable to register CertificateRequestController"} - EXCEPTION_WHILE_RUNNING_CONTROLLER_MANAGER = ExitCode{4, "an exception occurs while running k8s controller manager"} + FAILED_TO_CREATE_CONTROLLER_MANAGER = ExitCode{1, "Unable to create k8s controller manager"} + FAILED_TO_REGISTER_CMPv2_ISSUER_CONTROLLER = ExitCode{2, "Unable to register CMPv2Issuer controller"} + FAILED_TO_REGISTER_CERT_REQUEST_CONTROLLER = ExitCode{3, "Unable to register CertificateRequestController"} + EXCEPTION_WHILE_RUNNING_CONTROLLER_MANAGER = ExitCode{4, "An exception occurs while running k8s controller manager"} ) diff --git a/certServiceK8sExternalProvider/src/exit_code_test.go b/certServiceK8sExternalProvider/src/exit_code_test.go new file mode 100644 index 00000000..8a42909a --- /dev/null +++ b/certServiceK8sExternalProvider/src/exit_code_test.go @@ -0,0 +1,33 @@ +/* + * ============LICENSE_START======================================================= + * oom-certservice-k8s-external-provider + * ================================================================================ + * Copyright (C) 2020 Nokia. All rights reserved. + * ================================================================================ + * 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 app + +import ( + "testing" + "github.com/stretchr/testify/assert" +) + +func TestExitCodes(t *testing.T) { + assert.Equal(t, FAILED_TO_CREATE_CONTROLLER_MANAGER.Code, 1) + assert.Equal(t, FAILED_TO_REGISTER_CMPv2_ISSUER_CONTROLLER.Code, 2) + assert.Equal(t, FAILED_TO_REGISTER_CERT_REQUEST_CONTROLLER.Code, 3) + assert.Equal(t, EXCEPTION_WHILE_RUNNING_CONTROLLER_MANAGER.Code, 4) +} -- 2.16.6