JovianX / helm-release-plugin

Helm3 plugin that pulls(re-creates) helm Charts from deployed releases, and updates values of deployed releases without the chart.
Apache License 2.0
97 stars 12 forks source link

allow specifying service account to be used at ttl cleanup time #28

Open genisd opened 1 year ago

genisd commented 1 year ago

basically the default service account should not have the permissions needed to cleanup the helm release and the cronjob. hence with this PR one can specify which service account is to be used, avoiding the need for the default service account to get higher privileges

additionally don't use kubectl/helm latest versions implicitly, but specify which versions are to be used.

vadim-zabolotniy commented 1 year ago

Hi @genisd!

First of all I want to say thank you for your contribution to Helm release plugin. I have reviewed your changes. And on my opinion we have little issue that can be easily fixed. When we using does not existing service account we do not getting any warning or error message. I have prepared small MVP how it can be fixed you can use it as starting point:

service_accounts=`kubectl get serviceaccounts --output=yaml | yq  '.items[].metadata.name'`  # Fetching list of service accounts.
array=(${service_accounts})  # Converting it to array.

# Printing service accounts.
for index in ${!array[*]}
do
    printf '==> %d: %s\n' $index ${array[$index]}
done

service_account='default'

if [[ ${array[*]} =~ ${service_account} ]]; then  # 'contains' if condition example.
    printf "Service account list contains '$service_account'.\n"
fi

if [[ ! ${array[*]} =~ ${service_account} ]]; then  # 'not contains' if condition example
    printf "Service account list does not contains '$service_account'.\n"
fi

Another issue that I see is service account exists but don't have enough rights to remove Helm release. But for this issue I don't have easy solution.

Except this two issues I don't see any other blockers why we can't accept this PR. @rtpro?

genisd commented 1 year ago

I was sick for a bit, I'll hope to pick this up this week