mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-23 11:50:44 +00:00
Merge pull request #81436 from roycaihw/crd-openapi/skip-duplicate-path-definition
apiextensions: ignore path conflict and resolve definition conflict when merging openapi spec
This commit is contained in:
Kubernetes Prow Robot
GitHub
commit
21cdcd5776
@ -7,6 +7,7 @@ go 1.12
|
||||
require (
|
||||
github.com/coreos/etcd v3.3.15+incompatible
|
||||
github.com/emicklei/go-restful v2.9.5+incompatible
|
||||
github.com/ghodss/yaml v0.0.0-20180820084758-c7ce16629ff4 // indirect
|
||||
github.com/go-openapi/errors v0.19.2
|
||||
github.com/go-openapi/spec v0.19.2
|
||||
github.com/go-openapi/strfmt v0.19.0
|
||||
|
||||
@ -65,6 +65,8 @@ github.com/evanphx/json-patch v4.2.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLi
|
||||
github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I=
|
||||
github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
|
||||
github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
|
||||
github.com/ghodss/yaml v0.0.0-20180820084758-c7ce16629ff4 h1:bRzFpEzvausOAt4va+I/22BZ1vXDtERngp0BNYDKej0=
|
||||
github.com/ghodss/yaml v0.0.0-20180820084758-c7ce16629ff4/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
|
||||
github.com/globalsign/mgo v0.0.0-20180905125535-1ca0a4f7cbcb/go.mod h1:xkRDCp4j0OGD1HRkm4kmhM+pmpv3AKq5SU7GMg4oO/Q=
|
||||
github.com/globalsign/mgo v0.0.0-20181015135952-eeefdecb41b8 h1:DujepqpGd1hyOd7aW59XpK7Qymp8iy83xq74fLr21is=
|
||||
github.com/globalsign/mgo v0.0.0-20181015135952-eeefdecb41b8/go.mod h1:xkRDCp4j0OGD1HRkm4kmhM+pmpv3AKq5SU7GMg4oO/Q=
|
||||
|
||||
@ -27,6 +27,7 @@ go_library(
|
||||
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
|
||||
"//vendor/github.com/emicklei/go-restful:go_default_library",
|
||||
"//vendor/github.com/go-openapi/spec:go_default_library",
|
||||
"//vendor/k8s.io/kube-openapi/pkg/aggregator:go_default_library",
|
||||
"//vendor/k8s.io/kube-openapi/pkg/builder:go_default_library",
|
||||
"//vendor/k8s.io/kube-openapi/pkg/common:go_default_library",
|
||||
"//vendor/k8s.io/kube-openapi/pkg/util:go_default_library",
|
||||
|
||||
@ -18,12 +18,13 @@ package builder
|
||||
|
||||
import (
|
||||
"github.com/go-openapi/spec"
|
||||
"k8s.io/kube-openapi/pkg/aggregator"
|
||||
)
|
||||
|
||||
// MergeSpecs aggregates all OpenAPI specs, reusing the metadata of the first, static spec as the basis.
|
||||
// Later paths and definitions override earlier ones. None of the input is mutated, but input
|
||||
// and output share data structures.
|
||||
func MergeSpecs(staticSpec *spec.Swagger, crdSpecs ...*spec.Swagger) *spec.Swagger {
|
||||
// The static spec has the highest priority, and its paths and definitions won't get overlapped by
|
||||
// user-defined CRDs. None of the input is mutated, but input and output share data structures.
|
||||
func MergeSpecs(staticSpec *spec.Swagger, crdSpecs ...*spec.Swagger) (*spec.Swagger, error) {
|
||||
// create shallow copy of staticSpec, but replace paths and definitions because we modify them.
|
||||
specToReturn := *staticSpec
|
||||
if staticSpec.Definitions != nil {
|
||||
@ -41,11 +42,19 @@ func MergeSpecs(staticSpec *spec.Swagger, crdSpecs ...*spec.Swagger) *spec.Swagg
|
||||
}
|
||||
}
|
||||
|
||||
crdSpec := &spec.Swagger{}
|
||||
for _, s := range crdSpecs {
|
||||
mergeSpec(&specToReturn, s)
|
||||
// merge specs without checking conflicts, since the naming controller prevents
|
||||
// conflicts between user-defined CRDs
|
||||
mergeSpec(crdSpec, s)
|
||||
}
|
||||
|
||||
return &specToReturn
|
||||
// The static spec has the highest priority. Resolve conflicts to prevent user-defined
|
||||
// CRDs potentially overlapping the built-in apiextensions API
|
||||
if err := aggregator.MergeSpecsIgnorePathConflict(&specToReturn, crdSpec); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &specToReturn, nil
|
||||
}
|
||||
|
||||
// mergeSpec copies paths and definitions from source to dest, mutating dest, but not source.
|
||||
|
||||
@ -226,7 +226,11 @@ func (c *Controller) updateSpecLocked() error {
|
||||
crdSpecs = append(crdSpecs, s)
|
||||
}
|
||||
}
|
||||
return c.openAPIService.UpdateSpec(builder.MergeSpecs(c.staticSpec, crdSpecs...))
|
||||
mergedSpec, err := builder.MergeSpecs(c.staticSpec, crdSpecs...)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to merge specs: %v", err)
|
||||
}
|
||||
return c.openAPIService.UpdateSpec(mergedSpec)
|
||||
}
|
||||
|
||||
func (c *Controller) addCustomResourceDefinition(obj interface{}) {
|
||||
|
||||
@ -17,6 +17,7 @@ limitations under the License.
|
||||
package master
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
@ -26,18 +27,28 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/go-openapi/spec"
|
||||
appsv1 "k8s.io/api/apps/v1"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
|
||||
apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
|
||||
apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/util/wait"
|
||||
"k8s.io/apiserver/pkg/registry/generic/registry"
|
||||
"k8s.io/client-go/kubernetes"
|
||||
"k8s.io/kube-aggregator/pkg/apis/apiregistration"
|
||||
kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
|
||||
"k8s.io/kubernetes/test/integration/etcd"
|
||||
"k8s.io/kubernetes/test/integration/framework"
|
||||
)
|
||||
|
||||
const (
|
||||
// testApiextensionsOverlapProbeString is a probe string which identifies whether
|
||||
// a CRD change triggers an OpenAPI spec change
|
||||
testApiextensionsOverlapProbeString = "testApiextensionsOverlapProbeField"
|
||||
)
|
||||
|
||||
func TestRun(t *testing.T) {
|
||||
server := kubeapiservertesting.StartTestServerOrDie(t, nil, nil, framework.SharedEtcd())
|
||||
defer server.TearDownFn()
|
||||
@ -190,6 +201,204 @@ func TestOpenAPIDelegationChainPlumbing(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestOpenAPIApiextensionsOverlapProtection(t *testing.T) {
|
||||
server := kubeapiservertesting.StartTestServerOrDie(t, nil, nil, framework.SharedEtcd())
|
||||
defer server.TearDownFn()
|
||||
apiextensionsclient, err := apiextensionsclientset.NewForConfig(server.ClientConfig)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
crdPath, exist, err := getOpenAPIPath(apiextensionsclient, `/apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/{name}`)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error getting CRD OpenAPI path: %v", err)
|
||||
}
|
||||
if !exist {
|
||||
t.Fatalf("unexpected error: apiextensions OpenAPI path doesn't exist")
|
||||
}
|
||||
|
||||
// Create a CRD that overlaps OpenAPI path with the CRD API
|
||||
crd := &apiextensionsv1beta1.CustomResourceDefinition{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: "customresourcedefinitions.apiextensions.k8s.io",
|
||||
Annotations: map[string]string{"api-approved.kubernetes.io": "unapproved, test-only"},
|
||||
},
|
||||
Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{
|
||||
Group: "apiextensions.k8s.io",
|
||||
Version: "v1beta1",
|
||||
Scope: apiextensionsv1beta1.ClusterScoped,
|
||||
Names: apiextensionsv1beta1.CustomResourceDefinitionNames{
|
||||
Plural: "customresourcedefinitions",
|
||||
Singular: "customresourcedefinition",
|
||||
Kind: "CustomResourceDefinition",
|
||||
ListKind: "CustomResourceDefinitionList",
|
||||
},
|
||||
Validation: &apiextensionsv1beta1.CustomResourceValidation{
|
||||
OpenAPIV3Schema: &apiextensionsv1beta1.JSONSchemaProps{
|
||||
Type: "object",
|
||||
Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{
|
||||
testApiextensionsOverlapProbeString: {Type: "boolean"},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
etcd.CreateTestCRDs(t, apiextensionsclient, false, crd)
|
||||
|
||||
// Create a probe CRD foo that triggers an OpenAPI spec change
|
||||
if err := triggerSpecUpdateWithProbeCRD(t, apiextensionsclient, "foo"); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
// Expect the CRD path to not change
|
||||
path, _, err := getOpenAPIPath(apiextensionsclient, `/apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/{name}`)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
pathBytes, err := json.Marshal(path)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
crdPathBytes, err := json.Marshal(crdPath)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if !bytes.Equal(pathBytes, crdPathBytes) {
|
||||
t.Fatalf("expected CRD OpenAPI path to not change, but got different results: want %q, got %q", string(crdPathBytes), string(pathBytes))
|
||||
}
|
||||
|
||||
// Expect the orphan definition to be pruned from the spec
|
||||
exist, err = specHasProbe(apiextensionsclient, testApiextensionsOverlapProbeString)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if exist {
|
||||
t.Fatalf("unexpected error: orphan definition isn't pruned")
|
||||
}
|
||||
|
||||
// Create a CRD that overlaps OpenAPI definition with the CRD API
|
||||
crd = &apiextensionsv1beta1.CustomResourceDefinition{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: "customresourcedefinitions.apiextensions.apis.pkg.apiextensions-apiserver.k8s.io",
|
||||
Annotations: map[string]string{"api-approved.kubernetes.io": "unapproved, test-only"},
|
||||
},
|
||||
Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{
|
||||
Group: "apiextensions.apis.pkg.apiextensions-apiserver.k8s.io",
|
||||
Version: "v1beta1",
|
||||
Scope: apiextensionsv1beta1.ClusterScoped,
|
||||
Names: apiextensionsv1beta1.CustomResourceDefinitionNames{
|
||||
Plural: "customresourcedefinitions",
|
||||
Singular: "customresourcedefinition",
|
||||
Kind: "CustomResourceDefinition",
|
||||
ListKind: "CustomResourceDefinitionList",
|
||||
},
|
||||
Validation: &apiextensionsv1beta1.CustomResourceValidation{
|
||||
OpenAPIV3Schema: &apiextensionsv1beta1.JSONSchemaProps{
|
||||
Type: "object",
|
||||
Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{
|
||||
testApiextensionsOverlapProbeString: {Type: "boolean"},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
etcd.CreateTestCRDs(t, apiextensionsclient, false, crd)
|
||||
|
||||
// Create a probe CRD bar that triggers an OpenAPI spec change
|
||||
if err := triggerSpecUpdateWithProbeCRD(t, apiextensionsclient, "bar"); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
// Expect the apiextensions definition to not change, since the overlapping definition will get renamed.
|
||||
apiextensionsDefinition, exist, err := getOpenAPIDefinition(apiextensionsclient, `io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1beta1.CustomResourceDefinition`)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if !exist {
|
||||
t.Fatalf("unexpected error: apiextensions definition doesn't exist")
|
||||
}
|
||||
bytes, err := json.Marshal(apiextensionsDefinition)
|
||||
if exist := strings.Contains(string(bytes), testApiextensionsOverlapProbeString); exist {
|
||||
t.Fatalf("unexpected error: apiextensions definition gets overlapped")
|
||||
}
|
||||
}
|
||||
|
||||
// triggerSpecUpdateWithProbeCRD creates a probe CRD with suffix in name, and waits until
|
||||
// the path and definition for the probe CRD show up in the OpenAPI spec
|
||||
func triggerSpecUpdateWithProbeCRD(t *testing.T, apiextensionsclient *apiextensionsclientset.Clientset, suffix string) error {
|
||||
// Create a probe CRD that triggers OpenAPI spec change
|
||||
name := fmt.Sprintf("integration-test-%s-crd", suffix)
|
||||
kind := fmt.Sprintf("Integration-test-%s-crd", suffix)
|
||||
group := "probe.test.com"
|
||||
crd := &apiextensionsv1beta1.CustomResourceDefinition{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: name + "s." + group},
|
||||
Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{
|
||||
Group: group,
|
||||
Version: "v1",
|
||||
Scope: apiextensionsv1beta1.ClusterScoped,
|
||||
Names: apiextensionsv1beta1.CustomResourceDefinitionNames{
|
||||
Plural: name + "s",
|
||||
Singular: name,
|
||||
Kind: kind,
|
||||
ListKind: kind + "List",
|
||||
},
|
||||
},
|
||||
}
|
||||
etcd.CreateTestCRDs(t, apiextensionsclient, false, crd)
|
||||
|
||||
// Expect the probe CRD path to show up in the OpenAPI spec
|
||||
// TODO(roycaihw): expose response header in rest client and utilize etag here
|
||||
if err := wait.Poll(1*time.Second, wait.ForeverTestTimeout, func() (bool, error) {
|
||||
_, exist, err := getOpenAPIPath(apiextensionsclient, fmt.Sprintf(`/apis/%s/v1/%ss/{name}`, group, name))
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
return exist, nil
|
||||
}); err != nil {
|
||||
return fmt.Errorf("failed to observe probe CRD path in the spec: %v", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func specHasProbe(clientset *apiextensionsclientset.Clientset, probe string) (bool, error) {
|
||||
bs, err := clientset.RESTClient().Get().AbsPath("openapi", "v2").DoRaw()
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
return strings.Contains(string(bs), probe), nil
|
||||
}
|
||||
|
||||
func getOpenAPIPath(clientset *apiextensionsclientset.Clientset, path string) (spec.PathItem, bool, error) {
|
||||
bs, err := clientset.RESTClient().Get().AbsPath("openapi", "v2").DoRaw()
|
||||
if err != nil {
|
||||
return spec.PathItem{}, false, err
|
||||
}
|
||||
s := spec.Swagger{}
|
||||
if err := json.Unmarshal(bs, &s); err != nil {
|
||||
return spec.PathItem{}, false, err
|
||||
}
|
||||
if s.SwaggerProps.Paths == nil {
|
||||
return spec.PathItem{}, false, fmt.Errorf("unexpected empty path")
|
||||
}
|
||||
value, ok := s.SwaggerProps.Paths.Paths[path]
|
||||
return value, ok, nil
|
||||
}
|
||||
|
||||
func getOpenAPIDefinition(clientset *apiextensionsclientset.Clientset, definition string) (spec.Schema, bool, error) {
|
||||
bs, err := clientset.RESTClient().Get().AbsPath("openapi", "v2").DoRaw()
|
||||
if err != nil {
|
||||
return spec.Schema{}, false, err
|
||||
}
|
||||
s := spec.Swagger{}
|
||||
if err := json.Unmarshal(bs, &s); err != nil {
|
||||
return spec.Schema{}, false, err
|
||||
}
|
||||
if s.SwaggerProps.Definitions == nil {
|
||||
return spec.Schema{}, false, fmt.Errorf("unexpected empty path")
|
||||
}
|
||||
value, ok := s.SwaggerProps.Definitions[definition]
|
||||
return value, ok, nil
|
||||
}
|
||||
|
||||
// return the unique endpoint IPs
|
||||
func getEndpointIPs(endpoints *corev1.Endpoints) []string {
|
||||
endpointMap := make(map[string]bool)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user