Skip to content

Commit

Permalink
Check whether the action is a CSI action and whether CSI feature is
Browse files Browse the repository at this point in the history
enabled, before executing the action.

The DeleteItemAction is not checked, because the DIA doesn't have a
method to get the action's plugin name.
This should be OK, because the CSI will check whether the VS and VSC
have a backup name annotation. If the VS and VSC is not handled by
the CSI plugin, then they don't have the annotation.

Signed-off-by: Xun Jiang <[email protected]>
  • Loading branch information
Xun Jiang committed Oct 24, 2023
1 parent 5ff5073 commit 908e2c6
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/6968-blackpiglet
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Check whether the action is a CSI action and whether CSI feature is enabled, before executing the action.
1 change: 1 addition & 0 deletions internal/delete/delete_item_action_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ func InvokeDeleteActions(ctx *Context) error {
if !action.Selector.Matches(labels.Set(obj.GetLabels())) {
continue
}

err = action.DeleteItemAction.Execute(&velero.DeleteItemActionExecuteInput{
Item: obj,
Backup: ctx.Backup,
Expand Down
7 changes: 7 additions & 0 deletions pkg/backup/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import (
"github.com/vmware-tanzu/velero/pkg/builder"
"github.com/vmware-tanzu/velero/pkg/client"
"github.com/vmware-tanzu/velero/pkg/discovery"
"github.com/vmware-tanzu/velero/pkg/features"
"github.com/vmware-tanzu/velero/pkg/itemoperation"
"github.com/vmware-tanzu/velero/pkg/kuberesource"
"github.com/vmware-tanzu/velero/pkg/plugin/velero"
Expand Down Expand Up @@ -1379,6 +1380,12 @@ func TestBackupItemActionsForSkippedPV(t *testing.T) {
expectNotSkippedPVs: []string{"pv-1"},
},
}
// Enable CSI feature before running the test, because Velero will check whether
// CSI feature is enabled before executing CSI plugin actions.
features.NewFeatureFlagSet("EnableCSI")
defer func() {
features.NewFeatureFlagSet("")
}()
for _, tc := range tests {
t.Run(tc.name, func(tt *testing.T) {
var (
Expand Down
10 changes: 10 additions & 0 deletions pkg/backup/item_backupper.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import (
vsv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/volumesnapshotter/v1"
"github.com/vmware-tanzu/velero/pkg/podvolume"
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
csiutil "github.com/vmware-tanzu/velero/pkg/util/csi"
pdvolumeutil "github.com/vmware-tanzu/velero/pkg/util/podvolume"
"github.com/vmware-tanzu/velero/pkg/volume"
)
Expand Down Expand Up @@ -361,6 +362,14 @@ func (ib *itemBackupper) executeActions(
ib.trackSkippedPV(obj, groupResource, "", "skipped due to resource policy ", log)
continue
}

// If the EnableCSI feature is not enabled, but the executing action is from CSI plugin, skip the action.
if csiutil.ShouldSkipAction(actionName) {
log.Infof("Skip action %s for resource %s:%s/%s, because the CSI feature is not enabled. Feature setting is %s.",
actionName, groupResource.String(), metadata.GetNamespace(), metadata.GetName(), features.Serialize())
continue

Check warning on line 370 in pkg/backup/item_backupper.go

View check run for this annotation

Codecov / codecov/patch

pkg/backup/item_backupper.go#L368-L370

Added lines #L368 - L370 were not covered by tests
}

updatedItem, additionalItemIdentifiers, operationID, postOperationItems, err := action.Execute(obj, ib.backupRequest.Backup)
if err != nil {
return nil, itemFiles, errors.Wrapf(err, "error executing custom action (groupResource=%s, namespace=%s, name=%s)", groupResource.String(), namespace, name)
Expand Down Expand Up @@ -652,6 +661,7 @@ func (ib *itemBackupper) getMatchAction(obj runtime.Unstructured, groupResource
}
return ib.backupRequest.ResPolicies.GetMatchAction(pv)
}

return nil, nil
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ import (
"github.com/vmware-tanzu/velero/pkg/persistence"
"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt/process"
"github.com/vmware-tanzu/velero/pkg/plugin/framework/common"
"github.com/vmware-tanzu/velero/pkg/podexec"
"github.com/vmware-tanzu/velero/pkg/podvolume"
"github.com/vmware-tanzu/velero/pkg/repository"
Expand Down Expand Up @@ -309,6 +310,13 @@ func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*s
return nil, err
}

if !features.IsEnabled(velerov1api.CSIFeatureFlag) {
_, err = pluginRegistry.Get(common.PluginKindBackupItemActionV2, "velero.io/csi-pvc-backupper")
if err == nil {
logger.Warn("CSI plugins are registered, but the EnableCSI feature is not enabled.")
}

Check warning on line 317 in pkg/cmd/server/server.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/server/server.go#L313-L317

Added lines #L313 - L317 were not covered by tests
}

// cancelFunc is not deferred here because if it was, then ctx would immediately
// be canceled once this function exited, making it useless to any informers using later.
// That, in turn, causes the velero server to halt when the first informer tries to use it.
Expand Down
8 changes: 8 additions & 0 deletions pkg/restore/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import (
"github.com/vmware-tanzu/velero/pkg/podvolume"
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
"github.com/vmware-tanzu/velero/pkg/util/collections"
csiutil "github.com/vmware-tanzu/velero/pkg/util/csi"
"github.com/vmware-tanzu/velero/pkg/util/filesystem"
"github.com/vmware-tanzu/velero/pkg/util/kube"
"github.com/vmware-tanzu/velero/pkg/util/results"
Expand Down Expand Up @@ -1337,6 +1338,13 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
continue
}

// If the EnableCSI feature is not enabled, but the executing action is from CSI plugin, skip the action.
if csiutil.ShouldSkipAction(action.Name()) {
ctx.log.Infof("Skip action %s for resource %s:%s/%s, because the CSI feature is not enabled. Feature setting is %s.",
action.Name(), groupResource.String(), obj.GetNamespace(), obj.GetName(), features.Serialize())
continue

Check warning on line 1345 in pkg/restore/restore.go

View check run for this annotation

Codecov / codecov/patch

pkg/restore/restore.go#L1343-L1345

Added lines #L1343 - L1345 were not covered by tests
}

ctx.log.Infof("Executing item action for %v", &groupResource)
executeOutput, err := action.RestoreItemAction.Execute(&velero.RestoreItemActionExecuteInput{
Item: obj,
Expand Down
32 changes: 32 additions & 0 deletions pkg/util/csi/util.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
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 csi

import (
"strings"

velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/features"
)

const (
csiPluginNamePrefix = "velero.io/csi-"
)

func ShouldSkipAction(actionName string) bool {
return !features.IsEnabled(velerov1api.CSIFeatureFlag) && strings.Contains(actionName, csiPluginNamePrefix)
}
36 changes: 36 additions & 0 deletions pkg/util/csi/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
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 csi

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/vmware-tanzu/velero/pkg/features"
)

func TestCSIFeatureNotEnabledAndPluginIsFromCSI(t *testing.T) {

features.NewFeatureFlagSet("EnableCSI")
require.False(t, ShouldSkipAction("abc"))
require.False(t, ShouldSkipAction("velero.io/csi-pvc-backupper"))

features.NewFeatureFlagSet("")
require.True(t, ShouldSkipAction("velero.io/csi-pvc-backupper"))
require.False(t, ShouldSkipAction("abc"))
}

0 comments on commit 908e2c6

Please sign in to comment.