Skip to content

Commit

Permalink
Fix issue where controller is watching a secret that is never created (
Browse files Browse the repository at this point in the history
…#3067)

* Fix issue where controller is watching a secret that is never created

* Fix the unit tests
  • Loading branch information
rene-dekker authored Dec 15, 2023
1 parent 8a8dac1 commit ebf774d
Show file tree
Hide file tree
Showing 11 changed files with 37 additions and 16 deletions.
14 changes: 13 additions & 1 deletion pkg/controller/clusterconnection/clusterconnection_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,19 @@ func (r *ReconcileConnection) Reconcile(ctx context.Context, request reconcile.R
trustedCertBundle = certificateManager.CreateTrustedBundle()
}

for _, secretName := range []string{render.PacketCaptureServerCert, monitor.PrometheusServerTLSSecretName, render.ProjectCalicoAPIServerTLSSecretName(instl.Variant)} {
secretsToTrust := []string{render.PacketCaptureServerCert, render.ProjectCalicoAPIServerTLSSecretName(instl.Variant)}
// If external prometheus is enabled, the secret will be signed by the Calico CA and won't get rendered. We can skip
// adding it to the bundle, as trusting the CA will suffice.
monitorCR := &operatorv1.Monitor{}
if err := r.Client.Get(ctx, utils.DefaultTSEEInstanceKey, monitorCR); err != nil {
r.status.SetDegraded(operatorv1.ResourceReadError, "Error querying required Monitor resource: ", err, reqLogger)
return reconcile.Result{}, err
}
if monitorCR.Spec.ExternalPrometheus == nil {
secretsToTrust = append(secretsToTrust, monitor.PrometheusServerTLSSecretName)
}

for _, secretName := range secretsToTrust {
secret, err := certificateManager.GetCertificate(r.Client, secretName, common.OperatorNamespace())
if err != nil {
r.status.SetDegraded(operatorv1.ResourceReadError, fmt.Sprintf("Failed to retrieve %s", secretName), err, reqLogger)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ var _ = Describe("ManagementClusterConnection controller tests", func() {
ctx = context.Background()
mockStatus = &status.MockStatus{}
mockStatus.On("Run").Return()

mockStatus.On("AddDaemonsets", mock.Anything)
mockStatus.On("AddDeployments", mock.Anything)
mockStatus.On("AddStatefulSets", mock.Anything)
Expand All @@ -82,7 +81,9 @@ var _ = Describe("ManagementClusterConnection controller tests", func() {
mockStatus.On("OnCRFound").Return()
mockStatus.On("ReadyToMonitor")
mockStatus.On("SetMetaData", mock.Anything).Return()

Expect(c.Create(ctx, &operatorv1.Monitor{
ObjectMeta: metav1.ObjectMeta{Name: "tigera-secure"},
}))
r = clusterconnection.NewReconcilerWithShims(c, scheme, mockStatus, operatorv1.ProviderNone, ready)
dpl = &appsv1.Deployment{
TypeMeta: metav1.TypeMeta{Kind: "Deployment", APIVersion: "apps/v1"},
Expand Down
4 changes: 0 additions & 4 deletions pkg/controller/installation/core_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,6 @@ func add(c controller.Controller, r *ReconcileInstallation) error {
return fmt.Errorf("tigera-installation-controller failed to watch primary resource: %v", err)
}

if err = utils.AddSecretsWatch(c, monitor.PrometheusServerTLSSecretName, common.TigeraPrometheusNamespace); err != nil {
return fmt.Errorf("tigera-installation-controller failed to watch secret '%s' in '%s' namespace: %w", monitor.PrometheusServerTLSSecretName, common.OperatorNamespace(), err)
}

// watch for change to primary resource LogCollector
err = c.Watch(&source.Kind{Type: &operator.LogCollector{}}, &handler.EnqueueRequestForObject{})
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/installation/core_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@ var _ = Describe("Testing core-controller installation", func() {
certificateManager, err = certificatemanager.Create(c, nil, "", common.OperatorNamespace(), certificatemanager.AllowCACreation())
Expect(err).NotTo(HaveOccurred())
Expect(c.Create(ctx, certificateManager.KeyPair().Secret(common.OperatorNamespace()))) // Persist the root-ca in the operator namespace.
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(c, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusServerTLSSecretName})
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(c, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusClientTLSSecretName})
Expect(err).NotTo(HaveOccurred())
Expect(c.Create(ctx, prometheusTLS.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred())
Expect(c.Create(ctx, &v3.Tier{ObjectMeta: metav1.ObjectMeta{Name: "allow-tigera"}})).NotTo(HaveOccurred())
Expand Down Expand Up @@ -994,7 +994,7 @@ var _ = Describe("Testing core-controller installation", func() {
}
certificateManager, err := certificatemanager.Create(c, nil, "", common.OperatorNamespace(), certificatemanager.AllowCACreation())
Expect(err).NotTo(HaveOccurred())
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(c, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusServerTLSSecretName})
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(c, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusClientTLSSecretName})
Expect(err).NotTo(HaveOccurred())
Expect(c.Create(ctx, prometheusTLS.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred())
Expect(c.Create(ctx, &v3.Tier{ObjectMeta: metav1.ObjectMeta{Name: "allow-tigera"}})).NotTo(HaveOccurred())
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/installation/windows_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ var _ = Describe("windows-controller installation tests", func() {
Expect(updateInstallationWithDefaults(ctx, r.client, cr, r.autoDetectedProvider)).NotTo(HaveOccurred())
certificateManager, err := certificatemanager.Create(c, nil, "", common.OperatorNamespace(), certificatemanager.AllowCACreation())
Expect(err).NotTo(HaveOccurred())
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(c, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusServerTLSSecretName})
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(c, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusClientTLSSecretName})
Expect(err).NotTo(HaveOccurred())
Expect(c.Create(ctx, prometheusTLS.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred())
Expect(c.Create(ctx, &v3.Tier{ObjectMeta: metav1.ObjectMeta{Name: "allow-tigera"}})).NotTo(HaveOccurred())
Expand Down Expand Up @@ -604,7 +604,7 @@ var _ = Describe("windows-controller installation tests", func() {

certificateManager, err := certificatemanager.Create(c, nil, "", common.OperatorNamespace(), certificatemanager.AllowCACreation())
Expect(err).NotTo(HaveOccurred())
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(c, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusServerTLSSecretName})
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(c, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusClientTLSSecretName})
Expect(err).NotTo(HaveOccurred())
Expect(c.Create(ctx, prometheusTLS.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred())
Expect(c.Create(ctx, certificateManager.KeyPair().Secret(common.OperatorNamespace()))).NotTo(HaveOccurred())
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/logcollector/logcollector_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func add(mgr manager.Manager, c controller.Controller) error {
for _, secretName := range []string{
render.ElasticsearchEksLogForwarderUserSecret,
render.S3FluentdSecretName, render.EksLogForwarderSecret,
render.SplunkFluentdTokenSecretName, render.SplunkFluentdCertificateSecretName, monitor.PrometheusServerTLSSecretName,
render.SplunkFluentdTokenSecretName, render.SplunkFluentdCertificateSecretName, monitor.PrometheusClientTLSSecretName,
render.FluentdPrometheusTLSSecretName, render.TigeraLinseedSecret, render.VoltronLinseedPublicCert, render.EKSLogForwarderTLSSecretName,
} {
if err = utils.AddSecretsWatch(c, secretName, common.OperatorNamespace()); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ var _ = Describe("LogCollector controller tests", func() {
},
})).NotTo(HaveOccurred())

prometheusTLS, err := certificateManager.GetOrCreateKeyPair(c, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusServerTLSSecretName})
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(c, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusClientTLSSecretName})
Expect(err).NotTo(HaveOccurred())
Expect(c.Create(ctx, prometheusTLS.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred())

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ var _ = Describe("LogStorage controller", func() {
certificateManager, err = certificatemanager.Create(cli, nil, "", common.OperatorNamespace(), certificatemanager.AllowCACreation())
Expect(err).NotTo(HaveOccurred())
Expect(cli.Create(ctx, certificateManager.KeyPair().Secret(common.OperatorNamespace()))) // Persist the root-ca in the operator namespace.
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(cli, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusServerTLSSecretName})
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(cli, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusClientTLSSecretName})
Expect(err).NotTo(HaveOccurred())
Expect(cli.Create(ctx, prometheusTLS.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred())

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ var _ = Describe("External ES Controller", func() {
bundle := certificateManager.CreateTrustedBundle(esKeyPair)
Expect(cli.Create(ctx, bundle.ConfigMap(render.ElasticsearchNamespace))).NotTo(HaveOccurred())

prometheusTLS, err := certificateManager.GetOrCreateKeyPair(cli, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusServerTLSSecretName})
prometheusTLS, err := certificateManager.GetOrCreateKeyPair(cli, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace(), []string{monitor.PrometheusClientTLSSecretName})
Expect(err).NotTo(HaveOccurred())

Expect(cli.Create(ctx, prometheusTLS.Secret(common.OperatorNamespace()))).NotTo(HaveOccurred())
Expand Down
11 changes: 10 additions & 1 deletion pkg/controller/manager/manager_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,10 +395,19 @@ func (r *ReconcileManager) Reconcile(ctx context.Context, request reconcile.Requ
// any of them haven't been signed by the root CA.
trustedSecretNames = []string{
render.PacketCaptureServerCert,
monitor.PrometheusServerTLSSecretName,
render.ProjectCalicoAPIServerTLSSecretName(installation.Variant),
render.TigeraLinseedSecret,
}
// If external prometheus is enabled, the secret will be signed by the Calico CA and no secret will be created. We can skip
// adding it to the bundle, as trusting the CA will suffice.
monitorCR := &operatorv1.Monitor{}
if err := r.client.Get(ctx, utils.DefaultTSEEInstanceKey, monitorCR); err != nil {
r.status.SetDegraded(operatorv1.ResourceReadError, "Error querying required Monitor resource: ", err, logc)
return reconcile.Result{}, err
}
if monitorCR.Spec.ExternalPrometheus == nil {
trustedSecretNames = append(trustedSecretNames, monitor.PrometheusServerTLSSecretName)
}

if complianceLicenseFeatureActive && complianceCR != nil {
// Check that compliance is running.
Expand Down
3 changes: 3 additions & 0 deletions pkg/controller/manager/manager_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ var _ = Describe("Manager controller tests", func() {
c = fake.NewClientBuilder().WithScheme(scheme).Build()
ctx = context.Background()
replicas = 2
Expect(c.Create(ctx, &operatorv1.Monitor{
ObjectMeta: metav1.ObjectMeta{Name: "tigera-secure"},
}))
})

It("should query a default manager instance", func() {
Expand Down

0 comments on commit ebf774d

Please sign in to comment.