Address review comments

On-behalf-of: SAP <[email protected]>
Signed-off-by: Marvin Beckers <[email protected]>

README.md

@@ -34,7 +34,7 @@ kcp-operator will create the necessary resources to start a `Deployment` of a kc
 
 ### Certificate Management
 
-Since the operator supports multiple rootshards and frontproxies, its certificate structure differs from the helm chart slightly. The placeholders `$rootshard` and `$frontproxy` in the chart are used to denote the name of the corresponding operator resource.
+The placeholders `$rootshard` and `$frontproxy` in the chart are used to denote the name of the corresponding operator resource.
 
 ```mermaid
 graph TB

config/manager/kustomization.yaml

@@ -1,7 +1,7 @@
-resources:
-- manager.yaml
 apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
+resources:
+- manager.yaml
 images:
 - name: controller
   newName: ghcr.io/kcp-dev/kcp-operator

internal/controller/frontproxy_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 
+	certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	k8creconciling "k8c.io/reconciler/pkg/reconciling"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -54,6 +55,8 @@ func (r *FrontProxyReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Owns(&appsv1.Deployment{}).
 		Owns(&corev1.ConfigMap{}).
 		Owns(&corev1.Secret{}).
+		Owns(&corev1.Service{}).
+		Owns(&certmanagerv1.Certificate{}).
 		Complete(r)
 }
 
@@ -71,7 +74,7 @@ func (r *FrontProxyReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	var frontProxy operatorv1alpha1.FrontProxy
 	if err := r.Client.Get(ctx, req.NamespacedName, &frontProxy); err != nil {
 		if client.IgnoreNotFound(err) != nil {
-			return ctrl.Result{}, fmt.Errorf("failed to find %s/%s: %w", req.Namespace, req.Name, err)
+			return ctrl.Result{}, fmt.Errorf("failed to get FrontProxy object: %w", err)
 		}
 
 		// Object has apparently been deleted already.
@@ -93,18 +96,17 @@ func (r *FrontProxyReconciler) reconcile(ctx context.Context, frontProxy *operat
 	ownerRefWrapper := k8creconciling.OwnerRefWrapper(*metav1.NewControllerRef(frontProxy, operatorv1alpha1.SchemeGroupVersion.WithKind("FrontProxy")))
 
 	ref := frontProxy.Spec.RootShard.Reference
-	rootShard := &operatorv1alpha1.RootShard{}
-	switch {
-	case ref != nil:
-		if err := r.Client.Get(ctx, types.NamespacedName{Name: ref.Name, Namespace: frontProxy.Namespace}, rootShard); err != nil {
-			return fmt.Errorf("referenced RootShard '%s' could not be fetched", ref.Name)
-		}
-	default:
+	if ref == nil {
 		return fmt.Errorf("no valid RootShard in FrontProxy spec defined")
 	}
 
+	rootShard := &operatorv1alpha1.RootShard{}
+	if err := r.Client.Get(ctx, types.NamespacedName{Name: ref.Name, Namespace: frontProxy.Namespace}, rootShard); err != nil {
+		return fmt.Errorf("referenced RootShard '%s' could not be fetched", ref.Name)
+	}
+
 	configMapReconcilers := []k8creconciling.NamedConfigMapReconcilerFactory{
-		frontproxy.ConfigmapReconciler(frontProxy, rootShard),
+		frontproxy.PathMappingConfigMapReconciler(frontProxy, rootShard),
 	}
 
 	secretReconcilers := []k8creconciling.NamedSecretReconcilerFactory{
@@ -113,9 +115,9 @@ func (r *FrontProxyReconciler) reconcile(ctx context.Context, frontProxy *operat
 
 	certReconcilers := []reconciling.NamedCertificateReconcilerFactory{
 		frontproxy.ServerCertificateReconciler(frontProxy, rootShard),
-		frontproxy.KubeconfigReconciler(frontProxy, rootShard),
-		frontproxy.AdminKubeconfigReconciler(frontProxy, rootShard),
-		frontproxy.RequestHeaderReconciler(frontProxy, rootShard),
+		frontproxy.KubeconfigCertificateReconciler(frontProxy, rootShard),
+		frontproxy.AdminKubeconfigCertificateReconciler(frontProxy, rootShard),
+		frontproxy.RequestHeaderCertificateReconciler(frontProxy, rootShard),
 	}
 
 	deploymentReconcilers := []k8creconciling.NamedDeploymentReconcilerFactory{

internal/controller/frontproxy_controller_test.go

@@ -23,7 +23,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -40,7 +40,7 @@ var _ = Describe("FrontProxy Controller", func() {
 
 		typeNamespacedName := types.NamespacedName{
 			Name:      resourceName,
-			Namespace: "default", // TODO(user):Modify as needed
+			Namespace: "default",
 		}
 		frontproxy := &operatorv1alpha1.FrontProxy{}
 		rootShard := &operatorv1alpha1.RootShard{}
@@ -55,8 +55,8 @@ var _ = Describe("FrontProxy Controller", func() {
 			if err != nil && errors.IsNotFound(err) {
 				rootShard = &operatorv1alpha1.RootShard{
 					ObjectMeta: metav1.ObjectMeta{
-						Name:      fmt.Sprintf("rootshard-%s", resourceName),
-						Namespace: "default",
+						Name:      rootShardNamespacedName.Name,
+						Namespace: rootShardNamespacedName.Namespace,
 					},
 					Spec: operatorv1alpha1.RootShardSpec{
 						External: operatorv1alpha1.ExternalConfig{
@@ -78,12 +78,12 @@ var _ = Describe("FrontProxy Controller", func() {
 			if err != nil && errors.IsNotFound(err) {
 				resource := &operatorv1alpha1.FrontProxy{
 					ObjectMeta: metav1.ObjectMeta{
-						Name:      resourceName,
-						Namespace: "default",
+						Name:      typeNamespacedName.Name,
+						Namespace: typeNamespacedName.Namespace,
 					},
 					Spec: operatorv1alpha1.FrontProxySpec{
 						RootShard: operatorv1alpha1.RootShardConfig{
-							Reference: &v1.LocalObjectReference{
+							Reference: &corev1.LocalObjectReference{
 								Name: rootShard.Name,
 							},
 						},

internal/controller/rootshard_controller.go

@@ -106,20 +106,17 @@ func (r *RootShardReconciler) reconcile(ctx context.Context, rootShard *operator
 		v1alpha1.RequestHeaderClientCA,
 		v1alpha1.ClientCA,
 		v1alpha1.ServiceAccountCA,
+		v1alpha1.FrontProxyClientCA,
 	}
 
 	issuerReconcilers := []reconciling.NamedIssuerReconcilerFactory{
 		rootshard.RootCAIssuerReconciler(rootShard),
-		rootshard.ClientCAIssuerReconciler(rootShard),
-		rootshard.FrontProxyClientCAIssuerReconciler(rootShard),
 	}
 
 	certReconcilers := []reconciling.NamedCertificateReconcilerFactory{
 		rootshard.ServerCertificateReconciler(rootShard),
 		rootshard.ServiceAccountCertificateReconciler(rootShard),
 		rootshard.VirtualWorkspacesCertificateReconciler(rootShard),
-		rootshard.ClientCACertificateReconciler(rootShard),
-		rootshard.FrontProxyClientCACertificateReconciler(rootShard),
 	}
 
 	for _, ca := range intermediateCAs {

internal/resources/frontproxy/certificates.go

@@ -67,7 +67,7 @@ func ServerCertificateReconciler(frontproxy *operatorv1alpha1.FrontProxy, rootsh
 	}
 }
 
-func AdminKubeconfigReconciler(frontproxy *operatorv1alpha1.FrontProxy, rootshard *operatorv1alpha1.RootShard) reconciling.NamedCertificateReconcilerFactory {
+func AdminKubeconfigCertificateReconciler(frontproxy *operatorv1alpha1.FrontProxy, rootshard *operatorv1alpha1.RootShard) reconciling.NamedCertificateReconcilerFactory {
 	name := resources.GetFrontProxyCertificateName(rootshard, frontproxy, operatorv1alpha1.AdminKubeconfigClientCertificate)
 
 	return func() (string, reconciling.CertificateReconciler) {
@@ -105,7 +105,7 @@ func AdminKubeconfigReconciler(frontproxy *operatorv1alpha1.FrontProxy, rootshar
 	}
 }
 
-func KubeconfigReconciler(frontproxy *operatorv1alpha1.FrontProxy, rootshard *operatorv1alpha1.RootShard) reconciling.NamedCertificateReconcilerFactory {
+func KubeconfigCertificateReconciler(frontproxy *operatorv1alpha1.FrontProxy, rootshard *operatorv1alpha1.RootShard) reconciling.NamedCertificateReconcilerFactory {
 	name := resources.GetFrontProxyCertificateName(rootshard, frontproxy, operatorv1alpha1.KubeconfigCertificate)
 
 	return func() (string, reconciling.CertificateReconciler) {
@@ -143,8 +143,8 @@ func KubeconfigReconciler(frontproxy *operatorv1alpha1.FrontProxy, rootshard *op
 	}
 }
 
-func RequestHeaderReconciler(frontproxy *operatorv1alpha1.FrontProxy, rootshard *operatorv1alpha1.RootShard) reconciling.NamedCertificateReconcilerFactory {
-	name := resources.GetFrontProxyRequestheaderName(rootshard, frontproxy)
+func RequestHeaderCertificateReconciler(frontproxy *operatorv1alpha1.FrontProxy, rootshard *operatorv1alpha1.RootShard) reconciling.NamedCertificateReconcilerFactory {
+	name := resources.GetFrontProxyRequestHeaderName(rootshard, frontproxy)
 
 	return func() (string, reconciling.CertificateReconciler) {
 		return name, func(cert *certmanagerv1.Certificate) (*certmanagerv1.Certificate, error) {

internal/resources/frontproxy/configmap.go

@@ -26,7 +26,7 @@ import (
 	operatorv1alpha1 "github.com/kcp-dev/kcp-operator/sdk/apis/operator/v1alpha1"
 )
 
-func ConfigmapReconciler(frontproxy *operatorv1alpha1.FrontProxy, rootShard *operatorv1alpha1.RootShard) reconciling.NamedConfigMapReconcilerFactory {
+func PathMappingConfigMapReconciler(frontproxy *operatorv1alpha1.FrontProxy, rootShard *operatorv1alpha1.RootShard) reconciling.NamedConfigMapReconcilerFactory {
 	name := resources.GetFrontProxyConfigName(frontproxy)
 
 	return func() (string, reconciling.ConfigMapReconciler) {

internal/resources/resources.go

@@ -115,7 +115,7 @@ func GetFrontProxyDynamicKubeconfigName(r *operatorv1alpha1.RootShard, f *operat
 	return fmt.Sprintf("%s-%s-dynamic-kubeconfig", r.Name, f.Name)
 }
 
-func GetFrontProxyRequestheaderName(r *operatorv1alpha1.RootShard, f *operatorv1alpha1.FrontProxy) string {
+func GetFrontProxyRequestHeaderName(r *operatorv1alpha1.RootShard, f *operatorv1alpha1.FrontProxy) string {
 	return fmt.Sprintf("%s-%s-requestheader", r.Name, f.Name)
 }
 

internal/resources/rootshard/ca_certificates.go

@@ -94,65 +94,3 @@ func CACertificateReconciler(rootShard *operatorv1alpha1.RootShard, ca operatorv
 		}
 	}
 }
-
-func ClientCACertificateReconciler(rootshard *operatorv1alpha1.RootShard) reconciling.NamedCertificateReconcilerFactory {
-	name := resources.GetRootShardCAName(rootshard, operatorv1alpha1.ClientCA)
-
-	return func() (string, reconciling.CertificateReconciler) {
-		return name, func(cert *certmanagerv1.Certificate) (*certmanagerv1.Certificate, error) {
-			cert.SetLabels(resources.GetRootShardResourceLabels(rootshard))
-			cert.Spec = certmanagerv1.CertificateSpec{
-				IsCA:       true,
-				CommonName: name,
-				SecretName: name,
-				// Create CA certificate for ten years.
-				Duration:    &operatorv1alpha1.DefaultCADuration,
-				RenewBefore: &operatorv1alpha1.DefaultCARenewal,
-
-				PrivateKey: &certmanagerv1.CertificatePrivateKey{
-					Algorithm: certmanagerv1.RSAKeyAlgorithm,
-					Size:      4096,
-				},
-
-				IssuerRef: certmanagermetav1.ObjectReference{
-					Name:  resources.GetRootShardCAName(rootshard, operatorv1alpha1.RootCA),
-					Kind:  "Issuer",
-					Group: "cert-manager.io",
-				},
-			}
-
-			return cert, nil
-		}
-	}
-}
-
-func FrontProxyClientCACertificateReconciler(rootshard *operatorv1alpha1.RootShard) reconciling.NamedCertificateReconcilerFactory {
-	name := resources.GetRootShardCAName(rootshard, operatorv1alpha1.FrontProxyClientCA)
-
-	return func() (string, reconciling.CertificateReconciler) {
-		return name, func(cert *certmanagerv1.Certificate) (*certmanagerv1.Certificate, error) {
-			cert.SetLabels(resources.GetRootShardResourceLabels(rootshard))
-			cert.Spec = certmanagerv1.CertificateSpec{
-				IsCA:       true,
-				CommonName: name,
-				SecretName: name,
-				// Create CA certificate for ten years.
-				Duration:    &operatorv1alpha1.DefaultCADuration,
-				RenewBefore: &operatorv1alpha1.DefaultCARenewal,
-
-				PrivateKey: &certmanagerv1.CertificatePrivateKey{
-					Algorithm: certmanagerv1.RSAKeyAlgorithm,
-					Size:      4096,
-				},
-
-				IssuerRef: certmanagermetav1.ObjectReference{
-					Name:  resources.GetRootShardCAName(rootshard, operatorv1alpha1.RootCA),
-					Kind:  "Issuer",
-					Group: "cert-manager.io",
-				},
-			}
-
-			return cert, nil
-		}
-	}
-}