Skip to content

Commit

Permalink
fix: use CreateOrUpdate function to reconcile secrets.
Browse files Browse the repository at this point in the history 
Uses the controller-runtime helper function CreateOrUpdate to reconcile
the secrets storing the certificates used by policy server.

Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com>
  • Loading branch information
@jvanz
jvanz committed May 8, 2024
1 parent 1b8b92a commit da07b27
Show file tree
Hide file tree
Showing 4 changed files with 198 additions and 229 deletions.
166 changes: 79 additions & 87 deletions internal/pkg/admission/policy-server-ca-secret.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ import (

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"
Expand All @@ -22,70 +20,57 @@ type generateCAFunc = func() (*admissionregistration.CA, error)
type pemEncodeCertificateFunc = func(certificate []byte) ([]byte, error)
type generateCertFunc = func(ca []byte, commonName string, extraSANs []string, CAPrivateKey *rsa.PrivateKey) ([]byte, []byte, error)

func (r *Reconciler) reconcileCASecret(ctx context.Context, secret *corev1.Secret) error {
err := r.Client.Create(ctx, secret)
if err == nil || apierrors.IsAlreadyExists(err) {
return nil
}

return fmt.Errorf("error reconciling policy-server CA Secret: %w", err)
}

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: policyServer.NameWithPrefix()},
&policyServerSecret)
if err != nil && apierrors.IsNotFound(err) {
secret, err := r.buildPolicyServerCASecret(policyServer, caSecret, generateCert)
if err != nil {
return secret, fmt.Errorf("cannot fetch or initialize Policy Server CA secret: %w", err)
}
return secret, nil
}
if err != nil {
return &corev1.Secret{},
fmt.Errorf("cannot fetch or initialize Policy Server CA secret: %w", err)
}

policyServerSecret.ResourceVersion = ""

return &policyServerSecret, nil
}

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", 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", policyServer.NameWithPrefix(), err)
}
secretContents := map[string]string{
constants.PolicyServerTLSCert: string(servingCert),
constants.PolicyServerTLSKey: string(servingKey),
}
secret := &corev1.Secret{
func (r *Reconciler) fetchOrInitializePolicyServerCASecret(ctx context.Context, policyServer *policiesv1.PolicyServer, caSecret *corev1.Secret, generateCert generateCertFunc) error {
policyServerSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: policyServer.NameWithPrefix(),
Namespace: r.DeploymentsNamespace,
Name: policyServer.NameWithPrefix(),
},
StringData: secretContents,
Type: corev1.SecretTypeOpaque,
}
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)
}
_, err := controllerutil.CreateOrUpdate(ctx, r.Client, policyServerSecret, func() error {
if err := controllerutil.SetOwnerReference(policyServer, policyServerSecret, r.Client.Scheme()); err != nil {
return errors.Join(fmt.Errorf("failed to set policy server secret owner reference"), err)
}

// check if secret has the required data
_, hasTLSCert := policyServerSecret.Data[constants.PolicyServerTLSCert]
_, hasTLSKey := policyServerSecret.Data[constants.PolicyServerTLSKey]
if !hasTLSCert || !hasTLSKey {
admissionregCA, err := extractCaFromSecret(caSecret)
if err != nil {
return err
}
servingCert, servingKey, err := generateCert(
admissionregCA.CaCert,
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 fmt.Errorf("cannot generate policy-server %s certificate: %w", policyServer.NameWithPrefix(), err)
}
policyServerSecret.Type = corev1.SecretTypeOpaque
policyServerSecret.StringData = map[string]string{
constants.PolicyServerTLSCert: string(servingCert),
constants.PolicyServerTLSKey: string(servingKey),
}
}

return secret, nil
return nil
})
if err != nil {
setFalseConditionType(
&policyServer.Status.Conditions,
string(policiesv1.PolicyServerCASecretReconciled),
fmt.Sprintf("error reconciling secret: %v", err),
)
return errors.Join(fmt.Errorf("cannot fetch or initialize Policy Server CA secret"), err)
}
setTrueConditionType(
&policyServer.Status.Conditions,
string(policiesv1.PolicyServerCASecretReconciled),
)

return nil
}

