diff --git a/cmd/root.go b/cmd/root.go index 429827a4d..e13352387 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -62,6 +62,56 @@ import ( "k8s.io/klog/v2" ) +// bslDPAManagedError returns an error if the BSL has an ownerReference pointing to a +// DataProtectionApplication, indicating the location is managed by the DPA reconciler. +// This is the testable core — it accepts a pre-built client so tests can pass a fake. +func bslDPAManagedError(ctx context.Context, kbClient kbclient.Client, namespace, bslName string) error { + location := &velerov1.BackupStorageLocation{} + if err := kbClient.Get(ctx, kbclient.ObjectKey{ + Namespace: namespace, + Name: bslName, + }, location); err != nil { + return err + } + + for _, ref := range location.OwnerReferences { + if ref.Kind == "DataProtectionApplication" && strings.Contains(ref.APIVersion, "oadp.openshift.io") { + return fmt.Errorf( + "backup storage location %q is managed by DataProtectionApplication %q.\n"+ + "Direct modifications via 'oc oadp backup-location set' will be overwritten by the DPA reconciler.\n"+ + "To change settings such as '--cacert', update the DataProtectionApplication spec instead", + bslName, ref.Name, + ) + } + } + return nil +} + +// checkBSLNotDPAManaged is the factory-aware wrapper used by the CLI command's PreRunE. +func checkBSLNotDPAManaged(ctx context.Context, f clientcmd.Factory, bslName string) error { + kbClient, err := f.KubebuilderClient() + if err != nil { + return err + } + return bslDPAManagedError(ctx, kbClient, f.Namespace(), bslName) +} + +// injectDPAManagedGuard wraps the "set" subcommand of the given backup-location command +// with a PreRunE that rejects modifications to DPA-managed BSLs before the update is attempted. +func injectDPAManagedGuard(bslCmd *cobra.Command, f clientcmd.Factory) { + for _, sub := range bslCmd.Commands() { + if strings.HasPrefix(sub.Use, "set ") { + sub.PreRunE = wrapPreRunE(sub.PreRunE, func(c *cobra.Command, args []string) error { + if len(args) == 0 { + return nil + } + return checkBSLNotDPAManaged(c.Context(), f, args[0]) + }) + return + } + } +} + // globalRequestTimeout holds the request timeout value set by --request-timeout flag. // This is used by the timeoutFactory wrapper to apply dial timeout to all clients. var ( @@ -431,6 +481,9 @@ func NewVeleroRootCommand(baseName string) *cobra.Command { // Nonadmin commands continue using GetCurrentNamespace() for security isolation. f.BindFlags(c.PersistentFlags()) + bslCmd := backuplocation.NewCommand(f) + injectDPAManagedGuard(bslCmd, f) + c.AddCommand( backup.NewCommand(f), schedule.NewCommand(f), @@ -442,7 +495,7 @@ func NewVeleroRootCommand(baseName string) *cobra.Command { veldelete.NewCommand(f), cliclient.NewCommand(), completion.NewCommand(), - backuplocation.NewCommand(f), + bslCmd, snapshotlocation.NewCommand(f), debug.NewCommand(f), ) diff --git a/cmd/root_test.go b/cmd/root_test.go index 96cb819c4..7c1ec3564 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -30,7 +30,11 @@ import ( "github.com/migtools/oadp-cli/internal/testutil" "github.com/spf13/cobra" + velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" + kbfake "sigs.k8s.io/controller-runtime/pkg/client/fake" ) // TestRootCommand tests the root command functionality @@ -822,3 +826,136 @@ func TestApplyTimeoutToConfig_DialerTimeout(t *testing.T) { t.Errorf("Dial took too long: %v (expected ~%v)", elapsed, timeout) } } + +func TestBSLDPAManagedError(t *testing.T) { + const ns = "openshift-adp" + + scheme := runtime.NewScheme() + if err := velerov1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add velero scheme: %v", err) + } + + tests := []struct { + name string + bsl *velerov1.BackupStorageLocation + bslName string + wantErr bool + errContains string + }{ + { + name: "standalone BSL without ownerReference is allowed", + bslName: "standalone", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "standalone", + Namespace: ns, + }, + }, + wantErr: false, + }, + { + name: "BSL owned by DPA is rejected", + bslName: "default", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: ns, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "oadp.openshift.io/v1alpha1", + Kind: "DataProtectionApplication", + Name: "velero", + }, + }, + }, + }, + wantErr: true, + errContains: "managed by DataProtectionApplication", + }, + { + name: "error message names the DPA", + bslName: "default", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: ns, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "oadp.openshift.io/v1alpha1", + Kind: "DataProtectionApplication", + Name: "my-dpa", + }, + }, + }, + }, + wantErr: true, + errContains: "my-dpa", + }, + { + name: "DataProtectionApplication kind with wrong API group is allowed", + bslName: "foreign-dpa", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foreign-dpa", + Namespace: ns, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "other.io/v1", + Kind: "DataProtectionApplication", + Name: "some-other-dpa", + }, + }, + }, + }, + wantErr: false, + }, + { + name: "BSL with unrelated ownerReference is allowed", + bslName: "other-owned", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-owned", + Namespace: ns, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "ConfigMap", + Name: "some-config", + }, + }, + }, + }, + wantErr: false, + }, + { + name: "non-existent BSL returns not-found error", + bslName: "does-not-exist", + bsl: nil, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var objs []runtime.Object + if tt.bsl != nil { + objs = append(objs, tt.bsl) + } + fakeClient := kbfake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(objs...). + Build() + + err := bslDPAManagedError(context.Background(), fakeClient, ns, tt.bslName) + if tt.wantErr && err == nil { + t.Errorf("expected error but got nil") + } + if !tt.wantErr && err != nil { + t.Errorf("expected no error but got: %v", err) + } + if tt.errContains != "" && err != nil && !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("error %q does not contain %q", err.Error(), tt.errContains) + } + }) + } +}