non operator able to target and ssh node through bastion host #263

tedteng · 2020-08-19T03:22:53Z

What this PR does / why we need it:
allow non-operator user use project SA account in garden able to manage the shoots in their project via gardenctl target and gardenctl ssh to list nodes ssh shoot nodes through bastion host.

Which issue(s) this PR fixes:
Fixes #253
Fixes #233

Special notes for your reviewer:
as a request from the end-user currently the ssh only allow access aws provider.

Release note:

Allow `user` role use Project SA account in garden able to manage the shoots from their own project. `gardenctl ssh` shoot node as well.  `aws`,`gcp`,`azure` supported

tedteng · 2020-08-19T03:28:02Z

Testing

gg target -g ttt -p i333xxx -t zegfzwjhzh
Garden:
KUBECONFIG=/Users/i333xxx/gardener_kubeconfig/ttt/kubeconfig

Warning:
You are non-operator! usage are Limited

Shoot:
KUBECONFIG=/Users/i333878/.garden/cache/ttt/projects/i333878/zegfzwjhzh/kubeconfig.yaml

gg ssh list the node as expected.

gg ssh ip-xxxxx.eu-central-1.compute.internal

pkg/cmd/operate.go

pkg/cmd/miscellaneous.go

pkg/cmd/download.go

pkg/cmd/miscellaneous.go

pkg/cmd/ssh_aws.go

pkg/cmd/miscellaneous.go

petersutter · 2020-08-21T09:59:49Z

pkg/cmd/ssh.go

