[CI] add shellcheck-workflow

[led0nk]

Description:

So i fixed the issues which shellcheck displayed, which were only 3 double quote issues and the read -r issue.
After looking into the closed PR i also replaced printf-statements with echo.

For the shellcheck-worfklow I used https://github.com/ludeeus/action-shellcheck
I didn't find a way to check multiple directories at once, so we just use 2 steps here.

I added the disabled checks provided by the intentional issue, but i'm not quite sure if we need to add -x here. Couldn't find anything in the documentation so far.

On top of that i'm not quite sure if we should run the shellcheck in dependency of any other workflows or just on its own.

Edit:
I added the option ignore_paths so that shellcheck-action is only run in specified paths and ignores subpaths of this directory.

Link to tracking Issue:

• [ci] Add shellcheck linter #17279

Testing:

Documentation:

[led0nk]

@mx-psi

[mx-psi]

I added the disabled checks provided by the intentional issue, but i'm not quite sure if we need to add -x here. Couldn't find anything in the documentation so far.

I think we need it because of

opentelemetry-collector-contrib/.github/workflows/scripts/check-collector-module-version.sh

Line 11 in 394d9b4

source ./internal/buildscripts/modules

Running shellcheck spits:

  -x                  --external-sources         Allow 'source' outside of FILES

On top of that i'm not quite sure if we should run the shellcheck in dependency of any other workflows or just on its own.

Feels like a lint to me, I am also fine with a separate workflow at first to not make it required from the beginning.

I added the option ignore_paths so that shellcheck-action is only run in specified paths and ignores subpaths of this directory.

What's the motivation for that? I guess if something is inside .github/workflows/scripts/<something else> it's probably a script and we should lint it.

[led0nk]

What's the motivation for that? I guess if something is inside .github/workflows/scripts/<something else> it's probably a script and we should lint it.
@mx-psi

Well if you want to, we could change that and let it check every *.sh-type.
I just did it because your initial command was looking for these directories and not for subdirectories.
In my first run it also checked for e.g. yaml / .config .... files, so i tried to prevent this, but seems like it doesn't happen anymore.

Fixed it in latest commit.

Btw we should consider what our severity-level should be, because with nothing set we will get failed shellcheck-runs because of info / warning-messages..

Edit:
Maybe we should also consider adding sth to makefile, so the contributer/user can shellcheck it running a simple command.

[led0nk]

@TylerHelmuth what do you think?

[andrzej-stencel]

In general, I'm not a fan of using GitHub Actions "actions" for everything, even for relatively simple things like this. Reasons:

• It's not (easily) possible to reproduce the same run locally, which is a pain for contributors.
• We are opening another potential security hole - if the action is compromised or maliciously modified, our workflow runs arbitrary code.

In this case, running shellcheck should be easy enough with docker run -v ... koalaman/shellcheck:v0.10.0 - no need to even install anything. This can and should be enclosed in a Makefile target to easily run the same exact command both locally and in CI.

[evan-bradley]

Thanks @led0nk!