From e55e4995f67c1a6866ca3f92d657b7eb6d2bd689 Mon Sep 17 00:00:00 2001 From: Lawton Mizell Date: Tue, 7 Jul 2026 14:30:04 -0400 Subject: [PATCH] OCPBUGS-97592: skip plugin execution when backup does not target HCP resources Signed-off-by: Lawton Mizell --- pkg/common/utils.go | 28 ++++++---------------- pkg/common/utils_test.go | 51 +++++++++++++++++++++------------------- pkg/core/backup.go | 2 +- pkg/core/backup_test.go | 3 +++ pkg/core/restore.go | 8 +------ pkg/core/restore_test.go | 3 +++ 6 files changed, 42 insertions(+), 53 deletions(-) diff --git a/pkg/common/utils.go b/pkg/common/utils.go index c93a96319..13ea411c9 100644 --- a/pkg/common/utils.go +++ b/pkg/common/utils.go @@ -81,8 +81,6 @@ func GetConfig() (*rest.Config, error) { return cfg, nil } - - // GetCurrentNamespace reads the namespace from the Kubernetes service account // token file and returns it as a string. The file is expected to be located at // "/var/run/secrets/kubernetes.io/serviceaccount/namespace". If there is an error @@ -105,7 +103,6 @@ func MatchSuffixKind(kind string, suffixes ...string) bool { } } return false - } // AddAnnotation adds an annotation to the given metadata object. @@ -130,7 +127,6 @@ func RemoveAnnotation(metadata metav1.Object, key string) { metadata.SetAnnotations(annotations) } - // AddLabel adds a label with the specified key and value to the given metadata object. // If the metadata object does not have any labels, a new map is created to store the label. func AddLabel(metadata metav1.Object, key, value string) { @@ -205,33 +201,24 @@ func GetHostedCluster(ctx context.Context, c crclient.Client, includedNamespaces return nil, nil } -// ShouldEndPluginExecution checks if the plugin should end execution by verifying if the required -// Hypershift resources (HostedControlPlane and HostedCluster) exist in the cluster. -// Returns true if the plugin should end execution (i.e., if this is not a Hypershift cluster). -func ShouldEndPluginExecution(ctx context.Context, backup *veleroapiv1.Backup, c crclient.Client, log logrus.FieldLogger) (bool, error) { +// ShouldEndPluginExecution checks whether this backup targets a hosted control +// plane. It returns true (skip) when no namespaces are provided or when the +// backup's includedResources does not contain HCP resource types. +func ShouldEndPluginExecution(backup *veleroapiv1.Backup) (bool, error) { if len(backup.Spec.IncludedNamespaces) == 0 { return true, fmt.Errorf("no namespaces provided") } - // Check for both short and full resource names for _, resource := range backup.Spec.IncludedResources { - if strings.Contains(resource, "hostedcluster") || + if resource == "*" || + strings.Contains(resource, "hostedcluster") || strings.Contains(resource, "hostedcontrolplane") || strings.Contains(resource, "nodepool") { return false, nil } } - exists, err := CRDExists(ctx, "hostedcontrolplanes.hypershift.openshift.io", c) - if err != nil { - return true, fmt.Errorf("error checking for HostedControlPlane CRD: %v", err) - } - - if exists { - return false, nil - } - - return true, fmt.Errorf("no HostedControlPlane CRD found") + return true, nil } func CRDExists(ctx context.Context, crdName string, c crclient.Client) (bool, error) { @@ -245,4 +232,3 @@ func CRDExists(ctx context.Context, crdName string, c crclient.Client) (bool, er } return true, nil } - diff --git a/pkg/common/utils_test.go b/pkg/common/utils_test.go index eacfb8871..d1231467f 100644 --- a/pkg/common/utils_test.go +++ b/pkg/common/utils_test.go @@ -500,37 +500,27 @@ func (f *fakeClient) Delete(ctx context.Context, obj client.Object, opts ...clie func TestShouldEndPluginExecution(t *testing.T) { tests := []struct { name string - objects []client.Object includedNamespaces []string includedResources []string expectedResult bool expectError bool }{ { - name: "CRD exists", - objects: []client.Object{ - &apiextensionsv1.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "hostedcontrolplanes.hypershift.openshift.io", - }, - }, - }, + name: "HCP resource types included", includedNamespaces: []string{"test-namespace"}, includedResources: []string{"hostedcontrolplanes", "hostedclusters"}, expectedResult: false, expectError: false, }, { - name: "CRD does not exist", - objects: []client.Object{}, + name: "No HCP resource types", includedNamespaces: []string{"test-namespace"}, includedResources: []string{"secrets", "configmaps"}, expectedResult: true, - expectError: true, + expectError: false, }, { name: "No namespaces provided", - objects: []client.Object{}, includedNamespaces: []string{}, includedResources: []string{"hostedcontrolplanes", "hostedclusters"}, expectedResult: true, @@ -538,31 +528,44 @@ func TestShouldEndPluginExecution(t *testing.T) { }, { name: "No resources provided", - objects: []client.Object{}, includedNamespaces: []string{"test-namespace"}, includedResources: []string{}, expectedResult: true, - expectError: true, + expectError: false, + }, + { + name: "Wildcard resources", + includedNamespaces: []string{"test-namespace"}, + includedResources: []string{"*"}, + expectedResult: false, + expectError: false, + }, + { + name: "Nodepool only", + includedNamespaces: []string{"test-namespace"}, + includedResources: []string{"nodepool"}, + expectedResult: false, + expectError: false, + }, + { + name: "Full GVR resource name", + includedNamespaces: []string{"test-namespace"}, + includedResources: []string{"hostedcontrolplanes.hypershift.openshift.io"}, + expectedResult: false, + expectError: false, }, } - scheme := runtime.NewScheme() - _ = hyperv1.AddToScheme(scheme) - _ = apiextensionsv1.AddToScheme(scheme) - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build() - log := logrus.New() - - result, err := ShouldEndPluginExecution(context.TODO(), &veleroapiv1.Backup{ + result, err := ShouldEndPluginExecution(&veleroapiv1.Backup{ Spec: veleroapiv1.BackupSpec{ IncludedNamespaces: tt.includedNamespaces, IncludedResources: tt.includedResources, }, - }, c, log) + }) if tt.expectError { g.Expect(err).To(HaveOccurred()) } else { diff --git a/pkg/core/backup.go b/pkg/core/backup.go index eb12f8945..4413c04f3 100644 --- a/pkg/core/backup.go +++ b/pkg/core/backup.go @@ -138,7 +138,7 @@ func (p *BackupPlugin) Execute(item runtime.Unstructured, backup *velerov1.Backu p.log.Debug("Entering Hypershift backup plugin") ctx := context.Context(p.ctx) - if returnEarly, err := common.ShouldEndPluginExecution(ctx, backup, p.client, p.log); returnEarly { + if returnEarly, err := common.ShouldEndPluginExecution(backup); returnEarly { p.log.Infof("Skipping hypershift plugin execution - not a hypershift backup: %v", err) return item, nil, nil } diff --git a/pkg/core/backup_test.go b/pkg/core/backup_test.go index 8f62877ab..04417626d 100644 --- a/pkg/core/backup_test.go +++ b/pkg/core/backup_test.go @@ -19,6 +19,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +var testHCPResources = []string{"hostedcontrolplane", "hostedcluster", "nodepool"} + // mockValidator implements validation.BackupValidator for testing. type mockValidator struct { validatePlatformErr error @@ -85,6 +87,7 @@ func newTestBackup() *velerov1.Backup { ObjectMeta: metav1.ObjectMeta{Name: "test-backup", Namespace: "openshift-adp"}, Spec: velerov1.BackupSpec{ IncludedNamespaces: []string{"clusters", "clusters-test"}, + IncludedResources: testHCPResources, }, } } diff --git a/pkg/core/restore.go b/pkg/core/restore.go index 459c318d1..c4cf2322b 100644 --- a/pkg/core/restore.go +++ b/pkg/core/restore.go @@ -163,17 +163,11 @@ func (p *RestorePlugin) Execute(input *velero.RestoreItemActionExecuteInput) (*v } // if the backup is not a hypershift backup, return early - if returnEarly, err := common.ShouldEndPluginExecution(ctx, backup, p.client, p.log); returnEarly { + if returnEarly, err := common.ShouldEndPluginExecution(backup); returnEarly { p.log.Infof("Skipping hypershift plugin execution - not a hypershift backup: %v", err) return velero.NewRestoreItemActionExecuteOutput(input.Item), nil } - // if the IncludedNamespaces field is nil, return error - if backup.Spec.IncludedNamespaces == nil { - p.log.Error("IncludedNamespaces from backup object is nil") - return nil, fmt.Errorf("included namespaces from backup object is nil") - } - kind := input.Item.GetObjectKind().GroupVersionKind().Kind switch { case kind == common.HostedControlPlaneKind: diff --git a/pkg/core/restore_test.go b/pkg/core/restore_test.go index ae549bfb2..f6c5f4a8c 100644 --- a/pkg/core/restore_test.go +++ b/pkg/core/restore_test.go @@ -780,6 +780,7 @@ func TestRestoreExecuteEtcdStatefulSet(t *testing.T) { Spec: velerov1api.BackupSpec{ StorageLocation: "default", IncludedNamespaces: []string{"clusters", "clusters-test"}, + IncludedResources: testHCPResources, }, } restore := &velerov1api.Restore{ @@ -882,6 +883,7 @@ func TestRestoreExecuteSnapshotURL(t *testing.T) { Spec: velerov1api.BackupSpec{ StorageLocation: "default", IncludedNamespaces: []string{"clusters", "clusters-test"}, + IncludedResources: testHCPResources, }, } restore := &velerov1api.Restore{ @@ -1092,6 +1094,7 @@ func TestRestoreExecuteHCPSnapshotURL(t *testing.T) { Spec: velerov1api.BackupSpec{ StorageLocation: "default", IncludedNamespaces: []string{"clusters", "clusters-test"}, + IncludedResources: testHCPResources, }, } restore := &velerov1api.Restore{