timflannagan / rukpak

Rukpak runs in a Kubernetes cluster and defines an API for installing cloud native bundle content
Apache License 2.0
0 stars 0 forks source link

(tyslaton): Address unpacker pod allowing root users for image sources #62

Open github-actions[bot] opened 2 years ago

github-actions[bot] commented 2 years ago

In our current implementation, we are creating a pod that uses the image

provided by an image source. This pod is not always guaranteed to run as a

non-root user and thus will fail to initialize if running as root in a PSA

restricted namespace due to violations. As it currently stands, our compliance

with PSA is baseline which allows for pods to run as root users. However,

all RukPak processes and resources, except this unpacker pod for image sources,

are runnable in a PSA restricted environment. We should consider ways to make

this PSA definition either configurable or workable in a restricted namespace.

See https://github.com/operator-framework/rukpak/pull/539 for more detail.

https://github.com/timflannagan/rukpak/blob/4f19948f042ef508eafddd921f9fc52715fd052d/internal/source/image.go#L208


    _ = i.Client.Delete(ctx, pod)
    return fmt.Errorf("unexpected pod phase: %v", pod.Status.Phase)
}

func pendingImagePodResult(pod *corev1.Pod) *Result {
    var messages []string
    for _, cStatus := range append(pod.Status.InitContainerStatuses, pod.Status.ContainerStatuses...) {
        if waiting := cStatus.State.Waiting; waiting != nil {
            if waiting.Reason == "ErrImagePull" || waiting.Reason == "ImagePullBackOff" {
                messages = append(messages, waiting.Message)
            }
        }
    }
    return &Result{State: StatePending, Message: strings.Join(messages, "; ")}
}

// addSecurityContext is responsible for taking a container and defining the
// relevant security context values. By having a function do this, we can keep
// that configuration easily consistent and maintainable.
func addSecurityContext(pod *corev1.Pod) {
    // Check that pod is defined before proceeding
    if pod == nil {
        return
    }

    // Add security context for overall pod
    pod.Spec.SecurityContext = &corev1.PodSecurityContext{
        // TODO (tyslaton): Address unpacker pod allowing root users for image sources
        //
        // In our current implementation, we are creating a pod that uses the image
        // provided by an image source. This pod is not always guaranteed to run as a
        // non-root user and thus will fail to initialize if running as root in a PSA
        // restricted namespace due to violations. As it currently stands, our compliance
        // with PSA is baseline which allows for pods to run as root users. However,
        // all RukPak processes and resources, except this unpacker pod for image sources,
        // are runnable in a PSA restricted environment. We should consider ways to make
        // this PSA definition either configurable or workable in a restricted namespace.
        //
        // See https://github.com/operator-framework/rukpak/pull/539 for more detail.
        RunAsNonRoot: pointer.Bool(false),
        SeccompProfile: &corev1.SeccompProfile{
            Type: corev1.SeccompProfileTypeRuntimeDefault,
        },
    }

    // Add security context for containers
    for i := range pod.Spec.InitContainers {
        pod.Spec.InitContainers[i].SecurityContext = &corev1.SecurityContext{
            AllowPrivilegeEscalation: pointer.Bool(false),
            Capabilities: &corev1.Capabilities{
                Drop: []corev1.Capability{"ALL"},
            },
        }
    }

    // Add security context for containers
    for i := range pod.Spec.Containers {
        pod.Spec.Containers[i].SecurityContext = &corev1.SecurityContext{
            AllowPrivilegeEscalation: pointer.Bool(false),
            Capabilities: &corev1.Capabilities{
                Drop: []corev1.Capability{"ALL"},
            },
        }
    }
}
github-actions[bot] commented 1 year ago

This issue has become stale because it has been open 60 days with no activity. The maintainers of this repo will remove this label during issue triage or it will be removed automatically after an update. Adding the lifecycle/frozen label will cause this issue to ignore lifecycle events.