[OOM-K8S-CERT-EXTERNAL-PROVIDER] Refactoring & code improvements

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

2 files changed, 43 insertions, 27 deletions

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)
 }