Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

genisd
Copy link

@genisd genisd commented Feb 16, 2023

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.

lib/api/ttl.sh Show resolved Hide resolved
@vadim-zabolotniy
Copy link
Contributor

vadim-zabolotniy commented Feb 23, 2023

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
Copy link
Author

genisd commented Mar 1, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants