knative
Set secure-pod-defaults to "enabled" by default
In what area(s)?
/area API
Describe the feature
As per configmap comment:
https://github.com/knative/serving/blob/c91f8c43eb806fea4f6315d970f6d78e89a96d7c/config/core/configmaps/features.yaml#L46-L48
It says "probably Knative 1.10" but it is still disabled by default. I opened this for tracking issue.
/cc @evankanderson
Pairing with @kauana - we've discovered a few things
First the migrator job is doing extra patches when it shouldn't need to filed an issue here- https://github.com/knative/pkg/issues/2845
Oddly the empty patch is failing because we're hitting this validation - where the old and new spec are the same (unsure why) and the updater annotation is changing - which our webhook checks reject
https://github.com/knative/pkg/blob/6cf4b051de4f9859b0f1b80f790a83e84a75bc5b/apis/metadata_validation.go#L85
Even if we relaxed this requirement we observed the empty patch+defaulting triggers the creation of new a revision. It might make sense that this secure defaulting is only done during creation instead of update.
Finally, a more generic observation around this entire feature is that when we enable it our own hello world image doesn't work. This also applies to any image that runs as root. Hence enabling this by default will probably break ALOT of users especially for demos etc.
Curious what others think?
Thank you so much Kauana and Dave.
Finally, a more generic observation around this entire feature is that when we enable it our own hello world image doesn't work. This also applies to any image that runs as root. Hence enabling this by default will probably break ALOT of users especially for demos etc.
I think first of all we should start with adopting non-root for any images in our docs including hello world. Then, it would be better to make an announcement, observe the feedback for a while, and then consider changing the default?
I think first of all we should start with adopting non-root for any images in our docs including hello world.
I agree
Then, it would be better to make an announcement, observe the feedback for a while, and then consider changing the default?
I agree - that gathering data is probably the right thing to do.
Sounds good. Can I create an issue to make all serving code samples run as non-root?
Yup sounds good
cc @evankanderson for input
Curious if you have thoughts about the tension here where if we enable this by default it will break a lot of existing deployments and require folks to rewrite their images (if that's even possible for some).
I wanted to enable this at some point; if you explicitly request the insecure values, they are still allowed. It's just that the defaults will point towards secure values if not set. This will break people who are depending on the insecure values but haven't explicitly specified them. The trade-off is that people who are using the insecure values by accident will move to more secure settings. This mostly helps with defense-in-depth if the application is insecure, so it also isn't a critical defense.
The non-root part is probably the most disruptive; I'd be okay putting that behind a second flag to roll out the seccompProfile and allowPrivilegeEscalation bits earlier. But I think we definitely need to do a lot of communication about this, and I definitely haven't done so yet, so I don't think we can default this to "on" until we've done so.
Following up on Evan's feedback - we can make secure-pod-defaults an enum
Disabled- do nothingEnabled- enable most options but it still works with non-root images - still warn about nonRootRestricted- enable all options that the restricted profile requires