diff options
author | 2020-11-05 15:25:12 +0100 | |
---|---|---|
committer | 2020-11-10 13:08:23 +0100 | |
commit | 1815faa0ee14d5c27105a39f641a5b46939e72f8 (patch) | |
tree | a717a1b6a08a67d1a9341339bfd7c63170f5cf6c /certServiceK8sExternalProvider/src | |
parent | 8ad5e5e43ba2044e7deb1b92d131ceb086c89574 (diff) |
[OOM-K8S-CERT-EXTERNAL-PROVIDER] Refactoring & code improvements
Issue-ID: OOM-2559
Signed-off-by: Jan Malkiewicz <jan.malkiewicz@nokia.com>
Change-Id: I52c38aacf064682f0018ac37b1400893bd9e53fc
Diffstat (limited to 'certServiceK8sExternalProvider/src')
10 files changed, 307 insertions, 114 deletions
diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go index 2933b499..cb667bd6 100644 --- a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go +++ b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go @@ -29,9 +29,7 @@ import ( "context" "fmt" - apiutil "github.com/jetstack/cert-manager/pkg/api/util" cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" - cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" core "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -39,10 +37,11 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "onap.org/oom-certservice/k8s-external-provider/src/leveledlogger" "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" 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" ) @@ -53,9 +52,9 @@ const ( // CertificateRequestController reconciles a CMPv2Issuer object. type CertificateRequestController struct { - client.Client - Log leveledlogger.Logger + Client client.Client Recorder record.EventRecorder + Log leveledlogger.Logger } // Reconcile will read and validate a CMPv2Issuer resource associated to the @@ -67,6 +66,8 @@ func (controller *CertificateRequestController) Reconcile(k8sRequest ctrl.Reques // 1. Fetch the CertificateRequest resource being reconciled. certificateRequest := new(cmapi.CertificateRequest) + certUpdater := updater.NewCertificateRequestUpdater(controller.Client, controller.Recorder, certificateRequest, ctx, log) + log.Info("Registered new certificate sign request: ", "cert-name", certificateRequest.Name) if err := controller.Client.Get(ctx, k8sRequest.NamespacedName, certificateRequest); err != nil { err = handleErrorResourceNotFound(log, err) @@ -95,20 +96,20 @@ func (controller *CertificateRequestController) Reconcile(k8sRequest ctrl.Reques Name: certificateRequest.Spec.IssuerRef.Name, } if err := controller.Client.Get(ctx, issuerNamespaceName, &issuer); err != nil { - controller.handleErrorGettingCMPv2Issuer(ctx, log, err, certificateRequest, issuerNamespaceName, k8sRequest) + controller.handleErrorGettingCMPv2Issuer(certUpdater, log, err, certificateRequest, issuerNamespaceName, k8sRequest) return ctrl.Result{}, err } // 5. Check if CMPv2Issuer is ready to sing certificates if !isCMPv2IssuerReady(issuer) { - err := controller.handleErrorCMPv2IssuerIsNotReady(ctx, log, issuerNamespaceName, certificateRequest, k8sRequest) + err := controller.handleErrorCMPv2IssuerIsNotReady(certUpdater, log, issuerNamespaceName, certificateRequest, k8sRequest) return ctrl.Result{}, err } // 6. Load the provisioner that will sign the CertificateRequest provisioner, ok := provisioners.Load(issuerNamespaceName) if !ok { - err := controller.handleErrorCouldNotLoadCMPv2Provisioner(ctx, log, issuerNamespaceName, certificateRequest) + err := controller.handleErrorCouldNotLoadCMPv2Provisioner(certUpdater, log, issuerNamespaceName) return ctrl.Result{}, err } @@ -120,7 +121,7 @@ func (controller *CertificateRequestController) Reconcile(k8sRequest ctrl.Reques } var privateKeySecret core.Secret if err := controller.Client.Get(ctx, privateKeySecretNamespaceName, &privateKeySecret); err != nil { - controller.handleErrorGettingPrivateKey(ctx, log, err, certificateRequest, privateKeySecretNamespaceName) + controller.handleErrorGettingPrivateKey(certUpdater, log, err, privateKeySecretNamespaceName) return ctrl.Result{}, err } privateKeyBytes := privateKeySecret.Data[privateKeySecretKey] @@ -129,54 +130,36 @@ func (controller *CertificateRequestController) Reconcile(k8sRequest ctrl.Reques log.Info("Decoding CSR...") csr, err := x509utils.DecodeCSR(certificateRequest.Spec.Request) if err != nil { - controller.handleErrorFailedToDecodeCSR(ctx, log, err, certificateRequest) + controller.handleErrorFailedToDecodeCSR(certUpdater, log, err) return ctrl.Result{}, err } // 9. Log Certificate Request properties not supported or overridden by CertService API - logger.LogCertRequestProperties(leveledlogger.GetLoggerWithName("CSR details"), certificateRequest, csr) + logger.LogCertRequestProperties(leveledlogger.GetLoggerWithName("CSR details:"), certificateRequest, csr) // 10. Sign CertificateRequest signedPEM, trustedCAs, err := provisioner.Sign(ctx, certificateRequest, privateKeyBytes) if err != nil { - controller.handleErrorFailedToSignCertificate(ctx, log, err, certificateRequest) + controller.handleErrorFailedToSignCertificate(certUpdater, log, err) return ctrl.Result{}, nil } // 11. Store signed certificates in CertificateRequest certificateRequest.Status.Certificate = signedPEM certificateRequest.Status.CA = trustedCAs - if err := controller.updateCertificateRequestWithSignedCerficates(ctx, certificateRequest); err != nil { + if err := certUpdater.UpdateCertificateRequestWithSignedCertificates(); err != nil { return ctrl.Result{}, err } 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") -} - func (controller *CertificateRequestController) SetupWithManager(manager ctrl.Manager) error { return ctrl.NewControllerManagedBy(manager). For(&cmapi.CertificateRequest{}). Complete(controller) } -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) @@ -201,38 +184,38 @@ func isCMPv2CertificateRequest(certificateRequest *cmapi.CertificateRequest) boo // Error handling -func (controller *CertificateRequestController) handleErrorCouldNotLoadCMPv2Provisioner(ctx context.Context, log leveledlogger.Logger, issuerNamespaceName types.NamespacedName, certificateRequest *cmapi.CertificateRequest) error { +func (controller *CertificateRequestController) handleErrorCouldNotLoadCMPv2Provisioner(updater *updater.CertificateRequestStatusUpdater, log leveledlogger.Logger, issuerNamespaceName types.NamespacedName) 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) + _ = updater.UpdateStatusWithEventTypeWarning(cmapi.CertificateRequestReasonPending, "Failed to load provisioner for CMPv2Issuer resource %s", issuerNamespaceName) return err } -func (controller *CertificateRequestController) handleErrorCMPv2IssuerIsNotReady(ctx context.Context, log leveledlogger.Logger, issuerNamespaceName types.NamespacedName, certificateRequest *cmapi.CertificateRequest, req ctrl.Request) error { +func (controller *CertificateRequestController) handleErrorCMPv2IssuerIsNotReady(updater *updater.CertificateRequestStatusUpdater, log leveledlogger.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) + _ = updater.UpdateStatusWithEventTypeWarning(cmapi.CertificateRequestReasonPending, "CMPv2Issuer resource %s is not Ready", issuerNamespaceName) return err } -func (controller *CertificateRequestController) handleErrorGettingCMPv2Issuer(ctx context.Context, log leveledlogger.Logger, err error, certificateRequest *cmapi.CertificateRequest, issuerNamespaceName types.NamespacedName, req ctrl.Request) { +func (controller *CertificateRequestController) handleErrorGettingCMPv2Issuer(updater *updater.CertificateRequestStatusUpdater, log leveledlogger.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) + _ = updater.UpdateStatusWithEventTypeWarning(cmapi.CertificateRequestReasonPending, "Failed to retrieve CMPv2Issuer resource %s: %v", issuerNamespaceName, err) } -func (controller *CertificateRequestController) handleErrorGettingPrivateKey(ctx context.Context, log leveledlogger.Logger, err error, certificateRequest *cmapi.CertificateRequest, pkSecretNamespacedName types.NamespacedName) { +func (controller *CertificateRequestController) handleErrorGettingPrivateKey(updater *updater.CertificateRequestStatusUpdater, log leveledlogger.Logger, err error, pkSecretNamespacedName types.NamespacedName) { log.Error(err, "Failed to retrieve private key secret for certificate request", "namespace", pkSecretNamespacedName.Namespace, "name", pkSecretNamespacedName.Name) - _ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Failed to retrieve private key secret: %v", err) + _ = updater.UpdateStatusWithEventTypeWarning(cmapi.CertificateRequestReasonPending, "Failed to retrieve private key secret: %v", err) } -func (controller *CertificateRequestController) handleErrorFailedToSignCertificate(ctx context.Context, log leveledlogger.Logger, err error, certificateRequest *cmapi.CertificateRequest) { +func (controller *CertificateRequestController) handleErrorFailedToSignCertificate(updater *updater.CertificateRequestStatusUpdater, log leveledlogger.Logger, err error) { log.Error(err, "Failed to sign certificate request") - _ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "Failed to sign certificate request: %v", err) + _ = updater.UpdateStatusWithEventTypeWarning(cmapi.CertificateRequestReasonFailed, "Failed to sign certificate request: %v", err) } -func (controller *CertificateRequestController) handleErrorFailedToDecodeCSR(ctx context.Context, log leveledlogger.Logger, err error, certificateRequest *cmapi.CertificateRequest) { +func (controller *CertificateRequestController) handleErrorFailedToDecodeCSR(updater *updater.CertificateRequestStatusUpdater, log leveledlogger.Logger, err error) { log.Error(err, "Failed to decode certificate sign request") - _ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "Failed to decode CSR: %v", err) + _ = updater.UpdateStatusWithEventTypeWarning(cmapi.CertificateRequestReasonFailed, "Failed to decode CSR: %v", err) } func handleErrorResourceNotFound(log leveledlogger.Logger, err error) error { diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller.go b/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller.go index 32403c8c..3f3a4651 100644 --- a/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller.go +++ b/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_controller.go @@ -39,16 +39,17 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "onap.org/oom-certservice/k8s-external-provider/src/cmpv2api" + "onap.org/oom-certservice/k8s-external-provider/src/cmpv2controller/updater" provisioners "onap.org/oom-certservice/k8s-external-provider/src/cmpv2provisioner" "onap.org/oom-certservice/k8s-external-provider/src/leveledlogger" ) // CMPv2IssuerController reconciles a CMPv2Issuer object type CMPv2IssuerController struct { - client.Client - Log leveledlogger.Logger - Clock clock.Clock - Recorder record.EventRecorder + Client client.Client + Log leveledlogger.Logger + Clock clock.Clock + Recorder record.EventRecorder ProvisionerFactory provisioners.ProvisionerFactory } @@ -67,7 +68,7 @@ func (controller *CMPv2IssuerController) Reconcile(req ctrl.Request) (ctrl.Resul log.Info("CMPv2Issuer loaded: ", "issuer", issuer) // 2. Validate CMPv2Issuer - statusUpdater := newStatusUpdater(controller, issuer, log) + statusUpdater := updater.NewCMPv2IssuerStatusUpdater(controller.Client, controller.Recorder, issuer, controller.Clock, log) if err := validateCMPv2IssuerSpec(issuer.Spec); err != nil { handleErrorCMPv2IssuerValidation(ctx, log, err, statusUpdater) return ctrl.Result{}, err @@ -137,9 +138,9 @@ func validateCMPv2IssuerSpec(issuerSpec cmpv2api.CMPv2IssuerSpec) error { } } -func updateCMPv2IssuerStatusToVerified(statusUpdater *CMPv2IssuerStatusUpdater, ctx context.Context, log leveledlogger.Logger) error { +func updateCMPv2IssuerStatusToVerified(statusUpdater *updater.CMPv2IssuerStatusUpdater, ctx context.Context, log leveledlogger.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") + return statusUpdater.Update(ctx, cmpv2api.ConditionTrue, updater.Verified, "CMPv2Issuer verified and ready to sign certificates") } // Error handling @@ -152,21 +153,21 @@ func handleErrorLoadingCMPv2Issuer(log leveledlogger.Logger, err error) { log.Error(err, "Failed to retrieve CMPv2Issuer resource") } -func handleErrorProvisionerInitialization(ctx context.Context, log leveledlogger.Logger, err error, statusUpdater *CMPv2IssuerStatusUpdater) { +func handleErrorProvisionerInitialization(ctx context.Context, log leveledlogger.Logger, err error, statusUpdater *updater.CMPv2IssuerStatusUpdater) { log.Error(err, "Failed to initialize provisioner") - statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, Error, "Failed to initialize provisioner: %v", err) + statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, updater.Error, "Failed to initialize provisioner: %v", err) } -func handleErrorCMPv2IssuerValidation(ctx context.Context, log leveledlogger.Logger, err error, statusUpdater *CMPv2IssuerStatusUpdater) { +func handleErrorCMPv2IssuerValidation(ctx context.Context, log leveledlogger.Logger, err error, statusUpdater *updater.CMPv2IssuerStatusUpdater) { log.Error(err, "Failed to validate CMPv2Issuer resource") - statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, ValidationFailed, "Failed to validate resource: %v", err) + statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, updater.ValidationFailed, "Failed to validate resource: %v", err) } -func handleErrorInvalidSecret(ctx context.Context, log leveledlogger.Logger, err error, statusUpdater *CMPv2IssuerStatusUpdater, secretNamespaceName types.NamespacedName) { +func handleErrorInvalidSecret(ctx context.Context, log leveledlogger.Logger, err error, statusUpdater *updater.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) + statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, updater.NotFound, "Failed to retrieve provisioner secret: %v", err) } else { - statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, Error, "Failed to retrieve provisioner secret: %v", err) + statusUpdater.UpdateNoError(ctx, cmpv2api.ConditionFalse, updater.Error, "Failed to retrieve provisioner secret: %v", err) } } diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger.go b/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger.go index 786557c1..649ce47f 100644 --- a/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger.go +++ b/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger.go @@ -27,6 +27,7 @@ import ( "strconv" cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" + "onap.org/oom-certservice/k8s-external-provider/src/leveledlogger" ) @@ -36,26 +37,31 @@ const ( ) func LogCertRequestProperties(log leveledlogger.Logger, request *cmapi.CertificateRequest, csr *x509.CertificateRequest) { - logSupportedProperties(log, request, csr) + logSupportedProperties(log, csr) logPropertiesNotSupportedByCertService(log, request, csr) logPropertiesOverriddenByCMPv2Server(log, request) } -func logSupportedProperties(log leveledlogger.Logger, request *cmapi.CertificateRequest, csr *x509.CertificateRequest) { - logSupportedProperty(log, csr.Subject.Organization, "organization") - logSupportedProperty(log, csr.Subject.OrganizationalUnit, "organization unit") - logSupportedProperty(log, csr.Subject.Country, "country") - logSupportedProperty(log, csr.Subject.Province, "state") - logSupportedProperty(log, csr.Subject.Locality, "location") - logSupportedProperty(log, csr.DNSNames, "dns names") +func logSupportedProperties(log leveledlogger.Logger, csr *x509.CertificateRequest) { + logSupportedSingleValueProperty(log, csr.Subject.CommonName, "common name") + logSupportedMultiValueProperty(log, csr.Subject.Organization, "organization") + logSupportedMultiValueProperty(log, csr.Subject.OrganizationalUnit, "organization unit") + logSupportedMultiValueProperty(log, csr.Subject.Country, "country") + logSupportedMultiValueProperty(log, csr.Subject.Province, "state") + logSupportedMultiValueProperty(log, csr.Subject.Locality, "location") + logSupportedMultiValueProperty(log, csr.DNSNames, "dns names") } -func logSupportedProperty(log leveledlogger.Logger, values []string, propertyName string) { +func logSupportedMultiValueProperty(log leveledlogger.Logger, values []string, propertyName string) { if len(values) > 0 { log.Info(getSupportedMessage(propertyName, extractStringArray(values))) } } +func logSupportedSingleValueProperty(log leveledlogger.Logger, value string, propertyName string) { + log.Info(getSupportedMessage(propertyName, value)) +} + func logPropertiesOverriddenByCMPv2Server(log leveledlogger.Logger, request *cmapi.CertificateRequest) { if request.Spec.Duration != nil && len(request.Spec.Duration.String()) > 0 { log.Info(getOverriddenMessage("duration", request.Spec.Duration.Duration.String())) @@ -131,14 +137,14 @@ func extractIPAddresses(addresses []net.IP) string { return values } -func getNotSupportedMessage(property string, value string) string { - return "WARNING: Property '" + property + "' with value: " + value + " is not supported by " + CertServiceName +func getSupportedMessage(property string, value string) string { + return "+ property '" + property + "' with value '" + value + "' will be sent in certificate signing request to " + CMPv2ServerName } -func getSupportedMessage(property string, value string) string { - return "Property '" + property + "' with value: " + value + " will be sent in certificate signing request to " + CMPv2ServerName +func getNotSupportedMessage(property string, value string) string { + return "- property '" + property + "' with value '" + value + "' is not supported by " + CertServiceName } func getOverriddenMessage(property string, values string) string { - return "Property '" + property + "' with value: " + values + " will be overridden by " + CMPv2ServerName + return "* property '" + property + "' with value '" + values + "' will be overridden by " + CMPv2ServerName } diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger_test.go b/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger_test.go index e6a4d0e6..250fab8b 100644 --- a/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger_test.go +++ b/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger_test.go @@ -22,7 +22,6 @@ package logger import ( "bytes" - "io/ioutil" "log" "os" @@ -34,16 +33,27 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - x509utils "onap.org/oom-certservice/k8s-external-provider/src/x509" "onap.org/oom-certservice/k8s-external-provider/src/leveledlogger" + x509utils "onap.org/oom-certservice/k8s-external-provider/src/x509" ) -var checkedLogMessages = [7]string{"Property 'duration'", "Property 'usages'", "Property 'ipAddresses'", - "Property 'isCA'", "Property 'subject.streetAddress'", "Property 'subject.postalCodes'", - "Property 'subject.serialNumber'"} - -var supportedProperties = [7]string{"Property 'organization'", "Property 'organization unit'", "Property 'country'", - "Property 'state'", "Property 'location'", "Property 'dns names'"} +var unsupportedProperties = []string{ + "* property 'duration'", + "* property 'usages'", + "- property 'ipAddresses'", + "- property 'isCA'", + "- property 'subject.streetAddress'", + "- property 'subject.postalCodes'", + "- property 'subject.serialNumber'"} + +var supportedProperties = []string{ + "+ property 'common name'", + "+ property 'organization'", + "+ property 'organization unit'", + "+ property 'country'", + "+ property 'state'", + "+ property 'location'", + "+ property 'dns names'"} const RESULT_LOG = "testdata/test_result.log" @@ -67,8 +77,8 @@ func TestLogShouldNotProvideInformationAboutSkippedPropertiesIfNotExistInCSR(t * logsArray := convertLogFileToStringArray(RESULT_LOG) //then - for _, logMsg := range checkedLogMessages { - assert.False(t, logsContainExpectedMessage(logsArray, logMsg), "Logs contain: "+logMsg+", but should not") + for _, logMsg := range unsupportedProperties { + assert.False(t, logsContainExpectedMessage(logsArray, logMsg), "Logs should not contain: ["+logMsg+"]") } removeTemporaryFile(RESULT_LOG) } @@ -88,8 +98,8 @@ func TestLogShouldProvideInformationAboutSkippedPropertiesIfExistInCSR(t *testin logsArray := convertLogFileToStringArray(RESULT_LOG) //then - for _, logMsg := range checkedLogMessages { - assert.True(t, logsContainExpectedMessage(logsArray, logMsg), "Logs not contain: "+logMsg) + for _, logMsg := range unsupportedProperties { + assert.True(t, logsContainExpectedMessage(logsArray, logMsg), "Logs should contain: ["+logMsg+"]") } removeTemporaryFile(RESULT_LOG) } @@ -110,7 +120,7 @@ func TestLogShouldListSupportedProperties(t *testing.T) { //then for _, logMsg := range supportedProperties { - assert.True(t, logsContainExpectedMessage(logsArray, logMsg), "Logs not contain: "+logMsg) + assert.True(t, logsContainExpectedMessage(logsArray, logMsg), "Logs should contain: ["+logMsg+"]") } removeTemporaryFile(RESULT_LOG) } diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/updater/certificate_request_status_updater.go b/certServiceK8sExternalProvider/src/cmpv2controller/updater/certificate_request_status_updater.go new file mode 100644 index 00000000..d4ed0237 --- /dev/null +++ b/certServiceK8sExternalProvider/src/cmpv2controller/updater/certificate_request_status_updater.go @@ -0,0 +1,74 @@ +/* + * ============LICENSE_START======================================================= + * oom-certservice-k8s-external-provider + * ================================================================================ + * Copyright (C) 2020 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 updater + +import ( + "context" + "fmt" + + apiutil "github.com/jetstack/cert-manager/pkg/api/util" + cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" + cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + + "onap.org/oom-certservice/k8s-external-provider/src/leveledlogger" +) + +type CertificateRequestStatusUpdater struct { + client client.Client + recorder record.EventRecorder + logger leveledlogger.Logger + ctx context.Context + certificateRequest *cmapi.CertificateRequest +} + +func NewCertificateRequestUpdater(client client.Client, + recorder record.EventRecorder, certificateRequest *cmapi.CertificateRequest, ctx context.Context, log leveledlogger.Logger) *CertificateRequestStatusUpdater { + return &CertificateRequestStatusUpdater{ + client: client, + recorder: recorder, + logger: log, + ctx: ctx, + certificateRequest: certificateRequest, + } +} + +func (instance *CertificateRequestStatusUpdater) UpdateStatusWithEventTypeWarning(reason string, message string, args ...interface{}) error { + return instance.updateStatus(cmmeta.ConditionFalse, reason, message, args...) +} + +func (instance *CertificateRequestStatusUpdater) UpdateCertificateRequestWithSignedCertificates() error { + return instance.updateStatus(cmmeta.ConditionTrue, cmapi.CertificateRequestReasonIssued, "Certificate issued") +} + +func (instance *CertificateRequestStatusUpdater) updateStatus(status cmmeta.ConditionStatus, reason string, message string, args ...interface{}) error { + completeMessage := fmt.Sprintf(message, args...) + apiutil.SetCertificateRequestCondition(instance.certificateRequest, cmapi.CertificateRequestConditionReady, status, reason, completeMessage) + + FireEventCert(instance.recorder, instance.certificateRequest, status, reason, completeMessage) + + return instance.client.Status().Update(instance.ctx, instance.certificateRequest) +} diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_status_updater.go b/certServiceK8sExternalProvider/src/cmpv2controller/updater/cmpv2_issuer_status_updater.go index 400d1f00..e11c2b02 100644 --- a/certServiceK8sExternalProvider/src/cmpv2controller/cmpv2_issuer_status_updater.go +++ b/certServiceK8sExternalProvider/src/cmpv2controller/updater/cmpv2_issuer_status_updater.go @@ -23,50 +23,51 @@ * ============LICENSE_END========================================================= */ -package cmpv2controller +package updater import ( "context" "fmt" - core "k8s.io/api/core/v1" meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + "k8s.io/utils/clock" + "sigs.k8s.io/controller-runtime/pkg/client" "onap.org/oom-certservice/k8s-external-provider/src/cmpv2api" "onap.org/oom-certservice/k8s-external-provider/src/leveledlogger" ) type CMPv2IssuerStatusUpdater struct { - *CMPv2IssuerController - issuer *cmpv2api.CMPv2Issuer - logger leveledlogger.Logger + client client.Client + recorder record.EventRecorder + issuer *cmpv2api.CMPv2Issuer + clock clock.Clock + logger leveledlogger.Logger } -func newStatusUpdater(controller *CMPv2IssuerController, issuer *cmpv2api.CMPv2Issuer, log leveledlogger.Logger) *CMPv2IssuerStatusUpdater { +func NewCMPv2IssuerStatusUpdater(client client.Client, recorder record.EventRecorder, issuer *cmpv2api.CMPv2Issuer, clock clock.Clock, log leveledlogger.Logger) *CMPv2IssuerStatusUpdater { return &CMPv2IssuerStatusUpdater{ - CMPv2IssuerController: controller, - issuer: issuer, - logger: log, + client: client, + recorder: recorder, + issuer: issuer, + clock: clock, + logger: log, } } -func (updater *CMPv2IssuerStatusUpdater) Update(ctx context.Context, status cmpv2api.ConditionStatus, reason, message string, args ...interface{}) error { +func (instance *CMPv2IssuerStatusUpdater) Update(ctx context.Context, status cmpv2api.ConditionStatus, reason, message string, args ...interface{}) error { completeMessage := fmt.Sprintf(message, args...) - updater.setCondition(status, reason, completeMessage) + instance.setCondition(status, reason, completeMessage) - // Fire an Event to additionally inform users of the change - eventType := core.EventTypeNormal - if status == cmpv2api.ConditionFalse { - eventType = core.EventTypeWarning - } - updater.Recorder.Event(updater.issuer, eventType, reason, completeMessage) + FireEventIssuer(instance.recorder, instance.issuer, status, reason, completeMessage) - return updater.Client.Update(ctx, updater.issuer) + return instance.client.Update(ctx, instance.issuer) } -func (updater *CMPv2IssuerStatusUpdater) UpdateNoError(ctx context.Context, status cmpv2api.ConditionStatus, reason, message string, args ...interface{}) { - if err := updater.Update(ctx, status, reason, message, args...); err != nil { - updater.logger.Error(err, "failed to update", "status", status, "reason", reason) +func (instance *CMPv2IssuerStatusUpdater) UpdateNoError(ctx context.Context, status cmpv2api.ConditionStatus, reason, message string, args ...interface{}) { + if err := instance.Update(ctx, status, reason, message, args...); err != nil { + instance.logger.Error(err, "failed to update", "status", status, "reason", reason) } } @@ -79,8 +80,8 @@ func (updater *CMPv2IssuerStatusUpdater) UpdateNoError(ctx context.Context, stat // - If a condition of the same type and different state already exists, the // condition will be updated and the LastTransitionTime set to the current // time. -func (updater *CMPv2IssuerStatusUpdater) setCondition(status cmpv2api.ConditionStatus, reason, message string) { - now := meta.NewTime(updater.Clock.Now()) +func (instance *CMPv2IssuerStatusUpdater) setCondition(status cmpv2api.ConditionStatus, reason, message string) { + now := meta.NewTime(instance.clock.Now()) issuerCondition := cmpv2api.CMPv2IssuerCondition{ Type: cmpv2api.ConditionReady, Status: status, @@ -90,7 +91,7 @@ func (updater *CMPv2IssuerStatusUpdater) setCondition(status cmpv2api.ConditionS } // Search through existing conditions - for i, condition := range updater.issuer.Status.Conditions { + for i, condition := range instance.issuer.Status.Conditions { // Skip unrelated conditions if condition.Type != cmpv2api.ConditionReady { continue @@ -101,16 +102,16 @@ func (updater *CMPv2IssuerStatusUpdater) setCondition(status cmpv2api.ConditionS if condition.Status == status { issuerCondition.LastTransitionTime = condition.LastTransitionTime } else { - updater.logger.Info("found status change for CMPv2Issuer condition; setting lastTransitionTime", "condition", condition.Type, "old_status", condition.Status, "new_status", status, "time", now.Time) + instance.logger.Info("found status change for CMPv2Issuer condition; setting lastTransitionTime", "condition", condition.Type, "old_status", condition.Status, "new_status", status, "time", now.Time) } // Overwrite the existing condition - updater.issuer.Status.Conditions[i] = issuerCondition + instance.issuer.Status.Conditions[i] = issuerCondition return } // If we've not found an existing condition of this type, we simply insert // the new condition into the slice. - updater.issuer.Status.Conditions = append(updater.issuer.Status.Conditions, issuerCondition) - updater.logger.Info("setting lastTransitionTime for CMPv2Issuer condition", "condition", cmpv2api.ConditionReady, "time", now.Time) + instance.issuer.Status.Conditions = append(instance.issuer.Status.Conditions, issuerCondition) + instance.logger.Info("setting lastTransitionTime for CMPv2Issuer condition", "condition", cmpv2api.ConditionReady, "time", now.Time) } diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/updater/k8s_resource_updater.go b/certServiceK8sExternalProvider/src/cmpv2controller/updater/k8s_resource_updater.go new file mode 100644 index 00000000..e638d84a --- /dev/null +++ b/certServiceK8sExternalProvider/src/cmpv2controller/updater/k8s_resource_updater.go @@ -0,0 +1,49 @@ +/* + * ============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 updater + +import ( + core "k8s.io/api/core/v1" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + + cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" + + "onap.org/oom-certservice/k8s-external-provider/src/cmpv2api" +) + +// Fire an Event to additionally inform users of the change +func FireEventCert(recorder record.EventRecorder, resource runtime.Object, status cmmeta.ConditionStatus, reason string, message string) { + eventType := core.EventTypeNormal + if status == cmmeta.ConditionFalse { + eventType = core.EventTypeWarning + } + recorder.Event(resource, eventType, reason, message) +} + +func FireEventIssuer(recorder record.EventRecorder, resource runtime.Object, status cmpv2api.ConditionStatus, reason string, message string) { + eventType := core.EventTypeNormal + if status == cmpv2api.ConditionFalse { + eventType = core.EventTypeWarning + } + recorder.Event(resource, eventType, reason, message) +} diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/updater/k8s_resource_updater_test.go b/certServiceK8sExternalProvider/src/cmpv2controller/updater/k8s_resource_updater_test.go new file mode 100644 index 00000000..553c419f --- /dev/null +++ b/certServiceK8sExternalProvider/src/cmpv2controller/updater/k8s_resource_updater_test.go @@ -0,0 +1,68 @@ +/* + * ============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 updater + +import ( + "testing" + + cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" + "github.com/stretchr/testify/assert" + "k8s.io/client-go/tools/record" + + "onap.org/oom-certservice/k8s-external-provider/src/cmpv2api" +) + +const ( + recorderBufferSize = 3 +) + + +func Test_shouldFireWarningEvent_forCmpv2Issuer(t *testing.T) { + fakeRecorder := record.NewFakeRecorder(recorderBufferSize) + + FireEventIssuer(fakeRecorder, nil, cmpv2api.ConditionFalse, "testReason", "testMessage") + + assert.Equal(t, <-fakeRecorder.Events, "Warning testReason testMessage") +} + +func Test_shouldFireNormalEvent_forCmpv2Issuer(t *testing.T) { + fakeRecorder := record.NewFakeRecorder(recorderBufferSize) + + FireEventIssuer(fakeRecorder, nil, cmpv2api.ConditionTrue, "testReason", "testMessage") + + assert.Equal(t, <-fakeRecorder.Events, "Normal testReason testMessage") +} + +func Test_shouldFireWarningEvent_forCertRequest(t *testing.T) { + fakeRecorder := record.NewFakeRecorder(recorderBufferSize) + + FireEventCert(fakeRecorder, nil, cmmeta.ConditionFalse, "testReason", "testMessage") + + assert.Equal(t, <-fakeRecorder.Events, "Warning testReason testMessage") +} + +func Test_shouldFireNormalEvent_forCertRequest(t *testing.T) { + fakeRecorder := record.NewFakeRecorder(recorderBufferSize) + + FireEventCert(fakeRecorder, nil, cmmeta.ConditionTrue, "testReason", "testMessage") + + assert.Equal(t, <-fakeRecorder.Events, "Normal testReason testMessage") +} diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/status_reason.go b/certServiceK8sExternalProvider/src/cmpv2controller/updater/status_reason.go index fc1772e9..8fe8664c 100644 --- a/certServiceK8sExternalProvider/src/cmpv2controller/status_reason.go +++ b/certServiceK8sExternalProvider/src/cmpv2controller/updater/status_reason.go @@ -18,7 +18,7 @@ * ============LICENSE_END========================================================= */ -package cmpv2controller +package updater const ( NotFound = "NotFound" diff --git a/certServiceK8sExternalProvider/src/cmpv2provisioner/cmpv2_provisioner.go b/certServiceK8sExternalProvider/src/cmpv2provisioner/cmpv2_provisioner.go index dc5532ab..e89eb1f4 100644 --- a/certServiceK8sExternalProvider/src/cmpv2provisioner/cmpv2_provisioner.go +++ b/certServiceK8sExternalProvider/src/cmpv2provisioner/cmpv2_provisioner.go @@ -95,12 +95,13 @@ func (ca *CertServiceCA) Sign( log.Info("CA: ", "name", ca.name, "url", ca.url) csrBytes := certificateRequest.Spec.Request - log.Debug("Csr PEM: ", "bytes", csrBytes) + log.Debug("Original CSR PEM: ", "bytes", csrBytes) filteredCsrBytes, err := csr.FilterFieldsFromCSR(csrBytes, privateKeyBytes) if err != nil { return nil, nil, err } + log.Debug("Filtered out CSR PEM: ", "bytes", csrBytes) response, err := ca.certServiceClient.GetCertificates(filteredCsrBytes, privateKeyBytes) if err != nil { |