Remove support for reading legacy-format keypairs

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!)
+			f.clientset = vfsclientset.NewVFSClientset(basePath)
-			allowVFSList := true
-
-			f.clientset = vfsclientset.NewVFSClientset(basePath, allowVFSList)
 		}
 		if strings.HasPrefix(registryPath, "file://") {
 			klog.Warning("The local filesystem state store is not functional for running clusters")

examples/kops-api-example/apply.go

@@ -24,8 +24,7 @@ import (
 )
 
 func apply(ctx context.Context) error {
-	allowList := true
+	clientset := vfsclientset.NewVFSClientset(registryBase)
-	clientset := vfsclientset.NewVFSClientset(registryBase, allowList)
 
 	cluster, err := clientset.GetCluster(ctx, clusterName)
 	if err != nil {

examples/kops-api-example/up.go

@@ -30,8 +30,7 @@ import (
 )
 
 func up(ctx context.Context) error {
-	allowList := true
+	clientset := vfsclientset.NewVFSClientset(registryBase)
-	clientset := vfsclientset.NewVFSClientset(registryBase, allowList)
 
 	cluster := &api.Cluster{}
 	cluster.ObjectMeta.Name = clusterName

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

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")
 }
 

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)
 }
 

pkg/client/simple/vfsclientset/clientset.go

@@ -32,8 +32,7 @@ import (
 )
 
 type VFSClientset struct {
-	basePath  vfs.Path
+	basePath vfs.Path
-	allowList bool
 }
 
 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,
+		basePath: basePath,
-		allowList: allowList,
 	}
 	return vfsClientset
 }

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
 					},
 				},

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,
 			})
 		}
 	}

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
 ---

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
 ---

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
 ---

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
 ---

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,
 		})
 	}
 

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",
-    ],
-)

tests/keypair_test.go

@@ -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]))
-			}
-		}
-	}
-}

upup/pkg/fi/ca.go

@@ -33,11 +33,8 @@ const (
 	SecretNameSSHPrimary = "admin"
 )
 
-type KeysetFormat string
-
 const (
-	KeysetFormatLegacy   KeysetFormat = "legacy"
+	keysetFormatLatest = "v1alpha2"
-	KeysetFormatV1Alpha2 KeysetFormat = "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
+	// This func returns a cert, private key and a bool.  The bool value is whether the keypair is stored
-	// an empty string, which denotes a Legacy Keypair, or a value of "Keypair".  This string is used by a keypair
+	// in a legacy format. This bool is used by a keypair
-	// task convert a Legacy Keypair to the new Keypair API format.
+	// task to convert a Legacy Keypair to the new Keypair API format.
-	FindKeypair(name string) (*pki.Certificate, *pki.PrivateKey, KeysetFormat, error)
+	FindKeypair(name string) (*pki.Certificate, *pki.PrivateKey, bool, error)
 
 	CreateKeypair(signer string, name string, template *x509.Certificate, privateKey *pki.PrivateKey) (*pki.Certificate, error)
 

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
+	legacyFormat bool
-	items   map[string]*keysetItem
+	items        map[string]*keysetItem
-	primary *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

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 {

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)
 }
 

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
+	// LegacyFormat is whether the keypair is stored in a legacy format.
-	// is accessing legacy secrets that do not use keyset.yaml.
+	LegacyFormat bool `json:"oldFormat"`
-	Format string `json:"format"`
 }
 
 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

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")
 	}

upup/pkg/fi/vfs_castore.go

@@ -38,9 +38,8 @@ import (
 )
 
 type VFSCAStore struct {
-	basedir   vfs.Path
+	basedir vfs.Path
-	cluster   *kops.Cluster
+	cluster *kops.Cluster
-	allowList bool
 
 	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) {
+func (c *VFSCAStore) loadCertificates(p vfs.Path) (*keyset, error) {
-	// Attempt to load prebuilt bundle, which avoids having to list files, which is a permission that can be hard to
+	bundlePath := p.Join("keyset.yaml")
-	// give on GCE / other clouds
+	bundle, err := c.loadKeysetBundle(bundlePath)
-	if useBundle {
+	return bundle, err
-		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) 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) {
+func (c *VFSCAStore) FindKeypair(id string) (*pki.Certificate, *pki.PrivateKey, bool, error) {
-	cert, certFormat, err := c.findCert(id)
+	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) {
+func (c *VFSCAStore) loadPrivateKeys(p vfs.Path) (*keyset, error) {
-	// Attempt to load prebuilt bundle, which avoids having to list files, which is a permission that can be hard to
+	bundlePath := p.Join("keyset.yaml")
-	// give on GCE / other clouds
+	bundle, err := c.loadKeysetBundle(bundlePath)
-	if useBundle {
-		bundlePath := p.Join("keyset.yaml")
-		bundle, err := c.loadKeysetBundle(bundlePath)
 
-		if !c.allowList {
+	return bundle, err
-			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()
-
-	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
 }
 
 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) {
 		}
 	}
 
-	// Delete the file itself
+	return true, nil
-	{
-
-		p := c.buildPrivateKeyPath(name, id)
-		if err := p.Remove(); err != nil {
-			return false, err
-		}
-		return true, nil
-	}
 }
 
 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 && !os.IsNotExist(err) {
+			return false, err
+		}
+	}
+
 	// 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
+	return true, nil
-	{
-		p := c.buildCertificatePath(name, id)
-		if err := p.Remove(); err != nil {
-			return false, err
-		}
-		return true, nil
-	}
 }
 
 // AddSSHPublicKey stores an SSH public key