Skip to content

Commit

Permalink
feat: use owner reference with policy server resources.
Browse files Browse the repository at this point in the history
Updates the controller to set owner references for all the resources
created to make the policy server works. Which includes: service,
configmap, deployment and secret. Therefore, we can remove the
reconciliation loop code to remove this resources when the user deletes
the policy server and rely on Kubernetes garbage collector to do the
job.

Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com>
  • Loading branch information
jvanz committed Apr 15, 2024
1 parent 73b58b8 commit 3abe65b
Show file tree
Hide file tree
Showing 15 changed files with 337 additions and 246 deletions.
4 changes: 0 additions & 4 deletions controllers/policyserver_controller.go
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
134 changes: 127 additions & 7 deletions controllers/policyserver_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ import (
"fmt"
"time"

. "github.com/onsi/ginkgo/v2" //nolint:revive
. "github.com/onsi/gomega" //nolint:revive
. "github.com/onsi/ginkgo/v2" //nolint:revive
. "github.com/onsi/gomega" //nolint:revive
. "github.com/onsi/gomega/gstruct" //nolint:revive
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/util/intstr"
Expand All @@ -47,6 +48,94 @@ var _ = Describe("PolicyServer controller", func() {
}, timeout, pollInterval).Should(Succeed())
})

It("should create deployment with owner reference", func() {
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() {
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() {
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() {
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())
})

Context("with no assigned policies", func() {
It("should get its finalizer removed", func() {
By("deleting the policy server")
Expand Down Expand Up @@ -137,16 +226,47 @@ 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)),
)

})

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
policyServer, err := getTestPolicyServer(policyServerName)
Expect(err).Should(Succeed())
controllerutil.RemoveFinalizer(policyServer, IntegrationTestsFinalizer)
err = reconciler.Client.Update(ctx, policyServer)
Expect(err).ToNot(HaveOccurred())

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

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

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

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

Eventually(func() error {
_, err := getTestPolicyServerConfigMap(policyServerName)
return err
}, timeout, pollInterval).ShouldNot(Succeed())
})
})
})
Expand Down
42 changes: 29 additions & 13 deletions controllers/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,35 +155,51 @@ func getTestPolicyServer(name string) (*policiesv1.PolicyServer, error) {
}

func getTestPolicyServerService(policyServerName string) (*corev1.Service, error) {
policyServer := policiesv1.PolicyServer{
ObjectMeta: metav1.ObjectMeta{
Name: policyServerName,
},
}
serviceName := policyServer.NameWithPrefix()

serviceName := getPolicyServerNameWithPrefix(policyServerName)
service := corev1.Service{}
if err := reconciler.APIReader.Get(ctx, client.ObjectKey{Name: serviceName, Namespace: DeploymentsNamespace}, &service); err != nil {
return nil, errors.Join(errors.New("could not find Service owned by PolicyServer"), err)
}
return &service, nil
}

func getTestPolicyServerDeployment(policyServerName string) (*appsv1.Deployment, error) {
policyServer := policiesv1.PolicyServer{
ObjectMeta: metav1.ObjectMeta{
Name: policyServerName,
},
func getTestPolicyServerSecret(policyServerName string) (*corev1.Secret, error) {
secretName := getPolicyServerNameWithPrefix(policyServerName)
secret := corev1.Secret{}
if err := reconciler.APIReader.Get(ctx, client.ObjectKey{Name: secretName, Namespace: DeploymentsNamespace}, &secret); err != nil {
return nil, errors.Join(errors.New("could not find secret owned by PolicyServer"), err)
}
deploymentName := policyServer.NameWithPrefix()
return &secret, nil
}

func getTestPolicyServerDeployment(policyServerName string) (*appsv1.Deployment, error) {
deploymentName := getPolicyServerNameWithPrefix(policyServerName)
deployment := appsv1.Deployment{}
if err := reconciler.APIReader.Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: DeploymentsNamespace}, &deployment); err != nil {
return nil, errors.Join(errors.New("could not find Deployment owned by PolicyServer"), err)
}
return &deployment, nil
}

func getTestPolicyServerConfigMap(policyServerName string) (*corev1.ConfigMap, error) {
configMapName := getPolicyServerNameWithPrefix(policyServerName)

configmap := corev1.ConfigMap{}
if err := reconciler.APIReader.Get(ctx, client.ObjectKey{Name: configMapName, Namespace: DeploymentsNamespace}, &configmap); err != nil {
return nil, errors.Join(errors.New("could not find ConfigMap owned by PolicyServer"), err)
}
return &configmap, nil
}

func getPolicyServerNameWithPrefix(policyServerName string) string {
policyServer := policiesv1.PolicyServer{
ObjectMeta: metav1.ObjectMeta{
Name: policyServerName,
},
}
return policyServer.NameWithPrefix()
}

