Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seccomp profile in queue-proxy incompatible with gvisor #13471

Closed
daraghlowe opened this issue Nov 15, 2022 · 16 comments · Fixed by #13507
Closed

Seccomp profile in queue-proxy incompatible with gvisor #13471

daraghlowe opened this issue Nov 15, 2022 · 16 comments · Fixed by #13507
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Issues which should be fixed (post-triage)
Milestone

Comments

@daraghlowe
Copy link

What version of Knative?

1.8.0

Expected Behavior

Pods should be able to start on GKE nodes running gvisor.

Actual Behavior

Gvisor refuses to allow the pods to start as a seccomp profile has been set, the following error is shown in events which refuses to allow the pod to start:

Seccomp is not supported

#13376 added the config below to queue-proxy containers by default, however gvisor won't allow any profile or even a blank profile to be set.

          seccompProfile:
            type: RuntimeDefault

Maybe this could be configurable in a config map whether it gets added to the queue proxy?

Steps to Reproduce the Problem

Create a Knative service running on node running gvisor on a cluster running Knative 1.8.0.

@daraghlowe daraghlowe added the kind/bug Categorizes issue or PR as related to a bug. label Nov 15, 2022
@dprotaso
Copy link
Member

cc @skonto @evankanderson

@dprotaso
Copy link
Member

@daraghlowe I'm assuming you're dropping the seccompProfile from the installation yamls correct?

@dprotaso
Copy link
Member

I'm sorta inclined to roll this change back. Operators can enforce a RuntimeDefault seccomp for all workloads

https://kubernetes.io/docs/tutorials/security/seccomp/#enable-the-use-of-runtimedefault-as-the-default-seccomp-profile-for-all-workloads

@dprotaso
Copy link
Member

/triage accepted

@knative-prow knative-prow bot added the triage/accepted Issues which should be fixed (post-triage) label Nov 24, 2022
@dprotaso dprotaso added this to the v1.9.0 milestone Nov 24, 2022
@daraghlowe
Copy link
Author

I'm assuming you're dropping the seccompProfile from the installation yamls correct?

We run the Knative control plane on a non gvisor node pool so we didn't have any issues upgrading. Our Knative services run on a gvisor node pool however so we only saw the problem with the queue-proxy container.

@skonto
Copy link
Contributor

skonto commented Nov 24, 2022

@dprotaso
My proposal would be:
a) rollback since it breaks gvisor which does not allow any profile, so upgrades are unblocked for services.
b) make this configurable (if not already) because people may want to add the profile only for Knative and not for all workloads.

@dprotaso
Copy link
Member

I created a PR to address a)

I think b) would be covered by Evan's PR #13398

@dprotaso
Copy link
Member

I wonder if we should remove the default profile in our control plane release yamls as well.

If someone is running the Knative control plane in gVisor or an earlier OpenShift release it will fail to upgrade.

I think the sensible thing is to wait till our k8s min version supports 1.25 to introduce these defaults. I'm assuming that's when they have an effect.

Though in the gVisor situation it'll never work so maybe we encourage folks adopt the operator that can toggle this on and off. Or they do their own post processing of our yamls.

@daraghlowe
Copy link
Author

Thanks for the fix @dprotaso

@nak3
Copy link
Contributor

nak3 commented Nov 25, 2022

EDIT: The comment is just same with Dave's #13471 (comment) 😅 I think release yaml should have the consistency.

@dprotaso
Copy link
Member

Created a new issue

@dprotaso
Copy link
Member

From: #13512 (comment)

Seeing how prolific this setting is (many Knative repos and even their dependencies) it's not worth the hassle. Even if we make the changes end-users will have to customize yamls (ie. cert-manager) for this to work.

@kevinGC
Copy link

kevinGC commented Dec 2, 2022

@daraghlowe can you clarify what version of GKE you're using?

@daraghlowe
Copy link
Author

@daraghlowe can you clarify what version of GKE you're using?

@kevinGC We're using 1.24.4-gke.800 at the moment in the project we ran into this issue.

@kevinGC
Copy link

kevinGC commented Dec 6, 2022

Looks like knative rolled this back, but anyways per the GKE bug tracker RuntimeDefault and Unconfined are allowed in GKE Sandbox starting in 1.26.

@daraghlowe
Copy link
Author

Great stuff, thanks @kevinGC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Issues which should be fixed (post-triage)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants