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

refactor: tls policy status to state of the world tasks #885

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
36 changes: 33 additions & 3 deletions controllers/state_of_the_world.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
istioclientnetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
istioclientgosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/dynamic"
Expand Down Expand Up @@ -125,7 +126,11 @@ func NewPolicyMachineryController(manager ctrlruntime.Manager, client *dynamic.D
CertManagerIssuerKind,
CertManagerClusterIssuerKind,
),
// TODO: add object links
controller.WithObjectLinks(
LinkGatewayToCertificateFunc,
LinkGatewayToIssuerFunc,
LinkGatewayToClusterIssuerFunc,
),
)
// TODO: add tls policy specific tasks to workflow
}
Expand All @@ -135,9 +140,14 @@ func NewPolicyMachineryController(manager ctrlruntime.Manager, client *dynamic.D

func buildReconciler(client *dynamic.DynamicClient) controller.ReconcileFunc {
reconciler := &controller.Workflow{
Precondition: NewEventLogger().Log,
Precondition: (&controller.Workflow{
Precondition: NewEventLogger().Log,
Tasks: []controller.ReconcileFunc{
NewTopologyFileReconciler(client, operatorNamespace).Reconcile,
},
}).Run,
Tasks: []controller.ReconcileFunc{
NewTopologyFileReconciler(client, operatorNamespace).Reconcile,
NewTLSPolicyWorkflow(client).Run,
},
}
return reconciler.Run
Expand Down Expand Up @@ -181,6 +191,11 @@ func (r *TopologyFileReconciler) Reconcile(ctx context.Context, _ []controller.R
if len(existingTopologyConfigMaps) == 0 {
_, err := r.Client.Resource(controller.ConfigMapsResource).Namespace(cm.Namespace).Create(ctx, unstructuredCM, metav1.CreateOptions{})
if err != nil {
if errors.IsAlreadyExists(err) {
// This error can happen when the operator is starting, and the create event for the topology has not being processed.
Comment on lines +194 to +195
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm... We may expect more of these, and not only with this particular resource.

This is because, as a pattern, we check the presence of the resource in the topology, and not with the API server. But more importantly, because, even though it's "state of the world" reconciliation and therefore the controller already knows about all resources that exist (because it lists them all), the library watching for changes to the cache writes to the subscription channel resource by resource.

I think we should consider fixing this in the Policy Machinery instead, because now it may be defeating the purpose of "state of the world". We may be able to make the call to cache.Replace truly atomic – assuming it won't break with the granularity of multiple event types. I have a couple of ideas.

logger.Info("already created topology configmap, must not be in topology yet")
return nil
}
logger.Error(err, "failed to write topology configmap")
}
return err
Expand Down Expand Up @@ -234,3 +249,18 @@ func (e *EventLogger) Log(ctx context.Context, resourceEvents []controller.Resou

return nil
}

func NewTLSPolicyWorkflow(client *dynamic.DynamicClient) *controller.Workflow {
validation := NewValidateTLSPolicyTask()
status := NewTLSPolicyStatusTask(client)
return &controller.Workflow{
Precondition: (&controller.Subscription{
ReconcileFunc: validation.Reconcile,
Events: validation.Subscription(),
}).Reconcile,
Postcondition: (&controller.Subscription{
ReconcileFunc: status.Reconcile,
Events: status.Subscription(),
}).Reconcile,
}
}
8 changes: 4 additions & 4 deletions controllers/tlspolicy_certmanager_certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (r *TLSPolicyReconciler) reconcileCertificates(ctx context.Context, tlsPoli
// Reconcile Certificates for each gateway directly referred by the policy (existing and new)
for _, gw := range append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) {
log.V(1).Info("reconcileCertificates: gateway with valid or missing policy ref", "key", gw.Key())
expectedCertificates := r.expectedCertificatesForGateway(ctx, gw.Gateway, tlsPolicy)
expectedCertificates := expectedCertificatesForGateway(ctx, gw.Gateway, tlsPolicy)
if err := r.createOrUpdateGatewayCertificates(ctx, tlsPolicy, expectedCertificates); err != nil {
return fmt.Errorf("error creating and updating expected certificates for gateway %v: %w", gw.Gateway.Name, err)
}
Expand Down Expand Up @@ -102,7 +102,7 @@ func (r *TLSPolicyReconciler) deleteUnexpectedCertificates(ctx context.Context,
return nil
}

func (r *TLSPolicyReconciler) expectedCertificatesForGateway(ctx context.Context, gateway *gatewayapiv1.Gateway, tlsPolicy *v1alpha1.TLSPolicy) []*certmanv1.Certificate {
func expectedCertificatesForGateway(ctx context.Context, gateway *gatewayapiv1.Gateway, tlsPolicy *v1alpha1.TLSPolicy) []*certmanv1.Certificate {
log := crlog.FromContext(ctx)

tlsHosts := make(map[corev1.ObjectReference][]string)
Expand Down Expand Up @@ -130,12 +130,12 @@ func (r *TLSPolicyReconciler) expectedCertificatesForGateway(ctx context.Context

certs := make([]*certmanv1.Certificate, 0, len(tlsHosts))
for secretRef, hosts := range tlsHosts {
certs = append(certs, r.buildCertManagerCertificate(gateway, tlsPolicy, secretRef, hosts))
certs = append(certs, buildCertManagerCertificate(gateway, tlsPolicy, secretRef, hosts))
}
return certs
}

func (r *TLSPolicyReconciler) buildCertManagerCertificate(gateway *gatewayapiv1.Gateway, tlsPolicy *v1alpha1.TLSPolicy, secretRef corev1.ObjectReference, hosts []string) *certmanv1.Certificate {
func buildCertManagerCertificate(gateway *gatewayapiv1.Gateway, tlsPolicy *v1alpha1.TLSPolicy, secretRef corev1.ObjectReference, hosts []string) *certmanv1.Certificate {
tlsCertLabels := commonTLSCertificateLabels(client.ObjectKeyFromObject(gateway), tlsPolicy)

crt := &certmanv1.Certificate{
Expand Down
33 changes: 2 additions & 31 deletions controllers/tlspolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@ import (
"fmt"
"reflect"

"github.com/cert-manager/cert-manager/pkg/apis/certmanager"
certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime/schema"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -39,23 +37,12 @@ import (

"github.com/kuadrant/kuadrant-operator/api/v1alpha1"
kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi"
"github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant"
"github.com/kuadrant/kuadrant-operator/pkg/library/mappers"
"github.com/kuadrant/kuadrant-operator/pkg/library/reconcilers"
)

const TLSPolicyFinalizer = "kuadrant.io/tls-policy"

var (
CertManagerCertificatesResource = certmanagerv1.SchemeGroupVersion.WithResource("certificates")
CertManagerIssuersResource = certmanagerv1.SchemeGroupVersion.WithResource("issuers")
CertMangerClusterIssuersResource = certmanagerv1.SchemeGroupVersion.WithResource("clusterissuers")

CertManagerCertificateKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.CertificateKind}
CertManagerIssuerKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.IssuerKind}
CertManagerClusterIssuerKind = schema.GroupKind{Group: certmanager.GroupName, Kind: certmanagerv1.ClusterIssuerKind}
)

// TLSPolicyReconciler reconciles a TLSPolicy object
type TLSPolicyReconciler struct {
*reconcilers.BaseReconciler
Expand Down Expand Up @@ -99,7 +86,7 @@ func (r *TLSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
if delResErr == nil {
delResErr = err
}
return r.reconcileStatus(ctx, tlsPolicy, targetReferenceObject, kuadrant.NewErrTargetNotFound(tlsPolicy.Kind(), tlsPolicy.GetTargetRef(), delResErr))
return ctrl.Result{}, delResErr
}
return ctrl.Result{}, err
}
Expand All @@ -125,25 +112,9 @@ func (r *TLSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
return ctrl.Result{}, err
}
}

specErr := r.reconcileResources(ctx, tlsPolicy, targetReferenceObject)

statusResult, statusErr := r.reconcileStatus(ctx, tlsPolicy, targetReferenceObject, specErr)

if specErr != nil {
return ctrl.Result{}, specErr
}

if statusErr != nil {
return ctrl.Result{}, statusErr
}

if statusResult.Requeue {
log.V(1).Info("Reconciling status not finished. Requeing.")
return statusResult, nil
}

return statusResult, statusErr
return ctrl.Result{}, specErr
}

func (r *TLSPolicyReconciler) reconcileResources(ctx context.Context, tlsPolicy *v1alpha1.TLSPolicy, targetNetworkObject client.Object) error {
Expand Down
86 changes: 86 additions & 0 deletions controllers/tlspolicy_links.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package controllers

import (
certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
"github.com/kuadrant/policy-machinery/controller"
"github.com/kuadrant/policy-machinery/machinery"
"github.com/samber/lo"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
)

func LinkGatewayToCertificateFunc(objs controller.Store) machinery.LinkFunc {
gateways := lo.Map(objs.FilterByGroupKind(machinery.GatewayGroupKind), controller.ObjectAs[*gwapiv1.Gateway])

return machinery.LinkFunc{
From: machinery.GatewayGroupKind,
To: CertManagerCertificateKind,
Func: func(child machinery.Object) []machinery.Object {
o := child.(*controller.RuntimeObject)
cert := o.Object.(*certmanagerv1.Certificate)

gateway, ok := lo.Find(gateways, func(item *gwapiv1.Gateway) bool {
for _, l := range item.Spec.Listeners {
if l.TLS != nil && l.TLS.CertificateRefs != nil {
for _, certRef := range l.TLS.CertificateRefs {
certRefNS := ""
if certRef.Namespace == nil {
certRefNS = item.GetNamespace()
} else {
certRefNS = string(*certRef.Namespace)
}
if certRefNS == cert.GetNamespace() && string(certRef.Name) == cert.GetName() {
return true
}
}
}
}

return false
})

if ok {
return []machinery.Object{&machinery.Gateway{Gateway: gateway}}
}

return nil
},
}
}

func LinkGatewayToIssuerFunc(objs controller.Store) machinery.LinkFunc {
gateways := lo.Map(objs.FilterByGroupKind(machinery.GatewayGroupKind), controller.ObjectAs[*gwapiv1.Gateway])

return machinery.LinkFunc{
From: machinery.GatewayGroupKind,
To: CertManagerIssuerKind,
Func: func(child machinery.Object) []machinery.Object {
o := child.(*controller.RuntimeObject)
issuer := o.Object.(*certmanagerv1.Issuer)

// TODO: Refine
gateway, ok := lo.Find(gateways, func(item *gwapiv1.Gateway) bool {
return item.GetNamespace() == issuer.GetNamespace()
})

if ok {
return []machinery.Object{&machinery.Gateway{Gateway: gateway}}
}

return nil
},
}
}

func LinkGatewayToClusterIssuerFunc(objs controller.Store) machinery.LinkFunc {
gateways := lo.Map(objs.FilterByGroupKind(machinery.GatewayGroupKind), controller.ObjectAs[machinery.Object])

return machinery.LinkFunc{
From: machinery.GatewayGroupKind,
To: CertManagerClusterIssuerKind,
Func: func(child machinery.Object) []machinery.Object {
o := child.(*controller.RuntimeObject)
_ = o.Object.(*certmanagerv1.ClusterIssuer)
return gateways
},
}
}
Loading
Loading