Skip to content

Commit

Permalink
Add test toolkit and improve tests
Browse files Browse the repository at this point in the history
The test toolkit and courtesies are things that other policies could use
in their tests. The remaining functions in the utils.go file are more
specifically just for fakepolicy.

In preparation for more tests, the existing tests have been moved into a
separate package/suite. In particular, since some of them require very
specific details in the cluster (exact namespaces, configmaps, and no
extras), this lets them be run in a more isolated way.

Signed-off-by: Justin Kulikauskas <[email protected]>
  • Loading branch information
JustinKuli committed May 17, 2024
1 parent ac65408 commit 343128b
Show file tree
Hide file tree
Showing 17 changed files with 160 additions and 83 deletions.
9 changes: 8 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,17 @@ lint: $(GOLANGCI)

ENVTEST_K8S_VERSION ?= 1.29
.PHONY: test
test: manifests generate $(GINKGO) $(ENVTEST) ## Run tests.
test: manifests generate $(GINKGO) $(ENVTEST) ## Run all the tests
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" $(GINKGO) \
--coverpkg=./... --covermode=count --coverprofile=cover.out ./...

test-unit: ## Run only the unit tests
go test --coverpkg=./... --covermode=count --coverprofile=cover-unit.out ./api/... ./pkg/...

test-basicsuite: manifests generate $(GINKGO) $(ENVTEST) ## Run just the basic suite of tests
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" $(GINKGO) \
--coverpkg=./... --covermode=count --coverprofile=cover-basic.out ./test/fakepolicy/test/basic

.PHONY: fuzz-test
fuzz-test:
go test ./api/v1beta1 -fuzz=FuzzMatchesExcludeAll -fuzztime=20s
Expand Down
15 changes: 15 additions & 0 deletions pkg/testutils/courtesies.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright Contributors to the Open Cluster Management project

package testutils

import (
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
)

func ObjNN(obj client.Object) types.NamespacedName {
return types.NamespacedName{
Namespace: obj.GetNamespace(),
Name: obj.GetName(),
}
}
76 changes: 76 additions & 0 deletions pkg/testutils/toolkit.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Copyright Contributors to the Open Cluster Management project

package testutils

import (
"context"
"fmt"

"github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"
gomegaTypes "github.com/onsi/gomega/types"
"k8s.io/apimachinery/pkg/api/errors"
"sigs.k8s.io/controller-runtime/pkg/client"
)

type Toolkit struct {
client.Client
EventuallyPoll string
EventuallyTimeout string
ConsistentlyPoll string
ConsistentallyTimeout string
BackgroundCtx context.Context //nolint:containedctx
}

func NewToolkit(client client.Client) Toolkit {
return Toolkit{
Client: client,
EventuallyPoll: "100ms",
EventuallyTimeout: "1s",
ConsistentlyPoll: "100ms",
ConsistentallyTimeout: "1s",
BackgroundCtx: context.Background(),
}
}

// cleanlyCreate creates the given object, and registers a callback to delete the object which
// Ginkgo will call at the appropriate time. The error from the `Create` call is returned (so it
// can be checked) and the `Delete` callback handles 'NotFound' errors as a success.
func (tk Toolkit) CleanlyCreate(ctx context.Context, obj client.Object) error {
// Save and then re-set the GVK because the API call removes it
savedGVK := obj.GetObjectKind().GroupVersionKind()
createErr := tk.Create(ctx, obj)
obj.GetObjectKind().SetGroupVersionKind(savedGVK)

if createErr == nil {
ginkgo.DeferCleanup(func() {
ginkgo.GinkgoWriter.Printf("Deleting %v %v/%v\n",
obj.GetObjectKind().GroupVersionKind().Kind, obj.GetNamespace(), obj.GetName())

if err := tk.Delete(tk.BackgroundCtx, obj); err != nil {
if !errors.IsNotFound(err) {
// Use Fail in order to provide a custom message with useful information
ginkgo.Fail(fmt.Sprintf("Expected success or 'NotFound' error, got %v", err), 1)
}
}
})
}

return createErr
}

