diff --git a/pkg/builder/secret_builder.go b/pkg/builder/secret_builder.go index 405ef33cb..37f9df8a6 100644 --- a/pkg/builder/secret_builder.go +++ b/pkg/builder/secret_builder.go @@ -1,5 +1,5 @@ /* -Copyright 2019 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -55,3 +55,9 @@ func (b *SecretBuilder) ObjectMeta(opts ...ObjectMetaOpt) *SecretBuilder { return b } + +// Data sets the Secret data. +func (b *SecretBuilder) Data(data map[string][]byte) *SecretBuilder { + b.object.Data = data + return b +} diff --git a/pkg/builder/secret_key_selector_builder.go b/pkg/builder/secret_key_selector_builder.go new file mode 100644 index 000000000..cb82ee78a --- /dev/null +++ b/pkg/builder/secret_key_selector_builder.go @@ -0,0 +1,43 @@ +/* +Copyright the Velero contributors. + +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 builder + +import ( + corev1api "k8s.io/api/core/v1" +) + +// SecretKeySelectorBuilder builds SecretKeySelector objects. +type SecretKeySelectorBuilder struct { + object *corev1api.SecretKeySelector +} + +// ForSecretKeySelector is the constructor for a SecretKeySelectorBuilder. +func ForSecretKeySelector(name string, key string) *SecretKeySelectorBuilder { + return &SecretKeySelectorBuilder{ + object: &corev1api.SecretKeySelector{ + LocalObjectReference: corev1api.LocalObjectReference{ + Name: name, + }, + Key: key, + }, + } +} + +// Result returns the built SecretKeySelector. +func (b *SecretKeySelectorBuilder) Result() *corev1api.SecretKeySelector { + return b.object +} diff --git a/pkg/cmd/cli/backuplocation/create.go b/pkg/cmd/cli/backuplocation/create.go index 64d0007cd..a541d7589 100644 --- a/pkg/cmd/cli/backuplocation/create.go +++ b/pkg/cmd/cli/backuplocation/create.go @@ -27,12 +27,12 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" - corev1api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kbclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/builder" "github.com/vmware-tanzu/velero/pkg/client" "github.com/vmware-tanzu/velero/pkg/cmd" "github.com/vmware-tanzu/velero/pkg/cmd/util/flag" @@ -178,13 +178,8 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { CACert: caCertData, }, }, - Config: o.Config.Data(), - Credential: &corev1api.SecretKeySelector{ - LocalObjectReference: corev1api.LocalObjectReference{ - Name: secretName, - }, - Key: secretKey, - }, + Config: o.Config.Data(), + Credential: builder.ForSecretKeySelector(secretName, secretKey).Result(), Default: o.DefaultBackupStorageLocation, AccessMode: velerov1api.BackupStorageLocationAccessMode(o.AccessMode.String()), BackupSyncPeriod: backupSyncPeriod, diff --git a/pkg/cmd/cli/backuplocation/set.go b/pkg/cmd/cli/backuplocation/set.go index 90d0bd061..dda731ac6 100644 --- a/pkg/cmd/cli/backuplocation/set.go +++ b/pkg/cmd/cli/backuplocation/set.go @@ -25,11 +25,11 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" - corev1api "k8s.io/api/core/v1" kbclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/builder" "github.com/vmware-tanzu/velero/pkg/client" "github.com/vmware-tanzu/velero/pkg/cmd" "github.com/vmware-tanzu/velero/pkg/cmd/util/flag" @@ -139,13 +139,8 @@ func (o *SetOptions) Run(c *cobra.Command, f client.Factory) error { location.Spec.Default = o.DefaultBackupStorageLocation location.Spec.StorageType.ObjectStorage.CACert = caCertData - for k, v := range o.Credential.Data() { - location.Spec.Credential = &corev1api.SecretKeySelector{ - LocalObjectReference: corev1api.LocalObjectReference{ - Name: k, - }, - Key: v, - } + for name, key := range o.Credential.Data() { + location.Spec.Credential = builder.ForSecretKeySelector(name, key).Result() break } diff --git a/pkg/cmd/cli/restic/server.go b/pkg/cmd/cli/restic/server.go index 1921cb72e..c3cd57c0c 100644 --- a/pkg/cmd/cli/restic/server.go +++ b/pkg/cmd/cli/restic/server.go @@ -1,5 +1,5 @@ /* -Copyright 2020 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -49,7 +49,6 @@ import ( "github.com/vmware-tanzu/velero/pkg/controller" clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned" informers "github.com/vmware-tanzu/velero/pkg/generated/informers/externalversions" - "github.com/vmware-tanzu/velero/pkg/restic" "github.com/vmware-tanzu/velero/pkg/util/filesystem" "github.com/vmware-tanzu/velero/pkg/util/logging" @@ -102,7 +101,6 @@ type resticServer struct { veleroInformerFactory informers.SharedInformerFactory kubeInformerFactory kubeinformers.SharedInformerFactory podInformer cache.SharedIndexInformer - secretInformer cache.SharedIndexInformer logger logrus.FieldLogger ctx context.Context cancelFunc context.CancelFunc @@ -136,22 +134,6 @@ func newResticServer(logger logrus.FieldLogger, factory client.Factory, metricAd }, ) - // use a stand-alone secrets informer so we can filter to only the restic credentials - // secret(s) within the velero namespace - // - // note: using an informer to access the single secret for all velero-managed - // restic repositories is overkill for now, but will be useful when we move - // to fully-encrypted backups and have unique keys per repository. - secretInformer := corev1informers.NewFilteredSecretInformer( - kubeClient, - factory.Namespace(), - 0, - cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}, - func(opts *metav1.ListOptions) { - opts.FieldSelector = fmt.Sprintf("metadata.name=%s", restic.CredentialsSecretName) - }, - ) - ctx, cancelFunc := context.WithCancel(context.Background()) clientConfig, err := factory.ClientConfig() @@ -160,7 +142,9 @@ func newResticServer(logger logrus.FieldLogger, factory client.Factory, metricAd } ctrl.SetLogger(zap.New(zap.UseDevMode(true))) + velerov1api.AddToScheme(scheme) + v1.AddToScheme(scheme) mgr, err := ctrl.NewManager(clientConfig, ctrl.Options{ Scheme: scheme, }) @@ -174,7 +158,6 @@ func newResticServer(logger logrus.FieldLogger, factory client.Factory, metricAd veleroInformerFactory: informers.NewFilteredSharedInformerFactory(veleroClient, 0, factory.Namespace(), nil), kubeInformerFactory: kubeinformers.NewSharedInformerFactory(kubeClient, 0), podInformer: podInformer, - secretInformer: secretInformer, logger: logger, ctx: ctx, cancelFunc: cancelFunc, @@ -212,7 +195,6 @@ func (s *resticServer) run() { s.veleroInformerFactory.Velero().V1().PodVolumeBackups(), s.veleroClient.VeleroV1(), s.podInformer, - s.secretInformer, s.kubeInformerFactory.Core().V1().PersistentVolumeClaims(), s.kubeInformerFactory.Core().V1().PersistentVolumes(), s.metrics, @@ -225,7 +207,6 @@ func (s *resticServer) run() { s.veleroInformerFactory.Velero().V1().PodVolumeRestores(), s.veleroClient.VeleroV1(), s.podInformer, - s.secretInformer, s.kubeInformerFactory.Core().V1().PersistentVolumeClaims(), s.kubeInformerFactory.Core().V1().PersistentVolumes(), s.mgr.GetClient(), @@ -235,7 +216,6 @@ func (s *resticServer) run() { go s.veleroInformerFactory.Start(s.ctx.Done()) go s.kubeInformerFactory.Start(s.ctx.Done()) go s.podInformer.Run(s.ctx.Done()) - go s.secretInformer.Run(s.ctx.Done()) // TODO(2.0): presuming all controllers and resources are converted to runtime-controller // by v2.0, the block from this line and including the `s.mgr.Start() will be diff --git a/pkg/cmd/cli/snapshotlocation/create.go b/pkg/cmd/cli/snapshotlocation/create.go index c05add127..940c8a2cc 100644 --- a/pkg/cmd/cli/snapshotlocation/create.go +++ b/pkg/cmd/cli/snapshotlocation/create.go @@ -24,10 +24,10 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" - corev1api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/builder" "github.com/vmware-tanzu/velero/pkg/client" "github.com/vmware-tanzu/velero/pkg/cmd" "github.com/vmware-tanzu/velero/pkg/cmd/util/flag" @@ -113,14 +113,9 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { Labels: o.Labels.Data(), }, Spec: api.VolumeSnapshotLocationSpec{ - Provider: o.Provider, - Config: o.Config.Data(), - Credential: &corev1api.SecretKeySelector{ - LocalObjectReference: corev1api.LocalObjectReference{ - Name: o.secretName, - }, - Key: o.secretKey, - }, + Provider: o.Provider, + Config: o.Config.Data(), + Credential: builder.ForSecretKeySelector(o.secretName, o.secretKey).Result(), }, } diff --git a/pkg/cmd/cli/snapshotlocation/set.go b/pkg/cmd/cli/snapshotlocation/set.go index 383f9d6d2..f6b8ac368 100644 --- a/pkg/cmd/cli/snapshotlocation/set.go +++ b/pkg/cmd/cli/snapshotlocation/set.go @@ -23,11 +23,11 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" - corev1api "k8s.io/api/core/v1" kbclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/builder" "github.com/vmware-tanzu/velero/pkg/client" "github.com/vmware-tanzu/velero/pkg/cmd" "github.com/vmware-tanzu/velero/pkg/cmd/util/flag" @@ -99,13 +99,8 @@ func (o *SetOptions) Run(c *cobra.Command, f client.Factory) error { return errors.WithStack(err) } - for k, v := range o.Credential.Data() { - location.Spec.Credential = &corev1api.SecretKeySelector{ - LocalObjectReference: corev1api.LocalObjectReference{ - Name: k, - }, - Key: v, - } + for name, key := range o.Credential.Data() { + location.Spec.Credential = builder.ForSecretKeySelector(name, key).Result() break } diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 56bbf7a9d..08d80a645 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -1,5 +1,5 @@ /* -Copyright 2020 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -31,6 +31,7 @@ import ( "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/sirupsen/logrus" "github.com/spf13/cobra" + corev1api "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -40,10 +41,8 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" - corev1informers "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" - "k8s.io/client-go/tools/cache" snapshotv1beta1api "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1beta1" snapshotv1beta1client "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned" @@ -286,6 +285,8 @@ func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*s scheme := runtime.NewScheme() velerov1api.AddToScheme(scheme) + corev1api.AddToScheme(scheme) + mgr, err := ctrl.NewManager(clientConfig, ctrl.Options{ Scheme: scheme, }) @@ -483,28 +484,10 @@ func (s *server) initRestic() error { return err } - // use a stand-alone secrets informer so we can filter to only the restic credentials - // secret(s) within the velero namespace - // - // note: using an informer to access the single secret for all velero-managed - // restic repositories is overkill for now, but will be useful when we move - // to fully-encrypted backups and have unique keys per repository. - secretsInformer := corev1informers.NewFilteredSecretInformer( - s.kubeClient, - s.namespace, - 0, - cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}, - func(opts *metav1.ListOptions) { - opts.FieldSelector = fmt.Sprintf("metadata.name=%s", restic.CredentialsSecretName) - }, - ) - go secretsInformer.Run(s.ctx.Done()) - res, err := restic.NewRepositoryManager( s.ctx, s.namespace, s.veleroClient, - secretsInformer, s.sharedInformerFactory.Velero().V1().ResticRepositories(), s.veleroClient.VeleroV1(), s.mgr.GetClient(), diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 77564d2a8..16fc6c1a0 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2020 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -181,9 +181,9 @@ func TestProcessBackupValidationFailures(t *testing.T) { var fakeClient kbclient.Client if test.backupLocation != nil { - fakeClient = newFakeClient(t, test.backupLocation) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t, test.backupLocation) } else { - fakeClient = newFakeClient(t) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) } c := &backupController{ @@ -246,7 +246,7 @@ func TestBackupLocationLabel(t *testing.T) { clientset = fake.NewSimpleClientset(test.backup) sharedInformers = informers.NewSharedInformerFactory(clientset, 0) logger = logging.DefaultLogger(logrus.DebugLevel, formatFlag) - fakeClient = newFakeClient(t) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) ) apiServer := velerotest.NewAPIServer(t) @@ -306,7 +306,7 @@ func TestDefaultBackupTTL(t *testing.T) { formatFlag := logging.FormatText var ( clientset = fake.NewSimpleClientset(test.backup) - fakeClient = newFakeClient(t) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) logger = logging.DefaultLogger(logrus.DebugLevel, formatFlag) sharedInformers = informers.NewSharedInformerFactory(clientset, 0) ) @@ -783,9 +783,9 @@ func TestProcessBackupCompletions(t *testing.T) { var fakeClient kbclient.Client // add the test's backup storage location if it's different than the default if test.backupLocation != nil && test.backupLocation != defaultBackupLocation { - fakeClient = newFakeClient(t, test.backupLocation) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t, test.backupLocation) } else { - fakeClient = newFakeClient(t) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) } apiServer := velerotest.NewAPIServer(t) diff --git a/pkg/controller/backup_deletion_controller_test.go b/pkg/controller/backup_deletion_controller_test.go index 210e05a0b..3a36aafa0 100644 --- a/pkg/controller/backup_deletion_controller_test.go +++ b/pkg/controller/backup_deletion_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2020 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -144,7 +144,7 @@ func setupBackupDeletionControllerTest(t *testing.T, objects ...runtime.Object) var ( client = fake.NewSimpleClientset(append(objects, req)...) - fakeClient = newFakeClient(t, objects...) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t, objects...) sharedInformers = informers.NewSharedInformerFactory(client, 0) volumeSnapshotter = &velerotest.FakeVolumeSnapshotter{SnapshotsTaken: sets.NewString()} pluginManager = &pluginmocks.Manager{} @@ -1112,7 +1112,7 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { client := fake.NewSimpleClientset() - fakeClient := newFakeClient(t) + fakeClient := velerotest.NewFakeControllerRuntimeClient(t) sharedInformers := informers.NewSharedInformerFactory(client, 0) controller := NewBackupDeletionController( diff --git a/pkg/controller/backup_sync_controller_test.go b/pkg/controller/backup_sync_controller_test.go index a55de5e0f..19375c622 100644 --- a/pkg/controller/backup_sync_controller_test.go +++ b/pkg/controller/backup_sync_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017, 2020 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -335,7 +335,7 @@ func TestBackupSyncControllerRun(t *testing.T) { t.Run(test.name, func(t *testing.T) { var ( client = fake.NewSimpleClientset() - fakeClient = newFakeClient(t) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) sharedInformers = informers.NewSharedInformerFactory(client, 0) pluginManager = &pluginmocks.Manager{} backupStores = make(map[string]*persistencemocks.BackupStore) @@ -558,7 +558,7 @@ func TestDeleteOrphanedBackups(t *testing.T) { t.Run(test.name, func(t *testing.T) { var ( client = fake.NewSimpleClientset() - fakeClient = newFakeClient(t) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) sharedInformers = informers.NewSharedInformerFactory(client, 0) ) @@ -652,7 +652,7 @@ func TestStorageLabelsInDeleteOrphanedBackups(t *testing.T) { t.Run(test.name, func(t *testing.T) { var ( client = fake.NewSimpleClientset() - fakeClient = newFakeClient(t) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) sharedInformers = informers.NewSharedInformerFactory(client, 0) ) diff --git a/pkg/controller/download_request_controller_test.go b/pkg/controller/download_request_controller_test.go index 21363e24c..e00d2bb74 100644 --- a/pkg/controller/download_request_controller_test.go +++ b/pkg/controller/download_request_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -246,9 +246,9 @@ func TestProcessDownloadRequest(t *testing.T) { t.Run(tc.name, func(t *testing.T) { var fakeClient client.Client if tc.backupLocation != nil { - fakeClient = newFakeClient(t, tc.backupLocation) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t, tc.backupLocation) } else { - fakeClient = newFakeClient(t) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) } harness := newDownloadRequestTestHarness(t, fakeClient) diff --git a/pkg/controller/gc_controller_test.go b/pkg/controller/gc_controller_test.go index 7663b8951..3597a3c8c 100644 --- a/pkg/controller/gc_controller_test.go +++ b/pkg/controller/gc_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017, 2019 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -251,9 +251,9 @@ func TestGCControllerProcessQueueItem(t *testing.T) { var fakeClient kbclient.Client if test.backupLocation != nil { - fakeClient = newFakeClient(t, test.backupLocation) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t, test.backupLocation) } else { - fakeClient = newFakeClient(t) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) } controller := NewGCController( diff --git a/pkg/controller/pod_volume_backup_controller.go b/pkg/controller/pod_volume_backup_controller.go index 4bf653c4c..25acb38e9 100644 --- a/pkg/controller/pod_volume_backup_controller.go +++ b/pkg/controller/pod_volume_backup_controller.go @@ -1,5 +1,5 @@ /* -Copyright 2018, 2019 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -54,7 +54,6 @@ type podVolumeBackupController struct { podVolumeBackupClient velerov1client.PodVolumeBackupsGetter podVolumeBackupLister listers.PodVolumeBackupLister - secretLister corev1listers.SecretLister podLister corev1listers.PodLister pvcLister corev1listers.PersistentVolumeClaimLister pvLister corev1listers.PersistentVolumeLister @@ -73,7 +72,6 @@ func NewPodVolumeBackupController( podVolumeBackupInformer informers.PodVolumeBackupInformer, podVolumeBackupClient velerov1client.PodVolumeBackupsGetter, podInformer cache.SharedIndexInformer, - secretInformer cache.SharedIndexInformer, pvcInformer corev1informers.PersistentVolumeClaimInformer, pvInformer corev1informers.PersistentVolumeInformer, metrics *metrics.ServerMetrics, @@ -85,7 +83,6 @@ func NewPodVolumeBackupController( podVolumeBackupClient: podVolumeBackupClient, podVolumeBackupLister: podVolumeBackupInformer.Lister(), podLister: corev1listers.NewPodLister(podInformer.GetIndexer()), - secretLister: corev1listers.NewSecretLister(secretInformer.GetIndexer()), pvcLister: pvcInformer.Lister(), pvLister: pvInformer.Lister(), kbClient: kbClient, @@ -101,7 +98,6 @@ func NewPodVolumeBackupController( c.cacheSyncWaiters, podVolumeBackupInformer.Informer().HasSynced, podInformer.HasSynced, - secretInformer.HasSynced, pvcInformer.Informer().HasSynced, ) c.processBackupFunc = c.processBackup @@ -225,7 +221,7 @@ func (c *podVolumeBackupController) processBackup(req *velerov1api.PodVolumeBack log.WithField("path", path).Debugf("Found path matching glob") // temp creds - credentialsFile, err := restic.TempCredentialsFile(c.secretLister, req.Namespace, req.Spec.Pod.Namespace, c.fileSystem) + credentialsFile, err := restic.TempCredentialsFile(c.kbClient, req.Namespace, req.Spec.Pod.Namespace, c.fileSystem) if err != nil { log.WithError(err).Error("Error creating temp restic credentials file") return c.fail(req, errors.Wrap(err, "error creating temp restic credentials file").Error(), log) diff --git a/pkg/controller/pod_volume_restore_controller.go b/pkg/controller/pod_volume_restore_controller.go index 96d0e03a2..16876c072 100644 --- a/pkg/controller/pod_volume_restore_controller.go +++ b/pkg/controller/pod_volume_restore_controller.go @@ -1,5 +1,5 @@ /* -Copyright 2020 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -56,7 +56,6 @@ type podVolumeRestoreController struct { podVolumeRestoreClient velerov1client.PodVolumeRestoresGetter podVolumeRestoreLister listers.PodVolumeRestoreLister podLister corev1listers.PodLister - secretLister corev1listers.SecretLister pvcLister corev1listers.PersistentVolumeClaimLister pvLister corev1listers.PersistentVolumeLister backupLocationInformer k8scache.Informer @@ -74,7 +73,6 @@ func NewPodVolumeRestoreController( podVolumeRestoreInformer informers.PodVolumeRestoreInformer, podVolumeRestoreClient velerov1client.PodVolumeRestoresGetter, podInformer cache.SharedIndexInformer, - secretInformer cache.SharedIndexInformer, pvcInformer corev1informers.PersistentVolumeClaimInformer, pvInformer corev1informers.PersistentVolumeInformer, kbClient client.Client, @@ -85,7 +83,6 @@ func NewPodVolumeRestoreController( podVolumeRestoreClient: podVolumeRestoreClient, podVolumeRestoreLister: podVolumeRestoreInformer.Lister(), podLister: corev1listers.NewPodLister(podInformer.GetIndexer()), - secretLister: corev1listers.NewSecretLister(secretInformer.GetIndexer()), pvcLister: pvcInformer.Lister(), pvLister: pvInformer.Lister(), kbClient: kbClient, @@ -100,7 +97,6 @@ func NewPodVolumeRestoreController( c.cacheSyncWaiters, podVolumeRestoreInformer.Informer().HasSynced, podInformer.HasSynced, - secretInformer.HasSynced, pvcInformer.Informer().HasSynced, ) c.processRestoreFunc = c.processRestore @@ -304,7 +300,7 @@ func (c *podVolumeRestoreController) processRestore(req *velerov1api.PodVolumeRe return c.failRestore(req, errors.Wrap(err, "error getting volume directory name").Error(), log) } - credsFile, err := restic.TempCredentialsFile(c.secretLister, req.Namespace, req.Spec.Pod.Namespace, c.fileSystem) + credsFile, err := restic.TempCredentialsFile(c.kbClient, req.Namespace, req.Spec.Pod.Namespace, c.fileSystem) if err != nil { log.WithError(err).Error("Error creating temp restic credentials file") return c.failRestore(req, errors.Wrap(err, "error creating temp restic credentials file").Error(), log) diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 618c388ca..689ebf462 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2020 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -94,7 +94,7 @@ func TestFetchBackupInfo(t *testing.T) { t.Run(test.name, func(t *testing.T) { var ( client = fake.NewSimpleClientset() - fakeClient = newFakeClient(t) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) restorer = &fakeRestorer{} sharedInformers = informers.NewSharedInformerFactory(client, 0) logger = velerotest.NewLogger() @@ -409,7 +409,7 @@ func TestProcessQueueItem(t *testing.T) { t.Run(test.name, func(t *testing.T) { var ( client = fake.NewSimpleClientset() - fakeClient = newFakeClient(t) + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) restorer = &fakeRestorer{} sharedInformers = informers.NewSharedInformerFactory(client, 0) logger = velerotest.NewLogger() diff --git a/pkg/controller/suite_test.go b/pkg/controller/suite_test.go index 007d38edb..d4c07b995 100644 --- a/pkg/controller/suite_test.go +++ b/pkg/controller/suite_test.go @@ -1,5 +1,5 @@ /* -Copyright 2020 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -27,17 +27,13 @@ import ( "github.com/vmware-tanzu/velero/pkg/persistence" persistencemocks "github.com/vmware-tanzu/velero/pkg/persistence/mocks" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/klog" - "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/manager" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/stretchr/testify/require" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "k8s.io/client-go/kubernetes/scheme" @@ -98,6 +94,7 @@ type testEnvironment struct { // This function should be called only once for each package you're running tests within, // usually the environment is initialized in a suite_test.go file within a `BeforeSuite` ginkgo block. func newTestEnvironment() *testEnvironment { + // scheme.Scheme is initialized with all native Kubernetes types err := velerov1api.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) @@ -133,12 +130,6 @@ func (t *testEnvironment) stop() error { return env.Stop() } -func newFakeClient(t *testing.T, initObjs ...runtime.Object) client.Client { - err := velerov1api.AddToScheme(scheme.Scheme) - require.NoError(t, err) - return fake.NewFakeClientWithScheme(scheme.Scheme, initObjs...) -} - type fakeSingleObjectBackupStoreGetter struct { store persistence.BackupStore } diff --git a/pkg/restic/common.go b/pkg/restic/common.go index 4c98ea427..592f3a3b7 100644 --- a/pkg/restic/common.go +++ b/pkg/restic/common.go @@ -1,5 +1,5 @@ /* -Copyright 2018 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -29,12 +29,13 @@ import ( corev1api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - corev1listers "k8s.io/client-go/listers/core/v1" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/builder" velerov1listers "github.com/vmware-tanzu/velero/pkg/generated/listers/velero/v1" "github.com/vmware-tanzu/velero/pkg/label" "github.com/vmware-tanzu/velero/pkg/util/filesystem" + "github.com/vmware-tanzu/velero/pkg/util/kube" ) const ( @@ -241,14 +242,14 @@ func GetSnapshotsInBackup(backup *velerov1api.Backup, podVolumeBackupLister vele // encryption key for the given repo and returns its path. The // caller should generally call os.Remove() to remove the file // when done with it. -func TempCredentialsFile(secretLister corev1listers.SecretLister, veleroNamespace, repoName string, fs filesystem.Interface) (string, error) { - secretGetter := NewListerSecretGetter(secretLister) - +func TempCredentialsFile(client kbclient.Client, veleroNamespace, repoName string, fs filesystem.Interface) (string, error) { // For now, all restic repos share the same key so we don't need the repoName to fetch it. // When we move to full-backup encryption, we'll likely have a separate key per restic repo - // (all within the Velero server's namespace) so GetRepositoryKey will need to take the repo - // name as an argument as well. - repoKey, err := GetRepositoryKey(secretGetter, veleroNamespace) + // (all within the Velero server's namespace) so repoKeySelector will need to select the key + // for that repo. + repoKeySelector := builder.ForSecretKeySelector(CredentialsSecretName, CredentialsKey).Result() + + repoKey, err := kube.GetSecretKey(client, veleroNamespace, repoKeySelector) if err != nil { return "", err } @@ -261,7 +262,7 @@ func TempCredentialsFile(secretLister corev1listers.SecretLister, veleroNamespac if _, err := file.Write(repoKey); err != nil { // nothing we can do about an error closing the file here, and we're // already returning an error about the write failing. - file.Close() + _ = file.Close() return "", errors.WithStack(err) } diff --git a/pkg/restic/common_test.go b/pkg/restic/common_test.go index c29ee643f..cde5e9d37 100644 --- a/pkg/restic/common_test.go +++ b/pkg/restic/common_test.go @@ -1,5 +1,5 @@ /* -Copyright 2018 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -22,17 +22,10 @@ import ( "sort" "testing" - "k8s.io/apimachinery/pkg/runtime" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/scheme" - corev1listers "k8s.io/client-go/listers/core/v1" - "k8s.io/client-go/tools/cache" - "sigs.k8s.io/controller-runtime/pkg/client" - k8sfake "sigs.k8s.io/controller-runtime/pkg/client/fake" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/builder" @@ -352,10 +345,9 @@ func TestGetSnapshotsInBackup(t *testing.T) { func TestTempCredentialsFile(t *testing.T) { var ( - secretInformer = cache.NewSharedIndexInformer(nil, new(corev1api.Secret), 0, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}) - secretLister = corev1listers.NewSecretLister(secretInformer.GetIndexer()) - fs = velerotest.NewFakeFileSystem() - secret = &corev1api.Secret{ + fakeClient = velerotest.NewFakeControllerRuntimeClient(t) + fs = velerotest.NewFakeFileSystem() + secret = &corev1api.Secret{ ObjectMeta: metav1.ObjectMeta{ Namespace: "velero", Name: CredentialsSecretName, @@ -366,15 +358,15 @@ func TestTempCredentialsFile(t *testing.T) { } ) - // secret not in lister: expect an error - fileName, err := TempCredentialsFile(secretLister, "velero", "default", fs) + // secret not in server: expect an error + fileName, err := TempCredentialsFile(fakeClient, "velero", "default", fs) assert.Error(t, err) - // now add secret to lister - require.NoError(t, secretInformer.GetStore().Add(secret)) + // now add secret + require.NoError(t, fakeClient.Create(context.Background(), secret)) - // secret in lister: expect temp file to be created with password - fileName, err = TempCredentialsFile(secretLister, "velero", "default", fs) + // secret in server: expect temp file to be created with password + fileName, err = TempCredentialsFile(fakeClient, "velero", "default", fs) require.NoError(t, err) contents, err := fs.ReadFile(fileName) @@ -400,7 +392,7 @@ func TestTempCACertFile(t *testing.T) { } ) - fakeClient := newFakeClient(t) + fakeClient := velerotest.NewFakeControllerRuntimeClient(t) fakeClient.Create(context.Background(), bsl) // expect temp file to be created with cacert value @@ -645,9 +637,3 @@ func TestIsPVBMatchPod(t *testing.T) { } } - -func newFakeClient(t *testing.T, initObjs ...runtime.Object) client.Client { - err := velerov1api.AddToScheme(scheme.Scheme) - require.NoError(t, err) - return k8sfake.NewFakeClientWithScheme(scheme.Scheme, initObjs...) -} diff --git a/pkg/restic/repository_keys.go b/pkg/restic/repository_keys.go index 5d9527ccd..48f3e381e 100644 --- a/pkg/restic/repository_keys.go +++ b/pkg/restic/repository_keys.go @@ -1,5 +1,5 @@ /* -Copyright 2018 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -24,7 +24,6 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" - corev1listers "k8s.io/client-go/listers/core/v1" ) const ( @@ -62,53 +61,3 @@ func EnsureCommonRepositoryKey(secretClient corev1client.SecretsGetter, namespac return nil } - -type SecretGetter interface { - GetSecret(namespace, name string) (*corev1api.Secret, error) -} - -type clientSecretGetter struct { - client corev1client.SecretsGetter -} - -func NewClientSecretGetter(client corev1client.SecretsGetter) SecretGetter { - return &clientSecretGetter{client: client} -} - -func (c *clientSecretGetter) GetSecret(namespace, name string) (*corev1api.Secret, error) { - secret, err := c.client.Secrets(namespace).Get(context.TODO(), name, metav1.GetOptions{}) - if err != nil { - return nil, errors.WithStack(err) - } - return secret, nil -} - -type listerSecretGetter struct { - lister corev1listers.SecretLister -} - -func NewListerSecretGetter(lister corev1listers.SecretLister) SecretGetter { - return &listerSecretGetter{lister: lister} -} - -func (l *listerSecretGetter) GetSecret(namespace, name string) (*corev1api.Secret, error) { - secret, err := l.lister.Secrets(namespace).Get(name) - if err != nil { - return nil, errors.WithStack(err) - } - return secret, nil -} - -func GetRepositoryKey(secretGetter SecretGetter, namespace string) ([]byte, error) { - secret, err := secretGetter.GetSecret(namespace, CredentialsSecretName) - if err != nil { - return nil, err - } - - key, found := secret.Data[CredentialsKey] - if !found { - return nil, errors.Errorf("%q secret is missing data for key %q", CredentialsSecretName, CredentialsKey) - } - - return key, nil -} diff --git a/pkg/restic/repository_manager.go b/pkg/restic/repository_manager.go index 08b23ffd9..af01935e3 100644 --- a/pkg/restic/repository_manager.go +++ b/pkg/restic/repository_manager.go @@ -1,5 +1,5 @@ /* -Copyright 2018, 2019 the Velero contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -26,7 +26,6 @@ import ( "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" - corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" kbclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -83,7 +82,6 @@ type RestorerFactory interface { type repositoryManager struct { namespace string veleroClient clientset.Interface - secretsLister corev1listers.SecretLister repoLister velerov1listers.ResticRepositoryLister repoInformerSynced cache.InformerSynced kbClient kbclient.Client @@ -101,7 +99,6 @@ func NewRepositoryManager( ctx context.Context, namespace string, veleroClient clientset.Interface, - secretsInformer cache.SharedIndexInformer, repoInformer velerov1informers.ResticRepositoryInformer, repoClient velerov1client.ResticRepositoriesGetter, kbClient kbclient.Client, @@ -112,7 +109,6 @@ func NewRepositoryManager( rm := &repositoryManager{ namespace: namespace, veleroClient: veleroClient, - secretsLister: corev1listers.NewSecretLister(secretsInformer.GetIndexer()), repoLister: repoInformer.Lister(), repoInformerSynced: repoInformer.Informer().HasSynced, kbClient: kbClient, @@ -126,10 +122,6 @@ func NewRepositoryManager( fileSystem: filesystem.NewFileSystem(), } - if !cache.WaitForCacheSync(ctx.Done(), secretsInformer.HasSynced) { - return nil, errors.New("timed out waiting for cache to sync") - } - return rm, nil } @@ -235,7 +227,7 @@ func (rm *repositoryManager) Forget(ctx context.Context, snapshot SnapshotIdenti } func (rm *repositoryManager) exec(cmd *Command, backupLocation string) error { - file, err := TempCredentialsFile(rm.secretsLister, rm.namespace, cmd.RepoName(), rm.fileSystem) + file, err := TempCredentialsFile(rm.kbClient, rm.namespace, cmd.RepoName(), rm.fileSystem) if err != nil { return err } diff --git a/pkg/test/fake_controller_runtime_client.go b/pkg/test/fake_controller_runtime_client.go new file mode 100644 index 000000000..a1f20e9bf --- /dev/null +++ b/pkg/test/fake_controller_runtime_client.go @@ -0,0 +1,38 @@ +/* +Copyright the Velero contributors. + +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 test + +import ( + "testing" + + "github.com/stretchr/testify/require" + corev1api "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + k8sfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" +) + +func NewFakeControllerRuntimeClient(t *testing.T, initObjs ...runtime.Object) client.Client { + scheme := runtime.NewScheme() + err := velerov1api.AddToScheme(scheme) + require.NoError(t, err) + err = corev1api.AddToScheme(scheme) + require.NoError(t, err) + return k8sfake.NewFakeClientWithScheme(scheme, initObjs...) +} diff --git a/pkg/util/kube/secrets.go b/pkg/util/kube/secrets.go new file mode 100644 index 000000000..e9cb4c04c --- /dev/null +++ b/pkg/util/kube/secrets.go @@ -0,0 +1,51 @@ +/* +Copyright the Velero contributors. + +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 kube + +import ( + "context" + + "github.com/pkg/errors" + corev1api "k8s.io/api/core/v1" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +func GetSecret(client kbclient.Client, namespace, name string) (*corev1api.Secret, error) { + secret := &corev1api.Secret{} + if err := client.Get(context.TODO(), kbclient.ObjectKey{ + Namespace: namespace, + Name: name, + }, secret); err != nil { + return nil, err + } + + return secret, nil +} + +func GetSecretKey(client kbclient.Client, namespace string, selector *corev1api.SecretKeySelector) ([]byte, error) { + secret, err := GetSecret(client, namespace, selector.Name) + if err != nil { + return nil, err + } + + key, found := secret.Data[selector.Key] + if !found { + return nil, errors.Errorf("%q secret is missing data for key %q", selector.Name, selector.Key) + } + + return key, nil +} diff --git a/pkg/util/kube/secrets_test.go b/pkg/util/kube/secrets_test.go new file mode 100644 index 000000000..61c6f355b --- /dev/null +++ b/pkg/util/kube/secrets_test.go @@ -0,0 +1,97 @@ +/* +Copyright the Velero contributors. + +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 kube + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + corev1api "k8s.io/api/core/v1" + + "github.com/vmware-tanzu/velero/pkg/builder" + velerotest "github.com/vmware-tanzu/velero/pkg/test" +) + +func TestGetSecretKey(t *testing.T) { + testCases := []struct { + name string + secrets []*corev1api.Secret + namespace string + selector *corev1api.SecretKeySelector + expectedData string + expectedErr string + }{ + { + name: "error is returned when secret doesn't exist", + secrets: []*corev1api.Secret{ + builder.ForSecret("ns-1", "secret1").Data(map[string][]byte{ + "key1": []byte("ns1-secretdata1"), + }).Result(), + }, + namespace: "ns-1", + selector: builder.ForSecretKeySelector("non-existent-secret", "key2").Result(), + expectedErr: "secrets \"non-existent-secret\" not found", + }, + { + name: "error is returned when key is missing from secret", + secrets: []*corev1api.Secret{ + builder.ForSecret("ns-1", "secret1").Data(map[string][]byte{ + "key1": []byte("ns1-secretdata1"), + }).Result(), + }, + namespace: "ns-1", + selector: builder.ForSecretKeySelector("secret1", "non-existent-key").Result(), + expectedErr: "\"secret1\" secret is missing data for key \"non-existent-key\"", + }, + { + name: "key specified in selector is returned", + secrets: []*corev1api.Secret{ + builder.ForSecret("ns-1", "secret1").Data(map[string][]byte{ + "key1": []byte("ns1-secretdata1"), + "key2": []byte("ns1-secretdata2"), + }).Result(), + builder.ForSecret("ns-2", "secret1").Data(map[string][]byte{ + "key1": []byte("ns2-secretdata1"), + "key2": []byte("ns2-secretdata2"), + }).Result(), + }, + namespace: "ns-2", + selector: builder.ForSecretKeySelector("secret1", "key2").Result(), + expectedData: "ns2-secretdata2", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fakeClient := velerotest.NewFakeControllerRuntimeClient(t) + + for _, secret := range tc.secrets { + require.NoError(t, fakeClient.Create(context.Background(), secret)) + } + + data, err := GetSecretKey(fakeClient, tc.namespace, tc.selector) + if tc.expectedErr != "" { + require.Nil(t, data) + require.EqualError(t, err, tc.expectedErr) + } else { + require.NoError(t, err) + require.Equal(t, tc.expectedData, string(data)) + } + }) + } +}