Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use owner reference with policy server resources. #720

Merged
merged 4 commits into from
May 9, 2024
Merged

feat: use owner reference with policy server resources. #720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bc7fc72
fix: update generated base CRDs.
jvanz May 7, 2024
1b8b92a
feat: use owner reference with policy server resources.
jvanz Apr 15, 2024
da07b27
fix: use CreateOrUpdate function to reconcile secrets.
jvanz May 7, 2024
f80c28b
fix(test): fix some update calls.
jvanz May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Makefile
View file
Open in desktop
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
Open in desktop
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this coming from?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This came from a recent update in the apimachinery package. As we use label selector objects in the policy spec, the CRDs will be updated with the latest changes there as well. I've added a commit here to fix that in our main branch and avoid generating noise every time that we run the controller locally.

If you think is better, I can open another PR just for that update in the CRDs.

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
Open in desktop
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
Open in desktop

Large diffs are not rendered by default.

4 changes: 0 additions & 4 deletions controllers/policyserver_controller.go
View file
Open in desktop
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
Open in desktop
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
Loading