diff --git a/pkg/kubectl/cmd/apply/BUILD b/pkg/kubectl/cmd/apply/BUILD index a29c7e77e0..0b16d0c8e9 100644 --- a/pkg/kubectl/cmd/apply/BUILD +++ b/pkg/kubectl/cmd/apply/BUILD @@ -36,6 +36,7 @@ go_library( "//staging/src/k8s.io/cli-runtime/pkg/genericclioptions:go_default_library", "//staging/src/k8s.io/cli-runtime/pkg/genericclioptions/printers:go_default_library", "//staging/src/k8s.io/cli-runtime/pkg/genericclioptions/resource:go_default_library", + "//staging/src/k8s.io/client-go/discovery:go_default_library", "//staging/src/k8s.io/client-go/dynamic:go_default_library", "//vendor/github.com/ghodss/yaml:go_default_library", "//vendor/github.com/golang/glog:go_default_library", @@ -49,6 +50,7 @@ go_test( name = "go_default_test", srcs = ["apply_test.go"], data = [ + "//api/openapi-spec:swagger-spec", "//test/fixtures", ], embed = [":go_default_library"], @@ -71,6 +73,7 @@ go_test( "//staging/src/k8s.io/client-go/rest:go_default_library", "//staging/src/k8s.io/client-go/rest/fake:go_default_library", "//staging/src/k8s.io/client-go/testing:go_default_library", + "//vendor/github.com/googleapis/gnostic/OpenAPIv2:go_default_library", "//vendor/github.com/spf13/cobra:go_default_library", ], ) diff --git a/pkg/kubectl/cmd/apply/apply.go b/pkg/kubectl/cmd/apply/apply.go index e1ee8366ac..8b13130f72 100644 --- a/pkg/kubectl/cmd/apply/apply.go +++ b/pkg/kubectl/cmd/apply/apply.go @@ -42,6 +42,7 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/cli-runtime/pkg/genericclioptions/printers" "k8s.io/cli-runtime/pkg/genericclioptions/resource" + "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" oapi "k8s.io/kube-openapi/pkg/util/proto" "k8s.io/kubernetes/pkg/kubectl" @@ -76,11 +77,12 @@ type ApplyOptions struct { PruneWhitelist []string ShouldIncludeUninitialized bool - Validator validation.Schema - Builder *resource.Builder - Mapper meta.RESTMapper - DynamicClient dynamic.Interface - OpenAPISchema openapi.Resources + Validator validation.Schema + Builder *resource.Builder + Mapper meta.RESTMapper + DynamicClient dynamic.Interface + DiscoveryClient discovery.DiscoveryInterface + OpenAPISchema openapi.Resources Namespace string EnforceNamespace bool @@ -211,6 +213,11 @@ func (o *ApplyOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { return err } + o.DiscoveryClient, err = f.ToDiscoveryClient() + if err != nil { + return err + } + dynamicClient, err := f.DynamicClient() if err != nil { return err @@ -403,19 +410,25 @@ 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, - 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, + 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, } patchBytes, patchedObject, err := patcher.Patch(info.Object, modified, info.Source, info.Namespace, info.Name, o.ErrOut) @@ -668,9 +681,10 @@ func (p *Patcher) delete(namespace, name string) error { } type Patcher struct { - Mapping *meta.RESTMapping - Helper *resource.Helper - DynamicClient dynamic.Interface + Mapping *meta.RESTMapping + Helper *resource.Helper + DynamicClient dynamic.Interface + DryRunVerifier *DryRunVerifier Overwrite bool BackOff clockwork.Clock @@ -684,7 +698,52 @@ type Patcher struct { OpenapiSchema openapi.Resources } +// DryRunVerifier verifies if a given group-version-kind supports DryRun +// against the current server. Sending dryRun requests to apiserver that +// don't support it will result in objects being unwillingly persisted. +// +// It reads the OpenAPI to see if the given GVK supports dryRun. If the +// GVK can not be found, we assume that CRDs will have the same level of +// support as "namespaces", and non-CRDs will not be supported. We +// delay the check for CRDs as much as possible though, since it +// requires an extra round-trip to the server. +type DryRunVerifier struct { + Finder cmdutil.CRDFinder + OpenAPIGetter discovery.OpenAPISchemaInterface +} + +// HasSupport verifies if the given gvk supports DryRun. An error is +// returned if it doesn't. +func (v *DryRunVerifier) HasSupport(gvk schema.GroupVersionKind) error { + oapi, err := v.OpenAPIGetter.OpenAPISchema() + if err != nil { + return fmt.Errorf("failed to download openapi: %v", err) + } + supports, err := openapi.SupportsDryRun(oapi, gvk) + if err != nil { + // We assume that we couldn't find the type, then check for namespace: + supports, _ = openapi.SupportsDryRun(oapi, schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Namespace"}) + // If namespace supports dryRun, then we will support dryRun for CRDs only. + if supports { + supports, err = v.Finder.HasCRD(gvk.GroupKind()) + if err != nil { + return fmt.Errorf("failed to check CRD: %v", err) + } + } + } + if !supports { + return fmt.Errorf("%v doesn't support dry-run", gvk) + } + return nil +} + 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/apply/apply_test.go b/pkg/kubectl/cmd/apply/apply_test.go index 7611640b8c..8c1631f684 100644 --- a/pkg/kubectl/cmd/apply/apply_test.go +++ b/pkg/kubectl/cmd/apply/apply_test.go @@ -29,6 +29,7 @@ import ( "strings" "testing" + "github.com/googleapis/gnostic/OpenAPIv2" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" @@ -52,7 +53,7 @@ import ( ) var ( - fakeSchema = sptest.Fake{Path: filepath.Join("..", "..", "..", "api", "openapi-spec", "swagger.json")} + fakeSchema = sptest.Fake{Path: filepath.Join("..", "..", "..", "..", "api", "openapi-spec", "swagger.json")} testingOpenAPISchemaFns = []func() (openapi.Resources, error){nil, AlwaysErrorOpenAPISchemaFn, openAPISchemaFn} AlwaysErrorOpenAPISchemaFn = func() (openapi.Resources, error) { return nil, errors.New("cannot get openapi spec") @@ -1317,3 +1318,75 @@ func TestForceApply(t *testing.T) { }) } } + +func TestDryRunVerifier(t *testing.T) { + dryRunVerifier := DryRunVerifier{ + Finder: cmdutil.NewCRDFinder(func() ([]schema.GroupKind, error) { + return []schema.GroupKind{ + { + Group: "crd.com", + Kind: "MyCRD", + }, + { + Group: "crd.com", + Kind: "MyNewCRD", + }, + }, nil + }), + OpenAPIGetter: &fakeSchema, + } + + err := dryRunVerifier.HasSupport(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "NodeProxyOptions"}) + if err == nil { + t.Fatalf("NodeProxyOptions doesn't support dry-run, yet no error found") + } + + err = dryRunVerifier.HasSupport(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}) + if err != nil { + t.Fatalf("Pod should support dry-run: %v", err) + } + + err = dryRunVerifier.HasSupport(schema.GroupVersionKind{Group: "crd.com", Version: "v1", Kind: "MyCRD"}) + if err != nil { + t.Fatalf("MyCRD should support dry-run: %v", err) + } + + err = dryRunVerifier.HasSupport(schema.GroupVersionKind{Group: "crd.com", Version: "v1", Kind: "Random"}) + if err == nil { + t.Fatalf("Random doesn't support dry-run, yet no error found") + } +} + +type EmptyOpenAPI struct{} + +func (EmptyOpenAPI) OpenAPISchema() (*openapi_v2.Document, error) { + return &openapi_v2.Document{}, nil +} + +func TestDryRunVerifierNoOpenAPI(t *testing.T) { + dryRunVerifier := DryRunVerifier{ + Finder: cmdutil.NewCRDFinder(func() ([]schema.GroupKind, error) { + return []schema.GroupKind{ + { + Group: "crd.com", + Kind: "MyCRD", + }, + { + Group: "crd.com", + Kind: "MyNewCRD", + }, + }, nil + }), + OpenAPIGetter: EmptyOpenAPI{}, + } + + err := dryRunVerifier.HasSupport(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}) + if err == nil { + t.Fatalf("Pod doesn't support dry-run, yet no error found") + } + + err = dryRunVerifier.HasSupport(schema.GroupVersionKind{Group: "crd.com", Version: "v1", Kind: "MyCRD"}) + if err == nil { + t.Fatalf("MyCRD doesn't support dry-run, yet no error found") + } +} diff --git a/pkg/kubectl/cmd/diff/diff.go b/pkg/kubectl/cmd/diff/diff.go index 5f1ffc0fa6..5a58d6a8a2 100644 --- a/pkg/kubectl/cmd/diff/diff.go +++ b/pkg/kubectl/cmd/diff/diff.go @@ -226,10 +226,11 @@ 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 + LocalObj runtime.Object + Info *resource.Info + Encoder runtime.Encoder + OpenAPI openapi.Resources + DryRunVerifier *apply.DryRunVerifier } var _ Object = &InfoObject{} @@ -261,12 +262,13 @@ 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{ - Mapping: obj.Info.Mapping, - Helper: resource.NewHelper(obj.Info.Client, obj.Info.Mapping), - Overwrite: true, - BackOff: clockwork.NewRealClock(), - ServerDryRun: true, - OpenapiSchema: obj.OpenAPI, + 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, } _, result, err := patcher.Patch(obj.Info.Object, modified, obj.Info.Source, obj.Info.Namespace, obj.Info.Name, nil) @@ -330,6 +332,21 @@ func RunDiff(f cmdutil.Factory, diff *DiffProgram, options *DiffOptions) error { return err } + discovery, err := f.ToDiscoveryClient() + if err != nil { + return err + } + + dynamic, err := f.DynamicClient() + if err != nil { + return err + } + + dryRunVerifier := &apply.DryRunVerifier{ + Finder: cmdutil.NewCRDFinder(cmdutil.CRDFromDynamic(dynamic)), + OpenAPIGetter: discovery, + } + differ, err := NewDiffer("LIVE", "MERGED") if err != nil { return err @@ -367,10 +384,11 @@ func RunDiff(f cmdutil.Factory, diff *DiffProgram, options *DiffOptions) error { } obj := InfoObject{ - LocalObj: local, - Info: info, - Encoder: scheme.DefaultJSONEncoder(), - OpenAPI: schema, + LocalObj: local, + Info: info, + Encoder: scheme.DefaultJSONEncoder(), + OpenAPI: schema, + DryRunVerifier: dryRunVerifier, } return differ.Diff(obj, printer)