From dbf9093b78876b9ca5fdb598b2cc4ccc59e55653 Mon Sep 17 00:00:00 2001 From: Andrei Matveyeu Date: Fri, 15 Nov 2024 15:57:57 +0100 Subject: [PATCH 1/3] add reconcile for suite runner role and role binding --- internal/etos/suitestarter/suitestarter.go | 96 ++++++++++++++++------ 1 file changed, 71 insertions(+), 25 deletions(-) diff --git a/internal/etos/suitestarter/suitestarter.go b/internal/etos/suitestarter/suitestarter.go index 800644b..4498d01 100644 --- a/internal/etos/suitestarter/suitestarter.go +++ b/internal/etos/suitestarter/suitestarter.go @@ -56,26 +56,31 @@ func NewETOSSuiteStarterDeployment(spec etosv1alpha1.ETOSSuiteStarter, scheme *r // Reconcile will reconcile the ETOS suite starter to its expected state. func (r *ETOSSuiteStarterDeployment) Reconcile(ctx context.Context, cluster *etosv1alpha1.Cluster) error { var err error - name := fmt.Sprintf("%s-etos-suite-starter", cluster.Name) - logger := log.FromContext(ctx, "Reconciler", "ETOSSuiteStarter", "BaseName", name) - namespacedName := types.NamespacedName{Name: name, Namespace: cluster.Namespace} - _, err = r.reconcileSuiteRunnerServiceAccount(ctx, logger, namespacedName, cluster) + suiteStarterName := fmt.Sprintf("%s-etos-suite-starter", cluster.Name) + suiteRunnerName := fmt.Sprintf("%s-etos-suite-runner", cluster.Name) + + logger := log.FromContext(ctx, "Reconciler", "ETOSSuiteStarter", "BaseName", suiteStarterName) + + nsSuiteStarterName := types.NamespacedName{Name: suiteStarterName, Namespace: cluster.Namespace} + nsSuiteRunnerName := types.NamespacedName{Name: suiteRunnerName, Namespace: cluster.Namespace} + + _, err = r.reconcileServiceAccount(ctx, logger, nsSuiteRunnerName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the Suite runner service account") return err } // This secret is in use when running the TestRun controller. When the suite starter is removed, this MUST still be created. - secret, err := r.reconcileSuiteRunnerSecret(ctx, logger, namespacedName, cluster) + secret, err := r.reconcileSuiteRunnerSecret(ctx, logger, nsSuiteRunnerName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the Suite runner secret") return err } - cfg, err := r.reconcileConfig(ctx, logger, secret.ObjectMeta.Name, namespacedName, cluster) + cfg, err := r.reconcileConfig(ctx, logger, secret.ObjectMeta.Name, nsSuiteStarterName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the Suite starter config") return err } - template, err := r.reconcileTemplate(ctx, logger, namespacedName, cluster) + template, err := r.reconcileTemplate(ctx, logger, nsSuiteRunnerName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the Suite runner template") return err @@ -87,53 +92,73 @@ func (r *ETOSSuiteStarterDeployment) Reconcile(ctx context.Context, cluster *eto suiteRunnerTemplateName = r.SuiteRunnerTemplateSecretName } logger.Info("Suite runner template", "suiteRunnerTemplateName", suiteRunnerTemplateName) - _, err = r.reconcileDeployment(ctx, logger, cfg.ObjectMeta.Name, suiteRunnerTemplateName, namespacedName, cluster) + _, err = r.reconcileDeployment(ctx, logger, cfg.ObjectMeta.Name, suiteRunnerTemplateName, nsSuiteStarterName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the deployment for the ETOS Suite Starter") return err } - _, err = r.reconcileSecret(ctx, logger, namespacedName, cluster) + _, err = r.reconcileSecret(ctx, logger, nsSuiteStarterName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the secret for the ETOS Suite Starter") return err } - _, err = r.reconcileRole(ctx, logger, namespacedName, cluster) + _, err = r.reconcileRole(ctx, logger, nsSuiteStarterName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the role for the ETOS Suite Starter") return err } - _, err = r.reconcileServiceAccount(ctx, logger, namespacedName, cluster) + _, err = r.reconcileServiceAccount(ctx, logger, nsSuiteStarterName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the service account for the ETOS Suite Starter") return err } - _, err = r.reconcileRolebinding(ctx, logger, namespacedName, cluster) + _, err = r.reconcileRolebinding(ctx, logger, nsSuiteStarterName, "esr-handler", cluster) if err != nil { logger.Error(err, "Failed to reconcile the role binding for the ETOS Suite Starter") return err } + + _, err = r.reconcileSuiteRunnerRole(ctx, logger, nsSuiteRunnerName, cluster) + if err != nil { + logger.Error(err, "Failed to reconcile the Suite Runner role") + return err + } + + _, err = r.reconcileServiceAccount(ctx, logger, nsSuiteRunnerName, cluster) + if err != nil { + logger.Error(err, "Failed to reconcile the Suite Runner service account") + return err + } + + _, err = r.reconcileRolebinding(ctx, logger, nsSuiteRunnerName, "testrun-reader", cluster) + if err != nil { + logger.Error(err, "Failed to reconcile the Suite Runner role binding") + return err + } return err } -// reconcileSuiteRunnerServiceAccount will reconcile the ETOS SuiteStarter service account to its expected state. -func (r *ETOSSuiteStarterDeployment) reconcileSuiteRunnerServiceAccount(ctx context.Context, logger logr.Logger, name types.NamespacedName, owner metav1.Object) (*corev1.ServiceAccount, error) { - target := r.serviceaccount(name) +// reconcileSuiteRunnerRole will reconcile the ETOS SuiteStarter role to its expected state. +func (r *ETOSSuiteStarterDeployment) reconcileSuiteRunnerRole(ctx context.Context, logger logr.Logger, name types.NamespacedName, owner metav1.Object) (*rbacv1.Role, error) { + labelName := name.Name + name.Name = fmt.Sprintf("%s:sa:testrun-reader", name.Name) + + target := r.testReaderRole(name, labelName) if err := ctrl.SetControllerReference(owner, target, r.Scheme); err != nil { return target, err } - serviceaccount := &corev1.ServiceAccount{} - if err := r.Get(ctx, name, serviceaccount); err != nil { + role := &rbacv1.Role{} + if err := r.Get(ctx, name, role); err != nil { if !apierrors.IsNotFound(err) { - return serviceaccount, err + return role, err } - logger.Info("Creating a new service account for the suite runner") if err := r.Create(ctx, target); err != nil { return target, err } return target, nil } - return target, r.Patch(ctx, target, client.StrategicMergeFrom(serviceaccount)) + return target, r.Patch(ctx, target, client.StrategicMergeFrom(role)) } // reconcileSuiteRunnerSecret will reconcile the ETOS suite runner secret to its expected state. @@ -309,8 +334,8 @@ func (r *ETOSSuiteStarterDeployment) reconcileServiceAccount(ctx context.Context } // reconcileRolebinding will reconcile the ETOS SuiteStarter service account role binding to its expected state. -func (r *ETOSSuiteStarterDeployment) reconcileRolebinding(ctx context.Context, logger logr.Logger, name types.NamespacedName, owner metav1.Object) (*rbacv1.RoleBinding, error) { - target := r.rolebinding(name) +func (r *ETOSSuiteStarterDeployment) reconcileRolebinding(ctx context.Context, logger logr.Logger, name types.NamespacedName, roleName string, owner metav1.Object) (*rbacv1.RoleBinding, error) { + target := r.rolebinding(name, roleName) if err := ctrl.SetControllerReference(owner, target, r.Scheme); err != nil { return target, err } @@ -320,7 +345,7 @@ func (r *ETOSSuiteStarterDeployment) reconcileRolebinding(ctx context.Context, l if !apierrors.IsNotFound(err) { return rolebinding, err } - logger.Info("Creating a new role binding for the suite starter") + logger.Info(fmt.Sprintf("Creating a new role binding for the suite starter: %s", roleName)) if err := r.Create(ctx, target); err != nil { return target, err } @@ -440,6 +465,27 @@ func (r *ETOSSuiteStarterDeployment) role(name types.NamespacedName, labelName s } } +// testReader creates a role resource definition giving read permissions for testrun resource +func (r *ETOSSuiteStarterDeployment) testReaderRole(name types.NamespacedName, labelName string) *rbacv1.Role { + meta := r.meta(types.NamespacedName{Name: labelName, Namespace: name.Namespace}) + meta.Name = name.Name + meta.Annotations["rbac.authorization.kubernetes.io/autoupdate"] = "true" + return &rbacv1.Role{ + ObjectMeta: meta, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"etos.eiffel-community.github.io"}, + Resources: []string{ + "testruns", + }, + Verbs: []string{ + "get", "list", "watch", + }, + }, + }, + } +} + // serviceaccount creates a service account resource definition for the ETOS SuiteStarter. func (r *ETOSSuiteStarterDeployment) serviceaccount(name types.NamespacedName) *corev1.ServiceAccount { return &corev1.ServiceAccount{ @@ -448,13 +494,13 @@ func (r *ETOSSuiteStarterDeployment) serviceaccount(name types.NamespacedName) * } // rolebinding creates a rolebinding resource definition for the ETOS SuiteStarter. -func (r *ETOSSuiteStarterDeployment) rolebinding(name types.NamespacedName) *rbacv1.RoleBinding { +func (r *ETOSSuiteStarterDeployment) rolebinding(name types.NamespacedName, roleName string) *rbacv1.RoleBinding { return &rbacv1.RoleBinding{ ObjectMeta: r.meta(name), RoleRef: rbacv1.RoleRef{ APIGroup: rbacv1.SchemeGroupVersion.Group, Kind: "Role", - Name: fmt.Sprintf("%s:sa:esr-handler", name.Name), + Name: fmt.Sprintf("%s:sa:%s", name.Name, roleName), }, Subjects: []rbacv1.Subject{ { From 3f4add70a3dfded986dcbe312233362aff896aac Mon Sep 17 00:00:00 2001 From: Andrei Matveyeu Date: Thu, 28 Nov 2024 12:59:53 +0100 Subject: [PATCH 2/3] code review changes --- internal/etos/suitestarter/suitestarter.go | 42 ++++++++++------------ 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/internal/etos/suitestarter/suitestarter.go b/internal/etos/suitestarter/suitestarter.go index 4498d01..f9a7de9 100644 --- a/internal/etos/suitestarter/suitestarter.go +++ b/internal/etos/suitestarter/suitestarter.go @@ -56,31 +56,27 @@ func NewETOSSuiteStarterDeployment(spec etosv1alpha1.ETOSSuiteStarter, scheme *r // Reconcile will reconcile the ETOS suite starter to its expected state. func (r *ETOSSuiteStarterDeployment) Reconcile(ctx context.Context, cluster *etosv1alpha1.Cluster) error { var err error - suiteStarterName := fmt.Sprintf("%s-etos-suite-starter", cluster.Name) - suiteRunnerName := fmt.Sprintf("%s-etos-suite-runner", cluster.Name) + suiteStarterName := types.NamespacedName{Name: fmt.Sprintf("%s-etos-suite-starter", cluster.Name), Namespace: cluster.Namespace} + suiteRunnerName := types.NamespacedName{Name: fmt.Sprintf("%s-etos-suite-runner", cluster.Name), Namespace: cluster.Namespace} - logger := log.FromContext(ctx, "Reconciler", "ETOSSuiteStarter", "BaseName", suiteStarterName) - - nsSuiteStarterName := types.NamespacedName{Name: suiteStarterName, Namespace: cluster.Namespace} - nsSuiteRunnerName := types.NamespacedName{Name: suiteRunnerName, Namespace: cluster.Namespace} - - _, err = r.reconcileServiceAccount(ctx, logger, nsSuiteRunnerName, cluster) + logger := log.FromContext(ctx, "Reconciler", "ETOSSuiteStarter", "BaseName", suiteStarterName.Name) + _, err = r.reconcileServiceAccount(ctx, logger, suiteRunnerName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the Suite runner service account") return err } // This secret is in use when running the TestRun controller. When the suite starter is removed, this MUST still be created. - secret, err := r.reconcileSuiteRunnerSecret(ctx, logger, nsSuiteRunnerName, cluster) + secret, err := r.reconcileSuiteRunnerSecret(ctx, logger, suiteRunnerName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the Suite runner secret") return err } - cfg, err := r.reconcileConfig(ctx, logger, secret.ObjectMeta.Name, nsSuiteStarterName, cluster) + cfg, err := r.reconcileConfig(ctx, logger, secret.ObjectMeta.Name, suiteStarterName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the Suite starter config") return err } - template, err := r.reconcileTemplate(ctx, logger, nsSuiteRunnerName, cluster) + template, err := r.reconcileTemplate(ctx, logger, suiteRunnerName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the Suite runner template") return err @@ -92,45 +88,45 @@ func (r *ETOSSuiteStarterDeployment) Reconcile(ctx context.Context, cluster *eto suiteRunnerTemplateName = r.SuiteRunnerTemplateSecretName } logger.Info("Suite runner template", "suiteRunnerTemplateName", suiteRunnerTemplateName) - _, err = r.reconcileDeployment(ctx, logger, cfg.ObjectMeta.Name, suiteRunnerTemplateName, nsSuiteStarterName, cluster) + _, err = r.reconcileDeployment(ctx, logger, cfg.ObjectMeta.Name, suiteRunnerTemplateName, suiteStarterName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the deployment for the ETOS Suite Starter") return err } - _, err = r.reconcileSecret(ctx, logger, nsSuiteStarterName, cluster) + _, err = r.reconcileSecret(ctx, logger, suiteStarterName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the secret for the ETOS Suite Starter") return err } - _, err = r.reconcileRole(ctx, logger, nsSuiteStarterName, cluster) + _, err = r.reconcileRole(ctx, logger, suiteStarterName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the role for the ETOS Suite Starter") return err } - _, err = r.reconcileServiceAccount(ctx, logger, nsSuiteStarterName, cluster) + _, err = r.reconcileServiceAccount(ctx, logger, suiteStarterName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the service account for the ETOS Suite Starter") return err } - _, err = r.reconcileRolebinding(ctx, logger, nsSuiteStarterName, "esr-handler", cluster) + _, err = r.reconcileRolebinding(ctx, logger, suiteStarterName, "esr-handler", cluster) if err != nil { logger.Error(err, "Failed to reconcile the role binding for the ETOS Suite Starter") return err } - _, err = r.reconcileSuiteRunnerRole(ctx, logger, nsSuiteRunnerName, cluster) + _, err = r.reconcileSuiteRunnerRole(ctx, logger, suiteRunnerName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the Suite Runner role") return err } - _, err = r.reconcileServiceAccount(ctx, logger, nsSuiteRunnerName, cluster) + _, err = r.reconcileServiceAccount(ctx, logger, suiteRunnerName, cluster) if err != nil { logger.Error(err, "Failed to reconcile the Suite Runner service account") return err } - _, err = r.reconcileRolebinding(ctx, logger, nsSuiteRunnerName, "testrun-reader", cluster) + _, err = r.reconcileRolebinding(ctx, logger, suiteRunnerName, "testrun-reader", cluster) if err != nil { logger.Error(err, "Failed to reconcile the Suite Runner role binding") return err @@ -138,12 +134,12 @@ func (r *ETOSSuiteStarterDeployment) Reconcile(ctx context.Context, cluster *eto return err } -// reconcileSuiteRunnerRole will reconcile the ETOS SuiteStarter role to its expected state. +// reconcileSuiteRunnerRole will reconcile the ETOS Suite Runner role to its expected state. func (r *ETOSSuiteStarterDeployment) reconcileSuiteRunnerRole(ctx context.Context, logger logr.Logger, name types.NamespacedName, owner metav1.Object) (*rbacv1.Role, error) { labelName := name.Name name.Name = fmt.Sprintf("%s:sa:testrun-reader", name.Name) - target := r.testReaderRole(name, labelName) + target := r.testRunReaderRole(name, labelName) if err := ctrl.SetControllerReference(owner, target, r.Scheme); err != nil { return target, err } @@ -465,8 +461,8 @@ func (r *ETOSSuiteStarterDeployment) role(name types.NamespacedName, labelName s } } -// testReader creates a role resource definition giving read permissions for testrun resource -func (r *ETOSSuiteStarterDeployment) testReaderRole(name types.NamespacedName, labelName string) *rbacv1.Role { +// testRunReader creates a role resource definition giving read permissions for testrun resource +func (r *ETOSSuiteStarterDeployment) testRunReaderRole(name types.NamespacedName, labelName string) *rbacv1.Role { meta := r.meta(types.NamespacedName{Name: labelName, Namespace: name.Namespace}) meta.Name = name.Name meta.Annotations["rbac.authorization.kubernetes.io/autoupdate"] = "true" From 115eecc220ff3c96938147fd715fef950b6d072b Mon Sep 17 00:00:00 2001 From: Andrei Matveyeu Date: Thu, 28 Nov 2024 13:23:16 +0100 Subject: [PATCH 3/3] logger.Info() fix in reconcileRolebinding --- internal/etos/suitestarter/suitestarter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/etos/suitestarter/suitestarter.go b/internal/etos/suitestarter/suitestarter.go index f9a7de9..e679670 100644 --- a/internal/etos/suitestarter/suitestarter.go +++ b/internal/etos/suitestarter/suitestarter.go @@ -341,7 +341,7 @@ func (r *ETOSSuiteStarterDeployment) reconcileRolebinding(ctx context.Context, l if !apierrors.IsNotFound(err) { return rolebinding, err } - logger.Info(fmt.Sprintf("Creating a new role binding for the suite starter: %s", roleName)) + logger.Info("Creating a new role binding for the suite starter", "roleName", roleName) if err := r.Create(ctx, target); err != nil { return target, err }