Skip to content

Commit

Permalink
Add unit tests covering defaults auth and assign
Browse files Browse the repository at this point in the history
This covers a few happy and error cases with unit tests, uncovering a
few typos in error messages in the process.
  • Loading branch information
weyfonk committed Nov 14, 2024
1 parent fc5f374 commit 1f7b871
Show file tree
Hide file tree
Showing 4 changed files with 255 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr

// Restrictions / Overrides, gitrepo reconciler is responsible for setting error in status
oldStatus := gitrepo.Status.DeepCopy()
if err := authorizeAndAssignDefaults(ctx, r.Client, gitrepo); err != nil {
if err := AuthorizeAndAssignDefaults(ctx, r.Client, gitrepo); err != nil {
r.Recorder.Event(gitrepo, fleetevent.Warning, "FailedToApplyRestrictions", err.Error())
return ctrl.Result{}, updateErrorStatus(ctx, r.Client, req.NamespacedName, *oldStatus, err)
}
Expand Down
8 changes: 4 additions & 4 deletions internal/cmd/controller/gitops/reconciler/restrictions.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

// authorizeAndAssignDefaults applies restrictions and mutates the passed in
// AuthorizeAndAssignDefaults applies restrictions and mutates the passed in
// GitRepo if it passes the restrictions
func authorizeAndAssignDefaults(ctx context.Context, c client.Client, gitrepo *fleet.GitRepo) error {
func AuthorizeAndAssignDefaults(ctx context.Context, c client.Client, gitrepo *fleet.GitRepo) error {
restrictions := &fleet.GitRepoRestrictionList{}
err := c.List(ctx, restrictions, client.InNamespace(gitrepo.Namespace))
if err != nil {
Expand Down Expand Up @@ -44,14 +44,14 @@ func authorizeAndAssignDefaults(ctx context.Context, c client.Client, gitrepo *f

repo, err := isAllowedByRegex(gitrepo.Spec.Repo, "", restriction.AllowedRepoPatterns)
if err != nil {
return fmt.Errorf("disallowed repo %s: %w", gitrepo.Spec.ServiceAccount, err)
return fmt.Errorf("disallowed repo %s: %w", gitrepo.Spec.Repo, err)
}

clientSecretName, err := isAllowed(gitrepo.Spec.ClientSecretName,
restriction.DefaultClientSecretName,
restriction.AllowedClientSecretNames)
if err != nil {
return fmt.Errorf("disallowed clientSecretName %s: %w", gitrepo.Spec.ServiceAccount, err)
return fmt.Errorf("disallowed clientSecretName %s: %w", gitrepo.Spec.ClientSecretName, err)
}

// set the defaults back to the GitRepo
Expand Down
249 changes: 249 additions & 0 deletions internal/cmd/controller/gitops/reconciler/restrictions_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,249 @@
//go:generate mockgen --build_flags=--mod=mod -destination=../../../../mocks/client_mock.go -package=mocks sigs.k8s.io/controller-runtime/pkg/client Client

package reconciler_test

import (
"context"
"errors"
"reflect"
"regexp"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
crclient "sigs.k8s.io/controller-runtime/pkg/client"

"github.com/rancher/fleet/internal/cmd/controller/gitops/reconciler"
"github.com/rancher/fleet/internal/mocks"
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
fleetv1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
)

func TestAuthorizeAndAssignDefaults(t *testing.T) {
dummyErrMsg := "something happened"
dummyErr := errors.New(dummyErrMsg)
cases := []struct {
name string
inputGr fleet.GitRepo
restrictions *fleet.GitRepoRestrictionList
restrictionsListErr error
expectedGr fleet.GitRepo
expectedErr string
}{
{
name: "fail when listing GitRepo restrictions errors",
inputGr: fleet.GitRepo{},
restrictionsListErr: dummyErr,
expectedGr: fleet.GitRepo{},
expectedErr: dummyErrMsg,
},
{
name: "pass when list of GitRepo restrictions is empty",
inputGr: fleet.GitRepo{},
restrictions: &fleet.GitRepoRestrictionList{},
expectedGr: fleet.GitRepo{},
},
{
name: "deny empty targetNamespace when allowedTargetNamespaces restriction present",
inputGr: fleet.GitRepo{}, // no target ns provided
restrictions: &fleet.GitRepoRestrictionList{
Items: []fleet.GitRepoRestriction{
{
AllowedTargetNamespaces: []string{"foo"},
},
},
},
expectedGr: fleet.GitRepo{},
expectedErr: "empty targetNamespace denied.*allowedTargetNamespaces restriction is present",
},
{
name: "deny disallowed targetNamespace",
inputGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
TargetNamespace: "not-foo",
},
},
restrictions: &fleet.GitRepoRestrictionList{
Items: []fleet.GitRepoRestriction{
{
AllowedTargetNamespaces: []string{"foo"},
},
},
},
expectedGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
TargetNamespace: "not-foo",
},
},
expectedErr: "disallowed targetNamespace.*",
},
{
name: "deny disallowed service account",
inputGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
ServiceAccount: "not-foo",
},
},
restrictions: &fleet.GitRepoRestrictionList{
Items: []fleet.GitRepoRestriction{
{
AllowedServiceAccounts: []string{"foo"},
},
},
},
expectedGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
ServiceAccount: "not-foo",
},
},
expectedErr: "disallowed serviceAccount.*",
},
{
name: "deny disallowed repo pattern",
inputGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
Repo: "bar",
},
},
restrictions: &fleet.GitRepoRestrictionList{
Items: []fleet.GitRepoRestriction{
{
AllowedRepoPatterns: []string{"baz"},
},
},
},
expectedGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
Repo: "bar",
},
},
expectedErr: "disallowed repo.*",
},
{
name: "deny disallowed client secret name",
inputGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
ClientSecretName: "not-foo",
},
},
restrictions: &fleet.GitRepoRestrictionList{
Items: []fleet.GitRepoRestriction{
{
AllowedClientSecretNames: []string{"foo"},
},
},
},
expectedGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
ClientSecretName: "not-foo",
},
},
expectedErr: "disallowed clientSecretName.*",
},
{
name: "pass when no restrictions nor disallowed values exist",
inputGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
Repo: "http://foo.bar/baz",
},
},
restrictions: &fleet.GitRepoRestrictionList{},
expectedGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
Repo: "http://foo.bar/baz",
},
},
},
{
name: "pass when restrictions exist and the GitRepo matches allowed values",
inputGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
ClientSecretName: "csn",
Repo: "http://foo.bar/baz",
ServiceAccount: "sacc",
TargetNamespace: "tns",
},
},
restrictions: &fleet.GitRepoRestrictionList{
Items: []fleet.GitRepoRestriction{
{
AllowedClientSecretNames: []string{"csn"},
AllowedRepoPatterns: []string{".*foo.bar.*"},
AllowedServiceAccounts: []string{"sacc"},
AllowedTargetNamespaces: []string{"tns"},
},
},
},
expectedGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
ClientSecretName: "csn",
Repo: "http://foo.bar/baz",
ServiceAccount: "sacc",
TargetNamespace: "tns",
},
},
},
{
name: "pass and mutate repo with defaults when restrictions exist and the GitRepo matches allowed values",
inputGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
Repo: "http://foo.bar/baz",
TargetNamespace: "tns",
},
},
restrictions: &fleet.GitRepoRestrictionList{
Items: []fleet.GitRepoRestriction{
{
AllowedClientSecretNames: []string{"csn"},
AllowedRepoPatterns: []string{".*foo.bar.*"},
AllowedServiceAccounts: []string{"sacc"},
AllowedTargetNamespaces: []string{"tns"},
DefaultClientSecretName: "dcsn",
DefaultServiceAccount: "dsacc",
},
},
},
expectedGr: fleet.GitRepo{
Spec: fleet.GitRepoSpec{
ClientSecretName: "dcsn",
Repo: "http://foo.bar/baz",
ServiceAccount: "dsacc",
TargetNamespace: "tns",
},
},
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

client := mocks.NewMockClient(mockCtrl)

client.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().DoAndReturn(
func(_ context.Context, rl *fleetv1.GitRepoRestrictionList, ns crclient.InNamespace) error {
if c.restrictions != nil && len(c.restrictions.Items) > 0 {
rl.Items = c.restrictions.Items
}

return c.restrictionsListErr
},
)

err := reconciler.AuthorizeAndAssignDefaults(context.TODO(), client, &c.inputGr)

if len(c.expectedErr) > 0 {
require.NotNil(t, err)
assert.Regexp(t, regexp.MustCompile(c.expectedErr), err.Error())
} else {
assert.Nil(t, err)
}

if !reflect.DeepEqual(c.inputGr, c.expectedGr) {
t.Errorf("Expected res %v, got %v", c.expectedGr, c.inputGr)
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (r *StatusReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
}

// Restrictions / Overrides, gitrepo reconciler is responsible for setting error in status
if err := authorizeAndAssignDefaults(ctx, r.Client, gitrepo); err != nil {
if err := AuthorizeAndAssignDefaults(ctx, r.Client, gitrepo); err != nil {
// the gitjob_controller will handle the error
return ctrl.Result{}, nil
}
Expand Down

0 comments on commit 1f7b871

Please sign in to comment.