From a0396d3e3697dccadec009b20d671d0635626866 Mon Sep 17 00:00:00 2001 From: Andrew Obuchowicz Date: Fri, 17 Feb 2023 13:32:51 -0500 Subject: [PATCH 1/2] Revert "fix: check pods for unschedulable condition" This reverts commit 0cad9a076dc13cacad5f681b79b8dbdb6aeb7c6d. Fix #1046 Signed-off-by: Andrew Obuchowicz (cherry picked from commit 4aa1755f1fe8678f18b7d0b6c60ca732bcc1bca0) --- pkg/library/status/check.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/pkg/library/status/check.go b/pkg/library/status/check.go index 1836e8b40..3697eed9e 100644 --- a/pkg/library/status/check.go +++ b/pkg/library/status/check.go @@ -96,11 +96,6 @@ func CheckPodsState(workspaceID string, namespace string, labelSelector k8sclien if msg, err := CheckPodEvents(&pod, workspaceID, ignoredEvents, clusterAPI); err != nil || msg != "" { return msg, err } - if pod.Status.Phase == corev1.PodPending { - if msg := checkPodConditions(&pod); msg != "" { - return msg, nil - } - } } return "", nil } @@ -170,18 +165,6 @@ func checkIfUnrecoverableEventIgnored(reason string, ignoredEvents []string) (ig return false } -func checkPodConditions(pod *corev1.Pod) (msg string) { - if pod.Status.Conditions != nil { - for _, condition := range pod.Status.Conditions { - // Pod unschedulable condition - if condition.Type == corev1.PodScheduled && condition.Status == corev1.ConditionFalse && condition.Reason == corev1.PodReasonUnschedulable { - return fmt.Sprintf("Pod is unschedulable: %s", condition.Message) - } - } - } - return "" -} - // Returns the number of times an event has occurred. // This function exists for the following reasons: // From 2bc150af22f08315ed5ae5d37d0f5eb31cc4a692 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 21 Feb 2023 11:23:45 -0500 Subject: [PATCH 2/2] Update verbs used to check SAR for users when k8s components are used Update SAR checks for user permissions in webhook server to check whether a user can get/create/update/delete the resource rather than checking for '*' permissions. This is required as even if the user has the admin rolebinding, they do not have '*' permissions from the perspective of the cluster. Signed-off-by: Angel Misevski (cherry picked from commit 66c3f3f90f55f24c527b6f7f355031c0c1714bb5) --- webhook/workspace/handler/kubernetes.go | 76 +++++++++++++++---------- 1 file changed, 47 insertions(+), 29 deletions(-) diff --git a/webhook/workspace/handler/kubernetes.go b/webhook/workspace/handler/kubernetes.go index 1b67130c6..9c397af90 100644 --- a/webhook/workspace/handler/kubernetes.go +++ b/webhook/workspace/handler/kubernetes.go @@ -26,6 +26,13 @@ import ( "sigs.k8s.io/yaml" ) +var ( + // Verbs used in SAR checks when users attempt to use a Kubernetes/OpenShift component. A longer list results in + // more SAR checks per request. We cannot use '*' as a verb as the SAR will check for literal '*' permissions + // rather than "all verbs" + userVerbs = []string{"get", "create", "update", "delete"} +) + func (h *WebhookHandler) validateKubernetesObjectPermissionsOnCreate(ctx context.Context, req admission.Request, wksp *dwv2.DevWorkspaceTemplateSpec) error { kubeComponents := getKubeComponentsFromWorkspace(wksp) for componentName, component := range kubeComponents { @@ -97,37 +104,14 @@ func (h *WebhookHandler) validatePermissionsOnObject(ctx context.Context, req ad // Convert e.g. Pod -> pods, Deployment -> deployments resourceType := fmt.Sprintf("%ss", strings.ToLower(typeMeta.Kind)) - sar := &authv1.LocalSubjectAccessReview{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: req.Namespace, - }, - Spec: authv1.SubjectAccessReviewSpec{ - ResourceAttributes: &authv1.ResourceAttributes{ - Namespace: req.Namespace, - Verb: "*", - Group: typeMeta.GroupVersionKind().Group, - Version: typeMeta.GroupVersionKind().Version, - Resource: resourceType, - }, - User: req.UserInfo.Username, - Groups: req.UserInfo.Groups, - UID: req.UserInfo.UID, - }, - } - - err := h.Client.Create(ctx, sar) - if err != nil { - return fmt.Errorf("failed to create subjectaccessreview for request: %w", err) - } - - username := req.UserInfo.Username - if username == "" { - username = req.UserInfo.UID - } - if !sar.Status.Allowed { - return fmt.Errorf("user %s does not have permissions to work with objects of kind %s defined in component %s", username, typeMeta.GroupVersionKind().String(), componentName) + // Check that user has permissions to use the resource + for _, verb := range userVerbs { + if err := h.checkSAR(ctx, req, typeMeta, resourceType, verb, componentName); err != nil { + return err + } } + // Check that DWO has '*' permissions for the relevant resource ssar := &authv1.LocalSubjectAccessReview{ ObjectMeta: metav1.ObjectMeta{ Namespace: req.Namespace, @@ -250,3 +234,37 @@ func getKubeLikeComponent_v1alpha1(component *dwv1.Component) (*dwv1.K8sLikeComp } return nil, fmt.Errorf("component does not specify kubernetes or openshift fields") } + +func (h *WebhookHandler) checkSAR(ctx context.Context, req admission.Request, typeMeta *metav1.TypeMeta, resourceType, verb, componentName string) error { + sar := &authv1.LocalSubjectAccessReview{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: req.Namespace, + }, + Spec: authv1.SubjectAccessReviewSpec{ + ResourceAttributes: &authv1.ResourceAttributes{ + Namespace: req.Namespace, + Verb: verb, + Group: typeMeta.GroupVersionKind().Group, + Version: typeMeta.GroupVersionKind().Version, + Resource: resourceType, + }, + User: req.UserInfo.Username, + Groups: req.UserInfo.Groups, + UID: req.UserInfo.UID, + }, + } + + err := h.Client.Create(ctx, sar) + if err != nil { + return fmt.Errorf("failed to create subjectaccessreview for request: %w", err) + } + + username := req.UserInfo.Username + if username == "" { + username = req.UserInfo.UID + } + if !sar.Status.Allowed { + return fmt.Errorf("user %s does not have permissions to '%s' objects of kind %s defined in component %s", username, verb, typeMeta.GroupVersionKind().String(), componentName) + } + return nil +}