diff --git a/Makefile b/Makefile index 32611d7f..d55fdc51 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,8 @@ all: build ## Invokes the build target .PHONY: test test: ## Run tests - go test ./... -coverprofile cover.out + go test ./... -coverprofile cover.tmp.out + cat cover.tmp.out | grep -v "zz_generated.deepcopy.go" > cover.out .PHONY: build build: generate fmt vet $(BIN_FILENAME) ## Build manager binary diff --git a/config/rbac/controller/role.yaml b/config/rbac/controller/role.yaml index 67dadea4..a45b2b91 100644 --- a/config/rbac/controller/role.yaml +++ b/config/rbac/controller/role.yaml @@ -12,6 +12,36 @@ rules: verbs: - create - patch +- apiGroups: + - appuio.io + resources: + - organizationmembers + verbs: + - get + - list + - patch + - update + - watch +- apiGroups: + - appuio.io + resources: + - organizationmembers/status + verbs: + - get + - patch + - update +- apiGroups: + - appuio.io + resources: + - teams + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - appuio.io resources: @@ -30,6 +60,30 @@ rules: - get - patch - update +- apiGroups: + - organization.appuio.io + resources: + - organizations + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - rbac.appuio.io + resources: + - organizations + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - rbac.authorization.k8s.io resources: @@ -42,3 +96,15 @@ rules: - patch - update - watch +- apiGroups: + - rbac.authorization.k8s.io + resources: + - rolebindings + verbs: + - create + - delete + - get + - list + - patch + - update + - watch diff --git a/controller.go b/controller.go index 578c2c3d..6ef6d76e 100644 --- a/controller.go +++ b/controller.go @@ -42,6 +42,7 @@ func ControllerCommand() *cobra.Command { probeAddr := cmd.Flags().String("health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") usernamePrefix := cmd.Flags().String("username-prefix", "", "Prefix prepended to username claims. Usually the same as \"--oidc-username-prefix\" of the Kubernetes API server") rolePrefix := cmd.Flags().String("role-prefix", "control-api:user:", "Prefix prepended to generated cluster roles and bindings to prevent name collisions.") + memberRoles := cmd.Flags().StringSlice("member-roles", []string{}, "ClusterRoles to assign to every organization member for its namespace") cmd.Run = func(*cobra.Command, []string) { scheme := runtime.NewScheme() @@ -58,6 +59,7 @@ func ControllerCommand() *cobra.Command { mgr, err := setupManager( *usernamePrefix, *rolePrefix, + *memberRoles, ctrl.Options{ Scheme: scheme, MetricsBindAddress: *metricsAddr, @@ -81,7 +83,7 @@ func ControllerCommand() *cobra.Command { return cmd } -func setupManager(usernamePrefix, rolePrefix string, opt ctrl.Options) (ctrl.Manager, error) { +func setupManager(usernamePrefix, rolePrefix string, memberRoles []string, opt ctrl.Options) (ctrl.Manager, error) { mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), opt) if err != nil { return nil, err @@ -98,6 +100,19 @@ func setupManager(usernamePrefix, rolePrefix string, opt ctrl.Options) (ctrl.Man if err = ur.SetupWithManager(mgr); err != nil { return nil, err } + if len(memberRoles) > 0 { + omr := &controllers.OrganizationMembersReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("organization-members-controller"), + + UserPrefix: usernamePrefix, + MemberRoles: memberRoles, + } + if err = omr.SetupWithManager(mgr); err != nil { + return nil, err + } + } //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { diff --git a/controllers/organization_members_controller.go b/controllers/organization_members_controller.go new file mode 100644 index 00000000..69b717ea --- /dev/null +++ b/controllers/organization_members_controller.go @@ -0,0 +1,100 @@ +package controllers + +import ( + "context" + + "go.uber.org/multierr" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + controlv1 "github.com/appuio/control-api/apis/v1" +) + +// OrganizationMembersReconciler reconciles OrganizationMembers resources +type OrganizationMembersReconciler struct { + client.Client + Recorder record.EventRecorder + Scheme *runtime.Scheme + + // UserPrefix is the prefix applied to the user in the RoleBinding.subjects.name. + UserPrefix string + MemberRoles []string +} + +//+kubebuilder:rbac:groups=appuio.io,resources=organizationmembers,verbs=get;list;watch;update;patch +//+kubebuilder:rbac:groups=appuio.io,resources=organizationmembers/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch + +// Needed so that we are allowed to delegate common member roles +//+kubebuilder:rbac:groups="rbac.appuio.io",resources=organizations,verbs=get;list;watch;create;delete;patch;update +//+kubebuilder:rbac:groups="organization.appuio.io",resources=organizations,verbs=get;list;watch;create;delete;patch;update +//+kubebuilder:rbac:groups="appuio.io",resources=teams,verbs=get;list;watch;create;delete;patch;update +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=get;list;watch;create;update;patch;delete + +// Reconcile reacts on changes of users and mirrors these changes to Keycloak +func (r *OrganizationMembersReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := log.FromContext(ctx) + log.V(4).WithValues("request", req).Info("Reconciling") + + memb := controlv1.OrganizationMembers{} + if err := r.Get(ctx, req.NamespacedName, &memb); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + if !memb.ObjectMeta.DeletionTimestamp.IsZero() { + return ctrl.Result{}, nil + } + + var errGroup error + for _, role := range r.MemberRoles { + err := r.putRoleBinding(ctx, memb, role) + if err != nil { + errGroup = multierr.Append(errGroup, err) + r.Recorder.Event(&memb, "Warning", "RBACUpdateFailed", "Failed to set RBAC for Organization members") + } + } + + return ctrl.Result{}, errGroup +} + +func (r *OrganizationMembersReconciler) putRoleBinding(ctx context.Context, memb controlv1.OrganizationMembers, role string) error { + rb := rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: role, + Namespace: memb.Namespace, + }, + } + op, err := ctrl.CreateOrUpdate(ctx, r.Client, &rb, func() error { + sub := make([]rbacv1.Subject, len(memb.Spec.UserRefs)) + for i, ur := range memb.Spec.UserRefs { + sub[i] = rbacv1.Subject{ + APIGroup: rbacv1.GroupName, + Kind: "User", + Name: r.UserPrefix + ur.Name, + } + } + rb.Subjects = sub + rb.RoleRef = rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: role, + } + return ctrl.SetControllerReference(&memb, &rb, r.Scheme) + }) + log.FromContext(ctx).V(4).Info("reconcile RoleBinding", "operation", op) + return err +} + +// SetupWithManager sets up the controller with the Manager. +func (r *OrganizationMembersReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&controlv1.OrganizationMembers{}). + Owns(&rbacv1.RoleBinding{}). + Complete(r) +} diff --git a/controllers/organization_members_controller_test.go b/controllers/organization_members_controller_test.go new file mode 100644 index 00000000..36aa4ad4 --- /dev/null +++ b/controllers/organization_members_controller_test.go @@ -0,0 +1,172 @@ +package controllers_test + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + controlv1 "github.com/appuio/control-api/apis/v1" + . "github.com/appuio/control-api/controllers" +) + +var testMemb = controlv1.OrganizationMembers{ + ObjectMeta: metav1.ObjectMeta{ + Name: "members", + Namespace: "foo-gmbh", + }, + Spec: controlv1.OrganizationMembersSpec{ + UserRefs: []controlv1.UserRef{ + {Name: "u1"}, + {Name: "u2"}, + {Name: "u3"}, + }, + }, + Status: controlv1.OrganizationMembersStatus{ + ResolvedUserRefs: []controlv1.UserRef{ + {Name: "u1"}, + {Name: "u2"}, + {Name: "u3"}, + }, + }, +} + +var testUserPrefix = "control-api#" + +func Test_OrganizationMembersReconciler_Reconcile_Sucess(t *testing.T) { + ctx := context.Background() + c := prepareTest(t, &testMemb) + fakeRecorder := record.NewFakeRecorder(3) + membRoles := []string{"foo", "bar", "buzz"} + + _, err := (&OrganizationMembersReconciler{ + Client: c, + Scheme: c.Scheme(), + Recorder: fakeRecorder, + + MemberRoles: membRoles, + UserPrefix: testUserPrefix, + }).Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: testMemb.Name, + Namespace: testMemb.Namespace, + }, + }) + require.NoError(t, err) + + for _, role := range membRoles { + testRoleExists(t, c, role, testUserPrefix, testMemb) + } +} + +func Test_OrganizationMembersReconciler_Reconcile_PartialFailure(t *testing.T) { + ctx := context.Background() + c := failingClient{prepareTest(t, &testMemb)} + fakeRecorder := record.NewFakeRecorder(3) + membRoles := []string{"foo", "fail-bar", "buzz"} + + _, err := (&OrganizationMembersReconciler{ + Client: c, + Scheme: c.Scheme(), + Recorder: fakeRecorder, + + MemberRoles: membRoles, + UserPrefix: testUserPrefix, + }).Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: testMemb.Name, + Namespace: testMemb.Namespace, + }, + }) + assert.Error(t, err) + + for _, role := range []string{"foo", "buzz"} { + testRoleExists(t, c, role, testUserPrefix, testMemb) + } + rb := rbacv1.RoleBinding{} + assert.Errorf(t, c.Get(ctx, types.NamespacedName{Name: "fail-bar", Namespace: testMemb.Namespace}, &rb), "don't create bar") + require.Len(t, fakeRecorder.Events, 1) +} + +func Test_OrganizationMembersReconciler_Reconcile_MissmatchStatus(t *testing.T) { + ctx := context.Background() + membRoles := []string{"foo", "bar", "buzz"} + memb := testMemb + memb.Status.ResolvedUserRefs = []controlv1.UserRef{ + {Name: "u1"}, + {Name: "u3"}, + } + memb.Spec.UserRefs = []controlv1.UserRef{ + {Name: "u2"}, + {Name: "u3"}, + } + + c := prepareTest(t, &memb) + fakeRecorder := record.NewFakeRecorder(3) + + _, err := (&OrganizationMembersReconciler{ + Client: c, + Scheme: c.Scheme(), + Recorder: fakeRecorder, + + MemberRoles: membRoles, + UserPrefix: testUserPrefix, + }).Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: memb.Name, + Namespace: memb.Namespace, + }, + }) + assert.NoError(t, err) + + for _, role := range []string{"foo", "bar", "buzz"} { + testRoleExists(t, c, role, testUserPrefix, memb) + } +} + +func testRoleExists(t *testing.T, c client.WithWatch, role, userPrefix string, memb controlv1.OrganizationMembers) { + t.Run(role+" exists", func(t *testing.T) { + rb := rbacv1.RoleBinding{} + require.NoError(t, c.Get(context.TODO(), types.NamespacedName{Name: role, Namespace: memb.Namespace}, &rb)) + + assert.Equal(t, rb.RoleRef, rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: role, + }) + users := memb.Spec.UserRefs + require.Len(t, rb.Subjects, len(users)) + for _, u := range users { + assert.Contains(t, rb.Subjects, rbacv1.Subject{ + APIGroup: rbacv1.GroupName, + Kind: "User", + Name: userPrefix + u.Name, + }) + } + + require.Len(t, rb.OwnerReferences, 1, "controller must set owner reference") + assert.Equal(t, memb.Name, rb.OwnerReferences[0].Name, "owner reference name must match") + }) +} + +type failingClient struct { + client.WithWatch +} + +func (c failingClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if strings.HasPrefix(obj.GetName(), "fail-") { + return apierrors.NewInternalError(errors.New("ups")) + } + return c.WithWatch.Create(ctx, obj, opts...) +}