From 54889d581a35acf940d52a8a384cccaa0b597ddc Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Fri, 12 Oct 2018 11:30:49 -0700 Subject: [PATCH] kubectl dry-run: Check for server didn't work on create We changed the code to verify that the type supports dry-run, but we've only done it for patch, not for create, which means that people can create objects they don't want to create. We can actually simplify the code by moving the check much earlier in the method, which fixes all possible bug like this. --- pkg/kubectl/cmd/apply/apply.go | 54 ++++++++++++++++------------------ pkg/kubectl/cmd/diff/diff.go | 36 +++++++++++------------ 2 files changed, 44 insertions(+), 46 deletions(-) diff --git a/pkg/kubectl/cmd/apply/apply.go b/pkg/kubectl/cmd/apply/apply.go index b125c95cc5f..de8274169ea 100644 --- a/pkg/kubectl/cmd/apply/apply.go +++ b/pkg/kubectl/cmd/apply/apply.go @@ -25,7 +25,6 @@ import ( "github.com/golang/glog" "github.com/jonboulle/clockwork" "github.com/spf13/cobra" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -300,6 +299,11 @@ func (o *ApplyOptions) Run() error { openapiSchema = o.OpenAPISchema } + dryRunVerifier := &DryRunVerifier{ + Finder: cmdutil.NewCRDFinder(cmdutil.CRDFromDynamic(o.DynamicClient)), + OpenAPIGetter: o.DiscoveryClient, + } + // include the uninitialized objects by default if --prune is true // unless explicitly set --include-uninitialized=false r := o.Builder. @@ -361,6 +365,13 @@ func (o *ApplyOptions) Run() error { if !errors.IsNotFound(err) { return cmdutil.AddSourceToErr(fmt.Sprintf("retrieving current configuration of:\n%s\nfrom server for:", info.String()), info.Source, err) } + // If server-dry-run is requested but the type doesn't support it, fail right away. + if o.ServerDryRun { + if err := dryRunVerifier.HasSupport(info.Mapping.GroupVersionKind); err != nil { + return err + } + } + // Create the resource if it doesn't exist // First, update the annotation used by kubectl apply if err := kubectl.CreateApplyAnnotation(info.Object, unstructured.UnstructuredJSONScheme); err != nil { @@ -410,25 +421,19 @@ func (o *ApplyOptions) Run() error { fmt.Fprintf(o.ErrOut, warningNoLastAppliedConfigAnnotation, o.cmdBaseName) } - dryRunVerifier := &DryRunVerifier{ - Finder: cmdutil.NewCRDFinder(cmdutil.CRDFromDynamic(o.DynamicClient)), - OpenAPIGetter: o.DiscoveryClient, - } - helper := resource.NewHelper(info.Client, info.Mapping) patcher := &Patcher{ - Mapping: info.Mapping, - Helper: helper, - DynamicClient: o.DynamicClient, - DryRunVerifier: dryRunVerifier, - Overwrite: o.Overwrite, - BackOff: clockwork.NewRealClock(), - Force: o.DeleteOptions.ForceDeletion, - Cascade: o.DeleteOptions.Cascade, - Timeout: o.DeleteOptions.Timeout, - GracePeriod: o.DeleteOptions.GracePeriod, - ServerDryRun: o.ServerDryRun, - OpenapiSchema: openapiSchema, + Mapping: info.Mapping, + Helper: helper, + DynamicClient: o.DynamicClient, + Overwrite: o.Overwrite, + BackOff: clockwork.NewRealClock(), + Force: o.DeleteOptions.ForceDeletion, + Cascade: o.DeleteOptions.Cascade, + Timeout: o.DeleteOptions.Timeout, + GracePeriod: o.DeleteOptions.GracePeriod, + ServerDryRun: o.ServerDryRun, + OpenapiSchema: openapiSchema, } patchBytes, patchedObject, err := patcher.Patch(info.Object, modified, info.Source, info.Namespace, info.Name, o.ErrOut) @@ -681,10 +686,9 @@ func (p *Patcher) delete(namespace, name string) error { } type Patcher struct { - Mapping *meta.RESTMapping - Helper *resource.Helper - DynamicClient dynamic.Interface - DryRunVerifier *DryRunVerifier + Mapping *meta.RESTMapping + Helper *resource.Helper + DynamicClient dynamic.Interface Overwrite bool BackOff clockwork.Clock @@ -738,12 +742,6 @@ func (v *DryRunVerifier) HasSupport(gvk schema.GroupVersionKind) error { } func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, source, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, error) { - if p.ServerDryRun { - if err := p.DryRunVerifier.HasSupport(p.Mapping.GroupVersionKind); err != nil { - return nil, nil, err - } - } - // Serialize the current configuration of the object from the server. current, err := runtime.Encode(unstructured.UnstructuredJSONScheme, obj) if err != nil { diff --git a/pkg/kubectl/cmd/diff/diff.go b/pkg/kubectl/cmd/diff/diff.go index 18f8a3e3808..797a9fde1ff 100644 --- a/pkg/kubectl/cmd/diff/diff.go +++ b/pkg/kubectl/cmd/diff/diff.go @@ -26,7 +26,6 @@ import ( "github.com/ghodss/yaml" "github.com/jonboulle/clockwork" "github.com/spf13/cobra" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -226,11 +225,10 @@ type Object interface { // InfoObject is an implementation of the Object interface. It gets all // the information from the Info object. type InfoObject struct { - LocalObj runtime.Object - Info *resource.Info - Encoder runtime.Encoder - OpenAPI openapi.Resources - DryRunVerifier *apply.DryRunVerifier + LocalObj runtime.Object + Info *resource.Info + Encoder runtime.Encoder + OpenAPI openapi.Resources } var _ Object = &InfoObject{} @@ -262,13 +260,12 @@ func (obj InfoObject) Merged() (runtime.Object, error) { // This is using the patcher from apply, to keep the same behavior. // We plan on replacing this with server-side apply when it becomes available. patcher := &apply.Patcher{ - DryRunVerifier: obj.DryRunVerifier, - Mapping: obj.Info.Mapping, - Helper: resource.NewHelper(obj.Info.Client, obj.Info.Mapping), - Overwrite: true, - BackOff: clockwork.NewRealClock(), - ServerDryRun: true, - OpenapiSchema: obj.OpenAPI, + Mapping: obj.Info.Mapping, + Helper: resource.NewHelper(obj.Info.Client, obj.Info.Mapping), + Overwrite: true, + BackOff: clockwork.NewRealClock(), + ServerDryRun: true, + OpenapiSchema: obj.OpenAPI, } _, result, err := patcher.Patch(obj.Info.Object, modified, obj.Info.Source, obj.Info.Namespace, obj.Info.Name, nil) @@ -375,6 +372,10 @@ func RunDiff(f cmdutil.Factory, diff *DiffProgram, options *DiffOptions) error { return err } + if err := dryRunVerifier.HasSupport(info.Mapping.GroupVersionKind); err != nil { + return err + } + local := info.Object.DeepCopyObject() if err := info.Get(); err != nil { if !errors.IsNotFound(err) { @@ -384,11 +385,10 @@ func RunDiff(f cmdutil.Factory, diff *DiffProgram, options *DiffOptions) error { } obj := InfoObject{ - LocalObj: local, - Info: info, - Encoder: scheme.DefaultJSONEncoder(), - OpenAPI: schema, - DryRunVerifier: dryRunVerifier, + LocalObj: local, + Info: info, + Encoder: scheme.DefaultJSONEncoder(), + OpenAPI: schema, } return differ.Diff(obj, printer)