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

kubectl command wrapper #172

Merged
merged 12 commits into from
Mar 20, 2023
Merged

kubectl command wrapper #172

merged 12 commits into from
Mar 20, 2023

Conversation

Franr
Copy link
Contributor

@Franr Franr commented Feb 15, 2023

What?

Add support to run kubectl commands from the leverage CLI.
It works by taking the kubectl config file from your home folder and passing the commands directly to the binary.
The authentication for AWS (required for EKS) is handled by leverage transparently (although is slow, so a topic open for discussion).

Why?

To keep unifying flows into a single tool. More details here.

References

New toolbox image required: binbashar/le-docker-leverage-toolbox#43

Open questions (Update: issues were created for all of them already!)

  • What version of kubectl do we want to support? The binaries are kind of lightweight (~40MB) so we can have multiples on the image and provide a command to do the switch.
  • Do we want automatic refreshing of the tokens like we do with the terraform commands? That is the approach I took, but is proved to be slow (takes ~2 seconds every time you execute a command). For terraform is feasible but for kubectl looks like is not. I would like to take another approach.
  • Should we add a layer of commands on top of it? Like a wrapper for get/use-context. We should analyze the complexity of it before adventuring into this direction, doesn't seems trivial.

ToDo

  • unit tests
  • flow documentation (login, set/switch contexts, run commands)
  • migrate token generation/refresh into python?

@diego-ojeda-binbash
Copy link
Collaborator

@exequielrafaela same comment here about reviewers and PR state 🙏

@exequielrafaela
Copy link
Member

@exequielrafaela same comment here about reviewers and PR state 🙏

@diego-ojeda-binbash agreed as stated in the related PR 🤝 ✅ 💪🏼

@Franr Franr marked this pull request as ready for review February 21, 2023 16:59
return f"bash -c \"{chain.join(commands)}\""


class CustomEntryPoint:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the use of context managers, very elegant solution!

auth_method = container.TF_SSO_ENTRYPOINT
elif container.mfa_enabled:
auth_method = container.TF_MFA_ENTRYPOINT
# TODO: ask why this was necessary
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@angelofenoglio @juanmatias maybe you can clarify me this guys?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!
The original use case in which the design was based on (containerized terraform commands, particularly MFA authenticated) made, and makes, quite a bit of effort to avoid overriding the host's credentials, and configuration files for the aws cli.
So the default behavior is to mount those files elsewhere (/tmp), use them as source of truth to know which profiles to assume and generate new aws config files that are going to perish alongside the container everytime a command is run.
With the use of SSO this is less and less common and we should think if it is really worth it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering this: https://github.com/binbashar/leverage/blob/master/leverage/container.py#L413-L416
We may as well get rid of this behavior, the thing is, it would take some tweaks both on the scripts and most of the containers definitions,

auth_method = container.TF_SSO_ENTRYPOINT
elif container.mfa_enabled:
auth_method = container.TF_MFA_ENTRYPOINT
# TODO: ask why this was necessary
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!
The original use case in which the design was based on (containerized terraform commands, particularly MFA authenticated) made, and makes, quite a bit of effort to avoid overriding the host's credentials, and configuration files for the aws cli.
So the default behavior is to mount those files elsewhere (/tmp), use them as source of truth to know which profiles to assume and generate new aws config files that are going to perish alongside the container everytime a command is run.
With the use of SSO this is less and less common and we should think if it is really worth it.

auth_method = container.TF_SSO_ENTRYPOINT
elif container.mfa_enabled:
auth_method = container.TF_MFA_ENTRYPOINT
# TODO: ask why this was necessary
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering this: https://github.com/binbashar/leverage/blob/master/leverage/container.py#L413-L416
We may as well get rid of this behavior, the thing is, it would take some tweaks both on the scripts and most of the containers definitions,

@Franr Franr merged commit 1d96739 into master Mar 20, 2023
@Franr Franr deleted the kubectl branch March 20, 2023 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants