From f3e68c954c551b14789c1b8251d80add244497dc Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Sun, 12 Nov 2017 16:42:43 -0500 Subject: [PATCH] Remove use of deprecated create-if-missing functions Generally tightening up the interface to make it easier to remove list operations. --- nodeup/pkg/model/context.go | 14 ++++++++-- nodeup/pkg/model/convenience.go | 12 +++++++-- nodeup/pkg/model/kube_controller_manager.go | 6 ++++- nodeup/pkg/model/protokube.go | 16 +++++++++--- nodeup/pkg/model/secrets.go | 29 ++++++++++++++++----- upup/pkg/fi/ca.go | 12 +++++---- upup/pkg/fi/clientset_castore.go | 27 ------------------- upup/pkg/fi/vfs_castore.go | 26 ------------------ 8 files changed, 69 insertions(+), 73 deletions(-) diff --git a/nodeup/pkg/model/context.go b/nodeup/pkg/model/context.go index 1ee774c087..b4f43b62d0 100644 --- a/nodeup/pkg/model/context.go +++ b/nodeup/pkg/model/context.go @@ -120,15 +120,25 @@ func (c *NodeupModelContext) buildPKIKubeconfig(id string) (string, error) { if err != nil { return "", fmt.Errorf("error fetching CA certificate from keystore: %v", err) } + if caCertificate == nil { + return "", fmt.Errorf("CA certificate %q not found", fi.CertificateId_CA) + } - certificate, err := c.KeyStore.Cert(id, false) + certificate, err := c.KeyStore.FindCert(id) if err != nil { return "", fmt.Errorf("error fetching %q certificate from keystore: %v", id, err) } - privateKey, err := c.KeyStore.PrivateKey(id, false) + if certificate == nil { + return "", fmt.Errorf("certificate %q not found", id) + } + + privateKey, err := c.KeyStore.FindPrivateKey(id) if err != nil { return "", fmt.Errorf("error fetching %q private key from keystore: %v", id, err) } + if privateKey == nil { + return "", fmt.Errorf("private key %q not found", id) + } user := kubeconfig.KubectlUser{} user.ClientCertificateData, err = certificate.AsBytes() diff --git a/nodeup/pkg/model/convenience.go b/nodeup/pkg/model/convenience.go index 4a3a0d03c4..512a146b96 100644 --- a/nodeup/pkg/model/convenience.go +++ b/nodeup/pkg/model/convenience.go @@ -96,11 +96,15 @@ func getProxyEnvVars(proxies *kops.EgressProxySpec) []v1.EnvVar { // buildCertificateRequest retrieves the certificate from a keystore func buildCertificateRequest(c *fi.ModelBuilderContext, b *NodeupModelContext, name, path string) error { - cert, err := b.KeyStore.Cert(name, false) + cert, err := b.KeyStore.FindCert(name) if err != nil { return err } + if cert == nil { + return fmt.Errorf("certificate %q not found", name) + } + serialized, err := cert.AsString() if err != nil { return err @@ -123,11 +127,15 @@ func buildCertificateRequest(c *fi.ModelBuilderContext, b *NodeupModelContext, n // buildPrivateKeyRequest retrieves a private key from the store func buildPrivateKeyRequest(c *fi.ModelBuilderContext, b *NodeupModelContext, name, path string) error { - k, err := b.KeyStore.PrivateKey(name, false) + k, err := b.KeyStore.FindPrivateKey(name) if err != nil { return err } + if k == nil { + return fmt.Errorf("private key %q not found", name) + } + serialized, err := k.AsString() if err != nil { return err diff --git a/nodeup/pkg/model/kube_controller_manager.go b/nodeup/pkg/model/kube_controller_manager.go index f01c9d76df..3950b38701 100644 --- a/nodeup/pkg/model/kube_controller_manager.go +++ b/nodeup/pkg/model/kube_controller_manager.go @@ -49,11 +49,15 @@ func (b *KubeControllerManagerBuilder) Build(c *fi.ModelBuilderContext) error { // If we're using the CertificateSigner, include the CA Key // @TODO: use a per-machine key? use KMS? if b.useCertificateSigner() { - ca, err := b.KeyStore.PrivateKey(fi.CertificateId_CA, false) + ca, err := b.KeyStore.FindPrivateKey(fi.CertificateId_CA) if err != nil { return err } + if ca == nil { + return fmt.Errorf("CA private key %q not found", fi.CertificateId_CA) + } + serialized, err := ca.AsString() if err != nil { return err diff --git a/nodeup/pkg/model/protokube.go b/nodeup/pkg/model/protokube.go index 928e163540..8c47d2ec48 100644 --- a/nodeup/pkg/model/protokube.go +++ b/nodeup/pkg/model/protokube.go @@ -70,7 +70,7 @@ func (t *ProtokubeBuilder) Build(c *fi.ModelBuilderContext) error { // retrieve the etcd peer certificates and private keys from the keystore if t.UseEtcdTLS() { for _, x := range []string{"etcd", "etcd-client"} { - if err := t.buildCeritificateTask(c, x, fmt.Sprintf("%s.pem", x)); err != nil { + if err := t.buildCertificateTask(c, x, fmt.Sprintf("%s.pem", x)); err != nil { return err } } @@ -376,12 +376,16 @@ func (t *ProtokubeBuilder) writeProxyEnvVars(buffer *bytes.Buffer) { } // buildCertificateTask is responsible for build a certificate request task -func (t *ProtokubeBuilder) buildCeritificateTask(c *fi.ModelBuilderContext, name, filename string) error { - cert, err := t.KeyStore.Cert(name, false) +func (t *ProtokubeBuilder) buildCertificateTask(c *fi.ModelBuilderContext, name, filename string) error { + cert, err := t.KeyStore.FindCert(name) if err != nil { return err } + if cert == nil { + return fmt.Errorf("certificate %q not found", name) + } + serialized, err := cert.AsString() if err != nil { return err @@ -399,11 +403,15 @@ func (t *ProtokubeBuilder) buildCeritificateTask(c *fi.ModelBuilderContext, name // buildPrivateKeyTask is responsible for build a certificate request task func (t *ProtokubeBuilder) buildPrivateTask(c *fi.ModelBuilderContext, name, filename string) error { - cert, err := t.KeyStore.PrivateKey(name, false) + cert, err := t.KeyStore.FindPrivateKey(name) if err != nil { return err } + if cert == nil { + return fmt.Errorf("private key %q not found", name) + } + serialized, err := cert.AsString() if err != nil { return err diff --git a/nodeup/pkg/model/secrets.go b/nodeup/pkg/model/secrets.go index 1677cf8709..ab636e1baa 100644 --- a/nodeup/pkg/model/secrets.go +++ b/nodeup/pkg/model/secrets.go @@ -42,10 +42,13 @@ func (b *SecretBuilder) Build(c *fi.ModelBuilderContext) error { // retrieve the platform ca { - ca, err := b.KeyStore.CertificatePool(fi.CertificateId_CA, false) + ca, err := b.KeyStore.FindCertificatePool(fi.CertificateId_CA) if err != nil { return err } + if ca == nil { + return fmt.Errorf("certificate %q not found", fi.CertificateId_CA) + } serialized, err := ca.AsString() if err != nil { @@ -81,10 +84,13 @@ func (b *SecretBuilder) Build(c *fi.ModelBuilderContext) error { } { - cert, err := b.KeyStore.Cert("master", false) + cert, err := b.KeyStore.FindCert("master") if err != nil { return err } + if cert == nil { + return fmt.Errorf("certificate %q not found", "master") + } serialized, err := cert.AsString() if err != nil { @@ -100,11 +106,13 @@ func (b *SecretBuilder) Build(c *fi.ModelBuilderContext) error { } { - k, err := b.KeyStore.PrivateKey("master", false) + k, err := b.KeyStore.FindPrivateKey("master") if err != nil { return err } - + if k == nil { + return fmt.Errorf("private key %q not found", "master") + } serialized, err := k.AsString() if err != nil { return err @@ -121,10 +129,13 @@ func (b *SecretBuilder) Build(c *fi.ModelBuilderContext) error { if b.IsKubernetesGTE("1.7") { // TODO: Remove - we use the apiserver-aggregator keypair instead (which is signed by a different CA) - cert, err := b.KeyStore.Cert("apiserver-proxy-client", false) + cert, err := b.KeyStore.FindCert("apiserver-proxy-client") if err != nil { return fmt.Errorf("apiserver proxy client cert lookup failed: %v", err.Error()) } + if cert == nil { + return fmt.Errorf("certificate %q not found", "apiserver-proxy-client") + } serialized, err := cert.AsString() if err != nil { @@ -138,10 +149,13 @@ func (b *SecretBuilder) Build(c *fi.ModelBuilderContext) error { } c.AddTask(t) - key, err := b.KeyStore.PrivateKey("apiserver-proxy-client", false) + key, err := b.KeyStore.FindPrivateKey("apiserver-proxy-client") if err != nil { return fmt.Errorf("apiserver proxy client private key lookup failed: %v", err.Error()) } + if key == nil { + return fmt.Errorf("private key %q not found", "apiserver-proxy-client") + } serialized, err = key.AsString() if err != nil { @@ -250,6 +264,9 @@ func (b *SecretBuilder) writePrivateKey(c *fi.ModelBuilderContext, id string) er if err != nil { return fmt.Errorf("private key lookup failed for %q: %v", id, err) } + if key == nil { + return fmt.Errorf("private key %q not found", id) + } serialized, err := key.AsString() if err != nil { diff --git a/upup/pkg/fi/ca.go b/upup/pkg/fi/ca.go index ff22f7cbb0..dc205fab10 100644 --- a/upup/pkg/fi/ca.go +++ b/upup/pkg/fi/ca.go @@ -63,16 +63,18 @@ type HasVFSPath interface { type CAStore interface { Keystore - // Cert returns the primary specified certificate - // For createIfMissing=false, using FindCert is preferred - Cert(name string, createIfMissing bool) (*pki.Certificate, error) // CertificatePool returns all active certificates with the specified id + // Deprecated: prefer FindCertificatePool CertificatePool(name string, createIfMissing bool) (*CertificatePool, error) - PrivateKey(name string, createIfMissing bool) (*pki.PrivateKey, error) + + // FindCertificatePool returns the named CertificatePool, or (nil,nil) if not found + FindCertificatePool(name string) (*CertificatePool, error) + + // FindPrivateKey returns the named private key, or (nil,nil) if not found + FindPrivateKey(name string) (*pki.PrivateKey, error) // FindCert returns the specified certificate, if it exists, or nil if not found FindCert(name string) (*pki.Certificate, error) - FindPrivateKey(name string) (*pki.PrivateKey, error) // ListKeysets will return all the KeySets // The key material is not guaranteed to be populated - metadata like the name will be. diff --git a/upup/pkg/fi/clientset_castore.go b/upup/pkg/fi/clientset_castore.go index 7d65587f73..63fa91c4ab 100644 --- a/upup/pkg/fi/clientset_castore.go +++ b/upup/pkg/fi/clientset_castore.go @@ -194,20 +194,6 @@ func FindPrimary(keyset *kops.Keyset) *kops.KeysetItem { return primary } -// Cert implements CAStore::Cert -func (c *ClientsetCAStore) Cert(name string, createIfMissing bool) (*pki.Certificate, error) { - cert, err := c.FindCert(name) - if err == nil && cert == nil { - if !createIfMissing { - glog.Warningf("using empty certificate, because running with DryRun") - return &pki.Certificate{}, err - } - return nil, fmt.Errorf("cannot find certificate %q", name) - } - return cert, err - -} - // CertificatePool implements CAStore::CertificatePool func (c *ClientsetCAStore) CertificatePool(id string, createIfMissing bool) (*CertificatePool, error) { cert, err := c.FindCertificatePool(id) @@ -413,19 +399,6 @@ func (c *ClientsetCAStore) FindPrivateKey(name string) (*pki.PrivateKey, error) return nil, nil } -// PrivateKey implements CAStore::PrivateKey -func (c *ClientsetCAStore) PrivateKey(name string, createIfMissing bool) (*pki.PrivateKey, error) { - key, err := c.FindPrivateKey(name) - if err == nil && key == nil { - if !createIfMissing { - glog.Warningf("using empty certificate, because running with DryRun") - return &pki.PrivateKey{}, err - } - return nil, fmt.Errorf("cannot find SSL key %q", name) - } - return key, err -} - // CreateKeypair implements CAStore::CreateKeypair func (c *ClientsetCAStore) CreateKeypair(signer string, id string, template *x509.Certificate, privateKey *pki.PrivateKey) (*pki.Certificate, error) { serial := c.buildSerial() diff --git a/upup/pkg/fi/vfs_castore.go b/upup/pkg/fi/vfs_castore.go index a4cacb62db..e503fd997d 100644 --- a/upup/pkg/fi/vfs_castore.go +++ b/upup/pkg/fi/vfs_castore.go @@ -279,19 +279,6 @@ func (c *VFSCAStore) loadOneCertificate(p vfs.Path) (*pki.Certificate, error) { return cert, nil } -func (c *VFSCAStore) Cert(id string, createIfMissing bool) (*pki.Certificate, error) { - cert, err := c.FindCert(id) - if err == nil && cert == nil { - if !createIfMissing { - glog.Warningf("using empty certificate, because running with DryRun") - return &pki.Certificate{}, err - } - return nil, fmt.Errorf("cannot find certificate %q", id) - } - return cert, err - -} - func (c *VFSCAStore) CertificatePool(id string, createIfMissing bool) (*CertificatePool, error) { cert, err := c.FindCertificatePool(id) if err == nil && cert == nil { @@ -632,19 +619,6 @@ func (c *VFSCAStore) FindPrivateKey(id string) (*pki.PrivateKey, error) { return key, nil } -func (c *VFSCAStore) PrivateKey(id string, createIfMissing bool) (*pki.PrivateKey, error) { - key, err := c.FindPrivateKey(id) - if err == nil && key == nil { - if !createIfMissing { - glog.Warningf("using empty certificate, because running with DryRun") - return &pki.PrivateKey{}, err - } - return nil, fmt.Errorf("cannot find SSL key %q", id) - } - return key, err - -} - func (c *VFSCAStore) CreateKeypair(signer string, id string, template *x509.Certificate, privateKey *pki.PrivateKey) (*pki.Certificate, error) { serial := c.buildSerial()