func extractCaFromSecret(caSecret *corev1.Secret) (*admissionregistration.CA, error) {
Expand All @@ -105,47 +90,54 @@ func extractCaFromSecret(caSecret *corev1.Secret) (*admissionregistration.CA, er
return &admissionregistration.CA{CaCert: caCert, CaPrivateKey: caPrivateKey}, nil
}

func (r *Reconciler) fetchOrInitializePolicyServerCARootSecret(ctx context.Context, generateCA generateCAFunc, pemEncodeCertificate pemEncodeCertificateFunc) (*corev1.Secret, error) {
policyServerSecret := corev1.Secret{}
err := r.Client.Get(
ctx,
client.ObjectKey{
func (r *Reconciler) fetchOrInitializePolicyServerCARootSecret(ctx context.Context, policyServer *policiesv1.PolicyServer, generateCA generateCAFunc, pemEncodeCertificate pemEncodeCertificateFunc) (*corev1.Secret, error) {
policyServerSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: r.DeploymentsNamespace,
Name: constants.PolicyServerCARootSecretName},
&policyServerSecret)
if err != nil && apierrors.IsNotFound(err) {
return r.buildPolicyServerCARootSecret(generateCA, pemEncodeCertificate)
}
policyServerSecret.ResourceVersion = ""
if err != nil {
return &corev1.Secret{},
fmt.Errorf("cannot fetch or initialize Policy Server CA secret: %w", err)
Name: constants.PolicyServerCARootSecretName,
},
}
_, err := controllerutil.CreateOrUpdate(ctx, r.Client, policyServerSecret, func() error {
_, hasCARootCert := policyServerSecret.Data[constants.PolicyServerCARootCACert]
_, hasCARootPem := policyServerSecret.Data[constants.PolicyServerCARootPemName]
_, hasCARootPrivateKey := policyServerSecret.Data[constants.PolicyServerCARootPrivateKeyCertName]

return &policyServerSecret, nil
if !hasCARootCert || !hasCARootPem || !hasCARootPrivateKey {
return updateSecretCA(policyServerSecret, generateCA, pemEncodeCertificate)
}
return nil
})
if err != nil {
setFalseConditionType(
&policyServer.Status.Conditions,
string(policiesv1.PolicyServerCARootSecretReconciled),
fmt.Sprintf("error reconciling secret: %v", err),
)
return nil, errors.Join(fmt.Errorf("cannot fetch or initialize Policy Server CA secret"), err)
}
setTrueConditionType(
&policyServer.Status.Conditions,
string(policiesv1.PolicyServerCARootSecretReconciled),
)
return policyServerSecret, nil
}

func (r *Reconciler) buildPolicyServerCARootSecret(generateCA generateCAFunc, pemEncodeCertificate pemEncodeCertificateFunc) (*corev1.Secret, error) {
func updateSecretCA(policyServerSecret *corev1.Secret, generateCA generateCAFunc, pemEncodeCertificate pemEncodeCertificateFunc) error {
caRoot, err := generateCA()
if err != nil {
return nil, fmt.Errorf("cannot generate policy-server secret CA: %w", err)
return fmt.Errorf("cannot generate policy-server secret CA: %w", err)
}
caPEMEncoded, err := pemEncodeCertificate(caRoot.CaCert)
if err != nil {
return nil, fmt.Errorf("cannot encode policy-server secret CA: %w", err)
return fmt.Errorf("cannot encode policy-server secret CA: %w", err)
}
caPrivateKeyBytes := x509.MarshalPKCS1PrivateKey(caRoot.CaPrivateKey)
secretContents := map[string][]byte{
constants.PolicyServerCARootCACert: caRoot.CaCert,
constants.PolicyServerCARootPemName: caPEMEncoded,
constants.PolicyServerCARootPrivateKeyCertName: caPrivateKeyBytes,
}
return &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: constants.PolicyServerCARootSecretName,
Namespace: r.DeploymentsNamespace,
},
Data: secretContents,
Type: corev1.SecretTypeOpaque,
}, nil
policyServerSecret.Type = corev1.SecretTypeOpaque
policyServerSecret.Data = secretContents
return nil
}
124 changes: 117 additions & 7 deletions internal/pkg/admission/policy-server-ca-secret_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@ import (
"bytes"
"context"
"crypto/rsa"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"net/http"
"sync"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/kubewarden/kubewarden-controller/internal/pkg/admissionregistration"
Expand All @@ -18,6 +22,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

const port = "8181"

func TestFetchOrInitializePolicyServerCARootSecret(t *testing.T) {
caPemBytes := []byte{}
admissionregCA, err := admissionregistration.GenerateCA()
Expand Down Expand Up @@ -49,14 +55,15 @@ func TestFetchOrInitializePolicyServerCARootSecret(t *testing.T) {
secretContents map[string][]byte
generateCACalled bool
}{
{"Existing CA", createReconcilerWithExistingCA(), nil, mockSecretContents, false},
{"Existing CA", createReconcilerWithExistingCA(), nil, mockRootCASecretContents, false},
{"CA does not exist", newReconciler(nil, false, false), nil, caSecretContents, true},
}

for _, test := range tests {
ttest := test // ensure tt is correctly scoped when used in function literal
t.Run(ttest.name, func(t *testing.T) {
secret, err := ttest.r.fetchOrInitializePolicyServerCARootSecret(context.Background(), generateCAFunc, pemEncodeCertificateFunc)
policyServer := &policiesv1.PolicyServer{}
secret, err := ttest.r.fetchOrInitializePolicyServerCARootSecret(context.Background(), policyServer, generateCAFunc, pemEncodeCertificateFunc)
if diff := cmp.Diff(secret.Data, ttest.secretContents); diff != "" {
t.Errorf("got an unexpected secret, diff %s", diff)
}
Expand Down Expand Up @@ -110,7 +117,11 @@ func TestFetchOrInitializePolicyServerSecret(t *testing.T) {
Name: "policyServer",
},
}
secret, err := ttest.r.fetchOrInitializePolicyServerCASecret(context.Background(), policyServer, caSecret, generateCertFunc)
err := ttest.r.fetchOrInitializePolicyServerCASecret(context.Background(), policyServer, caSecret, generateCertFunc)

secret := &corev1.Secret{}
_ = ttest.r.Client.Get(context.Background(), client.ObjectKey{Namespace: namespace, Name: policyServer.NameWithPrefix()}, secret)

if diff := cmp.Diff(secret.StringData, ttest.secretContents); diff != "" {
t.Errorf("got an unexpected secret, diff %s", diff)
}
Expand All @@ -127,31 +138,130 @@ func TestFetchOrInitializePolicyServerSecret(t *testing.T) {
}
}

func TestCAAndCertificateCreationInAHttpsServer(t *testing.T) {
const domain = "localhost"
const maxRetries = 10
caSecret := &corev1.Secret{}
// create CA
err := updateSecretCA(caSecret, admissionregistration.GenerateCA, admissionregistration.PemEncodeCertificate)
if err != nil {
t.Errorf("CA secret could not be created: %s", err.Error())
}
admissionregCA, err := extractCaFromSecret(caSecret)
if err != nil {
t.Errorf("CA could not be extracted from secret: %s", err.Error())
}
// create cert using CA previously created
servingCert, servingKey, err := admissionregistration.GenerateCert(
admissionregCA.CaCert,
domain,
[]string{domain},
admissionregCA.CaPrivateKey)
if err != nil {
t.Errorf("failed generating cert: %s", err.Error())
}

var server http.Server
var waitGroup sync.WaitGroup
waitGroup.Add(1)

// create https server with the certificates created
go func() {
cert, err := tls.X509KeyPair(servingCert, servingKey)
if err != nil {
t.Errorf("could not load cert: %s", err.Error())
}
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
MinVersion: tls.VersionTLS12,
}
server = http.Server{
Addr: ":" + port,
TLSConfig: tlsConfig,
ReadHeaderTimeout: time.Second,
}
waitGroup.Done()
_ = server.ListenAndServeTLS("", "")
}()

// wait for https server to be ready to avoid race conditions
waitGroup.Wait()
rootCAs := x509.NewCertPool()
rootCAs.AppendCertsFromPEM(caSecret.Data[constants.PolicyServerCARootPemName])
retries := 0
var conn *tls.Conn
for retries < maxRetries {
// test ssl handshake using the ca pem
conn, err = tls.Dial("tcp", domain+":"+port, &tls.Config{RootCAs: rootCAs, MinVersion: tls.VersionTLS12})
if err == nil || !isConnectionRefusedError(err) {
break
}
// wait 50 millisecond and retry to avoid race conditions as server might still not be ready
time.Sleep(50 * time.Millisecond)
retries++
}
if err != nil {
t.Errorf("error when connecting to the https server : %s", err.Error())
}
err = conn.Close()
if err != nil {
t.Errorf("error when closing connection : %s", err.Error())
}
err = server.Shutdown(context.Background())
if err != nil {
t.Errorf("error when shutting down https server : %s", err.Error())
}
}

func isConnectionRefusedError(err error) bool {
return err.Error() == "dial tcp [::1]:"+port+": connect: connection refused"
}

const namespace = "namespace"

var mockSecretContents = map[string][]byte{"ca": []byte("secretContents")}
var mockRootCASecretContents = map[string][]byte{
constants.PolicyServerCARootCACert: []byte("caCert"),
constants.PolicyServerCARootPemName: []byte("caPem"),
constants.PolicyServerCARootPrivateKeyCertName: []byte("caPrivateKey"),
}

var mockRootCASecretCert = map[string]string{
constants.PolicyServerCARootCACert: "caCert",
constants.PolicyServerCARootPemName: "caPem",
constants.PolicyServerCARootPrivateKeyCertName: "caPrivateKey",
}

func createReconcilerWithExistingCA() Reconciler {
mockSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: constants.PolicyServerCARootSecretName,
Namespace: namespace,
},
Data: mockSecretContents,
Type: corev1.SecretTypeOpaque,
Data: mockRootCASecretContents,
StringData: mockRootCASecretCert,
Type: corev1.SecretTypeOpaque,
}

return newReconciler([]client.Object{mockSecret}, false, false)
}

var mockSecretCert = map[string]string{"cert": "certString"}
var mockSecretContents = map[string][]byte{
constants.PolicyServerTLSCert: []byte("tlsCert"),
constants.PolicyServerTLSKey: []byte("tlsKey"),
}

var mockSecretCert = map[string]string{
constants.PolicyServerTLSCert: "tlsCert",
constants.PolicyServerTLSKey: "tlsKey",
}

func createReconcilerWithExistingCert() Reconciler {
mockSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "policy-server-policyServer",
Namespace: namespace,
},
Data: mockSecretContents,
StringData: mockSecretCert,
Type: corev1.SecretTypeOpaque,
}
Expand Down
Loading

0 comments on commit da07b27

Please sign in to comment.