knative / serving

Kubernetes-based, scale-to-zero, request-driven compute

https://knative.dev/docs/serving/

Apache License 2.0

5.53k stars 1.15k forks source link

Add conformance test to validate runAsUser is as specified #3223

Closed dgerd closed 5 years ago

dgerd commented 5 years ago

In what area(s)?

/area API /area test-and-release

Describe the feature

This issue is to add a conformance test that validates the MUST, SHOULD, and SHALL statements of the user section ( https://github.com/knative/serving/blob/master/docs/runtime-contract.md#user ) in the runtime contract.

The test should validate that:

When passing a valid runAsUser the container runs with that user.
When passing an invalid runAsUser the container fails

dgerd commented 5 years ago

/assign @dgerd

dgerd commented 5 years ago

We do not seem to set a platform-specific default as called out in the runtime contract. We instead seem to implement the Kubernetes behavior which to "Default to user specified in image metadata if unspecified."

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.10/#container-v1-core

dgerd commented 5 years ago

We also currently accept negative runAs user values as well as values greater than an int32 which are invalid. Need to add validation to the webhook and a test.

dgerd commented 5 years ago

Need to answer:

How should platform providers and operators ban certain UIDs?
How should we validate UID until we can perform kubernetes dryruns?
Should we allow a platform default or update language? If we update langauge how do we easily ban UIDs?

mattmoor commented 5 years ago

/milestone Serving 0.6 We'll have the basic test in 0.5, but the open questions above will likely overshoot.

dgerd commented 5 years ago

How should platform providers and operators ban certain UIDs?

I don't think this is necessary to implement now. We should wait until we get requests from providers/operators.

How should we validate UID until we can perform kubernetes dryruns?

The Kubernetes validation logic for user was added in the webhook along with the fieldmask. We are unblocked here to add a conformance test for invalid user ids preventing container startup.

Should we allow a platform default or update language?

4035 updates the language from platform-default to container-defined default. This is our current behavior today and I believe represents the behavior we want going forward. We need to add a test here for this functionality. However, to do so we need a new base image that uses a non-root user.