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

Support population of kubeconfig files #102

Merged
merged 5 commits into from
Jul 7, 2021
Merged

Conversation

hasheddan
Copy link
Contributor

Description of your changes

Adds support for merging kubeconfig data for a control plane into local kubeconfig.

Fixes #86

All keys in the new kubeconfig entries are structured as upbound-<control-plane-id>

I have:

  • Read and followed Upbound's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR, as appropriate.

How has this code been tested

Tested the following scenarios:

  • Merge into existing default kubeconfig and verify that other entries are not overwritten:
cat token.txt | up cloud ctp kubeconfig get 7a25b9a5-43a9-474a-a92d-f2afe839095b --token=-
🤖 (up) k get pods -A
NAMESPACE        NAME                                      READY   STATUS    RESTARTS   AGE
upbound-system   crossplane-8644547dfd-ptwwx               1/1     Running   0          3h19m
upbound-system   crossplane-rbac-manager-57b44db5d-xt76w   1/1     Running   0          3h19m
upbound-system   upbound-agent-59f5c67c64-zrrjt            1/1     Running   0          3h19m
upbound-system   upbound-bootstrapper-69478b8c87-86lmh     1/1     Running   0          3h19m
upbound-system   xgql-67585677b9-8285b                     1/1     Running   2          3h19m
  • Create new kubeconfig:
cat token.txt | up cloud ctp kubeconfig get 7a25b9a5-43a9-474a-a92d-f2afe839095b --token=- -f kube.conf
🤖 (up) k get pods -A --kubeconfig=kube.conf
NAMESPACE        NAME                                      READY   STATUS    RESTARTS   AGE
upbound-system   crossplane-8644547dfd-ptwwx               1/1     Running   0          3h19m
upbound-system   crossplane-rbac-manager-57b44db5d-xt76w   1/1     Running   0          3h19m
upbound-system   upbound-agent-59f5c67c64-zrrjt            1/1     Running   0          3h19m
upbound-system   upbound-bootstrapper-69478b8c87-86lmh     1/1     Running   0          3h19m
upbound-system   xgql-67585677b9-8285b                     1/1     Running   2          3h19m

Adds a helper to extract the default kubeconfig and merge the data for
the specified control plane and auth user into it. Default kubeconfig
loading rules are followed. If a path that does not exist is provided, a
new file will be created.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Adds a command to get a kubeconfig for a specified control plane.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
The kubeconfig commands do not inherit anything from their parent
commands, but the nesting is more intuitive then separating into their
own group.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Adds documentation for the control plane kubeconfig subgroup.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Copy link

@AaronME AaronME left a comment

Choose a reason for hiding this comment

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

The trailing \n in the token is giving me the following error:

Unable to connect to the server: net/http: invalid header field value "Bearer eyJhb...\n" for key Authorization

This is caused by using a block when the kubeconfig is written:

- name: upbound-d21d63fe-88fd-406f-a14f-6a3eb665b772
  user:
    token: |
        eyJhb...

The following works:

- name: upbound-d21d63fe-88fd-406f-a14f-6a3eb665b772
  user:
    token: eyJhb...

And so does this:

- name: upbound-d21d63fe-88fd-406f-a14f-6a3eb665b772
  user:
    token: |-
        eyJhb...

Copy link
Contributor Author

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

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

@AaronME we are consuming upstream functionality to merge the kubeconfig, and IMO providing a token with a trailing newline is an invalid token. That being said, we could strip off all whitespace characters for a provided token to allow for a smoother UX. What do you think?

@AaronME
Copy link

AaronME commented Jul 7, 2021

This was a consequence of me running echo "${TOKEN}" > token.txt -- which added a newline to the token.

I agree that sanitizing the token input would improve the UX.

Trims unicode whitespace from tokens provided via stdin. This helps
alleviate issues where a token file contains trailing newlines, etc.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Copy link
Contributor Author

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

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

@AaronME updated, also added whitespace trimming to up uxp connect when token is provided via stdin 👍🏻

Copy link

@AaronME AaronME left a comment

Choose a reason for hiding this comment

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

LGTM!

- Flags:
- `--token = STRING` (*Required*): API token for authenticating to
control plane. If `-` is given the value will be read from stdin.
- `-f,--file = FILE`: File to merge `kubeconfig`.

Choose a reason for hiding this comment

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

Should we assume a default kubeconfig file name/path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@grantgumina yep :) since we use the upstream kubeconfig loading rules we automatically default to proper path and honor things like KUBECONFIG env var 👍🏻

@hasheddan hasheddan merged commit acb9bac into upbound:main Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support population of kubeconfig files
3 participants