-		return nil, err
+	if checkOperatorRole() == "user" {
+		KUBECONFIG = getKubeConfigOfClusterType("shoot")
+		out, err := ExecCmdReturnOutput("bash", "-c", "kubectl --kubeconfig="+KUBECONFIG+" get node")


Why haven't you used the go client but instead executed kubectl to fetch the nodes?

sure I will change it

at least call kubectl directly, no bash please for new code

tedteng · 2020-08-28T06:47:25Z

Hi let me know is the commit square ok and check it before merge PR . @gardener/gardenctl-maintainers

neo-liang-sap · 2020-08-28T12:01:43Z

/lgtm
looks good to me but i haven't test it

neo-liang-sap · 2020-08-31T06:19:19Z

@tedteng could you please squash commits into one?

tedteng · 2020-08-31T06:24:53Z

you mean all the commits for just two commits from mine?

neo-liang-sap · 2020-08-31T06:31:21Z

you mean all the commits for just two commits from mine?

why other people's commit exist in your PR? ideally before merge one PR should contains only one commit from your self.
maybe you can try rebase with master branch but i'm not sure whether it helps to tidy your commits.
also i saw one empty commit exist in your PR, 51b6527 not sure how it comes but it shouldn't be there

tedteng · 2020-08-31T07:12:37Z

~~I have no idea either that why I was asking check last week. let me just close and open new PR, I think it will be easier for all.~~

you mean all the commits for just two commits from mine?

why other people's commit exist in your PR? ideally before merge one PR should contains only one commit from your self.
maybe you can try rebase with master branch but i'm not sure whether it helps to tidy your commits.
also i saw one empty commit exist in your PR, 51b6527 not sure how it comes but it shouldn't be there

it seems not fixed the issue create new one

neo-liang-sap

please resolve conflict with master branch
these 4 commits are already in master branch, should not exist in this PR

add temporary solution in targetShoot method add getTechnicalID Update pkg/cmd/download.go remove getShootClusterName method remove bash -c Update pkg/cmd/miscellaneous.go

neo-liang-sap

/lgtm
however i haven't tested, confirmed with @tedteng he's fully confidence with it

tedteng requested a review from a team as a code owner August 19, 2020 03:22

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 19, 2020

gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 19, 2020

tedteng force-pushed the nonadmin_target_ssh branch from 9b1c33c to 31ef9d5 Compare August 19, 2020 03:53

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 19, 2020

This was referenced Aug 19, 2020

As non garden admin I am not able to target shoots #233

Closed

As non garden admin I am not able to get logs or ssh into a node #253

Closed

tedteng force-pushed the nonadmin_target_ssh branch from 31ef9d5 to 1471e8a Compare August 19, 2020 05:56

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 19, 2020

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 19, 2020

tedteng force-pushed the nonadmin_target_ssh branch from 1471e8a to 7bbabbe Compare August 19, 2020 10:43

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 19, 2020

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 19, 2020

tedteng force-pushed the nonadmin_target_ssh branch from 7bbabbe to 9112f80 Compare August 19, 2020 10:46

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 19, 2020

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 19, 2020

tedteng force-pushed the nonadmin_target_ssh branch from 9112f80 to 34349a2 Compare August 20, 2020 01:37

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 20, 2020

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 20, 2020

tedteng mentioned this pull request Aug 20, 2020

Limit access to ssh node to bastion IP only #261

Closed

petersutter suggested changes Aug 21, 2020

View reviewed changes

tedteng force-pushed the nonadmin_target_ssh branch from 34349a2 to 048fd71 Compare August 21, 2020 13:30

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 21, 2020

tedteng force-pushed the nonadmin_target_ssh branch from 048fd71 to 43e61e4 Compare August 21, 2020 13:36

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 21, 2020

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 28, 2020

tedteng force-pushed the nonadmin_target_ssh branch from a2804b2 to 6ceed1e Compare August 28, 2020 06:41

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 28, 2020

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 28, 2020

tedteng closed this Aug 31, 2020

tedteng reopened this Aug 31, 2020

tedteng force-pushed the nonadmin_target_ssh branch from 51b6527 to 6ceed1e Compare August 31, 2020 08:12

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 31, 2020

tedteng force-pushed the nonadmin_target_ssh branch from 723b039 to 6ceed1e Compare August 31, 2020 08:31

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 31, 2020

neo-liang-sap suggested changes Aug 31, 2020

View reviewed changes

tedteng force-pushed the nonadmin_target_ssh branch from 6ceed1e to 3ee1ac1 Compare August 31, 2020 14:31

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 31, 2020

non operator able to target and ssh node through bastion host

53544d5

add temporary solution in targetShoot method add getTechnicalID Update pkg/cmd/download.go remove getShootClusterName method remove bash -c Update pkg/cmd/miscellaneous.go

tedteng force-pushed the nonadmin_target_ssh branch from 3ee1ac1 to 53544d5 Compare August 31, 2020 14:33

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 31, 2020

neo-liang-sap approved these changes Sep 1, 2020

View reviewed changes

neo-liang-sap merged commit 28a56bf into gardener-attic:master Sep 1, 2020

This was referenced Sep 1, 2020

enhance user role able to access GCP host #294

Closed

enhance user role able to access Azure host #302

Closed

tedteng deleted the nonadmin_target_ssh branch September 4, 2020 02:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

non operator able to target and ssh node through bastion host #263

non operator able to target and ssh node through bastion host #263

tedteng commented Aug 19, 2020 •

edited

Loading

tedteng commented Aug 19, 2020

petersutter Aug 21, 2020

tedteng Aug 21, 2020

petersutter Aug 26, 2020

tedteng Aug 27, 2020

tedteng commented Aug 28, 2020 •

edited

Loading

neo-liang-sap commented Aug 28, 2020

neo-liang-sap commented Aug 31, 2020

tedteng commented Aug 31, 2020

neo-liang-sap commented Aug 31, 2020 •

edited

Loading

tedteng commented Aug 31, 2020 •

edited

Loading

neo-liang-sap left a comment

neo-liang-sap left a comment

non operator able to target and ssh node through bastion host #263

non operator able to target and ssh node through bastion host #263

Conversation

tedteng commented Aug 19, 2020 • edited Loading

tedteng commented Aug 19, 2020

petersutter Aug 21, 2020

Choose a reason for hiding this comment

tedteng Aug 21, 2020

Choose a reason for hiding this comment

petersutter Aug 26, 2020

Choose a reason for hiding this comment

tedteng Aug 27, 2020

Choose a reason for hiding this comment

tedteng commented Aug 28, 2020 • edited Loading

neo-liang-sap commented Aug 28, 2020

neo-liang-sap commented Aug 31, 2020

tedteng commented Aug 31, 2020

neo-liang-sap commented Aug 31, 2020 • edited Loading

tedteng commented Aug 31, 2020 • edited Loading

neo-liang-sap left a comment

Choose a reason for hiding this comment

neo-liang-sap left a comment

Choose a reason for hiding this comment

tedteng commented Aug 19, 2020 •

edited

Loading

tedteng commented Aug 28, 2020 •

edited

Loading

neo-liang-sap commented Aug 31, 2020 •

edited

Loading

tedteng commented Aug 31, 2020 •

edited

Loading