Provide default SSH access to machines

[akutz]

What this PR does / why we need it:
This patch introduces a new field to the ClusterProviderSpec: SSHKeyPair. This field will automatically be populated with a public and private key that may be used to access machines deployed to the cluster.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #413

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

* Provide default SSH access to machines

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akutz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [akutz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[akutz]

/assign @figo

[moshloop]

Why is this needed? Accessing a machine via SSH would be an anti-pattern in most cases?

[akutz]

Why is this needed? Accessing a machine via SSH would be an anti-pattern in most cases?

Hi @moshloop,

I disagree that this is anti-pattern. The CAPA provider uses AWS's native ability to inject SSH keys into machines to do the same. This is to enhance CAPV to support a default key pair for remote access.

In another location you asked how this differs from SSHAuthorizedKeys. I attempted to explain that in the GoDocs:

SSHKeyPair is the SSH key pair for accessing provisioned machines. This field is set at runtime and should not be assigned by an operator. Providing access to existing SSH keys is handled by SSHAuthorizedKeys. Please see SSHAuthorizedKeys for information on accessing the machines.

In other words, SSHKeyPair is always generated to guarantee there is a means to remotely access a provisioned machine. The field SSHAuthorizedKeys is for operators to provide additional public keys that should be granted access.

[akutz]

Hi @frapposelli / @ncdc ,

I'd love your opinions on this. Thanks!

[moshloop]

The CAPA provider uses AWS's native ability to inject SSH keys into machines to do the same.

Unless I am mistaken, CAPA specifies the SSH Key Name for AWS to inject, however, that Key refers to a public key only, the private key needs to be saved externally to AWS

[akutz]

Hi @moshloop,

That's correct, but the point is that it guarantees there's always SSH access if desired. This change doesn't inject a private key. It just generates a key pair to ensure there's always SSH access, even if someone neglected to add something to SSHAuthorizedKeys.

[ncdc]

I'm torn on this one. SSH access may be nice, and if you forget to set the authorized keys, this is a nice fallback. On the other hand, we want to be moving away from storing secret data in resources other than secrets (although this is adding "just one more" to a set of secrets in the spec).

How would you feel about generating a secret instead and storing a ref to it in the spec?

I'm also curious for @detiber's take since he predates me on the project.

[akutz]

Hi @ncdc,

I totally agree. I even brought this up in another conversation. The issue that I raised there is that cluserctl doesn't really support a first-class notion of a secrets.yaml or some manner to inject secret data into the bootstrap cluster. Else I would do this that way. That would also require secrets created pre-pivot be automatically moved to the initial target/management cluster a la kubernetes-sigs/cluster-api#1112.

How would you feel about generating a secret instead and storing a ref to it in the spec?

Again, I'd love to do this. I wouldn't even use a ref, just a well-known name (much like the kubeconfig secret). The issue is how that gets injected pre-pivot as well as how it is transferred as part of the pivot.

[figo]

looks good to me, a nit below

[moshloop]

@randomvariable do you have any comments on this from a CIS perspective?

[randomvariable]

The CIS benchmark for Kubernetes doesn't have anything to say on SSH access as it's a not related to Kubernetes itself.

FWIW, if we haven't already for CAPA, we were definitely going to make SSH access optional in case people wanted to use other things like SSH certificates, LDAP or AWS SSM Session Manager.

[frapposelli]

Should the generation of the key pair (if not provided) be delegated to clusterctl?

[ncdc]

clusterctl is somewhat "frozen" pending the outcome of kubernetes-sigs/cluster-api#1065 and kubernetes-sigs/cluster-api#1085

[frapposelli]

@ncdc clusterctl or whatever tool triggers the creation of a cluster, in other words, not letting the provider create those keys.

[k8s-ci-robot]

@akutz: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-cluster-api-provider-vsphere-e2e	97796d4	link	/test pull-cluster-api-provider-vsphere-e2e
pull-cluster-api-provider-vsphere-verify-lint	97796d4	link	/test pull-cluster-api-provider-vsphere-verify-lint
pull-cluster-api-provider-vsphere-verify-crds	97796d4	link	/test pull-cluster-api-provider-vsphere-verify-crds
pull-cluster-api-provider-vsphere-test	97796d4	link	/test pull-cluster-api-provider-vsphere-test
pull-cluster-api-provider-vsphere-verify-vet	97796d4	link	/test pull-cluster-api-provider-vsphere-verify-vet
pull-cluster-api-provider-vsphere-verify-fmt	97796d4	link	/test pull-cluster-api-provider-vsphere-verify-fmt

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[k8s-ci-robot]

@akutz: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[akutz]

/close

[k8s-ci-robot]

@akutz: Closed this PR.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.