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.

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: create root ca. #510

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Build the manager binary
FROM golang:1.19 as builder
FROM golang:1.20 as builder
jvanz marked this conversation as resolved.
Show resolved Hide resolved

WORKDIR /workspace
# Copy the Go Modules manifests
Expand Down
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ setup-envtest: $(SETUP_ENVTEST) # Build setup-envtest
unit-tests: manifests generate fmt vet setup-envtest ## Run unit tests.
KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test ./internal/... -test.v -coverprofile cover.out
KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test ./pkg/... -test.v -coverprofile cover.out
KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test . -test.v -coverprofile cover.out

.PHONY: setup-envtest integration-tests
integration-tests: manifests generate fmt vet setup-envtest ## Run integration tests.
Expand All @@ -124,7 +125,7 @@ build: generate fmt vet ## Build manager binary.
go build -o bin/manager .

run: manifests generate fmt vet ## Run a controller from your host.
go run . -deployments-namespace kubewarden --default-policy-server default
go run . -deployments-namespace kubewarden --default-policy-server default -zap-log-level debug

docker-build: test ## Build docker image with the manager.
docker build -t ${IMG} .
Expand Down
11 changes: 7 additions & 4 deletions controllers/admissionpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ var _ = Describe("Given an AdmissionPolicy", func() {
Context("and it has a non-empty policy server set on its spec", func() {
var (
policyNamespace = someNamespace.Name
policyName = "scheduled-policy"
policyServerName = "some-policy-server"
policyName = "scheduled-policy"
)
BeforeEach(func() {
Expect(
Expand All @@ -76,16 +76,19 @@ var _ = Describe("Given an AdmissionPolicy", func() {
func(admissionPolicy *policiesv1.AdmissionPolicy) policiesv1.PolicyStatusEnum {
return admissionPolicy.Status.PolicyStatus
},
Equal(policiesv1.PolicyStatusScheduled),
),
)
Equal(policiesv1.PolicyStatusScheduled)))
})
Context("and the targeted policy server is created", func() {
BeforeEach(func() {
Expect(
k8sClient.Create(ctx, policyServer(policyServerName)),
).To(HaveSucceededOrAlreadyExisted())
})
AfterEach(func() {
Expect(
k8sClient.Delete(ctx, policyServer(policyServerName)),
).To(Succeed())
})
It(fmt.Sprintf("should set its policy status to %q", policiesv1.PolicyStatusPending), func() {
Eventually(func(g Gomega) (*policiesv1.AdmissionPolicy, error) {
return getFreshAdmissionPolicy(policyNamespace, policyName)
Expand Down
2 changes: 1 addition & 1 deletion controllers/clusteradmissionpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (r *ClusterAdmissionPolicyReconciler) Reconcile(ctx context.Context, req ct
if apierrors.IsNotFound(err) {
return ctrl.Result{}, nil
}
return ctrl.Result{}, fmt.Errorf("cannot retrieve admission policy: %w", err)
return ctrl.Result{}, fmt.Errorf("cannot retrieve cluster admission policy: %w", err)
}

return startReconciling(ctx, r.Reconciler.Client, r.Reconciler, &clusterAdmissionPolicy)
Expand Down
9 changes: 6 additions & 3 deletions controllers/clusteradmissionpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ import (
"fmt"
"time"

// "github.com/kubewarden/kubewarden-controller/internal/pkg/constants"
policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
// admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
// "sigs.k8s.io/controller-runtime/pkg/client"
jvanz marked this conversation as resolved.
Show resolved Hide resolved
)

var _ = Describe("Given a ClusterAdmissionPolicy", func() {
Expand Down Expand Up @@ -54,11 +57,11 @@ var _ = Describe("Given a ClusterAdmissionPolicy", func() {
var (
policyName = "scheduled-policy"
policyServerName = "other-policy-server"
policy *policiesv1.ClusterAdmissionPolicy
)
BeforeEach(func() {
Expect(
k8sClient.Create(ctx, clusterAdmissionPolicyWithPolicyServerName(policyName, policyServerName)),
).To(HaveSucceededOrAlreadyExisted())
policy = clusterAdmissionPolicyWithPolicyServerName(policyName, policyServerName)
Expect(k8sClient.Create(ctx, policy)).To(HaveSucceededOrAlreadyExisted())
})
It(fmt.Sprintf("should set its policy status to %q", policiesv1.PolicyStatusScheduled), func() {
Eventually(func(g Gomega) (*policiesv1.ClusterAdmissionPolicy, error) {
Expand Down
10 changes: 8 additions & 2 deletions controllers/policy_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ func startReconciling(ctx context.Context, client client.Client, reconciler admi

_ = setPolicyStatus(ctx, reconciler.DeploymentsNamespace, reconciler.APIReader, policy)
if err := client.Status().Update(ctx, policy); err != nil {
if apierrors.IsConflict(err) {
return ctrl.Result{
Requeue: true,
RequeueAfter: time.Second * 5,
}, nil
}
return ctrl.Result{}, fmt.Errorf("update admission policy status error: %w", err)
}

Expand Down Expand Up @@ -144,8 +150,8 @@ func reconcilePolicy(ctx context.Context, client client.Client, reconciler admis
)

secret := corev1.Secret{}
if err := client.Get(ctx, types.NamespacedName{Namespace: reconciler.DeploymentsNamespace, Name: constants.PolicyServerCARootSecretName}, &secret); err != nil {
return ctrl.Result{}, errors.Wrap(err, "cannot find policy server secret")
if err := client.Get(ctx, types.NamespacedName{Namespace: reconciler.DeploymentsNamespace, Name: constants.KubewardenCARootSecretName}, &secret); err != nil {
return ctrl.Result{}, errors.Wrap(err, "cannot find root CA secret")
}

if policy.IsMutating() {
Expand Down
28 changes: 28 additions & 0 deletions controllers/policyserver_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/kubewarden/kubewarden-controller/internal/pkg/constants"
policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -80,6 +81,12 @@ func (r *PolicyServerReconciler) Reconcile(ctx context.Context, req ctrl.Request
reconcileResult, reconcileErr := r.reconcile(ctx, &policyServer, policies)

if err := r.Client.Status().Update(ctx, &policyServer); err != nil {
if apierrors.IsConflict(err) {
return ctrl.Result{
Requeue: true,
RequeueAfter: time.Second * 5,
}, nil
}
return ctrl.Result{}, fmt.Errorf("update policy server status error: %w", err)
}

Expand Down Expand Up @@ -191,6 +198,27 @@ func (r *PolicyServerReconciler) SetupWithManager(mgr ctrl.Manager) error {
},
}
})).
Watches(&source.Kind{Type: &corev1.Secret{}}, handler.EnqueueRequestsFromMapFunc(func(object client.Object) []reconcile.Request {
// watches for secret to detect when a policy server
// secret change. Therefore, the certificate can be
// recreated and the webhooks updated
Comment on lines +203 to +204
Copy link
Member

Choose a reason for hiding this comment

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

I'm still going through the whole PR, hence I might be wrong...

the certificate can be recreated

AFAIK the certificate is inside of the secret, hence the only thing to recreate (actually restart) would be the Deployment running Policy Server (so that it picks the new certificate).

Moreover:

the webhooks updated

The webhooks should not be updated when the certificate of the Policy Server is changed. That's because they are registered with the root CA certificate specified, not with the Policy Server certificate.
The only exception to this "rule" is when the root CA is updated, leading to a change of the Policy Server certificate. Only in this case the webhooks should be updated.

I haven't read the code yet, maybe all these things are already done. Nevertheless, the comment should be changed

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is what is done in the controller. The comment is wrong. I'm still walkingthrouhg your comments. But this is the goal. The webhooks always use the root CA to validate the server certificate and the policy server deployment will rollout a new version when the secret version change (indicating that policy server certificate change)

secret, ok := object.(*corev1.Secret)
if !ok {
r.Log.Info("object is not type of secret: %+v", "secret", secret)
return []ctrl.Request{}
}

if policyServerName, isPolicyServerSecret := secret.Labels[constants.PolicyServerLabelKey]; isPolicyServerSecret {
return []ctrl.Request{
{
NamespacedName: client.ObjectKey{
Name: policyServerName,
},
},
}
}
return []ctrl.Request{}
})).
Complete(r)

return errors.Wrap(err, "failed enrolling controller with manager")
Expand Down
93 changes: 92 additions & 1 deletion controllers/policyserver_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,110 @@ import (
policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
appsv1 "k8s.io/api/apps/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"

"github.com/kubewarden/kubewarden-controller/internal/pkg/constants"
)

var _ = Describe("Given a PolicyServer", func() {
var (
policyServerName = "policy-server"
policyServerName = "policy-server"
policyServerNameWithPrefix = policyServer(policyServerName).NameWithPrefix()
)
BeforeEach(func() {
Expect(
k8sClient.Create(ctx, policyServer(policyServerName)),
).To(HaveSucceededOrAlreadyExisted())
})
Context("policy server certificate", func() {
It("a secret for the policy server certificate should be created", func() {
Eventually(func(g Gomega) ([]string, error) {
secret := &corev1.Secret{}
err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, secret)
if err != nil {
return []string{}, fmt.Errorf("failed to get policy server certificate secret: %s", err.Error())
}
dataKeys := []string{}
for key := range secret.Data {
dataKeys = append(dataKeys, key)
}
return dataKeys, nil
}, 30*time.Second, 250*time.Millisecond).Should(Equal([]string{constants.PolicyServerTLSCert, constants.PolicyServerTLSKey}))
})
It("policy server should have a label with the latest certificate secret resource version", func() {
Eventually(func(g Gomega) bool {
secret := &corev1.Secret{}
err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, secret)
Expect(err).ToNot(HaveOccurred())

policyServerDeploy := &appsv1.Deployment{}
err = k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, policyServerDeploy)
Expect(err).ToNot(HaveOccurred())

return secret.GetResourceVersion() == policyServerDeploy.Spec.Template.Labels[constants.PolicyServerCertificateSecret]
}, 30*time.Second, 250*time.Millisecond).Should(Equal(true))
})
})
When("policy server secret is deleted", func() {
BeforeEach(func() {
Expect(
k8sClient.Delete(ctx, &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: policyServerNameWithPrefix,
Namespace: DeploymentsNamespace,
},
})).To(Succeed())
Eventually(func(g Gomega) bool {
err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, &corev1.Secret{})
return apierrors.IsNotFound(err)
}, 30*time.Second, 250*time.Millisecond).Should(BeTrue())
})
It("it should be recreated", func() {
Eventually(func(g Gomega) ([]string, error) {
secret := &corev1.Secret{}
err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, secret)
if err != nil {
return []string{}, fmt.Errorf("failed to get policy server certificate secret: %s", err.Error())
}
dataKeys := []string{}
for key := range secret.Data {
dataKeys = append(dataKeys, key)
}
return dataKeys, nil
}, 30*time.Second, 250*time.Millisecond).Should(Equal([]string{constants.PolicyServerTLSCert, constants.PolicyServerTLSKey}))
})
It("policy server should have a label with the latest certificate secret resource version", func() {
Eventually(func(g Gomega) (bool, error) {
secret := &corev1.Secret{}
if err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, secret); err != nil {
return false, fmt.Errorf("failed to get policy server certificate secret: $%s", err.Error())
}

policyServerDeploy := &appsv1.Deployment{}
if err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, policyServerDeploy); err != nil {
return false, fmt.Errorf("failed to get policy server deployment: %s", err.Error())
}

return secret.GetResourceVersion() == policyServerDeploy.Spec.Template.Labels[constants.PolicyServerCertificateSecret], nil
}, 30*time.Second, 250*time.Millisecond).Should(Equal(true))
})
})
When("policy server is deleted", func() {
It("its secret should be deleted as well", func() {
Expect(
k8sClient.Delete(ctx, policyServer(policyServerName)),
).To(Succeed())

Eventually(func(g Gomega) bool {
err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, &corev1.Secret{})
return apierrors.IsNotFound(err)
}, 30*time.Second, 250*time.Millisecond).Should(BeTrue())
})
})
When("it has no assigned policies", func() {
Context("and it is deleted", func() {
BeforeEach(func() {
Expand Down Expand Up @@ -64,6 +154,7 @@ var _ = Describe("Given a PolicyServer", func() {
k8sClient.Create(ctx, clusterAdmissionPolicyWithPolicyServerName(policyName, policyServerName)),
).To(HaveSucceededOrAlreadyExisted())
})

Context("and it is deleted", func() {
BeforeEach(func() {
Expect(
Expand Down
32 changes: 32 additions & 0 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controllers

import (
"context"
"crypto/x509"
"log"
"path/filepath"
"testing"
Expand All @@ -36,6 +37,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log/zap"

"github.com/kubewarden/kubewarden-controller/internal/pkg/admission"
"github.com/kubewarden/kubewarden-controller/internal/pkg/admissionregistration"
"github.com/kubewarden/kubewarden-controller/internal/pkg/constants"
//+kubebuilder:scaffold:imports
)

Expand All @@ -48,6 +51,7 @@ var testEnv *envtest.Environment
var ctx context.Context
var cancel context.CancelFunc
var reconciler admission.Reconciler
var caRootSecret *corev1.Secret

const (
DeploymentsNamespace = "kubewarden-integration-tests"
Expand Down Expand Up @@ -125,6 +129,34 @@ var _ = BeforeSuite(func() {
log.Fatalf("could not create namespace %q needed for the integration tests", DeploymentsNamespace)
}

// Create the root CA generated in the main of the controller
caRoot, err := admissionregistration.GenerateCA()
if err != nil {
log.Fatalf("cannot generate policy-server secret CA")
}
caPEMEncoded, err := admissionregistration.PemEncodeCertificate(caRoot.CaCert)
if err != nil {
log.Fatalf("cannot encode policy-server secret CA")
}
caPrivateKeyBytes := x509.MarshalPKCS1PrivateKey(caRoot.CaPrivateKey)
caRootSecret = &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: constants.KubewardenCARootSecretName,
Namespace: DeploymentsNamespace,
},
Data: map[string][]byte{
constants.CARootCACert: caRoot.CaCert,
constants.CARootCACertPem: caPEMEncoded,
constants.CARootPrivateKeyCertName: caPrivateKeyBytes,
},
Type: corev1.SecretTypeOpaque,
}

// Create the integration tests deployments namespace
if err := k8sClient.Create(ctx, caRootSecret); err != nil {
log.Fatalf("could not create root CA secret: ")
}

go func() {
defer GinkgoRecover()
err = k8sManager.Start(ctx)
Expand Down
2 changes: 1 addition & 1 deletion controllers/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ var (
}
)

func AlreadyExists() types.GomegaMatcher { //nolint:ireturn
func AlreadyExists() types.GomegaMatcher { //nolint:ireturn,nolintlint
return WithTransform(
func(err error) bool {
return err != nil && apierrors.IsAlreadyExists(err)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ module github.com/kubewarden/kubewarden-controller
go 1.20

require (
github.com/ereslibre/kube-webhook-wrapper v0.0.2
github.com/go-logr/logr v1.2.4
github.com/google/go-cmp v0.5.9
github.com/onsi/ginkgo/v2 v2.9.2
Expand Down Expand Up @@ -46,6 +45,7 @@ require (
github.com/josharian/intern v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/kr/pretty v0.2.1 // indirect
github.com/kubewarden/kube-webhook-wrapper v0.0.0-20230830124812-3fdf2b58aa57
github.com/mailru/easyjson v0.7.7 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,6 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20210217033140-668b12f5399d/go.m
github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.mod h1:hliV/p42l8fGbc6Y9bQ70uLwIvmJyVE5k4iMKlh8wCQ=
github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.mod h1:AFq3mo9L8Lqqiid3OhADV3RfLJnjiw63cSpi+fDTRC0=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/ereslibre/kube-webhook-wrapper v0.0.2 h1:GaSN5jfSPJV7KNuVzSFflS5Rr5wQNKVsZbL6Gq1ZHYw=
github.com/ereslibre/kube-webhook-wrapper v0.0.2/go.mod h1:vDSrlA2/6bFRlDdVCC0Woe21U5J9s9a7FfSgaxuTPv4=
github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ=
github.com/evanphx/json-patch v4.11.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U=
Expand Down Expand Up @@ -332,6 +330,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/kubewarden/kube-webhook-wrapper v0.0.0-20230830124812-3fdf2b58aa57 h1:vfvTjvPzMzc4Y+icuoAW7uITZvcMJpPQMgY0KfjY93s=
github.com/kubewarden/kube-webhook-wrapper v0.0.0-20230830124812-3fdf2b58aa57/go.mod h1:2VL5IDaVqv0klHS7TwpKGzFXs+1h9RvjF0qEvxN+TH4=
github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/admission/mutating-webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (r *Reconciler) mutatingWebhookConfiguration(
Name: fmt.Sprintf("%s.kubewarden.admission", policy.GetUniqueName()),
ClientConfig: admissionregistrationv1.WebhookClientConfig{
Service: &service,
CABundle: admissionSecret.Data[constants.PolicyServerCARootPemName],
CABundle: admissionSecret.Data[constants.CARootCACertPem],
},
Rules: policy.GetRules(),
FailurePolicy: policy.GetFailurePolicy(),
Expand Down
Loading
Loading