group perms for MH and UQ

.github/workflows/post-merge.yaml

@@ -20,7 +20,7 @@ jobs:
         uses: actions/checkout@v4
 
       - name: Log in to the Container registry
-        uses: docker/login-action@5139682d94efc37792e6b54386b5b470a68a4737
+        uses: docker/login-action@5f4866a30a54f16a52d2ecb4a3898e9e424939cf
         with:
           registry: ${{ env.REGISTRY }}
           username: ${{ github.actor }}

.github/workflows/pr.yaml

@@ -34,7 +34,7 @@ jobs:
         run: go vet ./...
 
       - name: Log into ghcr.io
-        uses: docker/login-action@5139682d94efc37792e6b54386b5b470a68a4737
+        uses: docker/login-action@5f4866a30a54f16a52d2ecb4a3898e9e424939cf
         with:
           registry: ${{ env.REGISTRY }}
           username: ${{ github.actor }}

.github/workflows/release.yaml

@@ -17,7 +17,7 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v4
       - name: Log in to the Container registry
-        uses: docker/login-action@5139682d94efc37792e6b54386b5b470a68a4737
+        uses: docker/login-action@5f4866a30a54f16a52d2ecb4a3898e9e424939cf
         with:
           registry: ${{ env.REGISTRY }}
           username: ${{ github.actor }}

cmd/main.go

