From 5d00cdb1f578fb5b171a0e1e40eff7466c746bdc Mon Sep 17 00:00:00 2001 From: Rizwana777 Date: Tue, 23 Jul 2024 17:35:26 +0530 Subject: [PATCH 1/2] Fix '.spec.additionalMetadata' field issue Signed-off-by: Rizwana777 --- controllers/resources.go | 397 ++++++++++++++++++++++++---------- controllers/resources_test.go | 373 +++++++++++++++++++++++++++++++- controllers/utils.go | 43 ++++ controllers/utils_test.go | 73 +++++++ 4 files changed, 764 insertions(+), 122 deletions(-) diff --git a/controllers/resources.go b/controllers/resources.go index 0c40293..09604b2 100644 --- a/controllers/resources.go +++ b/controllers/resources.go @@ -19,101 +19,151 @@ import ( // Reconciles Rollouts ServiceAccount. func (r *RolloutManagerReconciler) reconcileRolloutsServiceAccount(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) (*corev1.ServiceAccount, error) { - - sa := &corev1.ServiceAccount{ + expectedServiceAccount := &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: DefaultArgoRolloutsResourceName, Namespace: cr.Namespace, }, } - setRolloutsLabelsAndAnnotationsToObject(&sa.ObjectMeta, cr) + setRolloutsLabelsAndAnnotationsToObject(&expectedServiceAccount.ObjectMeta, cr) - if err := fetchObject(ctx, r.Client, cr.Namespace, sa.Name, sa); err != nil { + liveServiceAccount := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: expectedServiceAccount.Name, Namespace: expectedServiceAccount.Namespace}} + if err := fetchObject(ctx, r.Client, cr.Namespace, liveServiceAccount.Name, liveServiceAccount); err != nil { if !apierrors.IsNotFound(err) { - return nil, fmt.Errorf("failed to get the ServiceAccount associated with %s: %w", sa.Name, err) + return nil, fmt.Errorf("failed to get the ServiceAccount associated with %s: %w", liveServiceAccount.Name, err) } - if err := controllerutil.SetControllerReference(&cr, sa, r.Scheme); err != nil { + if err := controllerutil.SetControllerReference(&cr, expectedServiceAccount, r.Scheme); err != nil { return nil, err } - log.Info(fmt.Sprintf("Creating ServiceAccount %s", sa.Name)) - err := r.Client.Create(ctx, sa) - if err != nil { - return nil, err - } + log.Info(fmt.Sprintf("Creating ServiceAccount %s", expectedServiceAccount.Name)) + return expectedServiceAccount, r.Client.Create(ctx, expectedServiceAccount) + } + + updateNeeded := false + normalizedLiveServiceAccount := liveServiceAccount.DeepCopy() + removeUserLabelsAndAnnotations(&normalizedLiveServiceAccount.ObjectMeta, cr) + + if !reflect.DeepEqual(normalizedLiveServiceAccount.Labels, expectedServiceAccount.Labels) || !reflect.DeepEqual(normalizedLiveServiceAccount.Annotations, expectedServiceAccount.Annotations) { + updateNeeded = true + log.Info(fmt.Sprintf("Labels/Annotations of ServiceAccount %s do not match the expected state, hence updating it", liveServiceAccount.Name)) + + liveServiceAccount.Labels = combineStringMaps(liveServiceAccount.Labels, expectedServiceAccount.Labels) + liveServiceAccount.Annotations = combineStringMaps(liveServiceAccount.Annotations, expectedServiceAccount.Annotations) } - return sa, nil + + if updateNeeded { + // Update if the Role already exists and needs to be modified + return liveServiceAccount, r.Client.Update(ctx, liveServiceAccount) + } + + return liveServiceAccount, nil } // Reconciles Rollouts Role. func (r *RolloutManagerReconciler) reconcileRolloutsRole(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) (*rbacv1.Role, error) { - expectedPolicyRules := GetPolicyRules() - role := &rbacv1.Role{ + expectedRole := &rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ Name: DefaultArgoRolloutsResourceName, Namespace: cr.Namespace, }, } - setRolloutsLabelsAndAnnotationsToObject(&role.ObjectMeta, cr) + setRolloutsLabelsAndAnnotationsToObject(&expectedRole.ObjectMeta, cr) + + liveRole := &rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Name: expectedRole.Name, Namespace: expectedRole.Namespace}} - if err := fetchObject(ctx, r.Client, cr.Namespace, role.Name, role); err != nil { + if err := fetchObject(ctx, r.Client, cr.Namespace, liveRole.Name, liveRole); err != nil { if !apierrors.IsNotFound(err) { - return nil, fmt.Errorf("failed to reconcile the Role for the ServiceAccount associated with %s: %w", role.Name, err) + return nil, fmt.Errorf("failed to reconcile the Role for the ServiceAccount associated with %s: %w", liveRole.Name, err) } - if err = controllerutil.SetControllerReference(&cr, role, r.Scheme); err != nil { + if err = controllerutil.SetControllerReference(&cr, expectedRole, r.Scheme); err != nil { return nil, err } - log.Info(fmt.Sprintf("Creating Role %s", role.Name)) - role.Rules = expectedPolicyRules - return role, r.Client.Create(ctx, role) + log.Info(fmt.Sprintf("Creating Role %s", expectedRole.Name)) + expectedRole.Rules = expectedPolicyRules + return expectedRole, r.Client.Create(ctx, expectedRole) + } + + updateNeeded := false + + if !reflect.DeepEqual(liveRole.Rules, expectedPolicyRules) { + updateNeeded = true + + log.Info(fmt.Sprintf("PolicyRules of Role %s do not match the expected state, hence updating it", liveRole.Name)) + liveRole.Rules = expectedPolicyRules + } + + normalizedLiveRole := liveRole.DeepCopy() + + removeUserLabelsAndAnnotations(&normalizedLiveRole.ObjectMeta, cr) + + if !reflect.DeepEqual(normalizedLiveRole.Labels, expectedRole.Labels) || !reflect.DeepEqual(normalizedLiveRole.Annotations, expectedRole.Annotations) { + updateNeeded = true + log.Info(fmt.Sprintf("Labels/Annotations of Role %s do not match the expected state, hence updating it", liveRole.Name)) + + liveRole.Labels = combineStringMaps(liveRole.Labels, expectedRole.Labels) + liveRole.Annotations = combineStringMaps(liveRole.Annotations, expectedRole.Annotations) } - // Reconcile if the Role already exists and modified. - if !reflect.DeepEqual(role.Rules, expectedPolicyRules) { - log.Info(fmt.Sprintf("PolicyRules of Role %s do not match the expected state, hence updating it", role.Name)) - role.Rules = expectedPolicyRules - return role, r.Client.Update(ctx, role) + if updateNeeded { + // Update if the Role already exists and needs to be modified + return liveRole, r.Client.Update(ctx, liveRole) } - return role, nil + return liveRole, nil } // Reconciles Rollouts ClusterRole. func (r *RolloutManagerReconciler) reconcileRolloutsClusterRole(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) (*rbacv1.ClusterRole, error) { - expectedPolicyRules := GetPolicyRules() - clusterRole := &rbacv1.ClusterRole{ + expectedClusterRole := &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: DefaultArgoRolloutsResourceName, }, } - setRolloutsLabelsAndAnnotationsToObject(&clusterRole.ObjectMeta, cr) - - if err := fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole); err != nil { + setRolloutsLabelsAndAnnotationsToObject(&expectedClusterRole.ObjectMeta, cr) + liveClusterRole := &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: expectedClusterRole.Name, Namespace: expectedClusterRole.Namespace}} + if err := fetchObject(ctx, r.Client, "", liveClusterRole.Name, liveClusterRole); err != nil { if !apierrors.IsNotFound(err) { - return nil, fmt.Errorf("failed to Reconcile the ClusterRole for the ServiceAccount associated with %s: %w", clusterRole.Name, err) + return nil, fmt.Errorf("failed to Reconcile the ClusterRole for the ServiceAccount associated with %s: %w", liveClusterRole.Name, err) } - log.Info(fmt.Sprintf("Creating ClusterRole %s", clusterRole.Name)) - clusterRole.Rules = expectedPolicyRules - return clusterRole, r.Client.Create(ctx, clusterRole) + log.Info(fmt.Sprintf("Creating ClusterRole %s", liveClusterRole.Name)) + expectedClusterRole.Rules = expectedPolicyRules + return expectedClusterRole, r.Client.Create(ctx, expectedClusterRole) + } + + updateNeeded := false + + if !reflect.DeepEqual(liveClusterRole.Rules, expectedPolicyRules) { + updateNeeded = true + log.Info(fmt.Sprintf("PolicyRules of ClusterRole %s do not match the expected state, hence updating it", liveClusterRole.Name)) + liveClusterRole.Rules = expectedPolicyRules } - // Reconcile if the ClusterRole already exists and modified. - if !reflect.DeepEqual(clusterRole.Rules, expectedPolicyRules) { - log.Info(fmt.Sprintf("PolicyRules of ClusterRole %s do not match the expected state, hence updating it", clusterRole.Name)) - clusterRole.Rules = expectedPolicyRules - return clusterRole, r.Client.Update(ctx, clusterRole) + normalizedLiveClusterRole := liveClusterRole.DeepCopy() + removeUserLabelsAndAnnotations(&normalizedLiveClusterRole.ObjectMeta, cr) + + if !reflect.DeepEqual(normalizedLiveClusterRole.Labels, expectedClusterRole.Labels) || !reflect.DeepEqual(normalizedLiveClusterRole.Annotations, expectedClusterRole.Annotations) { + updateNeeded = true + log.Info(fmt.Sprintf("Labels/Annotations of Role %s do not match the expected state, hence updating it", liveClusterRole.Name)) + + liveClusterRole.Labels = combineStringMaps(liveClusterRole.Labels, expectedClusterRole.Labels) + liveClusterRole.Annotations = combineStringMaps(liveClusterRole.Annotations, expectedClusterRole.Annotations) } - return clusterRole, nil + if updateNeeded { + // Update if the ClusterRole already exists and needs to be modified + return liveClusterRole, r.Client.Update(ctx, liveClusterRole) + } + return liveClusterRole, nil } // Reconcile Rollouts RoleBinding. @@ -149,10 +199,9 @@ func (r *RolloutManagerReconciler) reconcileRolloutsRoleBinding(ctx context.Cont }, } - actualRoleBinding := &rbacv1.RoleBinding{} - // Fetch the RoleBinding if exists and store that in actualRoleBinding. - if err := fetchObject(ctx, r.Client, cr.Namespace, expectedRoleBinding.Name, actualRoleBinding); err != nil { + liveRoleBinding := &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: expectedRoleBinding.Name, Namespace: expectedRoleBinding.Namespace}} + if err := fetchObject(ctx, r.Client, cr.Namespace, liveRoleBinding.Name, liveRoleBinding); err != nil { if !apierrors.IsNotFound(err) { return fmt.Errorf("failed to get the RoleBinding associated with %s: %w", expectedRoleBinding.Name, err) } @@ -165,11 +214,29 @@ func (r *RolloutManagerReconciler) reconcileRolloutsRoleBinding(ctx context.Cont return r.Client.Create(ctx, expectedRoleBinding) } + updateNeeded := false + // Reconcile if the RoleBinding already exists and modified. - if !reflect.DeepEqual(expectedRoleBinding.Subjects, actualRoleBinding.Subjects) { - log.Info(fmt.Sprintf("Subjects of RoleBinding %s do not match the expected state, hence updating it", actualRoleBinding.Name)) - actualRoleBinding.Subjects = expectedRoleBinding.Subjects - if err := r.Client.Update(ctx, actualRoleBinding); err != nil { + if !reflect.DeepEqual(expectedRoleBinding.Subjects, liveRoleBinding.Subjects) { + updateNeeded = true + log.Info(fmt.Sprintf("Subjects of RoleBinding %s do not match the expected state, hence updating it", liveRoleBinding.Name)) + liveRoleBinding.Subjects = expectedRoleBinding.Subjects + + } + + normalizedLiveRoleBinding := liveRoleBinding.DeepCopy() + removeUserLabelsAndAnnotations(&normalizedLiveRoleBinding.ObjectMeta, cr) + if !reflect.DeepEqual(normalizedLiveRoleBinding.Labels, expectedRoleBinding.Labels) || !reflect.DeepEqual(normalizedLiveRoleBinding.Annotations, expectedRoleBinding.Annotations) { + updateNeeded = true + log.Info(fmt.Sprintf("Labels/Annotations of RoleBinding %s do not match the expected state, hence updating it", liveRoleBinding.Name)) + + liveRoleBinding.Labels = combineStringMaps(liveRoleBinding.Labels, expectedRoleBinding.Labels) + liveRoleBinding.Annotations = combineStringMaps(liveRoleBinding.Annotations, expectedRoleBinding.Annotations) + } + + if updateNeeded { + // Update if the RoleBinding already exists and needs to be modified + if err := r.Client.Update(ctx, liveRoleBinding); err != nil { return err } } @@ -209,10 +276,9 @@ func (r *RolloutManagerReconciler) reconcileRolloutsClusterRoleBinding(ctx conte }, } - actualClusterRoleBinding := &rbacv1.ClusterRoleBinding{} - // Fetch the ClusterRoleBinding if exists and store that in actualClusterRoleBinding. - if err := fetchObject(ctx, r.Client, "", expectedClusterRoleBinding.Name, actualClusterRoleBinding); err != nil { + liveClusterRoleBinding := &rbacv1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: expectedClusterRoleBinding.Name}} + if err := fetchObject(ctx, r.Client, "", liveClusterRoleBinding.Name, liveClusterRoleBinding); err != nil { if !apierrors.IsNotFound(err) { return fmt.Errorf("failed to get the ClusterRoleBinding associated with %s: %w", expectedClusterRoleBinding.Name, err) } @@ -221,11 +287,27 @@ func (r *RolloutManagerReconciler) reconcileRolloutsClusterRoleBinding(ctx conte return r.Client.Create(ctx, expectedClusterRoleBinding) } - // Reconcile if the ClusterRoleBinding already exists and modified. - if !reflect.DeepEqual(expectedClusterRoleBinding.Subjects, actualClusterRoleBinding.Subjects) { + updateNeeded := false + + if !reflect.DeepEqual(expectedClusterRoleBinding.Subjects, liveClusterRoleBinding.Subjects) { + updateNeeded = true log.Info(fmt.Sprintf("Subjects of ClusterRoleBinding %s do not match the expected state, hence updating it", expectedClusterRoleBinding.Name)) - actualClusterRoleBinding.Subjects = expectedClusterRoleBinding.Subjects - if err := r.Client.Update(ctx, actualClusterRoleBinding); err != nil { + liveClusterRoleBinding.Subjects = expectedClusterRoleBinding.Subjects + } + + normalizedLiveClusterRoleBinding := liveClusterRoleBinding.DeepCopy() + removeUserLabelsAndAnnotations(&normalizedLiveClusterRoleBinding.ObjectMeta, cr) + if !reflect.DeepEqual(normalizedLiveClusterRoleBinding.Labels, expectedClusterRoleBinding.Labels) || !reflect.DeepEqual(normalizedLiveClusterRoleBinding.Annotations, expectedClusterRoleBinding.Annotations) { + updateNeeded = true + log.Info(fmt.Sprintf("Labels/Annotations of ClusterRoleBinding %s do not match the expected state, hence updating it", liveClusterRoleBinding.Name)) + + liveClusterRoleBinding.Labels = combineStringMaps(liveClusterRoleBinding.Labels, expectedClusterRoleBinding.Labels) + liveClusterRoleBinding.Annotations = combineStringMaps(liveClusterRoleBinding.Annotations, expectedClusterRoleBinding.Annotations) + } + + if updateNeeded { + // Update if the ClusterRoleBinding already exists and needs to be modified + if err := r.Client.Update(ctx, liveClusterRoleBinding); err != nil { return err } } @@ -289,31 +371,47 @@ func (r *RolloutManagerReconciler) reconcileRolloutsAggregateToAdminClusterRole( expectedPolicyRules := GetAggregateToAdminPolicyRules() - clusterRole := &rbacv1.ClusterRole{ + expectedClusterRole := &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: name, }, } - setRolloutsAggregatedClusterRoleLabels(&clusterRole.ObjectMeta, name, aggregationType) - setAdditionalRolloutsLabelsAndAnnotationsToObject(&clusterRole.ObjectMeta, cr) + setRolloutsAggregatedClusterRoleLabels(&expectedClusterRole.ObjectMeta, name, aggregationType) + setAdditionalRolloutsLabelsAndAnnotationsToObject(&expectedClusterRole.ObjectMeta, cr) - if err := fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole); err != nil { + liveClusterRole := &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: expectedClusterRole.Name}} + if err := fetchObject(ctx, r.Client, "", liveClusterRole.Name, liveClusterRole); err != nil { if !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to reconcile the aggregated ClusterRole %s: %w", clusterRole.Name, err) + return fmt.Errorf("failed to reconcile the aggregated ClusterRole %s: %w", liveClusterRole.Name, err) } - log.Info(fmt.Sprintf("Creating aggregated ClusterRole %s", clusterRole.Name)) - clusterRole.Rules = expectedPolicyRules - return r.Client.Create(ctx, clusterRole) + log.Info(fmt.Sprintf("Creating aggregated ClusterRole %s", liveClusterRole.Name)) + expectedClusterRole.Rules = expectedPolicyRules + return r.Client.Create(ctx, expectedClusterRole) } - // Reconcile if the aggregated CusterRole already exists and modified. - if !reflect.DeepEqual(clusterRole.Rules, expectedPolicyRules) { - log.Info(fmt.Sprintf("PolicyRules of ClusterRole %s do not match the expected state, hence updating it", clusterRole.Name)) - clusterRole.Rules = expectedPolicyRules - return r.Client.Update(ctx, clusterRole) + updateNeeded := false + + if !reflect.DeepEqual(liveClusterRole.Rules, expectedPolicyRules) { + updateNeeded = true + log.Info(fmt.Sprintf("PolicyRules of ClusterRole %s do not match the expected state, hence updating it", liveClusterRole.Name)) + liveClusterRole.Rules = expectedPolicyRules + } + + normalizedLiveClusterRole := liveClusterRole.DeepCopy() + removeUserLabelsAndAnnotations(&normalizedLiveClusterRole.ObjectMeta, cr) + if !reflect.DeepEqual(normalizedLiveClusterRole.Labels, expectedClusterRole.Labels) || !reflect.DeepEqual(normalizedLiveClusterRole.Annotations, expectedClusterRole.Annotations) { + updateNeeded = true + log.Info(fmt.Sprintf("Labels/Annotations of aggregated ClusterRole %s do not match the expected state, hence updating it", liveClusterRole.Name)) + + liveClusterRole.Labels = combineStringMaps(liveClusterRole.Labels, expectedClusterRole.Labels) + liveClusterRole.Annotations = combineStringMaps(liveClusterRole.Annotations, expectedClusterRole.Annotations) } + if updateNeeded { + // Update if the aggregated ClusterRole already exists and needs to be modified + return r.Client.Update(ctx, liveClusterRole) + } return nil } @@ -325,31 +423,47 @@ func (r *RolloutManagerReconciler) reconcileRolloutsAggregateToEditClusterRole(c expectedPolicyRules := GetAggregateToEditPolicyRules() - clusterRole := &rbacv1.ClusterRole{ + expectedClusterRole := &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: name, }, } - setRolloutsAggregatedClusterRoleLabels(&clusterRole.ObjectMeta, name, aggregationType) - setAdditionalRolloutsLabelsAndAnnotationsToObject(&clusterRole.ObjectMeta, cr) + setRolloutsAggregatedClusterRoleLabels(&expectedClusterRole.ObjectMeta, name, aggregationType) + setAdditionalRolloutsLabelsAndAnnotationsToObject(&expectedClusterRole.ObjectMeta, cr) - if err := fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole); err != nil { + liveClusterRole := &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: expectedClusterRole.Name}} + if err := fetchObject(ctx, r.Client, "", liveClusterRole.Name, liveClusterRole); err != nil { if !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to reconcile the aggregated ClusterRole %s: %w", clusterRole.Name, err) + return fmt.Errorf("failed to reconcile the aggregated ClusterRole %s: %w", liveClusterRole.Name, err) } - log.Info(fmt.Sprintf("Creating aggregated ClusterRole %s", clusterRole.Name)) - clusterRole.Rules = expectedPolicyRules - return r.Client.Create(ctx, clusterRole) + log.Info(fmt.Sprintf("Creating aggregated ClusterRole %s", expectedClusterRole.Name)) + expectedClusterRole.Rules = expectedPolicyRules + return r.Client.Create(ctx, expectedClusterRole) + } + + updateNeeded := false + + if !reflect.DeepEqual(liveClusterRole.Rules, expectedPolicyRules) { + updateNeeded = true + log.Info(fmt.Sprintf("PolicyRules of ClusterRole %s do not match the expected state, hence updating it", liveClusterRole.Name)) + liveClusterRole.Rules = expectedPolicyRules } - // Reconcile if the aggregated ClusterRole already exists and modified. - if !reflect.DeepEqual(clusterRole.Rules, expectedPolicyRules) { - log.Info(fmt.Sprintf("PolicyRules of ClusterRole %s do not match the expected state, hence updating it", clusterRole.Name)) - clusterRole.Rules = expectedPolicyRules - return r.Client.Update(ctx, clusterRole) + normalizedLiveClusterRole := liveClusterRole.DeepCopy() + removeUserLabelsAndAnnotations(&normalizedLiveClusterRole.ObjectMeta, cr) + if !reflect.DeepEqual(normalizedLiveClusterRole.Labels, expectedClusterRole.Labels) || !reflect.DeepEqual(normalizedLiveClusterRole.Annotations, expectedClusterRole.Annotations) { + updateNeeded = true + log.Info(fmt.Sprintf("Labels/Annotations of aggregated ClusterRole %s do not match the expected state, hence updating it", liveClusterRole.Name)) + + liveClusterRole.Labels = combineStringMaps(liveClusterRole.Labels, expectedClusterRole.Labels) + liveClusterRole.Annotations = combineStringMaps(liveClusterRole.Annotations, expectedClusterRole.Annotations) } + if updateNeeded { + // Update if the aggregated ClusterRole already exists and needs to be modified + return r.Client.Update(ctx, liveClusterRole) + } return nil } @@ -361,29 +475,46 @@ func (r *RolloutManagerReconciler) reconcileRolloutsAggregateToViewClusterRole(c expectedPolicyRules := GetAggregateToViewPolicyRules() - clusterRole := &rbacv1.ClusterRole{ + expectedClusterRole := &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: name, }, } - setRolloutsAggregatedClusterRoleLabels(&clusterRole.ObjectMeta, name, aggregationType) - setAdditionalRolloutsLabelsAndAnnotationsToObject(&clusterRole.ObjectMeta, cr) + setRolloutsAggregatedClusterRoleLabels(&expectedClusterRole.ObjectMeta, name, aggregationType) + setAdditionalRolloutsLabelsAndAnnotationsToObject(&expectedClusterRole.ObjectMeta, cr) - if err := fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole); err != nil { + liveClusterRole := &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: expectedClusterRole.Name, Namespace: expectedClusterRole.Namespace}} + if err := fetchObject(ctx, r.Client, "", liveClusterRole.Name, liveClusterRole); err != nil { if !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to reconcile the aggregated ClusterRole %s: %w", clusterRole.Name, err) + return fmt.Errorf("failed to reconcile the aggregated ClusterRole %s: %w", liveClusterRole.Name, err) } - log.Info(fmt.Sprintf("Creating aggregated ClusterRole %s", clusterRole.Name)) - clusterRole.Rules = expectedPolicyRules - return r.Client.Create(ctx, clusterRole) + log.Info(fmt.Sprintf("Creating aggregated ClusterRole %s", expectedClusterRole.Name)) + expectedClusterRole.Rules = expectedPolicyRules + return r.Client.Create(ctx, expectedClusterRole) + } + + updateNeeded := false + + if !reflect.DeepEqual(liveClusterRole.Rules, expectedPolicyRules) { + updateNeeded = true + log.Info(fmt.Sprintf("PolicyRules of ClusterRole %s do not match the expected state, hence updating it", liveClusterRole.Name)) + liveClusterRole.Rules = expectedPolicyRules } - // Reconcile if the aggregated ClusterRole already exists and modified. - if !reflect.DeepEqual(clusterRole.Rules, expectedPolicyRules) { - log.Info(fmt.Sprintf("PolicyRules of ClusterRole %s do not match the expected state, hence updating it", clusterRole.Name)) - clusterRole.Rules = expectedPolicyRules - return r.Client.Update(ctx, clusterRole) + normalizedLiveClusterRole := liveClusterRole.DeepCopy() + removeUserLabelsAndAnnotations(&normalizedLiveClusterRole.ObjectMeta, cr) + if !reflect.DeepEqual(normalizedLiveClusterRole.Labels, expectedClusterRole.Labels) || !reflect.DeepEqual(normalizedLiveClusterRole.Annotations, expectedClusterRole.Annotations) { + updateNeeded = true + log.Info(fmt.Sprintf("Labels/Annotations of aggregated ClusterRole %s do not match the expected state, hence updating it", liveClusterRole.Name)) + + liveClusterRole.Labels = combineStringMaps(liveClusterRole.Labels, expectedClusterRole.Labels) + liveClusterRole.Annotations = combineStringMaps(liveClusterRole.Annotations, expectedClusterRole.Annotations) + } + + if updateNeeded { + // Update if the aggregated ClusterRole already exists and needs to be modified + return r.Client.Update(ctx, liveClusterRole) } return nil @@ -416,10 +547,8 @@ func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.C DefaultRolloutsSelectorKey: DefaultArgoRolloutsResourceName, } - actualSvc := &corev1.Service{} - - // Fetch the Service if exists and store that in actualSvc. - if err := fetchObject(ctx, r.Client, cr.Namespace, expectedSvc.Name, actualSvc); err != nil { + liveService := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: expectedSvc.Name, Namespace: expectedSvc.Namespace}} + if err := fetchObject(ctx, r.Client, cr.Namespace, liveService.Name, liveService); err != nil { if !apierrors.IsNotFound(err) { return fmt.Errorf("failed to get the Service %s: %w", expectedSvc.Name, err) } @@ -433,13 +562,32 @@ func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.C log.Error(err, "Error creating Service", "Name", expectedSvc.Name) return err } - actualSvc = expectedSvc + liveService = expectedSvc + + } + + updateNeeded := false + + if !reflect.DeepEqual(liveService.Spec.Ports, expectedSvc.Spec.Ports) { + updateNeeded = true + log.Info(fmt.Sprintf("Ports of metrics Service %s do not match the expected state, hence updating it", liveService.Name)) + liveService.Spec.Ports = expectedSvc.Spec.Ports + } - } else if !reflect.DeepEqual(actualSvc.Spec.Ports, expectedSvc.Spec.Ports) { - log.Info(fmt.Sprintf("Ports of Service %s do not match the expected state, hence updating it", actualSvc.Name)) - actualSvc.Spec.Ports = expectedSvc.Spec.Ports - if err := r.Client.Update(ctx, actualSvc); err != nil { - log.Error(err, "Error updating Ports of Service", "Name", actualSvc.Name) + normalizedLiveService := liveService.DeepCopy() + removeUserLabelsAndAnnotations(&normalizedLiveService.ObjectMeta, cr) + if !reflect.DeepEqual(normalizedLiveService.Labels, expectedSvc.Labels) || !reflect.DeepEqual(normalizedLiveService.Annotations, expectedSvc.Annotations) { + updateNeeded = true + log.Info(fmt.Sprintf("Labels/Annotations of metrics Service %s do not match the expected state, hence updating it", liveService.Name)) + + liveService.Labels = combineStringMaps(liveService.Labels, expectedSvc.Labels) + liveService.Annotations = combineStringMaps(liveService.Annotations, expectedSvc.Annotations) + } + + if updateNeeded { + // Update if the Service already exists and needs to be modified + if err := r.Client.Update(ctx, liveService); err != nil { + log.Error(err, "Error updating Ports of metrics Service", "Name", liveService.Name) return err } } @@ -468,7 +616,7 @@ func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.C return nil } else { - log.Error(err, "Error querying for ServiceMonitor", "Namespace", cr.Namespace, "Name", actualSvc.Name) + log.Error(err, "Error querying for ServiceMonitor", "Namespace", cr.Namespace, "Name", liveService.Name) return err } @@ -477,13 +625,13 @@ func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.C "Namespace", existingServiceMonitor.Namespace, "Name", existingServiceMonitor.Name) // Check if existing ServiceMonitor matches expected content - if !serviceMonitorMatches(existingServiceMonitor, actualSvc.Name) { + if !serviceMonitorMatches(existingServiceMonitor, liveService.Name) { log.Info("Updating existing ServiceMonitor instance", "Namespace", existingServiceMonitor.Namespace, "Name", existingServiceMonitor.Name) // Update ServiceMonitor with expected content existingServiceMonitor.Spec.Selector.MatchLabels = map[string]string{ - "app.kubernetes.io/name": actualSvc.Name, + "app.kubernetes.io/name": liveService.Name, } existingServiceMonitor.Spec.Endpoints = []monitoringv1.Endpoint{ { @@ -504,7 +652,7 @@ func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.C // Reconciles Secrets for Rollouts controller func (r *RolloutManagerReconciler) reconcileRolloutsSecrets(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) error { - secret := &corev1.Secret{ + expectedSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: DefaultRolloutsNotificationSecretName, Namespace: cr.Namespace, @@ -512,19 +660,38 @@ func (r *RolloutManagerReconciler) reconcileRolloutsSecrets(ctx context.Context, Type: corev1.SecretTypeOpaque, } - setRolloutsLabelsAndAnnotationsToObject(&secret.ObjectMeta, cr) + setRolloutsLabelsAndAnnotationsToObject(&expectedSecret.ObjectMeta, cr) - if err := fetchObject(ctx, r.Client, cr.Namespace, secret.Name, secret); err != nil { + liveSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: expectedSecret.Name, Namespace: expectedSecret.Namespace}} + if err := fetchObject(ctx, r.Client, cr.Namespace, liveSecret.Name, liveSecret); err != nil { if !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to get the Secret %s: %w", secret.Name, err) + return fmt.Errorf("failed to get the Secret %s: %w", liveSecret.Name, err) } - if err := controllerutil.SetControllerReference(&cr, secret, r.Scheme); err != nil { + if err := controllerutil.SetControllerReference(&cr, expectedSecret, r.Scheme); err != nil { return err } - log.Info(fmt.Sprintf("Creating Secret %s", secret.Name)) - return r.Client.Create(ctx, secret) + log.Info(fmt.Sprintf("Creating Secret %s", expectedSecret.Name)) + return r.Client.Create(ctx, expectedSecret) + } + + updateNeeded := false + + normalizedLiveSecret := liveSecret.DeepCopy() + removeUserLabelsAndAnnotations(&normalizedLiveSecret.ObjectMeta, cr) + + if !reflect.DeepEqual(normalizedLiveSecret.Labels, expectedSecret.Labels) || !reflect.DeepEqual(normalizedLiveSecret.Annotations, expectedSecret.Annotations) { + updateNeeded = true + log.Info(fmt.Sprintf("Labels/Annotations of Secret %s do not match the expected state, hence updating it", liveSecret.Name)) + + liveSecret.Labels = combineStringMaps(liveSecret.Labels, expectedSecret.Labels) + liveSecret.Annotations = combineStringMaps(liveSecret.Annotations, expectedSecret.Annotations) + } + + if updateNeeded { + // Update if the Secret already exists and needs to be modified + return r.Client.Update(ctx, liveSecret) } // secret found, do nothing diff --git a/controllers/resources_test.go b/controllers/resources_test.go index 46a3174..a7de0b5 100644 --- a/controllers/resources_test.go +++ b/controllers/resources_test.go @@ -29,42 +29,74 @@ var _ = Describe("Resource creation and cleanup tests", func() { BeforeEach(func() { ctx = context.Background() a = *makeTestRolloutManager() + + a.Spec = v1alpha1.RolloutManagerSpec{ + AdditionalMetadata: &v1alpha1.ResourceMetadata{ + Labels: map[string]string{ + "keylabel": "valuelabel", + }, + Annotations: map[string]string{ + "keyannotation": "valueannotation", + }, + }, + } + r = makeTestReconciler(&a) err := createNamespace(r, a.Namespace) Expect(err).ToNot(HaveOccurred()) + }) It("Test for reconcileRolloutsServiceAccount function", func() { - _, err := r.reconcileRolloutsServiceAccount(ctx, a) + sa, err := r.reconcileRolloutsServiceAccount(ctx, a) Expect(err).ToNot(HaveOccurred()) + Expect(sa.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(sa.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) }) It("Test for reconcileRolloutsRole function", func() { role, err := r.reconcileRolloutsRole(ctx, a) Expect(err).ToNot(HaveOccurred()) + Expect(role.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(role.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) By("Modify Rules of Role.") role.Rules[0].Verbs = append(role.Rules[0].Verbs, "test") Expect(r.Client.Update(ctx, role)).To(Succeed()) + By("Modify Labels of RM to verify whether label is updated in Role.") + a.Spec.AdditionalMetadata.Labels["keylabel"] = "keylabel-update" + Expect(r.Client.Update(ctx, &a)).To(Succeed()) + By("Reconciler should revert modifications.") role, err = r.reconcileRolloutsRole(ctx, a) Expect(err).ToNot(HaveOccurred()) Expect(role.Rules).To(Equal(GetPolicyRules())) + Expect(role.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(role.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) }) It("Test for reconcileRolloutsClusterRole function", func() { clusterRole, err := r.reconcileRolloutsClusterRole(ctx, a) Expect(err).ToNot(HaveOccurred()) + Expect(clusterRole.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRole.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) - By("Modify Rules of Role.") + By("Modify Rules of ClusterRole.") clusterRole.Rules[0].Verbs = append(clusterRole.Rules[0].Verbs, "test") Expect(r.Client.Update(ctx, clusterRole)).To(Succeed()) + By("Modify Labels of RM to verify whether label and annotation is updated in ClusterRole.") + a.Spec.AdditionalMetadata.Labels["keylabel"] = "keylabel-update" + a.Spec.AdditionalMetadata.Annotations["keyannotation"] = "keyannotation-update" + Expect(r.Client.Update(ctx, &a)).To(Succeed()) + By("Reconciler should revert modifications.") clusterRole, err = r.reconcileRolloutsClusterRole(ctx, a) Expect(err).ToNot(HaveOccurred()) Expect(clusterRole.Rules).To(Equal(GetPolicyRules())) + Expect(clusterRole.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRole.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) }) It("Test for reconcileRolloutsRoleBinding function", func() { @@ -75,7 +107,6 @@ var _ = Describe("Resource creation and cleanup tests", func() { Expect(r.reconcileRolloutsRoleBinding(ctx, a, role, sa)).To(Succeed()) - By("Modify Subject of RoleBinding.") rb := &rbacv1.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ Name: DefaultArgoRolloutsResourceName, @@ -84,14 +115,26 @@ var _ = Describe("Resource creation and cleanup tests", func() { } Expect(fetchObject(ctx, r.Client, a.Namespace, rb.Name, rb)).To(Succeed()) + By("Verify labels and annotations of RoleBinding are updated.") + Expect(rb.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(rb.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + + By("Modify Subject of RoleBinding.") subTemp := rb.Subjects rb.Subjects = append(rb.Subjects, rbacv1.Subject{Kind: rbacv1.ServiceAccountKind, Name: "test", Namespace: "test"}) Expect(r.Client.Update(ctx, rb)).To(Succeed()) + By("Modify Labels of RM to verify whether label and annotation is updated in RoleBinding(.") + a.Spec.AdditionalMetadata.Labels["keylabel"] = "keylabel-update" + a.Spec.AdditionalMetadata.Annotations["keyannotation"] = "keyannotation-update" + Expect(r.Client.Update(ctx, &a)).To(Succeed()) + By("Reconciler should revert modifications.") Expect(r.reconcileRolloutsRoleBinding(ctx, a, role, sa)).To(Succeed()) Expect(fetchObject(ctx, r.Client, a.Namespace, rb.Name, rb)).To(Succeed()) Expect(rb.Subjects).To(Equal(subTemp)) + Expect(rb.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(rb.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) }) It("Test for reconcileRolloutsClusterRoleBinding function", func() { @@ -102,7 +145,6 @@ var _ = Describe("Resource creation and cleanup tests", func() { Expect(r.reconcileRolloutsClusterRoleBinding(ctx, clusterRole, sa, a)).To(Succeed()) - By("Modify Subject of ClusterRoleBinding.") crb := &rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ Name: DefaultArgoRolloutsResourceName, @@ -110,79 +152,145 @@ var _ = Describe("Resource creation and cleanup tests", func() { } Expect(fetchObject(ctx, r.Client, "", crb.Name, crb)).To(Succeed()) + By("Verify labels and annotations of ClusterRoleBinding are updated.") + Expect(crb.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(crb.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + + By("Modify Subject of ClusterRoleBinding.") subTemp := crb.Subjects crb.Subjects = append(crb.Subjects, rbacv1.Subject{Kind: rbacv1.ServiceAccountKind, Name: "test", Namespace: "test"}) Expect(r.Client.Update(ctx, crb)).To(Succeed()) + By("Modify Labels of RM to verify whether label and annotation is updated in ClusterRoleBinding.") + a.Spec.AdditionalMetadata.Labels["keylabel"] = "keylabel-update" + a.Spec.AdditionalMetadata.Annotations["keyannotation"] = "keyannotation-update" + Expect(r.Client.Update(ctx, &a)).To(Succeed()) + By("Reconciler should revert modifications.") Expect(r.reconcileRolloutsClusterRoleBinding(ctx, clusterRole, sa, a)).To(Succeed()) Expect(fetchObject(ctx, r.Client, "", crb.Name, crb)).To(Succeed()) Expect(crb.Subjects).To(Equal(subTemp)) + Expect(crb.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(crb.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) }) It("Test for reconcileRolloutsAggregateToAdminClusterRole function", func() { Expect(r.reconcileRolloutsAggregateToAdminClusterRole(ctx, a)).To(Succeed()) - By("Modify Rules of ClusterRole.") clusterRole := &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: "argo-rollouts-aggregate-to-admin", }, } Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) + + By("Verify labels and annotations of ClusterRole are updated.") + Expect(clusterRole.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRole.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + + By("Modify Rules of ClusterRole.") clusterRole.Rules[0].Verbs = append(clusterRole.Rules[0].Verbs, "test") Expect(r.Client.Update(ctx, clusterRole)).To(Succeed()) + By("Modify Labels of RM to verify whether label and annotation is updated in ClusterRole.") + a.Spec.AdditionalMetadata.Labels["keylabel"] = "keylabel-update" + a.Spec.AdditionalMetadata.Annotations["keyannotation"] = "keyannotation-update" + Expect(r.Client.Update(ctx, &a)).To(Succeed()) + By("Reconciler should revert modifications.") Expect(r.reconcileRolloutsAggregateToAdminClusterRole(ctx, a)).To(Succeed()) Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) Expect(clusterRole.Rules).To(Equal(GetAggregateToAdminPolicyRules())) + Expect(clusterRole.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRole.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) }) It("Test for reconcileRolloutsAggregateToEditClusterRole function", func() { Expect(r.reconcileRolloutsAggregateToEditClusterRole(ctx, a)).To(Succeed()) - By("Modify Rules of ClusterRole.") clusterRole := &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: "argo-rollouts-aggregate-to-edit", }, } Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) + + By("Verify labels and annotations of ClusterRole are updated.") + Expect(clusterRole.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRole.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + + By("Modify Rules of ClusterRole.") clusterRole.Rules[0].Verbs = append(clusterRole.Rules[0].Verbs, "test") Expect(r.Client.Update(ctx, clusterRole)).To(Succeed()) + By("Modify Labels of RM to verify whether label and annotation is updated in ClusterRole.") + a.Spec.AdditionalMetadata.Labels["keylabel"] = "keylabel-update" + a.Spec.AdditionalMetadata.Annotations["keyannotation"] = "keyannotation-update" + Expect(r.Client.Update(ctx, &a)).To(Succeed()) + By("Reconciler should revert modifications.") Expect(r.reconcileRolloutsAggregateToEditClusterRole(ctx, a)).To(Succeed()) Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) Expect(clusterRole.Rules).To(Equal(GetAggregateToEditPolicyRules())) + Expect(clusterRole.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRole.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) }) It("Test for reconcileRolloutsAggregateToViewClusterRole function", func() { Expect(r.reconcileRolloutsAggregateToViewClusterRole(ctx, a)).To(Succeed()) - By("Modify Rules of ClusterRole.") clusterRole := &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: "argo-rollouts-aggregate-to-view", }, } Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) + + By("Verify labels and annotations of ClusterRole are updated.") + Expect(clusterRole.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRole.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + + By("Modify Rules of ClusterRole.") clusterRole.Rules[0].Verbs = append(clusterRole.Rules[0].Verbs, "test") Expect(r.Client.Update(ctx, clusterRole)).To(Succeed()) + By("Modify Labels of RM to verify whether label and annotation is updated in ClusterRole.") + a.Spec.AdditionalMetadata.Labels["keylabel"] = "keylabel-update" + a.Spec.AdditionalMetadata.Annotations["keyannotation"] = "keyannotation-update" + Expect(r.Client.Update(ctx, &a)).To(Succeed()) + By("Reconciler should revert modifications.") Expect(r.reconcileRolloutsAggregateToViewClusterRole(ctx, a)).To(Succeed()) Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) Expect(clusterRole.Rules).To(Equal(GetAggregateToViewPolicyRules())) + Expect(clusterRole.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRole.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) }) It("Test for reconcileRolloutsMetricsService function", func() { Expect(r.reconcileRolloutsMetricsService(ctx, a)).To(Succeed()) + service := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultArgoRolloutsMetricsServiceName, + Namespace: a.Namespace, + }, + } + Expect(fetchObject(ctx, r.Client, a.Namespace, service.Name, service)).To(Succeed()) + Expect(service.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(service.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) }) It("Test for reconcileRolloutsSecrets function", func() { Expect(r.reconcileRolloutsSecrets(ctx, a)).To(Succeed()) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultRolloutsNotificationSecretName, + Namespace: a.Namespace, + }, + } + Expect(fetchObject(ctx, r.Client, a.Namespace, secret.Name, secret)).To(Succeed()) + Expect(secret.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(secret.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) }) It("test for removeClusterScopedResourcesIfApplicable function", func() { @@ -232,6 +340,228 @@ var _ = Describe("Resource creation and cleanup tests", func() { Expect(r.removeClusterScopedResourcesIfApplicable(ctx)).To(Succeed(), "calling the function again should not return an error") }) + + Context("Verify whether existing labels are not getting removed from resources", func() { + var err error + It("verify for ServiceAccount", func() { + serviceAccount := createServiceAccount(DefaultArgoRolloutsResourceName, a.Namespace, map[string]string{ + "my-label": "my-value", + }) + Expect(r.Client.Create(ctx, serviceAccount)).To(Succeed()) + + serviceAccount, err = r.reconcileRolloutsServiceAccount(ctx, a) + Expect(err).ToNot(HaveOccurred()) + Expect(serviceAccount.ObjectMeta.Labels["my-label"]).ToNot(BeEmpty()) + Expect(serviceAccount.ObjectMeta.Labels["my-label"]).To(Equal("my-value")) + Expect(serviceAccount.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(serviceAccount.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + }) + + It("verify for Role", func() { + role := createRole(DefaultArgoRolloutsResourceName, a.Namespace, map[string]string{ + "my-label": "my-value", + }) + Expect(r.Client.Create(ctx, role)).To(Succeed()) + + role, err = r.reconcileRolloutsRole(ctx, a) + Expect(err).ToNot(HaveOccurred()) + + Expect(role.ObjectMeta.Labels["my-label"]).ToNot(BeEmpty()) + Expect(role.ObjectMeta.Labels["my-label"]).To(Equal("my-value")) + Expect(role.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(role.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + }) + + It("verify for ClusterRole", func() { + clusterRole := createClusterRole(DefaultArgoRolloutsResourceName, map[string]string{ + "my-label": "my-value", + }) + Expect(r.Client.Create(ctx, clusterRole)).To(Succeed()) + + clusterRole, err = r.reconcileRolloutsClusterRole(ctx, a) + Expect(err).ToNot(HaveOccurred()) + + Expect(clusterRole.ObjectMeta.Labels["my-label"]).ToNot(BeEmpty()) + Expect(clusterRole.ObjectMeta.Labels["my-label"]).To(Equal("my-value")) + Expect(clusterRole.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRole.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + }) + It("verify for RoleBinding", func() { + serviceAccount := createServiceAccount(DefaultArgoRolloutsResourceName, a.Namespace, map[string]string{ + "my-label": "my-value", + }) + Expect(r.Client.Create(ctx, serviceAccount)).To(Succeed()) + + role := createRole(DefaultArgoRolloutsResourceName, a.Namespace, map[string]string{ + "my-label": "my-value", + }) + Expect(r.Client.Create(ctx, role)).To(Succeed()) + + rb := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultArgoRolloutsResourceName, + Namespace: a.Namespace, + Labels: map[string]string{ + "my-label": "my-value", + }, + }, + } + Expect(r.Client.Create(ctx, rb)).To(Succeed()) + + err = r.reconcileRolloutsRoleBinding(ctx, a, role, serviceAccount) + Expect(err).ToNot(HaveOccurred()) + + Expect(fetchObject(ctx, r.Client, a.Namespace, rb.Name, rb)).To(Succeed()) + + Expect(rb.ObjectMeta.Labels["my-label"]).ToNot(BeEmpty()) + Expect(rb.ObjectMeta.Labels["my-label"]).To(Equal("my-value")) + Expect(rb.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(rb.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + }) + + It("verify for ClusterRoleBinding", func() { + serviceAccount := createServiceAccount(DefaultArgoRolloutsResourceName, a.Namespace, map[string]string{ + "my-label": "my-value", + }) + Expect(r.Client.Create(ctx, serviceAccount)).To(Succeed()) + + clusterRole := createClusterRole(DefaultArgoRolloutsResourceName, map[string]string{ + "my-label": "my-value", + }) + Expect(r.Client.Create(ctx, clusterRole)).To(Succeed()) + crb := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultArgoRolloutsResourceName, + Labels: map[string]string{ + "my-label": "my-value", + }, + }, + } + Expect(r.Client.Create(ctx, crb)).To(Succeed()) + + err = r.reconcileRolloutsClusterRoleBinding(ctx, clusterRole, serviceAccount, a) + Expect(err).ToNot(HaveOccurred()) + + Expect(fetchObject(ctx, r.Client, "", crb.Name, crb)).To(Succeed()) + + Expect(crb.ObjectMeta.Labels["my-label"]).ToNot(BeEmpty()) + Expect(crb.ObjectMeta.Labels["my-label"]).To(Equal("my-value")) + Expect(crb.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(crb.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + }) + + It("verify for aggregate-to-admin ClusterRole", func() { + clusterRoleAggregateToAdmin := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argo-rollouts-aggregate-to-admin", + Labels: map[string]string{ + "my-label": "my-value", + }, + }, + } + Expect(r.Client.Create(ctx, clusterRoleAggregateToAdmin)).To(Succeed()) + + err = r.reconcileRolloutsAggregateToAdminClusterRole(ctx, a) + Expect(err).ToNot(HaveOccurred()) + + Expect(fetchObject(ctx, r.Client, "", clusterRoleAggregateToAdmin.Name, clusterRoleAggregateToAdmin)).To(Succeed()) + + Expect(clusterRoleAggregateToAdmin.ObjectMeta.Labels["my-label"]).ToNot(BeEmpty()) + Expect(clusterRoleAggregateToAdmin.ObjectMeta.Labels["my-label"]).To(Equal("my-value")) + Expect(clusterRoleAggregateToAdmin.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRoleAggregateToAdmin.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + }) + + It("verify for aggregate-to-edit ClusterRole", func() { + clusterRoleAggregateToEdit := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argo-rollouts-aggregate-to-edit", + Labels: map[string]string{ + "my-label": "my-value", + }, + }, + } + Expect(r.Client.Create(ctx, clusterRoleAggregateToEdit)).To(Succeed()) + + err = r.reconcileRolloutsAggregateToEditClusterRole(ctx, a) + Expect(err).ToNot(HaveOccurred()) + + Expect(fetchObject(ctx, r.Client, "", clusterRoleAggregateToEdit.Name, clusterRoleAggregateToEdit)).To(Succeed()) + + Expect(clusterRoleAggregateToEdit.ObjectMeta.Labels["my-label"]).ToNot(BeEmpty()) + Expect(clusterRoleAggregateToEdit.ObjectMeta.Labels["my-label"]).To(Equal("my-value")) + Expect(clusterRoleAggregateToEdit.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRoleAggregateToEdit.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + }) + It("verify for aggregate-to-view ClusterRole", func() { + clusterRoleAggregateToView := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argo-rollouts-aggregate-to-view", + Labels: map[string]string{ + "my-label": "my-value", + }, + }, + } + Expect(r.Client.Create(ctx, clusterRoleAggregateToView)).To(Succeed()) + + err = r.reconcileRolloutsAggregateToViewClusterRole(ctx, a) + Expect(err).ToNot(HaveOccurred()) + + Expect(fetchObject(ctx, r.Client, "", clusterRoleAggregateToView.Name, clusterRoleAggregateToView)).To(Succeed()) + + Expect(clusterRoleAggregateToView.ObjectMeta.Labels["my-label"]).ToNot(BeEmpty()) + Expect(clusterRoleAggregateToView.ObjectMeta.Labels["my-label"]).To(Equal("my-value")) + Expect(clusterRoleAggregateToView.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(clusterRoleAggregateToView.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + }) + + It("verify for Service", func() { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultArgoRolloutsMetricsServiceName, + Namespace: a.Namespace, + Labels: map[string]string{ + "my-label": "my-value", + }, + }, + } + Expect(r.Client.Create(ctx, svc)).To(Succeed()) + + err = r.reconcileRolloutsMetricsService(ctx, a) + Expect(err).ToNot(HaveOccurred()) + + Expect(fetchObject(ctx, r.Client, a.Namespace, svc.Name, svc)).To(Succeed()) + + Expect(svc.ObjectMeta.Labels["my-label"]).ToNot(BeEmpty()) + Expect(svc.ObjectMeta.Labels["my-label"]).To(Equal("my-value")) + Expect(svc.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(svc.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + }) + + It("verify for Secret", func() { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultRolloutsNotificationSecretName, + Namespace: a.Namespace, + Labels: map[string]string{ + "my-label": "my-value", + }, + }, + Type: corev1.SecretTypeOpaque, + } + Expect(r.Client.Create(ctx, secret)).To(Succeed()) + + err = r.reconcileRolloutsSecrets(ctx, a) + Expect(err).ToNot(HaveOccurred()) + + Expect(fetchObject(ctx, r.Client, a.Namespace, secret.Name, secret)).To(Succeed()) + + Expect(secret.ObjectMeta.Labels["my-label"]).ToNot(BeEmpty()) + Expect(secret.ObjectMeta.Labels["my-label"]).To(Equal("my-value")) + Expect(secret.ObjectMeta.Labels["keylabel"]).To(Equal(a.Spec.AdditionalMetadata.Labels["keylabel"])) + Expect(secret.ObjectMeta.Annotations["keyannotation"]).To(Equal(a.Spec.AdditionalMetadata.Annotations["keyannotation"])) + }) + }) }) Context("Resource Cleanup test", func() { @@ -471,3 +801,32 @@ func serviceMonitor() *monitoringv1.ServiceMonitor { } return sm } + +func createServiceAccount(name, namespace string, labels map[string]string) *corev1.ServiceAccount { + return &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: labels, + }, + } +} + +func createRole(name, namespace string, labels map[string]string) *rbacv1.Role { + return &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: labels, + }, + } +} + +func createClusterRole(name string, labels map[string]string) *rbacv1.ClusterRole { + return &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: labels, + }, + } +} diff --git a/controllers/utils.go b/controllers/utils.go index 226d74a..352fda8 100644 --- a/controllers/utils.go +++ b/controllers/utils.go @@ -401,3 +401,46 @@ func createCondition(message string, reason ...string) metav1.Condition { Status: metav1.ConditionFalse, } } + +// removeUserLabelsAndAnnotations will remove any miscellaneous labels/annotations from obj, that are not used or expected by argo-rollouts-manager. For example, if a user added a label, "my-key": "my-value", to annotations of a Role that is created by our operator, this function wouls remove that label from 'obj'. +func removeUserLabelsAndAnnotations(obj *metav1.ObjectMeta, cr rolloutsmanagerv1alpha1.RolloutManager) { + + defaultLabelsAndAnnotations := metav1.ObjectMeta{} + setRolloutsLabelsAndAnnotationsToObject(&defaultLabelsAndAnnotations, cr) + + for objectLabelKey := range obj.Labels { + + existsInDefault := false + + for defaultLabelKey := range defaultLabelsAndAnnotations.Labels { + + if defaultLabelKey == objectLabelKey { + existsInDefault = true + break + } + } + + if !existsInDefault { + delete(obj.Labels, objectLabelKey) + } + + } + + for objectAnnotationKey := range obj.Annotations { + + existsInDefault := false + + for defaultAnnotationKey := range defaultLabelsAndAnnotations.Annotations { + + if defaultAnnotationKey == objectAnnotationKey { + existsInDefault = true + break + } + } + + if !existsInDefault { + delete(obj.Annotations, objectAnnotationKey) + } + } + +} diff --git a/controllers/utils_test.go b/controllers/utils_test.go index aac4910..3b8a79b 100644 --- a/controllers/utils_test.go +++ b/controllers/utils_test.go @@ -382,6 +382,79 @@ var _ = Describe("validateRolloutsScope tests", func() { }) }) +var _ = Describe("removeUserLabelsAndAnnotations tests", func() { + var ( + obj metav1.ObjectMeta + cr rolloutsmanagerv1alpha1.RolloutManager + ctx context.Context + k8sClient client.WithWatch + ) + + BeforeEach(func() { + s := scheme.Scheme + Expect(rolloutsmanagerv1alpha1.AddToScheme(s)).To(Succeed()) + + ctx = context.Background() + log = logger.FromContext(ctx) + k8sClient = fake.NewClientBuilder().WithStatusSubresource(&rolloutsmanagerv1alpha1.RolloutManager{}).WithScheme(s).Build() + + cr = rolloutsmanagerv1alpha1.RolloutManager{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rm-1", + Namespace: "test-ns-1", + }, + Spec: rolloutsmanagerv1alpha1.RolloutManagerSpec{ + NamespaceScoped: false, + }, + } + }) + + DescribeTable("should correctly remove user labels and annotations", + func(initialLabels, initialAnnotations, expectedLabels, expectedAnnotations map[string]string) { + obj = metav1.ObjectMeta{ + Labels: initialLabels, + Annotations: initialAnnotations, + } + + Expect(k8sClient.Create(ctx, &cr)).To(Succeed()) + setRolloutsLabelsAndAnnotations(&obj) + + removeUserLabelsAndAnnotations(&obj, cr) + + Expect(obj.Labels).To(Equal(expectedLabels)) + Expect(obj.Annotations).To(Equal(expectedAnnotations)) + }, + Entry("when no user-defined labels or annotations exist", + map[string]string{}, map[string]string{}, + map[string]string{"app.kubernetes.io/name": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/part-of": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/component": DefaultArgoRolloutsResourceName}, + map[string]string{}, + ), + Entry("when user-defined labels and annotations are present", + map[string]string{"user-label": "value"}, map[string]string{"user-annotation": "value"}, + map[string]string{"app.kubernetes.io/name": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/part-of": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/component": DefaultArgoRolloutsResourceName}, + map[string]string{}, + ), + Entry("when user-defined labels are present and annotations are empty", + map[string]string{"user-label": "value"}, map[string]string{}, + map[string]string{"app.kubernetes.io/name": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/part-of": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/component": DefaultArgoRolloutsResourceName}, + map[string]string{}, + ), + Entry("when user-defined labels are present and annotations are empty", + map[string]string{}, map[string]string{"user-annotation": "value"}, + map[string]string{"app.kubernetes.io/name": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/part-of": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/component": DefaultArgoRolloutsResourceName}, + map[string]string{}, + ), + ) +}) + const ( testNamespace = "rollouts" testRolloutManagerName = "rollouts" From 6e57897d0a747d847f95cf9a6142abeb5f6087bb Mon Sep 17 00:00:00 2001 From: Jonathan West Date: Mon, 29 Jul 2024 12:58:06 -0400 Subject: [PATCH 2/2] Refactor service/monitor function, add back existing tests Signed-off-by: Jonathan West --- controllers/reconcile.go | 2 +- controllers/resources.go | 134 +++++++++++---------- controllers/resources_test.go | 221 +++++++++++++++++++++++++++++++++- controllers/utils.go | 2 +- controllers/utils_test.go | 14 ++- 5 files changed, 306 insertions(+), 67 deletions(-) diff --git a/controllers/reconcile.go b/controllers/reconcile.go index 21acd45..77359dc 100644 --- a/controllers/reconcile.go +++ b/controllers/reconcile.go @@ -122,7 +122,7 @@ func (r *RolloutManagerReconciler) reconcileRolloutsManager(ctx context.Context, } log.Info("reconciling Rollouts Metrics Service") - if err := r.reconcileRolloutsMetricsService(ctx, cr); err != nil { + if err := r.reconcileRolloutsMetricsServiceAndMonitor(ctx, cr); err != nil { log.Error(err, "failed to reconcile Rollout's Metrics Service.") return wrapCondition(createCondition(err.Error())), err } diff --git a/controllers/resources.go b/controllers/resources.go index 09604b2..48fb495 100644 --- a/controllers/resources.go +++ b/controllers/resources.go @@ -520,8 +520,74 @@ func (r *RolloutManagerReconciler) reconcileRolloutsAggregateToViewClusterRole(c return nil } -// Reconcile Rollouts Metrics Service. -func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) error { +// reconcileRolloutsMetricsServiceAndMonitor reconciles the Rollouts Metrics Service and ServiceMonitor +func (r *RolloutManagerReconciler) reconcileRolloutsMetricsServiceAndMonitor(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) error { + + reconciledSvc, err := r.reconcileRolloutsMetricsService(ctx, cr) + if err != nil { + return fmt.Errorf("unable to reconcile metrics service: %w", err) + } + + // Checks if user is using the Prometheus operator by checking CustomResourceDefinition for ServiceMonitor + smCRD := &crdv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceMonitorsCRDName, + }, + } + + if err := fetchObject(ctx, r.Client, smCRD.Namespace, smCRD.Name, smCRD); err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get the ServiceMonitor %s : %s", smCRD.Name, err) + } + return nil + } + + // Create ServiceMonitor for Rollouts metrics + existingServiceMonitor := &monitoringv1.ServiceMonitor{} + if err := fetchObject(ctx, r.Client, cr.Namespace, DefaultArgoRolloutsResourceName, existingServiceMonitor); err != nil { + if apierrors.IsNotFound(err) { + if err := r.createServiceMonitorIfAbsent(ctx, cr.Namespace, cr, DefaultArgoRolloutsResourceName, reconciledSvc.Name); err != nil { + return err + } + return nil + + } else { + log.Error(err, "Error querying for ServiceMonitor", "Namespace", cr.Namespace, "Name", reconciledSvc.Name) + return err + } + + } else { + log.Info("A ServiceMonitor instance already exists", + "Namespace", existingServiceMonitor.Namespace, "Name", existingServiceMonitor.Name) + + // Check if existing ServiceMonitor matches expected content + if !serviceMonitorMatches(existingServiceMonitor, reconciledSvc.Name) { + log.Info("Updating existing ServiceMonitor instance", + "Namespace", existingServiceMonitor.Namespace, "Name", existingServiceMonitor.Name) + + // Update ServiceMonitor with expected content + existingServiceMonitor.Spec.Selector.MatchLabels = map[string]string{ + "app.kubernetes.io/name": reconciledSvc.Name, + } + existingServiceMonitor.Spec.Endpoints = []monitoringv1.Endpoint{ + { + Port: "metrics", + }, + } + + if err := r.Client.Update(ctx, existingServiceMonitor); err != nil { + log.Error(err, "Error updating existing ServiceMonitor instance", + "Namespace", existingServiceMonitor.Namespace, "Name", existingServiceMonitor.Name) + return err + } + } + return nil + } + +} + +// reconcileRolloutsMetricsService reconciles the Service which is used to gather metrics from Rollouts install +func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) (*corev1.Service, error) { expectedSvc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -550,17 +616,17 @@ func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.C liveService := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: expectedSvc.Name, Namespace: expectedSvc.Namespace}} if err := fetchObject(ctx, r.Client, cr.Namespace, liveService.Name, liveService); err != nil { if !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to get the Service %s: %w", expectedSvc.Name, err) + return nil, fmt.Errorf("failed to get the Service %s: %w", expectedSvc.Name, err) } if err := controllerutil.SetControllerReference(&cr, expectedSvc, r.Scheme); err != nil { - return err + return nil, err } log.Info(fmt.Sprintf("Creating Service %s", expectedSvc.Name)) if err := r.Client.Create(ctx, expectedSvc); err != nil { log.Error(err, "Error creating Service", "Name", expectedSvc.Name) - return err + return nil, err } liveService = expectedSvc @@ -588,65 +654,11 @@ func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.C // Update if the Service already exists and needs to be modified if err := r.Client.Update(ctx, liveService); err != nil { log.Error(err, "Error updating Ports of metrics Service", "Name", liveService.Name) - return err + return liveService, err } } - // Checks if user is using the Prometheus operator by checking CustomResourceDefinition for ServiceMonitor - smCRD := &crdv1.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: serviceMonitorsCRDName, - }, - } - - if err := fetchObject(ctx, r.Client, smCRD.Namespace, smCRD.Name, smCRD); err != nil { - if !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to get the ServiceMonitor %s : %s", smCRD.Name, err) - } - return nil - } - - // Create ServiceMonitor for Rollouts metrics - existingServiceMonitor := &monitoringv1.ServiceMonitor{} - if err := fetchObject(ctx, r.Client, cr.Namespace, DefaultArgoRolloutsResourceName, existingServiceMonitor); err != nil { - if apierrors.IsNotFound(err) { - if err := r.createServiceMonitorIfAbsent(ctx, cr.Namespace, cr, DefaultArgoRolloutsResourceName, expectedSvc.Name); err != nil { - return err - } - return nil - - } else { - log.Error(err, "Error querying for ServiceMonitor", "Namespace", cr.Namespace, "Name", liveService.Name) - return err - } - - } else { - log.Info("A ServiceMonitor instance already exists", - "Namespace", existingServiceMonitor.Namespace, "Name", existingServiceMonitor.Name) - - // Check if existing ServiceMonitor matches expected content - if !serviceMonitorMatches(existingServiceMonitor, liveService.Name) { - log.Info("Updating existing ServiceMonitor instance", - "Namespace", existingServiceMonitor.Namespace, "Name", existingServiceMonitor.Name) - - // Update ServiceMonitor with expected content - existingServiceMonitor.Spec.Selector.MatchLabels = map[string]string{ - "app.kubernetes.io/name": liveService.Name, - } - existingServiceMonitor.Spec.Endpoints = []monitoringv1.Endpoint{ - { - Port: "metrics", - }, - } - - if err := r.Client.Update(ctx, existingServiceMonitor); err != nil { - log.Error(err, "Error updating existing ServiceMonitor instance", - "Namespace", existingServiceMonitor.Namespace, "Name", existingServiceMonitor.Name) - return err - } - } - return nil - } + return liveService, nil } diff --git a/controllers/resources_test.go b/controllers/resources_test.go index a7de0b5..deb1670 100644 --- a/controllers/resources_test.go +++ b/controllers/resources_test.go @@ -19,7 +19,222 @@ import ( var _ = Describe("Resource creation and cleanup tests", func() { - Context("Resource creation test", func() { + Context("Verify resource creation when RolloutManger does not contain a user-defined label/annotation", func() { + var ( + ctx context.Context + a v1alpha1.RolloutManager + r *RolloutManagerReconciler + ) + + BeforeEach(func() { + ctx = context.Background() + a = *makeTestRolloutManager() + r = makeTestReconciler(&a) + err := createNamespace(r, a.Namespace) + Expect(err).ToNot(HaveOccurred()) + }) + + It("Test for reconcileRolloutsServiceAccount function", func() { + _, err := r.reconcileRolloutsServiceAccount(ctx, a) + Expect(err).ToNot(HaveOccurred()) + }) + + It("Test for reconcileRolloutsRole function", func() { + role, err := r.reconcileRolloutsRole(ctx, a) + Expect(err).ToNot(HaveOccurred()) + + By("Modify Rules of Role.") + role.Rules[0].Verbs = append(role.Rules[0].Verbs, "test") + Expect(r.Client.Update(ctx, role)).To(Succeed()) + + By("Reconciler should revert modifications.") + role, err = r.reconcileRolloutsRole(ctx, a) + Expect(err).ToNot(HaveOccurred()) + Expect(role.Rules).To(Equal(GetPolicyRules())) + }) + + It("Test for reconcileRolloutsClusterRole function", func() { + clusterRole, err := r.reconcileRolloutsClusterRole(ctx, a) + Expect(err).ToNot(HaveOccurred()) + + By("Modify Rules of Role.") + clusterRole.Rules[0].Verbs = append(clusterRole.Rules[0].Verbs, "test") + Expect(r.Client.Update(ctx, clusterRole)).To(Succeed()) + + By("Reconciler should revert modifications.") + clusterRole, err = r.reconcileRolloutsClusterRole(ctx, a) + Expect(err).ToNot(HaveOccurred()) + Expect(clusterRole.Rules).To(Equal(GetPolicyRules())) + }) + + It("Test for reconcileRolloutsRoleBinding function", func() { + sa, err := r.reconcileRolloutsServiceAccount(ctx, a) + Expect(err).ToNot(HaveOccurred()) + role, err := r.reconcileRolloutsRole(ctx, a) + Expect(err).ToNot(HaveOccurred()) + + Expect(r.reconcileRolloutsRoleBinding(ctx, a, role, sa)).To(Succeed()) + + By("Modify Subject of RoleBinding.") + rb := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultArgoRolloutsResourceName, + Namespace: a.Namespace, + }, + } + Expect(fetchObject(ctx, r.Client, a.Namespace, rb.Name, rb)).To(Succeed()) + + subTemp := rb.Subjects + rb.Subjects = append(rb.Subjects, rbacv1.Subject{Kind: rbacv1.ServiceAccountKind, Name: "test", Namespace: "test"}) + Expect(r.Client.Update(ctx, rb)).To(Succeed()) + + By("Reconciler should revert modifications.") + Expect(r.reconcileRolloutsRoleBinding(ctx, a, role, sa)).To(Succeed()) + Expect(fetchObject(ctx, r.Client, a.Namespace, rb.Name, rb)).To(Succeed()) + Expect(rb.Subjects).To(Equal(subTemp)) + }) + + It("Test for reconcileRolloutsClusterRoleBinding function", func() { + sa, err := r.reconcileRolloutsServiceAccount(ctx, a) + Expect(err).ToNot(HaveOccurred()) + clusterRole, err := r.reconcileRolloutsClusterRole(ctx, a) + Expect(err).ToNot(HaveOccurred()) + + Expect(r.reconcileRolloutsClusterRoleBinding(ctx, clusterRole, sa, a)).To(Succeed()) + + By("Modify Subject of ClusterRoleBinding.") + crb := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultArgoRolloutsResourceName, + }, + } + Expect(fetchObject(ctx, r.Client, "", crb.Name, crb)).To(Succeed()) + + subTemp := crb.Subjects + crb.Subjects = append(crb.Subjects, rbacv1.Subject{Kind: rbacv1.ServiceAccountKind, Name: "test", Namespace: "test"}) + Expect(r.Client.Update(ctx, crb)).To(Succeed()) + + By("Reconciler should revert modifications.") + Expect(r.reconcileRolloutsClusterRoleBinding(ctx, clusterRole, sa, a)).To(Succeed()) + Expect(fetchObject(ctx, r.Client, "", crb.Name, crb)).To(Succeed()) + Expect(crb.Subjects).To(Equal(subTemp)) + }) + + It("Test for reconcileRolloutsAggregateToAdminClusterRole function", func() { + Expect(r.reconcileRolloutsAggregateToAdminClusterRole(ctx, a)).To(Succeed()) + + By("Modify Rules of ClusterRole.") + clusterRole := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argo-rollouts-aggregate-to-admin", + }, + } + Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) + clusterRole.Rules[0].Verbs = append(clusterRole.Rules[0].Verbs, "test") + Expect(r.Client.Update(ctx, clusterRole)).To(Succeed()) + + By("Reconciler should revert modifications.") + Expect(r.reconcileRolloutsAggregateToAdminClusterRole(ctx, a)).To(Succeed()) + Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) + Expect(clusterRole.Rules).To(Equal(GetAggregateToAdminPolicyRules())) + }) + + It("Test for reconcileRolloutsAggregateToEditClusterRole function", func() { + Expect(r.reconcileRolloutsAggregateToEditClusterRole(ctx, a)).To(Succeed()) + + By("Modify Rules of ClusterRole.") + clusterRole := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argo-rollouts-aggregate-to-edit", + }, + } + Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) + clusterRole.Rules[0].Verbs = append(clusterRole.Rules[0].Verbs, "test") + Expect(r.Client.Update(ctx, clusterRole)).To(Succeed()) + + By("Reconciler should revert modifications.") + Expect(r.reconcileRolloutsAggregateToEditClusterRole(ctx, a)).To(Succeed()) + Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) + Expect(clusterRole.Rules).To(Equal(GetAggregateToEditPolicyRules())) + }) + + It("Test for reconcileRolloutsAggregateToViewClusterRole function", func() { + Expect(r.reconcileRolloutsAggregateToViewClusterRole(ctx, a)).To(Succeed()) + + By("Modify Rules of ClusterRole.") + clusterRole := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argo-rollouts-aggregate-to-view", + }, + } + Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) + clusterRole.Rules[0].Verbs = append(clusterRole.Rules[0].Verbs, "test") + Expect(r.Client.Update(ctx, clusterRole)).To(Succeed()) + + By("Reconciler should revert modifications.") + Expect(r.reconcileRolloutsAggregateToViewClusterRole(ctx, a)).To(Succeed()) + Expect(fetchObject(ctx, r.Client, "", clusterRole.Name, clusterRole)).To(Succeed()) + Expect(clusterRole.Rules).To(Equal(GetAggregateToViewPolicyRules())) + }) + + It("Test for reconcileRolloutsMetricsService function", func() { + Expect(r.reconcileRolloutsMetricsServiceAndMonitor(ctx, a)).To(Succeed()) + }) + + It("Test for reconcileRolloutsSecrets function", func() { + Expect(r.reconcileRolloutsSecrets(ctx, a)).To(Succeed()) + }) + + It("test for removeClusterScopedResourcesIfApplicable function", func() { + + By("creating default cluster-scoped ClusterRole/ClusterRoleBinding. These should be deleted by the call to removeClusterScopedResourcesIfApplicable") + clusterRole := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultArgoRolloutsResourceName, + }, + } + Expect(r.Client.Create(ctx, clusterRole)).To(Succeed()) + + clusterRoleBinding := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultArgoRolloutsResourceName, + }, + } + Expect(r.Client.Create(ctx, clusterRoleBinding)).To(Succeed()) + + By("creating default cluster-scoped ClusterRole/ClusterRoleBinding with a different name. These should not be deleted") + + unrelatedRole := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unrelated-resource-should-not-be-deleted", + }, + } + Expect(r.Client.Create(ctx, unrelatedRole)).To(Succeed()) + + unrelatedRoleBinding := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unrelated-resource-should-not-be-deleted", + }, + } + Expect(r.Client.Create(ctx, unrelatedRoleBinding)).To(Succeed()) + + By("calling removeClusterScopedResourcesIfApplicable, which should delete the cluster scoped resources") + Expect(r.removeClusterScopedResourcesIfApplicable(ctx)).To(Succeed()) + + Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRole), clusterRole)).ToNot(Succeed(), + "ClusterRole should have been deleted") + Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(clusterRoleBinding), clusterRoleBinding)).ToNot(Succeed(), "ClusterRoleBinding should have been deleted") + + Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(unrelatedRole), unrelatedRole)).To(Succeed(), + "Unrelated ClusterRole should not have been deleted") + Expect(r.Client.Get(ctx, client.ObjectKeyFromObject(unrelatedRoleBinding), unrelatedRoleBinding)).To(Succeed(), "Unrelated ClusterRoleBinding should not have been deleted") + + Expect(r.removeClusterScopedResourcesIfApplicable(ctx)).To(Succeed(), "calling the function again should not return an error") + + }) + }) + + Context("Verify resource creation when RolloutManger contains a user-defined label/annotation", func() { var ( ctx context.Context a v1alpha1.RolloutManager @@ -268,7 +483,7 @@ var _ = Describe("Resource creation and cleanup tests", func() { }) It("Test for reconcileRolloutsMetricsService function", func() { - Expect(r.reconcileRolloutsMetricsService(ctx, a)).To(Succeed()) + Expect(r.reconcileRolloutsMetricsServiceAndMonitor(ctx, a)).To(Succeed()) service := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: DefaultArgoRolloutsMetricsServiceName, @@ -527,7 +742,7 @@ var _ = Describe("Resource creation and cleanup tests", func() { } Expect(r.Client.Create(ctx, svc)).To(Succeed()) - err = r.reconcileRolloutsMetricsService(ctx, a) + err = r.reconcileRolloutsMetricsServiceAndMonitor(ctx, a) Expect(err).ToNot(HaveOccurred()) Expect(fetchObject(ctx, r.Client, a.Namespace, svc.Name, svc)).To(Succeed()) diff --git a/controllers/utils.go b/controllers/utils.go index 352fda8..55de897 100644 --- a/controllers/utils.go +++ b/controllers/utils.go @@ -402,7 +402,7 @@ func createCondition(message string, reason ...string) metav1.Condition { } } -// removeUserLabelsAndAnnotations will remove any miscellaneous labels/annotations from obj, that are not used or expected by argo-rollouts-manager. For example, if a user added a label, "my-key": "my-value", to annotations of a Role that is created by our operator, this function wouls remove that label from 'obj'. +// removeUserLabelsAndAnnotations will remove any miscellaneous labels/annotations from obj, that are not used or expected by argo-rollouts-manager. For example, if a user added a label, "my-key": "my-value", to annotations of a Role that is created by our operator, this function would remove that label from 'obj'. func removeUserLabelsAndAnnotations(obj *metav1.ObjectMeta, cr rolloutsmanagerv1alpha1.RolloutManager) { defaultLabelsAndAnnotations := metav1.ObjectMeta{} diff --git a/controllers/utils_test.go b/controllers/utils_test.go index 3b8a79b..1af2527 100644 --- a/controllers/utils_test.go +++ b/controllers/utils_test.go @@ -445,13 +445,25 @@ var _ = Describe("removeUserLabelsAndAnnotations tests", func() { "app.kubernetes.io/component": DefaultArgoRolloutsResourceName}, map[string]string{}, ), - Entry("when user-defined labels are present and annotations are empty", + Entry("when user-defined labels are empty and annotations are present", map[string]string{}, map[string]string{"user-annotation": "value"}, map[string]string{"app.kubernetes.io/name": DefaultArgoRolloutsResourceName, "app.kubernetes.io/part-of": DefaultArgoRolloutsResourceName, "app.kubernetes.io/component": DefaultArgoRolloutsResourceName}, map[string]string{}, ), + + Entry("when both user and non-user-defined labels are present, and annotations are empty", + map[string]string{"user-label": "value", + "app.kubernetes.io/name": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/part-of": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/component": DefaultArgoRolloutsResourceName}, + map[string]string{}, + map[string]string{"app.kubernetes.io/name": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/part-of": DefaultArgoRolloutsResourceName, + "app.kubernetes.io/component": DefaultArgoRolloutsResourceName}, + map[string]string{}, + ), ) })