// EC runs assertions on asynchronous behavior, both *E*ventually and *C*onsistently,
// using the polling and timeout settings of the toolkit. Its usage should feel familiar
// to gomega users, simply skip the `.Should(...)` call and put your matcher as the second
// parameter here.
func (tk Toolkit) EC(
actualOrCtx interface{}, matcher gomegaTypes.GomegaMatcher, optionalDescription ...interface{},
) bool {
gomega.Eventually(
actualOrCtx, tk.EventuallyTimeout, tk.EventuallyPoll,
).Should(matcher, optionalDescription...)

return gomega.Consistently(
actualOrCtx, tk.ConsistentallyTimeout, tk.ConsistentlyPoll,
).Should(matcher, optionalDescription...)
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright Contributors to the Open Cluster Management project

package test
package basic

import (
"fmt"
Expand All @@ -12,22 +12,24 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

nucleusv1beta1 "open-cluster-management.io/governance-policy-nucleus/api/v1beta1"
"open-cluster-management.io/governance-policy-nucleus/pkg/testutils"
fakev1beta1 "open-cluster-management.io/governance-policy-nucleus/test/fakepolicy/api/v1beta1"
. "open-cluster-management.io/governance-policy-nucleus/test/fakepolicy/test/utils"
)

var _ = Describe("FakePolicy NamespaceSelection", Ordered, func() {
defaultNamespaces := []string{"default", "kube-node-lease", "kube-public", "kube-system"}
sampleNamespaces := []string{"foo", "goo", "fake", "faze", "kube-one"}
allNamespaces := append(defaultNamespaces, sampleNamespaces...)

BeforeAll(func() {
BeforeAll(func(ctx SpecContext) {
By("Creating sample namespaces")
for _, ns := range sampleNamespaces {
nsObj := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{
Name: ns,
Labels: map[string]string{"sample": ns},
}}
Expect(cleanlyCreate(nsObj)).To(Succeed())
Expect(tk.CleanlyCreate(ctx, nsObj)).To(Succeed())
}

By("Ensuring the allNamespaces list is correct")
Expand All @@ -45,17 +47,17 @@ var _ = Describe("FakePolicy NamespaceSelection", Ordered, func() {
})

DescribeTable("Verifying NamespaceSelector behavior",
func(sel nucleusv1beta1.NamespaceSelector, desiredMatches []string, selErr string) {
policy := sampleFakePolicy()
func(ctx SpecContext, sel nucleusv1beta1.NamespaceSelector, desiredMatches []string, selErr string) {
policy := SampleFakePolicy()
policy.Spec.NamespaceSelector = sel

Expect(cleanlyCreate(&policy)).To(Succeed())
Expect(tk.CleanlyCreate(ctx, &policy)).To(Succeed())

slices.Sort(desiredMatches)

Eventually(func(g Gomega) {
foundPolicy := fakev1beta1.FakePolicy{}
g.Expect(k8sClient.Get(ctx, getNamespacedName(&policy), &foundPolicy)).To(Succeed())
g.Expect(k8sClient.Get(ctx, testutils.ObjNN(&policy), &foundPolicy)).To(Succeed())
g.Expect(foundPolicy.Status.SelectionComplete).To(BeTrue())

idx, cond := foundPolicy.Status.GetCondition("NamespaceSelection")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright Contributors to the Open Cluster Management project

package test
package basic

import (
"context"
Expand All @@ -17,6 +17,7 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"

"open-cluster-management.io/governance-policy-nucleus/pkg/testutils"
"open-cluster-management.io/governance-policy-nucleus/test/fakepolicy"
fakev1beta1 "open-cluster-management.io/governance-policy-nucleus/test/fakepolicy/api/v1beta1"
)
Expand All @@ -30,12 +31,13 @@ var (
testEnv *envtest.Environment
ctx context.Context
cancel context.CancelFunc
tk testutils.Toolkit
)

func TestAPIs(t *testing.T) {
RegisterFailHandler(Fail)

RunSpecs(t, "Controller Suite")
RunSpecs(t, "Basic Suite")
}

var _ = BeforeSuite(func() {
Expand All @@ -46,7 +48,7 @@ var _ = BeforeSuite(func() {

By("bootstrapping test environment")
testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")},
CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")},
ErrorIfCRDPathMissing: true,
}

Expand All @@ -65,6 +67,9 @@ var _ = BeforeSuite(func() {
Expect(err).NotTo(HaveOccurred())
Expect(k8sClient).NotTo(BeNil())

tk = testutils.NewToolkit(k8sClient)
tk.BackgroundCtx = ctx

go func() {
defer GinkgoRecover()
Expect(fakepolicy.Run(ctx, cfg)).To(Succeed())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright Contributors to the Open Cluster Management project

package test
package basic

import (
"fmt"
Expand All @@ -13,7 +13,9 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

nucleusv1beta1 "open-cluster-management.io/governance-policy-nucleus/api/v1beta1"
"open-cluster-management.io/governance-policy-nucleus/pkg/testutils"
fakev1beta1 "open-cluster-management.io/governance-policy-nucleus/test/fakepolicy/api/v1beta1"
. "open-cluster-management.io/governance-policy-nucleus/test/fakepolicy/test/utils"
)

var _ = Describe("FakePolicy TargetConfigMaps", func() {
Expand All @@ -32,7 +34,7 @@ var _ = Describe("FakePolicy TargetConfigMaps", func() {
}
allConfigMaps := append(defaultConfigMaps, sampleConfigMaps...)

beforeFunc := func() {
beforeFunc := func(ctx SpecContext) {
By("Creating sample configmaps")
for _, cm := range sampleConfigMaps {
ns, name, _ := strings.Cut(cm, "/")
Expand All @@ -44,7 +46,7 @@ var _ = Describe("FakePolicy TargetConfigMaps", func() {
},
Data: map[string]string{"foo": "bar"},
}
Expect(cleanlyCreate(cmObj)).To(Succeed())
Expect(tk.CleanlyCreate(ctx, cmObj)).To(Succeed())
}

By("Ensuring the allConfigMaps list is correct")
Expand Down Expand Up @@ -136,7 +138,7 @@ var _ = Describe("FakePolicy TargetConfigMaps", func() {
checkFunc := func(policy fakev1beta1.FakePolicy, desiredMatches []string, selErr string) func(g Gomega) {
return func(g Gomega) {
foundPolicy := fakev1beta1.FakePolicy{}
g.Expect(k8sClient.Get(ctx, getNamespacedName(&policy), &foundPolicy)).To(Succeed())
g.Expect(k8sClient.Get(ctx, testutils.ObjNN(&policy), &foundPolicy)).To(Succeed())
g.Expect(foundPolicy.Status.SelectionComplete).To(BeTrue())

slices.Sort(desiredMatches)
Expand All @@ -163,11 +165,11 @@ var _ = Describe("FakePolicy TargetConfigMaps", func() {
BeforeAll(beforeFunc)

DescribeTable("Verifying TargetConfigMaps behavior",
func(sel nucleusv1beta1.Target, desiredMatches []string, selErr string) {
policy := sampleFakePolicy()
func(ctx SpecContext, sel nucleusv1beta1.Target, desiredMatches []string, selErr string) {
policy := SampleFakePolicy()
policy.Spec.TargetConfigMaps = sel

Expect(cleanlyCreate(&policy)).To(Succeed())
Expect(tk.CleanlyCreate(ctx, &policy)).To(Succeed())

Eventually(checkFunc(policy, desiredMatches, selErr)).Should(Succeed())
},
Expand All @@ -179,7 +181,7 @@ var _ = Describe("FakePolicy TargetConfigMaps", func() {
BeforeAll(beforeFunc)

DescribeTable("Verifying TargetConfigMaps behavior",
func(sel nucleusv1beta1.Target, givenDesiredMatches []string, selErr string) {
func(ctx SpecContext, sel nucleusv1beta1.Target, givenDesiredMatches []string, selErr string) {
sel.Namespace = "default"

desiredMatches := make([]string, 0)
Expand All @@ -190,10 +192,10 @@ var _ = Describe("FakePolicy TargetConfigMaps", func() {
}
}

policy := sampleFakePolicy()
policy := SampleFakePolicy()
policy.Spec.TargetConfigMaps = sel

Expect(cleanlyCreate(&policy)).To(Succeed())
Expect(tk.CleanlyCreate(ctx, &policy)).To(Succeed())

Eventually(checkFunc(policy, desiredMatches, selErr)).Should(Succeed())
},
Expand All @@ -205,12 +207,12 @@ var _ = Describe("FakePolicy TargetConfigMaps", func() {
BeforeAll(beforeFunc)

DescribeTable("Verifying TargetConfigMaps behavior",
func(sel nucleusv1beta1.Target, desiredMatches []string, selErr string) {
policy := sampleFakePolicy()
func(ctx SpecContext, sel nucleusv1beta1.Target, desiredMatches []string, selErr string) {
policy := SampleFakePolicy()
policy.Spec.TargetConfigMaps = sel
policy.Spec.TargetUsingReflection = true

Expect(cleanlyCreate(&policy)).To(Succeed())
Expect(tk.CleanlyCreate(ctx, &policy)).To(Succeed())

Eventually(checkFunc(policy, desiredMatches, selErr)).Should(Succeed())
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
// Copyright Contributors to the Open Cluster Management project

package test
package basic

import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

. "open-cluster-management.io/governance-policy-nucleus/test/fakepolicy/test/utils"
)

var _ = Describe("FakePolicy CRD Validation", func() {
DescribeTable("Validating spec inputs",
func(severity, remediationAction string, include, exclude []string, isValid bool) {
policy := fromTestdata("fakepolicy-sample.yaml")
func(ctx SpecContext, severity, remediationAction string, include, exclude []string, isValid bool) {
policy := FromTestdata("fakepolicy-sample.yaml")

Expect(unstructured.SetNestedField(policy.Object,
severity, "spec", "severity")).To(Succeed())
Expand All @@ -24,8 +26,8 @@ var _ = Describe("FakePolicy CRD Validation", func() {
exclude, "spec", "namespaceSelector", "exclude")).To(Succeed())

if isValid {
Expect(cleanlyCreate(&policy)).To(Succeed())
} else if !errors.IsInvalid(cleanlyCreate(&policy)) {
Expect(tk.CleanlyCreate(ctx, &policy)).To(Succeed())
} else if !errors.IsInvalid(tk.CleanlyCreate(ctx, &policy)) {
Fail("Expected creating the policy to fail with an 'invalid' error")
}
},
Expand Down
Loading

0 comments on commit 343128b

Please sign in to comment.