helm-release-plugin
helm-release-plugin copied to clipboard
allow specifying service account to be used at ttl cleanup time
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.
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?
I was sick for a bit, I'll hope to pick this up this week
Hey @genisd @vadim-zabolotniy @rtpro , is there any chance to have this merged?
Yeah we still run the fork which would love not to do actually. The ball was in my court so let's pick it up.
Essentially the motivation is not to give the default
service account this kind of privileged access.
I'm no expert but it is my understanding that any code execution inside any container would also get those privileges.
Your suggestion @vadim-zabolotniy is kinda valid as I've found out. service accounts are specific to namespaces :melting_face: So it's easy to overlook that a service account doesn't exist in a specific namespace.
Let me see what I can make of it @duhow maybe you can help test/validate it :see_no_evil:
@genisd I'm also happy to validate an updated PR and merge.
When service account doesn't exist output is, as you can see we use htlm-ttl
for our service-account
$ helm release ttl health-pr-2359 --namespace=default --service-account='nonexisiting-service-account' --set='1 hour'
Service account list does not contains 'nonexisiting-service-account'.
Available service accounts: default helm-ttl
[help section follows here...]
I tested this locally and I think it looks okay. Looking forward to your feedback.
@duhow could you give this a spin as well please? since you're also interessted in having this land upstream :innocent:
easiest way to test this I found was this: https://github.com/JovianX/helm-release-plugin/pull/28/files#diff-eca12c0a30e25b4b46522ebf89465a03ba72a03f540796c979137931d8f92055R29
I added this as an alternative approach to testing.
Basically you can run:
helm plugin install https://github.com/gynzy/helm-release-plugin --version SRE-218-upstream
And then run the ttl as specified in the examples I.e.:
helm release ttl <RELEASE_NAME> --namespace=default --service-account='your-privileged-sa' --set='1 minute'
this should purge the release after 1 minute (please use a test helm release, don't get your production purged or anything)