Unverified Commit 22c2807b authored by Kubernetes Prow Robot's avatar Kubernetes Prow Robot Committed by GitHub

Merge pull request #80851 from sttts/sttts-crd-scoping-1.14

1.14: apiextensions: check request scope against CRD scope correctly
parents df8cd112 449163c3
...@@ -52,6 +52,7 @@ go_library( ...@@ -52,6 +52,7 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/version:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/version:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
......
...@@ -52,6 +52,7 @@ import ( ...@@ -52,6 +52,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/serializer/versioning" "k8s.io/apimachinery/pkg/runtime/serializer/versioning"
"k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/endpoints/handlers" "k8s.io/apiserver/pkg/endpoints/handlers"
...@@ -169,6 +170,10 @@ func NewCustomResourceDefinitionHandler( ...@@ -169,6 +170,10 @@ func NewCustomResourceDefinitionHandler(
return ret, nil return ret, nil
} }
// possiblyAcrossAllNamespacesVerbs contains those verbs which can be per-namespace and across all
// namespaces for namespaces resources. I.e. for these an empty namespace in the requestInfo is fine.
var possiblyAcrossAllNamespacesVerbs = sets.NewString("list", "watch")
func (r *crdHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { func (r *crdHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
ctx := req.Context() ctx := req.Context()
requestInfo, ok := apirequest.RequestInfoFrom(ctx) requestInfo, ok := apirequest.RequestInfoFrom(ctx)
...@@ -204,10 +209,24 @@ func (r *crdHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { ...@@ -204,10 +209,24 @@ func (r *crdHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
http.Error(w, err.Error(), http.StatusInternalServerError) http.Error(w, err.Error(), http.StatusInternalServerError)
return return
} }
// if the scope in the CRD and the scope in request differ (with exception of the verbs in possiblyAcrossAllNamespacesVerbs
// for namespaced resources), pass request to the delegate, which is supposed to lead to a 404.
namespacedCRD, namespacedReq := crd.Spec.Scope == apiextensions.NamespaceScoped, len(requestInfo.Namespace) > 0
if !namespacedCRD && namespacedReq {
r.delegate.ServeHTTP(w, req)
return
}
if namespacedCRD && !namespacedReq && !possiblyAcrossAllNamespacesVerbs.Has(requestInfo.Verb) {
r.delegate.ServeHTTP(w, req)
return
}
if !apiextensions.HasServedCRDVersion(crd, requestInfo.APIVersion) { if !apiextensions.HasServedCRDVersion(crd, requestInfo.APIVersion) {
r.delegate.ServeHTTP(w, req) r.delegate.ServeHTTP(w, req)
return return
} }
// There is a small chance that a CRD is being served because NamesAccepted condition is true, // There is a small chance that a CRD is being served because NamesAccepted condition is true,
// but it becomes "unserved" because another names update leads to a conflict // but it becomes "unserved" because another names update leads to a conflict
// and EstablishingController wasn't fast enough to put the CRD into the Established condition. // and EstablishingController wasn't fast enough to put the CRD into the Established condition.
......
...@@ -14,6 +14,7 @@ go_test( ...@@ -14,6 +14,7 @@ go_test(
"finalization_test.go", "finalization_test.go",
"objectmeta_test.go", "objectmeta_test.go",
"registration_test.go", "registration_test.go",
"scope_test.go",
"subresources_test.go", "subresources_test.go",
"table_test.go", "table_test.go",
"validation_test.go", "validation_test.go",
......
/*
Copyright 2019 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package integration
import (
"strings"
"testing"
"github.com/stretchr/testify/assert"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/dynamic"
apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
"k8s.io/apiextensions-apiserver/test/integration/fixtures"
)
func TestHandlerScope(t *testing.T) {
tearDown, apiExtensionClient, dynamicClient, err := fixtures.StartDefaultServerWithClients(t)
if err != nil {
t.Fatal(err)
}
defer tearDown()
for _, scope := range []apiextensionsv1beta1.ResourceScope{apiextensionsv1beta1.ClusterScoped, apiextensionsv1beta1.NamespaceScoped} {
t.Run(string(scope), func(t *testing.T) {
crd := &apiextensionsv1beta1.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(string(scope)) + "s.test.apiextensions-apiserver.k8s.io"},
Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{
Group: "test.apiextensions-apiserver.k8s.io",
Version: "v1beta1",
Names: apiextensionsv1beta1.CustomResourceDefinitionNames{
Plural: strings.ToLower(string(scope)) + "s",
Singular: strings.ToLower(string(scope)),
Kind: string(scope),
ListKind: string(scope) + "List",
},
Scope: scope,
},
}
crd.Spec.Subresources = &apiextensionsv1beta1.CustomResourceSubresources{
Status: &apiextensionsv1beta1.CustomResourceSubresourceStatus{},
Scale: &apiextensionsv1beta1.CustomResourceSubresourceScale{
SpecReplicasPath: ".spec.replicas",
StatusReplicasPath: ".status.replicas",
},
}
crd, err = fixtures.CreateNewCustomResourceDefinition(crd, apiExtensionClient, dynamicClient)
if err != nil {
t.Fatal(err)
}
gvr := schema.GroupVersionResource{Group: crd.Spec.Group, Version: crd.Spec.Versions[0].Name, Resource: crd.Spec.Names.Plural}
ns := "test"
var client dynamic.ResourceInterface = dynamicClient.Resource(gvr)
var otherScopeClient dynamic.ResourceInterface = dynamicClient.Resource(gvr).Namespace(ns)
if crd.Spec.Scope == apiextensionsv1beta1.NamespaceScoped {
client, otherScopeClient = otherScopeClient, client
}
name := "bar"
cr := &unstructured.Unstructured{
Object: map[string]interface{}{
"kind": crd.Spec.Names.Kind,
"apiVersion": gvr.GroupVersion().String(),
"metadata": map[string]interface{}{
"name": name,
},
},
}
_, err := otherScopeClient.Create(cr, metav1.CreateOptions{})
assert.True(t, apierrors.IsNotFound(err))
_, err = otherScopeClient.Create(cr, metav1.CreateOptions{}, "status")
assert.True(t, apierrors.IsNotFound(err))
_, err = otherScopeClient.Create(cr, metav1.CreateOptions{}, "scale")
assert.True(t, apierrors.IsNotFound(err))
_, err = client.Create(cr, metav1.CreateOptions{})
assert.NoError(t, err)
_, err = otherScopeClient.Get(name, metav1.GetOptions{})
assert.True(t, apierrors.IsNotFound(err))
_, err = otherScopeClient.Get(name, metav1.GetOptions{}, "status")
assert.True(t, apierrors.IsNotFound(err))
_, err = otherScopeClient.Get(name, metav1.GetOptions{}, "scale")
assert.True(t, apierrors.IsNotFound(err))
_, err = otherScopeClient.Update(cr, metav1.UpdateOptions{})
assert.True(t, apierrors.IsNotFound(err))
_, err = otherScopeClient.Update(cr, metav1.UpdateOptions{}, "status")
assert.True(t, apierrors.IsNotFound(err))
_, err = otherScopeClient.Update(cr, metav1.UpdateOptions{}, "scale")
assert.True(t, apierrors.IsNotFound(err))
_, err = otherScopeClient.Patch(name, types.MergePatchType, []byte(`{"metadata":{"annotations":{"test":"1"}}}`), metav1.PatchOptions{})
assert.True(t, apierrors.IsNotFound(err))
_, err = otherScopeClient.Patch(name, types.MergePatchType, []byte(`{"metadata":{"annotations":{"test":"1"}}}`), metav1.PatchOptions{}, "status")
assert.True(t, apierrors.IsNotFound(err))
_, err = otherScopeClient.Patch(name, types.MergePatchType, []byte(`{"metadata":{"annotations":{"test":"1"}}}`), metav1.PatchOptions{}, "scale")
assert.True(t, apierrors.IsNotFound(err))
err = otherScopeClient.Delete(name, &metav1.DeleteOptions{})
assert.True(t, apierrors.IsNotFound(err))
err = otherScopeClient.Delete(name, &metav1.DeleteOptions{}, "status")
assert.True(t, apierrors.IsNotFound(err))
err = otherScopeClient.Delete(name, &metav1.DeleteOptions{}, "scale")
assert.True(t, apierrors.IsNotFound(err))
err = otherScopeClient.DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{})
assert.True(t, apierrors.IsNotFound(err))
if scope == apiextensionsv1beta1.ClusterScoped {
_, err = otherScopeClient.List(metav1.ListOptions{})
assert.True(t, apierrors.IsNotFound(err))
_, err = otherScopeClient.Watch(metav1.ListOptions{})
assert.True(t, apierrors.IsNotFound(err))
} else {
_, err = otherScopeClient.List(metav1.ListOptions{})
assert.NoError(t, err)
w, err := otherScopeClient.Watch(metav1.ListOptions{})
assert.NoError(t, err)
if w != nil {
w.Stop()
}
}
})
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment