From 56c19f1056ad6d4a4bb926fe90e37f56a31c4e2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Fri, 17 Sep 2021 15:27:21 +0300 Subject: [PATCH 1/8] Introduce new prune parameter into diff command This PR introduces new prune and it's dependent parameters to simulate `kubectl apply --prune` command. --- .../src/k8s.io/kubectl/pkg/cmd/diff/diff.go | 94 ++++++- .../src/k8s.io/kubectl/pkg/cmd/diff/prune.go | 236 ++++++++++++++++++ .../k8s.io/kubectl/pkg/cmd/diff/prune_test.go | 98 ++++++++ 3 files changed, 419 insertions(+), 9 deletions(-) create mode 100644 staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go create mode 100644 staging/src/k8s.io/kubectl/pkg/cmd/diff/prune_test.go diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go index 5c47744389b..9da6888f11b 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go @@ -25,6 +25,9 @@ import ( "regexp" "strings" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/cli-runtime/pkg/printers" + "github.com/jonboulle/clockwork" "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/api/errors" @@ -107,15 +110,25 @@ type DiffOptions struct { FieldManager string ForceConflicts bool - Selector string - OpenAPISchema openapi.Resources - DiscoveryClient discovery.DiscoveryInterface - DynamicClient dynamic.Interface - DryRunVerifier *resource.DryRunVerifier - CmdNamespace string - EnforceNamespace bool - Builder *resource.Builder - Diff *DiffProgram + Selector string + OpenAPISchema openapi.Resources + DiscoveryClient discovery.DiscoveryInterface + DynamicClient dynamic.Interface + DryRunVerifier *resource.DryRunVerifier + CmdNamespace string + EnforceNamespace bool + Builder *resource.Builder + Diff *DiffProgram + Mapper meta.RESTMapper + Prune bool + PruneResources []pruneResource + VisitedUids sets.String + VisitedNamespaces sets.String + ToPrinter func(string) (printers.ResourcePrinter, error) + PrintFlags *genericclioptions.PrintFlags + All bool + PruneWhitelist []string + genericclioptions.IOStreams } func validateArgs(cmd *cobra.Command, args []string) error { @@ -131,6 +144,10 @@ func NewDiffOptions(ioStreams genericclioptions.IOStreams) *DiffOptions { Exec: exec.New(), IOStreams: ioStreams, }, + VisitedUids: sets.NewString(), + VisitedNamespaces: sets.NewString(), + PrintFlags: genericclioptions.NewPrintFlags("created").WithTypeSetter(scheme.Scheme), + IOStreams: ioStreams, } } @@ -145,6 +162,7 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckDiffErr(options.Complete(f, cmd)) cmdutil.CheckDiffErr(validateArgs(cmd, args)) + cmdutil.CheckErr(validatePruneAll(options.Prune, options.All, options.Selector)) // `kubectl diff` propagates the error code from // diff or `KUBECTL_EXTERNAL_DIFF`. Also, we // don't want to print an error if diff returns @@ -170,6 +188,9 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C usage := "contains the configuration to diff" cmd.Flags().StringVarP(&options.Selector, "selector", "l", options.Selector, "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)") + cmd.Flags().StringArrayVar(&options.PruneWhitelist, "prune-whitelist", options.PruneWhitelist, "Overwrite the default whitelist with for --prune") + cmd.Flags().BoolVar(&options.Prune, "prune", options.Prune, "Automatically diff for possibly will be deleted resource objects, Should be used with either -l or --all.") + cmd.Flags().BoolVar(&options.All, "all", options.All, "Select all resources in the namespace of the specified resource types.") cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage) cmdutil.AddServerSideApplyFlags(cmd) cmdutil.AddFieldManagerFlagVar(cmd, &options.FieldManager, apply.FieldManagerClientSideApply) @@ -177,6 +198,16 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C return cmd } +func validatePruneAll(prune, all bool, selector string) error { + if all && len(selector) > 0 { + return fmt.Errorf("cannot set --all and --selector at the same time") + } + if prune && !all && selector == "" { + return fmt.Errorf("all resources selected for prune without explicitly passing --all. To prune all resources, pass the --all flag. If you did not mean to prune all resources, specify a label selector") + } + return nil +} + // DiffProgram finds and run the diff program. The value of // KUBECTL_EXTERNAL_DIFF environment variable will be used a diff // program. By default, `diff(1)` will be used. @@ -618,6 +649,12 @@ func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { return fmt.Errorf("--force-conflicts only works with --server-side") } + o.ToPrinter = func(operation string) (printers.ResourcePrinter, error) { + o.PrintFlags.NamePrintFlags.Operation = operation + cmdutil.PrintFlagsWithDryRunStrategy(o.PrintFlags, cmdutil.DryRunServer) + return o.PrintFlags.ToPrinter() + } + if !o.ServerSideApply { o.OpenAPISchema, err = f.OpenAPISchema() if err != nil { @@ -642,6 +679,18 @@ func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { return err } + if o.Prune { + o.Mapper, err = f.ToRESTMapper() + if err != nil { + return err + } + + o.PruneResources, err = parsePruneResources(o.Mapper, o.PruneWhitelist) + if err != nil { + return err + } + } + o.Builder = f.NewBuilder() return nil } @@ -708,6 +757,8 @@ func (o *DiffOptions) Run() error { } err = differ.Diff(obj, printer) + o.MarkNamespaceVisited(info) + o.MarkObjectVisited(info) if !isConflict(err) { break } @@ -717,9 +768,34 @@ func (o *DiffOptions) Run() error { return err }) + + if o.Prune { + prune := newPruner(o) + prune.pruneAll(o) + } + if err != nil { return err } return differ.Run(o.Diff) } + +// MarkObjectVisited keeps track of UIDs of the applied +// objects. Used for pruning. +func (o *DiffOptions) MarkObjectVisited(info *resource.Info) error { + metadata, err := meta.Accessor(info.Object) + if err != nil { + return err + } + o.VisitedUids.Insert(string(metadata.GetUID())) + return nil +} + +// MarkNamespaceVisited keeps track of which namespaces the applied +// objects belong to. Used for pruning. +func (o *DiffOptions) MarkNamespaceVisited(info *resource.Info) { + if info.Namespaced() { + o.VisitedNamespaces.Insert(info.Namespace) + } +} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go new file mode 100644 index 00000000000..99086429003 --- /dev/null +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go @@ -0,0 +1,236 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package diff + +import ( + "context" + "fmt" + "io" + "strings" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/cli-runtime/pkg/printers" + "k8s.io/client-go/dynamic" +) + +type pruner struct { + mapper meta.RESTMapper + dynamicClient dynamic.Interface + + visitedUids sets.String + visitedNamespaces sets.String + labelSelector string + fieldSelector string + + cascadingStrategy metav1.DeletionPropagation + gracePeriod int + + toPrinter func(string) (printers.ResourcePrinter, error) + + out io.Writer +} + +func newPruner(o *DiffOptions) pruner { + return pruner{ + mapper: o.Mapper, + dynamicClient: o.DynamicClient, + + labelSelector: o.Selector, + visitedUids: o.VisitedUids, + visitedNamespaces: o.VisitedNamespaces, + + toPrinter: o.ToPrinter, + + cascadingStrategy: metav1.DeletePropagationBackground, + gracePeriod: -1, + + out: o.ErrOut, + } +} + +func (p *pruner) pruneAll(o *DiffOptions) error { + + namespacedRESTMappings, nonNamespacedRESTMappings, err := getRESTMappings(o.Mapper, &(o.PruneResources)) + if err != nil { + return fmt.Errorf("error retrieving RESTMappings to prune: %v", err) + } + + for n := range p.visitedNamespaces { + for _, m := range namespacedRESTMappings { + if err := p.prune(n, m); err != nil { + return fmt.Errorf("error pruning namespaced object %v: %v", m.GroupVersionKind, err) + } + } + } + for _, m := range nonNamespacedRESTMappings { + if err := p.prune(metav1.NamespaceNone, m); err != nil { + return fmt.Errorf("error pruning nonNamespaced object %v: %v", m.GroupVersionKind, err) + } + } + + return nil +} + +func (p *pruner) prune(namespace string, mapping *meta.RESTMapping) error { + objList, err := p.dynamicClient.Resource(mapping.Resource). + Namespace(namespace). + List(context.TODO(), metav1.ListOptions{ + LabelSelector: p.labelSelector, + FieldSelector: p.fieldSelector, + }) + if err != nil { + return err + } + + objs, err := meta.ExtractList(objList) + if err != nil { + return err + } + + for _, obj := range objs { + metadata, err := meta.Accessor(obj) + if err != nil { + return err + } + annots := metadata.GetAnnotations() + if _, ok := annots[corev1.LastAppliedConfigAnnotation]; !ok { + // don't prune resources not created with apply + continue + } + uid := metadata.GetUID() + if p.visitedUids.Has(string(uid)) { + continue + } + name := metadata.GetName() + if err := p.delete(namespace, name, mapping); err != nil { + return err + } + + printer, err := p.toPrinter("pruned") + if err != nil { + return err + } + + printer.PrintObj(obj, p.out) + } + return nil +} + +func (p *pruner) delete(namespace, name string, mapping *meta.RESTMapping) error { + return runDelete(namespace, name, mapping, p.dynamicClient, p.cascadingStrategy, p.gracePeriod, true) +} + +func runDelete(namespace, name string, mapping *meta.RESTMapping, c dynamic.Interface, cascadingStrategy metav1.DeletionPropagation, gracePeriod int, serverDryRun bool) error { + options := asDeleteOptions(cascadingStrategy, gracePeriod) + if serverDryRun { + options.DryRun = []string{metav1.DryRunAll} + } + return c.Resource(mapping.Resource).Namespace(namespace).Delete(context.TODO(), name, options) +} + +func asDeleteOptions(cascadingStrategy metav1.DeletionPropagation, gracePeriod int) metav1.DeleteOptions { + options := metav1.DeleteOptions{} + if gracePeriod >= 0 { + options = *metav1.NewDeleteOptions(int64(gracePeriod)) + } + options.PropagationPolicy = &cascadingStrategy + return options +} + +type pruneResource struct { + group string + version string + kind string + namespaced bool +} + +func (pr pruneResource) String() string { + return fmt.Sprintf("%v/%v, Kind=%v, Namespaced=%v", pr.group, pr.version, pr.kind, pr.namespaced) +} + +func getRESTMappings(mapper meta.RESTMapper, pruneResources *[]pruneResource) (namespaced, nonNamespaced []*meta.RESTMapping, err error) { + if len(*pruneResources) == 0 { + // default allowlist + *pruneResources = []pruneResource{ + {"", "v1", "ConfigMap", true}, + {"", "v1", "Endpoints", true}, + {"", "v1", "Namespace", false}, + {"", "v1", "PersistentVolumeClaim", true}, + {"", "v1", "PersistentVolume", false}, + {"", "v1", "Pod", true}, + {"", "v1", "ReplicationController", true}, + {"", "v1", "Secret", true}, + {"", "v1", "Service", true}, + {"batch", "v1", "Job", true}, + {"batch", "v1", "CronJob", true}, + {"networking.k8s.io", "v1", "Ingress", true}, + {"apps", "v1", "DaemonSet", true}, + {"apps", "v1", "Deployment", true}, + {"apps", "v1", "ReplicaSet", true}, + {"apps", "v1", "StatefulSet", true}, + } + } + + for _, resource := range *pruneResources { + addedMapping, err := mapper.RESTMapping(schema.GroupKind{Group: resource.group, Kind: resource.kind}, resource.version) + if err != nil { + return nil, nil, fmt.Errorf("invalid resource %v: %v", resource, err) + } + if resource.namespaced { + namespaced = append(namespaced, addedMapping) + } else { + nonNamespaced = append(nonNamespaced, addedMapping) + } + } + + return namespaced, nonNamespaced, nil +} + +func parsePruneResources(mapper meta.RESTMapper, gvks []string) ([]pruneResource, error) { + pruneResources := []pruneResource{} + for _, groupVersionKind := range gvks { + gvk := strings.Split(groupVersionKind, "/") + if len(gvk) != 3 { + return nil, fmt.Errorf("invalid GroupVersionKind format: %v, please follow ", groupVersionKind) + } + + if gvk[0] == "core" { + gvk[0] = "" + } + mapping, err := mapper.RESTMapping(schema.GroupKind{Group: gvk[0], Kind: gvk[2]}, gvk[1]) + if err != nil { + return pruneResources, err + } + var namespaced bool + namespaceScope := mapping.Scope.Name() + switch namespaceScope { + case meta.RESTScopeNameNamespace: + namespaced = true + case meta.RESTScopeNameRoot: + namespaced = false + default: + return pruneResources, fmt.Errorf("Unknown namespace scope: %q", namespaceScope) + } + + pruneResources = append(pruneResources, pruneResource{gvk[0], gvk[1], gvk[2], namespaced}) + } + return pruneResources, nil +} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune_test.go new file mode 100644 index 00000000000..b6a59c95c17 --- /dev/null +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune_test.go @@ -0,0 +1,98 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package diff + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +type testRESTMapper struct { + meta.RESTMapper + scope meta.RESTScope +} + +func (m *testRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { + return &meta.RESTMapping{ + Resource: schema.GroupVersionResource{ + Group: gk.Group, + Version: "", + Resource: "", + }, + GroupVersionKind: schema.GroupVersionKind{ + Group: gk.Group, + Version: "", + Kind: gk.Kind, + }, + Scope: m.scope, + }, nil +} + +func TestParsePruneResources(t *testing.T) { + tests := []struct { + mapper *testRESTMapper + gvks []string + expected []pruneResource + err bool + }{ + { + mapper: &testRESTMapper{ + scope: meta.RESTScopeNamespace, + }, + gvks: nil, + expected: []pruneResource{}, + err: false, + }, + { + mapper: &testRESTMapper{ + scope: meta.RESTScopeNamespace, + }, + gvks: []string{"group/kind/version/test"}, + expected: []pruneResource{}, + err: true, + }, + { + mapper: &testRESTMapper{ + scope: meta.RESTScopeNamespace, + }, + gvks: []string{"group/kind/version"}, + expected: []pruneResource{{group: "group", version: "kind", kind: "version", namespaced: true}}, + err: false, + }, + { + mapper: &testRESTMapper{ + scope: meta.RESTScopeRoot, + }, + gvks: []string{"group/kind/version"}, + expected: []pruneResource{{group: "group", version: "kind", kind: "version", namespaced: false}}, + err: false, + }, + } + + for _, tc := range tests { + actual, err := parsePruneResources(tc.mapper, tc.gvks) + if tc.err { + assert.NotEmptyf(t, err, "parsePruneResources error expected but not fired") + } else { + assert.Equal(t, actual, tc.expected, "parsePruneResources failed expected %v actual %v", tc.expected, actual) + } + } +} From 17de414905fb3463ec17ec63f40180fa09965d98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Fri, 24 Sep 2021 09:40:26 +0300 Subject: [PATCH 2/8] Add new unit tests for diff/prune --- .../src/k8s.io/kubectl/pkg/cmd/apply/apply.go | 8 +- .../src/k8s.io/kubectl/pkg/cmd/apply/prune.go | 86 +------- .../src/k8s.io/kubectl/pkg/cmd/diff/diff.go | 107 ++++------ .../src/k8s.io/kubectl/pkg/cmd/diff/prune.go | 186 +++--------------- .../k8s.io/kubectl/pkg/util/prune/prune.go | 89 +++++++++ .../{cmd/diff => util/prune}/prune_test.go | 41 +++- 6 files changed, 205 insertions(+), 312 deletions(-) create mode 100644 staging/src/k8s.io/kubectl/pkg/util/prune/prune.go rename staging/src/k8s.io/kubectl/pkg/{cmd/diff => util/prune}/prune_test.go (64%) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go index fe39617852a..56c689b68f1 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go @@ -21,6 +21,8 @@ import ( "io" "net/http" + "k8s.io/kubectl/pkg/util/prune" + "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -60,7 +62,7 @@ type ApplyFlags struct { FieldManager string Selector string Prune bool - PruneResources []pruneResource + PruneResources []prune.Resource All bool Overwrite bool OpenAPIPatch bool @@ -85,7 +87,7 @@ type ApplyOptions struct { DryRunStrategy cmdutil.DryRunStrategy DryRunVerifier *resource.DryRunVerifier Prune bool - PruneResources []pruneResource + PruneResources []prune.Resource cmdBaseName string All bool Overwrite bool @@ -278,7 +280,7 @@ func (flags *ApplyFlags) ToOptions(cmd *cobra.Command, baseName string, args []s } if flags.Prune { - flags.PruneResources, err = parsePruneResources(mapper, flags.PruneWhitelist) + flags.PruneResources, err = prune.ParseResources(mapper, flags.PruneWhitelist) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go index 1b65a3eb9cb..609f45e5a76 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go @@ -20,12 +20,12 @@ import ( "context" "fmt" "io" - "strings" + + "k8s.io/kubectl/pkg/util/prune" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/cli-runtime/pkg/printers" "k8s.io/client-go/dynamic" @@ -71,7 +71,7 @@ func newPruner(o *ApplyOptions) pruner { func (p *pruner) pruneAll(o *ApplyOptions) error { - namespacedRESTMappings, nonNamespacedRESTMappings, err := getRESTMappings(o.Mapper, &(o.PruneResources)) + namespacedRESTMappings, nonNamespacedRESTMappings, err := prune.GetRESTMappings(o.Mapper, &(o.PruneResources)) if err != nil { return fmt.Errorf("error retrieving RESTMappings to prune: %v", err) } @@ -158,83 +158,3 @@ func asDeleteOptions(cascadingStrategy metav1.DeletionPropagation, gracePeriod i options.PropagationPolicy = &cascadingStrategy return options } - -type pruneResource struct { - group string - version string - kind string - namespaced bool -} - -func (pr pruneResource) String() string { - return fmt.Sprintf("%v/%v, Kind=%v, Namespaced=%v", pr.group, pr.version, pr.kind, pr.namespaced) -} - -func getRESTMappings(mapper meta.RESTMapper, pruneResources *[]pruneResource) (namespaced, nonNamespaced []*meta.RESTMapping, err error) { - if len(*pruneResources) == 0 { - // default allowlist - *pruneResources = []pruneResource{ - {"", "v1", "ConfigMap", true}, - {"", "v1", "Endpoints", true}, - {"", "v1", "Namespace", false}, - {"", "v1", "PersistentVolumeClaim", true}, - {"", "v1", "PersistentVolume", false}, - {"", "v1", "Pod", true}, - {"", "v1", "ReplicationController", true}, - {"", "v1", "Secret", true}, - {"", "v1", "Service", true}, - {"batch", "v1", "Job", true}, - {"batch", "v1", "CronJob", true}, - {"networking.k8s.io", "v1", "Ingress", true}, - {"apps", "v1", "DaemonSet", true}, - {"apps", "v1", "Deployment", true}, - {"apps", "v1", "ReplicaSet", true}, - {"apps", "v1", "StatefulSet", true}, - } - } - - for _, resource := range *pruneResources { - addedMapping, err := mapper.RESTMapping(schema.GroupKind{Group: resource.group, Kind: resource.kind}, resource.version) - if err != nil { - return nil, nil, fmt.Errorf("invalid resource %v: %v", resource, err) - } - if resource.namespaced { - namespaced = append(namespaced, addedMapping) - } else { - nonNamespaced = append(nonNamespaced, addedMapping) - } - } - - return namespaced, nonNamespaced, nil -} - -func parsePruneResources(mapper meta.RESTMapper, gvks []string) ([]pruneResource, error) { - pruneResources := []pruneResource{} - for _, groupVersionKind := range gvks { - gvk := strings.Split(groupVersionKind, "/") - if len(gvk) != 3 { - return nil, fmt.Errorf("invalid GroupVersionKind format: %v, please follow ", groupVersionKind) - } - - if gvk[0] == "core" { - gvk[0] = "" - } - mapping, err := mapper.RESTMapping(schema.GroupKind{Group: gvk[0], Kind: gvk[2]}, gvk[1]) - if err != nil { - return pruneResources, err - } - var namespaced bool - namespaceScope := mapping.Scope.Name() - switch namespaceScope { - case meta.RESTScopeNameNamespace: - namespaced = true - case meta.RESTScopeNameRoot: - namespaced = false - default: - return pruneResources, fmt.Errorf("Unknown namespace scope: %q", namespaceScope) - } - - pruneResources = append(pruneResources, pruneResource{gvk[0], gvk[1], gvk[2], namespaced}) - } - return pruneResources, nil -} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go index 9da6888f11b..90e25042e5f 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go @@ -25,8 +25,7 @@ import ( "regexp" "strings" - "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/cli-runtime/pkg/printers" + "k8s.io/kubectl/pkg/util/prune" "github.com/jonboulle/clockwork" "github.com/spf13/cobra" @@ -36,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/cli-runtime/pkg/resource" "k8s.io/client-go/discovery" @@ -110,25 +110,18 @@ type DiffOptions struct { FieldManager string ForceConflicts bool - Selector string - OpenAPISchema openapi.Resources - DiscoveryClient discovery.DiscoveryInterface - DynamicClient dynamic.Interface - DryRunVerifier *resource.DryRunVerifier - CmdNamespace string - EnforceNamespace bool - Builder *resource.Builder - Diff *DiffProgram - Mapper meta.RESTMapper - Prune bool - PruneResources []pruneResource - VisitedUids sets.String - VisitedNamespaces sets.String - ToPrinter func(string) (printers.ResourcePrinter, error) - PrintFlags *genericclioptions.PrintFlags - All bool - PruneWhitelist []string - genericclioptions.IOStreams + Selector string + OpenAPISchema openapi.Resources + DiscoveryClient discovery.DiscoveryInterface + DynamicClient dynamic.Interface + DryRunVerifier *resource.DryRunVerifier + CmdNamespace string + EnforceNamespace bool + Builder *resource.Builder + Diff *DiffProgram + Prune bool + PruneWhitelist []string + pruner *pruner } func validateArgs(cmd *cobra.Command, args []string) error { @@ -144,10 +137,10 @@ func NewDiffOptions(ioStreams genericclioptions.IOStreams) *DiffOptions { Exec: exec.New(), IOStreams: ioStreams, }, - VisitedUids: sets.NewString(), - VisitedNamespaces: sets.NewString(), - PrintFlags: genericclioptions.NewPrintFlags("created").WithTypeSetter(scheme.Scheme), - IOStreams: ioStreams, + pruner: &pruner{ + visitedUids: sets.NewString(), + visitedNamespaces: sets.NewString(), + }, } } @@ -162,7 +155,6 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckDiffErr(options.Complete(f, cmd)) cmdutil.CheckDiffErr(validateArgs(cmd, args)) - cmdutil.CheckErr(validatePruneAll(options.Prune, options.All, options.Selector)) // `kubectl diff` propagates the error code from // diff or `KUBECTL_EXTERNAL_DIFF`. Also, we // don't want to print an error if diff returns @@ -189,8 +181,7 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C usage := "contains the configuration to diff" cmd.Flags().StringVarP(&options.Selector, "selector", "l", options.Selector, "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)") cmd.Flags().StringArrayVar(&options.PruneWhitelist, "prune-whitelist", options.PruneWhitelist, "Overwrite the default whitelist with for --prune") - cmd.Flags().BoolVar(&options.Prune, "prune", options.Prune, "Automatically diff for possibly will be deleted resource objects, Should be used with either -l or --all.") - cmd.Flags().BoolVar(&options.All, "all", options.All, "Select all resources in the namespace of the specified resource types.") + cmd.Flags().BoolVar(&options.Prune, "prune", options.Prune, "Automatically diff for possibly will be deleted resource objects, Should be used with either -l or --prune-all.") cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage) cmdutil.AddServerSideApplyFlags(cmd) cmdutil.AddFieldManagerFlagVar(cmd, &options.FieldManager, apply.FieldManagerClientSideApply) @@ -198,16 +189,6 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C return cmd } -func validatePruneAll(prune, all bool, selector string) error { - if all && len(selector) > 0 { - return fmt.Errorf("cannot set --all and --selector at the same time") - } - if prune && !all && selector == "" { - return fmt.Errorf("all resources selected for prune without explicitly passing --all. To prune all resources, pass the --all flag. If you did not mean to prune all resources, specify a label selector") - } - return nil -} - // DiffProgram finds and run the diff program. The value of // KUBECTL_EXTERNAL_DIFF environment variable will be used a diff // program. By default, `diff(1)` will be used. @@ -649,12 +630,6 @@ func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { return fmt.Errorf("--force-conflicts only works with --server-side") } - o.ToPrinter = func(operation string) (printers.ResourcePrinter, error) { - o.PrintFlags.NamePrintFlags.Operation = operation - cmdutil.PrintFlagsWithDryRunStrategy(o.PrintFlags, cmdutil.DryRunServer) - return o.PrintFlags.ToPrinter() - } - if !o.ServerSideApply { o.OpenAPISchema, err = f.OpenAPISchema() if err != nil { @@ -680,12 +655,13 @@ func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { } if o.Prune { - o.Mapper, err = f.ToRESTMapper() + o.pruner.dynamicClient = o.DynamicClient + o.pruner.mapper, err = f.ToRESTMapper() if err != nil { return err } - o.PruneResources, err = parsePruneResources(o.Mapper, o.PruneWhitelist) + o.pruner.resources, err = prune.ParseResources(o.pruner.mapper, o.PruneWhitelist) if err != nil { return err } @@ -756,9 +732,9 @@ func (o *DiffOptions) Run() error { IOStreams: o.Diff.IOStreams, } + o.markVisited(info) + err = differ.Diff(obj, printer) - o.MarkNamespaceVisited(info) - o.MarkObjectVisited(info) if !isConflict(err) { break } @@ -770,8 +746,18 @@ func (o *DiffOptions) Run() error { }) if o.Prune { - prune := newPruner(o) - prune.pruneAll(o) + prunedObjs, err := o.pruner.pruneAll() + if err != nil { + klog.Warningf("pruning failed and could not be evaluated err: %v", err) + } + + // Print pruned objects into old file and thus, diff + // command will show them as pruned. + for _, p := range prunedObjs { + if err := differ.From.Print(o.pruner.GetObjectName(p), p, printer); err != nil { + return err + } + } } if err != nil { @@ -781,21 +767,14 @@ func (o *DiffOptions) Run() error { return differ.Run(o.Diff) } -// MarkObjectVisited keeps track of UIDs of the applied -// objects. Used for pruning. -func (o *DiffOptions) MarkObjectVisited(info *resource.Info) error { +func (o *DiffOptions) markVisited(info *resource.Info) { + if info.Namespaced() { + o.pruner.visitedNamespaces.Insert(info.Namespace) + } + metadata, err := meta.Accessor(info.Object) if err != nil { - return err - } - o.VisitedUids.Insert(string(metadata.GetUID())) - return nil -} - -// MarkNamespaceVisited keeps track of which namespaces the applied -// objects belong to. Used for pruning. -func (o *DiffOptions) MarkNamespaceVisited(info *resource.Info) { - if info.Namespaced() { - o.VisitedNamespaces.Insert(info.Namespace) + return } + o.pruner.visitedUids.Insert(string(metadata.GetUID())) } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go index 99086429003..be39b8493b3 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go @@ -19,15 +19,17 @@ package diff import ( "context" "fmt" - "io" - "strings" + + "k8s.io/apimachinery/pkg/util/uuid" + + "k8s.io/apimachinery/pkg/runtime" + + "k8s.io/kubectl/pkg/util/prune" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/cli-runtime/pkg/printers" "k8s.io/client-go/dynamic" ) @@ -38,199 +40,73 @@ type pruner struct { visitedUids sets.String visitedNamespaces sets.String labelSelector string - fieldSelector string - - cascadingStrategy metav1.DeletionPropagation - gracePeriod int - - toPrinter func(string) (printers.ResourcePrinter, error) - - out io.Writer + resources []prune.Resource } -func newPruner(o *DiffOptions) pruner { - return pruner{ - mapper: o.Mapper, - dynamicClient: o.DynamicClient, - - labelSelector: o.Selector, - visitedUids: o.VisitedUids, - visitedNamespaces: o.VisitedNamespaces, - - toPrinter: o.ToPrinter, - - cascadingStrategy: metav1.DeletePropagationBackground, - gracePeriod: -1, - - out: o.ErrOut, - } -} - -func (p *pruner) pruneAll(o *DiffOptions) error { - - namespacedRESTMappings, nonNamespacedRESTMappings, err := getRESTMappings(o.Mapper, &(o.PruneResources)) +func (p *pruner) pruneAll() ([]runtime.Object, error) { + var allPruned []runtime.Object + namespacedRESTMappings, nonNamespacedRESTMappings, err := prune.GetRESTMappings(p.mapper, &(p.resources)) if err != nil { - return fmt.Errorf("error retrieving RESTMappings to prune: %v", err) + return allPruned, fmt.Errorf("error retrieving RESTMappings to prune: %v", err) } for n := range p.visitedNamespaces { for _, m := range namespacedRESTMappings { - if err := p.prune(n, m); err != nil { - return fmt.Errorf("error pruning namespaced object %v: %v", m.GroupVersionKind, err) + if pobjs, err := p.prune(n, m); err != nil { + return pobjs, fmt.Errorf("error pruning namespaced object %v: %v", m.GroupVersionKind, err) + } else { + allPruned = append(allPruned, pobjs...) } } } for _, m := range nonNamespacedRESTMappings { - if err := p.prune(metav1.NamespaceNone, m); err != nil { - return fmt.Errorf("error pruning nonNamespaced object %v: %v", m.GroupVersionKind, err) + if pobjs, err := p.prune(metav1.NamespaceNone, m); err != nil { + return allPruned, fmt.Errorf("error pruning nonNamespaced object %v: %v", m.GroupVersionKind, err) + } else { + allPruned = append(allPruned, pobjs...) } } - return nil + return allPruned, nil } -func (p *pruner) prune(namespace string, mapping *meta.RESTMapping) error { +func (p *pruner) prune(namespace string, mapping *meta.RESTMapping) ([]runtime.Object, error) { objList, err := p.dynamicClient.Resource(mapping.Resource). Namespace(namespace). List(context.TODO(), metav1.ListOptions{ LabelSelector: p.labelSelector, - FieldSelector: p.fieldSelector, }) if err != nil { - return err + return nil, err } objs, err := meta.ExtractList(objList) if err != nil { - return err + return nil, err } + var pobjs []runtime.Object for _, obj := range objs { metadata, err := meta.Accessor(obj) if err != nil { - return err + return pobjs, err } annots := metadata.GetAnnotations() if _, ok := annots[corev1.LastAppliedConfigAnnotation]; !ok { - // don't prune resources not created with apply continue } uid := metadata.GetUID() if p.visitedUids.Has(string(uid)) { continue } - name := metadata.GetName() - if err := p.delete(namespace, name, mapping); err != nil { - return err - } - printer, err := p.toPrinter("pruned") - if err != nil { - return err - } - - printer.PrintObj(obj, p.out) + pobjs = append(pobjs, obj) } - return nil + return pobjs, nil } -func (p *pruner) delete(namespace, name string, mapping *meta.RESTMapping) error { - return runDelete(namespace, name, mapping, p.dynamicClient, p.cascadingStrategy, p.gracePeriod, true) -} - -func runDelete(namespace, name string, mapping *meta.RESTMapping, c dynamic.Interface, cascadingStrategy metav1.DeletionPropagation, gracePeriod int, serverDryRun bool) error { - options := asDeleteOptions(cascadingStrategy, gracePeriod) - if serverDryRun { - options.DryRun = []string{metav1.DryRunAll} - } - return c.Resource(mapping.Resource).Namespace(namespace).Delete(context.TODO(), name, options) -} - -func asDeleteOptions(cascadingStrategy metav1.DeletionPropagation, gracePeriod int) metav1.DeleteOptions { - options := metav1.DeleteOptions{} - if gracePeriod >= 0 { - options = *metav1.NewDeleteOptions(int64(gracePeriod)) - } - options.PropagationPolicy = &cascadingStrategy - return options -} - -type pruneResource struct { - group string - version string - kind string - namespaced bool -} - -func (pr pruneResource) String() string { - return fmt.Sprintf("%v/%v, Kind=%v, Namespaced=%v", pr.group, pr.version, pr.kind, pr.namespaced) -} - -func getRESTMappings(mapper meta.RESTMapper, pruneResources *[]pruneResource) (namespaced, nonNamespaced []*meta.RESTMapping, err error) { - if len(*pruneResources) == 0 { - // default allowlist - *pruneResources = []pruneResource{ - {"", "v1", "ConfigMap", true}, - {"", "v1", "Endpoints", true}, - {"", "v1", "Namespace", false}, - {"", "v1", "PersistentVolumeClaim", true}, - {"", "v1", "PersistentVolume", false}, - {"", "v1", "Pod", true}, - {"", "v1", "ReplicationController", true}, - {"", "v1", "Secret", true}, - {"", "v1", "Service", true}, - {"batch", "v1", "Job", true}, - {"batch", "v1", "CronJob", true}, - {"networking.k8s.io", "v1", "Ingress", true}, - {"apps", "v1", "DaemonSet", true}, - {"apps", "v1", "Deployment", true}, - {"apps", "v1", "ReplicaSet", true}, - {"apps", "v1", "StatefulSet", true}, - } - } - - for _, resource := range *pruneResources { - addedMapping, err := mapper.RESTMapping(schema.GroupKind{Group: resource.group, Kind: resource.kind}, resource.version) - if err != nil { - return nil, nil, fmt.Errorf("invalid resource %v: %v", resource, err) - } - if resource.namespaced { - namespaced = append(namespaced, addedMapping) - } else { - nonNamespaced = append(nonNamespaced, addedMapping) - } - } - - return namespaced, nonNamespaced, nil -} - -func parsePruneResources(mapper meta.RESTMapper, gvks []string) ([]pruneResource, error) { - pruneResources := []pruneResource{} - for _, groupVersionKind := range gvks { - gvk := strings.Split(groupVersionKind, "/") - if len(gvk) != 3 { - return nil, fmt.Errorf("invalid GroupVersionKind format: %v, please follow ", groupVersionKind) - } - - if gvk[0] == "core" { - gvk[0] = "" - } - mapping, err := mapper.RESTMapping(schema.GroupKind{Group: gvk[0], Kind: gvk[2]}, gvk[1]) - if err != nil { - return pruneResources, err - } - var namespaced bool - namespaceScope := mapping.Scope.Name() - switch namespaceScope { - case meta.RESTScopeNameNamespace: - namespaced = true - case meta.RESTScopeNameRoot: - namespaced = false - default: - return pruneResources, fmt.Errorf("Unknown namespace scope: %q", namespaceScope) - } - - pruneResources = append(pruneResources, pruneResource{gvk[0], gvk[1], gvk[2], namespaced}) - } - return pruneResources, nil +func (p *pruner) GetObjectName(obj runtime.Object) string { + // Not compare anything, it is safe to assign random + // object name. + return string(uuid.NewUUID()) } diff --git a/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go b/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go new file mode 100644 index 00000000000..a05abdc85e3 --- /dev/null +++ b/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go @@ -0,0 +1,89 @@ +package prune + +import ( + "fmt" + "strings" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +type Resource struct { + group string + version string + kind string + namespaced bool +} + +func (pr Resource) String() string { + return fmt.Sprintf("%v/%v, Kind=%v, Namespaced=%v", pr.group, pr.version, pr.kind, pr.namespaced) +} + +func GetRESTMappings(mapper meta.RESTMapper, pruneResources *[]Resource) (namespaced, nonNamespaced []*meta.RESTMapping, err error) { + if len(*pruneResources) == 0 { + // default allowlist + *pruneResources = []Resource{ + {"", "v1", "ConfigMap", true}, + {"", "v1", "Endpoints", true}, + {"", "v1", "Namespace", false}, + {"", "v1", "PersistentVolumeClaim", true}, + {"", "v1", "PersistentVolume", false}, + {"", "v1", "Pod", true}, + {"", "v1", "ReplicationController", true}, + {"", "v1", "Secret", true}, + {"", "v1", "Service", true}, + {"batch", "v1", "Job", true}, + {"batch", "v1", "CronJob", true}, + {"networking.k8s.io", "v1", "Ingress", true}, + {"apps", "v1", "DaemonSet", true}, + {"apps", "v1", "Deployment", true}, + {"apps", "v1", "ReplicaSet", true}, + {"apps", "v1", "StatefulSet", true}, + } + } + + for _, resource := range *pruneResources { + addedMapping, err := mapper.RESTMapping(schema.GroupKind{Group: resource.group, Kind: resource.kind}, resource.version) + if err != nil { + return nil, nil, fmt.Errorf("invalid resource %v: %v", resource, err) + } + if resource.namespaced { + namespaced = append(namespaced, addedMapping) + } else { + nonNamespaced = append(nonNamespaced, addedMapping) + } + } + + return namespaced, nonNamespaced, nil +} + +func ParseResources(mapper meta.RESTMapper, gvks []string) ([]Resource, error) { + pruneResources := []Resource{} + for _, groupVersionKind := range gvks { + gvk := strings.Split(groupVersionKind, "/") + if len(gvk) != 3 { + return nil, fmt.Errorf("invalid GroupVersionKind format: %v, please follow ", groupVersionKind) + } + + if gvk[0] == "core" { + gvk[0] = "" + } + mapping, err := mapper.RESTMapping(schema.GroupKind{Group: gvk[0], Kind: gvk[2]}, gvk[1]) + if err != nil { + return pruneResources, err + } + var namespaced bool + namespaceScope := mapping.Scope.Name() + switch namespaceScope { + case meta.RESTScopeNameNamespace: + namespaced = true + case meta.RESTScopeNameRoot: + namespaced = false + default: + return pruneResources, fmt.Errorf("Unknown namespace scope: %q", namespaceScope) + } + + pruneResources = append(pruneResources, Resource{gvk[0], gvk[1], gvk[2], namespaced}) + } + return pruneResources, nil +} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune_test.go b/staging/src/k8s.io/kubectl/pkg/util/prune/prune_test.go similarity index 64% rename from staging/src/k8s.io/kubectl/pkg/cmd/diff/prune_test.go rename to staging/src/k8s.io/kubectl/pkg/util/prune/prune_test.go index b6a59c95c17..54f5d4c76aa 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune_test.go +++ b/staging/src/k8s.io/kubectl/pkg/util/prune/prune_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package diff +package prune import ( "testing" @@ -46,11 +46,38 @@ func (m *testRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (* }, nil } +func TestGetRESTMappings(t *testing.T) { + tests := []struct { + mapper *testRESTMapper + pr *[]Resource + expectedns int + expectednns int + expectederr error + }{ + { + mapper: &testRESTMapper{}, + pr: &[]Resource{}, + expectedns: 14, + expectednns: 2, + expectederr: nil, + }, + } + + for _, tc := range tests { + actualns, actualnns, actualerr := GetRESTMappings(tc.mapper, tc.pr) + if tc.expectederr != nil { + assert.NotEmptyf(t, actualerr, "getRESTMappings error expected but not fired") + } + assert.Equal(t, len(actualns), tc.expectedns, "getRESTMappings failed expected number namespaced %d actual %d", tc.expectedns, len(actualns)) + assert.Equal(t, len(actualnns), tc.expectednns, "getRESTMappings failed expected number nonnamespaced %d actual %d", tc.expectednns, len(actualnns)) + } +} + func TestParsePruneResources(t *testing.T) { tests := []struct { mapper *testRESTMapper gvks []string - expected []pruneResource + expected []Resource err bool }{ { @@ -58,7 +85,7 @@ func TestParsePruneResources(t *testing.T) { scope: meta.RESTScopeNamespace, }, gvks: nil, - expected: []pruneResource{}, + expected: []Resource{}, err: false, }, { @@ -66,7 +93,7 @@ func TestParsePruneResources(t *testing.T) { scope: meta.RESTScopeNamespace, }, gvks: []string{"group/kind/version/test"}, - expected: []pruneResource{}, + expected: []Resource{}, err: true, }, { @@ -74,7 +101,7 @@ func TestParsePruneResources(t *testing.T) { scope: meta.RESTScopeNamespace, }, gvks: []string{"group/kind/version"}, - expected: []pruneResource{{group: "group", version: "kind", kind: "version", namespaced: true}}, + expected: []Resource{{group: "group", version: "kind", kind: "version", namespaced: true}}, err: false, }, { @@ -82,13 +109,13 @@ func TestParsePruneResources(t *testing.T) { scope: meta.RESTScopeRoot, }, gvks: []string{"group/kind/version"}, - expected: []pruneResource{{group: "group", version: "kind", kind: "version", namespaced: false}}, + expected: []Resource{{group: "group", version: "kind", kind: "version", namespaced: false}}, err: false, }, } for _, tc := range tests { - actual, err := parsePruneResources(tc.mapper, tc.gvks) + actual, err := ParseResources(tc.mapper, tc.gvks) if tc.err { assert.NotEmptyf(t, err, "parsePruneResources error expected but not fired") } else { From a5b803670c178395c63a59c0331b69fac6c438b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 29 Sep 2021 10:56:08 +0300 Subject: [PATCH 3/8] Update vendor for util/prune --- vendor/modules.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/vendor/modules.txt b/vendor/modules.txt index cba08df75c3..267d4995c79 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2156,6 +2156,7 @@ k8s.io/kubectl/pkg/util/openapi k8s.io/kubectl/pkg/util/openapi/testing k8s.io/kubectl/pkg/util/openapi/validation k8s.io/kubectl/pkg/util/podutils +k8s.io/kubectl/pkg/util/prune k8s.io/kubectl/pkg/util/qos k8s.io/kubectl/pkg/util/rbac k8s.io/kubectl/pkg/util/resource From f4861725c9a8494481093d30a5f6e07f7ab8f2d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 29 Sep 2021 12:20:38 +0300 Subject: [PATCH 4/8] Add boilerplate header into util/prune --- .../src/k8s.io/kubectl/pkg/util/prune/prune.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go b/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go index a05abdc85e3..f44b2c639f2 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go +++ b/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go @@ -1,3 +1,19 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package prune import ( From 05100c0e49328cd149e96aa78fefc91c8c97ffee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Thu, 30 Sep 2021 11:26:40 +0300 Subject: [PATCH 5/8] Add pruner constructor, tweak DiffOptions --- staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go | 13 ++++--------- staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go | 7 +++++++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go index 90e25042e5f..f0a4897668f 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go @@ -35,7 +35,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/cli-runtime/pkg/resource" "k8s.io/client-go/discovery" @@ -120,7 +119,6 @@ type DiffOptions struct { Builder *resource.Builder Diff *DiffProgram Prune bool - PruneWhitelist []string pruner *pruner } @@ -137,10 +135,7 @@ func NewDiffOptions(ioStreams genericclioptions.IOStreams) *DiffOptions { Exec: exec.New(), IOStreams: ioStreams, }, - pruner: &pruner{ - visitedUids: sets.NewString(), - visitedNamespaces: sets.NewString(), - }, + pruner: newPruner(), } } @@ -180,8 +175,8 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C usage := "contains the configuration to diff" cmd.Flags().StringVarP(&options.Selector, "selector", "l", options.Selector, "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)") - cmd.Flags().StringArrayVar(&options.PruneWhitelist, "prune-whitelist", options.PruneWhitelist, "Overwrite the default whitelist with for --prune") - cmd.Flags().BoolVar(&options.Prune, "prune", options.Prune, "Automatically diff for possibly will be deleted resource objects, Should be used with either -l or --prune-all.") + cmd.Flags().StringArray("prunewhitelist", []string{}, "Overwrite the default whitelist with for --prune") + cmd.Flags().BoolVar(&options.Prune, "prune", options.Prune, "Include resources that would be deleted by pruning. Can be used with -l and default shows all would be pruned resources") cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage) cmdutil.AddServerSideApplyFlags(cmd) cmdutil.AddFieldManagerFlagVar(cmd, &options.FieldManager, apply.FieldManagerClientSideApply) @@ -661,7 +656,7 @@ func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { return err } - o.pruner.resources, err = prune.ParseResources(o.pruner.mapper, o.PruneWhitelist) + o.pruner.resources, err = prune.ParseResources(o.pruner.mapper, cmdutil.GetFlagStringArray(cmd, "prunewhitelist")) if err != nil { return err } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go index be39b8493b3..fff6db113a0 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go @@ -43,6 +43,13 @@ type pruner struct { resources []prune.Resource } +func newPruner() *pruner { + return &pruner{ + visitedUids: sets.NewString(), + visitedNamespaces: sets.NewString(), + } +} + func (p *pruner) pruneAll() ([]runtime.Object, error) { var allPruned []runtime.Object namespacedRESTMappings, nonNamespacedRESTMappings, err := prune.GetRESTMappings(p.mapper, &(p.resources)) From a19bd5e47436c1f0a2aea20c4a992da18f8ba480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 6 Oct 2021 17:01:19 +0300 Subject: [PATCH 6/8] Get object name correctly for pruned resources --- .../src/k8s.io/kubectl/pkg/cmd/diff/diff.go | 37 +++++++--------- .../src/k8s.io/kubectl/pkg/cmd/diff/prune.go | 44 ++++++++++++++++--- 2 files changed, 53 insertions(+), 28 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go index f0a4897668f..0315afceebf 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go @@ -118,7 +118,6 @@ type DiffOptions struct { EnforceNamespace bool Builder *resource.Builder Diff *DiffProgram - Prune bool pruner *pruner } @@ -135,7 +134,6 @@ func NewDiffOptions(ioStreams genericclioptions.IOStreams) *DiffOptions { Exec: exec.New(), IOStreams: ioStreams, }, - pruner: newPruner(), } } @@ -176,7 +174,7 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C usage := "contains the configuration to diff" cmd.Flags().StringVarP(&options.Selector, "selector", "l", options.Selector, "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)") cmd.Flags().StringArray("prunewhitelist", []string{}, "Overwrite the default whitelist with for --prune") - cmd.Flags().BoolVar(&options.Prune, "prune", options.Prune, "Include resources that would be deleted by pruning. Can be used with -l and default shows all would be pruned resources") + cmd.Flags().Bool("prune", false, "Include resources that would be deleted by pruning. Can be used with -l and default shows all resources would be pruned") cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage) cmdutil.AddServerSideApplyFlags(cmd) cmdutil.AddFieldManagerFlagVar(cmd, &options.FieldManager, apply.FieldManagerClientSideApply) @@ -649,17 +647,17 @@ func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { return err } - if o.Prune { - o.pruner.dynamicClient = o.DynamicClient - o.pruner.mapper, err = f.ToRESTMapper() + if cmdutil.GetFlagBool(cmd, "prune") { + mapper, err := f.ToRESTMapper() if err != nil { return err } - o.pruner.resources, err = prune.ParseResources(o.pruner.mapper, cmdutil.GetFlagStringArray(cmd, "prunewhitelist")) + resources, err := prune.ParseResources(mapper, cmdutil.GetFlagStringArray(cmd, "prunewhitelist")) if err != nil { return err } + o.pruner = newPruner(o.DynamicClient, mapper, resources) } o.Builder = f.NewBuilder() @@ -727,7 +725,9 @@ func (o *DiffOptions) Run() error { IOStreams: o.Diff.IOStreams, } - o.markVisited(info) + if o.pruner != nil { + o.pruner.MarkVisited(info) + } err = differ.Diff(obj, printer) if !isConflict(err) { @@ -740,7 +740,7 @@ func (o *DiffOptions) Run() error { return err }) - if o.Prune { + if o.pruner != nil { prunedObjs, err := o.pruner.pruneAll() if err != nil { klog.Warningf("pruning failed and could not be evaluated err: %v", err) @@ -749,7 +749,12 @@ func (o *DiffOptions) Run() error { // Print pruned objects into old file and thus, diff // command will show them as pruned. for _, p := range prunedObjs { - if err := differ.From.Print(o.pruner.GetObjectName(p), p, printer); err != nil { + name, err := o.pruner.GetObjectName(p) + if err != nil { + klog.Warningf("pruning failed and object name could not be retrieved: %v", err) + continue + } + if err := differ.From.Print(name, p, printer); err != nil { return err } } @@ -761,15 +766,3 @@ func (o *DiffOptions) Run() error { return differ.Run(o.Diff) } - -func (o *DiffOptions) markVisited(info *resource.Info) { - if info.Namespaced() { - o.pruner.visitedNamespaces.Insert(info.Namespace) - } - - metadata, err := meta.Accessor(info.Object) - if err != nil { - return - } - o.pruner.visitedUids.Insert(string(metadata.GetUID())) -} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go index fff6db113a0..d7d5d288db3 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go @@ -20,7 +20,7 @@ import ( "context" "fmt" - "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/cli-runtime/pkg/resource" "k8s.io/apimachinery/pkg/runtime" @@ -43,10 +43,13 @@ type pruner struct { resources []prune.Resource } -func newPruner() *pruner { +func newPruner(dc dynamic.Interface, m meta.RESTMapper, r []prune.Resource) *pruner { return &pruner{ visitedUids: sets.NewString(), visitedNamespaces: sets.NewString(), + dynamicClient: dc, + mapper: m, + resources: r, } } @@ -112,8 +115,37 @@ func (p *pruner) prune(namespace string, mapping *meta.RESTMapping) ([]runtime.O return pobjs, nil } -func (p *pruner) GetObjectName(obj runtime.Object) string { - // Not compare anything, it is safe to assign random - // object name. - return string(uuid.NewUUID()) +func (p *pruner) GetObjectName(obj runtime.Object) (string, error) { + gvk := obj.GetObjectKind().GroupVersionKind() + metadata, err := meta.Accessor(obj) + if err != nil { + return "", err + } + name := metadata.GetName() + ns := metadata.GetNamespace() + + group := "" + if gvk.Group != "" { + group = fmt.Sprintf("%v.", gvk.Group) + } + return group + fmt.Sprintf( + "%v.%v.%v.%v", + gvk.Version, + gvk.Kind, + ns, + name, + ), nil +} + +// MarkVisited marks visited namespaces and uids +func (p *pruner) MarkVisited(info *resource.Info) { + if info.Namespaced() { + p.visitedNamespaces.Insert(info.Namespace) + } + + metadata, err := meta.Accessor(info.Object) + if err != nil { + return + } + p.visitedUids.Insert(string(metadata.GetUID())) } From 34f0148414092940747f4a221d368c8a17034249 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Fri, 15 Oct 2021 12:54:47 +0300 Subject: [PATCH 7/8] Add prune test into test/cmd/diff.sh --- test/cmd/diff.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/cmd/diff.sh b/test/cmd/diff.sh index c1e75d683fe..7c5debba766 100755 --- a/test/cmd/diff.sh +++ b/test/cmd/diff.sh @@ -88,8 +88,29 @@ run_kubectl_diff_tests() { output_message=$(kubectl diff --server-side -f hack/testdata/pod-changed.yaml || test $? -eq 1) kube::test::if_has_string "${output_message}" 'k8s.gcr.io/pause:3.4' + ## kubectl diff --prune + kubectl create ns nsb + kubectl apply --namespace nsb -l prune-group=true -f hack/testdata/prune/a.yaml + kube::test::get_object_assert 'pods a -n nsb' "{{${id_field:?}}}" 'a' + # Make sure that kubectl diff does not return pod 'a' without prune flag + output_message=$(kubectl diff -l prune-group=true -f hack/testdata/prune/b.yaml || test $? -eq 1) + kube::test::if_has_not_string "${output_message}" "name: a" + # Make sure that for kubectl diff --prune: + # 1. the exit code for diff is 1 because it found a difference + # 2. the difference contains the pruned pod + output_message=$(kubectl diff --prune -l prune-group=true -f hack/testdata/prune/b.yaml || test $? -eq 1) + # pod 'a' should be in output, it is pruned + kube::test::if_has_string "${output_message}" 'name: a' + # apply b with namespace + kubectl apply --prune --namespace nsb -l prune-group=true -f hack/testdata/prune/b.yaml + # check right pod exists and wrong pod doesn't exist + kube::test::wait_object_assert 'pods -n nsb' "{{range.items}}{{${id_field:?}}}:{{end}}" 'b:' + # Make sure that diff --prune returns nothing (0 exit code) for 'b'. + kubectl diff --prune -l prune-group=true -f hack/testdata/prune/b.yaml + # Cleanup kubectl delete -f hack/testdata/pod.yaml + kubectl delete -f hack/testdata/prune/b.yaml set +o nounset set +o errexit From 6c449dd272c95f2aeb3bb77e67d312d8df21bd62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Mon, 8 Nov 2021 11:22:10 +0300 Subject: [PATCH 8/8] Refactor diff/prune This PR does some refactors for diff/prune; - GetRESTMappings takes value array instead reference - Move getObjectName into diff instead prune - License, etc. changes --- .../src/k8s.io/kubectl/pkg/cmd/apply/apply.go | 4 +-- .../src/k8s.io/kubectl/pkg/cmd/apply/prune.go | 5 ++- .../src/k8s.io/kubectl/pkg/cmd/diff/diff.go | 31 +++++++++++++--- .../src/k8s.io/kubectl/pkg/cmd/diff/prune.go | 35 +++---------------- .../k8s.io/kubectl/pkg/util/prune/prune.go | 8 ++--- .../kubectl/pkg/util/prune/prune_test.go | 4 +-- 6 files changed, 41 insertions(+), 46 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go index 56c689b68f1..ecdcf87c7b8 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go @@ -21,9 +21,8 @@ import ( "io" "net/http" - "k8s.io/kubectl/pkg/util/prune" - "github.com/spf13/cobra" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -44,6 +43,7 @@ import ( "k8s.io/kubectl/pkg/util" "k8s.io/kubectl/pkg/util/i18n" "k8s.io/kubectl/pkg/util/openapi" + "k8s.io/kubectl/pkg/util/prune" "k8s.io/kubectl/pkg/util/templates" "k8s.io/kubectl/pkg/validation" ) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go index 609f45e5a76..04ce54e5685 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/prune.go @@ -21,8 +21,6 @@ import ( "fmt" "io" - "k8s.io/kubectl/pkg/util/prune" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -30,6 +28,7 @@ import ( "k8s.io/cli-runtime/pkg/printers" "k8s.io/client-go/dynamic" cmdutil "k8s.io/kubectl/pkg/cmd/util" + "k8s.io/kubectl/pkg/util/prune" ) type pruner struct { @@ -71,7 +70,7 @@ func newPruner(o *ApplyOptions) pruner { func (p *pruner) pruneAll(o *ApplyOptions) error { - namespacedRESTMappings, nonNamespacedRESTMappings, err := prune.GetRESTMappings(o.Mapper, &(o.PruneResources)) + namespacedRESTMappings, nonNamespacedRESTMappings, err := prune.GetRESTMappings(o.Mapper, o.PruneResources) if err != nil { return fmt.Errorf("error retrieving RESTMappings to prune: %v", err) } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go index 0315afceebf..f2a1d439e52 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go @@ -25,8 +25,6 @@ import ( "regexp" "strings" - "k8s.io/kubectl/pkg/util/prune" - "github.com/jonboulle/clockwork" "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/api/errors" @@ -46,6 +44,7 @@ import ( "k8s.io/kubectl/pkg/util" "k8s.io/kubectl/pkg/util/i18n" "k8s.io/kubectl/pkg/util/openapi" + "k8s.io/kubectl/pkg/util/prune" "k8s.io/kubectl/pkg/util/templates" "k8s.io/utils/exec" "sigs.k8s.io/yaml" @@ -173,7 +172,7 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C usage := "contains the configuration to diff" cmd.Flags().StringVarP(&options.Selector, "selector", "l", options.Selector, "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)") - cmd.Flags().StringArray("prunewhitelist", []string{}, "Overwrite the default whitelist with for --prune") + cmd.Flags().StringArray("prune-allowlist", []string{}, "Overwrite the default whitelist with for --prune") cmd.Flags().Bool("prune", false, "Include resources that would be deleted by pruning. Can be used with -l and default shows all resources would be pruned") cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage) cmdutil.AddServerSideApplyFlags(cmd) @@ -653,7 +652,7 @@ func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { return err } - resources, err := prune.ParseResources(mapper, cmdutil.GetFlagStringArray(cmd, "prunewhitelist")) + resources, err := prune.ParseResources(mapper, cmdutil.GetFlagStringArray(cmd, "prune-allowlist")) if err != nil { return err } @@ -749,7 +748,7 @@ func (o *DiffOptions) Run() error { // Print pruned objects into old file and thus, diff // command will show them as pruned. for _, p := range prunedObjs { - name, err := o.pruner.GetObjectName(p) + name, err := getObjectName(p) if err != nil { klog.Warningf("pruning failed and object name could not be retrieved: %v", err) continue @@ -766,3 +765,25 @@ func (o *DiffOptions) Run() error { return differ.Run(o.Diff) } + +func getObjectName(obj runtime.Object) (string, error) { + gvk := obj.GetObjectKind().GroupVersionKind() + metadata, err := meta.Accessor(obj) + if err != nil { + return "", err + } + name := metadata.GetName() + ns := metadata.GetNamespace() + + group := "" + if gvk.Group != "" { + group = fmt.Sprintf("%v.", gvk.Group) + } + return group + fmt.Sprintf( + "%v.%v.%v.%v", + gvk.Version, + gvk.Kind, + ns, + name, + ), nil +} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go index d7d5d288db3..64a6d4fee36 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/prune.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Kubernetes Authors. +Copyright 2021 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -20,17 +20,14 @@ import ( "context" "fmt" - "k8s.io/cli-runtime/pkg/resource" - - "k8s.io/apimachinery/pkg/runtime" - - "k8s.io/kubectl/pkg/util/prune" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/cli-runtime/pkg/resource" "k8s.io/client-go/dynamic" + "k8s.io/kubectl/pkg/util/prune" ) type pruner struct { @@ -55,7 +52,7 @@ func newPruner(dc dynamic.Interface, m meta.RESTMapper, r []prune.Resource) *pru func (p *pruner) pruneAll() ([]runtime.Object, error) { var allPruned []runtime.Object - namespacedRESTMappings, nonNamespacedRESTMappings, err := prune.GetRESTMappings(p.mapper, &(p.resources)) + namespacedRESTMappings, nonNamespacedRESTMappings, err := prune.GetRESTMappings(p.mapper, p.resources) if err != nil { return allPruned, fmt.Errorf("error retrieving RESTMappings to prune: %v", err) } @@ -115,28 +112,6 @@ func (p *pruner) prune(namespace string, mapping *meta.RESTMapping) ([]runtime.O return pobjs, nil } -func (p *pruner) GetObjectName(obj runtime.Object) (string, error) { - gvk := obj.GetObjectKind().GroupVersionKind() - metadata, err := meta.Accessor(obj) - if err != nil { - return "", err - } - name := metadata.GetName() - ns := metadata.GetNamespace() - - group := "" - if gvk.Group != "" { - group = fmt.Sprintf("%v.", gvk.Group) - } - return group + fmt.Sprintf( - "%v.%v.%v.%v", - gvk.Version, - gvk.Kind, - ns, - name, - ), nil -} - // MarkVisited marks visited namespaces and uids func (p *pruner) MarkVisited(info *resource.Info) { if info.Namespaced() { diff --git a/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go b/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go index f44b2c639f2..0d49153fe4c 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go +++ b/staging/src/k8s.io/kubectl/pkg/util/prune/prune.go @@ -35,10 +35,10 @@ func (pr Resource) String() string { return fmt.Sprintf("%v/%v, Kind=%v, Namespaced=%v", pr.group, pr.version, pr.kind, pr.namespaced) } -func GetRESTMappings(mapper meta.RESTMapper, pruneResources *[]Resource) (namespaced, nonNamespaced []*meta.RESTMapping, err error) { - if len(*pruneResources) == 0 { +func GetRESTMappings(mapper meta.RESTMapper, pruneResources []Resource) (namespaced, nonNamespaced []*meta.RESTMapping, err error) { + if len(pruneResources) == 0 { // default allowlist - *pruneResources = []Resource{ + pruneResources = []Resource{ {"", "v1", "ConfigMap", true}, {"", "v1", "Endpoints", true}, {"", "v1", "Namespace", false}, @@ -58,7 +58,7 @@ func GetRESTMappings(mapper meta.RESTMapper, pruneResources *[]Resource) (namesp } } - for _, resource := range *pruneResources { + for _, resource := range pruneResources { addedMapping, err := mapper.RESTMapping(schema.GroupKind{Group: resource.group, Kind: resource.kind}, resource.version) if err != nil { return nil, nil, fmt.Errorf("invalid resource %v: %v", resource, err) diff --git a/staging/src/k8s.io/kubectl/pkg/util/prune/prune_test.go b/staging/src/k8s.io/kubectl/pkg/util/prune/prune_test.go index 54f5d4c76aa..cbc389b4af0 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/prune/prune_test.go +++ b/staging/src/k8s.io/kubectl/pkg/util/prune/prune_test.go @@ -49,14 +49,14 @@ func (m *testRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (* func TestGetRESTMappings(t *testing.T) { tests := []struct { mapper *testRESTMapper - pr *[]Resource + pr []Resource expectedns int expectednns int expectederr error }{ { mapper: &testRESTMapper{}, - pr: &[]Resource{}, + pr: []Resource{}, expectedns: 14, expectednns: 2, expectederr: nil,