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

@@ -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,
+				Mapping:       info.Mapping,
-				Helper:         helper,
+				Helper:        helper,
-				DynamicClient:  o.DynamicClient,
+				DynamicClient: o.DynamicClient,
-				DryRunVerifier: dryRunVerifier,
+				Overwrite:     o.Overwrite,
-				Overwrite:      o.Overwrite,
+				BackOff:       clockwork.NewRealClock(),
-				BackOff:        clockwork.NewRealClock(),
+				Force:         o.DeleteOptions.ForceDeletion,
-				Force:          o.DeleteOptions.ForceDeletion,
+				Cascade:       o.DeleteOptions.Cascade,
-				Cascade:        o.DeleteOptions.Cascade,
+				Timeout:       o.DeleteOptions.Timeout,
-				Timeout:        o.DeleteOptions.Timeout,
+				GracePeriod:   o.DeleteOptions.GracePeriod,
-				GracePeriod:    o.DeleteOptions.GracePeriod,
+				ServerDryRun:  o.ServerDryRun,
-				ServerDryRun:   o.ServerDryRun,
+				OpenapiSchema: openapiSchema,
-				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
+	Mapping       *meta.RESTMapping
-	Helper         *resource.Helper
+	Helper        *resource.Helper
-	DynamicClient  dynamic.Interface
+	DynamicClient dynamic.Interface
-	DryRunVerifier *DryRunVerifier
 
 	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 {

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
+	LocalObj runtime.Object
-	Info           *resource.Info
+	Info     *resource.Info
-	Encoder        runtime.Encoder
+	Encoder  runtime.Encoder
-	OpenAPI        openapi.Resources
+	OpenAPI  openapi.Resources
-	DryRunVerifier *apply.DryRunVerifier
 }
 
 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,
-		Mapping:        obj.Info.Mapping,
+		Helper:        resource.NewHelper(obj.Info.Client, obj.Info.Mapping),
-		Helper:         resource.NewHelper(obj.Info.Client, obj.Info.Mapping),
+		Overwrite:     true,
-		Overwrite:      true,
+		BackOff:       clockwork.NewRealClock(),
-		BackOff:        clockwork.NewRealClock(),
+		ServerDryRun:  true,
-		ServerDryRun:   true,
+		OpenapiSchema: obj.OpenAPI,
-		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,
+			LocalObj: local,
-			Info:           info,
+			Info:     info,
-			Encoder:        scheme.DefaultJSONEncoder(),
+			Encoder:  scheme.DefaultJSONEncoder(),
-			OpenAPI:        schema,
+			OpenAPI:  schema,
-			DryRunVerifier: dryRunVerifier,
 		}
 
 		return differ.Diff(obj, printer)