Skip to content

[installer-tests] add make targets to backup k8s user creds #13175

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

Merged
merged 1 commit into from
Sep 23, 2022

Conversation

nandajavarma
Copy link
Contributor

@nandajavarma nandajavarma commented Sep 21, 2022

Description

This PR basically adds a couple of make targets that will push user credentials for accessing kubeconfig of clusters created using the pipeline. The connection is slightly different for each managed cluster (documented in this internal doc):

  1. GKE: A user is created that has cluster developer access, and a JSON key is created for the same user. This key is uploaded to the GCS bucket we use for terraform backend. When the user needs access to it, they can login using gcloud auth login and running the make target make get-kubeconfig after setting the variable TF_VAR_TEST_ID
  2. k3s: The default workflow of K3s currently involves uploading the kubeconfig to the gcs bucket since it never expires. No change has been made here. When the user needs access to it, they can login using gcloud auth login and running the make target make get-kubeconfig after setting the variable TF_VAR_TEST_ID.
  3. eks: A new iam user is created with admin access to the cluster, and that has describe cluster policies attached. access key to this user is uploaded to the GCS bucket. When the user needs access to it, they can login using gcloud auth login and running the make target make get-kubeconfig after setting the variable TF_VAR_TEST_ID.
  4. aks: With azure, currently we expect user to have access to the azure portal. So basically, manual login using az login --use-device-code and then running: make get-kubeconfig having set the variable TF_VAR_TEST_ID.

Related Issue(s)

Fixes #

How to test

Please follow the documentation here.

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide

@nandajavarma nandajavarma force-pushed the nvn/add-gcp-auth branch 6 times, most recently from 17ecfb7 to 7ca1855 Compare September 22, 2022 14:37
@roboquat roboquat added size/L and removed size/M labels Sep 22, 2022
@nandajavarma nandajavarma changed the title [installer-tests] add basic connection setup to the clusters [installer-tests] add make targets to backup k8s user creds Sep 22, 2022
@nandajavarma nandajavarma marked this pull request as ready for review September 22, 2022 14:57
@nandajavarma nandajavarma requested a review from a team September 22, 2022 14:57
@nandajavarma nandajavarma added the team: delivery Issue belongs to the self-hosted team label Sep 22, 2022
@nandajavarma nandajavarma force-pushed the nvn/add-gcp-auth branch 7 times, most recently from dc4aae7 to 76b9a42 Compare September 22, 2022 21:51
Copy link
Contributor

@mrsimonemms mrsimonemms left a comment

Choose a reason for hiding this comment

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

/hold

There's a couple of suggestions/nits in here, but happy to approve on that basis. With regards to the indentation comment, I've noticed that it's currently consistent with the rest of the file - I think we probably want to do the "correct" indentation at some point, but I'm happy if you elect not to change in this file and we do it as a future task

@@ -33,35 +33,51 @@ help: Makefile
@sed -n 's/^##//p' $< | column -t -s ':' | sed -e 's/^/ /'
@echo

upload-gcp-cluster-creds:
export GKE_CREDS=$$(terraform output -json gke_user_key) && \
echo $$GKE_CREDS > gcp-creds
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
echo $$GKE_CREDS > gcp-creds
echo $$GKE_CREDS > gcp-creds

Worth indenting this once more as it's a multiline command

export KUBECONFIG=${KUBECONFIG} && \
gcloud container clusters get-credentials gp-${TF_VAR_TEST_ID} --zone europe-west1-d --project sh-automated-tests || $(MAKE) sync-kubeconfig || echo "No cluster present"
gcloud container clusters get-credentials gp-${TF_VAR_TEST_ID} --zone europe-west1-d --project sh-automated-tests || echo "No cluster present"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
gcloud container clusters get-credentials gp-${TF_VAR_TEST_ID} --zone europe-west1-d --project sh-automated-tests || echo "No cluster present"
gcloud container clusters get-credentials gp-${TF_VAR_TEST_ID} --zone europe-west1-d --project sh-automated-tests || echo "No cluster present"

Indentation as above

@echo "Done creating GKE cluster"

upload-eks-user:
export AWS_CLUSTER_USER=$$(terraform output -json aws_cluster_user) && \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest about the indentation again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am unholding this PR for now. Like you said, I have consistently not intended anywhere. So I think it would be better if I did a cleanup in a follow-up PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

That seems a very pragmatic decision 👍🏻

@nandajavarma nandajavarma force-pushed the nvn/add-gcp-auth branch 2 times, most recently from cbe0ef4 to e46c14f Compare September 23, 2022 12:26
@nandajavarma
Copy link
Contributor Author

/unhold

@roboquat roboquat merged commit f0d847c into main Sep 23, 2022
@roboquat roboquat deleted the nvn/add-gcp-auth branch September 23, 2022 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-none size/L team: delivery Issue belongs to the self-hosted team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants