Ensure defaulting applies to custom resource items in list response

k3s-v1.15.3
Jordan Liggitt 2019-06-05 21:47:50 -04:00
parent 932553a08c
commit a3bb81ff32
3 changed files with 215 additions and 32 deletions

View File

@ -235,6 +235,8 @@ func testPrinter(t *testing.T, printer printers.ResourcePrinter, unmarshalFunc f
TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Pod"},
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
}
// our decoder defaults, so we should default our expected object as well
legacyscheme.Scheme.Default(obj)
buf.Reset()
printer.PrintObj(obj, buf)
var objOut v1.Pod

View File

@ -17,6 +17,7 @@ limitations under the License.
package integration
import (
"fmt"
"strings"
"testing"
"time"
@ -29,6 +30,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/json"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apimachinery/pkg/watch"
utilfeature "k8s.io/apiserver/pkg/util/feature"
utilfeaturetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/utils/pointer"
@ -43,6 +45,18 @@ var defaultingFixture = &apiextensionsv1beta1.CustomResourceDefinition{
Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{
Group: "tests.apiextensions.k8s.io",
Version: "v1beta1",
Versions: []apiextensionsv1beta1.CustomResourceDefinitionVersion{
{
Name: "v1beta1",
Storage: false,
Served: true,
},
{
Name: "v1beta2",
Storage: true,
Served: false,
},
},
Names: apiextensionsv1beta1.CustomResourceDefinitionNames{
Plural: "foos",
Singular: "foo",
@ -57,7 +71,7 @@ var defaultingFixture = &apiextensionsv1beta1.CustomResourceDefinition{
},
}
const defaultingFooSchema = `
const defaultingFooV1beta1Schema = `
type: object
properties:
spec:
@ -69,6 +83,13 @@ properties:
b:
type: string
default: "B"
c:
type: string
v1beta1:
type: string
default: "v1beta1"
v1beta2:
type: string
status:
type: object
properties:
@ -78,20 +99,76 @@ properties:
b:
type: string
default: "B"
c:
type: string
v1beta1:
type: string
default: "v1beta1"
v1beta2:
type: string
`
func TestCustomResourceDefaulting(t *testing.T) {
const defaultingFooV1beta2Schema = `
type: object
properties:
spec:
type: object
properties:
a:
type: string
default: "A"
b:
type: string
default: "B"
c:
type: string
v1beta1:
type: string
v1beta2:
type: string
default: "v1beta2"
status:
type: object
properties:
a:
type: string
default: "A"
b:
type: string
default: "B"
c:
type: string
v1beta1:
type: string
v1beta2:
type: string
default: "v1beta2"
`
func TestCustomResourceDefaultingWithWatchCache(t *testing.T) {
testDefaulting(t, true)
}
func TestCustomResourceDefaultingWithoutWatchCache(t *testing.T) {
testDefaulting(t, false)
}
func testDefaulting(t *testing.T, watchCache bool) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CustomResourceDefaulting, true)()
tearDownFn, apiExtensionClient, dynamicClient, err := fixtures.StartDefaultServerWithClients(t)
tearDownFn, apiExtensionClient, dynamicClient, err := fixtures.StartDefaultServerWithClients(t, fmt.Sprintf("--watch-cache=%v", watchCache))
if err != nil {
t.Fatal(err)
}
defer tearDownFn()
crd := defaultingFixture.DeepCopy()
crd.Spec.Validation = &apiextensionsv1beta1.CustomResourceValidation{}
if err := yaml.Unmarshal([]byte(defaultingFooSchema), &crd.Spec.Validation.OpenAPIV3Schema); err != nil {
crd.Spec.Versions[0].Schema = &apiextensionsv1beta1.CustomResourceValidation{}
if err := yaml.Unmarshal([]byte(defaultingFooV1beta1Schema), &crd.Spec.Versions[0].Schema.OpenAPIV3Schema); err != nil {
t.Fatal(err)
}
crd.Spec.Versions[1].Schema = &apiextensionsv1beta1.CustomResourceValidation{}
if err := yaml.Unmarshal([]byte(defaultingFooV1beta2Schema), &crd.Spec.Versions[1].Schema.OpenAPIV3Schema); err != nil {
t.Fatal(err)
}
@ -101,13 +178,15 @@ func TestCustomResourceDefaulting(t *testing.T) {
}
mustExist := func(obj map[string]interface{}, pths [][]string) {
t.Helper()
for _, pth := range pths {
if _, found, _ := unstructured.NestedFieldNoCopy(obj, pth...); !found {
t.Errorf("Expected '%s' field exist", strings.Join(pth, "."))
t.Errorf("Expected '%s' field was missing", strings.Join(pth, "."))
}
}
}
mustNotExist := func(obj map[string]interface{}, pths [][]string) {
t.Helper()
for _, pth := range pths {
if fld, found, _ := unstructured.NestedFieldNoCopy(obj, pth...); found {
t.Errorf("Expected '%s' field to not exist, but it does: %v", strings.Join(pth, "."), fld)
@ -115,6 +194,7 @@ func TestCustomResourceDefaulting(t *testing.T) {
}
}
updateCRD := func(update func(*apiextensionsv1beta1.CustomResourceDefinition)) {
t.Helper()
var err error
for retry := 0; retry < 10; retry++ {
var obj *apiextensionsv1beta1.CustomResourceDefinition
@ -136,22 +216,34 @@ func TestCustomResourceDefaulting(t *testing.T) {
t.Fatal(err)
}
}
addDefault := func(key string, value interface{}) {
addDefault := func(version string, key string, value interface{}) {
t.Helper()
updateCRD(func(obj *apiextensionsv1beta1.CustomResourceDefinition) {
for _, root := range []string{"spec", "status"} {
obj.Spec.Validation.OpenAPIV3Schema.Properties[root].Properties[key] = apiextensionsv1beta1.JSONSchemaProps{
Type: "string",
Default: jsonPtr(value),
for i := range obj.Spec.Versions {
if obj.Spec.Versions[i].Name != version {
continue
}
obj.Spec.Versions[i].Schema.OpenAPIV3Schema.Properties[root].Properties[key] = apiextensionsv1beta1.JSONSchemaProps{
Type: "string",
Default: jsonPtr(value),
}
}
}
})
}
removeDefault := func(key string) {
removeDefault := func(version string, key string) {
t.Helper()
updateCRD(func(obj *apiextensionsv1beta1.CustomResourceDefinition) {
for _, root := range []string{"spec", "status"} {
props := obj.Spec.Validation.OpenAPIV3Schema.Properties[root].Properties[key]
props.Default = nil
obj.Spec.Validation.OpenAPIV3Schema.Properties[root].Properties[key] = props
for i := range obj.Spec.Versions {
if obj.Spec.Versions[i].Name != version {
continue
}
props := obj.Spec.Versions[i].Schema.OpenAPIV3Schema.Properties[root].Properties[key]
props.Default = nil
obj.Spec.Versions[i].Schema.OpenAPIV3Schema.Properties[root].Properties[key] = props
}
}
})
}
@ -168,8 +260,12 @@ func TestCustomResourceDefaulting(t *testing.T) {
if err != nil {
t.Fatalf("Unable to create CR: %v", err)
}
initialResourceVersion := foo.GetResourceVersion()
t.Logf("CR created: %#v", foo.UnstructuredContent())
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}})
// spec.a and spec.b are defaulted in both versions
// spec.v1beta1 is defaulted when reading the incoming request
// spec.v1beta2 is defaulted when reading the storage response
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"spec", "v1beta1"}, {"spec", "v1beta2"}})
mustNotExist(foo.Object, [][]string{{"status"}})
t.Logf("Updating status and expecting 'a' and 'b' to show up.")
@ -179,8 +275,90 @@ func TestCustomResourceDefaulting(t *testing.T) {
}
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"status", "a"}, {"status", "b"}})
t.Logf("Add 'c' default and wait until GET sees it in both status and spec")
addDefault("c", "C")
t.Logf("Add 'c' default to the storage version and wait until GET sees it in both status and spec")
addDefault("v1beta2", "c", "C")
t.Logf("wait until GET sees 'c' in both status and spec")
if err := wait.PollImmediate(100*time.Millisecond, wait.ForeverTestTimeout, func() (bool, error) {
obj, err := fooClient.Get(foo.GetName(), metav1.GetOptions{})
if err != nil {
return false, err
}
if _, found, _ := unstructured.NestedString(obj.Object, "spec", "c"); !found {
t.Log("will retry, did not find spec.c in the object")
return false, nil
}
foo = obj
return true, nil
}); err != nil {
t.Fatal(err)
}
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"spec", "c"}, {"status", "a"}, {"status", "b"}, {"status", "c"}})
t.Logf("wait until GET sees 'c' in both status and spec of cached get")
if err := wait.PollImmediate(100*time.Millisecond, wait.ForeverTestTimeout, func() (bool, error) {
obj, err := fooClient.Get(foo.GetName(), metav1.GetOptions{ResourceVersion: "0"})
if err != nil {
return false, err
}
if _, found, _ := unstructured.NestedString(obj.Object, "spec", "c"); !found {
t.Logf("will retry, did not find spec.c in the cached object")
return false, nil
}
foo = obj
return true, nil
}); err != nil {
t.Fatal(err)
}
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"spec", "c"}, {"status", "a"}, {"status", "b"}, {"status", "c"}})
t.Logf("verify LIST sees 'c' in both status and spec")
foos, err := fooClient.List(metav1.ListOptions{})
if err != nil {
t.Fatal(err)
}
for _, foo := range foos.Items {
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"spec", "c"}, {"status", "a"}, {"status", "b"}, {"status", "c"}})
}
t.Logf("verify LIST from cache sees 'c' in both status and spec")
foos, err = fooClient.List(metav1.ListOptions{ResourceVersion: "0"})
if err != nil {
t.Fatal(err)
}
for _, foo := range foos.Items {
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"spec", "c"}, {"status", "a"}, {"status", "b"}, {"status", "c"}})
}
// Omit this test when using the watch cache because changing the CRD resets the watch cache's minimum available resource version.
// The watch cache is populated by list and watch, which are both tested by this test.
// The contents of the watch cache are seen by list with rv=0, which is tested by this test.
if !watchCache {
t.Logf("verify WATCH sees 'c' in both status and spec")
w, err := fooClient.Watch(metav1.ListOptions{ResourceVersion: initialResourceVersion})
if err != nil {
t.Fatal(err)
}
select {
case event := <-w.ResultChan():
if event.Type != watch.Modified {
t.Fatalf("unexpected watch event: %v, %#v", event.Type, event.Object)
}
if e, a := "v1beta1", event.Object.GetObjectKind().GroupVersionKind().Version; e != a {
t.Errorf("watch event for v1beta1 API returned %v", a)
}
mustExist(
event.Object.(*unstructured.Unstructured).Object,
[][]string{{"spec", "a"}, {"spec", "b"}, {"spec", "c"}, {"status", "a"}, {"status", "b"}, {"status", "c"}},
)
case <-time.After(wait.ForeverTestTimeout):
t.Fatal("timed out without getting watch event")
}
}
t.Logf("Add 'c' default to the REST version, remove it from the storage version, and wait until GET no longer sees it in both status and spec")
addDefault("v1beta1", "c", "C")
removeDefault("v1beta2", "c")
if err := wait.PollImmediate(100*time.Millisecond, wait.ForeverTestTimeout, func() (bool, error) {
obj, err := fooClient.Get(foo.GetName(), metav1.GetOptions{})
if err != nil {
@ -188,22 +366,24 @@ func TestCustomResourceDefaulting(t *testing.T) {
}
_, found, _ := unstructured.NestedString(obj.Object, "spec", "c")
foo = obj
return found, nil
return !found, nil
}); err != nil {
t.Fatal(err)
}
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"spec", "c"}, {"status", "a"}, {"status", "b"}, {"status", "c"}})
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"status", "a"}, {"status", "b"}})
mustNotExist(foo.Object, [][]string{{"spec", "c"}, {"status", "c"}})
t.Logf("Updating status, expecting 'c' to be set in spec and status")
t.Logf("Updating status, expecting 'c' to be set in status only")
if foo, err = fooClient.UpdateStatus(foo, metav1.UpdateOptions{}); err != nil {
t.Fatal(err)
}
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"spec", "c"}, {"status", "a"}, {"status", "b"}, {"status", "c"}})
mustExist(foo.Object, [][]string{{"spec", "a"}, {"spec", "b"}, {"status", "a"}, {"status", "b"}, {"status", "c"}})
mustNotExist(foo.Object, [][]string{{"spec", "c"}})
t.Logf("Removing 'a', 'b' and `c` properties. Expecting that 'c' goes away in spec, but not in status. 'a' and 'b' were peristed.")
removeDefault("a")
removeDefault("b")
removeDefault("c")
t.Logf("Removing 'a', 'b' and `c` properties from the REST version. Expecting that 'c' goes away in spec, but not in status. 'a' and 'b' were presisted.")
removeDefault("v1beta1", "a")
removeDefault("v1beta1", "b")
removeDefault("v1beta1", "c")
if err := wait.PollImmediate(100*time.Millisecond, wait.ForeverTestTimeout, func() (bool, error) {
obj, err := fooClient.Get(foo.GetName(), metav1.GetOptions{})
if err != nil {

View File

@ -113,13 +113,6 @@ func (c *codec) Decode(data []byte, defaultGVK *schema.GroupVersionKind, into ru
// if we specify a target, use generic conversion.
if into != nil {
if into == obj {
if isVersioned {
return versioned, gvk, nil
}
return into, gvk, nil
}
// perform defaulting if requested
if c.defaulter != nil {
// create a copy to ensure defaulting is not applied to the original versioned objects
@ -133,6 +126,14 @@ func (c *codec) Decode(data []byte, defaultGVK *schema.GroupVersionKind, into ru
}
}
// Short-circuit conversion if the into object is same object
if into == obj {
if isVersioned {
return versioned, gvk, nil
}
return into, gvk, nil
}
if err := c.convertor.Convert(obj, into, c.decodeVersion); err != nil {
return nil, gvk, err
}