func getTestPolicyServerPod(policyServerName string) (*corev1.Pod, error) {
podList := corev1.PodList{}
if err := reconciler.APIReader.List(ctx, &podList, client.MatchingLabels{
Expand Down
28 changes: 18 additions & 10 deletions internal/pkg/admission/policy-server-ca-secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ import (
"context"
"crypto/rsa"
"crypto/x509"
"errors"
"fmt"

policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/kubewarden/kubewarden-controller/internal/pkg/admissionregistration"
"github.com/kubewarden/kubewarden-controller/internal/pkg/constants"
Expand All @@ -28,16 +31,16 @@ func (r *Reconciler) reconcileCASecret(ctx context.Context, secret *corev1.Secre
return fmt.Errorf("error reconciling policy-server CA Secret: %w", err)
}

func (r *Reconciler) fetchOrInitializePolicyServerCASecret(ctx context.Context, policyServerName string, caSecret *corev1.Secret, generateCert generateCertFunc) (*corev1.Secret, error) {
func (r *Reconciler) fetchOrInitializePolicyServerCASecret(ctx context.Context, policyServer *policiesv1.PolicyServer, caSecret *corev1.Secret, generateCert generateCertFunc) (*corev1.Secret, error) {
policyServerSecret := corev1.Secret{}
err := r.Client.Get(
ctx,
client.ObjectKey{
Namespace: r.DeploymentsNamespace,
Name: policyServerName},
Name: policyServer.NameWithPrefix()},
&policyServerSecret)
if err != nil && apierrors.IsNotFound(err) {
secret, err := r.buildPolicyServerCASecret(policyServerName, caSecret, generateCert)
secret, err := r.buildPolicyServerCASecret(policyServer, caSecret, generateCert)
if err != nil {
return secret, fmt.Errorf("cannot fetch or initialize Policy Server CA secret: %w", err)
}
Expand All @@ -53,31 +56,36 @@ func (r *Reconciler) fetchOrInitializePolicyServerCASecret(ctx context.Context,
return &policyServerSecret, nil
}

func (r *Reconciler) buildPolicyServerCASecret(policyServerName string, caSecret *corev1.Secret, generateCert generateCertFunc) (*corev1.Secret, error) {
func (r *Reconciler) buildPolicyServerCASecret(policyServer *policiesv1.PolicyServer, caSecret *corev1.Secret, generateCert generateCertFunc) (*corev1.Secret, error) {
admissionregCA, err := extractCaFromSecret(caSecret)
if err != nil {
return nil, err
}
servingCert, servingKey, err := generateCert(
admissionregCA.CaCert,
fmt.Sprintf("%s.%s.svc", policyServerName, r.DeploymentsNamespace),
[]string{fmt.Sprintf("%s.%s.svc", policyServerName, r.DeploymentsNamespace)},
fmt.Sprintf("%s.%s.svc", policyServer.NameWithPrefix(), r.DeploymentsNamespace),
[]string{fmt.Sprintf("%s.%s.svc", policyServer.NameWithPrefix(), r.DeploymentsNamespace)},
admissionregCA.CaPrivateKey)
if err != nil {
return nil, fmt.Errorf("cannot generate policy-server %s certificate: %w", policyServerName, err)
return nil, fmt.Errorf("cannot generate policy-server %s certificate: %w", policyServer.NameWithPrefix(), err)

Check warning on line 70 in internal/pkg/admission/policy-server-ca-secret.go

View check run for this annotation

Codecov / codecov/patch

internal/pkg/admission/policy-server-ca-secret.go#L70

Added line #L70 was not covered by tests
}
secretContents := map[string]string{
constants.PolicyServerTLSCert: string(servingCert),
constants.PolicyServerTLSKey: string(servingKey),
}
return &corev1.Secret{
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: policyServerName,
Name: policyServer.NameWithPrefix(),
Namespace: r.DeploymentsNamespace,
},
StringData: secretContents,
Type: corev1.SecretTypeOpaque,
}, nil
}
if err := controllerutil.SetOwnerReference(policyServer, secret, r.Client.Scheme()); err != nil {
return nil, errors.Join(fmt.Errorf("failed to set policy server secret owner reference"), err)

Check warning on line 85 in internal/pkg/admission/policy-server-ca-secret.go

View check run for this annotation

Codecov / codecov/patch

internal/pkg/admission/policy-server-ca-secret.go#L85

Added line #L85 was not covered by tests
}

return secret, nil
}

func extractCaFromSecret(caSecret *corev1.Secret) (*admissionregistration.CA, error) {
Expand Down
Loading

0 comments on commit 3abe65b

Please sign in to comment.