Copy Kubernetes objects in webhook

[dgerd]

In what area(s)?

/area API

/area autoscale
/area build
/area monitoring
/area networking
/area test-and-release

Describe the feature

Many of our CRD objects take Kubernetes object shapes as input such as corev1.Container, corev1.ObjectReference, corev1.Volume. Our webhook today takes these shapes and passes the user input through to the controller. However, given that the Kubernetes API is continuously growing new resource fields may be added to objects we consume. The behavior of those new fields may not be inline with the behavior we desire for Knative.

Instead of passing the objects through we should copy the fields we care about to a new object. This prevents additional fields that get added from being 'on by default'.

[dprotaso]

+1

…

On Wed, Mar 13, 2019 at 18:19 Dan Gerdesmeier ***@***.***> wrote: In what area(s)? /area API /area autoscale /area build /area monitoring /area networking /area test-and-release Describe the feature Many of our CRD objects take Kubernetes object shapes as input such as corev1.Container, corev1.ObjectReference, corev1.Volume. Our webhook today takes these shapes and passes the user input through to the controller. However, given that the Kubernetes API is continuously growing new resource fields may be added to objects we consume. The behavior of those new fields may not be inline with the behavior we desire for Knative. Instead of passing the objects through we should copy the fields we care about to a new object. This prevents additional fields that get added from being 'on by default'. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3424>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABIgnS719MMxKuvrjMZeq8hugMRAAYaks5vWXl1gaJpZM4bxDM2> .

[mattmoor]

+1

[dgerd]

/assign @dgerd

[dgerd]

Spent some time looking at implementing this. I think we will want to add this to the validator to replace all of the disallowed fields checks that we have today. Getting an error that a field is set that shouldn't be set seems preferable to un-setting disallowed fields through the defaulter.

There are a few ways we can implement the copy & compare functions although none of them stick out as great.

1. Create a custom, non-generated DeepCopy() method for the above objects in the Knative API package. This DeepCopy function only copies the fields we care about. cmp can then be used with a custom reporter to provide a list of fields which can be used to format apis.FieldErrors.
2. Use the reflect package and a set of allowed fields to traverse each object and find any set fields. Any fields not part of the allowed set are added to a disallowed list to format apis.FieldErrors.
3. Do In Route, add capability for watching for changes to Configuration so when Configuration gets updated, we'll pick up changes) #2, but with the kubernetes unstructured library.