@@ -18,6 +18,8 @@ import (
 	"flag"
 	"os"
 
+	userv1 "github.com/openshift/api/user/v1"
+
 	danav1 "github.com/dana-team/hns/api/v1"
 	"github.com/dana-team/hns/internal/namespacedb"
 	"github.com/dana-team/hns/internal/setup"
@@ -55,6 +57,7 @@ func init() {
 	utilruntime.Must(quotav1.Install(scheme))
 	utilruntime.Must(templatev1.Install(scheme))
 	utilruntime.Must(buildv1.Install(scheme))
+	utilruntime.Must(userv1.Install(scheme))
 	// +kubebuilder:scaffold:scheme
 }
 

config/manager/kustomization.yaml

@@ -1,3 +1,5 @@
+generatorOptions:
+  disableNameSuffixHash: true
 resources:
 - manager.yaml
 apiVersion: kustomize.config.k8s.io/v1beta1
@@ -6,3 +8,7 @@ images:
 - name: controller
   newName: controller
   newTag: latest
+configMapGenerator:
+- literals:
+  - PERMITTED_GROUPS='test'
+  name: permitted-groups-cm

config/manager/manager.yaml

@@ -36,9 +36,12 @@ spec:
           name: manager
           resources:
             limits:
-              cpu: 1
+              cpu: "1"
               memory: 1Gi
             requests:
-              cpu: 1
+              cpu: "1"
               memory: 1Gi
+          envFrom:
+            - configMapRef:
+                name: permitted-groups-cm
       terminationGracePeriodSeconds: 10

config/rbac/role.yaml

@@ -197,3 +197,11 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - user.openshift.io
+  resources:
+  - groups
+  verbs:
+  - get
+  - list
+  - watch

go.mod

@@ -5,11 +5,11 @@ go 1.21.6
 require (
 	github.com/go-logr/logr v1.4.1
 	github.com/onsi/ginkgo/v2 v2.17.1
-	github.com/onsi/gomega v1.31.1
+	github.com/onsi/gomega v1.32.0
 	github.com/openshift/api v0.0.0-20240111155829-b6df9ba0be95
 	golang.org/x/exp v0.0.0-20240112132812-db7319d0e0e3
-	k8s.io/api v0.29.2
-	k8s.io/apimachinery v0.29.2
+	k8s.io/api v0.29.3
+	k8s.io/apimachinery v0.29.3
 	k8s.io/client-go v0.29.2
 	sigs.k8s.io/controller-runtime v0.17.2
 )
@@ -28,7 +28,7 @@ require (
 	github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
-	github.com/golang/protobuf v1.5.3 // indirect
+	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/gnostic-models v0.6.8 // indirect
 	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect

go.sum

@@ -35,12 +35,10 @@ github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69
 github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da h1:oI5xCqsCo564l8iNU+DwB5epxmsaqB+rhGL0m5jtYqE=
 github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
 github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
-github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk=
-github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg=
-github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY=
+github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
+github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
 github.com/google/gnostic-models v0.6.8 h1:yo/ABAfM5IMRsS1VnXjTBvUb61tFIHozhlYvRgGre9I=
 github.com/google/gnostic-models v0.6.8/go.mod h1:5n7qKqH0f5wFt+aWF8CW6pZLLNOfYuF5OpfBSENuI8U=
-github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
 github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
@@ -80,8 +78,8 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/onsi/ginkgo/v2 v2.17.1 h1:V++EzdbhI4ZV4ev0UTIj0PzhzOcReJFyJaLjtSF55M8=
 github.com/onsi/ginkgo/v2 v2.17.1/go.mod h1:llBI3WDLL9Z6taip6f33H76YcWtJv+7R3HigUjbIBOs=
-github.com/onsi/gomega v1.31.1 h1:KYppCUK+bUgAZwHOu7EXVBKyQA6ILvOESHkn/tgoqvo=
-github.com/onsi/gomega v1.31.1/go.mod h1:y40C95dwAD1Nz36SsEnxvfFe8FFfNxzI5eJ0EYGyAy0=
+github.com/onsi/gomega v1.32.0 h1:JRYU78fJ1LPxlckP6Txi/EYqJvjtMrDC04/MM5XRHPk=
+github.com/onsi/gomega v1.32.0/go.mod h1:a4x4gW6Pz2yK1MAmvluYme5lvYTn61afQ2ETw/8n4Lg=
 github.com/openshift/api v0.0.0-20240111155829-b6df9ba0be95 h1:j0eYo5xy/lW+oeLvpbtJBNc7bvbiuJaW7ZXBoQ8gsjw=
 github.com/openshift/api v0.0.0-20240111155829-b6df9ba0be95/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
@@ -166,8 +164,6 @@ gomodules.xyz/jsonpatch/v2 v2.4.0 h1:Ci3iUJyx9UeRx7CeFN8ARgGbkESwJK+KB9lLcWxY/Zw
 gomodules.xyz/jsonpatch/v2 v2.4.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY=
 google.golang.org/appengine v1.6.7 h1:FZR1q0exgwxzPzp/aF+VccGrSfxfPpkBqjIIEq3ru6c=
 google.golang.org/appengine v1.6.7/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc=
-google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
-google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
 google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI=
 google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
@@ -181,12 +177,12 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-k8s.io/api v0.29.2 h1:hBC7B9+MU+ptchxEqTNW2DkUosJpp1P+Wn6YncZ474A=
-k8s.io/api v0.29.2/go.mod h1:sdIaaKuU7P44aoyyLlikSLayT6Vb7bvJNCX105xZXY0=
+k8s.io/api v0.29.3 h1:2ORfZ7+bGC3YJqGpV0KSDDEVf8hdGQ6A03/50vj8pmw=
+k8s.io/api v0.29.3/go.mod h1:y2yg2NTyHUUkIoTC+phinTnEa3KFM6RZ3szxt014a80=
 k8s.io/apiextensions-apiserver v0.29.0 h1:0VuspFG7Hj+SxyF/Z/2T0uFbI5gb5LRgEyUVE3Q4lV0=
 k8s.io/apiextensions-apiserver v0.29.0/go.mod h1:TKmpy3bTS0mr9pylH0nOt/QzQRrW7/h7yLdRForMZwc=
-k8s.io/apimachinery v0.29.2 h1:EWGpfJ856oj11C52NRCHuU7rFDwxev48z+6DSlGNsV8=
-k8s.io/apimachinery v0.29.2/go.mod h1:6HVkd1FwxIagpYrHSwJlQqZI3G9LfYWRPAkUvLnXTKU=
+k8s.io/apimachinery v0.29.3 h1:2tbx+5L7RNvqJjn7RIuIKu9XTsIZ9Z5wX2G22XAa5EU=
+k8s.io/apimachinery v0.29.3/go.mod h1:hx/S4V2PNW4OMg3WizRrHutyB5la0iCUbZym+W0EQIU=
 k8s.io/client-go v0.29.2 h1:FEg85el1TeZp+/vYJM7hkDlSTFZ+c5nnK44DJ4FyoRg=
 k8s.io/client-go v0.29.2/go.mod h1:knlvFZE58VpqbQpJNbCbctTVXcd35mMyAAwBdpt4jrA=
 k8s.io/component-base v0.29.0 h1:T7rjd5wvLnPBV1vC4zWd/iWRbV8Mdxs+nGaoaFzGw3s=

internal/common/validators.go

@@ -4,20 +4,28 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"os"
+	"slices"
+	"strings"
 
 	danav1 "github.com/dana-team/hns/api/v1"
-	"github.com/dana-team/hns/internal/namespace/nsutils"
-	"github.com/dana-team/hns/internal/objectcontext"
 	authv1 "k8s.io/api/authorization/v1"
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
+
+	"github.com/dana-team/hns/internal/namespace/nsutils"
+	"github.com/dana-team/hns/internal/objectcontext"
+	userv1 "github.com/openshift/api/user/v1"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
+const PERMITTEDGROUPLABEL = "PERMITTED_GROUPS"
+
 // ValidateNamespaceExist validates that a namespace exists.
 func ValidateNamespaceExist(ns *objectcontext.ObjectContext) admission.Response {
 	if !(ns.IsPresent()) {
@@ -78,14 +86,31 @@ func ValidateSecondaryRoot(ctx context.Context, c client.Client, aNSArray, bNSAr
 }
 
 // ValidatePermissions checks if a registered user has the needed permissions on the namespaces and denies otherwise
-// there are 3 scenarios in which things are allowed: if the user has the needed permissions on the Ancestor
+// there are 4 scenarios in which things are allowed: if the user is in a permitted group; if the user has the needed permissions on the Ancestor
 // of the two namespaces; if the user has the needed permissions on both namespaces; if the user has the needed
 // permissions on the namespace from which resources are moved and both namespaces are in the same branch
 // (only checked when the branch flag is true).
-func ValidatePermissions(ctx context.Context, aNS []string, aNSName, bNSName, ancestorNSName, reqUser string, branch bool) admission.Response {
-	hasSourcePermissions := permissionsExist(ctx, reqUser, aNSName)
-	hasDestPermissions := permissionsExist(ctx, reqUser, bNSName)
-	hasAncestorPermissions := permissionsExist(ctx, reqUser, ancestorNSName)
+func ValidatePermissions(ctx context.Context, aNS []string, aNSName, bNSName, ancestorNSName, reqUser string, branch bool, k8sClient client.Client) admission.Response {
+	inGroup, err := ValidatePermittedGroups(ctx, reqUser, k8sClient)
+	if err != nil {
+		return admission.Errored(http.StatusInternalServerError, err)
+	}
+	if inGroup {
+		return admission.Allowed("")
+	}
+
+	hasSourcePermissions, err := permissionsExist(ctx, reqUser, aNSName)
+	if err != nil {
+		return admission.Errored(http.StatusInternalServerError, err)
+	}
+	hasDestPermissions, err := permissionsExist(ctx, reqUser, bNSName)
+	if err != nil {
+		return admission.Errored(http.StatusInternalServerError, err)
+	}
+	hasAncestorPermissions, err := permissionsExist(ctx, reqUser, ancestorNSName)
+	if err != nil {
+		return admission.Errored(http.StatusInternalServerError, err)
+	}
 
 	inBranch := ContainsString(aNS, bNSName)
 
@@ -110,14 +135,15 @@ func ValidatePermissions(ctx context.Context, aNS []string, aNSName, bNSName, an
 // permissionsExist checks if a user has permission to create a pod in a given namespace.
 // It impersonates the reqUser and uses SelfSubjectAccessReview API to check if the action is allowed or denied.
 // It returns a boolean value indicating whether the user has permission to create the pod or not.
-func permissionsExist(ctx context.Context, reqUser, namespace string) bool {
+func permissionsExist(ctx context.Context, reqUser, namespace string) (bool, error) {
+
 	if reqUser == fmt.Sprintf("system:serviceaccount:%s:%s", danav1.SNSNamespace, danav1.SNSServiceAccount) {
-		return true
+		return true, nil
 	}
 
 	config, err := rest.InClusterConfig()
 	if err != nil {
-		panic(err.Error())
+		return false, err
 	}
 
 	// set the user to impersonate in the configuration
@@ -128,7 +154,7 @@ func permissionsExist(ctx context.Context, reqUser, namespace string) bool {
 	// create a new Kubernetes client using the configuration
 	clientSet, err := kubernetes.NewForConfig(config)
 	if err != nil {
-		panic(err.Error())
+		return false, err
 	}
 
 	// create a new SelfSubjectAccessReview API object for checking permissions
@@ -147,15 +173,59 @@ func permissionsExist(ctx context.Context, reqUser, namespace string) bool {
 	// check the permissions for the user
 	resp, err := clientSet.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, &selfCheck, metav1.CreateOptions{})
 	if err != nil {
-		panic(err.Error())
+		return false, err
 	}
 
 	// check the response status to determine whether the user has permission to create the pod or not
 	if resp.Status.Denied {
-		return false
+		return false, nil
 	}
 	if resp.Status.Allowed {
-		return true
+		return true, nil
+	}
+	return false, nil
+}
+
+// IsUserInGroup returns true if given user is in give group
+func IsUserInGroup(user string, group userv1.Group) bool {
+	return slices.Contains(group.Users, user)
+}
+
+// CheckGroup accepts groupname and username. Fetches group and checks if user is int it.
+func CheckGroup(ctx context.Context, user, groupName string, k8sClient client.Client) (bool, error) {
+	group := userv1.Group{}
+	err := k8sClient.Get(ctx, types.NamespacedName{Name: groupName}, &group)
+	if err != nil {
+		return false, err
+	} else {
+		if inGroup := IsUserInGroup(user, group); inGroup {
+			return true, nil
+		}
 	}
-	return false
+	return false, nil
+}
+
+// ValidatePermittedGroups validate if user is in a permitted group
+func ValidatePermittedGroups(ctx context.Context, user string, k8sClient client.Client) (bool, error) {
+	logger := log.FromContext(ctx)
+
+	permittedGroups, found := os.LookupEnv(PERMITTEDGROUPLABEL)
+	if !found {
+		logger.Info("no permitted groups found")
+	} else {
+		permittedGroupsSlice := strings.Split(permittedGroups, ",")
+		for _, groupName := range permittedGroupsSlice {
+			inGroup, err := CheckGroup(ctx, user, groupName, k8sClient)
+			if err != nil {
+				logger.Info(fmt.Sprintf("group %s not found", groupName))
+				return false, nil
+			}
+			if inGroup {
+				logger.Info(fmt.Sprintf("user %s found in group %s", user, groupName))
+				return true, nil
+			}
+		}
+	}
+
+	return false, nil
 }

internal/migrationhierarchy/createvalidator.go

@@ -73,7 +73,7 @@ func (v *MigrationHierarchyValidator) handleCreate(mhObject *objectcontext.Objec
 		}
 	}
 
-	if response := common.ValidatePermissions(ctx, currentNSSliced, currentNSName, toNSName, ancestorNSName, reqUser, false); !response.Allowed {
+	if response := common.ValidatePermissions(ctx, currentNSSliced, currentNSName, toNSName, ancestorNSName, reqUser, false, v.Client); !response.Allowed {
 		return response
 	}
 

internal/updatequota/controller.go

@@ -29,6 +29,7 @@ type UpdateQuotaReconciler struct {
 
 // +kubebuilder:rbac:groups=dana.hns.io,resources=updatequota,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=dana.hns.io,resources=updatequota/status,verbs=get;update;patch
+// +kubebuilder:rbac:groups=user.openshift.io,resources=groups,verbs=get;list;watch
 // +kubebuilder:rbac:groups="",resources=users,verbs=impersonate
 
 func (r *UpdateQuotaReconciler) SetupWithManager(mgr ctrl.Manager) error {

internal/updatequota/createvalidator.go

@@ -61,7 +61,7 @@ func (v *UpdateQuotaValidator) handleCreate(upqObject *objectcontext.ObjectConte
 		}
 	}
 
-	if response := common.ValidatePermissions(ctx, sourceNSSliced, sourceNSName, destNSName, ancestorNSName, username, true); !response.Allowed {
+	if response := common.ValidatePermissions(ctx, sourceNSSliced, sourceNSName, destNSName, ancestorNSName, username, true, v.Client); !response.Allowed {
 		return response
 	}
 

test/e2e_tests/migrationhierarchy_test.go

@@ -16,6 +16,7 @@ var _ = Describe("MigrationHierarchy", func() {
 
 		CleanupTestNamespaces(randPrefix)
 		CleanupTestMigrationHierarchies(randPrefix)
+		CleanupTestGroup("test")
 
 		nsRoot = GenerateE2EName("root", testPrefix, randPrefix)
 		CreateRootNS(nsRoot, randPrefix, rqDepth)
@@ -25,6 +26,8 @@ var _ = Describe("MigrationHierarchy", func() {
 	AfterEach(func() {
 		CleanupTestNamespaces(randPrefix)
 		CleanupTestMigrationHierarchies(randPrefix)
+		CleanupTestGroup("test")
+
 	})
 
 	It("should migrate subnamespace that have a CRQ or its direct parent have a CRQ,", func() {
@@ -441,4 +444,5 @@ var _ = Describe("MigrationHierarchy", func() {
 		// make sure the subnamespace was not migrated and the parent has not been updated
 		ShouldNotCreateMigrationHierarchy(nsA, nsC)
 	})
+
 })