From 4c83d36c4633e8f2eb262fba6924db6b26cfca48 Mon Sep 17 00:00:00 2001
From: Adam Grare <adam@grare.com>
Date: Wed, 8 Nov 2023 12:51:20 -0500
Subject: [PATCH] Check if a workflow payload is valid during sync

When syncing a repository and creating workflows check if the payload is
valid and if it isn't set a (hopefully) helpful error message.
---
 .../configuration_script_source.rb            | 18 ++++++++-
 .../configuration_script_source_spec.rb       | 38 +++++++++++++++++--
 spec/support/fake_workflows_repo.rb           | 19 ++++++++--
 3 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/app/models/manageiq/providers/workflows/automation_manager/configuration_script_source.rb b/app/models/manageiq/providers/workflows/automation_manager/configuration_script_source.rb
index f4268df..095882a 100644
--- a/app/models/manageiq/providers/workflows/automation_manager/configuration_script_source.rb
+++ b/app/models/manageiq/providers/workflows/automation_manager/configuration_script_source.rb
@@ -76,8 +76,24 @@ def sync_from_content(to_delete)
   end
 
   def create_workflow_from_payload(name, payload)
+    payload_valid, payload_error =
+      begin
+        Floe::Workflow.new(payload)
+        true
+      rescue Floe::InvalidWorkflowError => err
+        [false, err.message]
+      end
+
     configuration_script_payloads.find_or_initialize_by(:name => name).tap do |wf|
-      wf.update!(:name => name, :manager_id => manager_id, :type => self.class.module_parent::Workflow.name, :payload => payload, :payload_type => "json")
+      wf.update!(
+        :name          => name,
+        :manager_id    => manager_id,
+        :type          => self.class.module_parent::Workflow.name,
+        :payload       => payload,
+        :payload_type  => "json",
+        :payload_valid => payload_valid,
+        :payload_error => payload_error
+      )
     end
   end
 end
diff --git a/spec/models/manageiq/providers/workflows/automation_manager/configuration_script_source_spec.rb b/spec/models/manageiq/providers/workflows/automation_manager/configuration_script_source_spec.rb
index a0eb4be..393b92a 100644
--- a/spec/models/manageiq/providers/workflows/automation_manager/configuration_script_source_spec.rb
+++ b/spec/models/manageiq/providers/workflows/automation_manager/configuration_script_source_spec.rb
@@ -194,12 +194,44 @@ def files_in_repository(git_repo_dir)
         record = build_record
 
         expect(record.configuration_script_payloads.first).to have_attributes(
-          :name         => "hello_world.asl",
-          :payload      => a_string_including("\"Comment\": \"hello world\""),
-          :payload_type => "json"
+          :name          => "hello_world.asl",
+          :payload       => a_string_including("\"Comment\": \"hello world\""),
+          :payload_type  => "json",
+          :payload_valid => true,
+          :payload_error => nil
         )
       end
 
+      context "with workflows with invalid json" do
+        let(:repo_dir_structure) { %w[invalid_json.asl] }
+
+        it "sets the payload_valid and payload_error attributes" do
+          record = build_record
+          expect(record.configuration_script_payloads.first).to have_attributes(
+            :name          => "invalid_json.asl",
+            :payload       => "{\"Invalid Json\"\n",
+            :payload_type  => "json",
+            :payload_valid => false,
+            :payload_error => "unexpected token at '{\"Invalid Json\"\n'"
+          )
+        end
+      end
+
+      context "with workflows with missing states" do
+        let(:repo_dir_structure) { %w[missing_states.asl] }
+
+        it "sets the payload_valid and payload_error attributes" do
+          record = build_record
+          expect(record.configuration_script_payloads.first).to have_attributes(
+            :name          => "missing_states.asl",
+            :payload       => "{\"Comment\": \"Missing States\"}\n",
+            :payload_type  => "json",
+            :payload_valid => false,
+            :payload_error => "Missing field \"States\""
+          )
+        end
+      end
+
       context "with a nested dir" do
         let(:nested_repo) { File.join(clone_dir, "hello_world_nested") }
 
diff --git a/spec/support/fake_workflows_repo.rb b/spec/support/fake_workflows_repo.rb
index 167991e..d6611b3 100644
--- a/spec/support/fake_workflows_repo.rb
+++ b/spec/support/fake_workflows_repo.rb
@@ -12,10 +12,21 @@ def file_content(full_path)
         dummy_workflow_data_for(full_path) if full_path.fnmatch?("**/*.asl", File::FNM_EXTGLOB)
       end
 
-      def dummy_workflow_data_for(_filename)
-        <<~WORKFLOW_DATA
-          {"Comment": "hello world"}
-        WORKFLOW_DATA
+      def dummy_workflow_data_for(filename)
+        case filename.to_s
+        when /invalid_json.asl$/
+          <<~WORKFLOW_DATA
+            {"Invalid Json"
+          WORKFLOW_DATA
+        when /missing_states.asl$/
+          <<~WORKFLOW_DATA
+            {"Comment": "Missing States"}
+          WORKFLOW_DATA
+        else
+          <<~WORKFLOW_DATA
+            {"Comment": "hello world", "States": {"Start": {"Type": "Succeed"}}, "StartAt": "Start"}
+          WORKFLOW_DATA
+        end
       end
     end
   end