konstructio / kubefirst

The Kubefirst Open Source Platform
https://kubefirst.konstruct.io/docs
MIT License
1.79k stars 139 forks source link

store GitHub token when permission is granted #1065

Closed converge closed 1 year ago

converge commented 1 year ago

As a Kubefirsr user I want to have the option to store my GitHub token at ~/.k1/ with that, next kubefirst commands can use the stored GH token, and not request the user for a new token.

Acceptance criteria:

note: on a next iteration, we could encrypt/decrypt the file / not now since the story is already big

jarededwards commented 1 year ago

@converge we just discussed in the 1.11 review that this would be a 1.12 item and we would focus on fine grained tokens. please do not continue this effort. also, why did we close #866 for a story that solves this similar issue? feels like the previous story was a fine place to continue token work

converge commented 1 year ago

@jarededwards Im actually working on tasks that are assigned to a milestone, and before starting Im assigning that for myself. I created this ticket based on a old slack thread to keep the task updated. The new created was created to avoid misunderstanding on what should be done with the task.

6za commented 1 year ago

@jarededwards / @converge I don't think we need to create another place to store the token.

That would solve a token need for destroy.

For init and create it seems we a have already in play in the code to merge this command on an new command that handles the driver install. So, this would live in memory of this command and eventually be available in the cluster once vault-terraform is done.

So, we seems to be re-doing what vault is already doing for us.

6za commented 1 year ago

My suggestions is to close this ticket as this information is already stored in the cluster.

converge commented 1 year ago

@jarededwards / @converge I don't think we need to create another place to store the token.

  • CI Secrets already is aware of the token, in vault. We just need to read the token from the cluster itself, if it still running, it has a secret there with the token.
  • We can check if the token is still valid, if not run the request token logic.
  • This issue seems to over complicate that with another mechanism to handle it.

That would solve a token need for destroy.

For init and create it seems we a have already in play in the code to merge this command on an new command that handles the driver install. So, this would live in memory of this command and eventually be available in the cluster once vault-terraform is done.

So, we seems to be re-doing what vault is already doing for us.

Vault can help if the cluster is up, until the cluster is not running, CLI will constantly request the GH token.

Common user scenario for local:

1.  go run . local [GH token required]
2. it fails for some reason
3. go run . clean 
4. go run . local [GH token required]
5. it fails for some reason
6. go run . clean
7. go run . local [GH token required]

for cloud installs:

1. go run . init [GH token required]
4. it fails for some reason
5. go run . init # succeed
6. go run . cluster create [GH token required]
7. it fails for some reason
8. go run . clean
9. go run . init [GH token required]
10. go run . cluster create [GH token required]
6za commented 1 year ago

For this scenarios, it is just better to use env-variable token that is already supported, if the go it is run multiple install in less the 3-4 hours or how long our token expires.

If the variable is already loaded, that would prevent all that work:

export KUBEFIRST_GITHUB_AUTH_TOKEN

I still believe that is a redundancy of a feature we already have. We could print a waring to user, if they desire to export the variable and the value just generated for them, if they want to keep in terminal for multiple runs, or generate a bash, they can just run to do that for them, if go is not able to export a variable to an active shell.

6za commented 1 year ago

In bash, it would be a simple: source something.sh and the result variables would be injected on the active context, I guess go has something similar.

converge commented 1 year ago

My preference is for automation than manual work, we have the possibility to ask as less as possible to the user and automate the whole login process, we can even take care of expired tokens and refresh with no user interaction.

bash script, or manual export needs to be called on new shell sessions. (Go os.setenv doesn't expose the ENV. to outside of the current execution)

There also some other benefits:

up to @jarededwards and @johndietz

6za commented 1 year ago

Anyway, I just wanted to share the info is already in the cluster. If there is a desire to have another place with it, we just need to let the users aware and let them choose, to avoid it in case, they care.

As credential handling is key for some users.