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

Fix serialization of x-kubernetes-validations OpenAPI extension #107970

Merged
merged 2 commits into from Feb 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions staging/src/k8s.io/apiextensions-apiserver/go.mod
Expand Up @@ -30,6 +30,7 @@ require (
k8s.io/klog/v2 v2.40.1
k8s.io/kube-openapi v0.0.0-20211115234752-e816edb12b65
k8s.io/utils v0.0.0-20211208161948-7d6a63dca704
sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2
sigs.k8s.io/structured-merge-diff/v4 v4.2.1
sigs.k8s.io/yaml v1.2.0
)
Expand Down
Expand Up @@ -18,6 +18,7 @@ package v1

import (
"bytes"
unsafe "unsafe"

"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiequality "k8s.io/apimachinery/pkg/api/equality"
Expand Down Expand Up @@ -207,3 +208,8 @@ func Convert_apiextensions_CustomResourceConversion_To_v1_CustomResourceConversi
}
return nil
}

func Convert_apiextensions_ValidationRules_To_v1_ValidationRules(in *apiextensions.ValidationRules, out *ValidationRules, s conversion.Scope) error {
*out = *(*ValidationRules)(unsafe.Pointer(in))
return nil
}
Expand Up @@ -18,6 +18,7 @@ package v1

import (
"encoding/json"
fmt "fmt"
"reflect"
"strings"
"testing"
Expand Down Expand Up @@ -660,3 +661,71 @@ func TestJSONRoundTrip(t *testing.T) {
})
}
}

func TestMemoryEqual(t *testing.T) {
testcases := []struct {
a interface{}
b interface{}
}{
{apiextensions.JSONSchemaProps{}.XValidations, JSONSchemaProps{}.XValidations},
}

for _, tc := range testcases {
aType := reflect.TypeOf(tc.a)
bType := reflect.TypeOf(tc.b)
t.Run(aType.String(), func(t *testing.T) {
assertEqualTypes(t, nil, aType, bType)
})
}
}

func assertEqualTypes(t *testing.T, path []string, a, b reflect.Type) {
if a == b {
return
}

if a.Kind() != b.Kind() {
fatalTypeError(t, path, a, b, "mismatched Kind")
}

switch a.Kind() {
case reflect.Struct:
aFields := a.NumField()
bFields := b.NumField()
if aFields != bFields {
fatalTypeError(t, path, a, b, "mismatched field count")
}
for i := 0; i < aFields; i++ {
aField := a.Field(i)
bField := b.Field(i)
if aField.Name != bField.Name {
fatalTypeError(t, path, a, b, fmt.Sprintf("mismatched field name %d: %s %s", i, aField.Name, bField.Name))
}
if aField.Offset != bField.Offset {
fatalTypeError(t, path, a, b, fmt.Sprintf("mismatched field offset %d: %v %v", i, aField.Offset, bField.Offset))
}
if aField.Anonymous != bField.Anonymous {
fatalTypeError(t, path, a, b, fmt.Sprintf("mismatched field anonymous %d: %v %v", i, aField.Anonymous, bField.Anonymous))
}
if !reflect.DeepEqual(aField.Index, bField.Index) {
fatalTypeError(t, path, a, b, fmt.Sprintf("mismatched field index %d: %v %v", i, aField.Index, bField.Index))
}
path = append(path, aField.Name)
assertEqualTypes(t, path, aField.Type, bField.Type)
path = path[:len(path)-1]
}

case reflect.Ptr, reflect.Slice:
aElemType := a.Elem()
bElemType := b.Elem()
assertEqualTypes(t, path, aElemType, bElemType)

default:
fatalTypeError(t, path, a, b, "unhandled kind")
}
}

