From b02411760bbf8d13267a99a42717a3574552c24d Mon Sep 17 00:00:00 2001 From: Laura Fitzgerald Date: Thu, 21 Sep 2023 11:14:21 +0100 Subject: [PATCH] validate the presence of specified cluster or clusterisssuer in tlspolicy --- ...eway-controller.clusterserviceversion.yaml | 16 ++++- config/rbac/role.yaml | 14 +++++ .../tlspolicy/certmanager_helper.go | 20 ++++++ .../tlspolicy/tlspolicy_controller.go | 7 +++ test/e2e/gateway_single_spoke_test.go | 2 +- test/integration/tlspolicy_controller_test.go | 62 +++++++++++++++++-- test/util/suite_config.go | 5 ++ test/util/test_types.go | 18 ++++++ 8 files changed, 137 insertions(+), 7 deletions(-) diff --git a/bundle/manifests/multicluster-gateway-controller.clusterserviceversion.yaml b/bundle/manifests/multicluster-gateway-controller.clusterserviceversion.yaml index 832d59d0e..dbbed6b45 100644 --- a/bundle/manifests/multicluster-gateway-controller.clusterserviceversion.yaml +++ b/bundle/manifests/multicluster-gateway-controller.clusterserviceversion.yaml @@ -4,7 +4,7 @@ metadata: annotations: alm-examples: '[]' capabilities: Basic Install - createdAt: "2023-09-14T13:57:32Z" + createdAt: "2023-09-21T16:23:26Z" operators.operatorframework.io/builder: operator-sdk-v1.28.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 name: multicluster-gateway-controller.v0.0.0 @@ -206,6 +206,20 @@ spec: - patch - update - watch + - apiGroups: + - cert-manager.io + resources: + - clusterissuers + verbs: + - get + - list + - apiGroups: + - cert-manager.io + resources: + - issuers + verbs: + - get + - list - apiGroups: - cluster.open-cluster-management.io resources: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index f85c9be1f..a8ed95f9a 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -38,6 +38,20 @@ rules: - patch - update - watch +- apiGroups: + - cert-manager.io + resources: + - clusterissuers + verbs: + - get + - list +- apiGroups: + - cert-manager.io + resources: + - issuers + verbs: + - get + - list - apiGroups: - cluster.open-cluster-management.io resources: diff --git a/pkg/controllers/tlspolicy/certmanager_helper.go b/pkg/controllers/tlspolicy/certmanager_helper.go index 50259cf7b..3ea5620c9 100644 --- a/pkg/controllers/tlspolicy/certmanager_helper.go +++ b/pkg/controllers/tlspolicy/certmanager_helper.go @@ -1,10 +1,14 @@ package tlspolicy import ( + "context" + "fmt" + certmanv1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/Kuadrant/multicluster-gateway-controller/pkg/apis/v1alpha1" @@ -113,3 +117,19 @@ func translatePolicy(crt *certmanv1.Certificate, tlsPolicy v1alpha1.TLSPolicySpe } } + +// validateIssuer validates that the issuer specified exists +func validateIssuer(ctx context.Context, k8sClient client.Client, policy *v1alpha1.TLSPolicy) error { + var issuer client.Object + issuerNamespace := "" + switch policy.Spec.IssuerRef.Kind { + case "", certmanv1.IssuerKind: + issuer = &certmanv1.Issuer{} + issuerNamespace = policy.Namespace + case certmanv1.ClusterIssuerKind: + issuer = &certmanv1.ClusterIssuer{} + default: + return fmt.Errorf(`invalid value %q for issuerRef.kind. Must be empty, %q or %q`, policy.Spec.IssuerRef.Kind, certmanv1.IssuerKind, certmanv1.ClusterIssuerKind) + } + return k8sClient.Get(ctx, client.ObjectKey{Name: policy.Spec.IssuerRef.Name, Namespace: issuerNamespace}, issuer) +} diff --git a/pkg/controllers/tlspolicy/tlspolicy_controller.go b/pkg/controllers/tlspolicy/tlspolicy_controller.go index c7461e746..1cac85c38 100644 --- a/pkg/controllers/tlspolicy/tlspolicy_controller.go +++ b/pkg/controllers/tlspolicy/tlspolicy_controller.go @@ -65,6 +65,8 @@ type TLSPolicyReconciler struct { //+kubebuilder:rbac:groups=kuadrant.io,resources=tlspolicies,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=kuadrant.io,resources=tlspolicies/status,verbs=get;update;patch //+kubebuilder:rbac:groups=kuadrant.io,resources=tlspolicies/finalizers,verbs=update +//+kubebuilder:rbac:groups="cert-manager.io",resources=issuers,verbs=get;list; +//+kubebuilder:rbac:groups="cert-manager.io",resources=clusterissuers,verbs=get;list; func (r *TLSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := r.Logger().WithValues("TLSPolicy", req.NamespacedName) @@ -151,6 +153,11 @@ func (r *TLSPolicyReconciler) reconcileResources(ctx context.Context, tlsPolicy return err } + err = validateIssuer(ctx, r.Client(), tlsPolicy) + if err != nil { + return err + } + // reconcile based on gateway diffs gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, tlsPolicy, targetNetworkObject, &TLSPolicyRefsConfig{}) if err != nil { diff --git a/test/e2e/gateway_single_spoke_test.go b/test/e2e/gateway_single_spoke_test.go index 433af03c9..6a248dbbd 100644 --- a/test/e2e/gateway_single_spoke_test.go +++ b/test/e2e/gateway_single_spoke_test.go @@ -87,7 +87,7 @@ var _ = Describe("Gateway single target cluster", func() { err = tconfig.HubClient().Create(ctx, gw) Expect(err).ToNot(HaveOccurred()) - By("setting up TLSPolicy in the hub") + By("setting up TLSPolicy in the hub") tlsPolicy = &mgcv1alpha1.TLSPolicy{ ObjectMeta: metav1.ObjectMeta{ Name: testID, diff --git a/test/integration/tlspolicy_controller_test.go b/test/integration/tlspolicy_controller_test.go index 32a5ddbb4..617cd61d2 100644 --- a/test/integration/tlspolicy_controller_test.go +++ b/test/integration/tlspolicy_controller_test.go @@ -27,6 +27,7 @@ var _ = Describe("TLSPolicy", Ordered, func() { var testNamespace string var gatewayClass *gatewayv1beta1.GatewayClass + var issuer *certmanv1.Issuer BeforeAll(func() { logger = zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)) @@ -42,6 +43,11 @@ var _ = Describe("TLSPolicy", Ordered, func() { BeforeEach(func() { CreateNamespace(&testNamespace) + issuer = NewTestIssuer("testissuer", testNamespace) + Expect(k8sClient.Create(ctx, issuer)).To(BeNil()) + Eventually(func() error { //issuer exists + return k8sClient.Get(ctx, client.ObjectKey{Name: issuer.Name, Namespace: issuer.Namespace}, issuer) + }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) }) AfterEach(func() { @@ -55,6 +61,11 @@ var _ = Describe("TLSPolicy", Ordered, func() { for _, policy := range policyList.Items { k8sClient.Delete(ctx, &policy) } + issuerList := certmanv1.IssuerList{} + Expect(k8sClient.List(ctx, &issuerList)).To(BeNil()) + for _, issuer := range issuerList.Items { + k8sClient.Delete(ctx, &issuer) + } }) AfterAll(func() { @@ -74,7 +85,7 @@ var _ = Describe("TLSPolicy", Ordered, func() { Expect(err).ToNot(HaveOccurred()) }) - Context("valid target and policy", func() { + Context("valid target, issuer and policy", func() { BeforeEach(func() { gateway = NewTestGateway("test-gateway", gwClassName, testNamespace). @@ -84,7 +95,8 @@ var _ = Describe("TLSPolicy", Ordered, func() { return k8sClient.Get(ctx, client.ObjectKey{Name: gateway.Name, Namespace: gateway.Namespace}, gateway) }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) tlsPolicy = NewTestTLSPolicy("test-tls-policy", testNamespace). - WithTargetGateway(gateway.Name).TLSPolicy + WithTargetGateway(gateway.Name). + WithIssuer("testissuer", certmanv1.IssuerKind, "cert-manager.io").TLSPolicy Expect(k8sClient.Create(ctx, tlsPolicy)).To(BeNil()) Eventually(func() error { //tls policy exists return k8sClient.Get(ctx, client.ObjectKey{Name: tlsPolicy.Name, Namespace: tlsPolicy.Namespace}, tlsPolicy) @@ -144,6 +156,43 @@ var _ = Describe("TLSPolicy", Ordered, func() { }) + Context("valid target, clusterissuer and policy", func() { + var clusterIssuer *certmanv1.ClusterIssuer + + BeforeEach(func() { + gateway = NewTestGateway("test-gateway", gwClassName, testNamespace). + WithHTTPListener("test.example.com").Gateway + Expect(k8sClient.Create(ctx, gateway)).To(BeNil()) + Eventually(func() error { //gateway exists + return k8sClient.Get(ctx, client.ObjectKey{Name: gateway.Name, Namespace: gateway.Namespace}, gateway) + }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) + tlsPolicy = NewTestTLSPolicy("test-tls-policy", testNamespace). + WithTargetGateway(gateway.Name). + WithIssuer("testclusterissuer", certmanv1.ClusterIssuerKind, "cert-manager.io").TLSPolicy + Expect(k8sClient.Create(ctx, tlsPolicy)).To(BeNil()) + Eventually(func() error { //tls policy exists + return k8sClient.Get(ctx, client.ObjectKey{Name: tlsPolicy.Name, Namespace: tlsPolicy.Namespace}, tlsPolicy) + }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) + clusterIssuer = NewTestClusterIssuer("testclusterissuer") + Expect(k8sClient.Create(ctx, clusterIssuer)).To(BeNil()) + Eventually(func() error { //clusterIssuer exists + return k8sClient.Get(ctx, client.ObjectKey{Name: clusterIssuer.Name}, clusterIssuer) + }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) + }) + + It("should have ready status", func() { + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKey{Name: tlsPolicy.Name, Namespace: tlsPolicy.Namespace}, tlsPolicy); err != nil { + return err + } + if !meta.IsStatusConditionTrue(tlsPolicy.Status.Conditions, string(conditions.ConditionTypeReady)) { + return fmt.Errorf("expected tlsPolicy status condition to be %s", string(conditions.ConditionTypeReady)) + } + return nil + }, time.Second*15, time.Second).Should(BeNil()) + }) + }) + Context("with http listener", func() { BeforeEach(func() { @@ -154,7 +203,8 @@ var _ = Describe("TLSPolicy", Ordered, func() { return k8sClient.Get(ctx, client.ObjectKey{Name: gateway.Name, Namespace: gateway.Namespace}, gateway) }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) tlsPolicy = NewTestTLSPolicy("test-tls-policy", testNamespace). - WithTargetGateway(gateway.Name).TLSPolicy + WithTargetGateway(gateway.Name). + WithIssuer("testissuer", certmanv1.IssuerKind, "cert-manager.io").TLSPolicy Expect(k8sClient.Create(ctx, tlsPolicy)).To(BeNil()) Eventually(func() error { //tls policy exists return k8sClient.Get(ctx, client.ObjectKey{Name: tlsPolicy.Name, Namespace: tlsPolicy.Namespace}, tlsPolicy) @@ -182,7 +232,8 @@ var _ = Describe("TLSPolicy", Ordered, func() { return k8sClient.Get(ctx, client.ObjectKey{Name: gateway.Name, Namespace: gateway.Namespace}, gateway) }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) tlsPolicy = NewTestTLSPolicy("test-tls-policy", testNamespace). - WithTargetGateway(gateway.Name).TLSPolicy + WithTargetGateway(gateway.Name). + WithIssuer("testissuer", certmanv1.IssuerKind, "cert-manager.io").TLSPolicy Expect(k8sClient.Create(ctx, tlsPolicy)).To(BeNil()) Eventually(func() error { //tls policy exists return k8sClient.Get(ctx, client.ObjectKey{Name: tlsPolicy.Name, Namespace: tlsPolicy.Namespace}, tlsPolicy) @@ -218,7 +269,8 @@ var _ = Describe("TLSPolicy", Ordered, func() { return k8sClient.Get(ctx, client.ObjectKey{Name: gateway.Name, Namespace: gateway.Namespace}, gateway) }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) tlsPolicy = NewTestTLSPolicy("test-tls-policy", testNamespace). - WithTargetGateway(gateway.Name).TLSPolicy + WithTargetGateway(gateway.Name). + WithIssuer("testissuer", certmanv1.IssuerKind, "cert-manager.io").TLSPolicy Expect(k8sClient.Create(ctx, tlsPolicy)).To(BeNil()) Eventually(func() error { //tls policy exists return k8sClient.Get(ctx, client.ObjectKey{Name: tlsPolicy.Name, Namespace: tlsPolicy.Namespace}, tlsPolicy) diff --git a/test/util/suite_config.go b/test/util/suite_config.go index 8c8c02518..5ad7b82b7 100644 --- a/test/util/suite_config.go +++ b/test/util/suite_config.go @@ -10,6 +10,7 @@ import ( "strconv" "github.com/goombaio/namegenerator" + certmanv1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" ocm_cluster_v1 "open-cluster-management.io/api/cluster/v1" ocm_cluster_v1beta1 "open-cluster-management.io/api/cluster/v1beta1" ocm_cluster_v1beta2 "open-cluster-management.io/api/cluster/v1beta2" @@ -110,6 +111,10 @@ func (cfg *SuiteConfig) Build() error { if err != nil { return err } + err = certmanv1.AddToScheme(scheme.Scheme) + if err != nil { + return err + } cfg.cpClient, err = client.New(restcfg, client.Options{Scheme: scheme.Scheme}) if err != nil { diff --git a/test/util/test_types.go b/test/util/test_types.go index 2e67a867c..03537f9bf 100644 --- a/test/util/test_types.go +++ b/test/util/test_types.go @@ -3,6 +3,7 @@ package testutil import ( + certmanv1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -34,6 +35,23 @@ func NewTestGateway(gwName, gwClassName, ns string) *TestGateway { } } +func NewTestIssuer(name, ns string) *certmanv1.Issuer { + return &certmanv1.Issuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: ns, + }, + } +} + +func NewTestClusterIssuer(name string) *certmanv1.ClusterIssuer { + return &certmanv1.ClusterIssuer{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } +} + func (t *TestGateway) WithListener(listener gatewayv1beta1.Listener) *TestGateway { t.Spec.Listeners = append(t.Spec.Listeners, listener) return t