From 8a6d29cd40d2d5cbaae899ceb5906e5e556f1abd Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Fri, 15 May 2020 23:31:40 -0700 Subject: [PATCH] Remove support for reading legacy-format keypairs --- cmd/kops/util/factory.go | 5 +- examples/kops-api-example/apply.go | 3 +- examples/kops-api-example/up.go | 3 +- hack/.packages | 1 - nodeup/pkg/model/kube_apiserver_test.go | 2 +- nodeup/pkg/model/kubelet_test.go | 2 +- pkg/client/simple/vfsclientset/clientset.go | 10 +- pkg/kubeconfig/create_kubecfg_test.go | 16 +- pkg/model/components/etcdmanager/model.go | 6 - .../etcdmanager/tests/minimal/tasks.yaml | 10 +- .../tests/old_versions_mount_hosts/tasks.yaml | 10 +- .../tests/overwrite_settings/tasks.yaml | 10 +- .../etcdmanager/tests/proxy/tasks.yaml | 10 +- pkg/model/pki.go | 24 -- tests/BUILD.bazel | 13 - tests/keypair_test.go | 254 ------------------ upup/pkg/fi/ca.go | 13 +- upup/pkg/fi/clientset_castore.go | 15 +- .../cloudup/bootstrapchannelbuilder_test.go | 2 +- upup/pkg/fi/cloudup/populatecluster_test.go | 2 +- upup/pkg/fi/fitasks/keypair.go | 13 +- upup/pkg/fi/nodeup/command.go | 2 +- upup/pkg/fi/vfs_castore.go | 229 ++++------------ 23 files changed, 113 insertions(+), 542 deletions(-) delete mode 100644 tests/keypair_test.go diff --git a/cmd/kops/util/factory.go b/cmd/kops/util/factory.go index 314178d59c445..4ad1c0454e5f1 100644 --- a/cmd/kops/util/factory.go +++ b/cmd/kops/util/factory.go @@ -113,10 +113,7 @@ func (f *Factory) Clientset() (simple.Clientset, error) { return nil, field.Invalid(field.NewPath("State Store"), registryPath, INVALID_STATE_ERROR) } - // For kops CLI / controller, we do allow vfs list (unlike nodeup!) - allowVFSList := true - - f.clientset = vfsclientset.NewVFSClientset(basePath, allowVFSList) + f.clientset = vfsclientset.NewVFSClientset(basePath) } if strings.HasPrefix(registryPath, "file://") { klog.Warning("The local filesystem state store is not functional for running clusters") diff --git a/examples/kops-api-example/apply.go b/examples/kops-api-example/apply.go index d054d9fcd459e..44450b68fedcb 100644 --- a/examples/kops-api-example/apply.go +++ b/examples/kops-api-example/apply.go @@ -24,8 +24,7 @@ import ( ) func apply(ctx context.Context) error { - allowList := true - clientset := vfsclientset.NewVFSClientset(registryBase, allowList) + clientset := vfsclientset.NewVFSClientset(registryBase) cluster, err := clientset.GetCluster(ctx, clusterName) if err != nil { diff --git a/examples/kops-api-example/up.go b/examples/kops-api-example/up.go index 18cee93c87b12..2203237aaf115 100644 --- a/examples/kops-api-example/up.go +++ b/examples/kops-api-example/up.go @@ -30,8 +30,7 @@ import ( ) func up(ctx context.Context) error { - allowList := true - clientset := vfsclientset.NewVFSClientset(registryBase, allowList) + clientset := vfsclientset.NewVFSClientset(registryBase) cluster := &api.Cluster{} cluster.ObjectMeta.Name = clusterName diff --git a/hack/.packages b/hack/.packages index f957bc1418c1d..ae8afaf6330af 100644 --- a/hack/.packages +++ b/hack/.packages @@ -156,7 +156,6 @@ k8s.io/kops/protokube/pkg/gossip/openstack k8s.io/kops/protokube/pkg/hostmount k8s.io/kops/protokube/pkg/protokube k8s.io/kops/protokube/tests/integration/build_etcd_manifest -k8s.io/kops/tests k8s.io/kops/tests/codecs k8s.io/kops/tests/integration/channel k8s.io/kops/tests/integration/conversion diff --git a/nodeup/pkg/model/kube_apiserver_test.go b/nodeup/pkg/model/kube_apiserver_test.go index 3deb3535519b7..e7dbf03db73d2 100644 --- a/nodeup/pkg/model/kube_apiserver_test.go +++ b/nodeup/pkg/model/kube_apiserver_test.go @@ -70,7 +70,7 @@ type fakeKeyStore struct { T *testing.T } -func (k fakeKeyStore) FindKeypair(name string) (*pki.Certificate, *pki.PrivateKey, fi.KeysetFormat, error) { +func (k fakeKeyStore) FindKeypair(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { panic("implement me") } diff --git a/nodeup/pkg/model/kubelet_test.go b/nodeup/pkg/model/kubelet_test.go index 2bacdef120ce5..2b74c026afbd3 100644 --- a/nodeup/pkg/model/kubelet_test.go +++ b/nodeup/pkg/model/kubelet_test.go @@ -228,7 +228,7 @@ func mockedPopulateClusterSpec(c *kops.Cluster) (*kops.Cluster, error) { if err != nil { return nil, fmt.Errorf("error building vfspath: %v", err) } - clientset := vfsclientset.NewVFSClientset(basePath, true) + clientset := vfsclientset.NewVFSClientset(basePath) return cloudup.PopulateClusterSpec(clientset, c, assetBuilder) } diff --git a/pkg/client/simple/vfsclientset/clientset.go b/pkg/client/simple/vfsclientset/clientset.go index c06c7415db737..1fd7230df0afd 100644 --- a/pkg/client/simple/vfsclientset/clientset.go +++ b/pkg/client/simple/vfsclientset/clientset.go @@ -32,8 +32,7 @@ import ( ) type VFSClientset struct { - basePath vfs.Path - allowList bool + basePath vfs.Path } var _ simple.Clientset = &VFSClientset{} @@ -90,7 +89,7 @@ func (c *VFSClientset) KeyStore(cluster *kops.Cluster) (fi.CAStore, error) { return nil, err } basedir := configBase.Join("pki") - return fi.NewVFSCAStore(cluster, basedir, c.allowList), nil + return fi.NewVFSCAStore(cluster, basedir), nil } func (c *VFSClientset) SSHCredentialStore(cluster *kops.Cluster) (fi.SSHCredentialStore, error) { @@ -163,10 +162,9 @@ func (c *VFSClientset) DeleteCluster(ctx context.Context, cluster *kops.Cluster) return DeleteAllClusterState(configBase) } -func NewVFSClientset(basePath vfs.Path, allowList bool) simple.Clientset { +func NewVFSClientset(basePath vfs.Path) simple.Clientset { vfsClientset := &VFSClientset{ - basePath: basePath, - allowList: allowList, + basePath: basePath, } return vfsClientset } diff --git a/pkg/kubeconfig/create_kubecfg_test.go b/pkg/kubeconfig/create_kubecfg_test.go index 5496daa88ad56..5c34a8b46823b 100644 --- a/pkg/kubeconfig/create_kubecfg_test.go +++ b/pkg/kubeconfig/create_kubecfg_test.go @@ -48,7 +48,7 @@ func (f fakeStatusStore) GetApiIngressStatus(cluster *kops.Cluster) ([]kops.ApiI // mock a fake key store type fakeKeyStore struct { - FindKeypairFn func(name string) (*pki.Certificate, *pki.PrivateKey, fi.KeysetFormat, error) + FindKeypairFn func(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) CreateKeypairFn func(signer string, name string, template *x509.Certificate, privateKey *pki.PrivateKey) (*pki.Certificate, error) @@ -59,7 +59,7 @@ type fakeKeyStore struct { MirrorToFn func(basedir vfs.Path) error } -func (f fakeKeyStore) FindKeypair(name string) (*pki.Certificate, *pki.PrivateKey, fi.KeysetFormat, error) { +func (f fakeKeyStore) FindKeypair(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { return f.FindKeypairFn(name) } @@ -144,10 +144,10 @@ func TestBuildKubecfg(t *testing.T) { args{ publiccluster, fakeKeyStore{ - FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, fi.KeysetFormat, error) { + FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { return fakeCertificate(), fakePrivateKey(), - fi.KeysetFormatLegacy, + true, nil }, }, @@ -169,10 +169,10 @@ func TestBuildKubecfg(t *testing.T) { args{ emptyMasterPublicNameCluster, fakeKeyStore{ - FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, fi.KeysetFormat, error) { + FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { return fakeCertificate(), fakePrivateKey(), - fi.KeysetFormatLegacy, + true, nil }, }, @@ -194,10 +194,10 @@ func TestBuildKubecfg(t *testing.T) { args{ gossipCluster, fakeKeyStore{ - FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, fi.KeysetFormat, error) { + FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { return fakeCertificate(), fakePrivateKey(), - fi.KeysetFormatLegacy, + true, nil }, }, diff --git a/pkg/model/components/etcdmanager/model.go b/pkg/model/components/etcdmanager/model.go index 866a8e67a74a9..c204e137765f9 100644 --- a/pkg/model/components/etcdmanager/model.go +++ b/pkg/model/components/etcdmanager/model.go @@ -97,8 +97,6 @@ func (b *EtcdManagerBuilder) Build(c *fi.ModelBuilderContext) error { return err } - format := string(fi.KeysetFormatV1Alpha2) - c.AddTask(&fitasks.ManagedFile{ Contents: fi.WrapResource(fi.NewBytesResource(d)), Lifecycle: b.Lifecycle, @@ -112,7 +110,6 @@ func (b *EtcdManagerBuilder) Build(c *fi.ModelBuilderContext) error { Name: fi.String("etcd-manager-ca-" + etcdCluster.Name), Subject: "cn=etcd-manager-ca-" + etcdCluster.Name, Type: "ca", - Format: format, }) // We create a CA for etcd peers and a separate one for clients @@ -120,7 +117,6 @@ func (b *EtcdManagerBuilder) Build(c *fi.ModelBuilderContext) error { Name: fi.String("etcd-peers-ca-" + etcdCluster.Name), Subject: "cn=etcd-peers-ca-" + etcdCluster.Name, Type: "ca", - Format: format, }) // Because API server can only have a single client-cert, we need to share a client CA @@ -128,7 +124,6 @@ func (b *EtcdManagerBuilder) Build(c *fi.ModelBuilderContext) error { Name: fi.String("etcd-clients-ca"), Subject: "cn=etcd-clients-ca", Type: "ca", - Format: format, }); err != nil { return err } @@ -138,7 +133,6 @@ func (b *EtcdManagerBuilder) Build(c *fi.ModelBuilderContext) error { Name: fi.String("etcd-clients-ca-cilium"), Subject: "cn=etcd-clients-ca-cilium", Type: "ca", - Format: format, }) } } diff --git a/pkg/model/components/etcdmanager/tests/minimal/tasks.yaml b/pkg/model/components/etcdmanager/tests/minimal/tasks.yaml index 5732ef49b9a1a..cec87787c0457 100644 --- a/pkg/model/components/etcdmanager/tests/minimal/tasks.yaml +++ b/pkg/model/components/etcdmanager/tests/minimal/tasks.yaml @@ -3,7 +3,7 @@ Name: etcd-clients-ca Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-clients-ca type: ca --- @@ -12,7 +12,7 @@ Name: etcd-manager-ca-events Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-manager-ca-events type: ca --- @@ -21,7 +21,7 @@ Name: etcd-manager-ca-main Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-manager-ca-main type: ca --- @@ -30,7 +30,7 @@ Name: etcd-peers-ca-events Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-peers-ca-events type: ca --- @@ -39,7 +39,7 @@ Name: etcd-peers-ca-main Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-peers-ca-main type: ca --- diff --git a/pkg/model/components/etcdmanager/tests/old_versions_mount_hosts/tasks.yaml b/pkg/model/components/etcdmanager/tests/old_versions_mount_hosts/tasks.yaml index b143c39e04c02..38c6dc52c1764 100644 --- a/pkg/model/components/etcdmanager/tests/old_versions_mount_hosts/tasks.yaml +++ b/pkg/model/components/etcdmanager/tests/old_versions_mount_hosts/tasks.yaml @@ -3,7 +3,7 @@ Name: etcd-clients-ca Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-clients-ca type: ca --- @@ -12,7 +12,7 @@ Name: etcd-manager-ca-events Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-manager-ca-events type: ca --- @@ -21,7 +21,7 @@ Name: etcd-manager-ca-main Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-manager-ca-main type: ca --- @@ -30,7 +30,7 @@ Name: etcd-peers-ca-events Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-peers-ca-events type: ca --- @@ -39,7 +39,7 @@ Name: etcd-peers-ca-main Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-peers-ca-main type: ca --- diff --git a/pkg/model/components/etcdmanager/tests/overwrite_settings/tasks.yaml b/pkg/model/components/etcdmanager/tests/overwrite_settings/tasks.yaml index 2bac932080a2f..bace8398a60d4 100644 --- a/pkg/model/components/etcdmanager/tests/overwrite_settings/tasks.yaml +++ b/pkg/model/components/etcdmanager/tests/overwrite_settings/tasks.yaml @@ -3,7 +3,7 @@ Name: etcd-clients-ca Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-clients-ca type: ca --- @@ -12,7 +12,7 @@ Name: etcd-manager-ca-events Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-manager-ca-events type: ca --- @@ -21,7 +21,7 @@ Name: etcd-manager-ca-main Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-manager-ca-main type: ca --- @@ -30,7 +30,7 @@ Name: etcd-peers-ca-events Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-peers-ca-events type: ca --- @@ -39,7 +39,7 @@ Name: etcd-peers-ca-main Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-peers-ca-main type: ca --- diff --git a/pkg/model/components/etcdmanager/tests/proxy/tasks.yaml b/pkg/model/components/etcdmanager/tests/proxy/tasks.yaml index 68ce97a39e6a3..6fde5e71108fa 100644 --- a/pkg/model/components/etcdmanager/tests/proxy/tasks.yaml +++ b/pkg/model/components/etcdmanager/tests/proxy/tasks.yaml @@ -3,7 +3,7 @@ Name: etcd-clients-ca Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-clients-ca type: ca --- @@ -12,7 +12,7 @@ Name: etcd-manager-ca-events Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-manager-ca-events type: ca --- @@ -21,7 +21,7 @@ Name: etcd-manager-ca-main Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-manager-ca-main type: ca --- @@ -30,7 +30,7 @@ Name: etcd-peers-ca-events Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-peers-ca-events type: ca --- @@ -39,7 +39,7 @@ Name: etcd-peers-ca-main Signer: null alternateNameTasks: null alternateNames: null -format: v1alpha2 +oldFormat: false subject: cn=etcd-peers-ca-main type: ca --- diff --git a/pkg/model/pki.go b/pkg/model/pki.go index cba931f717701..f5e8f2dae9eea 100644 --- a/pkg/model/pki.go +++ b/pkg/model/pki.go @@ -38,17 +38,12 @@ var _ fi.ModelBuilder = &PKIModelBuilder{} // Build is responsible for generating the various pki assets. func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { - // We specify the KeysetFormatV1Alpha2 format, to upgrade from the legacy representation (separate files) - // to the newer keyset.yaml representation. - format := string(fi.KeysetFormatV1Alpha2) - // TODO: Only create the CA via this task defaultCA := &fitasks.Keypair{ Name: fi.String(fi.CertificateId_CA), Lifecycle: b.Lifecycle, Subject: "cn=kubernetes", Type: "ca", - Format: format, } c.AddTask(defaultCA) @@ -62,7 +57,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "o=" + rbac.NodesGroup + ",cn=kubelet", Type: "client", Signer: defaultCA, - Format: format, }) } } @@ -76,7 +70,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "cn=kubelet-api", Type: "client", Signer: defaultCA, - Format: format, }) } { @@ -86,7 +79,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "cn=" + rbac.KubeScheduler, Type: "client", Signer: defaultCA, - Format: format, } c.AddTask(t) } @@ -98,7 +90,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "cn=" + rbac.KubeProxy, Type: "client", Signer: defaultCA, - Format: format, } c.AddTask(t) } @@ -110,7 +101,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "cn=" + rbac.KubeControllerManager, Type: "client", Signer: defaultCA, - Format: format, } c.AddTask(t) } @@ -133,7 +123,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { // TODO: Can this be "server" now that we're not using it for peer connectivity? Type: "clientServer", Signer: defaultCA, - Format: format, }) // For peer authentication, the same cert is used both as a client @@ -162,7 +151,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "cn=etcd-peer", Type: "clientServer", Signer: defaultCA, - Format: format, }) c.AddTask(&fitasks.Keypair{ @@ -171,7 +159,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "cn=etcd-client", Type: "client", Signer: defaultCA, - Format: format, }) // @check if calico is enabled as the CNI provider @@ -182,7 +169,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "cn=calico-client", Type: "client", Signer: defaultCA, - Format: format, }) } } @@ -193,7 +179,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "cn=" + "system:kube-router", Type: "client", Signer: defaultCA, - Format: format, } c.AddTask(t) } @@ -205,7 +190,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "o=" + rbac.SystemPrivilegedGroup + ",cn=kubecfg", Type: "client", Signer: defaultCA, - Format: format, } c.AddTask(t) } @@ -217,7 +201,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "cn=apiserver-proxy-client", Type: "client", Signer: defaultCA, - Format: format, } c.AddTask(t) } @@ -228,7 +211,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Lifecycle: b.Lifecycle, Subject: "cn=apiserver-aggregator-ca", Type: "ca", - Format: format, } c.AddTask(aggregatorCA) @@ -239,7 +221,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "cn=aggregator", Type: "client", Signer: aggregatorCA, - Format: format, } c.AddTask(aggregator) } @@ -252,7 +233,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "o=" + rbac.SystemPrivilegedGroup + ",cn=kops", Type: "client", Signer: defaultCA, - Format: format, } c.AddTask(t) } @@ -290,7 +270,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Type: "server", AlternateNames: alternateNames, Signer: defaultCA, - Format: format, } c.AddTask(t) } @@ -308,7 +287,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Type: "server", AlternateNames: alternateNames, Signer: defaultCA, - Format: format, } c.AddTask(t) } @@ -335,7 +313,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Type: "server", AlternateNames: alternateNames, Signer: defaultCA, - Format: format, }) // @note: we use this for mutual tls between node and authorizer @@ -344,7 +321,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error { Subject: "cn=node-authorizer-client", Type: "client", Signer: defaultCA, - Format: format, }) } diff --git a/tests/BUILD.bazel b/tests/BUILD.bazel index 3999bd7487512..e69de29bb2d1d 100644 --- a/tests/BUILD.bazel +++ b/tests/BUILD.bazel @@ -1,13 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_test") - -go_test( - name = "go_default_test", - srcs = ["keypair_test.go"], - deps = [ - "//pkg/apis/kops:go_default_library", - "//pkg/diff:go_default_library", - "//upup/pkg/fi:go_default_library", - "//upup/pkg/fi/fitasks:go_default_library", - "//util/pkg/vfs:go_default_library", - ], -) diff --git a/tests/keypair_test.go b/tests/keypair_test.go deleted file mode 100644 index b42c25b0b52d7..0000000000000 --- a/tests/keypair_test.go +++ /dev/null @@ -1,254 +0,0 @@ -/* -Copyright 2019 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. -*/ - -/* -Copyright 2019 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 tests - -import ( - "math/big" - "reflect" - "sort" - "testing" - "time" - - "k8s.io/kops/pkg/apis/kops" - "k8s.io/kops/pkg/diff" - "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/fitasks" - "k8s.io/kops/util/pkg/vfs" -) - -type MockTarget struct { -} - -func (t *MockTarget) Finish(taskMap map[string]fi.Task) error { - return nil -} - -func (t *MockTarget) ProcessDeletions() bool { - return false -} - -var _ fi.Target = &MockTarget{} - -// Verifies that we regenerate keyset.yaml if they are deleted, which covers the upgrade scenario from kops 1.8 -> kops 1.9 -func TestKeypairUpgrade(t *testing.T) { - lifecycle := fi.LifecycleSync - - runTasksOptions := fi.RunTasksOptions{} - runTasksOptions.MaxTaskDuration = 2 * time.Second - - target := &MockTarget{} - - cluster := &kops.Cluster{} - vfs.Context.ResetMemfsContext(true) - - basedir, err := vfs.Context.BuildVfsPath("memfs://keystore") - if err != nil { - t.Fatalf("error building vfs path: %v", err) - } - - keystore := fi.NewVFSCAStore(cluster, basedir, true) - - // Generate predictable sequence numbers for testing - var n int64 - keystore.SerialGenerator = func() *big.Int { - n++ - return big.NewInt(n) - } - - // We define a function so we can rebuild the tasks, because we modify in-place when running - buildTasks := func() map[string]fi.Task { - format := string(fi.KeysetFormatV1Alpha2) - - ca := &fitasks.Keypair{ - Name: fi.String(fi.CertificateId_CA), - Lifecycle: &lifecycle, - Subject: "cn=kubernetes", - Type: "ca", - Format: format, - } - - kubelet := &fitasks.Keypair{ - Name: fi.String("kubelet"), - Lifecycle: &lifecycle, - Subject: "o=nodes,cn=kubelet", - Type: "client", - Signer: ca, - Format: format, - } - - tasks := make(map[string]fi.Task) - tasks["ca"] = ca - tasks["kubelet"] = kubelet - return tasks - } - - t.Logf("Building some keypairs") - { - allTasks := buildTasks() - - context, err := fi.NewContext(target, nil, nil, keystore, nil, nil, true, allTasks) - if err != nil { - t.Fatalf("error building context: %v", err) - } - - if err := context.RunTasks(runTasksOptions); err != nil { - t.Fatalf("unexpected error during Run: %v", err) - } - } - - // Check that the expected files were generated - expected := []string{ - "memfs://keystore/issued/ca/1.crt", - "memfs://keystore/issued/ca/keyset.yaml", - "memfs://keystore/issued/kubelet/2.crt", - "memfs://keystore/issued/kubelet/keyset.yaml", - "memfs://keystore/private/ca/1.key", - "memfs://keystore/private/ca/keyset.yaml", - "memfs://keystore/private/kubelet/2.key", - "memfs://keystore/private/kubelet/keyset.yaml", - } - checkPaths(t, basedir, expected) - - // Save the contents of those files - contents := make(map[string]string) - for _, k := range expected { - p, err := vfs.Context.BuildVfsPath(k) - if err != nil { - t.Fatalf("error building vfs path: %v", err) - } - b, err := p.ReadFile() - if err != nil { - t.Fatalf("error reading vfs path: %v", err) - } - contents[k] = string(b) - } - - t.Logf("verifying that rerunning tasks does not change keys") - { - allTasks := buildTasks() - - context, err := fi.NewContext(target, nil, nil, keystore, nil, nil, true, allTasks) - if err != nil { - t.Fatalf("error building context: %v", err) - } - - if err := context.RunTasks(runTasksOptions); err != nil { - t.Fatalf("unexpected error during Run: %v", err) - } - } - checkContents(t, basedir, contents) - - t.Logf("deleting keyset.yaml files and verifying they are recreated") - FailOnError(t, basedir.Join("issued/ca/keyset.yaml").Remove()) - FailOnError(t, basedir.Join("issued/kubelet/keyset.yaml").Remove()) - FailOnError(t, basedir.Join("private/ca/keyset.yaml").Remove()) - FailOnError(t, basedir.Join("private/kubelet/keyset.yaml").Remove()) - - { - allTasks := buildTasks() - - context, err := fi.NewContext(target, nil, nil, keystore, nil, nil, true, allTasks) - if err != nil { - t.Fatalf("error building context: %v", err) - } - - if err := context.RunTasks(runTasksOptions); err != nil { - t.Fatalf("unexpected error during Run: %v", err) - } - } - checkContents(t, basedir, contents) -} - -// FailOnError calls t.Fatalf if err != nil -func FailOnError(t *testing.T, err error) { - if err != nil { - t.Fatalf("unexpected error: %v", err) - } -} - -// checkPaths verifies that the path names in the tree rooted at basedir are exactly as expected -// Unlike checkContents, it only verifies the names, not the contents -func checkPaths(t *testing.T, basedir vfs.Path, expected []string) { - paths, err := basedir.ReadTree() - if err != nil { - t.Errorf("ReadTree failed: %v", err) - } - - var actual []string - for _, p := range paths { - actual = append(actual, p.Path()) - } - sort.Strings(actual) - if !reflect.DeepEqual(actual, expected) { - t.Fatalf("unexpected paths: %v", actual) - } -} - -// checkPaths verifies that the files and their contents in the tree rooted at basedir are exactly as expected -func checkContents(t *testing.T, basedir vfs.Path, expected map[string]string) { - paths, err := basedir.ReadTree() - if err != nil { - t.Errorf("ReadTree failed: %v", err) - } - - actual := make(map[string]string) - for _, p := range paths { - b, err := p.ReadFile() - if err != nil { - t.Fatalf("error reading vfs path %q: %v", p, err) - } - actual[p.Path()] = string(b) - } - - var actualKeys []string - for k := range actual { - actualKeys = append(actualKeys, k) - } - sort.Strings(actualKeys) - var expectedKeys []string - for k := range expected { - expectedKeys = append(expectedKeys, k) - } - sort.Strings(expectedKeys) - - if !reflect.DeepEqual(actualKeys, expectedKeys) { - t.Fatalf("unexpected paths: %v", actualKeys) - } - if !reflect.DeepEqual(actual, expected) { - for k := range actual { - if actual[k] != expected[k] { - t.Errorf("mismatch on key %q", k) - t.Errorf("diff: %s", diff.FormatDiff(actual[k], expected[k])) - } - } - } -} diff --git a/upup/pkg/fi/ca.go b/upup/pkg/fi/ca.go index 736c739b65dc0..4a8e7de2362aa 100644 --- a/upup/pkg/fi/ca.go +++ b/upup/pkg/fi/ca.go @@ -33,11 +33,8 @@ const ( SecretNameSSHPrimary = "admin" ) -type KeysetFormat string - const ( - KeysetFormatLegacy KeysetFormat = "legacy" - KeysetFormatV1Alpha2 KeysetFormat = "v1alpha2" + keysetFormatLatest = "v1alpha2" ) type KeystoreItem struct { @@ -51,10 +48,10 @@ type KeystoreItem struct { type Keystore interface { // FindKeypair finds a cert & private key, returning nil where either is not found // (if the certificate is found but not keypair, that is not an error: only the cert will be returned). - // This func returns a cert, private key and a string. The string value is the Format of the keystore which is either - // an empty string, which denotes a Legacy Keypair, or a value of "Keypair". This string is used by a keypair - // task convert a Legacy Keypair to the new Keypair API format. - FindKeypair(name string) (*pki.Certificate, *pki.PrivateKey, KeysetFormat, error) + // This func returns a cert, private key and a bool. The bool value is whether the keypair is stored + // in a legacy format. This bool is used by a keypair + // task to convert a Legacy Keypair to the new Keypair API format. + FindKeypair(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) CreateKeypair(signer string, name string, template *x509.Certificate, privateKey *pki.PrivateKey) (*pki.Certificate, error) diff --git a/upup/pkg/fi/clientset_castore.go b/upup/pkg/fi/clientset_castore.go index f1917fc6b1d53..b131f383e40c7 100644 --- a/upup/pkg/fi/clientset_castore.go +++ b/upup/pkg/fi/clientset_castore.go @@ -98,9 +98,9 @@ func (c *ClientsetCAStore) readCAKeypairs(ctx context.Context, id string) (*keys // keyset is a parsed Keyset type keyset struct { - format KeysetFormat - items map[string]*keysetItem - primary *keysetItem + legacyFormat bool + items map[string]*keysetItem + primary *keysetItem } // keysetItem is a parsed KeysetItem @@ -160,7 +160,6 @@ func (c *ClientsetCAStore) loadKeyset(ctx context.Context, name string) (*keyset if err != nil { return nil, err } - keyset.format = KeysetFormatV1Alpha2 return keyset, nil } @@ -205,18 +204,18 @@ func FindPrimary(keyset *kops.Keyset) *kops.KeysetItem { } // FindKeypair implements CAStore::FindKeypair -func (c *ClientsetCAStore) FindKeypair(name string) (*pki.Certificate, *pki.PrivateKey, KeysetFormat, error) { +func (c *ClientsetCAStore) FindKeypair(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { ctx := context.TODO() keyset, err := c.loadKeyset(ctx, name) if err != nil { - return nil, nil, "", err + return nil, nil, false, err } if keyset != nil && keyset.primary != nil { - return keyset.primary.certificate, keyset.primary.privateKey, keyset.format, nil + return keyset.primary.certificate, keyset.primary.privateKey, keyset.legacyFormat, nil } - return nil, nil, "", nil + return nil, nil, false, nil } // FindCert implements CAStore::FindCert diff --git a/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go b/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go index 7efaf94191907..9f35389a84e24 100644 --- a/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go +++ b/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go @@ -83,7 +83,7 @@ func runChannelBuilderTest(t *testing.T, key string, addonManifests []string) { if err != nil { t.Errorf("error building vfspath: %v", err) } - clientset := vfsclientset.NewVFSClientset(basePath, true) + clientset := vfsclientset.NewVFSClientset(basePath) secretStore, err := clientset.SecretStore(cluster) if err != nil { diff --git a/upup/pkg/fi/cloudup/populatecluster_test.go b/upup/pkg/fi/cloudup/populatecluster_test.go index 2ff099395c4df..a9ea7e519ae8b 100644 --- a/upup/pkg/fi/cloudup/populatecluster_test.go +++ b/upup/pkg/fi/cloudup/populatecluster_test.go @@ -109,7 +109,7 @@ func mockedPopulateClusterSpec(c *kopsapi.Cluster) (*kopsapi.Cluster, error) { if err != nil { return nil, fmt.Errorf("error building vfspath: %v", err) } - clientset := vfsclientset.NewVFSClientset(basePath, true) + clientset := vfsclientset.NewVFSClientset(basePath) return PopulateClusterSpec(clientset, c, assetBuilder) } diff --git a/upup/pkg/fi/fitasks/keypair.go b/upup/pkg/fi/fitasks/keypair.go index 3f4f85e1334f9..e8a775c4bc03f 100644 --- a/upup/pkg/fi/fitasks/keypair.go +++ b/upup/pkg/fi/fitasks/keypair.go @@ -51,9 +51,8 @@ type Keypair struct { Subject string `json:"subject"` // Type the type of certificate i.e. CA, server, client etc Type string `json:"type"` - // Format stores the api version of kops.Keyset. We are using this info in order to determine if kops - // is accessing legacy secrets that do not use keyset.yaml. - Format string `json:"format"` + // LegacyFormat is whether the keypair is stored in a legacy format. + LegacyFormat bool `json:"oldFormat"` } var _ fi.HasCheckExisting = &Keypair{} @@ -76,7 +75,7 @@ func (e *Keypair) Find(c *fi.Context) (*Keypair, error) { return nil, nil } - cert, key, format, err := c.Keystore.FindKeypair(name) + cert, key, legacyFormat, err := c.Keystore.FindKeypair(name) if err != nil { return nil, err } @@ -100,7 +99,7 @@ func (e *Keypair) Find(c *fi.Context) (*Keypair, error) { AlternateNames: alternateNames, Subject: pkixNameToString(&cert.Subject), Type: buildTypeDescription(cert.Certificate), - Format: string(format), + LegacyFormat: legacyFormat, } actual.Signer = &Keypair{Subject: pkixNameToString(&cert.Certificate.Issuer)} @@ -190,7 +189,7 @@ func (_ *Keypair) Render(c *fi.Context, a, e, changes *Keypair) error { } else if changes.Type != "" { createCertificate = true klog.V(8).Infof("creating certificate new Type") - } else if changes.Format != "" { + } else if changes.LegacyFormat { changeStoredFormat = true } else { klog.Warningf("Ignoring changes in key: %v", fi.DebugAsJsonString(changes)) @@ -244,7 +243,7 @@ func (_ *Keypair) Render(c *fi.Context, a, e, changes *Keypair) error { return err } - klog.Infof("updated Keypair %q to API format %q", name, e.Format) + klog.Infof("updated Keypair %q to new format", name) } return nil diff --git a/upup/pkg/fi/nodeup/command.go b/upup/pkg/fi/nodeup/command.go index 96d6b4fe03d3b..1473bfcbaa8cf 100644 --- a/upup/pkg/fi/nodeup/command.go +++ b/upup/pkg/fi/nodeup/command.go @@ -210,7 +210,7 @@ func (c *NodeUpCommand) Run(out io.Writer) error { return fmt.Errorf("error building key store path: %v", err) } - modelContext.KeyStore = fi.NewVFSCAStore(c.cluster, p, false) + modelContext.KeyStore = fi.NewVFSCAStore(c.cluster, p) } else { return fmt.Errorf("KeyStore not set") } diff --git a/upup/pkg/fi/vfs_castore.go b/upup/pkg/fi/vfs_castore.go index 898fb53684746..971f5740bb4f0 100644 --- a/upup/pkg/fi/vfs_castore.go +++ b/upup/pkg/fi/vfs_castore.go @@ -38,9 +38,8 @@ import ( ) type VFSCAStore struct { - basedir vfs.Path - cluster *kops.Cluster - allowList bool + basedir vfs.Path + cluster *kops.Cluster mutex sync.Mutex cachedCAs map[string]*cachedEntry @@ -58,12 +57,11 @@ type cachedEntry struct { var _ CAStore = &VFSCAStore{} var _ SSHCredentialStore = &VFSCAStore{} -func NewVFSCAStore(cluster *kops.Cluster, basedir vfs.Path, allowList bool) *VFSCAStore { +func NewVFSCAStore(cluster *kops.Cluster, basedir vfs.Path) *VFSCAStore { c := &VFSCAStore{ basedir: basedir, cluster: cluster, cachedCAs: make(map[string]*cachedEntry), - allowList: allowList, } c.SerialGenerator = func() *big.Int { @@ -100,7 +98,7 @@ func (s *VFSCAStore) readCAKeypairs(id string) (*keyset, *keyset, error) { return cached.certificates, cached.privateKeys, nil } - caCertificates, err := s.loadCertificates(s.buildCertificatePoolPath(id), true) + caCertificates, err := s.loadCertificates(s.buildCertificatePoolPath(id)) if err != nil { return nil, nil, err } @@ -108,7 +106,7 @@ func (s *VFSCAStore) readCAKeypairs(id string) (*keyset, *keyset, error) { var caPrivateKeys *keyset if caCertificates != nil { - caPrivateKeys, err = s.loadPrivateKeys(s.buildPrivateKeyPoolPath(id), true) + caPrivateKeys, err = s.loadPrivateKeys(s.buildPrivateKeyPoolPath(id)) if err != nil { return nil, nil, err } @@ -147,24 +145,24 @@ func (c *VFSCAStore) buildPrivateKeyPath(name string, id string) vfs.Path { return c.basedir.Join("private", name, id+".key") } -func (c *VFSCAStore) parseKeysetYaml(data []byte) (*kops.Keyset, KeysetFormat, error) { +func (c *VFSCAStore) parseKeysetYaml(data []byte) (*kops.Keyset, bool, error) { defaultReadVersion := v1alpha2.SchemeGroupVersion.WithKind("Keyset") object, gvk, err := kopscodecs.Decode(data, &defaultReadVersion) if err != nil { - return nil, "", fmt.Errorf("error parsing keyset: %v", err) + return nil, false, fmt.Errorf("error parsing keyset: %v", err) } keyset, ok := object.(*kops.Keyset) if !ok { - return nil, "", fmt.Errorf("object was not a keyset, was a %T", object) + return nil, false, fmt.Errorf("object was not a keyset, was a %T", object) } if gvk == nil { - return nil, "", fmt.Errorf("object did not have GroupVersionKind: %q", keyset.Name) + return nil, false, fmt.Errorf("object did not have GroupVersionKind: %q", keyset.Name) } - return keyset, KeysetFormat(gvk.Version), nil + return keyset, gvk.Version != keysetFormatLatest, nil } // loadCertificatesBundle loads a keyset from the path @@ -179,7 +177,7 @@ func (c *VFSCAStore) loadKeysetBundle(p vfs.Path) (*keyset, error) { return nil, fmt.Errorf("unable to read bundle %q: %v", p, err) } - o, format, err := c.parseKeysetYaml(data) + o, legacyFormat, err := c.parseKeysetYaml(data) if err != nil { return nil, fmt.Errorf("error parsing bundle %q: %v", p, err) } @@ -189,7 +187,7 @@ func (c *VFSCAStore) loadKeysetBundle(p vfs.Path) (*keyset, error) { return nil, fmt.Errorf("error mapping bundle %q: %v", p, err) } - keyset.format = format + keyset.legacyFormat = legacyFormat return keyset, nil } @@ -291,64 +289,10 @@ func SerializeKeyset(o *kops.Keyset) ([]byte, error) { return objectData.Bytes(), nil } -func (c *VFSCAStore) loadCertificates(p vfs.Path, useBundle bool) (*keyset, error) { - // Attempt to load prebuilt bundle, which avoids having to list files, which is a permission that can be hard to - // give on GCE / other clouds - if useBundle { - bundlePath := p.Join("keyset.yaml") - bundle, err := c.loadKeysetBundle(bundlePath) - if !c.allowList { - return bundle, err - } - - if err != nil { - klog.Warningf("unable to read bundle %q, falling back to directory-list method: %v", bundlePath, err) - } else if bundle == nil { - klog.V(2).Infof("no certificate bundle %q, falling back to directory-list method", bundlePath) - } else { - return bundle, nil - } - } - - keyset := &keyset{ - items: make(map[string]*keysetItem), - } - - files, err := p.ReadDir() - if err != nil { - if os.IsNotExist(err) { - return nil, nil - } - return nil, err - } - - for _, f := range files { - id := f.Base() - if strings.HasSuffix(id, ".yaml") { - // ignore bundle - continue - } - id = strings.TrimSuffix(id, ".crt") - - cert, err := c.loadOneCertificate(f) - if err != nil { - return nil, fmt.Errorf("error loading certificate %q: %v", f, err) - } - - keyset.items[id] = &keysetItem{ - id: id, - certificate: cert, - } - } - - if len(keyset.items) == 0 { - return nil, nil - } - - keyset.format = KeysetFormatLegacy - keyset.primary = keyset.findPrimary() - - return keyset, nil +func (c *VFSCAStore) loadCertificates(p vfs.Path) (*keyset, error) { + bundlePath := p.Join("keyset.yaml") + bundle, err := c.loadKeysetBundle(bundlePath) + return bundle, err } func (c *VFSCAStore) loadOneCertificate(p vfs.Path) (*pki.Certificate, error) { @@ -369,32 +313,32 @@ func (c *VFSCAStore) loadOneCertificate(p vfs.Path) (*pki.Certificate, error) { return cert, nil } -func (c *VFSCAStore) FindKeypair(id string) (*pki.Certificate, *pki.PrivateKey, KeysetFormat, error) { - cert, certFormat, err := c.findCert(id) +func (c *VFSCAStore) FindKeypair(id string) (*pki.Certificate, *pki.PrivateKey, bool, error) { + cert, legacyFormat, err := c.findCert(id) if err != nil { - return nil, nil, "", err + return nil, nil, false, err } key, err := c.FindPrivateKey(id) if err != nil { - return nil, nil, "", err + return nil, nil, false, err } - return cert, key, certFormat, nil + return cert, key, legacyFormat, nil } -func (c *VFSCAStore) findCert(name string) (*pki.Certificate, KeysetFormat, error) { +func (c *VFSCAStore) findCert(name string) (*pki.Certificate, bool, error) { p := c.buildCertificatePoolPath(name) - certs, err := c.loadCertificates(p, true) + certs, err := c.loadCertificates(p) if err != nil { - return nil, "", fmt.Errorf("error in 'FindCert' attempting to load cert %q: %v", name, err) + return nil, false, fmt.Errorf("error in 'FindCert' attempting to load cert %q: %v", name, err) } if certs != nil && certs.primary != nil { - return certs.primary.certificate, certs.format, nil + return certs.primary.certificate, certs.legacyFormat, nil } - return nil, "", nil + return nil, false, nil } func (c *VFSCAStore) FindCert(name string) (*pki.Certificate, error) { @@ -407,7 +351,7 @@ func (c *VFSCAStore) FindCertificatePool(name string) (*CertificatePool, error) var err error p := c.buildCertificatePoolPath(name) - certs, err = c.loadCertificates(p, true) + certs, err = c.loadCertificates(p) if err != nil { return nil, fmt.Errorf("error in 'FindCertificatePool' attempting to load cert %q: %v", name, err) } @@ -434,7 +378,7 @@ func (c *VFSCAStore) FindCertificatePool(name string) (*CertificatePool, error) func (c *VFSCAStore) FindCertificateKeyset(name string) (*kops.Keyset, error) { p := c.buildCertificatePoolPath(name) - certs, err := c.loadCertificates(p, true) + certs, err := c.loadCertificates(p) if err != nil { return nil, fmt.Errorf("error in 'FindCertificatePool' attempting to load cert %q: %v", name, err) } @@ -731,78 +675,11 @@ func (c *VFSCAStore) AddCert(name string, cert *pki.Certificate) error { return err } -func (c *VFSCAStore) loadPrivateKeys(p vfs.Path, useBundle bool) (*keyset, error) { - // Attempt to load prebuilt bundle, which avoids having to list files, which is a permission that can be hard to - // give on GCE / other clouds - if useBundle { - bundlePath := p.Join("keyset.yaml") - bundle, err := c.loadKeysetBundle(bundlePath) - - if !c.allowList { - return bundle, err - } - - if err != nil { - klog.Warningf("unable to read bundle %q, falling back to directory-list method: %v", bundlePath, err) - } else if bundle == nil { - klog.V(2).Infof("no private key bundle %q, falling back to directory-list method", bundlePath) - } else { - return bundle, nil - } - } - - files, err := p.ReadDir() - if err != nil { - if os.IsNotExist(err) { - return nil, nil - } - return nil, err - } - - keys := &keyset{ - items: make(map[string]*keysetItem), - } - - for _, f := range files { - id := f.Base() - if strings.HasSuffix(id, ".yaml") { - // ignore bundle - continue - } - id = strings.TrimSuffix(id, ".key") - - privateKey, err := c.loadOnePrivateKey(f) - if err != nil { - return nil, fmt.Errorf("error loading private key %q: %v", f, err) - } - keys.items[id] = &keysetItem{ - id: id, - privateKey: privateKey, - } - } - - if len(keys.items) == 0 { - return nil, nil - } - - keys.primary = keys.findPrimary() +func (c *VFSCAStore) loadPrivateKeys(p vfs.Path) (*keyset, error) { + bundlePath := p.Join("keyset.yaml") + bundle, err := c.loadKeysetBundle(bundlePath) - return keys, nil -} - -func (c *VFSCAStore) loadOnePrivateKey(p vfs.Path) (*pki.PrivateKey, error) { - data, err := p.ReadFile() - if err != nil { - if os.IsNotExist(err) { - return nil, nil - } - return nil, err - } - k, err := pki.ParsePEMPrivateKey(data) - if err != nil { - return nil, fmt.Errorf("error parsing private key from %q: %v", p, err) - } - return k, err + return bundle, err } func (c *VFSCAStore) findPrivateKeyset(id string) (*keyset, error) { @@ -816,7 +693,7 @@ func (c *VFSCAStore) findPrivateKeyset(id string) (*keyset, error) { } else { var err error p := c.buildPrivateKeyPoolPath(id) - keys, err = c.loadPrivateKeys(p, true) + keys, err = c.loadPrivateKeys(p) if err != nil { return nil, err } @@ -871,7 +748,7 @@ func (c *VFSCAStore) storePrivateKey(name string, ki *keysetItem) error { // Write the bundle { p := c.buildPrivateKeyPoolPath(name) - ks, err := c.loadPrivateKeys(p, false) + ks, err := c.loadPrivateKeys(p) if err != nil { return err } @@ -889,6 +766,7 @@ func (c *VFSCAStore) storePrivateKey(name string, ki *keysetItem) error { } } + // TODO stop writing and remove legacy format files after rollback to pre-kops 1.18 not needed // Write the data { var data bytes.Buffer @@ -913,7 +791,7 @@ func (c *VFSCAStore) storeCertificate(name string, ki *keysetItem) error { // Write the bundle { p := c.buildCertificatePoolPath(name) - ks, err := c.loadCertificates(p, false) + ks, err := c.loadCertificates(p) if err != nil { return err } @@ -931,6 +809,7 @@ func (c *VFSCAStore) storeCertificate(name string, ki *keysetItem) error { } } + // TODO stop writing and remove legacy format files after rollback to pre-kops 1.18 not needed // Write the data { var data bytes.Buffer @@ -948,10 +827,19 @@ func (c *VFSCAStore) storeCertificate(name string, ki *keysetItem) error { } func (c *VFSCAStore) deletePrivateKey(name string, id string) (bool, error) { + // Delete the file itself + { + + p := c.buildPrivateKeyPath(name, id) + if err := p.Remove(); err != nil && !os.IsNotExist(err) { + return false, err + } + } + // Update the bundle { p := c.buildPrivateKeyPoolPath(name) - ks, err := c.loadPrivateKeys(p, false) + ks, err := c.loadPrivateKeys(p) if err != nil { return false, err } @@ -966,22 +854,22 @@ func (c *VFSCAStore) deletePrivateKey(name string, id string) (bool, error) { } } + return true, nil +} + +func (c *VFSCAStore) deleteCertificate(name string, id string) (bool, error) { // Delete the file itself { - - p := c.buildPrivateKeyPath(name, id) - if err := p.Remove(); err != nil { + p := c.buildCertificatePath(name, id) + if err := p.Remove(); err != nil && !os.IsNotExist(err) { return false, err } - return true, nil } -} -func (c *VFSCAStore) deleteCertificate(name string, id string) (bool, error) { // Update the bundle { p := c.buildCertificatePoolPath(name) - ks, err := c.loadCertificates(p, false) + ks, err := c.loadCertificates(p) if err != nil { return false, err } @@ -996,14 +884,7 @@ func (c *VFSCAStore) deleteCertificate(name string, id string) (bool, error) { } } - // Delete the file itself - { - p := c.buildCertificatePath(name, id) - if err := p.Remove(); err != nil { - return false, err - } - return true, nil - } + return true, nil } // AddSSHPublicKey stores an SSH public key