diff --git a/api/v1alpha1/dnspolicy_types.go b/api/v1alpha1/dnspolicy_types.go index 91e5985db..c54555cf3 100644 --- a/api/v1alpha1/dnspolicy_types.go +++ b/api/v1alpha1/dnspolicy_types.go @@ -195,6 +195,10 @@ func (p *DNSPolicy) List(ctx context.Context, c client.Client, namespace string) return policies } +func (p *DNSPolicy) TargetProgrammedGatewaysOnly() bool { + return true +} + func (p *DNSPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass { return kuadrantgatewayapi.DirectPolicy } diff --git a/api/v1alpha1/tlspolicy_types.go b/api/v1alpha1/tlspolicy_types.go index 424b1353c..ab1c0b9a7 100644 --- a/api/v1alpha1/tlspolicy_types.go +++ b/api/v1alpha1/tlspolicy_types.go @@ -163,6 +163,10 @@ func (p *TLSPolicy) List(ctx context.Context, c client.Client, namespace string) return policies } +func (p *TLSPolicy) TargetProgrammedGatewaysOnly() bool { + return false +} + func (p *TLSPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass { return kuadrantgatewayapi.DirectPolicy } diff --git a/api/v1beta2/authpolicy_types.go b/api/v1beta2/authpolicy_types.go index 87be0e2e4..ee206041d 100644 --- a/api/v1beta2/authpolicy_types.go +++ b/api/v1beta2/authpolicy_types.go @@ -355,6 +355,10 @@ func (ap *AuthPolicy) List(ctx context.Context, c client.Client, namespace strin return policies } +func (ap *AuthPolicy) TargetProgrammedGatewaysOnly() bool { + return true +} + func (ap *AuthPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass { return kuadrantgatewayapi.InheritedPolicy } diff --git a/api/v1beta2/ratelimitpolicy_types.go b/api/v1beta2/ratelimitpolicy_types.go index db0b03a73..b3afe1a04 100644 --- a/api/v1beta2/ratelimitpolicy_types.go +++ b/api/v1beta2/ratelimitpolicy_types.go @@ -288,6 +288,10 @@ func (r *RateLimitPolicy) List(ctx context.Context, c client.Client, namespace s return policies } +func (r *RateLimitPolicy) TargetProgrammedGatewaysOnly() bool { + return true +} + func (r *RateLimitPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass { return kuadrantgatewayapi.InheritedPolicy } diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index e02aead06..f1f48b4f7 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -65,7 +65,7 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ markedForDeletion := ap.GetDeletionTimestamp() != nil // fetch the target network object - targetNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), ap.GetTargetRef(), ap.Namespace) + targetNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), ap.GetTargetRef(), ap.Namespace, ap.TargetProgrammedGatewaysOnly()) if err != nil { if !markedForDeletion { if apierrors.IsNotFound(err) { @@ -186,7 +186,7 @@ func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.A return err } - refNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), ref.GetTargetRef(), ref.Namespace) + refNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), ref.GetTargetRef(), ref.Namespace, ap.TargetProgrammedGatewaysOnly()) if err != nil { return err } diff --git a/controllers/dnspolicy_controller.go b/controllers/dnspolicy_controller.go index 48abcce6c..6c80ef23a 100644 --- a/controllers/dnspolicy_controller.go +++ b/controllers/dnspolicy_controller.go @@ -75,7 +75,7 @@ func (r *DNSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( markedForDeletion := dnsPolicy.GetDeletionTimestamp() != nil - targetNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), dnsPolicy.GetTargetRef(), dnsPolicy.Namespace) + targetNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), dnsPolicy.GetTargetRef(), dnsPolicy.Namespace, dnsPolicy.TargetProgrammedGatewaysOnly()) if err != nil { if !markedForDeletion { if apierrors.IsNotFound(err) { diff --git a/controllers/helper_test.go b/controllers/helper_test.go deleted file mode 100644 index 812329793..000000000 --- a/controllers/helper_test.go +++ /dev/null @@ -1,321 +0,0 @@ -//go:build integration - -package controllers - -import ( - "context" - "slices" - "strings" - "time" - - certmanv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - certmanmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/external-dns/endpoint" - gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - - kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" - - kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" -) - -const ( - TestTimeoutMedium = time.Second * 10 - TestTimeoutLong = time.Second * 30 - TestRetryIntervalMedium = time.Millisecond * 250 - TestGatewayName = "test-placed-gateway" - TestClusterNameOne = "test-placed-control" - TestClusterNameTwo = "test-placed-workload-1" - TestHostOne = "test.example.com" - TestHostTwo = "other.test.example.com" - TestHostWildcard = "*.example.com" - TestListenerNameWildcard = "wildcard" - TestListenerNameOne = "test-listener-1" - TestListenerNameTwo = "test-listener-2" - TestIPAddressOne = "172.0.0.1" - TestIPAddressTwo = "172.0.0.2" - TestHTTPRouteName = "toystore-route" -) - -func testRLPIsNotAccepted(ctx context.Context, rlpKey client.ObjectKey) func() bool { - return func() bool { - existingRLP := &kuadrantv1beta2.RateLimitPolicy{} - err := k8sClient.Get(ctx, rlpKey, existingRLP) - if err != nil { - logf.Log.V(1).Info("ratelimitpolicy not read", "rlp", rlpKey, "error", err) - return false - } - if meta.IsStatusConditionTrue(existingRLP.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)) { - logf.Log.V(1).Info("ratelimitpolicy still accepted", "rlp", rlpKey) - return false - } - - return true - } -} - -func testHTTPRouteWithoutDirectBackReference(routeKey client.ObjectKey, annotationName string) func() bool { - return testNetworkResourceWithoutDirectBackReference(routeKey, &gatewayapiv1.HTTPRoute{}, annotationName) -} - -func testGatewayWithoutDirectBackReference(gwKey client.ObjectKey, annotationName string) func() bool { - return testNetworkResourceWithoutDirectBackReference(gwKey, &gatewayapiv1.Gateway{}, annotationName) -} - -func testNetworkResourceWithoutDirectBackReference(objKey client.ObjectKey, obj client.Object, annotationName string) func() bool { - return func() bool { - err := k8sClient.Get(context.Background(), objKey, obj) - if err != nil { - logf.Log.V(1).Info("object not read", "object", objKey, - "kind", obj.GetObjectKind().GroupVersionKind(), "error", err) - return false - } - - _, ok := obj.GetAnnotations()[annotationName] - if ok { - logf.Log.V(1).Info("object sill has the direct ref annotation", - "object", objKey, "kind", obj.GetObjectKind().GroupVersionKind()) - return false - } - - return true - } -} - -func testHTTPRouteHasDirectBackReference(routeKey client.ObjectKey, annotationName, annotationVal string) func() bool { - return testNetworkResourceHasDirectBackReference(routeKey, &gatewayapiv1.HTTPRoute{}, annotationName, annotationVal) -} - -func testGatewayHasDirectBackReference(gwKey client.ObjectKey, annotationName, annotationVal string) func() bool { - return testNetworkResourceHasDirectBackReference(gwKey, &gatewayapiv1.Gateway{}, annotationName, annotationVal) -} - -func testNetworkResourceHasDirectBackReference(objKey client.ObjectKey, obj client.Object, annotationName, annotationVal string) func() bool { - return func() bool { - err := k8sClient.Get(context.Background(), objKey, obj) - if err != nil { - logf.Log.V(1).Info("object not read", "object", objKey, - "kind", obj.GetObjectKind().GroupVersionKind(), "error", err) - return false - } - - val, ok := obj.GetAnnotations()[annotationName] - if !ok { - logf.Log.V(1).Info("object does not have the direct ref annotation", - "object", objKey, "kind", obj.GetObjectKind().GroupVersionKind()) - return false - } - - if val != annotationVal { - logf.Log.V(1).Info("object direct ref annotation value does not match", - "object", objKey, "kind", obj.GetObjectKind().GroupVersionKind(), - "val", val) - return false - } - - return true - } -} - -func testObjectDoesNotExist(obj client.Object) func() bool { - return func() bool { - err := testClient().Get(context.Background(), client.ObjectKeyFromObject(obj), obj) - if err != nil && apierrors.IsNotFound(err) { - return true - } - - logf.Log.V(1).Info("object not deleted", "object", client.ObjectKeyFromObject(obj), - "kind", obj.GetObjectKind().GroupVersionKind()) - return false - } -} - -// DNS - -func testBuildManagedZone(name, ns, domainName, secretName string) *kuadrantdnsv1alpha1.ManagedZone { - return &kuadrantdnsv1alpha1.ManagedZone{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: kuadrantdnsv1alpha1.ManagedZoneSpec{ - DomainName: domainName, - Description: domainName, - SecretRef: kuadrantdnsv1alpha1.ProviderRef{ - Name: secretName, - }, - }, - } -} - -func testBuildInMemoryCredentialsSecret(name, ns string) *corev1.Secret { - return &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Data: map[string][]byte{}, - Type: "kuadrant.io/inmemory", - } -} - -// testEndpointsTraversable consumes an array of endpoints and returns a boolean -// indicating presence of that path from host to all destinations -// this function DOES NOT report a presence of an endpoint with one of destinations DNSNames -func testEndpointsTraversable(endpoints []*endpoint.Endpoint, host string, destinations []string) bool { - allDestinationsFound := len(destinations) > 0 - for _, destination := range destinations { - allTargetsFound := false - for _, ep := range endpoints { - // the host exists as a DNSName on an endpoint - if ep.DNSName == host { - // we found destination in the targets of the endpoint. - if slices.Contains(ep.Targets, destination) { - return true - } - // destination is not found on the endpoint. Use target as a host and check for existence of Endpoints with such a DNSName - for _, target := range ep.Targets { - // if at least one returns as true allTargetsFound will be locked in true - // this means that at least one of the targets on the endpoint leads to the destination - allTargetsFound = allTargetsFound || testEndpointsTraversable(endpoints, target, []string{destination}) - } - } - } - // we must match all destinations - allDestinationsFound = allDestinationsFound && allTargetsFound - } - // there are no destinations to look for: len(destinations) == 0 locks allDestinationsFound into false - // or every destination was matched to a target on the endpoint - return allDestinationsFound -} - -//Gateway - -func testBuildGatewayClass(name, ns, controllerName string) *gatewayapiv1.GatewayClass { - return &gatewayapiv1.GatewayClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: gatewayapiv1.GatewayClassSpec{ - ControllerName: gatewayapiv1.GatewayController(controllerName), - }, - } -} - -// GatewayBuilder wrapper for Gateway builder helper -type GatewayBuilder struct { - *gatewayapiv1.Gateway -} - -func NewGatewayBuilder(gwName, gwClassName, ns string) *GatewayBuilder { - return &GatewayBuilder{ - &gatewayapiv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: gwName, - Namespace: ns, - }, - Spec: gatewayapiv1.GatewaySpec{ - GatewayClassName: gatewayapiv1.ObjectName(gwClassName), - Listeners: []gatewayapiv1.Listener{}, - }, - }, - } -} - -func (t *GatewayBuilder) WithListener(listener gatewayapiv1.Listener) *GatewayBuilder { - t.Spec.Listeners = append(t.Spec.Listeners, listener) - return t -} - -func (t *GatewayBuilder) WithLabels(labels map[string]string) *GatewayBuilder { - if t.Labels == nil { - t.Labels = map[string]string{} - } - for key, value := range labels { - t.Labels[key] = value - } - return t -} - -func (t *GatewayBuilder) WithHTTPListener(name, hostname string) *GatewayBuilder { - typedHostname := gatewayapiv1.Hostname(hostname) - t.WithListener(gatewayapiv1.Listener{ - Name: gatewayapiv1.SectionName(name), - Hostname: &typedHostname, - Port: gatewayapiv1.PortNumber(80), - Protocol: gatewayapiv1.HTTPProtocolType, - }) - return t -} - -func (t *GatewayBuilder) WithHTTPSListener(hostname, tlsSecretName string) *GatewayBuilder { - typedHostname := gatewayapiv1.Hostname(hostname) - typedNamespace := gatewayapiv1.Namespace(t.GetNamespace()) - typedNamed := gatewayapiv1.SectionName(strings.Replace(hostname, "*", "wildcard", 1)) - t.WithListener(gatewayapiv1.Listener{ - Name: typedNamed, - Hostname: &typedHostname, - Port: gatewayapiv1.PortNumber(443), - Protocol: gatewayapiv1.HTTPSProtocolType, - TLS: &gatewayapiv1.GatewayTLSConfig{ - Mode: ptr.To(gatewayapiv1.TLSModeTerminate), - CertificateRefs: []gatewayapiv1.SecretObjectReference{ - { - Name: gatewayapiv1.ObjectName(tlsSecretName), - Namespace: ptr.To(typedNamespace), - }, - }, - }, - }) - return t -} - -//CertMan - -func testBuildSelfSignedIssuer(name, ns string) (*certmanv1.Issuer, *certmanmetav1.ObjectReference) { - issuer := &certmanv1.Issuer{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: createSelfSignedIssuerSpec(), - } - objRef := &certmanmetav1.ObjectReference{ - Group: certmanv1.SchemeGroupVersion.Group, - Kind: certmanv1.IssuerKind, - Name: issuer.Name, - } - return issuer, objRef -} - -func testBuildSelfSignedClusterIssuer(name, ns string) (*certmanv1.ClusterIssuer, *certmanmetav1.ObjectReference) { - issuer := &certmanv1.ClusterIssuer{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: createSelfSignedIssuerSpec(), - } - objRef := &certmanmetav1.ObjectReference{ - Group: certmanv1.SchemeGroupVersion.Group, - Kind: certmanv1.ClusterIssuerKind, - Name: issuer.Name, - } - return issuer, objRef -} - -func createSelfSignedIssuerSpec() certmanv1.IssuerSpec { - return certmanv1.IssuerSpec{ - IssuerConfig: certmanv1.IssuerConfig{ - SelfSigned: &certmanv1.SelfSignedIssuer{}, - }, - } -} diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index 4d982b1f7..1cbd2efea 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -87,7 +87,7 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl markedForDeletion := rlp.GetDeletionTimestamp() != nil // fetch the target network object - targetNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), rlp.GetTargetRef(), rlp.Namespace) + targetNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), rlp.GetTargetRef(), rlp.Namespace, rlp.TargetProgrammedGatewaysOnly()) if err != nil { if !markedForDeletion { if apierrors.IsNotFound(err) { diff --git a/controllers/suite_test.go b/controllers/suite_test.go deleted file mode 100644 index df2591098..000000000 --- a/controllers/suite_test.go +++ /dev/null @@ -1,86 +0,0 @@ -//go:build integration - -/* -Copyright 2021. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package controllers - -import ( - "os" - "testing" - "time" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/envtest" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/log/zap" - - "github.com/kuadrant/kuadrant-operator/pkg/log" -) - -// These tests use Ginkgo (BDD-style Go testing framework). Refer to -// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. - -// This test suite will be run on k8s with GatewayAPI CRDS and at least one GatewayAPI provider installed - -var k8sClient client.Client -var testEnv *envtest.Environment - -func testClient() client.Client { return k8sClient } - -func TestAPIs(t *testing.T) { - RegisterFailHandler(Fail) - - RunSpecs(t, "Controller Suite") -} - -var _ = BeforeSuite(func(ctx SpecContext) { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - - By("bootstrapping test environment") - testEnv = &envtest.Environment{ - UseExistingCluster: &[]bool{true}[0], - } - - cfg, err := testEnv.Start() - Expect(err).NotTo(HaveOccurred()) - Expect(cfg).NotTo(BeNil()) - - SetupKuadrantOperatorForTest(scheme.Scheme, cfg) - - k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) - Expect(err).NotTo(HaveOccurred()) - Expect(k8sClient).NotTo(BeNil()) -}) - -var _ = AfterSuite(func(ctx SpecContext) { - By("tearing down the test environment") - err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) -}, NodeTimeout(3*time.Minute)) - -func TestMain(m *testing.M) { - logger := log.NewLogger( - log.SetLevel(log.DebugLevel), - log.SetMode(log.ModeDev), - log.WriteTo(GinkgoWriter), - ).WithName("controller_test") - log.SetLogger(logger) - os.Exit(m.Run()) -} diff --git a/controllers/test_common.go b/controllers/test_common.go index 6fb9d6b34..5c04667cd 100644 --- a/controllers/test_common.go +++ b/controllers/test_common.go @@ -104,6 +104,7 @@ func SetupKuadrantOperatorForTest(s *runtime.Scheme, cfg *rest.Config) { err = (&TLSPolicyReconciler{ BaseReconciler: tlsPolicyBaseReconciler, TargetRefReconciler: reconcilers.TargetRefReconciler{Client: mgr.GetClient()}, + RestMapper: mgr.GetRESTMapper(), }).SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred()) diff --git a/controllers/tlspolicy_certmanager_certificates.go b/controllers/tlspolicy_certmanager_certificates.go index 082f62f6a..c9c65aa52 100644 --- a/controllers/tlspolicy_certmanager_certificates.go +++ b/controllers/tlspolicy_certmanager_certificates.go @@ -3,11 +3,11 @@ package controllers import ( "context" "fmt" + "reflect" "slices" certmanv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" corev1 "k8s.io/api/core/v1" - k8serror "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/validation/field" @@ -34,7 +34,7 @@ func (r *TLSPolicyReconciler) reconcileCertificates(ctx context.Context, tlsPoli 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) - if err := r.createOrUpdateGatewayCertificates(ctx, expectedCertificates); err != nil { + 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) } if err := r.deleteUnexpectedCertificates(ctx, expectedCertificates, gw.Gateway, tlsPolicy); err != nil { @@ -44,20 +44,15 @@ func (r *TLSPolicyReconciler) reconcileCertificates(ctx context.Context, tlsPoli return nil } -func (r *TLSPolicyReconciler) createOrUpdateGatewayCertificates(ctx context.Context, expectedCertificates []*certmanv1.Certificate) error { +func (r *TLSPolicyReconciler) createOrUpdateGatewayCertificates(ctx context.Context, tlspolicy *v1alpha1.TLSPolicy, expectedCertificates []*certmanv1.Certificate) error { //create or update all expected Certificates - for _, cert := range expectedCertificates { - p := &certmanv1.Certificate{} - if err := r.Client().Get(ctx, client.ObjectKeyFromObject(cert), p); k8serror.IsNotFound(err) { - if err := r.Client().Create(ctx, cert); err != nil { - return err - } - } else if client.IgnoreNotFound(err) == nil { - p.Spec = cert.Spec - if err := r.Client().Update(ctx, p); err != nil { - return err - } - } else { + for idx := range expectedCertificates { + cert := expectedCertificates[idx] + if err := r.SetOwnerReference(tlspolicy, cert); err != nil { + return err + } + + if err := r.ReconcileResource(ctx, &certmanv1.Certificate{}, cert, certificateBasicMutator); err != nil { return err } } @@ -187,3 +182,22 @@ func gatewayTLSCertificateLabels(gwKey client.ObjectKey) map[string]string { "gateway": gwKey.Name, } } + +func certificateBasicMutator(existingObj, desiredObj client.Object) (bool, error) { + existing, ok := existingObj.(*certmanv1.Certificate) + if !ok { + return false, fmt.Errorf("%T is not an *certmanv1.Certificate", existingObj) + } + desired, ok := desiredObj.(*certmanv1.Certificate) + if !ok { + return false, fmt.Errorf("%T is not an *certmanv1.Certificate", desiredObj) + } + + if reflect.DeepEqual(existing.Spec, desired.Spec) { + return false, nil + } + + existing.Spec = desired.Spec + + return true, nil +} diff --git a/controllers/tlspolicy_controller.go b/controllers/tlspolicy_controller.go index bf30af851..d310e7252 100644 --- a/controllers/tlspolicy_controller.go +++ b/controllers/tlspolicy_controller.go @@ -19,14 +19,19 @@ package controllers import ( "context" "fmt" + "reflect" - "github.com/go-logr/logr" + certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" crlog "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -43,6 +48,7 @@ const TLSPolicyFinalizer = "kuadrant.io/tls-policy" type TLSPolicyReconciler struct { *reconcilers.BaseReconciler TargetRefReconciler reconcilers.TargetRefReconciler + RestMapper meta.RESTMapper } //+kubebuilder:rbac:groups=kuadrant.io,resources=tlspolicies,verbs=get;list;watch;update;patch;delete @@ -71,7 +77,7 @@ func (r *TLSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( markedForDeletion := tlsPolicy.GetDeletionTimestamp() != nil - targetReferenceObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), tlsPolicy.GetTargetRef(), tlsPolicy.Namespace) + targetReferenceObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), tlsPolicy.GetTargetRef(), tlsPolicy.Namespace, tlsPolicy.TargetProgrammedGatewaysOnly()) log.V(3).Info("TLSPolicyReconciler targetReferenceObject", "targetReferenceObject", targetReferenceObject) if err != nil { if !markedForDeletion { @@ -196,28 +202,55 @@ func (r *TLSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { return nil } + ok, err = kuadrantgatewayapi.IsCertManagerInstalled(mgr.GetRESTMapper(), r.Logger()) + if err != nil { + return err + } + if !ok { + r.Logger().Info("TLSPolicy controller disabled. CertManager was not found") + return nil + } + gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")), mappers.WithClient(mgr.GetClient())) + issuerStatusChangedPredicate := predicate.Funcs{ + UpdateFunc: func(ev event.UpdateEvent) bool { + oldPolicy, ok := ev.ObjectOld.(certmanagerv1.GenericIssuer) + if !ok { + return false + } + newPolicy, ok := ev.ObjectNew.(certmanagerv1.GenericIssuer) + if !ok { + return false + } + oldStatus := oldPolicy.GetStatus() + newStatus := newPolicy.GetStatus() + return !reflect.DeepEqual(oldStatus, newStatus) + }, + } + return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.TLSPolicy{}). + Owns(&certmanagerv1.Certificate{}). Watches( &gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { return gatewayEventMapper.MapToPolicy(ctx, object, &v1alpha1.TLSPolicy{}) }), ). + Watches( + &certmanagerv1.Issuer{}, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { + return mapIssuerToPolicy(ctx, mgr.GetClient(), r.Logger(), object) + }), + builder.WithPredicates(issuerStatusChangedPredicate), + ). + Watches( + &certmanagerv1.ClusterIssuer{}, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { + return mapClusterIssuerToPolicy(ctx, mgr.GetClient(), r.Logger(), object) + }), + builder.WithPredicates(issuerStatusChangedPredicate), + ). Complete(r) } - -func (r *TLSPolicyReconciler) FetchValidGateway(ctx context.Context, key client.ObjectKey) (*gatewayapiv1.Gateway, error) { - logger, _ := logr.FromContext(ctx) - - gw := &gatewayapiv1.Gateway{} - err := r.Client().Get(ctx, key, gw) - logger.V(1).Info("FetchValidGateway", "gateway", key, "err", err) - if err != nil { - return nil, err - } - - return gw, nil -} diff --git a/controllers/tlspolicy_mappers.go b/controllers/tlspolicy_mappers.go new file mode 100644 index 000000000..f4be97032 --- /dev/null +++ b/controllers/tlspolicy_mappers.go @@ -0,0 +1,59 @@ +package controllers + +import ( + "context" + "fmt" + + certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + "github.com/go-logr/logr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/kuadrant/kuadrant-operator/api/v1alpha1" + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" +) + +func mapIssuerToPolicy(ctx context.Context, k8sClient client.Client, logger logr.Logger, object client.Object) []reconcile.Request { + _, ok := object.(*certmanagerv1.Issuer) + if !ok { + logger.V(1).Info("cannot map Issuer related event to tls policy", "error", fmt.Sprintf("%T is not a *certmanagerv1.Issuer", object)) + return nil + } + + policies := &v1alpha1.TLSPolicyList{} + if err := k8sClient.List(ctx, policies, client.InNamespace(object.GetNamespace())); err != nil { + logger.V(1).Error(err, "cannot list policies", "namespace", object.GetNamespace()) + return nil + } + + return policiesToRequests(logger, policies, object, certmanagerv1.IssuerKind) +} + +func mapClusterIssuerToPolicy(ctx context.Context, k8sClient client.Client, logger logr.Logger, object client.Object) []reconcile.Request { + _, ok := object.(*certmanagerv1.ClusterIssuer) + if !ok { + logger.V(1).Info("cannot map ClusterIssuer related event to tls policy", "error", fmt.Sprintf("%T is not a *certmanagerv1.ClusterIssuer", object)) + return nil + } + + policies := &v1alpha1.TLSPolicyList{} + if err := k8sClient.List(ctx, policies); err != nil { + logger.V(1).Error(err, "cannot list policies for all namespaces") + return nil + } + + return policiesToRequests(logger, policies, object, certmanagerv1.ClusterIssuerKind) +} + +func policiesToRequests(logger logr.Logger, policies *v1alpha1.TLSPolicyList, object client.Object, issuerKind string) []reconcile.Request { + filteredPolicies := utils.Filter(policies.Items, func(policy v1alpha1.TLSPolicy) bool { + return policy.Spec.IssuerRef.Name == object.GetName() && + policy.Spec.IssuerRef.Kind == issuerKind && + policy.Spec.IssuerRef.Group == certmanagerv1.SchemeGroupVersion.Group + }) + + return utils.Map(filteredPolicies, func(p v1alpha1.TLSPolicy) reconcile.Request { + logger.V(1).Info("tls policy possibly affected by related event", "eventKind", issuerKind, "policyName", p.Name, "policyNamespace", p.Namespace) + return reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&p)} + }) +} diff --git a/controllers/tlspolicy_mappers_test.go b/controllers/tlspolicy_mappers_test.go new file mode 100644 index 000000000..2ba29b5cd --- /dev/null +++ b/controllers/tlspolicy_mappers_test.go @@ -0,0 +1,223 @@ +//go:build unit + +package controllers + +import ( + "context" + "errors" + "reflect" + "testing" + + certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + v1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + "github.com/go-logr/logr" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/kuadrant/kuadrant-operator/api/v1alpha1" +) + +func Test_mapClusterIssuerToPolicy(t *testing.T) { + clusterIssuer := &certmanagerv1.ClusterIssuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-issuer", + }, + } + + s := runtime.NewScheme() + sb := runtime.NewSchemeBuilder(certmanagerv1.AddToScheme, v1alpha1.AddToScheme) + if err := sb.AddToScheme(s); err != nil { + t.Fatal(err) + } + + type args struct { + k8sClient client.Client + object client.Object + } + tests := []struct { + name string + args args + want []reconcile.Request + }{ + { + name: "not a cluster issuer", + args: args{ + object: &certmanagerv1.Certificate{}, + }, + want: nil, + }, + { + name: "list error", + args: args{ + k8sClient: fake.NewClientBuilder().WithScheme(s).WithInterceptorFuncs(interceptor.Funcs{ + List: func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + return errors.New("list error") + }, + }).Build(), + object: clusterIssuer, + }, + want: nil, + }, + { + name: "map cluster issuer to matching policies", + args: args{ + k8sClient: fake.NewClientBuilder().WithScheme(s).WithObjects(clusterIssuer).WithLists(testInitTLSPolicies(clusterIssuer.Name, certmanagerv1.ClusterIssuerKind)).Build(), + object: clusterIssuer, + }, + want: []reconcile.Request{ + { + NamespacedName: types.NamespacedName{Name: "p1", Namespace: "n1"}, + }, + { + NamespacedName: types.NamespacedName{Name: "p4", Namespace: "n2"}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := mapClusterIssuerToPolicy(context.Background(), tt.args.k8sClient, logr.Logger{}, tt.args.object); !reflect.DeepEqual(got, tt.want) { + t.Errorf("mapClusterIssuerToPolicy() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_mapIssuerToPolicy(t *testing.T) { + issuer := &certmanagerv1.Issuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-issuer", + Namespace: "n1", + }, + } + + s := runtime.NewScheme() + sb := runtime.NewSchemeBuilder(certmanagerv1.AddToScheme, v1alpha1.AddToScheme) + if err := sb.AddToScheme(s); err != nil { + t.Fatal(err) + } + + type args struct { + ctx context.Context + k8sClient client.Client + logger logr.Logger + object client.Object + } + tests := []struct { + name string + args args + want []reconcile.Request + }{ + { + name: "not an issuer", + args: args{ + object: &certmanagerv1.Certificate{}, + }, + want: nil, + }, + { + name: "list error", + args: args{ + k8sClient: fake.NewClientBuilder().WithScheme(s).WithInterceptorFuncs(interceptor.Funcs{ + List: func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + return errors.New("list error") + }, + }).Build(), + object: issuer, + }, + want: nil, + }, + { + name: "map issuer to matching policies", + args: args{ + k8sClient: fake.NewClientBuilder().WithScheme(s).WithObjects(issuer).WithLists(testInitTLSPolicies(issuer.Name, certmanagerv1.IssuerKind)).Build(), + object: issuer, + }, + want: []reconcile.Request{ + { + NamespacedName: types.NamespacedName{Name: "p1", Namespace: "n1"}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := mapIssuerToPolicy(tt.args.ctx, tt.args.k8sClient, tt.args.logger, tt.args.object); !reflect.DeepEqual(got, tt.want) { + t.Errorf("mapIssuerToPolicy() = %v, want %v", got, tt.want) + } + }) + } +} + +func testInitTLSPolicies(issuerName, issuerKind string) *v1alpha1.TLSPolicyList { + return &v1alpha1.TLSPolicyList{ + Items: []v1alpha1.TLSPolicy{ + + { + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + Namespace: "n1", + }, + Spec: v1alpha1.TLSPolicySpec{ + CertificateSpec: v1alpha1.CertificateSpec{ + IssuerRef: v1.ObjectReference{ + Name: issuerName, + Group: certmanagerv1.SchemeGroupVersion.Group, + Kind: issuerKind, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "p2", + Namespace: "n1", + }, + Spec: v1alpha1.TLSPolicySpec{ + CertificateSpec: v1alpha1.CertificateSpec{ + IssuerRef: v1.ObjectReference{ + Name: issuerName, + Group: "unknown.example.com", + Kind: issuerKind, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "p3", + Namespace: "n1", + }, + Spec: v1alpha1.TLSPolicySpec{ + CertificateSpec: v1alpha1.CertificateSpec{ + IssuerRef: v1.ObjectReference{ + Name: issuerName, + Group: certmanagerv1.SchemeGroupVersion.Group, + Kind: "Unknown", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "p4", + Namespace: "n2", + }, + Spec: v1alpha1.TLSPolicySpec{ + CertificateSpec: v1alpha1.CertificateSpec{ + IssuerRef: v1.ObjectReference{ + Name: issuerName, + Group: certmanagerv1.SchemeGroupVersion.Group, + Kind: issuerKind, + }, + }, + }, + }, + }, + } +} diff --git a/controllers/tlspolicy_status.go b/controllers/tlspolicy_status.go index 7796e67d0..b185b5944 100644 --- a/controllers/tlspolicy_status.go +++ b/controllers/tlspolicy_status.go @@ -123,7 +123,7 @@ func (r *TLSPolicyReconciler) isIssuerReady(ctx context.Context, tlsPolicy *v1al return metav1.Condition{Reason: c.Reason, Status: metav1.ConditionStatus(c.Status), Type: string(c.Type), Message: c.Message} }) - if meta.IsStatusConditionFalse(transformedCond, string(certmanv1.IssuerConditionReady)) { + if !meta.IsStatusConditionTrue(transformedCond, string(certmanv1.IssuerConditionReady)) { return errors.New("issuer not ready") } @@ -152,7 +152,7 @@ func (r *TLSPolicyReconciler) isCertificatesReady(ctx context.Context, tlsPolicy return metav1.Condition{Reason: c.Reason, Status: metav1.ConditionStatus(c.Status), Type: string(c.Type), Message: c.Message} }) - if meta.IsStatusConditionFalse(conditions, string(certmanv1.CertificateConditionReady)) { + if !meta.IsStatusConditionTrue(conditions, string(certmanv1.CertificateConditionReady)) { return fmt.Errorf("certificate %s not ready", cert.Name) } } diff --git a/controllers/tlspolicy_status_test.go b/controllers/tlspolicy_status_test.go index 8980984f7..e0fb8cc74 100644 --- a/controllers/tlspolicy_status_test.go +++ b/controllers/tlspolicy_status_test.go @@ -233,6 +233,28 @@ func TestTLSPolicyReconciler_enforcedCondition(t *testing.T) { Message: "TLSPolicy has encountered some issues: issuer not ready", }, }, + { + name: "issuer has no ready condition", + fields: fields{ + BaseReconciler: reconcilers.NewBaseReconciler( + fake.NewClientBuilder(). + WithObjects( + issuerFactory(func(issuer *certmanv1.Issuer) { + issuer.Status.Conditions = []certmanv1.IssuerCondition{} + })). + WithScheme(scheme).Build(), nil, nil, log.NewLogger(), nil, + ), + }, + args: args{ + tlsPolicy: policyFactory(), + }, + want: &metav1.Condition{ + Type: string(kuadrant.PolicyConditionEnforced), + Status: metav1.ConditionFalse, + Reason: string(kuadrant.PolicyReasonUnknown), + Message: "TLSPolicy has encountered some issues: issuer not ready", + }, + }, { name: "no valid gateways found", fields: fields{ @@ -290,6 +312,28 @@ func TestTLSPolicyReconciler_enforcedCondition(t *testing.T) { Message: fmt.Sprintf("TLSPolicy has encountered some issues: certificate %s not ready", certificateName), }, }, + { + name: "certificate has no ready condition", + fields: fields{ + BaseReconciler: reconcilers.NewBaseReconciler( + fake.NewClientBuilder().WithObjects( + issuerFactory(), gwFactory(), certificateFactory(func(certificate *certmanv1.Certificate) { + certificate.Status.Conditions = []certmanv1.CertificateCondition{} + })). + WithScheme(scheme).Build(), nil, nil, log.NewLogger(), nil, + ), + }, + args: args{ + tlsPolicy: policyFactory(), + targetNetworkObject: gwFactory(), + }, + want: &metav1.Condition{ + Type: string(kuadrant.PolicyConditionEnforced), + Status: metav1.ConditionFalse, + Reason: string(kuadrant.PolicyReasonUnknown), + Message: fmt.Sprintf("TLSPolicy has encountered some issues: certificate %s not ready", certificateName), + }, + }, { name: "is enforced", fields: fields{ diff --git a/main.go b/main.go index 6b04ec7fa..3853017d0 100644 --- a/main.go +++ b/main.go @@ -208,6 +208,7 @@ func main() { if err = (&controllers.TLSPolicyReconciler{ BaseReconciler: tlsPolicyBaseReconciler, TargetRefReconciler: reconcilers.TargetRefReconciler{Client: mgr.GetClient()}, + RestMapper: mgr.GetRESTMapper(), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "TLSPolicy") os.Exit(1) diff --git a/pkg/library/gatewayapi/topology.go b/pkg/library/gatewayapi/topology.go index 154a9726a..24aa0c46b 100644 --- a/pkg/library/gatewayapi/topology.go +++ b/pkg/library/gatewayapi/topology.go @@ -84,10 +84,11 @@ func (h httpRouteDAGNode) ID() string { } type topologyOptions struct { - gateways []*gatewayapiv1.Gateway - routes []*gatewayapiv1.HTTPRoute - policies []Policy - logger logr.Logger + gateways []*gatewayapiv1.Gateway + routes []*gatewayapiv1.HTTPRoute + policies []Policy + logger logr.Logger + programmedGatewaysOnly bool } // TopologyOpts allows to manipulate topologyOptions. @@ -117,10 +118,17 @@ func WithPolicies(policies []Policy) TopologyOpts { } } +func WithProgrammedGatewaysOnly(programmedGatewaysOnly bool) TopologyOpts { + return func(o *topologyOptions) { + o.programmedGatewaysOnly = programmedGatewaysOnly + } +} + func NewTopology(opts ...TopologyOpts) (*Topology, error) { // defaults o := &topologyOptions{ - logger: logr.Discard(), + logger: logr.Discard(), + programmedGatewaysOnly: true, } for _, opt := range opts { @@ -140,7 +148,7 @@ func NewTopology(opts ...TopologyOpts) (*Topology, error) { graph := dag.NewDAG(typeIndexer) - gatewayDAGNodes := buildGatewayDAGNodes(o.gateways, o.policies) + gatewayDAGNodes := buildGatewayDAGNodes(o.gateways, o.policies, o.programmedGatewaysOnly) routeDAGNodes := buildHTTPRouteDAGNodes(o.routes, o.policies) @@ -199,12 +207,15 @@ func buildDAGEdges(gateways []gatewayDAGNode, routes []httpRouteDAGNode) []edge return edges } -func buildGatewayDAGNodes(gateways []*gatewayapiv1.Gateway, policies []Policy) []gatewayDAGNode { - programmedGateways := utils.Filter(gateways, func(g *gatewayapiv1.Gateway) bool { - return meta.IsStatusConditionTrue(g.Status.Conditions, string(gatewayapiv1.GatewayConditionProgrammed)) - }) +func buildGatewayDAGNodes(gateways []*gatewayapiv1.Gateway, policies []Policy, programmedGatewaysOnly bool) []gatewayDAGNode { + targetedGateways := gateways + if programmedGatewaysOnly { + targetedGateways = utils.Filter(gateways, func(g *gatewayapiv1.Gateway) bool { + return meta.IsStatusConditionTrue(g.Status.Conditions, string(gatewayapiv1.GatewayConditionProgrammed)) + }) + } - return utils.Map(programmedGateways, func(g *gatewayapiv1.Gateway) gatewayDAGNode { + return utils.Map(targetedGateways, func(g *gatewayapiv1.Gateway) gatewayDAGNode { // Compute attached policies attachedPolicies := utils.Filter(policies, func(p Policy) bool { group := p.GetTargetRef().Group diff --git a/pkg/library/gatewayapi/types.go b/pkg/library/gatewayapi/types.go index 80746facb..3088ab68f 100644 --- a/pkg/library/gatewayapi/types.go +++ b/pkg/library/gatewayapi/types.go @@ -26,6 +26,7 @@ type Policy interface { Kind() string BackReferenceAnnotationName() string DirectReferenceAnnotationName() string + TargetProgrammedGatewaysOnly() bool } type PolicyStatus interface { diff --git a/pkg/library/gatewayapi/types_test.go b/pkg/library/gatewayapi/types_test.go index fba01d28f..758125dd6 100644 --- a/pkg/library/gatewayapi/types_test.go +++ b/pkg/library/gatewayapi/types_test.go @@ -60,6 +60,9 @@ func (p *TestPolicy) GetStatus() PolicyStatus { return &p.Status } +func (p *TestPolicy) TargetProgrammedGatewaysOnly() bool { + return true +} func (p *TestPolicy) DeepCopyObject() runtime.Object { if c := p.DeepCopy(); c != nil { return c diff --git a/pkg/library/gatewayapi/utils.go b/pkg/library/gatewayapi/utils.go index 09f9ba5df..bb9cd7687 100644 --- a/pkg/library/gatewayapi/utils.go +++ b/pkg/library/gatewayapi/utils.go @@ -6,6 +6,9 @@ import ( "reflect" "strings" + "github.com/cert-manager/cert-manager/pkg/apis/certmanager" + certmanv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -151,9 +154,32 @@ func FilterValidSubdomains(domains, subdomains []gatewayapiv1.Hostname) []gatewa } func IsGatewayAPIInstalled(restMapper meta.RESTMapper) (bool, error) { + return isCRDInstalled(restMapper, gatewayapiv1.GroupName, "HTTPRoute", gatewayapiv1.GroupVersion.Version) +} + +func IsCertManagerInstalled(restMapper meta.RESTMapper, logger logr.Logger) (bool, error) { + if ok, err := isCRDInstalled(restMapper, certmanager.GroupName, certmanv1.CertificateKind, certmanv1.SchemeGroupVersion.Version); !ok || err != nil { + logger.V(1).Error(err, "CertManager CRD was not installed", "group", certmanager.GroupName, "kind", certmanv1.CertificateKind, "version", certmanv1.SchemeGroupVersion.Version) + return false, err + } + + if ok, err := isCRDInstalled(restMapper, certmanager.GroupName, certmanv1.IssuerKind, certmanv1.SchemeGroupVersion.Version); !ok || err != nil { + logger.V(1).Error(err, "CertManager CRD was not installed", "group", certmanager.GroupName, "kind", certmanv1.IssuerKind, "version", certmanv1.SchemeGroupVersion.Version) + return false, err + } + + if ok, err := isCRDInstalled(restMapper, certmanager.GroupName, certmanv1.ClusterIssuerKind, certmanv1.SchemeGroupVersion.Version); !ok || err != nil { + logger.V(1).Error(err, "CertManager CRD was not installed", "group", certmanager.GroupName, "kind", certmanv1.ClusterIssuerKind, "version", certmanv1.SchemeGroupVersion.Version) + return false, err + } + + return true, nil +} + +func isCRDInstalled(restMapper meta.RESTMapper, group, kind, version string) (bool, error) { _, err := restMapper.RESTMapping( - schema.GroupKind{Group: gatewayapiv1.GroupName, Kind: "HTTPRoute"}, - gatewayapiv1.SchemeGroupVersion.Version, + schema.GroupKind{Group: group, Kind: kind}, + version, ) if err == nil { return true, nil diff --git a/pkg/library/kuadrant/test_utils.go b/pkg/library/kuadrant/test_utils.go index 30e1ab952..95dd09e2a 100644 --- a/pkg/library/kuadrant/test_utils.go +++ b/pkg/library/kuadrant/test_utils.go @@ -75,6 +75,10 @@ func (_ *FakePolicy) PolicyClass() kuadrantgatewayapi.PolicyClass { return kuadrantgatewayapi.DirectPolicy } +func (p *FakePolicy) TargetProgrammedGatewaysOnly() bool { + return true +} + type FakePolicyStatus struct{} func (s *FakePolicyStatus) GetConditions() []metav1.Condition { diff --git a/pkg/library/mappers/gateway.go b/pkg/library/mappers/gateway.go index ef381547b..7c14b274f 100644 --- a/pkg/library/mappers/gateway.go +++ b/pkg/library/mappers/gateway.go @@ -49,6 +49,7 @@ func (m *gatewayEventMapper) MapToPolicy(ctx context.Context, obj client.Object, kuadrantgatewayapi.WithRoutes(utils.Map(routeList.Items, ptr.To[gatewayapiv1.HTTPRoute])), kuadrantgatewayapi.WithPolicies(policies), kuadrantgatewayapi.WithLogger(logger), + kuadrantgatewayapi.WithProgrammedGatewaysOnly(false), ) if err != nil { logger.V(1).Error(err, "unable to build topology for gateway") diff --git a/pkg/library/reconcilers/fetcher.go b/pkg/library/reconcilers/fetcher.go index 64a4a2a7c..3c66c251b 100644 --- a/pkg/library/reconcilers/fetcher.go +++ b/pkg/library/reconcilers/fetcher.go @@ -13,7 +13,7 @@ import ( ) // FetchTargetRefObject fetches the target reference object and checks the status is valid -func FetchTargetRefObject(ctx context.Context, k8sClient client.Reader, targetRef gatewayapiv1alpha2.PolicyTargetReference, defaultNs string) (client.Object, error) { +func FetchTargetRefObject(ctx context.Context, k8sClient client.Reader, targetRef gatewayapiv1alpha2.PolicyTargetReference, defaultNs string, fetchOnlyProgrammedGateways bool) (client.Object, error) { ns := defaultNs if targetRef.Namespace != nil { ns = string(*targetRef.Namespace) @@ -23,6 +23,9 @@ func FetchTargetRefObject(ctx context.Context, k8sClient client.Reader, targetRe switch targetRef.Kind { case "Gateway": + if fetchOnlyProgrammedGateways { + return fetchProgrammedGateway(ctx, k8sClient, objKey) + } return fetchGateway(ctx, k8sClient, objKey) case "HTTPRoute": return fetchHTTPRoute(ctx, k8sClient, objKey) @@ -41,6 +44,14 @@ func fetchGateway(ctx context.Context, k8sClient client.Reader, key client.Objec return nil, err } + return gw, nil +} + +func fetchProgrammedGateway(ctx context.Context, k8sClient client.Reader, key client.ObjectKey) (*gatewayapiv1.Gateway, error) { + gw, err := fetchGateway(ctx, k8sClient, key) + if err != nil { + return nil, err + } if meta.IsStatusConditionFalse(gw.Status.Conditions, string(gatewayapiv1.GatewayConditionProgrammed)) { return nil, fmt.Errorf("gateway (%v) not ready", key) } diff --git a/pkg/library/reconcilers/fetcher_test.go b/pkg/library/reconcilers/fetcher_test.go index 9fd26b9cb..690d6924b 100644 --- a/pkg/library/reconcilers/fetcher_test.go +++ b/pkg/library/reconcilers/fetcher_test.go @@ -135,7 +135,7 @@ func TestFetchTargetRefObject(t *testing.T) { t.Run("fetch http route", func(subT *testing.T) { existingRoute := routeFactory(metav1.ConditionTrue) clientAPIReader := clientFactory(existingRoute) - res, err := FetchTargetRefObject(ctx, clientAPIReader, routeTargetRef, namespace) + res, err := FetchTargetRefObject(ctx, clientAPIReader, routeTargetRef, namespace, true) assert.NilError(subT, err) assert.Equal(subT, res == nil, false) assertion(res, existingRoute) @@ -144,7 +144,7 @@ func TestFetchTargetRefObject(t *testing.T) { t.Run("fetch http route - not accepted", func(subT *testing.T) { existingRoute := routeFactory(metav1.ConditionFalse) clientAPIReader := clientFactory(existingRoute) - res, err := FetchTargetRefObject(ctx, clientAPIReader, routeTargetRef, namespace) + res, err := FetchTargetRefObject(ctx, clientAPIReader, routeTargetRef, namespace, true) assert.Error(subT, err, fmt.Sprintf("httproute (%s/%s) not accepted", namespace, routeName)) assert.DeepEqual(subT, res, (*gatewayapiv1.HTTPRoute)(nil)) }) @@ -152,7 +152,7 @@ func TestFetchTargetRefObject(t *testing.T) { t.Run("fetch gateway", func(subT *testing.T) { existingGateway := gatewayFactory(metav1.ConditionTrue) clientAPIReader := clientFactory(existingGateway) - res, err := FetchTargetRefObject(ctx, clientAPIReader, gatewayTargetRef, namespace) + res, err := FetchTargetRefObject(ctx, clientAPIReader, gatewayTargetRef, namespace, true) assert.NilError(subT, err) assert.Equal(subT, res == nil, false) assertion(res, existingGateway) @@ -161,16 +161,25 @@ func TestFetchTargetRefObject(t *testing.T) { t.Run("fetch gateway - not ready", func(subT *testing.T) { existingGateway := gatewayFactory(metav1.ConditionFalse) clientAPIReader := clientFactory(existingGateway) - res, err := FetchTargetRefObject(ctx, clientAPIReader, gatewayTargetRef, namespace) + res, err := FetchTargetRefObject(ctx, clientAPIReader, gatewayTargetRef, namespace, true) assert.Error(subT, err, fmt.Sprintf("gateway (%s/%s) not ready", namespace, gatewayName)) assert.DeepEqual(subT, res, (*gatewayapiv1.Gateway)(nil)) }) + t.Run("fetch gateway - not ready - skip check for only ready gateways", func(subT *testing.T) { + existingGateway := gatewayFactory(metav1.ConditionFalse) + clientAPIReader := clientFactory(existingGateway) + res, err := FetchTargetRefObject(ctx, clientAPIReader, gatewayTargetRef, namespace, false) + assert.NilError(subT, err) + assert.Equal(subT, res == nil, false) + assertion(res, existingGateway) + }) + t.Run("unknown network resource", func(subT *testing.T) { ns := gatewayapiv1.Namespace(namespace) targetRef := gatewayapiv1alpha2.PolicyTargetReference{Kind: "Service", Name: "my-sv", Namespace: &ns} clientAPIReader := clientFactory() - res, err := FetchTargetRefObject(ctx, clientAPIReader, targetRef, namespace) + res, err := FetchTargetRefObject(ctx, clientAPIReader, targetRef, namespace, true) assert.Error(subT, err, fmt.Sprintf("FetchValidTargetRef: targetRef (%v) to unknown network resource", targetRef)) assert.DeepEqual(subT, res, nil) }) diff --git a/tests/common/dnspolicy/dnspolicy_controller_test.go b/tests/common/dnspolicy/dnspolicy_controller_test.go index cb8f42d25..15fa1ecd4 100644 --- a/tests/common/dnspolicy/dnspolicy_controller_test.go +++ b/tests/common/dnspolicy/dnspolicy_controller_test.go @@ -232,7 +232,7 @@ var _ = Describe("DNSPolicy controller", func() { "Message": Equal("Provider ensured the dns record"), })), ) - }, tests.TimeoutMedium, tests.RetryIntervalMedium).Should(Succeed()) + }, tests.TimeoutLong, tests.RetryIntervalMedium).Should(Succeed()) // create policy2 targeting gateway2 with the load-balanced strategy dnsPolicy2 := v1alpha1.NewDNSPolicy("test-dns-policy2", testNamespace). diff --git a/tests/common/tlspolicy/tlspolicy_controller_test.go b/tests/common/tlspolicy/tlspolicy_controller_test.go index a6261fe52..4e3ba90ed 100644 --- a/tests/common/tlspolicy/tlspolicy_controller_test.go +++ b/tests/common/tlspolicy/tlspolicy_controller_test.go @@ -8,12 +8,11 @@ import ( "fmt" "time" + certmanv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + certmanmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gstruct" - - certmanv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - certmanmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" k8certsv1 "k8s.io/api/certificates/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" @@ -415,12 +414,14 @@ var _ = Describe("TLSPolicy controller", func() { //confirm a certificate has been deleted Eventually(func() error { certificateList := &certmanv1.CertificateList{} - Expect(k8sClient.List(ctx, certificateList, &client.ListOptions{Namespace: testNamespace})).To(BeNil()) + if err := k8sClient.List(ctx, certificateList, &client.ListOptions{Namespace: testNamespace}); err != nil { + return err + } if len(certificateList.Items) != 2 { return fmt.Errorf("expected 2 certificates, found: %v", len(certificateList.Items)) } return nil - }, time.Second*120, time.Second).Should(BeNil()) + }, tests.TimeoutMedium, time.Second).Should(BeNil()) }) It("should delete all tls certificates when tls policy is removed even if gateway is already removed", func() {