diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/fallback_query_param_verifier.go b/staging/src/k8s.io/cli-runtime/pkg/resource/fallback_query_param_verifier.go index 198d5c9d5d2..05418801e8f 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/fallback_query_param_verifier.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/fallback_query_param_verifier.go @@ -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" ) @@ -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 diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/fallback_query_param_verifier_test.go b/staging/src/k8s.io/cli-runtime/pkg/resource/fallback_query_param_verifier_test.go index ae5a556789a..6dc0b6a0ae9 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/fallback_query_param_verifier_test.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/fallback_query_param_verifier_test.go @@ -17,6 +17,7 @@ limitations under the License. package resource import ( + "fmt" "testing" "k8s.io/apimachinery/pkg/api/errors" @@ -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": { @@ -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 { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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": { @@ -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) @@ -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 +} diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/query_param_verifier_v3.go b/staging/src/k8s.io/cli-runtime/pkg/resource/query_param_verifier_v3.go index 050174fd98e..502c58cf3d3 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/query_param_verifier_v3.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/query_param_verifier_v3.go @@ -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" @@ -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 @@ -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) } @@ -103,11 +100,13 @@ 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 false + return fmt.Errorf("Invalid OpenAPI V3 document") } for _, path := range doc.Paths.Paths { // If operation is not PATCH, then continue. @@ -123,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) } diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/query_param_verifier_v3_test.go b/staging/src/k8s.io/cli-runtime/pkg/resource/query_param_verifier_v3_test.go index e20fe9b7347..d3452ff2277 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/query_param_verifier_v3_test.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/query_param_verifier_v3_test.go @@ -17,6 +17,7 @@ limitations under the License. package resource import ( + "strings" "testing" "k8s.io/apimachinery/pkg/runtime/schema" @@ -141,24 +142,18 @@ func TestInvalidOpenAPIV3Document(t *testing.T) { tests := map[string]struct { spec *spec3.OpenAPI }{ - "nil document correctly returns Unsupported error": { + "nil document returns error": { spec: nil, }, - "empty document correctly returns Unsupported error": { + "empty document returns error": { spec: &spec3.OpenAPI{}, }, - "minimal document correctly returns Unsupported error": { + "minimal document returns error": { spec: &spec3.OpenAPI{ Version: "openapi 3.0.0", Paths: nil, }, }, - "document with empty Paths correctly returns Unsupported error": { - spec: &spec3.OpenAPI{ - Version: "openapi 3.0.0", - Paths: &spec3.Paths{}, - }, - }, } gvk := schema.GroupVersionKind{ @@ -177,8 +172,8 @@ func TestInvalidOpenAPIV3Document(t *testing.T) { queryParam: QueryParamFieldValidation, } err := verifier.HasSupport(gvk) - if err == nil { - t.Errorf("Expected not supports error, but none received.") + if !strings.Contains(err.Error(), "Invalid OpenAPI V3 document") { + t.Errorf("Expected invalid document error, but none received.") } }) }