func fatalTypeError(t *testing.T, path []string, a, b reflect.Type, message string) {
t.Helper()
t.Fatalf("%s: %s: %s %s", strings.Join(path, "."), message, a, b)
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Expand Up @@ -20,7 +20,7 @@ import (
"strings"
"testing"

"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
)

Expand Down
Expand Up @@ -24,7 +24,7 @@ import (
"github.com/google/cel-go/common/types/ref"
"github.com/google/cel-go/interpreter"

"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
"k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model"
"k8s.io/apimachinery/pkg/util/validation/field"
Expand Down
Expand Up @@ -22,7 +22,7 @@ import (
"strings"
"testing"

"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
)
Expand Down
Expand Up @@ -19,6 +19,7 @@ package schema
import (
"fmt"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
)

// NewStructural converts an OpenAPI v3 schema into a structural schema. A pre-validated JSONSchemaProps will
Expand Down Expand Up @@ -246,7 +247,9 @@ func newExtensions(s *apiextensions.JSONSchemaProps) (*Extensions, error) {
XListMapKeys: s.XListMapKeys,
XListType: s.XListType,
XMapType: s.XMapType,
XValidations: s.XValidations,
}
if err := apiextensionsv1.Convert_apiextensions_ValidationRules_To_v1_ValidationRules(&s.XValidations, &ret.XValidations, nil); err != nil {
return nil, err
}

if s.XPreserveUnknownFields != nil {
Expand Down
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
package schema

import (
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/runtime"
)

Expand Down Expand Up @@ -130,7 +130,8 @@ type Extensions struct {
XMapType *string

// x-kubernetes-validations describes a list of validation rules for expression validation.
XValidations apiextensions.ValidationRules
// Use the v1 struct since this gets serialized as an extension.
XValidations apiextensionsv1.ValidationRules
}

// +k8s:deepcopy-gen=true
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Expand Up @@ -21,6 +21,7 @@ import (
"strings"

"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
openapierrors "k8s.io/kube-openapi/pkg/validation/errors"
"k8s.io/kube-openapi/pkg/validation/spec"
Expand Down Expand Up @@ -255,7 +256,11 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou
out.VendorExtensible.AddExtension("x-kubernetes-map-type", *in.XMapType)
}
if len(in.XValidations) != 0 {
out.VendorExtensible.AddExtension("x-kubernetes-validations", in.XValidations)
var serializationValidationRules apiextensionsv1.ValidationRules
if err := apiextensionsv1.Convert_apiextensions_ValidationRules_To_v1_ValidationRules(&in.XValidations, &serializationValidationRules, nil); err != nil {
return err
}
out.VendorExtensible.AddExtension("x-kubernetes-validations", serializationValidationRules)
}
return nil
}
Expand Down
Expand Up @@ -18,7 +18,14 @@ package validation

import (
"math/rand"
"os"
"strconv"
"testing"
"time"

"github.com/google/go-cmp/cmp"

kjson "sigs.k8s.io/json"

"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensionsfuzzer "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer"
Expand Down Expand Up @@ -46,12 +53,21 @@ func TestRoundTrip(t *testing.T) {
t.Fatal(err)
}

seed := rand.Int63()
t.Logf("seed: %d", seed)
seed := int64(time.Now().Nanosecond())
if override := os.Getenv("TEST_RAND_SEED"); len(override) > 0 {
overrideSeed, err := strconv.Atoi(override)
if err != nil {
t.Fatal(err)
}
seed = int64(overrideSeed)
t.Logf("using overridden seed: %d", seed)
} else {
t.Logf("seed (override with TEST_RAND_SEED if desired): %d", seed)
}
fuzzerFuncs := fuzzer.MergeFuzzerFuncs(apiextensionsfuzzer.Funcs)
f := fuzzer.FuzzerFor(fuzzerFuncs, rand.NewSource(seed), codecs)

for i := 0; i < 20; i++ {
for i := 0; i < 50; i++ {
// fuzz internal types
internal := &apiextensions.JSONSchemaProps{}
f.Fuzz(internal)
Expand All @@ -70,8 +86,10 @@ func TestRoundTrip(t *testing.T) {

// JSON -> in-memory JSON => convertNullTypeToNullable => JSON
var j interface{}
if err := json.Unmarshal(openAPIJSON, &j); err != nil {
if strictErrs, err := kjson.UnmarshalStrict(openAPIJSON, &j); err != nil {
t.Fatal(err)
} else if len(strictErrs) > 0 {
t.Fatal(strictErrs)
}
j = stripIntOrStringType(j)
openAPIJSON, err = json.Marshal(j)
Expand All @@ -81,8 +99,10 @@ func TestRoundTrip(t *testing.T) {

// JSON -> external
external := &apiextensionsv1.JSONSchemaProps{}
if err := json.Unmarshal(openAPIJSON, external); err != nil {
if strictErrs, err := kjson.UnmarshalStrict(openAPIJSON, external); err != nil {
t.Fatal(err)
} else if len(strictErrs) > 0 {
t.Fatal(strictErrs)
}

// external -> internal
Expand All @@ -92,7 +112,8 @@ func TestRoundTrip(t *testing.T) {
}

if !apiequality.Semantic.DeepEqual(internal, internalRoundTripped) {
t.Fatalf("%d: expected\n\t%#v, got \n\t%#v", i, internal, internalRoundTripped)
t.Log(string(openAPIJSON))
t.Fatalf("%d: unexpected diff\n\t%s", i, cmp.Diff(internal, internalRoundTripped))
}
}
}
Expand Down