Skip to content

Commit 6e2dba5

Browse files
ritazhritazh
authored
fix: update unit test for vap generation; add custom assets for envtest (#3289)
Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
1 parent 0ff1f8d commit 6e2dba5

File tree

2 files changed

+105
-53
lines changed

2 files changed

+105
-53
lines changed

‎Makefile

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,21 @@ endif
110110

111111
all: lint test manager
112112

113+
## Location to install custom assets
114+
CUSTOMENVTEST = $(LOCALBIN)/k8s/1.28.7-linux-amd64
115+
$(CUSTOMENVTEST):
116+
if [ ! -d "$(CUSTOMENVTEST)" ]; then \
117+
mkdir -p $(LOCALBIN)/k8s/1.28.7-linux-amd64; \
118+
curl -L https://sertaccdn.azureedge.net/kube-vap-fix/etcd --output $(LOCALBIN)/k8s/1.28.7-linux-amd64/etcd && chmod +x $(LOCALBIN)/k8s/1.28.7-linux-amd64/etcd; \
119+
curl -L https://sertaccdn.azureedge.net/kube-vap-fix/kube-apiserver --output $(LOCALBIN)/k8s/1.28.7-linux-amd64/kube-apiserver && chmod +x $(LOCALBIN)/k8s/1.28.7-linux-amd64/kube-apiserver; \
120+
curl -L https://sertaccdn.azureedge.net/kube-vap-fix/kubectl --output $(LOCALBIN)/k8s/1.28.7-linux-amd64/kubectl && chmod +x $(LOCALBIN)/k8s/1.28.7-linux-amd64/kubectl; \
121+
fi
113122
# Run tests
114-
native-test: envtest
115-
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(KUBERNETES_VERSION) --bin-dir $(LOCALBIN) -p path)" \
123+
# TODO(ritazh): replace custom asset when new release is available in kubebuilder
124+
# NOTE: custom asset is built from https://github.com/kubernetes/kubernetes/pull/123477 on top of 1.28.
125+
# "$(shell $(ENVTEST) use $(KUBERNETES_VERSION) --bin-dir $(LOCALBIN) -p path)"
126+
native-test: $(CUSTOMENVTEST) envtest
127+
KUBEBUILDER_ASSETS="$(CUSTOMENVTEST)" \
116128
GO111MODULE=on \
117129
go test -mod vendor ./pkg/... ./apis/... ./cmd/gator/... -race -bench . -coverprofile cover.out
118130

‎pkg/controller/constrainttemplate/constrainttemplate_controller_test.go

