diff --git a/controllers/pkg/.mockery.yaml b/controllers/pkg/.mockery.yaml index 80b2e0f2..14b79c84 100644 --- a/controllers/pkg/.mockery.yaml +++ b/controllers/pkg/.mockery.yaml @@ -10,4 +10,4 @@ packages: Client: config: dir: "mocks/external/{{ .InterfaceName | lower }}" - outpkg: "mocks" \ No newline at end of file + outpkg: "mocks" diff --git a/controllers/pkg/reconcilers/approval/reconciler.go b/controllers/pkg/reconcilers/approval/reconciler.go index b8279d56..9ec88f18 100644 --- a/controllers/pkg/reconcilers/approval/reconciler.go +++ b/controllers/pkg/reconcilers/approval/reconciler.go @@ -67,7 +67,7 @@ func (r *reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, c i return nil, fmt.Errorf("cannot initialize, expecting controllerConfig, got: %s", reflect.TypeOf(c).Name()) } - r.Client = mgr.GetClient() + r.baseClient = mgr.GetClient() r.porchClient = cfg.PorchClient r.porchRESTClient = cfg.PorchRESTClient r.recorder = mgr.GetEventRecorderFor("approval-controller") @@ -80,7 +80,7 @@ func (r *reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, c i // reconciler reconciles a NetworkInstance object type reconciler struct { - client.Client + baseClient client.Client porchClient client.Client porchRESTClient rest.Interface recorder record.EventRecorder @@ -91,7 +91,7 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu log.Info("reconcile approval") pr := &porchv1alpha1.PackageRevision{} - if err := r.Get(ctx, req.NamespacedName, pr); err != nil { + if err := r.baseClient.Get(ctx, req.NamespacedName, pr); err != nil { // There's no need to requeue if we no longer exist. Otherwise we'll be // requeued implicitly because we return an error. if resource.IgnoreNotFound(err) != nil { @@ -194,7 +194,7 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu action = "proposing" reason = "Proposed" pr.Spec.Lifecycle = porchv1alpha1.PackageRevisionLifecycleProposed - err = r.Update(ctx, pr) + err = r.baseClient.Update(ctx, pr) } else { err = porchclient.UpdatePackageRevisionApproval(ctx, r.porchRESTClient, client.ObjectKey{ Namespace: pr.Namespace, @@ -251,7 +251,7 @@ func manageDelay(pr *porchv1alpha1.PackageRevision) (time.Duration, error) { func (r *reconciler) policyInitial(ctx context.Context, pr *porchv1alpha1.PackageRevision) (bool, error) { var prList porchv1alpha1.PackageRevisionList - if err := r.Client.List(ctx, &prList); err != nil { + if err := r.baseClient.List(ctx, &prList); err != nil { return false, err } diff --git a/controllers/pkg/reconcilers/approval/reconciler_test.go b/controllers/pkg/reconcilers/approval/reconciler_test.go index dc8a5157..bb91c22d 100644 --- a/controllers/pkg/reconcilers/approval/reconciler_test.go +++ b/controllers/pkg/reconcilers/approval/reconciler_test.go @@ -15,12 +15,17 @@ package approval import ( + context "context" + "fmt" "testing" "time" + "github.com/stretchr/testify/mock" + porchapi "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/nephio-project/nephio/controllers/pkg/mocks/external/client" ) func TestShouldProcess(t *testing.T) { @@ -117,7 +122,7 @@ func TestManageDelay(t *testing.T) { "not old enough": { pr: porchapi.PackageRevision{ ObjectMeta: metav1.ObjectMeta{ - CreationTimestamp: metav1.Time{now}, + CreationTimestamp: metav1.Time{Time: now}, Annotations: map[string]string{ "approval.nephio.org/delay": "1h", }, @@ -129,7 +134,7 @@ func TestManageDelay(t *testing.T) { "old enough": { pr: porchapi.PackageRevision{ ObjectMeta: metav1.ObjectMeta{ - CreationTimestamp: metav1.Time{now.AddDate(-1, 0, 0)}, + CreationTimestamp: metav1.Time{Time: now.AddDate(-1, 0, 0)}, Annotations: map[string]string{ "approval.nephio.org/delay": "1h", }, @@ -147,3 +152,86 @@ func TestManageDelay(t *testing.T) { }) } } + + + +func TestPolicyInitial(t *testing.T) { + + testCases := map[string]struct { + pr porchapi.PackageRevision + prl *porchapi.PackageRevisionList + expectedApprove bool + expectedError error + mockReturnErr error + }{ + "Draft with proposed lifecycle": { + pr: porchapi.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "approval.nephio.org/policy": "initial", + }, + }, + }, + prl: &porchapi.PackageRevisionList{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "Blah", + Kind: "Blah", + }, + Items: []porchapi.PackageRevision{ + { + Spec: porchapi.PackageRevisionSpec{ + Lifecycle: porchapi.PackageRevisionLifecycleProposed, + }, + }, + }, + }, + expectedApprove: true, + expectedError: nil, + mockReturnErr: nil, + }, + "Draft with existing version": { + pr: porchapi.PackageRevision{ + Spec: porchapi.PackageRevisionSpec{ + RepositoryName: "MyRepo", + PackageName: "MyPackage", + }, + }, + prl: &porchapi.PackageRevisionList{ + Items: []porchapi.PackageRevision{ + { + Spec: porchapi.PackageRevisionSpec{ + Lifecycle: porchapi.PackageRevisionLifecyclePublished, + RepositoryName: "MyRepo", + PackageName: "MyPackage", + }, + }, + }, + }, + expectedApprove: false, + expectedError: nil, + mockReturnErr: nil, + }, + "runtime client list failure": { + pr: porchapi.PackageRevision{}, + prl: &porchapi.PackageRevisionList{}, + expectedApprove: false, + expectedError: fmt.Errorf("Failed to list items"), + mockReturnErr: fmt.Errorf("Failed to list items"), + }, + } + for tn, tc := range testCases { + // Create a new instance of the mock object + clientMock := new(mocks.MockClient) + clientMock.On("List", context.TODO(), mock.AnythingOfType("*v1alpha1.PackageRevisionList")).Return(tc.mockReturnErr).Run(func(args mock.Arguments) { + packRevList := args.Get(1).(*porchapi.PackageRevisionList) + *packRevList = *tc.prl // tc.prl is what r.Get will store in 2nd Argument + }) + // Create an instance of the component under test + r := reconciler{baseClient: clientMock} + t.Run(tn, func(t *testing.T) { + actualApproval, actualError := r.policyInitial(context.TODO(), &tc.pr) + require.Equal(t, tc.expectedApprove, actualApproval) + require.Equal(t, tc.expectedError, actualError) + }) + } +}