kubectl apply: Deprecate --prune-whitelist in favor of --prune-allowlist

Changes in kubectl apply --prune to support k8s Inclusive Naming Initiative:
* Deprecated the --prune-whitelist flag.
* Deprecated the PruneWhitelist field on ApplyFlags struct.
* Removed PruneWhitelist field (not used anywhere) from ApplyOptions struct.
* Added --prune-allowlist flag.
* Added PruneAllowlist field on ApplyFlags struct.
* Added unit tests for prune with allowlist

This commit also fixes a bug where the command would fail if you specified
the sameGVK multiple times for --allow-whitelist. Now it only attempts to
prune the unique set of allowed GVKs.

Kubernetes-commit: f7ebf4d8852d4500f24100ca9a4ca665efc1fada
This commit is contained in:
Brian Pursley 2022-10-30 20:50:19 -04:00 committed by Kubernetes Publisher
parent 9cebc60d68
commit 41407b14e8
4 changed files with 318 additions and 6 deletions

View File

@ -46,6 +46,7 @@ import (
"k8s.io/kubectl/pkg/util/i18n"
"k8s.io/kubectl/pkg/util/openapi"
"k8s.io/kubectl/pkg/util/prune"
"k8s.io/kubectl/pkg/util/slice"
"k8s.io/kubectl/pkg/util/templates"
"k8s.io/kubectl/pkg/validation"
)
@ -68,7 +69,10 @@ type ApplyFlags struct {
All bool
Overwrite bool
OpenAPIPatch bool
PruneWhitelist []string
// DEPRECATED: Use PruneAllowlist instead
PruneWhitelist []string // TODO: Remove this in kubectl 1.28 or later
PruneAllowlist []string
genericclioptions.IOStreams
}
@ -95,7 +99,6 @@ type ApplyOptions struct {
All bool
Overwrite bool
OpenAPIPatch bool
PruneWhitelist []string
ValidationDirective string
Validator validation.Schema
@ -161,7 +164,7 @@ var (
kubectl apply --prune -f manifest.yaml -l app=nginx
# Apply the configuration in manifest.yaml and delete all the other config maps that are not in the file
kubectl apply --prune -f manifest.yaml --all --prune-whitelist=core/v1/ConfigMap`))
kubectl apply --prune -f manifest.yaml --all --prune-allowlist=core/v1/ConfigMap`))
warningNoLastAppliedConfigAnnotation = "Warning: resource %[1]s is missing the %[2]s annotation which is required by %[3]s apply. %[3]s apply should only be used on resources created declaratively by either %[3]s create --save-config or %[3]s apply. The missing annotation will be patched automatically.\n"
warningChangesOnDeletingResource = "Warning: Detected changes to resource %[1]s which is currently being deleted.\n"
@ -229,7 +232,9 @@ func (flags *ApplyFlags) AddFlags(cmd *cobra.Command) {
cmd.Flags().BoolVar(&flags.Overwrite, "overwrite", flags.Overwrite, "Automatically resolve conflicts between the modified and live configuration by using values from the modified configuration")
cmd.Flags().BoolVar(&flags.Prune, "prune", flags.Prune, "Automatically delete resource objects, that do not appear in the configs and are created by either apply or create --save-config. Should be used with either -l or --all.")
cmd.Flags().BoolVar(&flags.All, "all", flags.All, "Select all resources in the namespace of the specified resource types.")
cmd.Flags().StringArrayVar(&flags.PruneWhitelist, "prune-whitelist", flags.PruneWhitelist, "Overwrite the default whitelist with <group/version/kind> for --prune")
cmd.Flags().StringArrayVar(&flags.PruneAllowlist, "prune-allowlist", flags.PruneAllowlist, "Overwrite the default allowlist with <group/version/kind> for --prune")
cmd.Flags().StringArrayVar(&flags.PruneWhitelist, "prune-whitelist", flags.PruneWhitelist, "Overwrite the default whitelist with <group/version/kind> for --prune") // TODO: Remove this in kubectl 1.28 or later
cmd.Flags().MarkDeprecated("prune-whitelist", "Use --prune-allowlist instead.")
cmd.Flags().BoolVar(&flags.OpenAPIPatch, "openapi-patch", flags.OpenAPIPatch, "If true, use openapi to calculate diff when the openapi presents and the resource can be found in the openapi spec. Otherwise, fall back to use baked-in types.")
}
@ -300,7 +305,8 @@ func (flags *ApplyFlags) ToOptions(cmd *cobra.Command, baseName string, args []s
}
if flags.Prune {
flags.PruneResources, err = prune.ParseResources(mapper, flags.PruneWhitelist)
pruneAllowlist := slice.ToSet(flags.PruneAllowlist, flags.PruneWhitelist)
flags.PruneResources, err = prune.ParseResources(mapper, pruneAllowlist)
if err != nil {
return nil, err
}
@ -326,7 +332,6 @@ func (flags *ApplyFlags) ToOptions(cmd *cobra.Command, baseName string, args []s
All: flags.All,
Overwrite: flags.Overwrite,
OpenAPIPatch: flags.OpenAPIPatch,
PruneWhitelist: flags.PruneWhitelist,
Recorder: recorder,
Namespace: namespace,

View File

@ -49,12 +49,14 @@ import (
dynamicfakeclient "k8s.io/client-go/dynamic/fake"
restclient "k8s.io/client-go/rest"
"k8s.io/client-go/rest/fake"
testing2 "k8s.io/client-go/testing"
"k8s.io/client-go/util/csaupgrade"
cmdtesting "k8s.io/kubectl/pkg/cmd/testing"
cmdutil "k8s.io/kubectl/pkg/cmd/util"
"k8s.io/kubectl/pkg/scheme"
"k8s.io/kubectl/pkg/util/openapi"
utilpointer "k8s.io/utils/pointer"
"k8s.io/utils/strings/slices"
)
var (
@ -714,6 +716,250 @@ func TestApplyPruneObjects(t *testing.T) {
}
}
func TestApplyPruneObjectsWithAllowlist(t *testing.T) {
cmdtesting.InitTestErrorHandler(t)
// Read ReplicationController from the file we will use to apply. This one will not be pruned because it exists in the file.
rc := readUnstructuredFromFile(t, filenameRC)
err := setLastAppliedConfigAnnotation(rc)
if err != nil {
t.Fatal(err)
}
// Create another ReplicationController that can be pruned
rc2 := &unstructured.Unstructured{
Object: map[string]interface{}{
"kind": "ReplicationController",
"apiVersion": "v1",
"metadata": map[string]interface{}{
"name": "test-rc2",
"namespace": "test",
"uid": "uid-rc2",
},
},
}
err = setLastAppliedConfigAnnotation(rc2)
if err != nil {
t.Fatal(err)
}
// Create a ConfigMap that can be pruned
cm := &unstructured.Unstructured{
Object: map[string]interface{}{
"kind": "ConfigMap",
"apiVersion": "v1",
"metadata": map[string]interface{}{
"name": "test-cm",
"namespace": "test",
"uid": "uid-cm",
},
},
}
err = setLastAppliedConfigAnnotation(cm)
if err != nil {
t.Fatal(err)
}
// Create a ConfigMap without a UID. Resources without a UID will not be pruned.
cmNoUID := &unstructured.Unstructured{
Object: map[string]interface{}{
"kind": "ConfigMap",
"apiVersion": "v1",
"metadata": map[string]interface{}{
"name": "test-cm-nouid",
"namespace": "test",
},
},
}
err = setLastAppliedConfigAnnotation(cmNoUID)
if err != nil {
t.Fatal(err)
}
// Create a ConfigMap without a last applied annotation. Resources without a last applied annotation will not be pruned.
cmNoLastApplied := &unstructured.Unstructured{
Object: map[string]interface{}{
"kind": "ConfigMap",
"apiVersion": "v1",
"metadata": map[string]interface{}{
"name": "test-cm-nolastapplied",
"namespace": "test",
"uid": "uid-cm-nolastapplied",
},
},
}
if err != nil {
t.Fatal(err)
}
testCases := map[string]struct {
currentResources []runtime.Object
pruneAllowlist []string
expectedPrunedResources []string
expectedOutputs []string
}{
"prune without allowlist should delete resources that are not in the specified file": {
currentResources: []runtime.Object{rc, rc2, cm},
expectedPrunedResources: []string{"test/test-cm", "test/test-rc2"},
expectedOutputs: []string{
"replicationcontroller/test-rc unchanged",
"configmap/test-cm pruned",
"replicationcontroller/test-rc2 pruned",
},
},
"prune with allowlist should delete only matching resources": {
currentResources: []runtime.Object{rc, rc2, cm},
pruneAllowlist: []string{"core/v1/ConfigMap"},
expectedPrunedResources: []string{"test/test-cm"},
expectedOutputs: []string{
"replicationcontroller/test-rc unchanged",
"configmap/test-cm pruned",
},
},
"prune with allowlist specifying the same resource type multiple times should not fail": {
currentResources: []runtime.Object{rc, rc2, cm},
pruneAllowlist: []string{"core/v1/ConfigMap", "core/v1/ConfigMap"},
expectedPrunedResources: []string{"test/test-cm"},
expectedOutputs: []string{
"replicationcontroller/test-rc unchanged",
"configmap/test-cm pruned",
},
},
"prune with allowlist should not delete resources that exist in the specified file": {
currentResources: []runtime.Object{rc, rc2, cm},
pruneAllowlist: []string{"core/v1/ReplicationController"},
expectedPrunedResources: []string{"test/test-rc2"},
expectedOutputs: []string{
"replicationcontroller/test-rc unchanged",
"replicationcontroller/test-rc2 pruned",
},
},
"prune with allowlist specifying multiple resource types should delete matching resources": {
currentResources: []runtime.Object{rc, rc2, cm},
pruneAllowlist: []string{"core/v1/ConfigMap", "core/v1/ReplicationController"},
expectedPrunedResources: []string{"test/test-cm", "test/test-rc2"},
expectedOutputs: []string{
"replicationcontroller/test-rc unchanged",
"configmap/test-cm pruned",
"replicationcontroller/test-rc2 pruned",
},
},
"prune should not delete resources that are missing a UID": {
currentResources: []runtime.Object{rc, cm, cmNoUID},
expectedPrunedResources: []string{"test/test-cm"},
expectedOutputs: []string{
"replicationcontroller/test-rc unchanged",
"configmap/test-cm pruned",
},
},
"prune should not delete resources that are missing the last applied config annotation": {
currentResources: []runtime.Object{rc, cm, cmNoLastApplied},
expectedPrunedResources: []string{"test/test-cm"},
expectedOutputs: []string{
"replicationcontroller/test-rc unchanged",
"configmap/test-cm pruned",
},
},
}
for testCaseName, tc := range testCases {
for _, testingOpenAPISchema := range testingOpenAPISchemas {
t.Run(testCaseName, 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 == "/namespaces/test/replicationcontrollers/test-rc" && m == "GET":
encoded := runtime.EncodeOrDie(unstructured.UnstructuredJSONScheme, rc)
bodyRC := io.NopCloser(strings.NewReader(encoded))
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: bodyRC}, nil
case p == "/namespaces/test/replicationcontrollers/test-rc" && m == "PATCH":
encoded := runtime.EncodeOrDie(unstructured.UnstructuredJSONScheme, rc)
bodyRC := io.NopCloser(strings.NewReader(encoded))
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 = testingOpenAPISchema.OpenAPISchemaFn
tf.FakeOpenAPIGetter = testingOpenAPISchema.OpenAPIGetter
tf.ClientConfigVal = cmdtesting.DefaultClientConfig()
for _, resource := range tc.currentResources {
if err := tf.FakeDynamicClient.Tracker().Add(resource); err != nil {
t.Fatal(err)
}
}
ioStreams, _, buf, errBuf := genericclioptions.NewTestIOStreams()
cmd := NewCmdApply("kubectl", tf, ioStreams)
cmd.Flags().Set("filename", filenameRC)
cmd.Flags().Set("prune", "true")
cmd.Flags().Set("namespace", "test")
cmd.Flags().Set("all", "true")
for _, allow := range tc.pruneAllowlist {
cmd.Flags().Set("prune-allowlist", allow)
}
cmd.Run(cmd, []string{})
if errBuf.String() != "" {
t.Fatalf("unexpected error output: %s", errBuf.String())
}
actualOutput := buf.String()
for _, expectedOutput := range tc.expectedOutputs {
if !strings.Contains(actualOutput, expectedOutput) {
t.Fatalf("expected output to contain %q, but it did not. Actual Output:\n%s", expectedOutput, actualOutput)
}
}
var prunedResources []string
for _, action := range tf.FakeDynamicClient.Actions() {
if action.GetVerb() == "delete" {
deleteAction := action.(testing2.DeleteAction)
prunedResources = append(prunedResources, deleteAction.GetNamespace()+"/"+deleteAction.GetName())
}
}
// Make sure nothing unexpected was pruned
for _, resource := range prunedResources {
if !slices.Contains(tc.expectedPrunedResources, resource) {
t.Fatalf("expected %s not to be pruned, but it was", resource)
}
}
// Make sure everything that was expected to be pruned was pruned
for _, resource := range tc.expectedPrunedResources {
if !slices.Contains(prunedResources, resource) {
t.Fatalf("expected %s to be pruned, but it was not", resource)
}
}
})
}
}
}
func setLastAppliedConfigAnnotation(obj runtime.Object) error {
accessor, err := meta.Accessor(obj)
if err != nil {
return err
}
annotations := accessor.GetAnnotations()
if annotations == nil {
annotations = make(map[string]string)
accessor.SetAnnotations(annotations)
}
annotations[corev1.LastAppliedConfigAnnotation] = runtime.EncodeOrDie(unstructured.NewJSONFallbackEncoder(codec), obj)
accessor.SetAnnotations(annotations)
return nil
}
// Tests that apply of object in need of CSA migration results in a call
// to patch it.
func TestApplyCSAMigration(t *testing.T) {

View File

@ -36,3 +36,22 @@ func ContainsString(slice []string, s string, modifier func(s string) string) bo
}
return false
}
// ToSet returns a single slice containing the unique values from one or more slices. The order of the items in the
// result is not guaranteed.
func ToSet[T comparable](slices ...[]T) []T {
if len(slices) == 0 {
return nil
}
m := map[T]struct{}{}
for _, slice := range slices {
for _, value := range slice {
m[value] = struct{}{}
}
}
result := []T{}
for k := range m {
result = append(result, k)
}
return result
}

