Add ability to handle non-existent & malformed Checks supplied to catalog

[jamie-suse]

Currently, when the user provides a Check that does not exist, or a malformed Check file, when starting execution via the CLI, the server crashes with an exception.

Current example scenario:

iex(1)> execution_id = UUID.uuid4()
"60172c74-26b3-4e54-9870-232668abfb1a"
iex(2)> targets = [%Wanda.Executions.Target{agent_id: "99cf8a3a-48d6-57a4-b302-6e4482227ab6", checks: ["non_existent_check", "156F64"]}]
[
  %Wanda.Executions.Target{
    agent_id: "99cf8a3a-48d6-57a4-b302-6e4482227ab6",
    checks: ["non_existent_check", "156F64"]
  }
]
iex(3)> Wanda.Executions.Server.start_execution(execution_id, UUID.uuid4(), targets, %{})
** (YamlElixir.FileNotFoundError) Failed to open file "priv/catalog/non_existent_check.yaml": no such file or directory
    (yaml_elixir 2.9.0) lib/yaml_elixir.ex:22: YamlElixir.read_from_file!/2
    (wanda 0.1.0) lib/wanda/catalog.ex:35: Wanda.Catalog.get_check/1
    (elixir 1.13.4) lib/enum.ex:1593: Enum."-map/2-lists^map/1-0-"/2
    (wanda 0.1.0) lib/wanda/executions/server.ex:35: Wanda.Executions.Server.start_execution/5

And similarly for a malformed Check file:

iex(1)> execution_id = UUID.uuid4()
"180577b8-fe6c-4f05-be8e-3431e3a28e5f"
iex(2)> targets = [%Wanda.Executions.Target{agent_id: "99cf8a3a-48d6-57a4-b302-6e4482227ab6", checks: ["malformed_check", "non_existent_check", "156F64"]}]
[
  %Wanda.Executions.Target{
    agent_id: "99cf8a3a-48d6-57a4-b302-6e4482227ab6",
    checks: ["malformed_check", "non_existent_check", "156F64"]
  }
]
iex(3)> Wanda.Executions.Server.start_execution(execution_id, UUID.uuid4(), targets, %{})
** (FunctionClauseError) no function clause matching in Wanda.Catalog.map_check/1    
    
    The following arguments were given to Wanda.Catalog.map_check/1:
    
        # 1
        %{
          "description" => "A malformed check\n",
          "expectations" => [
            %{"expect_same" => "facts.jedi", "name" => "some_expectation"}
          ],
          "group" => "Test",
          "id" => "malformed_check",
          "name" => "Malformed check",
          "remediation" => "## Remediation\nRemediation text\n"
        }
    
    Attempted function clauses (showing 1 out of 1):
    
        defp map_check(%{
      "id" => id,
      "name" => name,
      "group" => group,
      "description" => description,
      "remediation" => remediation,
      "facts" => facts,
      "expectations" => expectations
    } = check)
    
    (wanda 0.1.0) lib/wanda/catalog.ex:51: Wanda.Catalog.map_check/1
    (elixir 1.13.4) lib/enum.ex:1593: Enum."-map/2-lists^map/1-0-"/2
    (wanda 0.1.0) lib/wanda/executions/server.ex:35: Wanda.Executions.Server.start_execution/5

This PR changes the behaviour so that when the catalogue is given a non-existent or malformed Check, it logs an error and continues processing the other checks in the list.

iex(1)> execution_id = UUID.uuid4()
"710b8bfd-f2b3-4eb2-b390-b6a3f56e0cad"
iex(2)> targets = [%Wanda.Executions.Target{agent_id: "99cf8a3a-48d6-57a4-b302-6e4482227ab6", checks: ["malformed_check", "non_existent_check", "156F64"]}]
[
  %Wanda.Executions.Target{
    agent_id: "99cf8a3a-48d6-57a4-b302-6e4482227ab6",
    checks: ["malformed_check", "non_existent_check", "156F64"]
  }
]
iex(3)> Wanda.Executions.Server.start_execution(execution_id, UUID.uuid4(), targets, %{})
[error] Check with ID malformed_check is malformed. Check if all the required fields are present.
[error] Error getting Check with ID non_existent_check: %YamlElixir.FileNotFoundError{message: "Failed to open file \"priv/catalog/non_existent_check.yaml\": no such file or directory"}
[debug] Starting execution: 710b8bfd-f2b3-4eb2-b390-b6a3f56e0cad
:ok

[coveralls]

Coverage increased (+0.6%) to 96.078% when pulling 076f680 on handle_non_existent_check into 35d62ca on main.

[arbulu89]

@jamie-suse The implementation and tests look great.
I have added a side note on making map_check more resilient against malformed check files.
I think it is pretty simple. Do you think we could include it in this PR?
Otherwise, I'm fine on merging this and creating a next PR with the update.

[nelsonkopliku]

I tried the timeout scenario and I got an error while the server was handling this.

** (MatchError) no match of right hand side value: nil
    (wanda 0.1.0) lib/wanda/executions/evaluation.ex:42

Thing is that non_existent_check remains in the targets piece of state of the execution and if that target timeouts, non_existent_check would still be taken into account for evaluation execution causing an error because it won't be found among the available checks, when queried for it. See server.ex#L31, server.ex#L140 and evaluation.ex#L42

At execution startup we could simply clean up targets state from non existing checks, if we do not use them.

[arbulu89]

Ok @jamie-suse ,
Green light from my side.
Let's give some time to others to review if they want. We can merge next week otherwise

[arbulu89]

I tried the timeout scenario and I got an error while the server was handling this.

** (MatchError) no match of right hand side value: nil
    (wanda 0.1.0) lib/wanda/executions/evaluation.ex:42

Thing is that non_existent_check remains in the targets piece of state of the execution and if that target timeouts, non_existent_check would still be taken into account for evaluation execution causing an error because it won't be found among the available checks, when queried for it. See server.ex#L31, server.ex#L140 and evaluation.ex#L42

At execution startup we could simply clean up targets state from non existing checks, if we do not use them.

Really good catch @nelsonkopliku ,
We need to exclude the unknown check ids from the rest of the execution.
Work for monday hehe

[fabriziosestito]

Agreed with @arbulu89 to make this pass and work on improved error handing in another PR

[nelsonkopliku]

Looks good, thanks for taking care also of the timeout scenario!.

Left just a couple of non blocking comments.
One extra thing coming to my mind is if it would be valuable enhancing the tests for the timeout scenario to cover this case.

Up to you.