From 4d19b82663f9ebc9fa00ed4bfb9fea12570fead6 Mon Sep 17 00:00:00 2001
From: Jason Frey <fryguy9@gmail.com>
Date: Tue, 14 Jan 2025 14:18:01 -0500
Subject: [PATCH] Merge pull request #124 from
 agrare/skip_invalid_workflows_during_sync

Skip invalid workflows during sync

(cherry picked from commit 0606398d9b5f77c49792ad7c5a19d712e2ff28eb)
---
 .../configuration_script_source.rb            | 14 ++++++----
 .../configuration_script_source_spec.rb       | 28 ++++++++++++++++---
 spec/support/fake_workflows_repo.rb           | 19 ++++++++++---
 3 files changed, 48 insertions(+), 13 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 b818754..4f72675 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
@@ -55,7 +55,8 @@ def sync_from_git_repository(to_delete)
 
         payload  = worktree.read_file(filename)
         workflow = create_workflow_from_payload(filename, payload, fail_on_invalid_workflow: false)
-        to_delete.delete(workflow.name)
+
+        to_delete.delete(workflow.name) if workflow
       end
     end
   end
@@ -68,21 +69,24 @@ def sync_from_content(to_delete)
         payload       = File.read(filename)
         workflow      = create_workflow_from_payload(workflow_name, payload, fail_on_invalid_workflow: true)
 
-        to_delete.delete(workflow.name)
+        to_delete.delete(workflow.name) if workflow
       end
     end
   end
 
   def create_workflow_from_payload(name, payload, fail_on_invalid_workflow:)
-    floe_workflow =
+    floe_workflow, payload_error =
       begin
         Floe::Workflow.new(payload)
-      rescue Floe::InvalidWorkflowError
+      rescue Floe::InvalidWorkflowError, NotImplementedError => err
+        _log.warn("Invalid ASL file [#{name}]: #{err}")
         raise if fail_on_invalid_workflow
 
-        nil
+        [nil, err.message]
       end
 
+    return if payload_error
+
     description = floe_workflow&.comment
 
     configuration_script_payloads.find_or_initialize_by(:name => name).tap do |wf|
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 8e8c40d..039185d 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,13 +194,33 @@ def files_in_repository(git_repo_dir)
         record = build_record
 
         expect(record.configuration_script_payloads.first).to have_attributes(
-          :name         => "hello_world.asl",
-          :description  => "hello world",
-          :payload      => a_string_including("\"Comment\": \"hello world\""),
-          :payload_type => "json"
+          :name          => "hello_world.asl",
+          :description   => "hello world",
+          :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 "skips the invalid workflow file" do
+          record = build_record
+          expect(record.configuration_script_payloads).to be_empty
+        end
+      end
+
+      context "with workflows with missing states" do
+        let(:repo_dir_structure) { %w[missing_states.asl] }
+
+        it "skips the invalid workflow file" do
+          record = build_record
+          expect(record.configuration_script_payloads).to be_empty
+        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 9d43550..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", "States": {"Start": {"Type": "Succeed"}}, "StartAt": "Start"}
-        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