diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 7b9a2120764..fa4c3632c7f 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -159,6 +159,11 @@ func (in *CloudStorageLocation) DeepCopyInto(out *CloudStorageLocation) { *out = new(metav1.Duration) **out = **in } + if in.CACert != nil { + in, out := &in.CACert, &out.CACert + *out = make([]byte, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CloudStorageLocation. diff --git a/controllers/bsl.go b/controllers/bsl.go index 84be85b74b0..d5dfb982b3e 100644 --- a/controllers/bsl.go +++ b/controllers/bsl.go @@ -14,11 +14,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) -func (r *DPAReconciler) ValidateBackupStorageLocations(log logr.Logger) (bool, error) { - dpa := oadpv1alpha1.DataProtectionApplication{} - if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil { - return false, err - } +func (r *DPAReconciler) ValidateBackupStorageLocations(dpa oadpv1alpha1.DataProtectionApplication) (bool, error) { if dpa.Spec.Configuration == nil || dpa.Spec.Configuration.Velero == nil { return false, errors.New("DPA CR Velero configuration cannot be nil") } diff --git a/controllers/bsl_test.go b/controllers/bsl_test.go index a91567f9730..b219dde3ae8 100644 --- a/controllers/bsl_test.go +++ b/controllers/bsl_test.go @@ -1218,7 +1218,7 @@ func TestDPAReconciler_ValidateBackupStorageLocations(t *testing.T) { }, EventRecorder: record.NewFakeRecorder(10), } - got, err := r.ValidateBackupStorageLocations(r.Log) + got, err := r.ValidateBackupStorageLocations(*tt.dpa) if (err != nil) != tt.wantErr { t.Errorf("ValidateBackupStorageLocations() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/controllers/dpa_controller.go b/controllers/dpa_controller.go index a214c6add1c..f8cb2890ad6 100644 --- a/controllers/dpa_controller.go +++ b/controllers/dpa_controller.go @@ -90,16 +90,13 @@ func (r *DPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R _, err := ReconcileBatch(r.Log, r.ValidateDataProtectionCR, r.ReconcileResticRestoreHelperConfig, - r.ValidateBackupStorageLocations, r.ReconcileBackupStorageLocations, r.ReconcileRegistrySecrets, r.ReconcileRegistries, r.ReconcileRegistrySVCs, r.ReconcileRegistryRoutes, r.ReconcileRegistryRouteConfigs, - r.ValidateVolumeSnapshotLocations, r.LabelVSLSecrets, - r.ReconcileVolumeSnapshotLocations, r.ReconcileVeleroDeployment, r.ReconcileResticDaemonset, r.ReconcileVeleroMetricsSVC, @@ -214,6 +211,8 @@ type ReconcileFunc func(logr.Logger) (bool, error) // reconcileBatch steps through a list of reconcile functions until one returns // false or an error. func ReconcileBatch(l logr.Logger, reconcileFuncs ...ReconcileFunc) (bool, error) { + // TODO: #1127 DPAReconciler already have a logger, use it instead of passing to each reconcile functions + // TODO: #1128 Right now each reconcile functions call get for DPA, we can do it once and pass it to each function for _, f := range reconcileFuncs { if cont, err := f(l); !cont || err != nil { return cont, err diff --git a/controllers/validator.go b/controllers/validator.go index 48a91b88f89..287c589a963 100644 --- a/controllers/validator.go +++ b/controllers/validator.go @@ -14,6 +14,9 @@ import ( "github.com/openshift/oadp-operator/pkg/credentials" ) +// ValidateDataProtectionCR function validates the DPA CR, returns true if valid, false otherwise +// it calls other validation functions to validate the DPA CR +// TODO: #1129 Clean up duplicate logic for validating backupstoragelocations and volumesnapshotlocations in dpa func (r *DPAReconciler) ValidateDataProtectionCR(log logr.Logger) (bool, error) { dpa := oadpv1alpha1.DataProtectionApplication{} if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil { @@ -99,7 +102,8 @@ func (r *DPAReconciler) ValidateDataProtectionCR(log logr.Logger) (bool, error) if _, err := getResticResourceReqs(&dpa); err != nil { return false, err } - + r.ValidateBackupStorageLocations(dpa) + r.ValidateVolumeSnapshotLocations(dpa) return true, nil } diff --git a/controllers/validator_test.go b/controllers/validator_test.go index b8352d291e9..0b3e43da363 100644 --- a/controllers/validator_test.go +++ b/controllers/validator_test.go @@ -995,15 +995,13 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) { }, Configuration: &oadpv1alpha1.ApplicationConfig{ Velero: &oadpv1alpha1.VeleroConfig{ - DefaultPlugins: []oadpv1alpha1.DefaultPlugin{ - }, + DefaultPlugins: []oadpv1alpha1.DefaultPlugin{}, }, }, BackupImages: pointer.Bool(true), }, }, - objects: []client.Object{ - }, + objects: []client.Object{}, wantErr: true, want: false, }, @@ -1027,8 +1025,7 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) { }, Configuration: &oadpv1alpha1.ApplicationConfig{ Velero: &oadpv1alpha1.VeleroConfig{ - DefaultPlugins: []oadpv1alpha1.DefaultPlugin{ - }, + DefaultPlugins: []oadpv1alpha1.DefaultPlugin{}, }, }, BackupImages: pointer.Bool(true), diff --git a/controllers/vsl.go b/controllers/vsl.go index 11d4b97daff..5d49ec32aa8 100644 --- a/controllers/vsl.go +++ b/controllers/vsl.go @@ -104,11 +104,7 @@ func (r *DPAReconciler) LabelVSLSecrets(log logr.Logger) (bool, error) { return true, nil } -func (r *DPAReconciler) ValidateVolumeSnapshotLocations(log logr.Logger) (bool, error) { - dpa := oadpv1alpha1.DataProtectionApplication{} - if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil { - return false, err - } +func (r *DPAReconciler) ValidateVolumeSnapshotLocations(dpa oadpv1alpha1.DataProtectionApplication) (bool, error) { if dpa.Spec.Configuration == nil { return false, errors.New("application configuration not found") } diff --git a/controllers/vsl_test.go b/controllers/vsl_test.go index 8ac1e869923..af51d77828d 100644 --- a/controllers/vsl_test.go +++ b/controllers/vsl_test.go @@ -510,7 +510,7 @@ func TestDPAReconciler_ValidateVolumeSnapshotLocation(t *testing.T) { }, EventRecorder: record.NewFakeRecorder(10), } - got, err := r.ValidateVolumeSnapshotLocations(r.Log) + got, err := r.ValidateVolumeSnapshotLocations(*tt.dpa) if (err != nil) != tt.wantErr { t.Errorf("ValidateVolumeSnapshotLocations() error = %v, wantErr %v", err, tt.wantErr) return