From 56bd57803b52ce6f3face95b7a906a324ef0eb8c Mon Sep 17 00:00:00 2001 From: Kyle Wuolle Date: Wed, 13 Nov 2024 10:20:15 -0800 Subject: [PATCH] Combine the cluster / machine permissions per review --- api/v1alpha1/common.go | 3 +++ .../controller/managedcluster_controller_test.go | 10 ++++++---- .../multiclusterservice_controller_test.go | 10 ++++++---- internal/controller/unmanagedcluster_controller.go | 13 ------------- .../controller/unmanagedcluster_controller_test.go | 6 +----- .../crds/hmc.mirantis.com_managedclusters.yaml | 3 +++ .../crds/hmc.mirantis.com_multiclusterservices.yaml | 3 +++ .../crds/hmc.mirantis.com_unmanagedclusters.yaml | 3 +++ .../hmc/templates/rbac/controller/roles.yaml | 13 ++++--------- 9 files changed, 29 insertions(+), 35 deletions(-) diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index 6d45205b2..3b5da3072 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -58,6 +58,9 @@ type ServicesType struct { // that could be installed on the target cluster. Services []ServiceSpec `json:"services,omitempty"` + // +kubebuilder:default:=100 + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=2147483646 // ServicesPriority sets the priority for the services defined in this spec. // Higher value means higher priority and lower means lower. // In case of conflict with another object managing the service, diff --git a/internal/controller/managedcluster_controller_test.go b/internal/controller/managedcluster_controller_test.go index ac37d1901..2ddf12716 100644 --- a/internal/controller/managedcluster_controller_test.go +++ b/internal/controller/managedcluster_controller_test.go @@ -180,10 +180,12 @@ var _ = Describe("ManagedCluster Controller", func() { Spec: hmc.ManagedClusterSpec{ Template: templateName, Credential: credentialName, - Services: []hmc.ServiceSpec{ - { - Template: svcTemplateName, - Name: "test-svc-name", + ServicesType: hmc.ServicesType{ + Services: []hmc.ServiceSpec{ + { + Template: svcTemplateName, + Name: "test-svc-name", + }, }, }, }, diff --git a/internal/controller/multiclusterservice_controller_test.go b/internal/controller/multiclusterservice_controller_test.go index 817eba6f5..19b3e3c10 100644 --- a/internal/controller/multiclusterservice_controller_test.go +++ b/internal/controller/multiclusterservice_controller_test.go @@ -176,10 +176,12 @@ var _ = Describe("MultiClusterService Controller", func() { }, }, Spec: hmc.MultiClusterServiceSpec{ - Services: []hmc.ServiceSpec{ - { - Template: serviceTemplateName, - Name: helmChartReleaseName, + ServicesType: hmc.ServicesType{ + Services: []hmc.ServiceSpec{ + { + Template: serviceTemplateName, + Name: helmChartReleaseName, + }, }, }, }, diff --git a/internal/controller/unmanagedcluster_controller.go b/internal/controller/unmanagedcluster_controller.go index f6830745f..3ca24b836 100644 --- a/internal/controller/unmanagedcluster_controller.go +++ b/internal/controller/unmanagedcluster_controller.go @@ -33,7 +33,6 @@ import ( "k8s.io/client-go/tools/clientcmd" "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util/kubeconfig" - "sigs.k8s.io/cluster-api/util/secret" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -311,18 +310,6 @@ func (r *UnmanagedClusterReconciler) reconcileDeletion(ctx context.Context, unma return ctrl.Result{Requeue: true}, fmt.Errorf("failed to delete unmanaged machines: %w", err) } - if err := r.Delete(ctx, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: unmanagedCluster.Namespace, - Name: secret.Name(unmanagedCluster.Name, secret.Kubeconfig), - Labels: map[string]string{ - v1beta1.ClusterNameLabel: unmanagedCluster.Name, - }, - }, - }); err != nil && !apierrors.IsNotFound(err) { - return ctrl.Result{Requeue: true}, fmt.Errorf("failed to delete cluster secret: %w", err) - } - if err := r.Delete(ctx, &v1beta1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Namespace: unmanagedCluster.Namespace, diff --git a/internal/controller/unmanagedcluster_controller_test.go b/internal/controller/unmanagedcluster_controller_test.go index d70e3e137..3a6fc7fdf 100644 --- a/internal/controller/unmanagedcluster_controller_test.go +++ b/internal/controller/unmanagedcluster_controller_test.go @@ -76,11 +76,7 @@ var _ = Describe("UnmanagedCluster Controller", func() { Name: unmanagedClusterName, Namespace: unmanagedClusterNamespace, }, - Spec: hmc.UnmanagedClusterSpec{ - Services: nil, - ServicesPriority: 1, - StopOnConflict: true, - }, + Spec: hmc.UnmanagedClusterSpec{}, } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) } diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml index b33882246..c1ef43e63 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml @@ -103,12 +103,15 @@ spec: type: object type: array servicesPriority: + default: 100 description: |- ServicesPriority sets the priority for the services defined in this spec. Higher value means higher priority and lower means lower. In case of conflict with another object managing the service, the one with higher priority will get to deploy its services. format: int32 + maximum: 2147483646 + minimum: 1 type: integer stopOnConflict: default: false diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_multiclusterservices.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_multiclusterservices.yaml index 3e76cdf22..e15a1e115 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_multiclusterservices.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_multiclusterservices.yaml @@ -121,12 +121,15 @@ spec: type: object type: array servicesPriority: + default: 100 description: |- ServicesPriority sets the priority for the services defined in this spec. Higher value means higher priority and lower means lower. In case of conflict with another object managing the service, the one with higher priority will get to deploy its services. format: int32 + maximum: 2147483646 + minimum: 1 type: integer stopOnConflict: default: false diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_unmanagedclusters.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_unmanagedclusters.yaml index 883c4a2f0..cb8bc9c51 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_unmanagedclusters.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_unmanagedclusters.yaml @@ -75,12 +75,15 @@ spec: type: object type: array servicesPriority: + default: 100 description: |- ServicesPriority sets the priority for the services defined in this spec. Higher value means higher priority and lower means lower. In case of conflict with another object managing the service, the one with higher priority will get to deploy its services. format: int32 + maximum: 2147483646 + minimum: 1 type: integer stopOnConflict: default: false diff --git a/templates/provider/hmc/templates/rbac/controller/roles.yaml b/templates/provider/hmc/templates/rbac/controller/roles.yaml index 79e2a3978..f977d242a 100644 --- a/templates/provider/hmc/templates/rbac/controller/roles.yaml +++ b/templates/provider/hmc/templates/rbac/controller/roles.yaml @@ -19,7 +19,8 @@ rules: - cluster.x-k8s.io resources: - clusters - verbs: {{ include "rbac.viewerVerbs" . | nindent 4 }} + verbs: {{ include "rbac.editorVerbs" . | nindent 4 }} + - delete - apiGroups: - helm.toolkit.fluxcd.io resources: @@ -145,7 +146,8 @@ rules: - cluster.x-k8s.io resources: - machines - verbs: {{ include "rbac.viewerVerbs" . | nindent 4 }} + verbs: {{ include "rbac.editorVerbs" . | nindent 4 }} + - delete - apiGroups: - "" resources: @@ -250,13 +252,6 @@ rules: - get - patch - update -- apiGroups: - - cluster.x-k8s.io - resources: - - clusters - - machines - verbs: {{ include "rbac.editorVerbs" . | nindent 4 }} - - delete - apiGroups: - config.projectsveltos.io resources: