From cbfe9f72c17888308ba248c070b50928879a1e31 Mon Sep 17 00:00:00 2001 From: Erik Stidham Date: Wed, 12 Feb 2025 09:52:59 -0600 Subject: [PATCH 1/2] Make manager proceed with deploying even with voltron route config error Deploy the majority of manager even if there is a voltron route config error for the cases where part of the config depends on something manager will create. --- pkg/controller/manager/manager_controller.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/controller/manager/manager_controller.go b/pkg/controller/manager/manager_controller.go index 91ce8ea621..6ed08a4eb9 100644 --- a/pkg/controller/manager/manager_controller.go +++ b/pkg/controller/manager/manager_controller.go @@ -637,10 +637,9 @@ func (r *ReconcileManager) Reconcile(ctx context.Context, request reconcile.Requ return reconcile.Result{}, err } - routeConfig, err := getVoltronRouteConfig(ctx, r.client, helper.InstallNamespace()) - if err != nil { - r.status.SetDegraded(operatorv1.InternalServerError, "Failed to create Voltron Route Configuration", err, logc) - return reconcile.Result{}, err + routeConfig, routeConfigErr := getVoltronRouteConfig(ctx, r.client, helper.InstallNamespace()) + if routeConfigErr != nil { + log.Error(routeConfigErr, "error with voltron route config, continuing") } // Check if non-cluster host log ingestion is enabled. @@ -725,6 +724,11 @@ func (r *ReconcileManager) Reconcile(ctx context.Context, request reconcile.Requ } } + if routeConfigErr != nil { + r.status.SetDegraded(operatorv1.InternalServerError, "Failed to create Voltron Route Configuration", routeConfigErr, logc) + return reconcile.Result{}, routeConfigErr + } + // Clear the degraded bit if we've reached this far. r.status.ClearDegraded() instance.Status.State = operatorv1.TigeraStatusReady From f88ac587aa76354b95df4fdb6ccd768bbb3f10a1 Mon Sep 17 00:00:00 2001 From: Erik Stidham Date: Tue, 18 Feb 2025 12:26:13 -0600 Subject: [PATCH 2/2] Add tests for route config changes --- pkg/controller/manager/manager_controller.go | 4 +- .../manager/manager_controller_test.go | 178 +++++++++++++++++- 2 files changed, 179 insertions(+), 3 deletions(-) diff --git a/pkg/controller/manager/manager_controller.go b/pkg/controller/manager/manager_controller.go index 6ed08a4eb9..0bba9986fa 100644 --- a/pkg/controller/manager/manager_controller.go +++ b/pkg/controller/manager/manager_controller.go @@ -639,7 +639,7 @@ func (r *ReconcileManager) Reconcile(ctx context.Context, request reconcile.Requ routeConfig, routeConfigErr := getVoltronRouteConfig(ctx, r.client, helper.InstallNamespace()) if routeConfigErr != nil { - log.Error(routeConfigErr, "error with voltron route config, continuing") + log.Error(routeConfigErr, "error with voltron route config, route configuration will not be added to voltron") } // Check if non-cluster host log ingestion is enabled. @@ -724,6 +724,8 @@ func (r *ReconcileManager) Reconcile(ctx context.Context, request reconcile.Requ } } + // Don't block creating manager resources if there is a route error (so we are waiting until after CreateOrUpdateOrDelete) + // but do go degraded after creating the resources if there is a route error. if routeConfigErr != nil { r.status.SetDegraded(operatorv1.InternalServerError, "Failed to create Voltron Route Configuration", routeConfigErr, logc) return reconcile.Result{}, routeConfigErr diff --git a/pkg/controller/manager/manager_controller_test.go b/pkg/controller/manager/manager_controller_test.go index f613a6ab23..593770f409 100644 --- a/pkg/controller/manager/manager_controller_test.go +++ b/pkg/controller/manager/manager_controller_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2020-2024 Tigera, Inc. All rights reserved. +// Copyright (c) 2020-2025 Tigera, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -426,7 +426,6 @@ var _ = Describe("Manager controller tests", func() { var mockStatus *status.MockStatus var licenseKey *v3.LicenseKey var compliance *operatorv1.Compliance - var certificateManager certificatemanager.CertificateManager var installation *operatorv1.Installation BeforeEach(func() { @@ -495,6 +494,7 @@ var _ = Describe("Manager controller tests", func() { }) Context("single-tenant", func() { + var certificateManager certificatemanager.CertificateManager BeforeEach(func() { mockStatus.On("AddDaemonsets", mock.Anything).Return() mockStatus.On("AddDeployments", mock.Anything).Return() @@ -1341,6 +1341,180 @@ var _ = Describe("Manager controller tests", func() { Expect(kerror.IsNotFound(test.GetResource(c, &tenantBRoutes))).Should(BeTrue()) }) }) + + Context("TSL route misconfiguration", func() { + var certificateManager certificatemanager.CertificateManager + BeforeEach(func() { + mockStatus.On("AddDaemonsets", mock.Anything).Return() + mockStatus.On("AddDeployments", mock.Anything).Return() + mockStatus.On("AddStatefulSets", mock.Anything).Return() + mockStatus.On("AddCronJobs", mock.Anything) + mockStatus.On("IsAvailable").Return(true) + mockStatus.On("OnCRFound").Return() + mockStatus.On("ClearDegraded") + mockStatus.On("RemoveCertificateSigningRequests", mock.Anything) + mockStatus.On("ReadyToMonitor") + mockStatus.On("SetMetaData", mock.Anything).Return() + + // Mark that watches were successful. + r.licenseAPIReady.MarkAsReady() + r.tierWatchReady.MarkAsReady() + }) + It("single-tenant", func() { + compliance = &operatorv1.Compliance{ + ObjectMeta: metav1.ObjectMeta{Name: "tigera-secure"}, + Status: operatorv1.ComplianceStatus{ + State: operatorv1.TigeraStatusReady, + }, + } + Expect(c.Create(ctx, compliance)).NotTo(HaveOccurred()) + // Provision certificates that the controller will query as part of the test. + var err error + certificateManager, err = certificatemanager.Create(c, nil, "", common.OperatorNamespace(), certificatemanager.AllowCACreation()) + Expect(err).NotTo(HaveOccurred()) + caSecret := certificateManager.KeyPair().Secret(common.OperatorNamespace()) + Expect(c.Create(ctx, caSecret)).NotTo(HaveOccurred()) + complianceKp, err := certificateManager.GetOrCreateKeyPair(c, render.ComplianceServerCertSecret, common.OperatorNamespace(), []string{render.ComplianceServerCertSecret}) + Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(ctx, complianceKp.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred()) + pcapKp, err := certificateManager.GetOrCreateKeyPair(c, render.PacketCaptureServerCert, common.OperatorNamespace(), []string{render.PacketCaptureServerCert}) + Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(ctx, pcapKp.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred()) + promKp, err := certificateManager.GetOrCreateKeyPair(c, monitor.PrometheusServerTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusServerTLSSecretName}) + Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(ctx, promKp.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred()) + linseedKp, err := certificateManager.GetOrCreateKeyPair(c, render.TigeraLinseedSecret, common.OperatorNamespace(), []string{render.TigeraLinseedSecret}) + Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(ctx, linseedKp.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred()) + queryServerKp, err := certificateManager.GetOrCreateKeyPair(c, render.ProjectCalicoAPIServerTLSSecretName(operatorv1.TigeraSecureEnterprise), common.OperatorNamespace(), []string{render.ProjectCalicoAPIServerTLSSecretName(operatorv1.TigeraSecureEnterprise)}) + Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(ctx, queryServerKp.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred()) + internalCertKp, err := certificateManager.GetOrCreateKeyPair(c, render.ManagerInternalTLSSecretName, common.OperatorNamespace(), []string{render.ManagerInternalTLSSecretName}) + Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(ctx, internalCertKp.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred()) + + Expect(c.Create(ctx, relasticsearch.NewClusterConfig("cluster", 1, 1, 1).ConfigMap())).NotTo(HaveOccurred()) + Expect(c.Create(ctx, &operatorv1.Manager{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tigera-secure", + }, + })).NotTo(HaveOccurred()) + Expect(c.Create(ctx, &operatorv1.TLSTerminatedRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tigera-manager", + Name: "route", + }, + Spec: operatorv1.TLSTerminatedRouteSpec{ + CABundle: &corev1.ConfigMapKeySelector{ + Key: "tigera-ca-bundle.crt", + LocalObjectReference: corev1.LocalObjectReference{ + Name: "non-existent-configmap", + }, + }, + Destination: "https://internal.foo.svc", + PathMatch: &operatorv1.PathMatch{ + Path: "/foo/", + }, + Target: "UI", + }, + })).NotTo(HaveOccurred()) + + // Expect a degraded status to be set since the CABundle does not exist + mockStatus.On("SetDegraded", operatorv1.InternalServerError, "Failed to create Voltron Route Configuration", mock.Anything, mock.Anything).Return() + _, err = r.Reconcile(ctx, reconcile.Request{}) + Expect(err).To(HaveOccurred()) + + deployment := appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{Kind: "Deployment", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "tigera-manager", + Namespace: render.ManagerNamespace, + }, + } + // Ensure a deployment was created for the manager + err = test.GetResource(c, &deployment) + Expect(err).NotTo(HaveOccurred()) + }) + It("Tenant configuration", func() { + tenantANamespace := "tenant-a" + r.multiTenant = true + + Expect(c.Create(ctx, &operatorv1.TLSTerminatedRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: tenantANamespace, + Name: "tenant-a-route", + }, + Spec: operatorv1.TLSTerminatedRouteSpec{ + CABundle: &corev1.ConfigMapKeySelector{ + Key: "tigera-ca-bundle.crt", + LocalObjectReference: corev1.LocalObjectReference{ + Name: "non-existent-configmap", + }, + }, + Destination: "https://internal.foo.svc", + PathMatch: &operatorv1.PathMatch{ + Path: "/foo/", + }, + Target: "UI", + }, + })).NotTo(HaveOccurred()) + + // Create the Tenant resources for tenant-a and tenant-b. + tenantA := &operatorv1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: tenantANamespace, + }, + Spec: operatorv1.TenantSpec{ID: "tenant-a"}, + } + Expect(c.Create(ctx, tenantA)).NotTo(HaveOccurred()) + + managementCluster := &operatorv1.ManagementCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tigera-secure", + }, + } + Expect(c.Create(ctx, managementCluster)).NotTo(HaveOccurred()) + + certificateManagerTenantA, err := certificatemanager.Create(c, nil, "", tenantANamespace, certificatemanager.AllowCACreation(), certificatemanager.WithTenant(tenantA)) + Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(ctx, certificateManagerTenantA.KeyPair().Secret(tenantANamespace))) + managerTLSTenantA, err := certificateManagerTenantA.GetOrCreateKeyPair(c, render.ManagerInternalTLSSecretName, tenantANamespace, []string{render.ManagerInternalTLSSecretName}) + Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(ctx, managerTLSTenantA.Secret(tenantANamespace))).NotTo(HaveOccurred()) + bundleA, err := certificateManagerTenantA.CreateMultiTenantTrustedBundleWithSystemRootCertificates() + Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(ctx, bundleA.ConfigMap(tenantANamespace))).NotTo(HaveOccurred()) + + err = c.Create(ctx, &operatorv1.Manager{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tigera-secure", + Namespace: tenantANamespace, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + // Expect a degraded status to be set since the CABundle does not exist + mockStatus.On("SetDegraded", operatorv1.InternalServerError, "Failed to create Voltron Route Configuration", mock.Anything, mock.Anything).Return() + _, err = r.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: tenantANamespace, + }, + }) + Expect(err).To(HaveOccurred()) + + deployment := appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{Kind: "Deployment", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "tigera-manager", + Namespace: tenantANamespace, + }, + } + // Ensure a deployment was created for the manager + err = test.GetResource(c, &deployment) + Expect(err).NotTo(HaveOccurred()) + }) + }) }) })