Skip to content

Commit

Permalink
Revert "refactor: use gateways again for now to check for expected ce…
Browse files Browse the repository at this point in the history
…rts"

This reverts commit 552d9f3.
  • Loading branch information
KevFan committed Oct 15, 2024
1 parent e9449e5 commit e95be4a
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 19 deletions.
36 changes: 30 additions & 6 deletions controllers/effective_tls_policies_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,6 @@ func expectedCertificatesForGateway(ctx context.Context, gateway *gatewayapiv1.G
continue
}

hostname := "*"
if l.Hostname != nil {
hostname = string(*l.Hostname)
}

for _, certRef := range l.TLS.CertificateRefs {
secretRef := corev1.ObjectReference{
Name: string(certRef.Name),
Expand All @@ -233,8 +228,37 @@ func expectedCertificatesForGateway(ctx context.Context, gateway *gatewayapiv1.G
}
// Gateway API hostname explicitly disallows IP addresses, so this
// should be OK.
tlsHosts[secretRef] = append(tlsHosts[secretRef], hostname)
tlsHosts[secretRef] = append(tlsHosts[secretRef], string(*l.Hostname))
}
}

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

func expectedCertificatesForListener(l *machinery.Listener, tlsPolicy *kuadrantv1alpha1.TLSPolicy) []*certmanv1.Certificate {
tlsHosts := make(map[corev1.ObjectReference][]string)

hostname := "*"
if l.Hostname != nil {
hostname = string(*l.Hostname)
}

for _, certRef := range l.TLS.CertificateRefs {
secretRef := corev1.ObjectReference{
Name: string(certRef.Name),
}
if certRef.Namespace != nil {
secretRef.Namespace = string(*certRef.Namespace)
} else {
secretRef.Namespace = l.GetNamespace()
}
// Gateway API hostname explicitly disallows IP addresses, so this
// should be OK.
tlsHosts[secretRef] = append(tlsHosts[secretRef], hostname)
}

certs := make([]*certmanv1.Certificate, 0, len(tlsHosts))
Expand Down
29 changes: 16 additions & 13 deletions controllers/tlspolicy_status_updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/client-go/dynamic"
"k8s.io/utils/ptr"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
Expand Down Expand Up @@ -106,7 +107,7 @@ func (t *TLSPolicyStatusUpdaterReconciler) enforcedCondition(ctx context.Context
return kuadrant.EnforcedCondition(tlsPolicy, kuadrant.NewErrUnknown(tlsPolicy.Kind(), err), false)
}

if err := t.isCertificatesReady(ctx, tlsPolicy, topology); err != nil {
if err := t.isCertificatesReady(tlsPolicy, topology); err != nil {
return kuadrant.EnforcedCondition(tlsPolicy, kuadrant.NewErrUnknown(tlsPolicy.Kind(), err), false)
}

Expand Down Expand Up @@ -179,32 +180,34 @@ func (t *TLSPolicyStatusUpdaterReconciler) isIssuerReady(ctx context.Context, tl
return nil
}

func (t *TLSPolicyStatusUpdaterReconciler) isCertificatesReady(ctx context.Context, p machinery.Policy, topology *machinery.Topology) error {
func (t *TLSPolicyStatusUpdaterReconciler) isCertificatesReady(p machinery.Policy, topology *machinery.Topology) error {
tlsPolicy, ok := p.(*kuadrantv1alpha1.TLSPolicy)
if !ok {
return errors.New("invalid policy")
}

// Get all listeners where the gateway contains this
// TODO: Update when targeting by section name is allowed, the listener will contain the policy rather than the gateway
gateways := lo.FilterMap(topology.Targetables().Items(), func(t machinery.Targetable, index int) (*machinery.Gateway, bool) {
gw, ok := t.(*machinery.Gateway)
return gw, ok && lo.Contains(gw.Policies(), p)
listeners := lo.FilterMap(topology.Targetables().Items(), func(t machinery.Targetable, index int) (*machinery.Listener, bool) {
l, ok := t.(*machinery.Listener)
return l, ok && lo.Contains(l.Gateway.Policies(), p)
})

if len(gateways) == 0 {
if len(listeners) == 0 {
return errors.New("no valid gateways found")
}

// Use gateway instead of listener for calculating expected certs
// This is because listeners that reference the same cert secret but with different host names are merged to a
// singular Certificate resource containing the hostnames. However, this means for Gateways with multiple listeners
// the expected certificates will be checked multiple times
for _, gw := range gateways {
expectedCertificates := expectedCertificatesForGateway(ctx, gw.Gateway, tlsPolicy)
for i, l := range listeners {
// Not valid - so no need to check if cert is ready since there should not be one created
err := validateGatewayListenerBlock(field.NewPath("").Index(i), *l.Listener, l.Gateway).ToAggregate()
if err != nil {
continue
}

expectedCertificates := expectedCertificatesForListener(l, tlsPolicy)

for _, cert := range expectedCertificates {
objs := topology.Objects().Children(gw)
objs := topology.Objects().Children(l)
obj, ok := lo.Find(objs, func(o machinery.Object) bool {
return o.GroupVersionKind().GroupKind() == CertManagerCertificateKind && o.GetNamespace() == cert.GetNamespace() && o.GetName() == cert.GetName()
})
Expand Down

0 comments on commit e95be4a

Please sign in to comment.