Lines changed: 91 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ import (
5252
"k8s.io/apimachinery/pkg/types"
5353
"k8s.io/client-go/kubernetes"
5454
"k8s.io/client-go/util/retry"
55-
"k8s.io/utils/ptr"
5655
"sigs.k8s.io/controller-runtime/pkg/client"
5756
"sigs.k8s.io/controller-runtime/pkg/event"
5857
)
@@ -97,7 +96,9 @@ violation[{"msg": "denied!"}] {
9796

9897
func makeReconcileConstraintTemplateForVap(suffix string, labels map[string]string) *v1beta1.ConstraintTemplate {
9998
source := &celSchema.Source{
100-
FailurePolicy: ptr.To[string]("Fail"),
99+
// FailurePolicy: ptr.To[string]("Fail"),
100+
// TODO(ritazh): enable fail when VAP reduces 30s discovery of CRDs
101+
// due to discovery mechanism to pickup the change to the CRD list
101102
MatchConditions: []celSchema.MatchCondition{
102103
{
103104
Name: "must_match_something",
@@ -342,44 +343,83 @@ func TestReconcile(t *testing.T) {
342343
t.Fatal(err)
343344
}
344345
})
345-
// TODO(ritazh): uncomment this test after the fix for https://github.com/kubernetes/kubernetes/issues/122658 makes its way to a k8s release
346-
// t.Run("VapBinding should be created", func(t *testing.T) {
347-
// suffix := "VapBindingShouldBeCreated"
348-
349-
// logger.Info("Running test: VapBinding should be created")
350-
// labels := map[string]string{
351-
// constraint.VapGenerationLabel: constraint.Yes,
352-
// }
353-
// constraintTemplate := makeReconcileConstraintTemplateForVap(suffix, labels)
354-
// cstr := newDenyAllCstrWithLabel(suffix, labels)
355-
// t.Cleanup(testutils.DeleteObjectAndConfirm(ctx, t, c, expectedCRD(suffix)))
356-
// testutils.CreateThenCleanup(ctx, t, c, constraintTemplate)
357-
358-
// err = retry.OnError(testutils.ConstantRetry, func(err error) bool {
359-
// return true
360-
// }, func() error {
361-
// // check if vap resource exists now
362-
// vap := &admissionregistrationv1beta1.ValidatingAdmissionPolicy{}
363-
// vapName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix))
364-
// if err := c.Get(ctx, types.NamespacedName{Name: vapName}, vap); err != nil {
365-
// return err
366-
// }
367-
// return c.Create(ctx, cstr)
368-
// })
369-
// if err != nil {
370-
// logger.Error(err, "get vap and create cstr")
371-
// t.Fatal(err)
372-
// }
373-
// logger.Info("cstr created")
374-
// // check if vapbinding resource exists now
375-
// vapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{}
376-
// vapBindingName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix))
377-
// if err := c.Get(ctx, types.NamespacedName{Name: vapBindingName}, vapBinding); err != nil {
378-
// logger.Error(err, "get vapBinding")
379-
// t.Fatal(err)
380-
// }
381-
// logger.Info("vapbinding found")
382-
// })
346+
t.Run("VapBinding should be created", func(t *testing.T) {
347+
suffix := "VapBindingShouldBeCreated"
348+
349+
logger.Info("Running test: VapBinding should be created")
350+
labels := map[string]string{
351+
constraint.VapGenerationLabel: constraint.Yes,
352+
}
353+
constraintTemplate := makeReconcileConstraintTemplateForVap(suffix, labels)
354+
cstr := newDenyAllCstrWithLabel(suffix, labels)
355+
t.Cleanup(testutils.DeleteObjectAndConfirm(ctx, t, c, expectedCRD(suffix)))
356+
testutils.CreateThenCleanup(ctx, t, c, constraintTemplate)
357+
358+
err = retry.OnError(testutils.ConstantRetry, func(err error) bool {
359+
return true
360+
}, func() error {
361+
// check if vap resource exists now
362+
vap := &admissionregistrationv1beta1.ValidatingAdmissionPolicy{}
363+
vapName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix))
364+
if err := c.Get(ctx, types.NamespacedName{Name: vapName}, vap); err != nil {
365+
return err
366+
}
367+
return c.Create(ctx, cstr)
368+
})
369+
if err != nil {
370+
logger.Error(err, "get vap and create cstr")
371+
t.Fatal(err)
372+
}
373+
// check if vapbinding resource exists now
374+
vapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{}
375+
vapBindingName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix))
376+
if err := c.Get(ctx, types.NamespacedName{Name: vapBindingName}, vapBinding); err != nil {
377+
logger.Error(err, "get vapBinding")
378+
t.Fatal(err)
379+
}
380+
})
381+
382+
t.Run("VapBinding should not be created", func(t *testing.T) {
383+
suffix := "VapBindingShouldNotBeCreated"
384+
385+
logger.Info("Running test: VapBinding should not be created")
386+
labels := map[string]string{
387+
constraint.VapGenerationLabel: constraint.Yes,
388+
}
389+
constraintLabels := map[string]string{
390+
constraint.VapGenerationLabel: constraint.No,
391+
}
392+
constraintTemplate := makeReconcileConstraintTemplateForVap(suffix, labels)
393+
cstr := newDenyAllCstrWithLabel(suffix, constraintLabels)
394+
t.Cleanup(testutils.DeleteObjectAndConfirm(ctx, t, c, expectedCRD(suffix)))
395+
testutils.CreateThenCleanup(ctx, t, c, constraintTemplate)
396+
397+
err = retry.OnError(testutils.ConstantRetry, func(err error) bool {
398+
return true
399+
}, func() error {
400+
// check if vap resource exists now
401+
vap := &admissionregistrationv1beta1.ValidatingAdmissionPolicy{}
402+
vapName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix))
403+
if err := c.Get(ctx, types.NamespacedName{Name: vapName}, vap); err != nil {
404+
return err
405+
}
406+
return c.Create(ctx, cstr)
407+
})
408+
if err != nil {
409+
logger.Error(err, "get vap and create cstr")
410+
t.Fatal(err)
411+
}
412+
// check if vapbinding resource exists now
413+
vapBinding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{}
414+
vapBindingName := fmt.Sprintf("gatekeeper-%s", denyall+strings.ToLower(suffix))
415+
if err := c.Get(ctx, types.NamespacedName{Name: vapBindingName}, vapBinding); err != nil {
416+
if !apierrors.IsNotFound(err) {
417+
t.Fatal(err)
418+
}
419+
} else {
420+
t.Fatal("should result in error, vapbinding not found")
421+
}
422+
})
383423

384424
t.Run("Constraint is marked as enforced", func(t *testing.T) {
385425
suffix := "MarkedEnforced"
@@ -843,17 +883,17 @@ func newDenyAllCstr(suffix string) *unstructured.Unstructured {
843883
return cstr
844884
}
845885

846-
// func newDenyAllCstrWithLabel(suffix string, labels map[string]string) *unstructured.Unstructured {
847-
// cstr := &unstructured.Unstructured{}
848-
// cstr.SetGroupVersionKind(schema.GroupVersionKind{
849-
// Group: "constraints.gatekeeper.sh",
850-
// Version: "v1beta1",
851-
// Kind: DenyAll + suffix,
852-
// })
853-
// cstr.SetName("denyallconstraintforvapbinding")
854-
// cstr.SetLabels(labels)
855-
// return cstr
856-
// }
886+
func newDenyAllCstrWithLabel(suffix string, labels map[string]string) *unstructured.Unstructured {
887+
cstr := &unstructured.Unstructured{}
888+
cstr.SetGroupVersionKind(schema.GroupVersionKind{
889+
Group: "constraints.gatekeeper.sh",
890+
Version: "v1beta1",
891+
Kind: DenyAll + suffix,
892+
})
893+
cstr.SetName(denyall + strings.ToLower(suffix))
894+
cstr.SetLabels(labels)
895+
return cstr
896+
}
857897

858898
func getCTByPodStatus(templ *v1beta1.ConstraintTemplate) (v1beta1.ByPodStatus, bool) {
859899
statuses := templ.Status.ByPod

0 commit comments

Comments
 (0)