Skip to content

Commit

Permalink
Merge pull request #720 from jvanz/issue660-owner-references
Browse files Browse the repository at this point in the history 
feat: use owner reference with policy server resources.
  • Loading branch information
@jvanz
jvanz authored May 9, 2024
2 parents 86ef275 + f80c28b commit f562dbe
Show file tree
Hide file tree
Showing 17 changed files with 630 additions and 470 deletions.
6 changes: 3 additions & 3 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ unit-tests: manifests generate fmt vet setup-envtest ## Run unit tests.
integration-tests: manifests generate fmt vet setup-envtest ## Run integration tests.
ACK_GINKGO_DEPRECATIONS=2.12.0 K3S_TESTCONTAINER_VERSION="$(K3S_TESTCONTAINER_VERSION)" POLICY_SERVER_VERSION="$(POLICY_SERVER_VERSION)" \
go test -v ./controllers/... -ginkgo.v -ginkgo.progress -race -test.v \
-ginkgo.randomize-all -ginkgo.timeout=$(TEST_TIMEOUT) \
-coverprofile=coverage/integration-tests/coverage-controllers.txt \
-covermode=atomic -timeout=$(TEST_TIMEOUT) -coverpkg=all
-ginkgo.randomize-all -ginkgo.timeout=$(TEST_TIMEOUT) \
-coverprofile=coverage/integration-tests/coverage-controllers.txt \
-covermode=atomic -timeout=$(TEST_TIMEOUT) -coverpkg=all

.PHONY: generate-crds
generate-crds: $(KUSTOMIZE) manifests kustomize ## generate final crds with kustomize. Normally shipped in Helm charts.
Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/policies.kubewarden.io_admissionpolicies.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,13 @@ spec:
items:
type: string
type: array
x-kubernetes-list-type: atomic
required:
- key
- operator
type: object
type: array
x-kubernetes-list-type: atomic
matchLabels:
additionalProperties:
type: string
Expand Down Expand Up @@ -500,11 +502,13 @@ spec:
items:
type: string
type: array
x-kubernetes-list-type: atomic
required:
- key
- operator
type: object
type: array
x-kubernetes-list-type: atomic
matchLabels:
additionalProperties:
type: string
Expand Down
8 changes: 8 additions & 0 deletions config/crd/bases/policies.kubewarden.io_clusteradmissionpolicies.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,13 @@ spec:
items:
type: string
type: array
x-kubernetes-list-type: atomic
required:
- key
- operator
type: object
type: array
x-kubernetes-list-type: atomic
matchLabels:
additionalProperties:
type: string
Expand Down Expand Up @@ -252,11 +254,13 @@ spec:
items:
type: string
type: array
x-kubernetes-list-type: atomic
required:
- key
- operator
type: object
type: array
x-kubernetes-list-type: atomic
matchLabels:
additionalProperties:
type: string
Expand Down Expand Up @@ -605,11 +609,13 @@ spec:
items:
type: string
type: array
x-kubernetes-list-type: atomic
required:
- key
- operator
type: object
type: array
x-kubernetes-list-type: atomic
matchLabels:
additionalProperties:
type: string
Expand Down Expand Up @@ -658,11 +664,13 @@ spec:
items:
type: string
type: array
x-kubernetes-list-type: atomic
required:
- key
- operator
type: object
type: array
x-kubernetes-list-type: atomic
matchLabels:
additionalProperties:
type: string
Expand Down
135 changes: 109 additions & 26 deletions config/crd/bases/policies.kubewarden.io_policyservers.yaml
View file

Large diffs are not rendered by default.

4 changes: 0 additions & 4 deletions controllers/policyserver_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,6 @@ func (r *PolicyServerReconciler) reconcileDeletion(ctx context.Context, policySe
return r.deletePoliciesAndRequeue(ctx, policyServer, policies)
}

if err := r.Reconciler.ReconcileDeletion(ctx, policyServer); err != nil {
return ctrl.Result{}, errors.Join(errors.New("could not reconcile policy server deletion"), err)
}

