(panic): exec mixin being passed string instead of a map causes panic

[schristoff]

When using the exec mixin on a bundle the following:

exec: echo hello world

Will cause a panic
however

exec:
   command: '"echo hello world"'

Will not

We should not get to the point where the former will cause a panic and we should just error

[matheuscumpian]

Hi guys, I would like to tackle this one.

I'll come back with questions if I have, now I'm trying to learn a little bit of the project code 💌

[matheuscumpian]

Hi guys, I think that the issue was in the Validate() method of the Step struct in the manifest.go file, we were trying to access a field of a map right after a type casting without check if the cast happened with success.

d, ok := children.(map[string]interface{})["description"] --> Right here
f !ok {
    return "", nil
}

I've added another step to get the description after the casting, with that we can return an error, I've just tried to use an error message that looks like the other ones, but I don´t know if it makes sense:

invalid mixin type (type) for mixin step (mixin name)

With that, when we run this invalid exec mixin:

...
install:
  - exec: "I'm a string, but I should be an object 🫠"
...

We get the following output in the install command:

$ porter install

1 error occurred:
        * validation of action "install" failed: invalid mixin type (string) for mixin step (exec)

1 error occurred:
        * validation of action "install" failed: invalid mixin type (string) for mixin step (exec)

We're "duplicating" the error logs, I tried to figure out what was happening and It seems that Cobra has a "fallback" error log in the end of a command execution that logs the error if the silence error flags is not on, so because we're logging the error in the span.Error() we see the log duplicated.

I've tried other types of error and they also duplicated the logs, but I don't know if this is expected or if I should try to handle this