diff --git a/controllers/cluster/role.go b/controllers/cluster/role.go deleted file mode 100644 index faf1d14c..00000000 --- a/controllers/cluster/role.go +++ /dev/null @@ -1,50 +0,0 @@ -package cluster - -import ( - "fmt" - - synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1" - "github.com/projectsyn/lieutenant-operator/pipeline" - roleUtil "github.com/projectsyn/lieutenant-operator/role" - rbacv1 "k8s.io/api/rbac/v1" - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" -) - -func clusterUpdateRole(obj pipeline.Object, data *pipeline.Context) pipeline.Result { - cluster, ok := obj.(*synv1alpha1.Cluster) - if !ok { - return pipeline.Result{Err: fmt.Errorf("object is not a cluster")} - } - - name := types.NamespacedName{Name: cluster.Spec.TenantRef.Name, Namespace: cluster.Namespace} - role := &rbacv1.Role{} - if err := data.Client.Get(data.Context, name, role); err != nil { - if errors.IsNotFound(err) || runtime.IsNotRegisteredError(err) { - // The absence of a role is not an error. - // The role might not yet be created. It gets update on a future reconciliation. - data.Log.Info("No role found to update.") - return pipeline.Result{} - } - return pipeline.Result{Err: fmt.Errorf("failed to get role for cluster: %v", err)} - } - - updated := false - - if data.Deleted { - updated = roleUtil.RemoveResourceNames(role, cluster.Name) - } else { - updated = roleUtil.AddResourceNames(role, cluster.Name) - } - - if !updated { - return pipeline.Result{} - } - - if err := data.Client.Update(data.Context, role); err != nil { - return pipeline.Result{Err: fmt.Errorf("failed to update role for cluster: %v", err)} - } - - return pipeline.Result{} -} diff --git a/controllers/cluster/steps.go b/controllers/cluster/steps.go index 956729de..e482c6a7 100644 --- a/controllers/cluster/steps.go +++ b/controllers/cluster/steps.go @@ -14,7 +14,6 @@ func SpecificSteps(obj pipeline.Object, data *pipeline.Context) pipeline.Result {Name: "delete vault entries", F: vault.HandleVaultDeletion}, {Name: "set tenant owner", F: setTenantOwner}, {Name: "apply cluster template from tenant", F: applyClusterTemplateFromTenant}, - {Name: "update Role", F: clusterUpdateRole}, } return pipeline.RunPipeline(obj, data, steps) diff --git a/controllers/tenant/role.go b/controllers/tenant/role.go index aef50451..2148797b 100644 --- a/controllers/tenant/role.go +++ b/controllers/tenant/role.go @@ -3,73 +3,66 @@ package tenant import ( "fmt" + "github.com/projectsyn/lieutenant-operator/api/v1alpha1" synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1" "github.com/projectsyn/lieutenant-operator/pipeline" - roleUtil "github.com/projectsyn/lieutenant-operator/role" rbacv1 "k8s.io/api/rbac/v1" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) -func createRole(obj pipeline.Object, data *pipeline.Context) pipeline.Result { +func reconcileRole(obj pipeline.Object, data *pipeline.Context) pipeline.Result { tenant, ok := obj.(*synv1alpha1.Tenant) if !ok { return pipeline.Result{Err: fmt.Errorf("object is not a tenant")} } - role, err := newRole(data.Client.Scheme(), tenant) - if err != nil { - return pipeline.Result{Err: fmt.Errorf("failed to create Role for tenant: %w", err)} + cls := v1alpha1.ClusterList{} + if err := data.Client.List(data.Context, &cls, + client.InNamespace(tenant.Namespace), + client.MatchingFields{"spec.tenantRef.name": tenant.Name}, + ); err != nil { + return pipeline.Result{Err: fmt.Errorf("failed to list clusters: %w", err)} } - - err = data.Client.Create(data.Context, role) - if err != nil && !errors.IsAlreadyExists(err) { - return pipeline.Result{Err: fmt.Errorf("create role: %w", err)} + clusterNames := make([]string, 0, len(cls.Items)) + for _, c := range cls.Items { + clusterNames = append(clusterNames, c.Name) } - return pipeline.Result{} -} - -// newRole returns a new Role object. -// The Role controls access to the tenant and its related clusters. -func newRole(scheme *runtime.Scheme, tenant *synv1alpha1.Tenant) (*rbacv1.Role, error) { - role := &rbacv1.Role{ + role := rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ Name: tenant.Name, Namespace: tenant.Namespace, }, } - setManagedByLabel(role) - if err := controllerutil.SetControllerReference(tenant, role, scheme); err != nil { - return nil, err - } - roleUtil.EnsureRules(role) - - return role, nil -} - -func tenantUpdateRole(obj pipeline.Object, data *pipeline.Context) pipeline.Result { - tenant, ok := obj.(*synv1alpha1.Tenant) - if !ok { - return pipeline.Result{Err: fmt.Errorf("object is not a tenant")} - } - - name := types.NamespacedName{Name: tenant.Name, Namespace: tenant.Namespace} - role := &rbacv1.Role{} - if err := data.Client.Get(data.Context, name, role); err != nil { - return pipeline.Result{Err: fmt.Errorf("failed to get role for tenant: %v", err)} - } - - if !roleUtil.AddResourceNames(role, tenant.Name) { - return pipeline.Result{} - } - - if err := data.Client.Update(data.Context, role); err != nil { - return pipeline.Result{Err: fmt.Errorf("failed to update role for tenant: %v", err)} + op, err := controllerutil.CreateOrUpdate(data.Context, data.Client, &role, func() error { + role.Rules = []rbacv1.PolicyRule{ + { + APIGroups: []string{synv1alpha1.GroupVersion.Group}, + Verbs: []string{"get"}, + Resources: []string{"tenants"}, + ResourceNames: []string{tenant.Name}, + }, + { + APIGroups: []string{synv1alpha1.GroupVersion.Group}, + Verbs: []string{"get"}, + Resources: []string{"clusters"}, + ResourceNames: clusterNames, + }, + { + APIGroups: []string{synv1alpha1.GroupVersion.Group}, + Verbs: []string{"get", "update", "patch"}, + Resources: []string{"clusters/status"}, + ResourceNames: clusterNames, + }, + } + return controllerutil.SetControllerReference(tenant, &role, data.Client.Scheme()) + }) + if err != nil { + return pipeline.Result{Err: fmt.Errorf("failed to create or update role (op: %s): %w", op, err)} } + data.Log.Info("Role reconciled", "role", role.Name, "operation", op) return pipeline.Result{} } diff --git a/controllers/tenant/steps.go b/controllers/tenant/steps.go index 1b39bc2d..c10cdb65 100644 --- a/controllers/tenant/steps.go +++ b/controllers/tenant/steps.go @@ -11,9 +11,8 @@ func Steps(obj pipeline.Object, data *pipeline.Context) pipeline.Result { {Name: "uptade tenant git repo", F: updateTenantGitRepo}, {Name: "set global git repo url", F: setGlobalGitRepoURL}, {Name: "create ServiceAccount", F: createServiceAccount}, - {Name: "create Role", F: createRole}, + {Name: "reconcile Role", F: reconcileRole}, {Name: "create RoleBinding", F: createRoleBinding}, - {Name: "update Role", F: tenantUpdateRole}, } return pipeline.RunPipeline(obj, data, steps) diff --git a/main.go b/main.go index 4591bad4..be8cd622 100644 --- a/main.go +++ b/main.go @@ -16,6 +16,7 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/metrics/server" @@ -51,6 +52,8 @@ func init() { } func main() { + ctx := ctrl.SetupSignalHandler() + var metricsAddr string var enableLeaderElection bool var probeAddr string @@ -101,6 +104,13 @@ func main() { setupLog.Error(err, "unable to start manager") os.Exit(1) } + if err := mgr.GetFieldIndexer().IndexField(ctx, &synv1alpha1.Cluster{}, "spec.tenantRef.name", func(o client.Object) []string { + cluster := o.(*synv1alpha1.Cluster) + return []string{cluster.Spec.TenantRef.Name} + }); err != nil { + setupLog.Error(err, "unable to create tenantRef field indexer for Cluster") + os.Exit(1) + } if err = (&controllers.ClusterReconciler{ Client: mgr.GetClient(), @@ -140,7 +150,7 @@ func main() { } setupLog.Info("starting manager") - if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { + if err := mgr.Start(ctx); err != nil { setupLog.Error(err, "problem running manager") os.Exit(1) } diff --git a/role/role.go b/role/role.go deleted file mode 100644 index 44de75ba..00000000 --- a/role/role.go +++ /dev/null @@ -1,83 +0,0 @@ -package role - -import ( - "fmt" - "reflect" - - synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1" - rbacv1 "k8s.io/api/rbac/v1" -) - -func AddResourceNames(role *rbacv1.Role, name string) bool { - i := EnsureRules(role) - - c, _ := contains(role.Rules[i].ResourceNames, name) - if c { - return false - } - - role.Rules[i].ResourceNames = append(role.Rules[i].ResourceNames, name) - - return true -} - -func RemoveResourceNames(role *rbacv1.Role, name string) bool { - i, err := getRuleIndex(role) - if err != nil { - return false - } - - names := role.Rules[i].ResourceNames - - c, j := contains(names, name) - if !c { - return false - } - - // Remove element at position `i` assuming order is not relevant. - names[j] = names[len(names)-1] - role.Rules[i].ResourceNames = names[:len(names)-1] - - return true -} - -func EnsureRules(role *rbacv1.Role) int { - if role.Rules == nil { - role.Rules = []rbacv1.PolicyRule{} - } - i, err := getRuleIndex(role) - if err != nil { - rule := rbacv1.PolicyRule{ - APIGroups: []string{synv1alpha1.GroupVersion.Group}, - Verbs: []string{"get"}, - Resources: []string{"tenants", "clusters"}, - } - role.Rules = append(role.Rules, rule) - i = len(role.Rules) - 1 - } - - return i -} - -func getRuleIndex(role *rbacv1.Role) (int, error) { - for i, rule := range role.Rules { - matchGroups := reflect.DeepEqual(rule.APIGroups, []string{synv1alpha1.GroupVersion.Group}) - matchVerbs := reflect.DeepEqual(rule.Verbs, []string{"get"}) - matchTenants, _ := contains(rule.Resources, "tenants") - matchClusters, _ := contains(rule.Resources, "clusters") - if matchGroups && matchVerbs && matchTenants && matchClusters { - return i, nil - } - } - return 0, fmt.Errorf("no matching rule") -} - -func contains(items []string, value string) (bool, int) { - for i, item := range items { - if item == value { - return true, i - } - } - - return false, 0 -} diff --git a/role/role_test.go b/role/role_test.go deleted file mode 100644 index 2afd5b8e..00000000 --- a/role/role_test.go +++ /dev/null @@ -1,226 +0,0 @@ -package role_test - -import ( - "testing" - - synv1alpha1 "github.com/projectsyn/lieutenant-operator/api/v1alpha1" - roleUtil "github.com/projectsyn/lieutenant-operator/role" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - rbacv1 "k8s.io/api/rbac/v1" -) - -var addTests = map[string]struct { - name string - role *rbacv1.Role - updated bool - assert func(*testing.T, *rbacv1.Role) -}{ - "tenant name is added to ResourceName": { - name: "my-tenant", - role: &rbacv1.Role{}, - updated: true, - assert: func(t *testing.T, role *rbacv1.Role) { - require.Len(t, role.Rules, 1) - assert.Len(t, role.Rules[0].ResourceNames, 1) - assert.Contains(t, role.Rules[0].ResourceNames, "my-tenant") - }, - }, - "no changes": { - name: "my-tenant", - role: &rbacv1.Role{ - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{synv1alpha1.GroupVersion.Group}, - Verbs: []string{"get"}, - Resources: []string{"tenants", "clusters"}, - ResourceNames: []string{"my-tenant"}, - }, - }, - }, - updated: false, - assert: func(t *testing.T, role *rbacv1.Role) { - require.Len(t, role.Rules, 1) - assert.Len(t, role.Rules[0].ResourceNames, 1) - assert.Contains(t, role.Rules[0].ResourceNames, "my-tenant") - }, - }, - "other resource names remain untouched": { - name: "my-tenant", - role: &rbacv1.Role{ - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{synv1alpha1.GroupVersion.Group}, - Verbs: []string{"get"}, - Resources: []string{"tenants", "clusters"}, - ResourceNames: []string{"other-resource"}, - }, - }, - }, - updated: true, - assert: func(t *testing.T, role *rbacv1.Role) { - require.Len(t, role.Rules, 1) - assert.Len(t, role.Rules[0].ResourceNames, 2) - assert.Contains(t, role.Rules[0].ResourceNames, "my-tenant") - assert.Contains(t, role.Rules[0].ResourceNames, "other-resource") - }, - }, - "add new rule when no rule matches": { - name: "my-tenant", - role: &rbacv1.Role{ - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"some-group"}, - Verbs: []string{"update"}, - Resources: []string{"some-resource"}, - ResourceNames: []string{"before"}, - }, - }, - }, - updated: true, - assert: func(t *testing.T, role *rbacv1.Role) { - require.Len(t, role.Rules, 2) - assert.Len(t, role.Rules[1].ResourceNames, 1) - assert.Contains(t, role.Rules[1].ResourceNames, "my-tenant") - }, - }, - "the correct rule gets updated": { - name: "my-tenant", - role: &rbacv1.Role{ - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"some-group"}, - Verbs: []string{"update"}, - Resources: []string{"some-resource"}, - ResourceNames: []string{"before"}, - }, - { - APIGroups: []string{synv1alpha1.GroupVersion.Group}, - Verbs: []string{"get"}, - Resources: []string{"tenants", "clusters"}, - ResourceNames: []string{"other-resource"}, - }, - { - APIGroups: []string{"some-other-group"}, - Verbs: []string{"delete"}, - Resources: []string{"some-other-resource"}, - ResourceNames: []string{"after"}, - }, - }, - }, - updated: true, - assert: func(t *testing.T, role *rbacv1.Role) { - require.Len(t, role.Rules, 3) - assert.Len(t, role.Rules[1].ResourceNames, 2) - assert.Contains(t, role.Rules[1].ResourceNames, "my-tenant") - assert.Contains(t, role.Rules[1].ResourceNames, "other-resource") - }, - }, -} - -func TestAddResourceNames(t *testing.T) { - for name, tt := range addTests { - t.Run(name, func(t *testing.T) { - updated := roleUtil.AddResourceNames(tt.role, tt.name) - assert.Equal(t, tt.updated, updated) - tt.assert(t, tt.role) - }) - } -} - -var removeTests = map[string]struct { - name string - role *rbacv1.Role - updated bool - assert func(*testing.T, *rbacv1.Role) -}{ - "tenant name is remove from ResourceName": { - name: "my-tenant", - role: &rbacv1.Role{ - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{synv1alpha1.GroupVersion.Group}, - Verbs: []string{"get"}, - Resources: []string{"tenants", "clusters"}, - ResourceNames: []string{"my-tenant"}, - }, - }, - }, - updated: true, - assert: func(t *testing.T, role *rbacv1.Role) { - require.Len(t, role.Rules, 1) - assert.Len(t, role.Rules[0].ResourceNames, 0) - assert.NotContains(t, role.Rules[0].ResourceNames, "my-tenant") - }, - }, - "no changes": { - name: "my-tenant", - role: &rbacv1.Role{}, - updated: false, - assert: func(t *testing.T, role *rbacv1.Role) { - assert.Len(t, role.Rules, 0) - }, - }, - "other resource names remain untouched": { - name: "my-tenant", - role: &rbacv1.Role{ - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{synv1alpha1.GroupVersion.Group}, - Verbs: []string{"get"}, - Resources: []string{"tenants", "clusters"}, - ResourceNames: []string{"other-resource", "my-tenant"}, - }, - }, - }, - updated: true, - assert: func(t *testing.T, role *rbacv1.Role) { - require.Len(t, role.Rules, 1) - assert.Len(t, role.Rules[0].ResourceNames, 1) - assert.NotContains(t, role.Rules[0].ResourceNames, "my-tenant") - assert.Contains(t, role.Rules[0].ResourceNames, "other-resource") - }, - }, - "the correct rule gets updated": { - name: "my-tenant", - role: &rbacv1.Role{ - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"some-group"}, - Verbs: []string{"update"}, - Resources: []string{"some-resource"}, - ResourceNames: []string{"before"}, - }, - { - APIGroups: []string{synv1alpha1.GroupVersion.Group}, - Verbs: []string{"get"}, - Resources: []string{"tenants", "clusters"}, - ResourceNames: []string{"other-resource", "my-tenant"}, - }, - { - APIGroups: []string{"some-other-group"}, - Verbs: []string{"delete"}, - Resources: []string{"some-other-resource"}, - ResourceNames: []string{"after"}, - }, - }, - }, - updated: true, - assert: func(t *testing.T, role *rbacv1.Role) { - require.Len(t, role.Rules, 3) - assert.Len(t, role.Rules[1].ResourceNames, 1) - assert.NotContains(t, role.Rules[1].ResourceNames, "my-tenant") - assert.Contains(t, role.Rules[1].ResourceNames, "other-resource") - }, - }, -} - -func TestRemoveResourceNames(t *testing.T) { - for name, tt := range removeTests { - t.Run(name, func(t *testing.T) { - updated := roleUtil.RemoveResourceNames(tt.role, tt.name) - assert.Equal(t, tt.updated, updated) - tt.assert(t, tt.role) - }) - } -}