controllerutil.RemoveFinalizer(policyServer, constants.KubewardenFinalizer)
if err := r.Update(ctx, policyServer); err != nil {
// return if PolicyServer was previously deleted
Expand Down
201 changes: 166 additions & 35 deletions controllers/policyserver_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,63 @@ var _ = Describe("PolicyServer controller", func() {
createPolicyServerAndWaitForItsService(policyServerFactory(policyServerName))
})

Context("with no assigned policies", func() {
It("should get its finalizer removed", func() {
Expect(
k8sClient.Delete(ctx, policyServerFactory(policyServerName)),
).To(Succeed())

Eventually(func() (*policiesv1.PolicyServer, error) {
return getTestPolicyServer(policyServerName)
}, timeout, pollInterval).ShouldNot(
HaveField("Finalizers", ContainElement(constants.KubewardenFinalizer)),
)
})

It("policy server resources should be gone after it being deleted", func() {
// It's necessary remove the test finalizer to make the
// Kubernetes garbage collector to remove the resources
Eventually(func() error {
policyServer, err := getTestPolicyServer(policyServerName)
if err != nil {
return err
}
controllerutil.RemoveFinalizer(policyServer, IntegrationTestsFinalizer)
return reconciler.Client.Update(ctx, policyServer)
}).Should(Succeed())

Expect(
k8sClient.Delete(ctx, policyServerFactory(policyServerName)),
).To(Succeed())

Eventually(func() error {
_, err := getTestPolicyServer(policyServerName)
return err
}, timeout, pollInterval).Should(notFound())

Eventually(func() error {
_, err := getTestPolicyServerService(policyServerName)
return err
}, timeout, pollInterval).Should(notFound())

Eventually(func() error {
_, err := getTestPolicyServerSecret(policyServerName)
return err
}, timeout, pollInterval).Should(notFound())

Eventually(func() error {
_, err := getTestPolicyServerDeployment(policyServerName)
return err
}, timeout, pollInterval).Should(notFound())

Eventually(func() error {
_, err := getTestPolicyServerConfigMap(policyServerName)
return err
}, timeout, pollInterval).Should(notFound())
})

})

Context("with assigned policies", func() {
var policyName string

Expand Down Expand Up @@ -100,12 +157,14 @@ var _ = Describe("PolicyServer controller", func() {
})

It(fmt.Sprintf("should get its %q finalizer removed", constants.KubewardenFinalizer), func() {
policy, err := getTestClusterAdmissionPolicy(policyName)
Expect(err).ToNot(HaveOccurred())

controllerutil.RemoveFinalizer(policy, IntegrationTestsFinalizer)
err = reconciler.Client.Update(ctx, policy)
Expect(err).ToNot(HaveOccurred())
Eventually(func() error {
policy, err := getTestClusterAdmissionPolicy(policyName)
if err != nil {
return err
}
controllerutil.RemoveFinalizer(policy, IntegrationTestsFinalizer)
return reconciler.Client.Update(ctx, policy)
}).Should(Succeed())

Expect(
k8sClient.Delete(ctx, policyServerFactory(policyServerName)),
Expand All @@ -117,33 +176,16 @@ var _ = Describe("PolicyServer controller", func() {
return err
}, timeout, pollInterval).ShouldNot(Succeed())

Eventually(func() error {
_, err := getTestPolicyServerService(policyServerName)
return err
}, timeout, pollInterval).ShouldNot(Succeed())

Eventually(func() (*policiesv1.PolicyServer, error) {
return getTestPolicyServer(policyServerName)
}, timeout, pollInterval).ShouldNot(
HaveField("Finalizers", ContainElement(constants.KubewardenFinalizer)),
)
})
})

Context("with no assigned policies", func() {
It("should get its finalizer removed", func() {
Expect(
k8sClient.Delete(ctx, policyServerFactory(policyServerName)),
).To(Succeed())

Eventually(func() (*policiesv1.PolicyServer, error) {
return getTestPolicyServer(policyServerName)
}, timeout, pollInterval).ShouldNot(
HaveField("Finalizers", ContainElement(constants.KubewardenFinalizer)),
)
})

})

})

