diff --git a/config/prometheus/clusterrole_binding.yaml b/config/prometheus/clusterrole_binding.yaml index 0e42b559f..586124381 100644 --- a/config/prometheus/clusterrole_binding.yaml +++ b/config/prometheus/clusterrole_binding.yaml @@ -14,4 +14,4 @@ subjects: - kind: ServiceAccount name: metrics-reader - namespace: openshift-lightspeed + namespace: system diff --git a/internal/controller/appserver/assets.go b/internal/controller/appserver/assets.go index a0a39743d..712dea4cd 100644 --- a/internal/controller/appserver/assets.go +++ b/internal/controller/appserver/assets.go @@ -1004,6 +1004,33 @@ func GenerateMetricsReaderSecret(r reconciler.Reconciler, cr *olsv1alpha1.OLSCon return secret, nil } +func generateMetricsReaderClusterRoleBinding(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) (*rbacv1.ClusterRoleBinding, error) { + rb := rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: utils.MetricsReaderClusterRoleBindingName, + Labels: utils.GenerateAppServerSelectorLabels(), + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + Name: utils.MetricsReaderServiceAccountName, + Namespace: r.GetNamespace(), + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: utils.MetricsReaderClusterRoleName, + }, + } + + if err := controllerutil.SetControllerReference(cr, &rb, r.GetScheme()); err != nil { + return nil, fmt.Errorf("%s: %w", utils.ErrSetMetricsReaderCRBOwnerReference, err) + } + + return &rb, nil +} + func serviceTLSProfileType(profileType configv1.TLSProfileType) string { switch profileType { case configv1.TLSProfileOldType: diff --git a/internal/controller/appserver/assets_test.go b/internal/controller/appserver/assets_test.go index 9bc02b2d1..73a23a4e1 100644 --- a/internal/controller/appserver/assets_test.go +++ b/internal/controller/appserver/assets_test.go @@ -1661,6 +1661,19 @@ var _ = Describe("App server assets", func() { Expect(secret.Type).To(Equal(corev1.SecretTypeServiceAccountToken)) }) + It("should generate the metrics reader ClusterRoleBinding", func() { + rb, err := generateMetricsReaderClusterRoleBinding(testReconcilerInstance, cr) + Expect(err).NotTo(HaveOccurred()) + Expect(rb.Name).To(Equal(utils.MetricsReaderClusterRoleBindingName)) + Expect(rb.Labels).To(Equal(utils.GenerateAppServerSelectorLabels())) + Expect(rb.RoleRef.Kind).To(Equal("ClusterRole")) + Expect(rb.RoleRef.Name).To(Equal(utils.MetricsReaderClusterRoleName)) + Expect(rb.Subjects).To(HaveLen(1)) + Expect(rb.Subjects[0].Kind).To(Equal("ServiceAccount")) + Expect(rb.Subjects[0].Name).To(Equal(utils.MetricsReaderServiceAccountName)) + Expect(rb.Subjects[0].Namespace).To(Equal(utils.OLSNamespaceDefault)) + }) + It("should generate the OLS prometheus rules", func() { prometheusRule, err := GeneratePrometheusRule(testReconcilerInstance, cr) Expect(err).NotTo(HaveOccurred()) diff --git a/internal/controller/appserver/reconciler.go b/internal/controller/appserver/reconciler.go index 17ab31117..89bedaf9b 100644 --- a/internal/controller/appserver/reconciler.go +++ b/internal/controller/appserver/reconciler.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "os" + "reflect" "time" "github.com/openshift/lightspeed-operator/internal/controller/reconciler" @@ -71,6 +72,10 @@ func ReconcileAppServerResources(r reconciler.Reconciler, ctx context.Context, o Name: "reconcile Metrics Reader Secret", Task: reconcileMetricsReaderSecret, }, + { + Name: "reconcile Metrics Reader ClusterRoleBinding", + Task: reconcileMetricsReaderClusterRoleBinding, + }, { Name: "reconcile App NetworkPolicy", Task: reconcileAppServerNetworkPolicy, @@ -405,6 +410,51 @@ func reconcileMetricsReaderSecret(r reconciler.Reconciler, ctx context.Context, return nil } +// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=get;create;update +func reconcileMetricsReaderClusterRoleBinding(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig) error { + if os.Getenv("LOCAL_DEV_MODE") == "true" { + r.GetLogger().Info("Skipping metrics reader ClusterRoleBinding reconciliation in LOCAL_DEV_MODE") + return nil + } + + desired, err := generateMetricsReaderClusterRoleBinding(r, cr) + if err != nil { + return fmt.Errorf("%s: %w", utils.ErrGenerateMetricsReaderCRB, err) + } + + found := &rbacv1.ClusterRoleBinding{} + err = r.Get(ctx, client.ObjectKey{Name: desired.Name}, found) + if err != nil && errors.IsNotFound(err) { + r.GetLogger().Info("creating metrics reader ClusterRoleBinding", "ClusterRoleBinding", desired.Name) + err = r.Create(ctx, desired) + if err != nil { + return fmt.Errorf("%s: %w", utils.ErrCreateMetricsReaderCRB, err) + } + return nil + } else if err != nil { + return fmt.Errorf("%s: %w", utils.ErrGetMetricsReaderCRB, err) + } + + needsUpdate := false + if !reflect.DeepEqual(found.Subjects, desired.Subjects) { + found.Subjects = desired.Subjects + needsUpdate = true + } + + if needsUpdate { + r.GetLogger().Info("updating metrics reader ClusterRoleBinding subject namespace", + "ClusterRoleBinding", found.Name, "namespace", r.GetNamespace()) + err = r.Update(ctx, found) + if err != nil { + return fmt.Errorf("%s: %w", utils.ErrUpdateMetricsReaderCRB, err) + } + } else { + r.GetLogger().Info("metrics reader ClusterRoleBinding reconciled", "ClusterRoleBinding", found.Name) + } + + return nil +} + func reconcileServiceMonitor(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig) error { sm, err := GenerateServiceMonitor(r, cr) if err != nil { diff --git a/internal/controller/appserver/reconciler_test.go b/internal/controller/appserver/reconciler_test.go index c9d4c03d6..5a50ff8fa 100644 --- a/internal/controller/appserver/reconciler_test.go +++ b/internal/controller/appserver/reconciler_test.go @@ -452,6 +452,37 @@ var _ = Describe("App server reconciliator", Ordered, func() { Expect(err).NotTo(HaveOccurred()) }) + It("should create a metrics reader ClusterRoleBinding with correct namespace", func() { + By("Get the metrics reader ClusterRoleBinding") + rb := &rbacv1.ClusterRoleBinding{} + err := k8sClient.Get(ctx, client.ObjectKey{Name: utils.MetricsReaderClusterRoleBindingName}, rb) + Expect(err).NotTo(HaveOccurred()) + Expect(rb.Subjects).To(HaveLen(1)) + Expect(rb.Subjects[0].Name).To(Equal(utils.MetricsReaderServiceAccountName)) + Expect(rb.Subjects[0].Namespace).To(Equal(utils.OLSNamespaceDefault)) + Expect(rb.RoleRef.Name).To(Equal(utils.MetricsReaderClusterRoleName)) + }) + + It("should fix a metrics reader ClusterRoleBinding with wrong namespace", func() { + By("Patch the ClusterRoleBinding to have a wrong namespace") + rb := &rbacv1.ClusterRoleBinding{} + err := k8sClient.Get(ctx, client.ObjectKey{Name: utils.MetricsReaderClusterRoleBindingName}, rb) + Expect(err).NotTo(HaveOccurred()) + + rb.Subjects[0].Namespace = "wrong-namespace" + err = k8sClient.Update(ctx, rb) + Expect(err).NotTo(HaveOccurred()) + + By("Re-reconcile to fix the namespace") + err = ReconcileAppServer(testReconcilerInstance, ctx, cr) + Expect(err).NotTo(HaveOccurred()) + + By("Verify the namespace was corrected") + err = k8sClient.Get(ctx, client.ObjectKey{Name: utils.MetricsReaderClusterRoleBindingName}, rb) + Expect(err).NotTo(HaveOccurred()) + Expect(rb.Subjects[0].Namespace).To(Equal(utils.OLSNamespaceDefault)) + }) + It("should create a prometheus rule", func() { By("Get the prometheus rule") pr := &monv1.PrometheusRule{} diff --git a/internal/controller/utils/constants.go b/internal/controller/utils/constants.go index 536069ca0..a4e6a5d78 100644 --- a/internal/controller/utils/constants.go +++ b/internal/controller/utils/constants.go @@ -454,6 +454,10 @@ ssl_ca_file = '/etc/certs/cm-olspostgresca/service-ca.crt' MetricsReaderServiceAccountTokenSecretName = "metrics-reader-token" // #nosec G101 // MetricsReaderServiceAccountName is the name of the service account for the metrics reader MetricsReaderServiceAccountName = "lightspeed-operator-metrics-reader" + // MetricsReaderClusterRoleName is the name of the ClusterRole granting metrics read access + MetricsReaderClusterRoleName = "lightspeed-operator-ols-metrics-reader" + // MetricsReaderClusterRoleBindingName is the name of the ClusterRoleBinding for the metrics reader + MetricsReaderClusterRoleBindingName = "lightspeed-operator-ols-metrics-reader" // OpenShiftMCPServerHTTPSPort is the standalone Service/container HTTPS port (service-ca TLS). OpenShiftMCPServerHTTPSPort = 8443 // OpenShiftMCPServerDeploymentName is the standalone openshift-mcp-server Deployment. diff --git a/internal/controller/utils/errors.go b/internal/controller/utils/errors.go index 15ff23d07..11bedc2bd 100644 --- a/internal/controller/utils/errors.go +++ b/internal/controller/utils/errors.go @@ -79,6 +79,11 @@ const ( ErrGetSARClusterRoleBinding = "failed to get SAR cluster role binding" ErrGetServiceMonitor = "failed to get ServiceMonitor" ErrGetMetricsReaderSecret = "failed to get metrics reader secret" + ErrGenerateMetricsReaderCRB = "failed to generate metrics reader cluster role binding" + ErrCreateMetricsReaderCRB = "failed to create metrics reader cluster role binding" + ErrGetMetricsReaderCRB = "failed to get metrics reader cluster role binding" + ErrUpdateMetricsReaderCRB = "failed to update metrics reader cluster role binding" + ErrSetMetricsReaderCRBOwnerReference = "failed to set metrics reader cluster role binding owner reference" ErrGetPrometheusRule = "failed to get PrometheusRule" ErrUpdateAPIConfigmap = "failed to update OLS configmap" ErrUpdateAPIDeployment = "failed to update OLS deployment"