kinvolk / lokomotive

🪦 DISCONTINUED Further Lokomotive development has been discontinued. Lokomotive is a 100% open-source, easy to use and secure Kubernetes distribution from the volks at Kinvolk
https://kinvolk.io/lokomotive-kubernetes/
Apache License 2.0
320 stars 49 forks source link

Make SSH keys DRY on all platforms #578

Open ipochi opened 4 years ago

ipochi commented 4 years ago

This issue is about how to inherit the values of duplicate fields that exist in worker pools and controllers, if not provided by the user.

Example of an AWS configuration:

variable "ssh_public_keys {}"

cluster "aws" {
  ...
  ssh_pubkeys = var.ssh_public_keys
  ...

  worker_pool "p1" {
    ssh_pubkeys = var.ssh_public_keys
  }
}

This would mean that worker_pool.ssh_pubkeys is optional and if not present in the configuration, the value is inherited from the controller configuration.

Current status in the supported non-managed platforms:

Packet: worker_pool.ssh_pubkeys field not present but there should be as part of feature and platform parity. AWS: worker_pool.ssh_pubkeys is required and explicitly duplicated by the user. Baremetal: no worker_pool concept yet.

invidian commented 4 years ago

I think it's fine to do the inheritance. I can imagine the case where different SSH keys are desired for worker nodes. It seems we can also use optional for []string in HCL, which allows passing empty array explicitly in the configuration, so SSH keys can be completely disabled for workers, if desired.

So I'd say:

  1. require ssh_pubkeys to be not-empty: len(c.SSHPubKeys) > 0
  2. Inherit controller SSH keys if worker keys are not defined in configuration: if workerPool.SSHPubKeys == nil { workerPool.SSHPubKeys = c.SSHPubKeys }.
iaguis commented 4 years ago

I agree with @invidian.

johananl commented 4 years ago

I agree with the above. To emphasize, I think the default behavior should be the simplest, most common use case (one key to rule them all :ring:) and at the same time the user should be able to specify key A for controllers, key B for worker pool 1 and key C for worker pool 2.

In terms of implementation, I'm for the "implicit approach" (worker pools use the controller keys unless specified otherwise).