Adding skip-deploy bitops configuration option

[PhillypHenning]

Updating the README with the skip-deploy action

[arm4b]

Looks like the older version SKIP_DEPLOY is listed in the docs, will need updating too:
https://bitops.sh/tool-configuration/configuration-helm/#options-configuration

[arm4b]

Looks like it's not true, but True upper-case per code here:

helm/scripts/helm_handle_chart.sh

Lines 18 to 22 in b77f296

	# Check for helm skip deploy condition
	if [ "$HELM_SKIP_DEPLOY" == "True" ]; then
	echo "helm.options.skip-deploy (HELM_SKIP_DEPLOY) set. Skipping deployment for $ENVIRONMENT/helm/$HELM_CHART"
	exit 0
	fi

Sounds like a bug? I see we use true lowercase for other plugins.

[arm4b]

I see only one config setting namespace in this README mentions the required.

Do we really need to list the required property? What's the convention we'd like to go with?

[PhillypHenning]

All other documentation has the "required" flag. Defaults to false.

How did you want to handle this?

[PhillypHenning]

Adding "tolowercase" before check to ensure standard input

test.sh

HELM_SKIP_DEPLOY=$1

SANITIZED_HELM_SKIP_DEPLOY=$(echo "$HELM_SKIP_DEPLOY" | tr '[:upper:]' '[:lower:]')

if [ "$SANITIZED_HELM_SKIP_DEPLOY" == "true" ]; then
    echo "helm.options.skip-deploy (HELM_SKIP_DEPLOY) set.  Skipping deployment for $ENVIRONMENT/helm/$HELM_CHART"
    exit 0
fi

output

[Bitovi] [helm] $ ./test.sh True
helm.options.skip-deploy (HELM_SKIP_DEPLOY) set.  Skipping deployment for /helm/

[PhillypHenning]

bitops.sh doc updated
bitovi/bitops#354

[arm4b]

This works for $HELM_SKIP_DEPLOY, but doesn't scale well as there are many ENV vars in the script, and doing it only for one var but leaving others broken is not consistent. Also, there might be other plugins having the same issue.

Just look below, there are many such vars in this script:

helm/scripts/helm_handle_chart.sh

Line 47 in a763179

if [[ "${FETCH_KUBECONFIG}" == "True" ]]; then

The ideal approach would be:
Creating a bash function, list all the boolean-type ENV vars and apply a lowercase function for all of them.
For scaling out, because this function should be common for all the plugins, we can create a repo with bash libs we can share via git submodule with all the plugins. It may be a home for other reusable helpers like color support, different logging levels, casting 0,1,yes,no to "bool" string and so on.

With that,
The simple approach with a filter for a single var is not enough as we fix bug only for one var.
The second overhaul approach is too much for the current PR scope.

---

For now the pragmatic way I think is just to change True -> true in the code and call it done. Considering the level of current adoption and the fact that lowercase was documented it should be OK.
We just need to keep in mind to use lowercase true/false in the future where possible. Worth checking other scripts and vars for the same bug.

For the future we can document an enhancement creating a shared repo with bash helpers.

@PhillypHenning WDYT?

[PhillypHenning]

I agree and have already applied the same sanity check for the SKIP_DEPLOY option across all plugins. I agree that the scope of this PR isn't enough to make the large scale change you're mentioning.

Finally - this is a change that only needs to be done and noted ON OUR SIDE. From the client perspective, they shouldn't know about the sanitation we do. I.E this doesn't need doc updates