Merge pull request #231 from justinsb/revert_ssh_key

Revert SSH public key fix

cmd/kops/create_cluster.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"github.com/golang/glog"
 	"github.com/spf13/cobra"
-	"io/ioutil"
 	"k8s.io/kops/upup/pkg/api"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup"
@@ -113,17 +112,6 @@ func (c *CreateClusterCmd) Run(args []string) error {
 		c.OutDir = "out"
 	}
 
-	var sshPublicKeyData []byte
-	if c.SSHPublicKey != "" {
-		sshPublicKey := c.SSHPublicKey
-		sshPublicKey = utils.ExpandPath(sshPublicKey)
-		authorized, err := ioutil.ReadFile(sshPublicKey)
-		if err != nil {
-			return fmt.Errorf("error reading SSH key file %q: %v", sshPublicKey, err)
-		}
-		sshPublicKeyData = authorized
-	}
-
 	clusterRegistry, err := rootCommand.ClusterRegistry()
 	if err != nil {
 		return err
@@ -309,6 +297,10 @@ func (c *CreateClusterCmd) Run(args []string) error {
 		}
 	}
 
+	if c.SSHPublicKey != "" {
+		c.SSHPublicKey = utils.ExpandPath(c.SSHPublicKey)
+	}
+
 	if c.AdminAccess != "" {
 		cluster.Spec.AdminAccess = []string{c.AdminAccess}
 	}
@@ -353,14 +345,6 @@ func (c *CreateClusterCmd) Run(args []string) error {
 		return fmt.Errorf("error writing updated configuration: %v", err)
 	}
 
