[Fix] Improve validation for service parameters in MtaTransformer and correct error logging in util (#87)

Author: anirudhprasad-sap

bin/cap-op-plugin.js

@@ -36,7 +36,7 @@ async function capOperatorPlugin(cmd, option, yamlPath) {
             if (option === '--with-input-yaml' && !yamlPath)
                 return _usage(`Input yaml path is missing.`)
 
-            if (option === '--with-input-yaml' && !yamlPath && cds.utils.exists(cds.utils.path.join(cds.root, yamlPath)))
+            if (option === '--with-input-yaml' && yamlPath && !cds.utils.exists(cds.utils.path.join(cds.root, yamlPath)))
                 return _usage(`Input yaml path ${yamlPath} does not exist.`)
 
             await generateRuntimeValues(option, yamlPath)
@@ -46,7 +46,7 @@ async function capOperatorPlugin(cmd, option, yamlPath) {
             if (option === '--with-runtime-yaml' && !yamlPath)
                 return _usage(`Input runtime yaml path is missing.`)
 
-            if (option === '--with-runtime-yaml' && !yamlPath && cds.utils.exists(cds.utils.path.join(cds.root, yamlPath)))
+            if (option === '--with-runtime-yaml' && yamlPath && !cds.utils.exists(cds.utils.path.join(cds.root, yamlPath)))
                 return _usage(`Input runtime yaml path ${yamlPath} does not exist.`)
 
             await convertToconfigurableTemplateChart(option, yamlPath)

lib/mta-transformer.js

@@ -23,16 +23,16 @@ module.exports = class MtaTransformer {
 
     async readMtaExtensions() {
         let mtaExtensions = []
-        for(let file of this.mtaExtensionPaths) {
-          let mtaExt = cds.parse.yaml(await cds.utils.read(cds.utils.path.join(cds.root, file)))
-          if (mtaExt && mtaExt["_schema-version"].split(".")[0] === this.mta["_schema-version"].split(".")[0] && mtaExt["extends"] === this.mta["ID"]) {
-            mtaExtensions.push(mtaExt)
-          } else {
-            console.log(`⚠️  ${file} is not a valid mta extension file. _schema major version or extends doesn't match. Skipping...`)
-          }
+        for (let file of this.mtaExtensionPaths) {
+            let mtaExt = cds.parse.yaml(await cds.utils.read(cds.utils.path.join(cds.root, file)))
+            if (mtaExt && mtaExt["_schema-version"].split(".")[0] === this.mta["_schema-version"].split(".")[0] && mtaExt["extends"] === this.mta["ID"]) {
+                mtaExtensions.push(mtaExt)
+            } else {
+                console.log(`⚠️  ${file} is not a valid mta extension file. _schema major version or extends doesn't match. Skipping...`)
+            }
         }
         return mtaExtensions
-      }
+    }
 
     async getMta() {
         this.mta = cds.parse.yaml(await cds.utils.read(join(cds.root, this.mtaFilePath)))
@@ -41,7 +41,7 @@ module.exports = class MtaTransformer {
 
         this.mergedMta = this.merge(this.mta, this.mtaExtensions)
 
-        if (this.mergedMta.parameters){
+        if (this.mergedMta.parameters) {
             this.mergedMta.modules = replacePlaceholders(this.mergedMta.modules, this.mergedMta.parameters)
             this.mergedMta.resources = replacePlaceholders(this.mergedMta.resources, this.mergedMta.parameters)
         }
@@ -78,7 +78,7 @@ module.exports = class MtaTransformer {
     merge(mta, mtaExtensions) {
         let mergedMta = mta
 
-        for(let mtaExtension of mtaExtensions) {
+        for (let mtaExtension of mtaExtensions) {
             // Merge parameters
             mergedMta.parameters = mergeObj(mergedMta.parameters, mtaExtension.parameters)
 
@@ -94,21 +94,26 @@ module.exports = class MtaTransformer {
     }
 
     async getServiceInstances() {
-        if(this.serviceInstances.size !== 0) return this.serviceInstances
+        if (this.serviceInstances.size !== 0) return this.serviceInstances
 
-        if(!this.mergedMta) await this.getMta()
+        if (!this.mergedMta) await this.getMta()
 
         let managedSvcs = this.mergedMta?.resources.filter(r => r.type === 'org.cloudfoundry.managed-service') || []
-        for(let svc of managedSvcs) {
-            this.serviceInstances.set(
-                svc.parameters["service"] && svc.parameters["service-plan"] ? convertHypenNameToCamelcase(svc.parameters["service"] + "-" +svc.parameters["service-plan"]) : null,
-                {
-                    name: svc.name,
-                    serviceOfferingName: svc.parameters["service"] ? svc.parameters["service"] : null,
-                    servicePlanName: svc.parameters["service-plan"] ? svc.parameters["service-plan"] : null,
-                    parameters: svc.parameters["config"] ? svc.parameters["config"] : {}
-                }
-            )
+        for (let svc of managedSvcs) {
+            try {
+                this.serviceInstances.set(
+                    convertHypenNameToCamelcase(svc.parameters["service"] + "-" + svc.parameters["service-plan"]),
+                    {
+                        name: svc.name,
+                        serviceOfferingName: svc.parameters["service"],
+                        servicePlanName: svc.parameters["service-plan"],
+                        parameters: svc.parameters["config"] || {}
+                    }
+                )
+            } catch (err) {
+                console.warn(`⚠️  Error processing service instance ${svc.name} - ${err.message}. Skipping...`)
+                continue
+            }
         }
 
         // for xsuaa service instance, remove oauth2-configuration -> redirect-uris as it is filled by service-instance.yaml template
@@ -151,7 +156,7 @@ module.exports = class MtaTransformer {
     async getServiceBindings() {
         if (this.serviceBindings.size !== 0) return this.serviceBindings
 
-        if (this.workloads.size === 0 ) await this.getWorkloads()
+        if (this.workloads.size === 0) await this.getWorkloads()
 
         return this.serviceBindings
     }
@@ -181,9 +186,9 @@ module.exports = class MtaTransformer {
                 serviceInstanceName: consumedBTPService.name,
             }
             if (this.serviceBindings.has(serviceBindingNameKey) &&
-                    md5(JSON.stringify(this.serviceBindings.get(serviceBindingNameKey))) !== md5(JSON.stringify(serviceBindingValue))) {
+                md5(JSON.stringify(this.serviceBindings.get(serviceBindingNameKey))) !== md5(JSON.stringify(serviceBindingValue))) {
 
-                let serviceBindingNewName = consumedBTPService.name +"-"+ module.name + this.serviceBindingNameSuffix
+                let serviceBindingNewName = consumedBTPService.name + "-" + module.name + this.serviceBindingNameSuffix
                 let serviceBindingNewNameKey = convertHypenNameToCamelcase(serviceBindingNewName)
                 this.serviceBindings.set(serviceBindingNewNameKey, {
                     name: serviceBindingNewName,
@@ -194,14 +199,14 @@ module.exports = class MtaTransformer {
                 })
                 workloadServiceBindings.push(serviceBindingNewName)
             } else if (this.serviceBindings.has(serviceBindingNameKey) &&
-                    md5(JSON.stringify(this.serviceBindings.get(serviceBindingNameKey))) === md5(JSON.stringify(serviceBindingValue))) {
+                md5(JSON.stringify(this.serviceBindings.get(serviceBindingNameKey))) === md5(JSON.stringify(serviceBindingValue))) {
 
                 workloadServiceBindings.push(serviceBindingName)
                 continue
             }
             else {
                 workloadServiceBindings.push(serviceBindingName)
-                this.serviceBindings.set(serviceBindingNameKey,serviceBindingValue)
+                this.serviceBindings.set(serviceBindingNameKey, serviceBindingValue)
             }
         }
         return workloadServiceBindings
@@ -212,7 +217,7 @@ module.exports = class MtaTransformer {
         for (const [key, value] of Object.entries(properties)) {
             env.push({
                 name: key,
-                value: typeof value === 'string'? value : JSON.stringify(value)
+                value: typeof value === 'string' ? value : JSON.stringify(value)
             })
         }
         return env
@@ -231,7 +236,7 @@ module.exports = class MtaTransformer {
             }
             if (module.path?.includes("gen/mtx/sidecar") || module.type === 'com.sap.application.content') {
                 workload.jobDefinition = {
-                    type:  module.type === 'com.sap.application.content'? "Content": "TenantOperation",
+                    type: module.type === 'com.sap.application.content' ? "Content" : "TenantOperation",
                     image: null
                 }
             } else if (module.path?.includes("gen/srv")) {
@@ -255,9 +260,9 @@ module.exports = class MtaTransformer {
             // delete existing TENANT_HOST_PATTERN if any; Handled in cap-operator-cro.yaml template
             if (workload.deploymentDefinition?.type == "Router") {
                 if (workload.deploymentDefinition.env) {
-                   const index = workload.deploymentDefinition.env.findIndex(e => e.name === "TENANT_HOST_PATTERN")
-                   if (index != -1)
-                     workload.deploymentDefinition.env.splice(index, 1)
+                    const index = workload.deploymentDefinition.env.findIndex(e => e.name === "TENANT_HOST_PATTERN")
+                    if (index != -1)
+                        workload.deploymentDefinition.env.splice(index, 1)
                 }
             }
 

lib/util.js

@@ -137,7 +137,7 @@ function extractAndReplaceHanaInstanceId(env, valuesYaml) {
       env[index].value = JSON.stringify(cdsConfig)
     }
   } catch (err) {
-    console.warn(`Failed to parse CDS_CONFIG: ${error.message}`)
+    console.warn(`Failed to parse CDS_CONFIG: ${err.message}`)
   }
 }