From 8b9145f6c4b4139df44e4fb8ccb9a858c87b6c7c Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Thu, 21 May 2020 22:37:01 -0700 Subject: [PATCH] Create nodetasks.IssueCert() --- nodeup/pkg/model/etcd_manager_tls.go | 43 +----- .../pkg/model/kube_apiserver_healthcheck.go | 58 +------- nodeup/pkg/model/networking/BUILD.bazel | 1 - nodeup/pkg/model/networking/cilium.go | 38 +---- upup/pkg/fi/nodeup/command.go | 8 +- upup/pkg/fi/nodeup/nodetasks/BUILD.bazel | 8 +- upup/pkg/fi/nodeup/nodetasks/file.go | 4 + upup/pkg/fi/nodeup/nodetasks/issue_cert.go | 140 ++++++++++++++++++ .../fi/nodeup/nodetasks/issue_cert_test.go | 53 +++++++ upup/pkg/fi/nodeup/nodetasks/service.go | 4 +- upup/pkg/fi/resources.go | 19 +++ 11 files changed, 247 insertions(+), 129 deletions(-) create mode 100644 upup/pkg/fi/nodeup/nodetasks/issue_cert.go create mode 100644 upup/pkg/fi/nodeup/nodetasks/issue_cert_test.go diff --git a/nodeup/pkg/model/etcd_manager_tls.go b/nodeup/pkg/model/etcd_manager_tls.go index 20ca353f5b..694e79c5ff 100644 --- a/nodeup/pkg/model/etcd_manager_tls.go +++ b/nodeup/pkg/model/etcd_manager_tls.go @@ -18,13 +18,10 @@ package model import ( "crypto/x509/pkix" - "fmt" - "os" - "path/filepath" "k8s.io/klog" - "k8s.io/kops/pkg/pki" "k8s.io/kops/upup/pkg/fi" + "k8s.io/kops/upup/pkg/fi/nodeup/nodetasks" ) // EtcdManagerTLSBuilder configures TLS support for etcd-manager @@ -69,48 +66,24 @@ func (b *EtcdManagerTLSBuilder) Build(ctx *fi.ModelBuilderContext) error { } // We also dynamically generate the client keypair for apiserver - if err := b.buildKubeAPIServerKeypair(); err != nil { + if err := b.buildKubeAPIServerKeypair(ctx); err != nil { return err } return nil } -func (b *EtcdManagerTLSBuilder) buildKubeAPIServerKeypair() error { - req := &pki.IssueCertRequest{ +func (b *EtcdManagerTLSBuilder) buildKubeAPIServerKeypair(c *fi.ModelBuilderContext) error { + name := "etcd-client" + issueCert := &nodetasks.IssueCert{ + Name: name, Signer: "etcd-clients-ca", Type: "client", Subject: pkix.Name{ CommonName: "kube-apiserver", }, - MinValidDays: 455, - } - dir := "/etc/kubernetes/pki/kube-apiserver" - name := "etcd-client" - humanName := dir + "/" + name - klog.Infof("signing certificate for %q", humanName) - cert, privateKey, etcdClientsCACertificate, err := pki.IssueCert(req, b.KeyStore) - if err != nil { - return fmt.Errorf("error signing certificate for %q: %v", humanName, err) - } - - if err := os.MkdirAll(dir, 0755); err != nil { - return fmt.Errorf("error creating directories %q: %v", dir, err) - } - - { - p := filepath.Join(dir, "etcd-ca.crt") - if err := etcdClientsCACertificate.WriteToFile(p, 0644); err != nil { - return fmt.Errorf("error writing certificate key file %q: %v", p, err) - } - } - - p := filepath.Join(dir, name) - if err := cert.WriteToFile(p+".crt", 0644); err != nil { - return fmt.Errorf("error writing certificate key file %q: %v", p+".crt", err) - } - if err := privateKey.WriteToFile(p+".key", 0600); err != nil { - return fmt.Errorf("error writing private key file %q: %v", p+".key", err) } + c.AddTask(issueCert) + issueCert.AddFileTasks(c, "/etc/kubernetes/pki/kube-apiserver", name, "etcd-ca", nil) return nil } diff --git a/nodeup/pkg/model/kube_apiserver_healthcheck.go b/nodeup/pkg/model/kube_apiserver_healthcheck.go index 6fef6c1a8d..557a1ecdc5 100644 --- a/nodeup/pkg/model/kube_apiserver_healthcheck.go +++ b/nodeup/pkg/model/kube_apiserver_healthcheck.go @@ -19,12 +19,9 @@ package model import ( "crypto/x509/pkix" "fmt" - "path/filepath" corev1 "k8s.io/api/core/v1" - "k8s.io/klog" "k8s.io/kops/pkg/apis/nodeup" - "k8s.io/kops/pkg/pki" "k8s.io/kops/pkg/wellknownusers" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/nodeup/nodetasks" @@ -85,63 +82,16 @@ func (b *KubeAPIServerBuilder) addHealthcheckSidecarTasks(c *fi.ModelBuilderCont }) } - req := &pki.IssueCertRequest{ + issueCert := &nodetasks.IssueCert{ + Name: id, Signer: fi.CertificateId_CA, Type: "client", Subject: pkix.Name{ CommonName: id, }, - MinValidDays: 455, } - - klog.Infof("signing certificate for %q", id) - clientCert, clientKey, _, err := pki.IssueCert(req, b.KeyStore) - if err != nil { - return err - } - - c.AddTask(&nodetasks.File{ - Path: filepath.Join(secretsDir), - Type: nodetasks.FileType_Directory, - Mode: s("0755"), - }) - - clientCertBytes, err := clientCert.AsBytes() - if err != nil { - return err - } - c.AddTask(&nodetasks.File{ - Path: filepath.Join(secretsDir, "client.crt"), - Contents: fi.NewBytesResource(clientCertBytes), - Type: nodetasks.FileType_File, - Mode: s("0644"), - Owner: s(userName), - }) - - clientKeyBytes, err := clientKey.AsBytes() - if err != nil { - return err - } - c.AddTask(&nodetasks.File{ - Path: filepath.Join(secretsDir, "client.key"), - Contents: fi.NewBytesResource(clientKeyBytes), - Type: nodetasks.FileType_File, - Mode: s("0600"), - Owner: s(userName), - }) - - cert, err := b.GetCert(fi.CertificateId_CA) - if err != nil { - return err - } - - c.AddTask(&nodetasks.File{ - Path: filepath.Join(secretsDir, "ca.crt"), - Contents: fi.NewBytesResource(cert), - Type: nodetasks.FileType_File, - Mode: s("0644"), - Owner: s(userName), - }) + c.AddTask(issueCert) + issueCert.AddFileTasks(c, secretsDir, "client", "ca", s(userName)) return nil } diff --git a/nodeup/pkg/model/networking/BUILD.bazel b/nodeup/pkg/model/networking/BUILD.bazel index f0fa178506..b0cf17063a 100644 --- a/nodeup/pkg/model/networking/BUILD.bazel +++ b/nodeup/pkg/model/networking/BUILD.bazel @@ -16,7 +16,6 @@ go_library( "//nodeup/pkg/model:go_default_library", "//pkg/apis/kops:go_default_library", "//pkg/model/components:go_default_library", - "//pkg/pki:go_default_library", "//pkg/systemd:go_default_library", "//upup/pkg/fi:go_default_library", "//upup/pkg/fi/nodeup/nodetasks:go_default_library", diff --git a/nodeup/pkg/model/networking/cilium.go b/nodeup/pkg/model/networking/cilium.go index d41cc1b7ad..6853f401be 100644 --- a/nodeup/pkg/model/networking/cilium.go +++ b/nodeup/pkg/model/networking/cilium.go @@ -19,14 +19,10 @@ package networking import ( "crypto/x509/pkix" "fmt" - "os" - "path/filepath" "k8s.io/kops/nodeup/pkg/model" "golang.org/x/sys/unix" - "k8s.io/klog" - "k8s.io/kops/pkg/pki" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/nodeup/nodetasks" ) @@ -127,41 +123,17 @@ func (b *CiliumBuilder) buildCiliumEtcdSecrets(c *fi.ModelBuilderContext) error } } - req := &pki.IssueCertRequest{ + name := "etcd-client-cilium" + issueCert := &nodetasks.IssueCert{ + Name: name, Signer: "etcd-clients-ca-cilium", Type: "client", Subject: pkix.Name{ CommonName: "cilium", }, - MinValidDays: 455, - } - dir := "/etc/kubernetes/pki/cilium" - name := "etcd-client-cilium" - humanName := dir + "/" + name - klog.Infof("signing certificate for %q", humanName) - cert, privateKey, etcdClientsCACertificate, err := pki.IssueCert(req, b.KeyStore) - if err != nil { - return fmt.Errorf("error signing certificate for %q: %v", humanName, err) - } - - if err := os.MkdirAll(dir, 0755); err != nil { - return fmt.Errorf("error creating directories %q: %v", dir, err) - } - - { - p := filepath.Join(dir, "etcd-ca.crt") - if err := etcdClientsCACertificate.WriteToFile(p, 0644); err != nil { - return fmt.Errorf("error writing certificate key file %q: %v", p, err) - } - } - - p := filepath.Join(dir, name) - if err := cert.WriteToFile(p+".crt", 0644); err != nil { - return fmt.Errorf("error writing certificate key file %q: %v", p+".crt", err) - } - if err := privateKey.WriteToFile(p+".key", 0600); err != nil { - return fmt.Errorf("error writing private key file %q: %v", p+".key", err) } + c.AddTask(issueCert) + issueCert.AddFileTasks(c, "/etc/kubernetes/pki/cilium", name, "etcd-ca", nil) return nil } diff --git a/upup/pkg/fi/nodeup/command.go b/upup/pkg/fi/nodeup/command.go index 2db765e46f..4a25fefb80 100644 --- a/upup/pkg/fi/nodeup/command.go +++ b/upup/pkg/fi/nodeup/command.go @@ -199,6 +199,8 @@ func (c *NodeUpCommand) Run(out io.Writer) error { NodeupConfig: c.config, } + var secretStore fi.SecretStore + var keyStore fi.Keystore if c.cluster.Spec.SecretStore != "" { klog.Infof("Building SecretStore at %q", c.cluster.Spec.SecretStore) p, err := vfs.Context.BuildVfsPath(c.cluster.Spec.SecretStore) @@ -206,7 +208,8 @@ func (c *NodeUpCommand) Run(out io.Writer) error { return fmt.Errorf("error building secret store path: %v", err) } - modelContext.SecretStore = secrets.NewVFSSecretStore(c.cluster, p) + secretStore = secrets.NewVFSSecretStore(c.cluster, p) + modelContext.SecretStore = secretStore } else { return fmt.Errorf("SecretStore not set") } @@ -219,6 +222,7 @@ func (c *NodeUpCommand) Run(out io.Writer) error { } modelContext.KeyStore = fi.NewVFSCAStore(c.cluster, p) + keyStore = modelContext.KeyStore } else { return fmt.Errorf("KeyStore not set") } @@ -285,8 +289,6 @@ func (c *NodeUpCommand) Run(out io.Writer) error { // Protokube load image task is in ProtokubeBuilder var cloud fi.Cloud - var keyStore fi.Keystore - var secretStore fi.SecretStore var target fi.Target checkExisting := true diff --git a/upup/pkg/fi/nodeup/nodetasks/BUILD.bazel b/upup/pkg/fi/nodeup/nodetasks/BUILD.bazel index de77260dfc..c461db20db 100644 --- a/upup/pkg/fi/nodeup/nodetasks/BUILD.bazel +++ b/upup/pkg/fi/nodeup/nodetasks/BUILD.bazel @@ -10,6 +10,7 @@ go_library( "createsdir.go", "file.go", "group.go", + "issue_cert.go", "load_image.go", "package.go", "service.go", @@ -20,6 +21,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/backoff:go_default_library", + "//pkg/pki:go_default_library", "//upup/pkg/fi:go_default_library", "//upup/pkg/fi/nodeup/cloudinit:go_default_library", "//upup/pkg/fi/nodeup/local:go_default_library", @@ -37,9 +39,13 @@ go_test( "archive_test.go", "bindmount_test.go", "file_test.go", + "issue_cert_test.go", "loadimage_test.go", "service_test.go", ], embed = [":go_default_library"], - deps = ["//upup/pkg/fi:go_default_library"], + deps = [ + "//upup/pkg/fi:go_default_library", + "//vendor/github.com/stretchr/testify/assert:go_default_library", + ], ) diff --git a/upup/pkg/fi/nodeup/nodetasks/file.go b/upup/pkg/fi/nodeup/nodetasks/file.go index 60481b766c..279180142d 100644 --- a/upup/pkg/fi/nodeup/nodetasks/file.go +++ b/upup/pkg/fi/nodeup/nodetasks/file.go @@ -98,6 +98,10 @@ func (e *File) GetDependencies(tasks map[string]fi.Task) []fi.Task { // Requires parent directories to be created deps = append(deps, findCreatesDirParents(e.Path, tasks)...) + if hasDep, ok := e.Contents.(fi.HasDependencies); ok { + deps = append(deps, hasDep.GetDependencies(tasks)...) + } + // Requires other files to be created first for _, f := range e.AfterFiles { for _, v := range tasks { diff --git a/upup/pkg/fi/nodeup/nodetasks/issue_cert.go b/upup/pkg/fi/nodeup/nodetasks/issue_cert.go new file mode 100644 index 0000000000..0647935d35 --- /dev/null +++ b/upup/pkg/fi/nodeup/nodetasks/issue_cert.go @@ -0,0 +1,140 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package nodetasks + +import ( + "bytes" + "crypto/x509/pkix" + "fmt" + "io" + "path/filepath" + + "k8s.io/klog" + "k8s.io/kops/pkg/pki" + "k8s.io/kops/upup/pkg/fi" +) + +type IssueCert struct { + Name string + + Signer string `json:"signer"` + Type string `json:"type"` + Subject pkix.Name `json:"subject"` + AlternateNames []string `json:"alternateNames,omitempty"` + + cert *fi.TaskDependentResource + key *fi.TaskDependentResource + ca *fi.TaskDependentResource +} + +var _ fi.Task = &IssueCert{} +var _ fi.HasName = &IssueCert{} + +func (i *IssueCert) GetName() *string { + return &i.Name +} + +func (i *IssueCert) SetName(name string) { + i.Name = name +} + +// String returns a string representation, implementing the Stringer interface +func (i *IssueCert) String() string { + return fmt.Sprintf("IssueCert: %s", i.Name) +} + +func (i *IssueCert) GetResources() (certResource, keyResource, caResource *fi.TaskDependentResource) { + if i.cert == nil { + i.cert = &fi.TaskDependentResource{Task: i} + i.key = &fi.TaskDependentResource{Task: i} + i.ca = &fi.TaskDependentResource{Task: i} + } + return i.cert, i.key, i.ca +} + +func (i *IssueCert) AddFileTasks(c *fi.ModelBuilderContext, dir string, name string, caName string, owner *string) { + certResource, keyResource, caResource := i.GetResources() + c.AddTask(&File{ + Path: dir, + Type: FileType_Directory, + Mode: fi.String("0755"), + }) + + c.AddTask(&File{ + Path: filepath.Join(dir, name+".crt"), + Contents: certResource, + Type: FileType_File, + Mode: fi.String("0644"), + Owner: owner, + }) + + c.AddTask(&File{ + Path: filepath.Join(dir, name+".key"), + Contents: keyResource, + Type: FileType_File, + Mode: fi.String("0600"), + Owner: owner, + }) + + if caName != "" { + c.AddTask(&File{ + Path: filepath.Join(dir, caName+".crt"), + Contents: caResource, + Type: FileType_File, + Mode: fi.String("0644"), + Owner: owner, + }) + } +} + +func (e *IssueCert) Run(c *fi.Context) error { + req := &pki.IssueCertRequest{ + Signer: e.Signer, + Type: e.Type, + Subject: e.Subject, + MinValidDays: 455, + } + + klog.Infof("signing certificate for %q", e.Name) + certificate, privateKey, caCertificate, err := pki.IssueCert(req, c.Keystore) + if err != nil { + return err + } + + certResource, keyResource, caResource := e.GetResources() + certResource.Resource = &asBytesResource{certificate} + keyResource.Resource = &asBytesResource{privateKey} + caResource.Resource = &asBytesResource{caCertificate} + + return nil +} + +type hasAsBytes interface { + AsBytes() ([]byte, error) +} + +type asBytesResource struct { + hasAsBytes +} + +func (a asBytesResource) Open() (io.Reader, error) { + data, err := a.AsBytes() + if err != nil { + return nil, err + } + return bytes.NewReader(data), nil +} diff --git a/upup/pkg/fi/nodeup/nodetasks/issue_cert_test.go b/upup/pkg/fi/nodeup/nodetasks/issue_cert_test.go new file mode 100644 index 0000000000..98ab0c47b1 --- /dev/null +++ b/upup/pkg/fi/nodeup/nodetasks/issue_cert_test.go @@ -0,0 +1,53 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package nodetasks + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/kops/upup/pkg/fi" +) + +func TestIssueCertFileDependencies(t *testing.T) { + context := &fi.ModelBuilderContext{ + Tasks: make(map[string]fi.Task), + } + + issue := &IssueCert{Name: "testCert"} + context.AddTask(issue) + issue.AddFileTasks(context, "/tmp", "testCert", "testCa", nil) + var taskNames []string + for name := range context.Tasks { + taskNames = append(taskNames, name) + } + assert.ElementsMatch(t, []string{"IssueCert/testCert", "File//tmp", "File//tmp/testCert.crt", "File//tmp/testCert.key", "File//tmp/testCa.crt"}, taskNames) + + for _, fileName := range []string{"/tmp/testCert.crt", "/tmp/testCert.key", "/tmp/testCa.crt"} { + task := context.Tasks["File/"+fileName] + if !assert.NotNil(t, task) { + continue + } + deps := task.(fi.HasDependencies).GetDependencies(context.Tasks) + + taskNames = nil + for _, task := range deps { + taskNames = append(taskNames, *task.(fi.HasName).GetName()) + } + assert.ElementsMatch(t, []string{"testCert", "/tmp"}, taskNames) + } +} diff --git a/upup/pkg/fi/nodeup/nodetasks/service.go b/upup/pkg/fi/nodeup/nodetasks/service.go index 41b1f7375e..dd099feeee 100644 --- a/upup/pkg/fi/nodeup/nodetasks/service.go +++ b/upup/pkg/fi/nodeup/nodetasks/service.go @@ -64,13 +64,13 @@ func (p *Service) GetDependencies(tasks map[string]fi.Task) []fi.Task { var deps []fi.Task for _, v := range tasks { // We assume that services depend on everything except for - // LoadImageTask. If there are any LoadImageTasks (e.g. we're + // LoadImageTask or IssueCert. If there are any LoadImageTasks (e.g. we're // launching a custom Kubernetes build), they all depend on // the "docker.service" Service task. switch v.(type) { case *File, *Package, *UpdatePackages, *UserTask, *GroupTask, *Chattr, *BindMount, *Archive: deps = append(deps, v) - case *Service, *LoadImageTask: + case *Service, *LoadImageTask, *IssueCert: // ignore default: klog.Warningf("Unhandled type %T in Service::GetDependencies: %v", v, v) diff --git a/upup/pkg/fi/resources.go b/upup/pkg/fi/resources.go index 772fd00bf6..f5a57e104a 100644 --- a/upup/pkg/fi/resources.go +++ b/upup/pkg/fi/resources.go @@ -248,3 +248,22 @@ func WrapResource(r Resource) *ResourceHolder { Resource: r, } } + +type TaskDependentResource struct { + Resource Resource + Task Task +} + +var _ Resource = &TaskDependentResource{} +var _ HasDependencies = &TaskDependentResource{} + +func (r *TaskDependentResource) Open() (io.Reader, error) { + if r.Resource == nil { + return nil, fmt.Errorf("resource opened before it is ready") + } + return r.Resource.Open() +} + +func (r *TaskDependentResource) GetDependencies(tasks map[string]Task) []Task { + return []Task{r.Task} +}