View File

@ -18,6 +18,7 @@ package slice
import (
"reflect"
"sort"
"testing"
)
@ -29,3 +30,44 @@ func TestSortInts64(t *testing.T) {
t.Errorf("func Ints64 didnt sort correctly, %v !- %v", src, expected)
}
}
func TestToSet(t *testing.T) {
testCases := map[string]struct {
input [][]string
expected []string
}{
"nil should be returned if no slices are passed to the function": {
input: [][]string{},
expected: nil,
},
"empty slice should be returned if an empty slice is passed to the function": {
input: [][]string{{}},
expected: []string{},
},
"a single slice with no duplicates should have the same values": {
input: [][]string{{"a", "b", "c"}},
expected: []string{"a", "b", "c"},
},
"duplicates should be removed from a single slice": {
input: [][]string{{"a", "b", "a", "c", "b"}},
expected: []string{"a", "b", "c"},
},
"multiple slices with no duplicates should be combined": {
input: [][]string{{"a", "b", "c"}, {"d", "e", "f"}},
expected: []string{"a", "b", "c", "d", "e", "f"},
},
"duplicates should be removed from multiple slices": {
input: [][]string{{"a", "b", "c"}, {"d", "b", "e"}, {"e", "f", "a"}},
expected: []string{"a", "b", "c", "d", "e", "f"},
},
}
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
actual := ToSet(tc.input...)
sort.Strings(actual) // Sort is needed to compare the output because ToSet is non-deterministic
if !reflect.DeepEqual(actual, tc.expected) {
t.Errorf("wrong output. Actual=%v, Expected=%v", actual, tc.expected)
}
})
}
}