When("creating a PolicyServer", func() {
Expand Down Expand Up @@ -306,17 +348,10 @@ var _ = Describe("PolicyServer controller", func() {
policies, err := json.Marshal(policiesMap)
Expect(err).ToNot(HaveOccurred())

Eventually(func() error {
_, err := getTestClusterAdmissionPolicy(policyName)
return err
}, timeout, pollInterval).Should(Succeed())
Eventually(func() error {
_, err := getTestPolicyServerConfigMap(policyServerName)
return err
}, timeout, pollInterval).Should(Succeed())
configMap, err := getTestPolicyServerConfigMap(policyServerName)
Expect(err).ToNot(HaveOccurred())
Expect(configMap).To(PointTo(MatchFields(IgnoreExtras, Fields{
Eventually(func() *corev1.ConfigMap {
configMap, _ := getTestPolicyServerConfigMap(policyServerName)
return configMap
}, timeout, pollInterval).Should(PointTo(MatchFields(IgnoreExtras, Fields{
"Data": MatchAllKeys(Keys{
constants.PolicyServerConfigPoliciesEntry: MatchJSON(policies),
constants.PolicyServerConfigSourcesEntry: Equal("{}"),
Expand Down Expand Up @@ -429,6 +464,102 @@ var _ = Describe("PolicyServer controller", func() {
}, timeout, pollInterval).Should(Succeed())
})

It("should create deployment with owner reference", func() {
policyServer := policyServerFactory(policyServerName)
createPolicyServerAndWaitForItsService(policyServer)
Eventually(func() error {
deployment, err := getTestPolicyServerDeployment(policyServerName)
if err != nil {
return err
}
policyServer, err := getTestPolicyServer(policyServerName)
if err != nil {
return err
}
Expect(deployment.OwnerReferences).To(ContainElement(
MatchFields(IgnoreExtras, Fields{
"UID": Equal(policyServer.GetUID()),
"Name": Equal(policyServer.GetName()),
"Kind": Equal(policyServer.GetObjectKind().GroupVersionKind().Kind),
"APIVersion": Equal(policyServer.GetObjectKind().GroupVersionKind().GroupVersion().String()),
}),
))
return nil
}).Should(Succeed())
})

It("should create configmap with owner reference", func() {
policyServer := policyServerFactory(policyServerName)
createPolicyServerAndWaitForItsService(policyServer)
Eventually(func() error {
configmap, err := getTestPolicyServerConfigMap(policyServerName)
if err != nil {
return err
}
policyServer, err := getTestPolicyServer(policyServerName)
if err != nil {
return err
}
Expect(configmap.OwnerReferences).To(ContainElement(
MatchFields(IgnoreExtras, Fields{
"UID": Equal(policyServer.GetUID()),
"Name": Equal(policyServer.GetName()),
"Kind": Equal(policyServer.GetObjectKind().GroupVersionKind().Kind),
"APIVersion": Equal(policyServer.GetObjectKind().GroupVersionKind().GroupVersion().String()),
}),
))
return nil
}).Should(Succeed())
})

It("should create service with owner reference", func() {
policyServer := policyServerFactory(policyServerName)
createPolicyServerAndWaitForItsService(policyServer)
Eventually(func() error {
service, err := getTestPolicyServerService(policyServerName)
if err != nil {
return err
}
policyServer, err := getTestPolicyServer(policyServerName)
if err != nil {
return err
}
Expect(service.OwnerReferences).To(ContainElement(
MatchFields(IgnoreExtras, Fields{
"UID": Equal(policyServer.GetUID()),
"Name": Equal(policyServer.GetName()),
"Kind": Equal(policyServer.GetObjectKind().GroupVersionKind().Kind),
"APIVersion": Equal(policyServer.GetObjectKind().GroupVersionKind().GroupVersion().String()),
}),
))
return nil
}).Should(Succeed())
})

It("should create secret with owner reference", func() {
policyServer := policyServerFactory(policyServerName)
createPolicyServerAndWaitForItsService(policyServer)
Eventually(func() error {
secret, err := getTestPolicyServerSecret(policyServerName)
if err != nil {
return err
}
policyServer, err := getTestPolicyServer(policyServerName)
if err != nil {
return err
}
Expect(secret.OwnerReferences).To(ContainElement(
MatchFields(IgnoreExtras, Fields{
"UID": Equal(policyServer.GetUID()),
"Name": Equal(policyServer.GetName()),
"Kind": Equal(policyServer.GetObjectKind().GroupVersionKind().Kind),
"APIVersion": Equal(policyServer.GetObjectKind().GroupVersionKind().GroupVersion().String()),
}),
))
return nil
}).Should(Succeed())
})

})

When("updating the PolicyServer", func() {
Expand Down
Loading

0 comments on commit f562dbe

Please sign in to comment.