Remove shell specification in Makefile for Ansible and Helm

[varshaprasad96]

Remove the shell specification in ansible
and helm makefiles.

Description of the change:
Currently, we are hardcoding SHELL to point to /bin/sh. Its not necessary that /bin/sh invokes sh or points to bash, it can be a symbolic link invoking any other shell which may give errors. Instead, if required we could add a documentation similar to as we have for Golang operators, suggesting to use bash.

Motivation for the change:
Closes: #4403

Checklist

If the pull request includes user-facing changes, extra documentation is required:

• Add a new changelog fragment in changelog/fragments (see changelog/fragments/00-template.yaml)
• Add or update relevant sections of the docs website in website/content/en/docs

[jmrodri]

I thought we put these in for a reason but I can't find why. We have it set to bash in the main Makefile.

[jmrodri]

Does this need a changelog? It will affect the generated Makefile right?

[jmrodri]

/lgtm

[joelanford]

/hold

These may be necessary due to the PATH setup that includes the project-local bin directory.

[joelanford]

If there's an alternate way of ensuring executables from $(pwd)/bin are preferred over others in $PATH when invoking them, that would probably be fine as well.

However I'm a bit confused. Based on the documentation here, it seems like we're basically just running make's default shell with a modified $PATH.

[varshaprasad96]

Ah, yes. Right now, the problem is that /bin/sh can have symbolic link to other shells and not necessary bash. And we setting it explicitly assuming it to be bash is causing errors. Can we instead just point to to bash directly, and document it ?

[joelanford]

@varshaprasad96 I'm wondering if the original issue reported is more of an edge case than a normal case. See https://stackoverflow.com/questions/4814006/can-i-assume-bash-is-installed

It seems like it's safer to assume /bin/sh than /bin/bash.

I'm not personally super opposed to switching to /bin/bash, but that has a chance of breaking someone else who has /bin/sh but not /bin/bash.

[joelanford]

Looking at this a bit closer and testing this PR out, I think this is actually a safe change. The changes @estroz made in #4407 make the SHELL setup redundant.

/hold cancel

[varshaprasad96]

/retest

[joelanford]

Is the PATH assignment on the line above this now obsolete as well?

[varshaprasad96]

yeah, given that we are specifically checking for binaries in $(pwd)/bin/ (like here for kustomize) and downloading if necessary at the right place in $pwd/bin.

[jmrodri]

/lgtm

[estroz]

/lgtm