[OOM-K8S-CERT-EXTERNAL-PROVIDER] Add logging of supported CSR properties

Issue-ID: OOM-2559
Signed-off-by: Jan Malkiewicz <jan.malkiewicz@nokia.com>
Change-Id: I8e6a55eea3d87b6bb5f3a26ca9a11d618bb61a77

3 files changed, 103 insertions, 36 deletions

diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go
index f77642cd..03eef35c 100644
--- a/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go
+++ b/certServiceK8sExternalProvider/src/cmpv2controller/certificate_request_controller.go
@@ -43,6 +43,7 @@ import (
 	"onap.org/oom-certservice/k8s-external-provider/src/cmpv2api"
 	"onap.org/oom-certservice/k8s-external-provider/src/cmpv2controller/logger"
 	provisioners "onap.org/oom-certservice/k8s-external-provider/src/cmpv2provisioner"
+	x509utils "onap.org/oom-certservice/k8s-external-provider/src/x509"
 )
 
 const (
@@ -124,17 +125,25 @@ func (controller *CertificateRequestController) Reconcile(k8sRequest ctrl.Reques
 	}
 	privateKeyBytes := privateKeySecret.Data[privateKeySecretKey]
 
-	// 8. Log Certificate Request properties not supported or overridden by CertService API
-	logger.LogCertRequestProperties(ctrl.Log.WithName("CSR details"), certificateRequest)
+	// 8. Decode CSR
+	log.Info("Decoding CSR...")
+	csr, err := x509utils.DecodeCSR(certificateRequest.Spec.Request)
+	if err != nil {
+		controller.handleErrorFailedToDecodeCSR(ctx, log, err, certificateRequest)
+		return ctrl.Result{}, err
+	}
+
+	// 9. Log Certificate Request properties not supported or overridden by CertService API
+	logger.LogCertRequestProperties(ctrl.Log.WithName("CSR details"), certificateRequest, csr)
 
-	// 9. Sign CertificateRequest
+	// 10. Sign CertificateRequest
 	signedPEM, trustedCAs, err := provisioner.Sign(ctx, certificateRequest, privateKeyBytes)
 	if err != nil {
 		controller.handleErrorFailedToSignCertificate(ctx, log, err, certificateRequest)
 		return ctrl.Result{}, err
 	}
 
-	// 10. Store signed certificates in CertificateRequest
+	// 11. Store signed certificates in CertificateRequest
 	certificateRequest.Status.Certificate = signedPEM
 	certificateRequest.Status.CA = trustedCAs
 	if err := controller.updateCertificateRequestWithSignedCerficates(ctx, certificateRequest); err != nil {
@@ -221,6 +230,12 @@ func (controller *CertificateRequestController) handleErrorFailedToSignCertifica
 	_ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "Failed to sign certificate request: %v", err)
 }
 
