diff --git a/cmd/kube-apiserver/app/aggregator.go b/cmd/kube-apiserver/app/aggregator.go index cf073846fc70..35580a635ae5 100644 --- a/cmd/kube-apiserver/app/aggregator.go +++ b/cmd/kube-apiserver/app/aggregator.go @@ -67,6 +67,9 @@ func createAggregatorConfig( genericConfig := kubeAPIServerConfig genericConfig.PostStartHooks = map[string]genericapiserver.PostStartHookConfigEntry{} genericConfig.RESTOptionsGetter = nil + // prevent generic API server from installing the OpenAPI handler. Aggregator server + // has its own customized OpenAPI handler. + genericConfig.SkipOpenAPIInstallation = true // override genericConfig.AdmissionControl with kube-aggregator's scheme, // because aggregator apiserver should use its own scheme to convert its own resources. diff --git a/staging/src/k8s.io/apimachinery/pkg/util/json/json.go b/staging/src/k8s.io/apimachinery/pkg/util/json/json.go index 204834883fac..778e58f704b9 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/json/json.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/json/json.go @@ -39,7 +39,8 @@ func Marshal(v interface{}) ([]byte, error) { const maxDepth = 10000 // Unmarshal unmarshals the given data -// If v is a *map[string]interface{}, numbers are converted to int64 or float64 +// If v is a *map[string]interface{}, *[]interface{}, or *interface{} numbers +// are converted to int64 or float64 func Unmarshal(data []byte, v interface{}) error { switch v := v.(type) { case *map[string]interface{}: @@ -52,7 +53,7 @@ func Unmarshal(data []byte, v interface{}) error { return err } // If the decode succeeds, post-process the map to convert json.Number objects to int64 or float64 - return convertMapNumbers(*v, 0) + return ConvertMapNumbers(*v, 0) case *[]interface{}: // Build a decoder from the given data @@ -64,7 +65,7 @@ func Unmarshal(data []byte, v interface{}) error { return err } // If the decode succeeds, post-process the map to convert json.Number objects to int64 or float64 - return convertSliceNumbers(*v, 0) + return ConvertSliceNumbers(*v, 0) case *interface{}: // Build a decoder from the given data @@ -76,29 +77,31 @@ func Unmarshal(data []byte, v interface{}) error { return err } // If the decode succeeds, post-process the map to convert json.Number objects to int64 or float64 - return convertInterfaceNumbers(v, 0) + return ConvertInterfaceNumbers(v, 0) default: return json.Unmarshal(data, v) } } -func convertInterfaceNumbers(v *interface{}, depth int) error { +// ConvertInterfaceNumbers converts any json.Number values to int64 or float64. +// Values which are map[string]interface{} or []interface{} are recursively visited +func ConvertInterfaceNumbers(v *interface{}, depth int) error { var err error switch v2 := (*v).(type) { case json.Number: *v, err = convertNumber(v2) case map[string]interface{}: - err = convertMapNumbers(v2, depth+1) + err = ConvertMapNumbers(v2, depth+1) case []interface{}: - err = convertSliceNumbers(v2, depth+1) + err = ConvertSliceNumbers(v2, depth+1) } return err } -// convertMapNumbers traverses the map, converting any json.Number values to int64 or float64. +// ConvertMapNumbers traverses the map, converting any json.Number values to int64 or float64. // values which are map[string]interface{} or []interface{} are recursively visited -func convertMapNumbers(m map[string]interface{}, depth int) error { +func ConvertMapNumbers(m map[string]interface{}, depth int) error { if depth > maxDepth { return fmt.Errorf("exceeded max depth of %d", maxDepth) } @@ -109,9 +112,9 @@ func convertMapNumbers(m map[string]interface{}, depth int) error { case json.Number: m[k], err = convertNumber(v) case map[string]interface{}: - err = convertMapNumbers(v, depth+1) + err = ConvertMapNumbers(v, depth+1) case []interface{}: - err = convertSliceNumbers(v, depth+1) + err = ConvertSliceNumbers(v, depth+1) } if err != nil { return err @@ -120,9 +123,9 @@ func convertMapNumbers(m map[string]interface{}, depth int) error { return nil } -// convertSliceNumbers traverses the slice, converting any json.Number values to int64 or float64. +// ConvertSliceNumbers traverses the slice, converting any json.Number values to int64 or float64. // values which are map[string]interface{} or []interface{} are recursively visited -func convertSliceNumbers(s []interface{}, depth int) error { +func ConvertSliceNumbers(s []interface{}, depth int) error { if depth > maxDepth { return fmt.Errorf("exceeded max depth of %d", maxDepth) } @@ -133,9 +136,9 @@ func convertSliceNumbers(s []interface{}, depth int) error { case json.Number: s[i], err = convertNumber(v) case map[string]interface{}: - err = convertMapNumbers(v, depth+1) + err = ConvertMapNumbers(v, depth+1) case []interface{}: - err = convertSliceNumbers(v, depth+1) + err = ConvertSliceNumbers(v, depth+1) } if err != nil { return err diff --git a/staging/src/k8s.io/apimachinery/pkg/util/yaml/BUILD b/staging/src/k8s.io/apimachinery/pkg/util/yaml/BUILD index ad28913e0823..96a964861490 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/yaml/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/util/yaml/BUILD @@ -18,6 +18,7 @@ go_library( importmap = "k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/yaml", importpath = "k8s.io/apimachinery/pkg/util/yaml", deps = [ + "//staging/src/k8s.io/apimachinery/pkg/util/json:go_default_library", "//vendor/k8s.io/klog:go_default_library", "//vendor/sigs.k8s.io/yaml:go_default_library", ], diff --git a/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder.go b/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder.go index a9a3853ac3d2..934ca9781506 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder.go @@ -26,10 +26,41 @@ import ( "strings" "unicode" + jsonutil "k8s.io/apimachinery/pkg/util/json" "k8s.io/klog" + "sigs.k8s.io/yaml" ) +// Unmarshal unmarshals the given data +// If v is a *map[string]interface{}, *[]interface{}, or *interface{} numbers +// are converted to int64 or float64 +func Unmarshal(data []byte, v interface{}) error { + preserveIntFloat := func(d *json.Decoder) *json.Decoder { + d.UseNumber() + return d + } + switch v := v.(type) { + case *map[string]interface{}: + if err := yaml.Unmarshal(data, v, preserveIntFloat); err != nil { + return err + } + return jsonutil.ConvertMapNumbers(*v, 0) + case *[]interface{}: + if err := yaml.Unmarshal(data, v, preserveIntFloat); err != nil { + return err + } + return jsonutil.ConvertSliceNumbers(*v, 0) + case *interface{}: + if err := yaml.Unmarshal(data, v, preserveIntFloat); err != nil { + return err + } + return jsonutil.ConvertInterfaceNumbers(v, 0) + default: + return yaml.Unmarshal(data, v) + } +} + // ToJSON converts a single YAML document into a JSON document // or returns an error. If the document appears to be JSON the // YAML decoding path is not used (so that error messages are diff --git a/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder_test.go b/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder_test.go index fa5e16a4f650..c39d80ca74ed 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder_test.go @@ -403,3 +403,44 @@ stuff: 1 t.Fatalf("expected %q to be of type YAMLSyntaxError", err.Error()) } } + +func TestUnmarshal(t *testing.T) { + mapWithIntegerBytes := []byte(`replicas: 1`) + mapWithInteger := make(map[string]interface{}) + if err := Unmarshal(mapWithIntegerBytes, &mapWithInteger); err != nil { + t.Fatalf("unexpected error unmarshaling yaml: %v", err) + } + if _, ok := mapWithInteger["replicas"].(int64); !ok { + t.Fatalf(`Expected number in map to be int64 but got "%T"`, mapWithInteger["replicas"]) + } + + sliceWithIntegerBytes := []byte(`- 1`) + var sliceWithInteger []interface{} + if err := Unmarshal(sliceWithIntegerBytes, &sliceWithInteger); err != nil { + t.Fatalf("unexpected error unmarshaling yaml: %v", err) + } + if _, ok := sliceWithInteger[0].(int64); !ok { + t.Fatalf(`Expected number in slice to be int64 but got "%T"`, sliceWithInteger[0]) + } + + integerBytes := []byte(`1`) + var integer interface{} + if err := Unmarshal(integerBytes, &integer); err != nil { + t.Fatalf("unexpected error unmarshaling yaml: %v", err) + } + if _, ok := integer.(int64); !ok { + t.Fatalf(`Expected number to be int64 but got "%T"`, integer) + } + + otherTypeBytes := []byte(`123: 2`) + otherType := make(map[int]interface{}) + if err := Unmarshal(otherTypeBytes, &otherType); err != nil { + t.Fatalf("unexpected error unmarshaling yaml: %v", err) + } + if _, ok := otherType[123].(int64); ok { + t.Fatalf(`Expected number not to be converted to int64`) + } + if _, ok := otherType[123].(float64); !ok { + t.Fatalf(`Expected number to be float64 but got "%T"`, otherType[123]) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD index 3d54973b8905..fc0bac33af1b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD @@ -86,6 +86,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/yaml:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/audit:go_default_library", @@ -107,7 +108,6 @@ go_library( "//vendor/google.golang.org/grpc/status:go_default_library", "//vendor/k8s.io/klog:go_default_library", "//vendor/k8s.io/utils/trace:go_default_library", - "//vendor/sigs.k8s.io/yaml:go_default_library", ], ) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index c295d0aa6593..e56f76dd809a 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -38,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/authorization/authorizer" @@ -49,7 +50,6 @@ import ( "k8s.io/apiserver/pkg/util/dryrun" utilfeature "k8s.io/apiserver/pkg/util/feature" utiltrace "k8s.io/utils/trace" - "sigs.k8s.io/yaml" ) const ( diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index 00efd0cc9d94..0894c9116575 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -160,6 +160,8 @@ type Config struct { Serializer runtime.NegotiatedSerializer // OpenAPIConfig will be used in generating OpenAPI spec. This is nil by default. Use DefaultOpenAPIConfig for "working" defaults. OpenAPIConfig *openapicommon.Config + // SkipOpenAPIInstallation avoids installing the OpenAPI handler if set to true. + SkipOpenAPIInstallation bool // RESTOptionsGetter is used to construct RESTStorage types via the generic registry. RESTOptionsGetter genericregistry.RESTOptionsGetter @@ -558,7 +560,8 @@ func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*G listedPathProvider: apiServerHandler, - openAPIConfig: c.OpenAPIConfig, + openAPIConfig: c.OpenAPIConfig, + skipOpenAPIInstallation: c.SkipOpenAPIInstallation, postStartHooks: map[string]postStartHookEntry{}, preShutdownHooks: map[string]preShutdownHookEntry{}, diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go index 9c623f6485d6..4a4830a68c9a 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go +++ b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go @@ -129,8 +129,14 @@ type GenericAPIServer struct { // Enable swagger and/or OpenAPI if these configs are non-nil. openAPIConfig *openapicommon.Config + // SkipOpenAPIInstallation indicates not to install the OpenAPI handler + // during PrepareRun. + // Set this to true when the specific API Server has its own OpenAPI handler + // (e.g. kube-aggregator) + skipOpenAPIInstallation bool + // OpenAPIVersionedService controls the /openapi/v2 endpoint, and can be used to update the served spec. - // It is set during PrepareRun. + // It is set during PrepareRun if `openAPIConfig` is non-nil unless `skipOpenAPIInstallation` is true. OpenAPIVersionedService *handler.OpenAPIService // StaticOpenAPISpec is the spec derived from the restful container endpoints. @@ -283,7 +289,7 @@ type preparedGenericAPIServer struct { func (s *GenericAPIServer) PrepareRun() preparedGenericAPIServer { s.delegationTarget.PrepareRun() - if s.openAPIConfig != nil { + if s.openAPIConfig != nil && !s.skipOpenAPIInstallation { s.OpenAPIVersionedService, s.StaticOpenAPISpec = routes.OpenAPI{ Config: s.openAPIConfig, }.Install(s.Handler.GoRestfulContainer, s.Handler.NonGoRestfulMux) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go index 02c887c2a51d..035a5e719b71 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go @@ -158,11 +158,6 @@ func (cfg *Config) Complete() CompletedConfig { // NewWithDelegate returns a new instance of APIAggregator from the given config. func (c completedConfig) NewWithDelegate(delegationTarget genericapiserver.DelegationTarget) (*APIAggregator, error) { - // Prevent generic API server to install OpenAPI handler. Aggregator server - // has its own customized OpenAPI handler. - openAPIConfig := c.GenericConfig.OpenAPIConfig - c.GenericConfig.OpenAPIConfig = nil - genericServer, err := c.GenericConfig.New("kube-aggregator", delegationTarget) if err != nil { return nil, err @@ -188,7 +183,7 @@ func (c completedConfig) NewWithDelegate(delegationTarget genericapiserver.Deleg lister: informerFactory.Apiregistration().V1().APIServices().Lister(), APIRegistrationInformers: informerFactory, serviceResolver: c.ExtraConfig.ServiceResolver, - openAPIConfig: openAPIConfig, + openAPIConfig: c.GenericConfig.OpenAPIConfig, egressSelector: c.GenericConfig.EgressSelector, } diff --git a/test/integration/apiserver/apply/status_test.go b/test/integration/apiserver/apply/status_test.go index 2214c72a4994..bcf70c71bfaa 100644 --- a/test/integration/apiserver/apply/status_test.go +++ b/test/integration/apiserver/apply/status_test.go @@ -57,15 +57,7 @@ var statusData = map[schema.GroupVersionResource]string{ gvr("certificates.k8s.io", "v1beta1", "certificatesigningrequests"): `{"status": {"conditions": [{"type": "MyStatus"}]}}`, } -const statusDefault = `{"status": {"conditions": [{"type": "MyStatus", "status":"true"}]}}` - -// DO NOT ADD TO THIS LIST. -// This list is used to ignore known bugs. We shouldn't introduce new bugs. -var ignoreList = map[schema.GroupVersionResource]struct{}{ - // TODO(#89264): apiservices doesn't work because the openapi is not routed properly. - gvr("apiregistration.k8s.io", "v1beta1", "apiservices"): {}, - gvr("apiregistration.k8s.io", "v1", "apiservices"): {}, -} +const statusDefault = `{"status": {"conditions": [{"type": "MyStatus", "status":"True"}]}}` func gvr(g, v, r string) schema.GroupVersionResource { return schema.GroupVersionResource{Group: g, Version: v, Resource: r} @@ -137,9 +129,6 @@ func TestApplyStatus(t *testing.T) { t.Fatal(err) } t.Run(mapping.Resource.String(), func(t *testing.T) { - if _, ok := ignoreList[mapping.Resource]; ok { - t.Skip() - } status, ok := statusData[mapping.Resource] if !ok { status = statusDefault @@ -158,8 +147,17 @@ func TestApplyStatus(t *testing.T) { namespace = "" } name := newObj.GetName() + + // etcd test stub data doesn't contain apiVersion/kind (!), but apply requires it + newObj.SetGroupVersionKind(mapping.GroupVersionKind) + createData, err := json.Marshal(newObj.Object) + if err != nil { + t.Fatal(err) + } + rsc := dynamicClient.Resource(mapping.Resource).Namespace(namespace) - _, err := rsc.Create(context.TODO(), &newObj, metav1.CreateOptions{FieldManager: "create_test"}) + // apply to create + _, err = rsc.Patch(context.TODO(), name, types.ApplyPatchType, []byte(createData), metav1.PatchOptions{FieldManager: "create_test"}) if err != nil { t.Fatal(err) } diff --git a/test/integration/etcd/data.go b/test/integration/etcd/data.go index 82ab2b97e6c8..b824c19b5d89 100644 --- a/test/integration/etcd/data.go +++ b/test/integration/etcd/data.go @@ -44,7 +44,7 @@ func GetEtcdStorageDataForNamespace(namespace string) map[schema.GroupVersionRes ExpectedEtcdPath: "/registry/configmaps/" + namespace + "/cm1", }, gvr("", "v1", "services"): { - Stub: `{"metadata": {"name": "service1"}, "spec": {"externalName": "service1name", "ports": [{"port": 10000, "targetPort": 11000}], "selector": {"test": "data"}}}`, + Stub: `{"metadata": {"name": "service1"}, "spec": {"externalName": "service1name", "ports": [{"port": 10000, "targetPort": 11000, "protocol":"TCP"}], "selector": {"test": "data"}}}`, ExpectedEtcdPath: "/registry/services/specs/" + namespace + "/service1", }, gvr("", "v1", "podtemplates"): {