Skip to content

Commit

Permalink
Lazy load OpenAPIV2
Browse files Browse the repository at this point in the history
Kubernetes-commit: eb32969ab8f3d1455c33a595bba5b3ce4d8a03b8
  • Loading branch information
Jefftree authored and k8s-publishing-bot committed Oct 30, 2023
1 parent 8c8a04f commit 351ba8b
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 73 deletions.
17 changes: 9 additions & 8 deletions pkg/cmd/apply/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import (
"k8s.io/cli-runtime/pkg/printers"
"k8s.io/cli-runtime/pkg/resource"
"k8s.io/client-go/dynamic"
cachedopenapi "k8s.io/client-go/openapi/cached"
"k8s.io/client-go/openapi3"
"k8s.io/client-go/util/csaupgrade"
"k8s.io/component-base/version"
Expand Down Expand Up @@ -107,7 +106,7 @@ type ApplyOptions struct {
Builder *resource.Builder
Mapper meta.RESTMapper
DynamicClient dynamic.Interface
OpenAPISchema openapi.Resources
OpenAPIGetter openapi.OpenAPIResourcesGetter
OpenAPIV3Root openapi3.Root

Namespace string
Expand Down Expand Up @@ -285,12 +284,14 @@ func (flags *ApplyFlags) ToOptions(f cmdutil.Factory, cmd *cobra.Command, baseNa
return nil, err
}

openAPISchema, _ := f.OpenAPISchema()
var openAPIV3Root openapi3.Root
openAPIV3Client, err := f.OpenAPIV3Client()
if err == nil && !cmdutil.OpenAPIV3Apply.IsDisabled() {
cachedOpenAPIV3Client := cachedopenapi.NewClient(openAPIV3Client)
openAPIV3Root = openapi3.NewRoot(cachedOpenAPIV3Client)
if !cmdutil.OpenAPIV3Patch.IsDisabled() {
openAPIV3Client, err := f.OpenAPIV3Client()
if err == nil {
openAPIV3Root = openapi3.NewRoot(openAPIV3Client)
} else {
klog.V(4).Infof("warning: OpenAPI V3 Patch is enabled but is unable to be loaded. Will fall back to OpenAPI V2")
}
}

validationDirective, err := cmdutil.GetValidationDirective(cmd)
Expand Down Expand Up @@ -369,7 +370,7 @@ func (flags *ApplyFlags) ToOptions(f cmdutil.Factory, cmd *cobra.Command, baseNa
Builder: builder,
Mapper: mapper,
DynamicClient: dynamicClient,
OpenAPISchema: openAPISchema,
OpenAPIGetter: f,
OpenAPIV3Root: openAPIV3Root,

IOStreams: flags.IOStreams,
Expand Down
79 changes: 61 additions & 18 deletions pkg/cmd/apply/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,9 @@ import (
)

var (
fakeSchema = sptest.Fake{Path: filepath.Join("..", "..", "..", "testdata", "openapi", "swagger.json")}
fakeOpenAPIV3Legacy = sptest.OpenAPIV3Getter{Path: filepath.Join("..", "..", "..", "testdata", "openapi", "v3", "api", "v1.json")}
fakeOpenAPIV3AppsV1 = sptest.OpenAPIV3Getter{Path: filepath.Join("..", "..", "..", "testdata", "openapi", "v3", "apis", "apps", "v1.json")}
// testingOpenAPISchemas = []testOpenAPISchema{FakeOpenAPISchema}
fakeSchema = sptest.Fake{Path: filepath.Join("..", "..", "..", "testdata", "openapi", "swagger.json")}
fakeOpenAPIV3Legacy = sptest.OpenAPIV3Getter{Path: filepath.Join("..", "..", "..", "testdata", "openapi", "v3", "api", "v1.json")}
fakeOpenAPIV3AppsV1 = sptest.OpenAPIV3Getter{Path: filepath.Join("..", "..", "..", "testdata", "openapi", "v3", "apis", "apps", "v1.json")}
testingOpenAPISchemas = []testOpenAPISchema{AlwaysErrorsOpenAPISchema, FakeOpenAPISchema}

AlwaysErrorsOpenAPISchema = testOpenAPISchema{
Expand All @@ -94,13 +93,9 @@ var (
return c, nil
},
}
OpenAPIV3PanicSchema = testOpenAPISchema{
AlwaysPanicSchema = testOpenAPISchema{
OpenAPISchemaFn: func() (openapi.Resources, error) {
s, err := fakeSchema.OpenAPISchema()
if err != nil {
return nil, err
}
return openapi.NewOpenAPIData(s)
panic("error, openAPIV2 should not be called")
},
OpenAPIV3ClientFunc: func() (openapiclient.Client, error) {
return &OpenAPIV3ClientAlwaysPanic{}, nil
Expand All @@ -116,14 +111,14 @@ func (o *OpenAPIV3ClientAlwaysPanic) Paths() (map[string]openapiclient.GroupVers
panic("Cannot get paths")
}

func noopOpenAPIV3Apply(t *testing.T, f func(t *testing.T)) {
func noopOpenAPIV3Patch(t *testing.T, f func(t *testing.T)) {
f(t)
}
func disableOpenAPIV3Apply(t *testing.T, f func(t *testing.T)) {
cmdtesting.WithAlphaEnvsDisabled([]cmdutil.FeatureGate{cmdutil.OpenAPIV3Apply}, t, f)
func disableOpenAPIV3Patch(t *testing.T, f func(t *testing.T)) {
cmdtesting.WithAlphaEnvsDisabled([]cmdutil.FeatureGate{cmdutil.OpenAPIV3Patch}, t, f)
}

var applyFeatureToggles = []func(*testing.T, func(t *testing.T)){noopOpenAPIV3Apply, disableOpenAPIV3Apply}
var applyFeatureToggles = []func(*testing.T, func(t *testing.T)){noopOpenAPIV3Patch, disableOpenAPIV3Patch}

type testOpenAPISchema struct {
OpenAPISchemaFn func() (openapi.Resources, error)
Expand Down Expand Up @@ -747,7 +742,7 @@ func TestApplyObjectWithoutAnnotation(t *testing.T) {
}
}

func TestOpenAPIV3ApplyFeatureFlag(t *testing.T) {
func TestOpenAPIV3PatchFeatureFlag(t *testing.T) {
// OpenAPIV3 smp apply is on by default.
// Test that users can disable it to use OpenAPI V2 smp
// An OpenAPI V3 root that always panics is used to ensure
Expand All @@ -757,7 +752,7 @@ func TestOpenAPIV3ApplyFeatureFlag(t *testing.T) {
pathRC := "/namespaces/test/replicationcontrollers/" + nameRC

t.Run("test apply when a local object is specified - openapi v2 smp", func(t *testing.T) {
disableOpenAPIV3Apply(t, func(t *testing.T) {
disableOpenAPIV3Patch(t, func(t *testing.T) {
tf := cmdtesting.NewTestFactory().WithNamespace("test")
defer tf.Cleanup()

Expand All @@ -778,8 +773,8 @@ func TestOpenAPIV3ApplyFeatureFlag(t *testing.T) {
}
}),
}
tf.OpenAPISchemaFunc = OpenAPIV3PanicSchema.OpenAPISchemaFn
tf.OpenAPIV3ClientFunc = OpenAPIV3PanicSchema.OpenAPIV3ClientFunc
tf.OpenAPISchemaFunc = FakeOpenAPISchema.OpenAPISchemaFn
tf.OpenAPIV3ClientFunc = AlwaysPanicSchema.OpenAPIV3ClientFunc
tf.ClientConfigVal = cmdtesting.DefaultClientConfig()

ioStreams, _, buf, errBuf := genericiooptions.NewTestIOStreams()
Expand All @@ -801,6 +796,54 @@ func TestOpenAPIV3ApplyFeatureFlag(t *testing.T) {

}

func TestOpenAPIV3DoesNotLoadV2(t *testing.T) {
cmdtesting.InitTestErrorHandler(t)
nameRC, currentRC := readAndAnnotateReplicationController(t, filenameRC)
pathRC := "/namespaces/test/replicationcontrollers/" + nameRC

t.Run("test apply when a local object is specified - openapi v3 smp", func(t *testing.T) {
tf := cmdtesting.NewTestFactory().WithNamespace("test")
defer tf.Cleanup()

tf.UnstructuredClient = &fake.RESTClient{
NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer,
Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
switch p, m := req.URL.Path, req.Method; {
case p == pathRC && m == "GET":
bodyRC := io.NopCloser(bytes.NewReader(currentRC))
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: bodyRC}, nil
case p == pathRC && m == "PATCH":
validatePatchApplication(t, req, types.StrategicMergePatchType)
bodyRC := io.NopCloser(bytes.NewReader(currentRC))
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: bodyRC}, nil
default:
t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
return nil, nil
}
}),
}
tf.OpenAPISchemaFunc = AlwaysPanicSchema.OpenAPISchemaFn
tf.OpenAPIV3ClientFunc = FakeOpenAPISchema.OpenAPIV3ClientFunc
tf.ClientConfigVal = cmdtesting.DefaultClientConfig()

ioStreams, _, buf, errBuf := genericiooptions.NewTestIOStreams()
cmd := NewCmdApply("kubectl", tf, ioStreams)
cmd.Flags().Set("filename", filenameRC)
cmd.Flags().Set("output", "name")
cmd.Run(cmd, []string{})

// uses the name from the file, not the response
expectRC := "replicationcontroller/" + nameRC + "\n"
if buf.String() != expectRC {
t.Fatalf("unexpected output: %s\nexpected: %s", buf.String(), expectRC)
}
if errBuf.String() != "" {
t.Fatalf("unexpected error output: %s", errBuf.String())
}
})

}

func TestApplyObject(t *testing.T) {
cmdtesting.InitTestErrorHandler(t)
nameRC, currentRC := readAndAnnotateReplicationController(t, filenameRC)
Expand Down
68 changes: 35 additions & 33 deletions pkg/cmd/apply/patcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,17 @@ type Patcher struct {
// Number of retries to make if the patch fails with conflict
Retries int

OpenapiSchema openapi.Resources
OpenAPIGetter openapi.OpenAPIResourcesGetter
OpenAPIV3Root openapi3.Root
}

func newPatcher(o *ApplyOptions, info *resource.Info, helper *resource.Helper) (*Patcher, error) {
var openapiSchema openapi.Resources
var openAPIGetter openapi.OpenAPIResourcesGetter
var openAPIV3Root openapi3.Root

if o.OpenAPIPatch {
openapiSchema = o.OpenAPISchema
openAPIGetter = o.OpenAPIGetter
openAPIV3Root = o.OpenAPIV3Root
}

return &Patcher{
Expand All @@ -99,8 +102,8 @@ func newPatcher(o *ApplyOptions, info *resource.Info, helper *resource.Helper) (
CascadingStrategy: o.DeleteOptions.CascadingStrategy,
Timeout: o.DeleteOptions.Timeout,
GracePeriod: o.DeleteOptions.GracePeriod,
OpenapiSchema: openapiSchema,
OpenAPIV3Root: o.OpenAPIV3Root,
OpenAPIGetter: openAPIGetter,
OpenAPIV3Root: openAPIV3Root,
Retries: maxPatchRetry,
}, nil
}
Expand Down Expand Up @@ -135,34 +138,34 @@ func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, namespace, na
// with OpenAPI V3 not present in V2.
klog.V(5).Infof("warning: OpenAPI V3 path does not exist - group: %s, version %s, kind %s\n",
p.Mapping.GroupVersionKind.Group, p.Mapping.GroupVersionKind.Version, p.Mapping.GroupVersionKind.Kind)
} else {
if gvkSupported {
patch, err = p.buildStrategicMergePatchFromOpenAPIV3(original, modified, current)
if err != nil {
// Fall back to OpenAPI V2 if there is a problem
// We should remove the fallback in the future,
// but for the first release it might be beneficial
// to fall back to OpenAPI V2 while logging the error
// and seeing if we get any bug reports.
fmt.Fprintf(errOut, "warning: error calculating patch from openapi v3 spec: %v\n", err)
} else {
patchType = types.StrategicMergePatchType
}
} else if gvkSupported {
patch, err = p.buildStrategicMergePatchFromOpenAPIV3(original, modified, current)
if err != nil {
// Fall back to OpenAPI V2 if there is a problem
// We should remove the fallback in the future,
// but for the first release it might be beneficial
// to fall back to OpenAPI V2 while logging the error
// and seeing if we get any bug reports.
fmt.Fprintf(errOut, "warning: error calculating patch from openapi v3 spec: %v\n", err)
} else {
klog.V(5).Infof("warning: OpenAPI V3 path does not support strategic merge patch - group: %s, version %s, kind %s\n",
p.Mapping.GroupVersionKind.Group, p.Mapping.GroupVersionKind.Version, p.Mapping.GroupVersionKind.Kind)
patchType = types.StrategicMergePatchType
}
} else {
klog.V(5).Infof("warning: OpenAPI V3 path does not support strategic merge patch - group: %s, version %s, kind %s\n",
p.Mapping.GroupVersionKind.Group, p.Mapping.GroupVersionKind.Version, p.Mapping.GroupVersionKind.Kind)
}
}

if patch == nil && p.OpenapiSchema != nil {
// if openapischema is used, we'll try to get required patch type for this GVK from Open API.
// if it fails or could not find any patch type, fall back to baked-in patch type determination.
if patchType, err = p.getPatchTypeFromOpenAPI(p.Mapping.GroupVersionKind); err == nil && patchType == types.StrategicMergePatchType {
patch, err = p.buildStrategicMergeFromOpenAPI(original, modified, current)
if err != nil {
// Warn user about problem and continue strategic merge patching using builtin types.
fmt.Fprintf(errOut, "warning: error calculating patch from openapi spec: %v\n", err)
if patch == nil {
if openAPISchema, err := p.OpenAPIGetter.OpenAPISchema(); err == nil && openAPISchema != nil {
// if openapischema is used, we'll try to get required patch type for this GVK from Open API.
// if it fails or could not find any patch type, fall back to baked-in patch type determination.
if patchType, err = p.getPatchTypeFromOpenAPI(openAPISchema, p.Mapping.GroupVersionKind); err == nil && patchType == types.StrategicMergePatchType {
patch, err = p.buildStrategicMergeFromOpenAPI(openAPISchema, original, modified, current)
if err != nil {
// Warn user about problem and continue strategic merge patching using builtin types.
fmt.Fprintf(errOut, "warning: error calculating patch from openapi spec: %v\n", err)
}
}
}
}
Expand Down Expand Up @@ -222,7 +225,6 @@ func (p *Patcher) buildMergePatch(original, modified, current []byte) ([]byte, e
// gvkSupportsPatchOpenAPIV3 checks if a particular GVK supports the patch operation.
// It returns an error if the OpenAPI V3 could not be downloaded.
func (p *Patcher) gvkSupportsPatchOpenAPIV3(gvk schema.GroupVersionKind) (bool, error) {
// Bypassing root to save apiserver memory.
gvSpec, err := p.OpenAPIV3Root.GVSpec(schema.GroupVersion{
Group: p.Mapping.GroupVersionKind.Group,
Version: p.Mapping.GroupVersionKind.Version,
Expand Down Expand Up @@ -306,8 +308,8 @@ func (p *Patcher) buildStrategicMergePatchFromOpenAPIV3(original, modified, curr

// buildStrategicMergeFromOpenAPI builds patch from OpenAPI if it is enabled.
// This is used for core types which is published in openapi.
func (p *Patcher) buildStrategicMergeFromOpenAPI(original, modified, current []byte) ([]byte, error) {
schema := p.OpenapiSchema.LookupResource(p.Mapping.GroupVersionKind)
func (p *Patcher) buildStrategicMergeFromOpenAPI(openAPISchema openapi.Resources, original, modified, current []byte) ([]byte, error) {
schema := openAPISchema.LookupResource(p.Mapping.GroupVersionKind)
if schema == nil {
// Missing schema returns nil patch; also no error.
return nil, nil
Expand All @@ -321,8 +323,8 @@ func (p *Patcher) buildStrategicMergeFromOpenAPI(original, modified, current []b
}

// getPatchTypeFromOpenAPI looks up patch types supported by given GroupVersionKind in Open API.
func (p *Patcher) getPatchTypeFromOpenAPI(gvk schema.GroupVersionKind) (types.PatchType, error) {
if pc := p.OpenapiSchema.GetConsumes(p.Mapping.GroupVersionKind, "PATCH"); pc != nil {
func (p *Patcher) getPatchTypeFromOpenAPI(openAPISchema openapi.Resources, gvk schema.GroupVersionKind) (types.PatchType, error) {
if pc := openAPISchema.GetConsumes(p.Mapping.GroupVersionKind, "PATCH"); pc != nil {
for _, c := range pc {
if c == string(types.StrategicMergePatchType) {
return types.StrategicMergePatchType, nil
Expand Down
24 changes: 12 additions & 12 deletions pkg/cmd/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ type DiffOptions struct {

Concurrency int
Selector string
OpenAPISchema openapi.Resources
OpenAPIGetter openapi.OpenAPIResourcesGetter
OpenAPIV3Root openapi3.Root
DynamicClient dynamic.Interface
CmdNamespace string
Expand Down Expand Up @@ -325,7 +325,7 @@ type InfoObject struct {
LocalObj runtime.Object
Info *resource.Info
Encoder runtime.Encoder
OpenAPI openapi.Resources
OpenAPIGetter openapi.OpenAPIResourcesGetter
OpenAPIV3Root openapi3.Root
Force bool
ServerSideApply bool
Expand Down Expand Up @@ -398,7 +398,7 @@ func (obj InfoObject) Merged() (runtime.Object, error) {
Helper: helper,
Overwrite: true,
BackOff: clockwork.NewRealClock(),
OpenapiSchema: obj.OpenAPI,
OpenAPIGetter: obj.OpenAPIGetter,
OpenAPIV3Root: obj.OpenAPIV3Root,
ResourceVersion: resourceVersion,
}
Expand Down Expand Up @@ -641,15 +641,15 @@ func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []str
}

if !o.ServerSideApply {
o.OpenAPISchema, err = f.OpenAPISchema()
if err != nil {
return err
}
openAPIV3Client, err := f.OpenAPIV3Client()
if err != nil {
return err
o.OpenAPIGetter = f
if !cmdutil.OpenAPIV3Patch.IsDisabled() {
openAPIV3Client, err := f.OpenAPIV3Client()
if err == nil {
o.OpenAPIV3Root = openapi3.NewRoot(openAPIV3Client)
} else {
klog.V(4).Infof("warning: OpenAPI V3 Patch is enabled but is unable to be loaded. Will fall back to OpenAPI V2")
}
}
o.OpenAPIV3Root = openapi3.NewRoot(openAPIV3Client)
}

o.DynamicClient, err = f.DynamicClient()
Expand Down Expand Up @@ -730,7 +730,7 @@ func (o *DiffOptions) Run() error {
LocalObj: local,
Info: info,
Encoder: scheme.DefaultJSONEncoder(),
OpenAPI: o.OpenAPISchema,
OpenAPIGetter: o.OpenAPIGetter,
OpenAPIV3Root: o.OpenAPIV3Root,
Force: force,
ServerSideApply: o.ServerSideApply,
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/util/factory_client_access.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,5 +214,5 @@ func (f *factoryImpl) OpenAPIV3Client() (openapiclient.Client, error) {
return nil, err
}

return discovery.OpenAPIV3(), nil
return cached.NewClient(discovery.OpenAPIV3()), nil
}
2 changes: 1 addition & 1 deletion pkg/cmd/util/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ const (
ApplySet FeatureGate = "KUBECTL_APPLYSET"
CmdPluginAsSubcommand FeatureGate = "KUBECTL_ENABLE_CMD_SHADOW"
InteractiveDelete FeatureGate = "KUBECTL_INTERACTIVE_DELETE"
OpenAPIV3Apply FeatureGate = "KUBECTL_OPENAPIV3_APPLY"
OpenAPIV3Patch FeatureGate = "KUBECTL_OPENAPIV3_PATCH"
RemoteCommandWebsockets FeatureGate = "KUBECTL_REMOTE_COMMAND_WEBSOCKETS"
)

Expand Down

0 comments on commit 351ba8b

Please sign in to comment.