-	if sshPublicKeyData != nil {
-		keystore := clusterRegistry.KeyStore(clusterName)
-		err := keystore.AddSSHPublicKey("admin", sshPublicKeyData)
-		if err != nil {
-			return fmt.Errorf("error storing SSH public key: %v", err)
-		}
-	}
-
 	err = clusterRegistry.WriteCompletedConfig(fullCluster)
 	if err != nil {
 		return fmt.Errorf("error writing completed cluster spec: %v", err)
@@ -376,6 +360,7 @@ func (c *CreateClusterCmd) Run(args []string) error {
 		Models:          strings.Split(c.Models, ","),
 		ClusterRegistry: clusterRegistry,
 		Target:          c.Target,
+		SSHPublicKey:    c.SSHPublicKey,
 		OutDir:          c.OutDir,
 		DryRun:          isDryrun,
 	}

cmd/kops/get_secrets.go

@@ -1,85 +0,0 @@
-package main
-
-import (
-	"fmt"
-	"os"
-
-	"github.com/golang/glog"
-	"github.com/spf13/cobra"
-	"k8s.io/kops/upup/pkg/fi"
-)
-
-type GetSecretsCommand struct {
-}
-
-var getSecretsCommand GetSecretsCommand
-
-func init() {
-	cmd := &cobra.Command{
-		Use:     "secrets",
-		Aliases: []string{"secret"},
-		Short:   "get secrets",
-		Long:    `List or get secrets.`,
-		Run: func(cmd *cobra.Command, args []string) {
-			err := getSecretsCommand.Run()
-			if err != nil {
-				glog.Exitf("%v", err)
-			}
-		},
-	}
-
-	getCmd.AddCommand(cmd)
-}
-
-func (c *GetSecretsCommand) Run() error {
-	var items []*fi.KeystoreItem
-	{
-		caStore, err := rootCommand.KeyStore()
-		if err != nil {
-			return err
-		}
-		l, err := caStore.List()
-		if err != nil {
-			return fmt.Errorf("error listing CA store items %v", err)
-		}
-
-		for _, i := range l {
-			items = append(items, i)
-		}
-	}
-
-	{
-		secretStore, err := rootCommand.SecretStore()
-		if err != nil {
-			return err
-		}
-		l, err := secretStore.ListSecrets()
-		if err != nil {
-			return fmt.Errorf("error listing secrets %v", err)
-		}
-
-		for _, id := range l {
-			info := &fi.KeystoreItem{
-				Name: id,
-				Type: fi.SecretTypeSecret,
-			}
-			items = append(items, info)
-		}
-	}
-
-	if len(items) == 0 {
-		return nil
-	}
-
-	t := &Table{}
-	t.AddColumn("NAME", func(i *fi.KeystoreItem) string {
-		return i.Name
-	})
-	t.AddColumn("ID", func(i *fi.KeystoreItem) string {
-		return i.Id
-	})
-	t.AddColumn("TYPE", func(i *fi.KeystoreItem) string {
-		return i.Type
-	})
-	return t.Render(items, os.Stdout, "TYPE", "NAME", "ID")
-}

cmd/kops/secrets_create.go

@@ -6,7 +6,6 @@ import (
 	"crypto/x509"
 	"github.com/golang/glog"
 	"github.com/spf13/cobra"
-	"k8s.io/kops/upup/pkg/fi"
 	"net"
 	"strings"
 )
@@ -63,13 +62,9 @@ func (cmd *CreateSecretsCommand) Run() error {
 			if err != nil {
 				return err
 			}
-			secret, err := fi.CreateSecret()
+			_, created, err := secretStore.GetOrCreateSecret(cmd.Id)
 			if err != nil {
-				return fmt.Errorf("error creating secret: %v", err)
+				return fmt.Errorf("error creating secrets %v", err)
-			}
-			_, created, err := secretStore.GetOrCreateSecret(cmd.Id, secret)
-			if err != nil {
-				return fmt.Errorf("error creating secret: %v", err)
 			}
 			if !created {
 				return fmt.Errorf("secret already exists")

cmd/kops/secrets_describe.go

@@ -48,18 +48,12 @@ func (c *DescribeSecretsCommand) Run() error {
 		if err != nil {
 			return err
 		}
-		items, err := caStore.List()
+		ids, err := caStore.List()
 		if err != nil {
 			return fmt.Errorf("error listing CA store items %v", err)
 		}
 
-		for _, item := range items {
+		for _, id := range ids {
-			if item.Type != fi.SecretTypeKeypair {
-				continue
-			}
-
-			id := item.Name
-
 			cert, err := caStore.FindCert(id)
 			if err != nil {
 				return fmt.Errorf("error retrieving cert %q: %v", id, err)

cmd/kops/secrets_get.go

@@ -1,10 +1,20 @@
 package main
 
 import (
+	"fmt"
+
+	"bytes"
 	"github.com/golang/glog"
 	"github.com/spf13/cobra"
+	"os"
+	"text/tabwriter"
 )
 
+type GetSecretsCommand struct {
+}
+
+var getSecretsCommand GetSecretsCommand
+
 func init() {
 	cmd := &cobra.Command{
 		Use:   "get",
@@ -20,3 +30,73 @@ func init() {
 
 	secretsCmd.AddCommand(cmd)
 }
+
+type SecretInfo struct {
+	Id   string
+	Type string
+}
+
+func (c *GetSecretsCommand) Run() error {
+	var infos []*SecretInfo
+	{
+		caStore, err := rootCommand.KeyStore()
+		if err != nil {
+			return err
+		}
+		ids, err := caStore.List()
+		if err != nil {
+			return fmt.Errorf("error listing CA store items %v", err)
+		}
+
+		for _, id := range ids {
+			info := &SecretInfo{
+				Id:   id,
+				Type: "keypair",
+			}
+			infos = append(infos, info)
+		}
+	}
+
+	{
+		secretStore, err := rootCommand.SecretStore()
+		if err != nil {
+			return err
+		}
+		ids, err := secretStore.ListSecrets()
+		if err != nil {
+			return fmt.Errorf("error listing secrets %v", err)
+		}
+
+		for _, id := range ids {
+			info := &SecretInfo{
+				Id:   id,
+				Type: "secret",
+			}
+			infos = append(infos, info)
+		}
+	}
+
+	var b bytes.Buffer
+	w := new(tabwriter.Writer)
+
+	// Format in tab-separated columns with a tab stop of 8.
+	w.Init(os.Stdout, 0, 8, 0, '\t', tabwriter.StripEscape)
+	for _, info := range infos {
+		b.WriteByte(tabwriter.Escape)
+		b.WriteString(info.Type)
+		b.WriteByte(tabwriter.Escape)
+		b.WriteByte('\t')
+		b.WriteByte(tabwriter.Escape)
+		b.WriteString(info.Id)
+		b.WriteByte(tabwriter.Escape)
+		b.WriteByte('\n')
+
+		_, err := w.Write(b.Bytes())
+		if err != nil {
+			return fmt.Errorf("error writing to output: %v", err)
+		}
+		b.Reset()
+	}
+	w.Flush()
+	return nil
+}

cmd/kops/update_cluster.go

@@ -6,6 +6,7 @@ import (
 	"github.com/spf13/cobra"
 	"k8s.io/kops/upup/pkg/api"
 	"k8s.io/kops/upup/pkg/fi/cloudup"
+	"k8s.io/kops/upup/pkg/fi/utils"
 	"k8s.io/kops/upup/pkg/kutil"
 	"strings"
 )
@@ -15,6 +16,7 @@ type UpdateClusterCmd struct {
 	Target       string
 	Models       string
 	OutDir       string
+	SSHPublicKey string
 }
 
 var updateCluster UpdateClusterCmd
@@ -37,6 +39,7 @@ func init() {
 	cmd.Flags().BoolVar(&updateCluster.Yes, "yes", false, "Actually create cloud resources")
 	cmd.Flags().StringVar(&updateCluster.Target, "target", "direct", "Target - direct, terraform")
 	cmd.Flags().StringVar(&updateCluster.Models, "model", "config,proto,cloudup", "Models to apply (separate multiple models with commas)")
+	cmd.Flags().StringVar(&updateCluster.SSHPublicKey, "ssh-public-key", "~/.ssh/id_rsa.pub", "SSH public key to use")
 	cmd.Flags().StringVar(&updateCluster.OutDir, "out", "", "Path to write any local output")
 }
 
@@ -83,6 +86,10 @@ func (c *UpdateClusterCmd) Run(args []string) error {
 		return err
 	}
 
+	if c.SSHPublicKey != "" {
+		c.SSHPublicKey = utils.ExpandPath(c.SSHPublicKey)
+	}
+
 	strict := false
 	err = api.DeepValidate(cluster, fullInstanceGroups, strict)
 	if err != nil {
@@ -95,6 +102,7 @@ func (c *UpdateClusterCmd) Run(args []string) error {
 		Models:          strings.Split(c.Models, ","),
 		ClusterRegistry: clusterRegistry,
 		Target:          c.Target,
+		SSHPublicKey:    c.SSHPublicKey,
 		OutDir:          c.OutDir,
 		DryRun:          isDryrun,
 	}

docs/arguments.md

@@ -1,13 +1,5 @@
 # Detailed description of arguments
 
-# create
-
-## ssh-public-key
-
-`--ssh-public-key` is the path to the SSH public key to set up on the cluster.  (The SSH username is admin)
-
-This will automatically create an SSH public-key in the keystore (`kubectl get secret`) with a name of `admin`.
-
 ## admin-access
 
 `admin-access` controls the CIDR which can access the admin endpoints (SSH to each node, HTTPS to the master).

upup/pkg/fi/ca.go

@@ -28,19 +28,6 @@ type Certificate struct {
 	PublicKey   crypto.PublicKey
 }
 
-const (
-	SecretTypeSSHPublicKey = "SSHPublicKey"
-	SecretTypeKeypair      = "Keypair"
-	SecretTypeSecret       = "Secret"
-)
-
-type KeystoreItem struct {
-	Type string
-	Name string
-	Id   string
-	Data []byte
-}
-
 func (c *Certificate) UnmarshalJSON(b []byte) error {
 	s := ""
 	if err := json.Unmarshal(b, &s); err == nil {
@@ -80,33 +67,26 @@ func (c *Certificate) MarshalJSON() ([]byte, error) {
 
 type CAStore interface {
 	// Cert returns the primary specified certificate
-	Cert(name string) (*Certificate, error)
+	Cert(id string) (*Certificate, error)
 	// CertificatePool returns all active certificates with the specified id
-	CertificatePool(name string) (*CertificatePool, error)
+	CertificatePool(id string) (*CertificatePool, error)
-	PrivateKey(name string) (*PrivateKey, error)
+	PrivateKey(id string) (*PrivateKey, error)
 
-	FindCert(name string) (*Certificate, error)
+	FindCert(id string) (*Certificate, error)
-	FindPrivateKey(name string) (*PrivateKey, error)
+	FindPrivateKey(id string) (*PrivateKey, error)
 
 	//IssueCert(id string, privateKey *PrivateKey, template *x509.Certificate) (*Certificate, error)
 	//CreatePrivateKey(id string) (*PrivateKey, error)
 
-	CreateKeypair(name string, template *x509.Certificate) (*Certificate, *PrivateKey, error)
+	CreateKeypair(id string, template *x509.Certificate) (*Certificate, *PrivateKey, error)
 
-	// List will list all the items, but will not fetch the data
+	List() ([]string, error)
-	List() ([]*KeystoreItem, error)
 
 	// VFSPath returns the path where the CAStore is stored
 	VFSPath() vfs.Path
 
 	// AddCert adds an alternative certificate to the pool (primarily useful for CAs)
-	AddCert(name string, cert *Certificate) error
+	AddCert(id string, cert *Certificate) error
-
-	// AddSSHPublicKey adds an SSH public key
-	AddSSHPublicKey(name string, data []byte) error
-
-	// FindSSHPublicKeys retrieves the SSH public keys with the specific name
-	FindSSHPublicKeys(name string) ([]*KeystoreItem, error)
 }
 
 func (c *Certificate) AsString() (string, error) {

upup/pkg/fi/cloudup/apply_cluster.go

@@ -3,6 +3,7 @@ package cloudup
 import (
 	"fmt"
 	"github.com/golang/glog"
+	"io/ioutil"
 	"k8s.io/kops/upup/pkg/api"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awstasks"
@@ -31,7 +32,8 @@ type ApplyClusterCmd struct {
 	Target string
 	//// The node model to use
 	//NodeModel string
-
+	// The SSH public key (file) to use
+	SSHPublicKey string
 	// OutDir is a local directory in which we place output, can cache files etc
 	OutDir string
 
@@ -235,23 +237,11 @@ func (c *ApplyClusterCmd) Run() error {
 				"dnsZone": &awstasks.DNSZone{},
 			})
 
+			if c.SSHPublicKey == "" {
+				return fmt.Errorf("SSH public key must be specified when running with AWS")
+			}
+
 			l.TemplateFunctions["MachineTypeInfo"] = awsup.GetMachineTypeInfo
-
-			{
-				sshPublicKeys, err := keyStore.FindSSHPublicKeys("admin")
-				if err != nil {
-					return fmt.Errorf("error reading SSH public key %q: %v", "admin", err)
-				}
-				if len(sshPublicKeys) == 0 {
-					return fmt.Errorf("Must specify SSH public key when running with AWS")
-				}
-				if len(sshPublicKeys) != 1 {
-					glog.Warningf("Found multiple SSH public keys - choosing arbitrarily")
-				}
-				sshPublicKey := sshPublicKeys[0]
-				l.Resources["ssh-public-key"] = fi.NewStringResource(string(sshPublicKey.Data))
-			}
-
 		}
 
 	default:
@@ -358,6 +348,15 @@ func (c *ApplyClusterCmd) Run() error {
 
 	tf.AddTo(l.TemplateFunctions)
 
+	if c.SSHPublicKey != "" {
+		authorized, err := ioutil.ReadFile(c.SSHPublicKey)
+		if err != nil {
+			return fmt.Errorf("error reading SSH key file %q: %v", c.SSHPublicKey, err)
+		}
+
+		l.Resources["ssh-public-key"] = fi.NewStringResource(string(authorized))
+	}
+
 	taskMap, err := l.BuildTasks(modelStore, c.Models)
 	if err != nil {
 		return fmt.Errorf("error building tasks: %v", err)

upup/pkg/fi/fitasks/secret.go

@@ -61,12 +61,7 @@ func (_ *Secret) Render(c *fi.Context, a, e, changes *Secret) error {
 
 	secrets := c.SecretStore
 
-	secret, err := fi.CreateSecret()
+	_, _, err := secrets.GetOrCreateSecret(name)
-	if err != nil {
-		return fmt.Errorf("error creating secret %q: %v", name, err)
-	}
-
-	_, _, err = secrets.GetOrCreateSecret(name, secret)
 	if err != nil {
 		return fmt.Errorf("error creating secret %q: %v", name, err)
 	}

upup/pkg/fi/secrets.go

@@ -14,7 +14,7 @@ type SecretStore interface {
 	// Find a secret, if exists.  Returns nil,nil if not found
 	FindSecret(id string) (*Secret, error)
 	// Create or replace a secret
-	GetOrCreateSecret(id string, secret *Secret) (current *Secret, created bool, err error)
+	GetOrCreateSecret(id string) (secret *Secret, created bool, err error)
 	// Lists the ids of all known secrets
 	ListSecrets() ([]string, error)
 

upup/pkg/fi/vfs_castore.go

@@ -2,14 +2,12 @@ package fi
 
 import (
 	"bytes"
-	"crypto/md5"
 	crypto_rand "crypto/rand"
 	"crypto/rsa"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"fmt"
 	"github.com/golang/glog"
-	"golang.org/x/crypto/ssh"
 	"k8s.io/kops/upup/pkg/fi/vfs"
 	"math/big"
 	"os"
@@ -318,66 +316,20 @@ func (c *VFSCAStore) FindCertificatePool(id string) (*CertificatePool, error) {
 	return pool, nil
 }
 
-func (c *VFSCAStore) List() ([]*KeystoreItem, error) {
+func (c *VFSCAStore) List() ([]string, error) {
-	var items []*KeystoreItem
+	var ids []string
 
-	{
+	issuedDir := c.basedir.Join("issued")
-		baseDir := c.basedir.Join("issued")
+	files, err := issuedDir.ReadDir()
-		files, err := baseDir.ReadTree()
 	if err != nil {
-			return nil, fmt.Errorf("error reading directory %q: %v", baseDir, err)
+		return nil, fmt.Errorf("error reading directory %q: %v", issuedDir, err)
 	}
 
 	for _, f := range files {
-			relativePath, err := vfs.RelativePath(baseDir, f)
+		name := f.Base()
-			if err != nil {
+		ids = append(ids, name)
-				return nil, err
 	}
-
+	return ids, nil
-			tokens := strings.Split(relativePath, "/")
-			if len(tokens) != 2 {
-				glog.V(2).Infof("ignoring unexpected file in keystore: %q", f)
-				continue
-			}
-
-			item := &KeystoreItem{
-				Name: tokens[0],
-				Id:   strings.TrimSuffix(tokens[1], ".crt"),
-				Type: SecretTypeKeypair,
-			}
-			items = append(items, item)
-		}
-	}
-
-	{
-		baseDir := c.basedir.Join("ssh", "public")
-		files, err := baseDir.ReadTree()
-		if err != nil {
-			return nil, fmt.Errorf("error reading directory %q: %v", baseDir, err)
-		}
-
-		for _, f := range files {
-			relativePath, err := vfs.RelativePath(baseDir, f)
-			if err != nil {
-				return nil, err
-			}
-
-			tokens := strings.Split(relativePath, "/")
-			if len(tokens) != 2 {
-				glog.V(2).Infof("ignoring unexpected file in keystore: %q", f)
-				continue
-			}
-
-			item := &KeystoreItem{
-				Name: tokens[0],
-				Id:   tokens[1],
-				Type: SecretTypeSSHPublicKey,
-			}
-			items = append(items, item)
-		}
-	}
-
-	return items, nil
 }
 
 func (c *VFSCAStore) IssueCert(id string, serial *big.Int, privateKey *PrivateKey, template *x509.Certificate) (*Certificate, error) {
@@ -620,93 +572,3 @@ func buildSerial(timestamp int64) *big.Int {
 
 	return serial
 }
-
-// AddSSHPublicKey stores an SSH public key
-func (c *VFSCAStore) AddSSHPublicKey(name string, pubkey []byte) error {
-	var id string
-	{
-		sshPublicKey, _, _, _, err := ssh.ParseAuthorizedKey(pubkey)
-		if err != nil {
-			return fmt.Errorf("error parsing public key: %v", err)
-		}
-
-		// compute fingerprint to serve as id
-		h := md5.New()
-		_, err = h.Write(sshPublicKey.Marshal())
-		if err != nil {
-			return err
-		}
-		id = fmt.Sprintf("%x", h.Sum(nil))
-	}
-
-	p := c.buildSSHPublicKeyPath(name, id)
-	return c.storeData(pubkey, p)
-}
-
-func (c *VFSCAStore) buildSSHPublicKeyPath(name string, id string) vfs.Path {
-	return c.basedir.Join("ssh", "public", name, id)
-}
-
-func (c *VFSCAStore) storeData(data []byte, p vfs.Path) error {
-	return p.WriteFile(data)
-}
-
-func (c *VFSCAStore) FindSSHPublicKeys(name string) ([]*KeystoreItem, error) {
-	p := c.basedir.Join("ssh", "public", name)
-
-	items, err := c.loadPath(p)
-	if err != nil {
-		return nil, err
-	}
-	for _, item := range items {
-		item.Type = SecretTypeSSHPublicKey
-		item.Name = name
-	}
-	return items, nil
-}
-
-func (c *VFSCAStore) loadPath(p vfs.Path) ([]*KeystoreItem, error) {
-	files, err := p.ReadDir()
-	if err != nil {
-		if os.IsNotExist(err) {
-			return nil, nil
-		}
-		return nil, err
-	}
-
-	var keystoreItems []*KeystoreItem
-
-	for _, f := range files {
-		data, err := f.ReadFile()
-		if err != nil {
-			if os.IsNotExist(err) {
-				glog.V(2).Infof("Ignoring not-found issue reading %q", f)
-				continue
-			}
-			return nil, fmt.Errorf("error loading keystore item %q: %v", f, err)
-		}
-		name := f.Base()
-		keystoreItem := &KeystoreItem{
-			Id:   name,
-			Data: data,
-		}
-		keystoreItems = append(keystoreItems, keystoreItem)
-	}
-
-	return keystoreItems, nil
-}
-
-func (c *VFSCAStore) loadData(p vfs.Path) (*PrivateKey, error) {
-	data, err := p.ReadFile()
-	if err != nil {
-		if os.IsNotExist(err) {
-			return nil, nil
-		}
-		return nil, err
-	}
-	k, err := ParsePEMPrivateKey(data)
-	if err != nil {
-		return nil, fmt.Errorf("error parsing private key from %q: %v", p, err)
-	}
-	return k, err
-}

upup/pkg/fi/vfs_secretstore.go

@@ -62,7 +62,7 @@ func (c *VFSSecretStore) Secret(id string) (*Secret, error) {
 	return s, nil
 }
 
-func (c *VFSSecretStore) GetOrCreateSecret(id string, secret *Secret) (*Secret, bool, error) {
+func (c *VFSSecretStore) GetOrCreateSecret(id string) (*Secret, bool, error) {
 	p := c.buildSecretPath(id)
 
 	for i := 0; i < 2; i++ {
@@ -75,7 +75,12 @@ func (c *VFSSecretStore) GetOrCreateSecret(id string, secret *Secret) (*Secret,
 			return s, false, nil
 		}
 
-		err = c.createSecret(secret, p)
+		s, err = CreateSecret()
+		if err != nil {
+			return nil, false, err
+		}
+
+		err = c.createSecret(s, p)
 		if err != nil {
 			if os.IsExist(err) && i == 0 {
 				glog.Infof("Got already-exists error when writing secret; likely due to concurrent creation.  Will retry")