From c5a6f33c37cac42691f8f338b9545f84e65ce81b Mon Sep 17 00:00:00 2001
From: Oliver Sand <oliver.sand@sda-se.com>
Date: Fri, 4 Jun 2021 16:48:38 +0200
Subject: [PATCH] Treat empty string as undefined

Signed-off-by: Oliver Sand <oliver.sand@sda-se.com>
---
 .../v1beta2-demo/template/catalog-info.yaml   |  2 +-
 .../src/scaffolder/tasks/TaskWorker.test.ts   |  2 +-
 .../src/scaffolder/tasks/TaskWorker.ts        | 34 ++++++++++++++++++-
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template/catalog-info.yaml b/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template/catalog-info.yaml
index b87ea174f0..02da577b39 100644
--- a/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template/catalog-info.yaml
+++ b/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template/catalog-info.yaml
@@ -7,6 +7,6 @@ spec:
   type: website
   lifecycle: experimental
   owner: {{cookiecutter.owner | jsonify}}
-{%- if cookiecutter.system != "" %}
+{%- if 'system' in cookiecutter %}
   system: {{ cookiecutter.system | jsonify }}
 {%- endif %}
diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts
index b381d8af65..7136998aa5 100644
--- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts
+++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts
@@ -237,7 +237,7 @@ describe('TaskWorker', () => {
 
     const { events } = await storage.listEvents({ taskId });
     const event = events.find(e => e.type === 'completion');
-    expect((event?.body?.output as JsonObject).result).toEqual('');
+    expect((event?.body?.output as JsonObject).result).toBeUndefined();
   });
 
   it('should parse strings as objects if possible', async () => {
diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts
index f7ba181c8f..94eaa62c85 100644
--- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts
+++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts
@@ -122,6 +122,11 @@ export class TaskWorker {
                       preventIndent: true,
                     })(templateCtx);
 
+                    // If it's just an empty string, treat it as undefined
+                    if (templated === '') {
+                      return undefined;
+                    }
+
                     try {
                       return JSON.parse(templated);
                     } catch {
@@ -165,8 +170,14 @@ export class TaskWorker {
                   preventIndent: true,
                 })(templateCtx);
 
+                // If it's just an empty string, treat it as undefined
+                if (templated === '') {
+                  return undefined;
+                }
+
                 // If it smells like a JSON object then give it a parse as an object and if it fails return the string
                 if (
+                  (templated.startsWith('"') && templated.endsWith('"')) ||
                   (templated.startsWith('{') && templated.endsWith('}')) ||
                   (templated.startsWith('[') && templated.endsWith(']'))
                 ) {
@@ -245,11 +256,32 @@ export class TaskWorker {
         JSON.stringify(task.spec.output),
         (_key, value) => {
           if (typeof value === 'string') {
-            return this.handlebars.compile(value, {
+            const templated = this.handlebars.compile(value, {
               noEscape: true,
               data: false,
               preventIndent: true,
             })(templateCtx);
+
+            // If it's just an empty string, treat it as undefined
+            if (templated === '') {
+              return undefined;
+            }
+
+            // If it smells like a JSON object then give it a parse as an object and if it fails return the string
+            if (
+              (templated.startsWith('"') && templated.endsWith('"')) ||
+              (templated.startsWith('{') && templated.endsWith('}')) ||
+              (templated.startsWith('[') && templated.endsWith(']'))
+            ) {
+              try {
+                // Don't recursively JSON parse the values of this string.
+                // Shouldn't need to, don't want to encourage the use of returning handlebars from somewhere else
+                return JSON.parse(templated);
+              } catch {
+                return templated;
+              }
+            }
+            return templated;
           }
           return value;
         },