+func (controller *CertificateRequestController) handleErrorFailedToDecodeCSR(ctx context.Context, log logr.Logger, err error, certificateRequest *cmapi.CertificateRequest) {
+	log.Error(err, "Failed to decode certificate sign request")
+	_ = controller.setStatus(ctx, certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "Failed to decode CSR: %v", err)
+}
+
+
 func handleErrorResourceNotFound(log logr.Logger, err error) error {
 	if apierrors.IsNotFound(err) {
 		log.Error(err, "CertificateRequest resource not found")
diff --git a/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger.go b/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger.go
index da439fb8..0aaf48d3 100644
--- a/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger.go
+++ b/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger.go
@@ -21,8 +21,7 @@
 package logger
 
 import (
-	"crypto/x509"
-	"encoding/pem"
+	x509 "crypto/x509"
 	"net"
 	"net/url"
 	"strconv"
@@ -36,9 +35,25 @@ const (
 	CMPv2ServerName = "CMPv2 Server"
 )
 
-func LogCertRequestProperties(log logr.Logger, request *cmapi.CertificateRequest) {
+func LogCertRequestProperties(log logr.Logger, request *cmapi.CertificateRequest, csr *x509.CertificateRequest) {
+	logSupportedProperties(log, request, csr)
+	logPropertiesNotSupportedByCertService(log, request, csr)
 	logPropertiesOverriddenByCMPv2Server(log, request)
-	logPropertiesNotSupportedByCertService(log, request)
+}
+
+func logSupportedProperties(log logr.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 logSupportedProperty(log logr.Logger, values []string, propertyName string) {
+	if len(values) > 0 {
+		log.Info(getSupportedMessage(propertyName, extractStringArray(values)))
+	}
 }
 
 func logPropertiesOverriddenByCMPv2Server(log logr.Logger, request *cmapi.CertificateRequest) {
@@ -58,53 +73,44 @@ func extractUsages(usages []cmapi.KeyUsage) string {
 	return values
 }
 
-func getOverriddenMessage(property string, values string) string {
-	return "Property '" + property + "' with value: " + values + ", will be overridden by " + CMPv2ServerName
-}
-
-func logPropertiesNotSupportedByCertService(log logr.Logger, request *cmapi.CertificateRequest) {
+func logPropertiesNotSupportedByCertService(log logr.Logger, request *cmapi.CertificateRequest, csr *x509.CertificateRequest) {
 
-	block, _ := pem.Decode(request.Spec.Request)
-	cert, err := x509.ParseCertificateRequest(block.Bytes)
-	if err != nil {
-		log.Error(err, "Cannot parse Certificate Signing Request")
-	}
 	//IP addresses in SANs
-	if len(cert.IPAddresses) > 0 {
-		log.Info(getNotSupportedMessage("ipAddresses", extractIPAddresses(cert.IPAddresses)))
+	if len(csr.IPAddresses) > 0 {
+		log.Info(getNotSupportedMessage("ipAddresses", extractIPAddresses(csr.IPAddresses)))
 	}
 	//URIs in SANs
-	if len(cert.URIs) > 0 {
-		log.Info(getNotSupportedMessage("uris", extractURIs(cert.URIs)))
+	if len(csr.URIs) > 0 {
+		log.Info(getNotSupportedMessage("uris", extractURIs(csr.URIs)))
 	}
 
 	//Email addresses in SANs
-	if len(cert.EmailAddresses) > 0 {
-		log.Info(getNotSupportedMessage("emailAddresses", extractStringArray(cert.EmailAddresses)))
+	if len(csr.EmailAddresses) > 0 {
+		log.Info(getNotSupportedMessage("emailAddresses", extractStringArray(csr.EmailAddresses)))
 	}
 
 	if request.Spec.IsCA == true {
 		log.Info(getNotSupportedMessage("isCA", strconv.FormatBool(request.Spec.IsCA)))
 	}
 
-	if len(cert.Subject.StreetAddress) > 0 {
-		log.Info(getNotSupportedMessage("subject.streetAddress", extractStringArray(cert.Subject.StreetAddress)))
+	if len(csr.Subject.StreetAddress) > 0 {
+		log.Info(getNotSupportedMessage("subject.streetAddress", extractStringArray(csr.Subject.StreetAddress)))
 	}
 
-	if len(cert.Subject.PostalCode) > 0 {
-		log.Info(getNotSupportedMessage("subject.postalCodes", extractStringArray(cert.Subject.PostalCode)))
+	if len(csr.Subject.PostalCode) > 0 {
+		log.Info(getNotSupportedMessage("subject.postalCodes", extractStringArray(csr.Subject.PostalCode)))
 	}
 
-	if len(cert.Subject.SerialNumber) > 0 {
-		log.Info(getNotSupportedMessage("subject.serialNumber", cert.Subject.SerialNumber))
+	if len(csr.Subject.SerialNumber) > 0 {
+		log.Info(getNotSupportedMessage("subject.serialNumber", csr.Subject.SerialNumber))
 	}
 
 }
 
 func extractStringArray(strArray []string) string {
 	values := ""
-	for _, emailSANs := range strArray {
-		values = values + emailSANs + ", "
+	for _, val := range strArray {
+		values = values + val + ", "
 	}
 	return values
 }
@@ -125,6 +131,14 @@ func extractIPAddresses(addresses []net.IP) string {
 	return values
 }
 
-func getNotSupportedMessage(property string, values string) string {
-	return "WARNING: Property '" + property + "' with value: " + values + " is not supported by " + CertServiceName
+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 getOverriddenMessage(property string, values string) string {
+	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 7d1abc2c..ea1076dc 100644
--- a/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger_test.go
+++ b/certServiceK8sExternalProvider/src/cmpv2controller/logger/certificate_request_logger_test.go
@@ -33,12 +33,18 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog/v2"
 	"k8s.io/klog/v2/klogr"
+
+	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'"}
+
+
 func TestMain(m *testing.M) {
 	klog.InitFlags(nil)
 	flag.CommandLine.Set("v", "10")
@@ -55,8 +61,13 @@ func TestLogShouldNotProvideInformationAboutSkippedPropertiesIfNotExistInCSR(t *
 	request := getCertificateRequestWithoutSkippedProperties()
 	tmpWriteBuffer := getLogBuffer()
 
+	csr, err := x509utils.DecodeCSR(request.Spec.Request)
+	if err != nil {
+		assert.FailNow(t, "Could not parse Certificate Sign Request")
+	}
+
 	//when
-	LogCertRequestProperties(logger, request)
+	LogCertRequestProperties(logger, request, csr)
 	closeLogBuffer()
 	logsArray := convertBufferToStringArray(tmpWriteBuffer)
 	//then
@@ -71,8 +82,13 @@ func TestLogShouldProvideInformationAboutSkippedPropertiesIfExistInCSR(t *testin
 	request := getCertificateRequestWithSkippedProperties()
 	tmpWriteBuffer := getLogBuffer()
 
+	csr, err := x509utils.DecodeCSR(request.Spec.Request)
+	if err != nil {
+		assert.FailNow(t, "Could not parse Certificate Sign Request")
+	}
+
 	//when
-	LogCertRequestProperties(logger, request)
+	LogCertRequestProperties(logger, request, csr)
 	closeLogBuffer()
 	logsArray := convertBufferToStringArray(tmpWriteBuffer)
 
@@ -82,6 +98,28 @@ func TestLogShouldProvideInformationAboutSkippedPropertiesIfExistInCSR(t *testin
 	}
 }
 
+func TestLogShouldListSupportedProperties(t *testing.T) {
+	//given
+	logger := klogr.New()
+	request := getCertificateRequestWithoutSkippedProperties()
+	tmpWriteBuffer := getLogBuffer()
+
+	csr, err := x509utils.DecodeCSR(request.Spec.Request)
+	if err != nil {
+		assert.FailNow(t, "Could not parse Certificate Sign Request")
+	}
+
+	//when
+	LogCertRequestProperties(logger, request, csr)
+	closeLogBuffer()
+	logsArray := convertBufferToStringArray(tmpWriteBuffer)
+
+	//then
+	for _, logMsg := range supportedProperties {
+		assert.True(t, logsContainExpectedMessage(logsArray, logMsg), "Logs not contain: "+logMsg)
+	}
+}
+
 func getCertificateRequestWithoutSkippedProperties() *cmapi.CertificateRequest {
 	request := new(cmapi.CertificateRequest)
 	request.Spec.Request = []byte(csrWithoutSkippedProperties)