mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-22 11:21:47 +00:00
Merge pull request #80074 from yue9944882/bugfix/publish-path-parameter
Fixes missing path parameter to CRD restful container
This commit is contained in:
Kubernetes Prow Robot
GitHub
commit
96dc74a437
@ -54,11 +54,14 @@ go_test(
|
||||
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
|
||||
"//staging/src/k8s.io/apimachinery/pkg/util/json:go_default_library",
|
||||
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
|
||||
"//staging/src/k8s.io/apiserver/pkg/endpoints:go_default_library",
|
||||
"//vendor/github.com/go-openapi/spec:go_default_library",
|
||||
"//vendor/github.com/google/go-cmp/cmp:go_default_library",
|
||||
"//vendor/github.com/google/gofuzz:go_default_library",
|
||||
"//vendor/github.com/googleapis/gnostic/OpenAPIv2:go_default_library",
|
||||
"//vendor/github.com/googleapis/gnostic/compiler:go_default_library",
|
||||
"//vendor/github.com/stretchr/testify/assert:go_default_library",
|
||||
"//vendor/github.com/stretchr/testify/require:go_default_library",
|
||||
"//vendor/gopkg.in/yaml.v2:go_default_library",
|
||||
"//vendor/k8s.io/kube-openapi/pkg/util/proto:go_default_library",
|
||||
],
|
||||
|
||||
@ -52,6 +52,9 @@ const (
|
||||
|
||||
var (
|
||||
swaggerPartialObjectMetadataDescriptions = metav1beta1.PartialObjectMetadata{}.SwaggerDoc()
|
||||
|
||||
nameToken = "{name}"
|
||||
namespaceToken = "{namespace}"
|
||||
)
|
||||
|
||||
var definitions map[string]common.OpenAPIDefinition
|
||||
@ -96,33 +99,33 @@ func BuildSwagger(crd *apiextensions.CustomResourceDefinition, version string) (
|
||||
|
||||
routes := make([]*restful.RouteBuilder, 0)
|
||||
root := fmt.Sprintf("/apis/%s/%s/%s", b.group, b.version, b.plural)
|
||||
|
||||
if b.namespaced {
|
||||
routes = append(routes, b.buildRoute(root, "", "GET", "list", sampleList).
|
||||
Operation("list"+b.kind+"ForAllNamespaces"))
|
||||
routes = append(routes, b.buildRoute(root, "", "GET", "list", "list", sampleList).Operation("list"+b.kind+"ForAllNamespaces"))
|
||||
root = fmt.Sprintf("/apis/%s/%s/namespaces/{namespace}/%s", b.group, b.version, b.plural)
|
||||
}
|
||||
routes = append(routes, b.buildRoute(root, "", "GET", "list", sampleList))
|
||||
routes = append(routes, b.buildRoute(root, "", "POST", "create", sample).Reads(sample))
|
||||
routes = append(routes, b.buildRoute(root, "", "DELETE", "deletecollection", status))
|
||||
routes = append(routes, b.buildRoute(root, "", "GET", "list", "list", sampleList))
|
||||
routes = append(routes, b.buildRoute(root, "", "POST", "post", "create", sample).Reads(sample))
|
||||
routes = append(routes, b.buildRoute(root, "", "DELETE", "deletecollection", "deletecollection", status))
|
||||
|
||||
routes = append(routes, b.buildRoute(root, "/{name}", "GET", "read", sample))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}", "PUT", "replace", sample).Reads(sample))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}", "DELETE", "delete", status))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}", "PATCH", "patch", sample).Reads(patch))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}", "GET", "get", "read", sample))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}", "PUT", "put", "replace", sample).Reads(sample))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}", "DELETE", "delete", "delete", status))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}", "PATCH", "patch", "patch", sample).Reads(patch))
|
||||
|
||||
subresources, err := apiextensions.GetSubresourcesForVersion(crd, version)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if subresources != nil && subresources.Status != nil {
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/status", "GET", "read", sample))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/status", "PUT", "replace", sample).Reads(sample))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/status", "PATCH", "patch", sample).Reads(patch))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/status", "GET", "get", "read", sample))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/status", "PUT", "put", "replace", sample).Reads(sample))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/status", "PATCH", "patch", "patch", sample).Reads(patch))
|
||||
}
|
||||
if subresources != nil && subresources.Scale != nil {
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/scale", "GET", "read", scale))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/scale", "PUT", "replace", scale).Reads(scale))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/scale", "PATCH", "patch", scale).Reads(patch))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/scale", "GET", "get", "read", scale))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/scale", "PUT", "put", "replace", scale).Reads(scale))
|
||||
routes = append(routes, b.buildRoute(root, "/{name}/scale", "PATCH", "patch", "patch", scale).Reads(patch))
|
||||
}
|
||||
|
||||
for _, route := range routes {
|
||||
@ -189,9 +192,9 @@ func subresource(path string) string {
|
||||
panic("failed to parse subresource; invalid path")
|
||||
}
|
||||
|
||||
func (b *builder) descriptionFor(path, verb string) string {
|
||||
func (b *builder) descriptionFor(path, operationVerb string) string {
|
||||
var article string
|
||||
switch verb {
|
||||
switch operationVerb {
|
||||
case "list":
|
||||
article = " objects of kind "
|
||||
case "read", "replace":
|
||||
@ -209,7 +212,7 @@ func (b *builder) descriptionFor(path, verb string) string {
|
||||
if len(sub) > 0 {
|
||||
sub = " " + sub + " of"
|
||||
}
|
||||
switch verb {
|
||||
switch operationVerb {
|
||||
case "patch":
|
||||
description = "partially update" + sub + article + b.kind
|
||||
case "deletecollection":
|
||||
@ -219,7 +222,7 @@ func (b *builder) descriptionFor(path, verb string) string {
|
||||
}
|
||||
description = "delete collection of" + sub + " " + b.kind
|
||||
default:
|
||||
description = verb + sub + article + b.kind
|
||||
description = operationVerb + sub + article + b.kind
|
||||
}
|
||||
|
||||
return description
|
||||
@ -229,29 +232,35 @@ func (b *builder) descriptionFor(path, verb string) string {
|
||||
// action can be one of: GET, PUT, PATCH, POST, DELETE;
|
||||
// verb can be one of: list, read, replace, patch, create, delete, deletecollection;
|
||||
// sample is the sample Go type for response type.
|
||||
func (b *builder) buildRoute(root, path, action, verb string, sample interface{}) *restful.RouteBuilder {
|
||||
func (b *builder) buildRoute(root, path, httpMethod, actionVerb, operationVerb string, sample interface{}) *restful.RouteBuilder {
|
||||
var namespaced string
|
||||
if b.namespaced {
|
||||
namespaced = "Namespaced"
|
||||
}
|
||||
route := b.ws.Method(action).
|
||||
route := b.ws.Method(httpMethod).
|
||||
Path(root+path).
|
||||
To(func(req *restful.Request, res *restful.Response) {}).
|
||||
Doc(b.descriptionFor(path, verb)).
|
||||
Doc(b.descriptionFor(path, operationVerb)).
|
||||
Param(b.ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")).
|
||||
Operation(verb+namespaced+b.kind+strings.Title(subresource(path))).
|
||||
Operation(operationVerb+namespaced+b.kind+strings.Title(subresource(path))).
|
||||
Metadata(endpoints.ROUTE_META_GVK, metav1.GroupVersionKind{
|
||||
Group: b.group,
|
||||
Version: b.version,
|
||||
Kind: b.kind,
|
||||
}).
|
||||
Metadata(endpoints.ROUTE_META_ACTION, strings.ToLower(action)).
|
||||
Metadata(endpoints.ROUTE_META_ACTION, actionVerb).
|
||||
Produces("application/json", "application/yaml").
|
||||
Returns(http.StatusOK, "OK", sample).
|
||||
Writes(sample)
|
||||
if strings.Contains(root, namespaceToken) || strings.Contains(path, namespaceToken) {
|
||||
route.Param(b.ws.PathParameter("namespace", "object name and auth scope, such as for teams and projects").DataType("string"))
|
||||
}
|
||||
if strings.Contains(root, nameToken) || strings.Contains(path, nameToken) {
|
||||
route.Param(b.ws.PathParameter("name", "name of the "+b.kind).DataType("string"))
|
||||
}
|
||||
|
||||
// Build consume media types
|
||||
if action == "PATCH" {
|
||||
if httpMethod == "PATCH" {
|
||||
route.Consumes("application/json-patch+json",
|
||||
"application/merge-patch+json",
|
||||
"application/strategic-merge-patch+json")
|
||||
@ -260,16 +269,16 @@ func (b *builder) buildRoute(root, path, action, verb string, sample interface{}
|
||||
}
|
||||
|
||||
// Build option parameters
|
||||
switch verb {
|
||||
switch actionVerb {
|
||||
case "get":
|
||||
// TODO: CRD support for export is still under consideration
|
||||
endpoints.AddObjectParams(b.ws, route, &metav1.GetOptions{})
|
||||
case "list", "deletecollection":
|
||||
endpoints.AddObjectParams(b.ws, route, &metav1.ListOptions{})
|
||||
case "replace", "patch":
|
||||
case "put", "patch":
|
||||
// TODO: PatchOption added in feature branch but not in master yet
|
||||
endpoints.AddObjectParams(b.ws, route, &metav1.UpdateOptions{})
|
||||
case "create":
|
||||
case "post":
|
||||
endpoints.AddObjectParams(b.ws, route, &metav1.CreateOptions{})
|
||||
case "delete":
|
||||
endpoints.AddObjectParams(b.ws, route, &metav1.DeleteOptions{})
|
||||
@ -277,13 +286,13 @@ func (b *builder) buildRoute(root, path, action, verb string, sample interface{}
|
||||
}
|
||||
|
||||
// Build responses
|
||||
switch verb {
|
||||
case "create":
|
||||
switch actionVerb {
|
||||
case "post":
|
||||
route.Returns(http.StatusAccepted, "Accepted", sample)
|
||||
route.Returns(http.StatusCreated, "Created", sample)
|
||||
case "delete":
|
||||
route.Returns(http.StatusAccepted, "Accepted", sample)
|
||||
case "replace":
|
||||
case "put":
|
||||
route.Returns(http.StatusCreated, "Created", sample)
|
||||
}
|
||||
|
||||
|
||||
@ -21,6 +21,8 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/go-openapi/spec"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
|
||||
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
|
||||
@ -28,6 +30,7 @@ import (
|
||||
"k8s.io/apimachinery/pkg/util/diff"
|
||||
"k8s.io/apimachinery/pkg/util/json"
|
||||
"k8s.io/apimachinery/pkg/util/sets"
|
||||
"k8s.io/apiserver/pkg/endpoints"
|
||||
)
|
||||
|
||||
func TestNewBuilder(t *testing.T) {
|
||||
@ -417,6 +420,107 @@ func TestNewBuilder(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCRDRouteParameterBuilder(t *testing.T) {
|
||||
testCRDKind := "Foo"
|
||||
testCRDGroup := "foo-group"
|
||||
testCRDVersion := "foo-version"
|
||||
testCRDResourceName := "foos"
|
||||
|
||||
testCases := []struct {
|
||||
scope apiextensions.ResourceScope
|
||||
paths map[string]struct {
|
||||
expectNamespaceParam bool
|
||||
expectNameParam bool
|
||||
expectedActions sets.String
|
||||
}
|
||||
}{
|
||||
{
|
||||
scope: apiextensions.NamespaceScoped,
|
||||
paths: map[string]struct {
|
||||
expectNamespaceParam bool
|
||||
expectNameParam bool
|
||||
expectedActions sets.String
|
||||
}{
|
||||
"/apis/foo-group/foo-version/foos": {expectNamespaceParam: false, expectNameParam: false, expectedActions: sets.NewString("list")},
|
||||
"/apis/foo-group/foo-version/namespaces/{namespace}/foos": {expectNamespaceParam: true, expectNameParam: false, expectedActions: sets.NewString("post", "list", "deletecollection")},
|
||||
"/apis/foo-group/foo-version/namespaces/{namespace}/foos/{name}": {expectNamespaceParam: true, expectNameParam: true, expectedActions: sets.NewString("get", "put", "patch", "delete")},
|
||||
"/apis/foo-group/foo-version/namespaces/{namespace}/foos/{name}/scale": {expectNamespaceParam: true, expectNameParam: true, expectedActions: sets.NewString("get", "patch", "put")},
|
||||
"/apis/foo-group/foo-version/namespaces/{namespace}/foos/{name}/status": {expectNamespaceParam: true, expectNameParam: true, expectedActions: sets.NewString("get", "patch", "put")},
|
||||
},
|
||||
},
|
||||
{
|
||||
scope: apiextensions.ClusterScoped,
|
||||
paths: map[string]struct {
|
||||
expectNamespaceParam bool
|
||||
expectNameParam bool
|
||||
expectedActions sets.String
|
||||
}{
|
||||
"/apis/foo-group/foo-version/foos": {expectNamespaceParam: false, expectNameParam: false, expectedActions: sets.NewString("post", "list", "deletecollection")},
|
||||
"/apis/foo-group/foo-version/foos/{name}": {expectNamespaceParam: false, expectNameParam: true, expectedActions: sets.NewString("get", "put", "patch", "delete")},
|
||||
"/apis/foo-group/foo-version/foos/{name}/scale": {expectNamespaceParam: false, expectNameParam: true, expectedActions: sets.NewString("get", "patch", "put")},
|
||||
"/apis/foo-group/foo-version/foos/{name}/status": {expectNamespaceParam: false, expectNameParam: true, expectedActions: sets.NewString("get", "patch", "put")},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range testCases {
|
||||
testNamespacedCRD := &apiextensions.CustomResourceDefinition{
|
||||
Spec: apiextensions.CustomResourceDefinitionSpec{
|
||||
Scope: testCase.scope,
|
||||
Group: testCRDGroup,
|
||||
Names: apiextensions.CustomResourceDefinitionNames{
|
||||
Kind: testCRDKind,
|
||||
Plural: testCRDResourceName,
|
||||
},
|
||||
Versions: []apiextensions.CustomResourceDefinitionVersion{
|
||||
{
|
||||
Name: testCRDVersion,
|
||||
},
|
||||
},
|
||||
Subresources: &apiextensions.CustomResourceSubresources{
|
||||
Status: &apiextensions.CustomResourceSubresourceStatus{},
|
||||
Scale: &apiextensions.CustomResourceSubresourceScale{},
|
||||
},
|
||||
},
|
||||
}
|
||||
swagger, err := BuildSwagger(testNamespacedCRD, testCRDVersion)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, len(testCase.paths), len(swagger.Paths.Paths), testCase.scope)
|
||||
for path, expected := range testCase.paths {
|
||||
t.Run(path, func(t *testing.T) {
|
||||
path, ok := swagger.Paths.Paths[path]
|
||||
if !ok {
|
||||
t.Errorf("unexpected path %v", path)
|
||||
}
|
||||
|
||||
hasNamespaceParam := false
|
||||
hasNameParam := false
|
||||
for _, param := range path.Parameters {
|
||||
if param.In == "path" && param.Name == "namespace" {
|
||||
hasNamespaceParam = true
|
||||
}
|
||||
if param.In == "path" && param.Name == "name" {
|
||||
hasNameParam = true
|
||||
}
|
||||
}
|
||||
assert.Equal(t, expected.expectNamespaceParam, hasNamespaceParam)
|
||||
assert.Equal(t, expected.expectNameParam, hasNameParam)
|
||||
|
||||
actions := sets.NewString()
|
||||
for _, operation := range []*spec.Operation{path.Get, path.Post, path.Put, path.Patch, path.Delete} {
|
||||
if operation != nil {
|
||||
action, ok := operation.VendorExtensible.Extensions.GetString(endpoints.ROUTE_META_ACTION)
|
||||
if ok {
|
||||
actions.Insert(action)
|
||||
}
|
||||
}
|
||||
}
|
||||
assert.Equal(t, expected.expectedActions, actions)
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func properties(p map[string]spec.Schema) sets.String {
|
||||
ret := sets.NewString()
|
||||
for k := range p {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user