From f58effbce9bf62bff941d92f81775fdab89af1d0 Mon Sep 17 00:00:00 2001 From: dvir Date: Mon, 1 Apr 2024 12:34:16 +0000 Subject: [PATCH] Added permitted groups for MH and UQ This PR introduces a configmap containing a list of comma delimeted "permitted groups". Users in these groups will automatically be able to create an updateQuota or migrationHierarchy, even if they are not admins in the relevant namespaces. Also added test cases for this behavior. Bumped versions for k8s.api, gomega, and docker/login-command --- .github/workflows/post-merge.yaml | 2 +- .github/workflows/pr.yaml | 2 +- .github/workflows/release.yaml | 2 +- cmd/main.go | 3 + config/manager/kustomization.yaml | 6 ++ config/manager/manager.yaml | 7 +- config/rbac/role.yaml | 8 ++ go.mod | 8 +- go.sum | 20 ++-- internal/common/validators.go | 102 +++++++++++++++--- .../migrationhierarchy/createvalidator.go | 2 +- internal/updatequota/controller.go | 1 + internal/updatequota/createvalidator.go | 2 +- test/e2e_tests/migrationhierarchy_test.go | 4 + test/e2e_tests/updatequota_test.go | 20 ++++ test/testutils/composeutils.go | 30 +++++- test/testutils/testutils.go | 21 ++++ 17 files changed, 199 insertions(+), 41 deletions(-) diff --git a/.github/workflows/post-merge.yaml b/.github/workflows/post-merge.yaml index a6f11ac9..1067c862 100644 --- a/.github/workflows/post-merge.yaml +++ b/.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 }} diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index ee2a1c14..b1377e35 100644 --- a/.github/workflows/pr.yaml +++ b/.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 }} diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index e1143c9b..0c6ea2de 100644 --- a/.github/workflows/release.yaml +++ b/.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 }} diff --git a/cmd/main.go b/cmd/main.go index 0f7aa8df..7e6043cf 100644 --- a/cmd/main.go +++ b/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 } diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index ad13e96b..7001a7bf 100644 --- a/config/manager/kustomization.yaml +++ b/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 diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index bcbac55c..a6558502 100644 --- a/config/manager/manager.yaml +++ b/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 diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 79df2beb..a4e27ee7 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -197,3 +197,11 @@ rules: - patch - update - watch +- apiGroups: + - user.openshift.io + resources: + - groups + verbs: + - get + - list + - watch diff --git a/go.mod b/go.mod index d1d990b5..3e3093e2 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 4fe4dacc..ad40ff9e 100644 --- a/go.sum +++ b/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= diff --git a/internal/common/validators.go b/internal/common/validators.go index 56e82f88..ebd15a1a 100644 --- a/internal/common/validators.go +++ b/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 } diff --git a/internal/migrationhierarchy/createvalidator.go b/internal/migrationhierarchy/createvalidator.go index a1352ba1..8496f78a 100644 --- a/internal/migrationhierarchy/createvalidator.go +++ b/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 } diff --git a/internal/updatequota/controller.go b/internal/updatequota/controller.go index 6c719d10..97044c26 100644 --- a/internal/updatequota/controller.go +++ b/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 { diff --git a/internal/updatequota/createvalidator.go b/internal/updatequota/createvalidator.go index 53625bce..98b02e0d 100644 --- a/internal/updatequota/createvalidator.go +++ b/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 } diff --git a/test/e2e_tests/migrationhierarchy_test.go b/test/e2e_tests/migrationhierarchy_test.go index 88f79d98..eb3fbf4b 100644 --- a/test/e2e_tests/migrationhierarchy_test.go +++ b/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) }) + }) diff --git a/test/e2e_tests/updatequota_test.go b/test/e2e_tests/updatequota_test.go index 0fce0b39..eabf6642 100644 --- a/test/e2e_tests/updatequota_test.go +++ b/test/e2e_tests/updatequota_test.go @@ -15,6 +15,7 @@ var _ = Describe("UpdateQuota", func() { CleanupTestNamespaces(randPrefix) CleanupTestUsers(randPrefix) + CleanupTestGroup("test") nsRoot = GenerateE2EName("root", testPrefix, randPrefix) CreateRootNS(nsRoot, randPrefix, rqDepth) @@ -24,6 +25,8 @@ var _ = Describe("UpdateQuota", func() { AfterEach(func() { CleanupTestNamespaces(randPrefix) CleanupTestUsers(randPrefix) + CleanupTestGroup("test") + }) It("should move resources from the root namespace to a subnamespace", func() { @@ -220,4 +223,21 @@ var _ = Describe("UpdateQuota", func() { // verify after update FieldShouldContain("subnamespace", nsA, nsC, ".status.total.free.pods", "25") }) + It("should allow the creation of an updatequota with a user in a permitted group", func() { + nsA := GenerateE2EName("a", testPrefix, randPrefix) + nsB := GenerateE2EName("b", testPrefix, randPrefix) + CreateSubnamespace(nsA, nsRoot, randPrefix, false, storage, "50Gi", cpu, "50", memory, "50Gi", pods, "50", gpu, "50") + CreateSubnamespace(nsB, nsRoot, randPrefix, false, storage, "25Gi", cpu, "25", memory, "25Gi", pods, "25", gpu, "25") + + userA := GenerateE2EUserName("user-a") + + CreateUser(userA, randPrefix) + CreateGroup("test", userA, randPrefix) + GrantTestingUserAdmin(userA, nsA) + + CreateUpdateQuota("updatequota-from-"+nsA+"-to-"+nsB, nsA, nsB, userA, "pods", "10") + + FieldShouldContain("subnamespace", nsRoot, nsB, ".status.total.free.pods", "35") + + }) }) diff --git a/test/testutils/composeutils.go b/test/testutils/composeutils.go index a64773e8..3642bac5 100644 --- a/test/testutils/composeutils.go +++ b/test/testutils/composeutils.go @@ -43,9 +43,16 @@ func RandStr() string { // GenerateE2EUserName generates a name for a namespace and subnamespace. func GenerateE2EUserName(nm string) string { prefix := namespacePrefix + "-" + RandStr() + "-user-" - snsName := prefix + nm + userName := prefix + nm - return snsName + return userName +} + +func GenerateE2EGroupName(nm string) string { + prefix := namespacePrefix + "-" + RandStr() + "-group-" + groupName := prefix + nm + + return groupName } // GrantTestingUserAdmin gives admin rolebinding to a user on a namnamespace. @@ -180,6 +187,14 @@ func CreateUser(u, randPrefix string) { labelTestingUsers(u, randPrefix) } +// CreateGroup creates the specified Group. +func CreateGroup(g, u, randPrefix string) { + group := generateGroupManifest(g, u) + MustApplyYAML(group) + RunShouldContain(g, propagationTime, "kubectl get groups") + labelTestingGroup(g, randPrefix) +} + // CreatePod creates a pod in the specified namespace with the required cpu and memory(Gi). func CreatePod(ns, name, randPrefix, cpu, memory string) { pod := generatePodManifest(ns, name, cpu, memory) @@ -292,6 +307,17 @@ groups: - e2e-test` } +// generateGroupManifest generates an User manifest. +func generateGroupManifest(nm string, user string) string { + return `# temp file created by group_test.go +apiVersion: user.openshift.io/v1 +kind: Group +metadata: + name: ` + nm + ` +users: + - ` + user +} + // generatePodManifest generates an Pod manifest. func generatePodManifest(ns, name, cpu, memory string) string { return `# temp file created by pod_test.go diff --git a/test/testutils/testutils.go b/test/testutils/testutils.go index 40f2873a..05ed9095 100644 --- a/test/testutils/testutils.go +++ b/test/testutils/testutils.go @@ -16,6 +16,7 @@ import ( const testingNamespaceLabel = "dana.hns.io/testNamespace" const testingMigrationHierarchyLabel = "dana.hns.io/testMigrationHierarchy" const testingUserLabel = "dana.hns.io/testUser" +const testingGroupLabel = "dana.hns.io/testGroup" func FieldShouldContain(resource, ns, nm, field, want string) { fieldShouldContainMultipleWithTimeout(1, resource, ns, nm, field, []string{want}, eventuallyTimeout) @@ -219,6 +220,11 @@ func LabelTestingMigrationHierarchies(mh, randPrefix string) { MustRun("kubectl label --overwrite migrationhierarchy", mh, randPrefix+"-"+testingMigrationHierarchyLabel+"=true") } +// labelTestingGroups marks testing groups with a label for future search and lookup. +func labelTestingGroup(group, randPrefix string) { + MustRun("kubectl label --overwrite group", group, randPrefix+"-"+testingGroupLabel+"=true") +} + // labelTestingUsers marks testing users with a label for future search and lookup. func labelTestingUsers(user, randPrefix string) { MustRun("kubectl label --overwrite user", user, randPrefix+"-"+testingUserLabel+"=true") @@ -274,6 +280,15 @@ func CleanupTestUsers(randPrefix string) { cleanupUsers(users...) } +// CleanupTestGroups deletes a specific group +func CleanupTestGroup(groupName string) { + _, err := RunCommand("kubectl get groups -o custom-columns=:.metadata.name --no-headers=true", groupName) + if err != nil { + return + } + cleanupGroup(groupName) +} + // reverseSlice takes a slice and returns it in reverse order func reverseSlice(nss []string) []string { for i, j := 0, len(nss)-1; i < j; i, j = i+1, j-1 { @@ -340,6 +355,12 @@ func cleanupUsers(users ...string) { } } +// cleanupGroups does everything it can to delete the passed-in namespaces +func cleanupGroup(group string) { + + err := TryRun("kubectl delete group", group) + Expect(err).Should(BeNil()) +} func writeTempFile(cxt string) string { f, err := os.CreateTemp(os.TempDir(), "e2e-test-*.yaml") Expect(err).Should(BeNil())