OCPBUGS-97592: skip plugin execution when backup does not target HCP resources

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

pkg/common/utils_test.go

@@ -500,69 +500,72 @@ 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,
 			expectError:        true,
 		},
 		{
 			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 {

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
 	}

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

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:

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{