Skip to content

Commit

Permalink
Merge pull request #117918 from seans3/automated-cherry-pick-of-#117768-
Browse files Browse the repository at this point in the history 
#117796-origin-release-1.27

Automated cherry pick of #117768: QueryParamVerifierV3 resilient to minimal OpenAPI V3
#117796: QueryParamVerifier falls back on invalid v3 document
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored May 11, 2023
2 parents 4c39cdc + 642ea89 commit 74a7d8a
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 21 deletions.
13 changes: 8 additions & 5 deletions staging/src/k8s.io/cli-runtime/pkg/resource/fallback_query_param_verifier.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package resource

import (
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/klog/v2"
)
Expand All @@ -44,12 +43,16 @@ func NewFallbackQueryParamVerifier(primary Verifier, secondary Verifier) Verifie
// HasSupport returns an error if the passed GVK does not support the
// query param (fieldValidation), as determined by the primary and
// secondary OpenAPI endpoints. The primary endoint is checked first,
// but if it not found, the secondary attempts to determine support.
// If the GVK supports the query param, nil is returned.
// but if there is an error retrieving the OpenAPI V3 document, the
// secondary attempts to determine support. If the GVK supports the query param,
// nil is returned.
func (f *fallbackQueryParamVerifier) HasSupport(gvk schema.GroupVersionKind) error {
err := f.primary.HasSupport(gvk)
if errors.IsNotFound(err) {
klog.V(7).Infoln("openapi v3 endpoint not found...falling back to legacy")
// If an error was returned from the primary OpenAPI endpoint,
// we fallback to check the secondary OpenAPI endpoint for
// any error *except* "paramUnsupportedError".
if err != nil && !IsParamUnsupportedError(err) {
klog.V(7).Infof("openapi v3 error...falling back to legacy: %s", err)
err = f.secondary.HasSupport(gvk)
}
return err
Expand Down
58 changes: 55 additions & 3 deletions staging/src/k8s.io/cli-runtime/pkg/resource/fallback_query_param_verifier_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package resource

import (
"fmt"
"testing"

"k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -32,7 +33,6 @@ func TestFallbackQueryParamVerifier_PrimaryNoFallback(t *testing.T) {
crds []schema.GroupKind // CRDFinder returns these CRD's
gvk schema.GroupVersionKind // GVK whose OpenAPI spec is checked
queryParam VerifiableQueryParam // Usually "fieldValidation"
primaryError error
expectedSupports bool
}{
"Field validation query param is supported for batch/v1/Job, primary verifier": {
Expand Down Expand Up @@ -123,7 +123,8 @@ func TestFallbackQueryParamVerifier_PrimaryNoFallback(t *testing.T) {
for tn, tc := range tests {
t.Run(tn, func(t *testing.T) {
primary := createFakeV3Verifier(tc.crds, root, tc.queryParam)
secondary := createFakeLegacyVerifier(tc.crds, &fakeSchema, tc.queryParam)
// secondary verifier should not be called.
secondary := &failingVerifier{name: "secondary", t: t}
verifier := NewFallbackQueryParamVerifier(primary, secondary)
err := verifier.HasSupport(tc.gvk)
if tc.expectedSupports && err != nil {
Expand Down Expand Up @@ -151,6 +152,29 @@ func TestFallbackQueryParamVerifier_SecondaryFallback(t *testing.T) {
Kind: "Job",
},
queryParam: QueryParamFieldValidation,
primaryError: errors.NewNotFound(schema.GroupResource{}, "OpenAPI V3 endpoint not found"),
expectedSupports: true,
},
"Field validation query param is supported for batch/v1/Job, invalid v3 document error": {
crds: []schema.GroupKind{},
gvk: schema.GroupVersionKind{
Group: "batch",
Version: "v1",
Kind: "Job",
},
queryParam: QueryParamFieldValidation,
primaryError: fmt.Errorf("Invalid OpenAPI V3 document"),
expectedSupports: true,
},
"Field validation query param is supported for batch/v1/Job, timeout error": {
crds: []schema.GroupKind{},
gvk: schema.GroupVersionKind{
Group: "batch",
Version: "v1",
Kind: "Job",
},
queryParam: QueryParamFieldValidation,
primaryError: fmt.Errorf("timeout"),
expectedSupports: true,
},
"Field validation query param supported for core/v1/Namespace, secondary verifier": {
Expand All @@ -161,6 +185,7 @@ func TestFallbackQueryParamVerifier_SecondaryFallback(t *testing.T) {
Kind: "Namespace",
},
queryParam: QueryParamFieldValidation,
primaryError: errors.NewNotFound(schema.GroupResource{}, "OpenAPI V3 endpoint not found"),
expectedSupports: true,
},
"Field validation unsupported for unknown GVK, secondary verifier": {
Expand All @@ -171,6 +196,18 @@ func TestFallbackQueryParamVerifier_SecondaryFallback(t *testing.T) {
Kind: "Uknown",
},
queryParam: QueryParamFieldValidation,
primaryError: errors.NewNotFound(schema.GroupResource{}, "OpenAPI V3 endpoint not found"),
expectedSupports: false,
},
"Field validation unsupported for unknown GVK, invalid document causes secondary verifier": {
crds: []schema.GroupKind{},
gvk: schema.GroupVersionKind{
Group: "bad",
Version: "v1",
Kind: "Uknown",
},
queryParam: QueryParamFieldValidation,
primaryError: fmt.Errorf("Invalid OpenAPI V3 document"),
expectedSupports: false,
},
"Unknown query param unsupported (for all GVK's), secondary verifier": {
Expand All @@ -181,6 +218,7 @@ func TestFallbackQueryParamVerifier_SecondaryFallback(t *testing.T) {
Kind: "Deployment",
},
queryParam: "UnknownQueryParam",
primaryError: errors.NewNotFound(schema.GroupResource{}, "OpenAPI V3 endpoint not found"),
expectedSupports: false,
},
"Field validation query param supported for found CRD, secondary verifier": {
Expand All @@ -197,6 +235,7 @@ func TestFallbackQueryParamVerifier_SecondaryFallback(t *testing.T) {
Kind: "ExampleCRD",
},
queryParam: QueryParamFieldValidation,
primaryError: errors.NewNotFound(schema.GroupResource{}, "OpenAPI V3 endpoint not found"),
expectedSupports: true,
},
"Field validation query param unsupported for missing CRD, secondary verifier": {
Expand All @@ -213,6 +252,7 @@ func TestFallbackQueryParamVerifier_SecondaryFallback(t *testing.T) {
Kind: "ExampleCRD",
},
queryParam: QueryParamFieldValidation,
primaryError: errors.NewNotFound(schema.GroupResource{}, "OpenAPI V3 endpoint not found"),
expectedSupports: false,
},
"List GVK is specifically unsupported": {
Expand All @@ -223,16 +263,17 @@ func TestFallbackQueryParamVerifier_SecondaryFallback(t *testing.T) {
Kind: "List",
},
queryParam: QueryParamFieldValidation,
primaryError: errors.NewNotFound(schema.GroupResource{}, "OpenAPI V3 endpoint not found"),
expectedSupports: false,
},
}

// Primary OpenAPI client always returns "NotFound" error, so secondary verifier is used.
fakeOpenAPIClient := openapitest.NewFakeClient()
fakeOpenAPIClient.ForcedErr = errors.NewNotFound(schema.GroupResource{}, "OpenAPI V3 endpoint not found")
root := openapi3.NewRoot(fakeOpenAPIClient)
for tn, tc := range tests {
t.Run(tn, func(t *testing.T) {
fakeOpenAPIClient.ForcedErr = tc.primaryError
primary := createFakeV3Verifier(tc.crds, root, tc.queryParam)
secondary := createFakeLegacyVerifier(tc.crds, &fakeSchema, tc.queryParam)
verifier := NewFallbackQueryParamVerifier(primary, secondary)
Expand Down Expand Up @@ -269,3 +310,14 @@ func createFakeLegacyVerifier(crds []schema.GroupKind, fakeSchema discovery.Open
queryParam: queryParam,
}
}

// failingVerifier always crashes when called; implements Verifier
type failingVerifier struct {
name string
t *testing.T
}

func (c *failingVerifier) HasSupport(gvk schema.GroupVersionKind) error {
c.t.Fatalf("%s verifier should not be called", c.name)
return nil
}
28 changes: 15 additions & 13 deletions staging/src/k8s.io/cli-runtime/pkg/resource/query_param_verifier_v3.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package resource

import (
"fmt"

"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/openapi"
Expand Down Expand Up @@ -62,10 +64,7 @@ func (v *queryParamVerifierV3) HasSupport(gvk schema.GroupVersionKind) error {
}
gvSpec, err := v.root.GVSpec(gvk.GroupVersion())
if err == nil {
if supports := supportsQueryParamV3(gvSpec, gvk, v.queryParam); supports {
return nil
}
return NewParamUnsupportedError(gvk, v.queryParam)
return supportsQueryParamV3(gvSpec, gvk, v.queryParam)
}
if _, isErr := err.(*openapi3.GroupVersionNotFoundError); !isErr {
return err
Expand All @@ -78,9 +77,7 @@ func (v *queryParamVerifierV3) HasSupport(gvk schema.GroupVersionKind) error {
// If error retrieving Namespace spec, propagate error.
return err
}
if supports := supportsQueryParamV3(namespaceSpec, namespaceGVK, v.queryParam); supports {
return nil
}
return supportsQueryParamV3(namespaceSpec, namespaceGVK, v.queryParam)
}
return NewParamUnsupportedError(gvk, v.queryParam)
}
Expand All @@ -103,9 +100,14 @@ func hasGVKExtensionV3(extensions spec.Extensions, gvk schema.GroupVersionKind)

// supportsQueryParam is a method that let's us look in the OpenAPI if the
// specific group-version-kind supports the specific query parameter for
// the PATCH end-point. Returns true if the query param is supported by the
// spec for the passed GVK; false otherwise.
func supportsQueryParamV3(doc *spec3.OpenAPI, gvk schema.GroupVersionKind, queryParam VerifiableQueryParam) bool {
// the PATCH end-point. Returns nil if the passed GVK supports the passed
// query parameter; otherwise, a "paramUnsupportedError" is returned (except
// when an invalid document error is returned when an invalid OpenAPI V3
// is passed in).
func supportsQueryParamV3(doc *spec3.OpenAPI, gvk schema.GroupVersionKind, queryParam VerifiableQueryParam) error {
if doc == nil || doc.Paths == nil {
return fmt.Errorf("Invalid OpenAPI V3 document")
}
for _, path := range doc.Paths.Paths {
// If operation is not PATCH, then continue.
op := path.PathProps.Patch
Expand All @@ -120,10 +122,10 @@ func supportsQueryParamV3(doc *spec3.OpenAPI, gvk schema.GroupVersionKind, query
// for the PATCH operation.
for _, param := range op.OperationProps.Parameters {
if param.ParameterProps.Name == string(queryParam) {
return true
return nil
}
}
return false
return NewParamUnsupportedError(gvk, queryParam)
}
return false
return NewParamUnsupportedError(gvk, queryParam)
}
63 changes: 63 additions & 0 deletions staging/src/k8s.io/cli-runtime/pkg/resource/query_param_verifier_v3_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ limitations under the License.
package resource

import (
"strings"
"testing"

"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/openapi/cached"
"k8s.io/client-go/openapi/openapitest"
"k8s.io/client-go/openapi3"
"k8s.io/kube-openapi/pkg/spec3"
)

func TestV3SupportsQueryParamBatchV1(t *testing.T) {
Expand Down Expand Up @@ -135,3 +137,64 @@ func TestV3SupportsQueryParamBatchV1(t *testing.T) {
})
}
}

func TestInvalidOpenAPIV3Document(t *testing.T) {
tests := map[string]struct {
spec *spec3.OpenAPI
}{
"nil document returns error": {
spec: nil,
},
"empty document returns error": {
spec: &spec3.OpenAPI{},
},
"minimal document returns error": {
spec: &spec3.OpenAPI{
Version: "openapi 3.0.0",
Paths: nil,
},
},
}

gvk := schema.GroupVersionKind{
Group: "batch",
Version: "v1",
Kind: "Job",
}
for tn, tc := range tests {
t.Run(tn, func(t *testing.T) {

verifier := &queryParamVerifierV3{
finder: NewCRDFinder(func() ([]schema.GroupKind, error) {
return []schema.GroupKind{}, nil
}),
root: &fakeRoot{tc.spec},
queryParam: QueryParamFieldValidation,
}
err := verifier.HasSupport(gvk)
if !strings.Contains(err.Error(), "Invalid OpenAPI V3 document") {
t.Errorf("Expected invalid document error, but none received.")
}
})
}
}

// fakeRoot implements Root interface; manually specifies the returned OpenAPI V3 document.
type fakeRoot struct {
spec *spec3.OpenAPI
}

func (f *fakeRoot) GroupVersions() ([]schema.GroupVersion, error) {
// Unused
return nil, nil
}

// GVSpec returns hard-coded OpenAPI V3 document.
func (f *fakeRoot) GVSpec(gv schema.GroupVersion) (*spec3.OpenAPI, error) {
return f.spec, nil
}

func (f *fakeRoot) GVSpecAsMap(gv schema.GroupVersion) (map[string]interface{}, error) {
// Unused
return nil, nil
}

0 comments on commit 74a7d8a

Please sign in to comment.