Merge pull request #899 from equinor/fix/use-default-adgroup-when-not…

…-set Use default ad group when not set in RR
equinor · Aug 1, 2023 · c603f28 · c603f28
2 parents dab5f1e + 068c28e
commit c603f28
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 16 deletions.
diff --git a/charts/radix-operator/Chart.yaml b/charts/radix-operator/Chart.yaml
@@ -1,7 +1,7 @@
 apiVersion: v2
 name: radix-operator
-version: 1.20.0
-appVersion: 1.40.0
+version: 1.20.1
+appVersion: 1.40.1
 kubeVersion: ">=1.24.0"
 description: Radix Operator
 keywords:

diff --git a/pkg/apis/application/application_test.go b/pkg/apis/application/application_test.go
@@ -14,6 +14,7 @@ import (
 	radixclient "github.com/equinor/radix-operator/pkg/client/clientset/versioned"
 	fakeradix "github.com/equinor/radix-operator/pkg/client/clientset/versioned/fake"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -247,21 +248,27 @@ func TestOnSync_RegistrationCreated_AppNamespaceReconciled(t *testing.T) {
 func TestOnSync_NoUserGroupDefined_DefaultUserGroupSet(t *testing.T) {
 	// Setup
 	tu, client, kubeUtil, radixClient := setupTest()
+	defaultRole := "9876-54321-09876"
 	defer os.Clearenv()
-	os.Setenv(defaults.OperatorDefaultUserGroupEnvironmentVariable, "9876-54321-09876")
+	os.Setenv(defaults.OperatorDefaultUserGroupEnvironmentVariable, defaultRole)
 
 	// Test
 	applyRegistrationWithSync(tu, client, kubeUtil, radixClient, utils.ARadixRegistration().
 		WithName("any-app").
-		WithAdGroups([]string{}))
+		WithAdGroups([]string{}).
+		WithReaderAdGroups([]string{}))
 
 	rolebindings, _ := client.RbacV1().RoleBindings("any-app-app").List(context.TODO(), metav1.ListOptions{})
 	assert.Equal(t, 5, len(rolebindings.Items))
 	assert.True(t, roleBindingByNameExists(defaults.AppAdminRoleName, rolebindings))
 	assert.True(t, roleBindingByNameExists(defaults.PipelineAppRoleName, rolebindings))
 	assert.True(t, roleBindingByNameExists(defaults.RadixTektonAppRoleName, rolebindings))
-	assert.Equal(t, "9876-54321-09876", getRoleBindingByName(defaults.AppAdminRoleName, rolebindings).Subjects[0].Name)
+	assert.Equal(t, defaultRole, getRoleBindingByName(defaults.AppAdminRoleName, rolebindings).Subjects[0].Name)
 
+	clusterRoleBindings, _ := client.RbacV1().ClusterRoleBindings().List(context.Background(), metav1.ListOptions{})
+	require.Len(t, getClusterRoleBindingByName("radix-platform-user-rr-any-app", clusterRoleBindings).Subjects, 1)
+	assert.Equal(t, defaultRole, getClusterRoleBindingByName("radix-platform-user-rr-any-app", clusterRoleBindings).Subjects[0].Name)
+	assert.Len(t, getClusterRoleBindingByName("radix-platform-user-rr-reader-any-app", clusterRoleBindings).Subjects, 0)
 }
 
 func TestOnSync_LimitsDefined_LimitsSet(t *testing.T) {

diff --git a/pkg/apis/application/rolebinding.go b/pkg/apis/application/rolebinding.go
@@ -2,6 +2,7 @@ package application
 
 import (
 	"fmt"
+
 	"github.com/equinor/radix-operator/pkg/apis/defaults"
 	"github.com/equinor/radix-operator/pkg/apis/defaults/k8s"
 	"github.com/equinor/radix-operator/pkg/apis/kube"
@@ -46,31 +47,34 @@ func (app Application) applyRbacAppNamespace() error {
 
 // ApplyRbacRadixRegistration Grants access to radix registration
 func (app Application) applyRbacRadixRegistration() error {
-	k := app.kubeutil
-
 	rr := app.registration
 	appName := rr.Name
 
+	// Admin RBAC
 	clusterRoleName := fmt.Sprintf("radix-platform-user-rr-%s", appName)
-	clusterRoleReaderName := fmt.Sprintf("radix-platform-user-rr-reader-%s", appName)
-
 	adminClusterRole := app.rrClusterRole(clusterRoleName, []string{"get", "list", "watch", "update", "patch", "delete"})
-	appAdminSubjects := getAppAdminSubjects(rr)
+	appAdminSubjects, err := getAppAdminSubjects(rr)
+	if err != nil {
+		return err
+	}
 	adminClusterRoleBinding := app.rrClusterroleBinding(adminClusterRole, appAdminSubjects)
 
+	// Reader RBAC
+	clusterRoleReaderName := fmt.Sprintf("radix-platform-user-rr-reader-%s", appName)
 	readerClusterRole := app.rrClusterRole(clusterRoleReaderName, []string{"get", "list", "watch"})
 	appReaderSubjects := kube.GetRoleBindingGroups(rr.Spec.ReaderAdGroups)
 	readerClusterRoleBinding := app.rrClusterroleBinding(readerClusterRole, appReaderSubjects)
 
+	// Apply roles and bindings
 	for _, clusterRole := range []*auth.ClusterRole{adminClusterRole, readerClusterRole} {
-		err := k.ApplyClusterRole(clusterRole)
+		err := app.kubeutil.ApplyClusterRole(clusterRole)
 		if err != nil {
 			return err
 		}
 	}
 
 	for _, clusterRoleBindings := range []*auth.ClusterRoleBinding{adminClusterRoleBinding, readerClusterRoleBinding} {
-		err := k.ApplyClusterRoleBinding(clusterRoleBindings)
+		err := app.kubeutil.ApplyClusterRoleBinding(clusterRoleBindings)
 		if err != nil {
 			return err
 		}
@@ -79,17 +83,20 @@ func (app Application) applyRbacRadixRegistration() error {
 	return nil
 }
 
-func getAppAdminSubjects(rr *v1.RadixRegistration) []auth.Subject {
-	subjects := kube.GetRoleBindingGroups(rr.Spec.AdGroups)
-
+func getAppAdminSubjects(rr *v1.RadixRegistration) ([]auth.Subject, error) {
+	adGroups, err := utils.GetAdGroups(rr)
+	if err != nil {
+		return nil, err
+	}
+	subjects := kube.GetRoleBindingGroups(adGroups)
 	if rr.Spec.MachineUser {
 		subjects = append(subjects, auth.Subject{
 			Kind:      "ServiceAccount",
 			Name:      defaults.GetMachineUserRoleName(rr.Name),
 			Namespace: utils.GetAppNamespace(rr.Name),
 		})
 	}
-	return subjects
+	return subjects, nil
 }
 
 // ApplyRbacOnPipelineRunner Grants access to radix pipeline