mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-09-13 03:24:39 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge branch 'main' into use-pv-topology-for-block-data-mover-for-windows

Browse Source
This commit is contained in:
lyndon-li
2026-08-27 17:06:15 +08:00
committed by GitHub
parent f6aab3b144 836689f6cc
commit 72466dbd52
7 changed files with 152 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changelogs/unreleased/10353-PranjalManhgaye
+1
View File
@@ -0,0 +1 @@
Reject velero backup delete when backup storage location is read-only
changelogs/unreleased/10362-opbot-xd
+1
View File
@@ -0,0 +1 @@
Unify duplicate test harness structs across restore and delete tests into a shared pkg/test.Harness
internal/delete/delete_item_action_handler_test.go
+9 -43
View File
@@ -26,9 +26,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/builder"
@@ -154,9 +151,9 @@ func TestInvokeDeleteItemActionsRunForCorrectItems(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// test harness contains the fake API server/discovery client
h := newHarness(t)
h, dh := newHarness(t)
for _, r := range tc.apiResources {
h.addResource(t, r)
h.AddResource(t, dh, r)
}
// Get the plugins out of the map in order to use them.
@@ -169,7 +166,7 @@ func TestInvokeDeleteItemActionsRunForCorrectItems(t *testing.T) {
Backup: tc.backup,
BackupReader: tc.tarball,
Filesystem: fs,
DiscoveryHelper: h.discoveryHelper,
DiscoveryHelper: dh,
Actions: actions,
Log: log,
}
@@ -187,46 +184,15 @@ func TestInvokeDeleteItemActionsRunForCorrectItems(t *testing.T) {
}
}
// TODO: unify this with the test harness in pkg/restore/restore_test.go
type harness struct {
*test.APIServer
discoveryHelper discovery.Helper
}
func newHarness(t *testing.T) *harness {
func newHarness(t *testing.T) (*test.Harness, discovery.Helper) {
t.Helper()
apiServer := test.NewAPIServer(t)
log := logrus.StandardLogger()
discoveryHelper, err := discovery.NewHelper(apiServer.DiscoveryClient, log)
dh, err := discovery.NewHelper(apiServer.DiscoveryClient, log)
require.NoError(t, err)
return &harness{
APIServer: apiServer,
discoveryHelper: discoveryHelper,
}
}
// addResource adds an APIResource and it's items to a faked API server for testing.
func (h *harness) addResource(t *testing.T, resource *test.APIResource) {
t.Helper()
h.DiscoveryClient.WithAPIResource(resource)
require.NoError(t, h.discoveryHelper.Refresh())
for _, item := range resource.Items {
obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(item)
require.NoError(t, err)
unstructuredObj := &unstructured.Unstructured{Object: obj}
if resource.Namespaced {
_, err = h.DynamicClient.Resource(resource.GVR()).Namespace(item.GetNamespace()).Create(t.Context(), unstructuredObj, metav1.CreateOptions{})
} else {
_, err = h.DynamicClient.Resource(resource.GVR()).Create(t.Context(), unstructuredObj, metav1.CreateOptions{})
}
require.NoError(t, err)
}
return test.NewHarness(t, apiServer), dh
}
// recordResourcesAction is a delete item action that can be configured to run
@@ -306,14 +272,14 @@ func TestInvokeDeleteActionsReturnsPluginErrors(t *testing.T) {
action := &failingAction{err: errors.New("could not delete artifact")}
h := newHarness(t)
h.addResource(t, test.Pods())
h, dh := newHarness(t)
h.AddResource(t, dh, test.Pods())
c := &Context{
Backup: builder.ForBackup("velero", "velero").Result(),
BackupReader: tarball,
Filesystem: fs,
DiscoveryHelper: h.discoveryHelper,
DiscoveryHelper: dh,
Actions: []velero.DeleteItemAction{action},
Log: log,
}
pkg/cmd/cli/backup/delete.go
+22
View File
@@ -121,7 +121,29 @@ func Run(o *cli.DeleteOptions) error {
}
// create a backup deletion request for each
bslCache := make(map[string]*velerov1api.BackupStorageLocation)
for _, b := range backups {
storageLocationName := b.Spec.StorageLocation
if storageLocationName == "" {
errs = append(errs, errors.Errorf("cannot delete backup %q because it does not have a backup storage location set", b.Name))
continue
}
location, ok := bslCache[storageLocationName]
if !ok {
location = &velerov1api.BackupStorageLocation{}
if err := o.Client.Get(context.TODO(), controllerclient.ObjectKey{Namespace: o.Namespace, Name: storageLocationName}, location); err != nil {
errs = append(errs, errors.Wrapf(err, "error getting backup storage location %q for backup %q", storageLocationName, b.Name))
continue
}
bslCache[storageLocationName] = location
}
if location.Spec.AccessMode == velerov1api.BackupStorageLocationAccessModeReadOnly {
errs = append(errs, errors.Errorf("cannot delete backup %q because backup storage location %q is currently in read-only mode", b.Name, location.Name))
continue
}
deleteRequest := builder.ForDeleteBackupRequest(o.Namespace, "").BackupName(b.Name).
ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, label.GetValidName(b.Name),
velerov1api.BackupUIDLabel, string(b.UID)), builder.WithGenerateName(b.Name+"-")).Result()
pkg/cmd/cli/backup/delete_test.go
+31
View File
@@ -26,6 +26,7 @@ import (
"github.com/stretchr/testify/require"
controllerclient "sigs.k8s.io/controller-runtime/pkg/client"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/builder"
factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks"
"github.com/vmware-tanzu/velero/pkg/cmd/cli"
@@ -82,3 +83,33 @@ func TestDeleteCommand(t *testing.T) {
require.Contains(t, stdout, fmt.Sprintf("backups.velero.io \"%s\" not found.", backup2))
}
}
func TestDeleteCommandReadOnlyBSL(t *testing.T) {
const (
backupName = "backup-readonly"
bslName = "readonly-bsl"
)
client := velerotest.NewFakeControllerRuntimeClient(t)
require.NoError(t, client.Create(t.Context(), builder.ForBackupStorageLocation(cmdtest.VeleroNameSpace, bslName).
AccessMode(velerov1api.BackupStorageLocationAccessModeReadOnly).
Phase(velerov1api.BackupStorageLocationPhaseAvailable).
Result(), &controllerclient.CreateOptions{}))
require.NoError(t, client.Create(t.Context(), builder.ForBackup(cmdtest.VeleroNameSpace, backupName).
StorageLocation(bslName).
Result(), &controllerclient.CreateOptions{}))
o := cli.NewDeleteOptions("backup")
o.Client = client
o.Namespace = cmdtest.VeleroNameSpace
o.Confirm = true
o.Names = []string{backupName}
err := Run(o)
require.Error(t, err)
require.Contains(t, err.Error(), fmt.Sprintf("cannot delete backup %q because backup storage location %q is currently in read-only mode", backupName, bslName))
deleteRequestList := new(velerov1api.DeleteBackupRequestList)
require.NoError(t, client.List(t.Context(), deleteRequestList, &controllerclient.ListOptions{Namespace: cmdtest.VeleroNameSpace}))
require.Empty(t, deleteRequestList.Items)
}
pkg/restore/restore_test.go
+15 -35
View File
@@ -235,7 +235,7 @@ func TestRestorePVWithVolumeInfo(t *testing.T) {
for _, r := range tc.apiResources {
h.DiscoveryClient.WithAPIResource(r)
}
require.NoError(t, h.restorer.discoveryHelper.Refresh())
require.NoError(t, h.discoveryHelper.Refresh())
data := &Request{
Log: h.log,
@@ -788,7 +788,7 @@ func TestRestoreResourceFiltering(t *testing.T) {
for _, r := range tc.apiResources {
h.DiscoveryClient.WithAPIResource(r)
}
require.NoError(t, h.restorer.discoveryHelper.Refresh())
require.NoError(t, h.discoveryHelper.Refresh())
// We need to fetch the policies using the actual function
resPolicies, err := resourcepolicies.GetResourcePoliciesFromRestore(t.Context(), tc.restore, h.restorer.kbClient, h.log)
@@ -868,7 +868,7 @@ func TestRestoreMustHaveResourceNamespaceEnforcement(t *testing.T) {
for _, r := range tc.apiResources {
h.DiscoveryClient.WithAPIResource(r)
}
require.NoError(t, h.restorer.discoveryHelper.Refresh())
require.NoError(t, h.discoveryHelper.Refresh())
resPolicies, err := resourcepolicies.GetResourcePoliciesFromRestore(t.Context(), tc.restore, h.restorer.kbClient, h.log)
require.NoError(t, err)
@@ -954,7 +954,7 @@ func TestRestoreNamespaceMapping(t *testing.T) {
for _, r := range tc.apiResources {
h.DiscoveryClient.WithAPIResource(r)
}
require.NoError(t, h.restorer.discoveryHelper.Refresh())
require.NoError(t, h.discoveryHelper.Refresh())
data := &Request{
Log: h.log,
@@ -1038,7 +1038,7 @@ func TestRestoreResourcePriorities(t *testing.T) {
for _, r := range tc.apiResources {
h.DiscoveryClient.WithAPIResource(r)
}
require.NoError(t, h.restorer.discoveryHelper.Refresh())
require.NoError(t, h.discoveryHelper.Refresh())
data := &Request{
Log: h.log,
@@ -1115,7 +1115,7 @@ func TestInvalidTarballContents(t *testing.T) {
for _, r := range tc.apiResources {
h.DiscoveryClient.WithAPIResource(r)
}
require.NoError(t, h.restorer.discoveryHelper.Refresh())
require.NoError(t, h.discoveryHelper.Refresh())
data := &Request{
Log: h.log,
@@ -4548,10 +4548,10 @@ func assertNonEmptyResults(t *testing.T, typeMsg string, res ...Result) {
}
type harness struct {
*test.APIServer
restorer *kubernetesRestorer
log logrus.FieldLogger
*test.Harness
discoveryHelper discovery.Helper
restorer *kubernetesRestorer
log logrus.FieldLogger
}
func newHarness(t *testing.T) *harness {
@@ -4561,13 +4561,14 @@ func newHarness(t *testing.T) *harness {
log := logrus.StandardLogger()
kbClient := test.NewFakeControllerRuntimeClient(t)
discoveryHelper, err := discovery.NewHelper(apiServer.DiscoveryClient, log)
dh, err := discovery.NewHelper(apiServer.DiscoveryClient, log)
require.NoError(t, err)
return &harness{
APIServer: apiServer,
Harness: test.NewHarness(t, apiServer),
discoveryHelper: dh,
restorer: &kubernetesRestorer{
discoveryHelper: discoveryHelper,
discoveryHelper: dh,
dynamicFactory: client.NewDynamicFactory(apiServer.DynamicClient),
namespaceClient: apiServer.KubeClient.CoreV1().Namespaces(),
resourceTerminatingTimeout: time.Minute,
@@ -4586,28 +4587,7 @@ func newHarness(t *testing.T) *harness {
func (h *harness) AddItems(t *testing.T, resource *test.APIResource) {
t.Helper()
h.DiscoveryClient.WithAPIResource(resource)
require.NoError(t, h.restorer.discoveryHelper.Refresh())
for _, item := range resource.Items {
obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(item)
require.NoError(t, err)
unstructuredObj := &unstructured.Unstructured{Object: obj}
// These fields have non-nil zero values in the unstructured objects. We remove
// them to make comparison easier in our tests.
unstructured.RemoveNestedField(unstructuredObj.Object, "metadata", "creationTimestamp")
unstructured.RemoveNestedField(unstructuredObj.Object, "status")
if resource.Namespaced {
_, err = h.DynamicClient.Resource(resource.GVR()).Namespace(item.GetNamespace()).Create(t.Context(), unstructuredObj, metav1.CreateOptions{})
} else {
_, err = h.DynamicClient.Resource(resource.GVR()).Create(t.Context(), unstructuredObj, metav1.CreateOptions{})
}
require.NoError(t, err)
}
h.Harness.AddResource(t, h.discoveryHelper, resource)
}
func Test_resetVolumeBindingInfo(t *testing.T) {
pkg/test/harness.go
+73
View File
@@ -0,0 +1,73 @@
/*
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"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
)
type refreshable interface {
Refresh() error
}
// Harness wraps an APIServer and provides AddResource for seeding
// the fake API server with resources and items.
// It is shared across delete, restore, and backup test packages.
type Harness struct {
*APIServer
}
// NewHarness creates a Harness from an existing APIServer.
func NewHarness(t *testing.T, apiServer *APIServer) *Harness {
t.Helper()
require.NotNil(t, apiServer, "apiServer must not be nil")
return &Harness{APIServer: apiServer}
}
// AddResource registers an API resource with the discovery client,
// refreshes the discovery helper, and creates all of the resource's
// items in the fake dynamic client.
func (h *Harness) AddResource(t *testing.T, dh refreshable, resource *APIResource) {
t.Helper()
h.DiscoveryClient.WithAPIResource(resource)
require.NoError(t, dh.Refresh())
for _, item := range resource.Items {
obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(item)
require.NoError(t, err)
unstructuredObj := &unstructured.Unstructured{Object: obj}
// These fields have non-nil zero values in the unstructured objects. We remove
// them to make comparison easier in our tests.
unstructured.RemoveNestedField(unstructuredObj.Object, "metadata", "creationTimestamp")
unstructured.RemoveNestedField(unstructuredObj.Object, "status")
if resource.Namespaced {
_, err = h.DynamicClient.Resource(resource.GVR()).Namespace(item.GetNamespace()).Create(t.Context(), unstructuredObj, metav1.CreateOptions{})
} else {
_, err = h.DynamicClient.Resource(resource.GVR()).Create(t.Context(), unstructuredObj, metav1.CreateOptions{})
}
require.NoError(t, err)
}
}