Merge pull request #36672 from ymqytw/check_annotation_for_apply

Automatic merge from submit-queue (batch tested with PRs 38453, 36672, 38629, 34966, 38630)

Warn user if they try to apply on an object without the annotation

Give user a warning when using `kubectl apply` on a resource lacking the `LastAppliedConfig` annotation

fixes #36620

Ref: #35134 and #36509.

**Release note**:
```release-note
Issue a warning when using `kubectl apply` on a resource lacking the `LastAppliedConfig` annotation
```

pkg/kubectl/cmd/apply.go

@@ -61,6 +61,8 @@ const (
 	backOffPeriod = 1 * time.Second
 	// how many times we can retry before back off
 	triesBeforeBackOff = 1
+
+	warningNoLastAppliedConfigAnnotation = "Warning: kubectl apply should be used on resource created by either kubectl create --save-config or kubectl apply\n"
 )
 
 var (
@@ -88,7 +90,7 @@ var (
 		kubectl apply --prune -f manifest.yaml --all --prune-whitelist=core/v1/ConfigMap`)
 )
 
-func NewCmdApply(f cmdutil.Factory, out io.Writer) *cobra.Command {
+func NewCmdApply(f cmdutil.Factory, out, errOut io.Writer) *cobra.Command {
 	var options ApplyOptions
 
 	cmd := &cobra.Command{
@@ -100,7 +102,7 @@ func NewCmdApply(f cmdutil.Factory, out io.Writer) *cobra.Command {
 			cmdutil.CheckErr(validateArgs(cmd, args))
 			cmdutil.CheckErr(cmdutil.ValidateOutputArgs(cmd))
 			cmdutil.CheckErr(validatePruneAll(options.Prune, cmdutil.GetFlagBool(cmd, "all"), options.Selector))
-			cmdutil.CheckErr(RunApply(f, cmd, out, &options))
+			cmdutil.CheckErr(RunApply(f, cmd, out, errOut, &options))
 		},
 	}
 
@@ -161,7 +163,7 @@ func parsePruneResources(gvks []string) ([]pruneResource, error) {
 	return pruneResources, nil
 }
 
-func RunApply(f cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *ApplyOptions) error {
+func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, options *ApplyOptions) error {
 	shortOutput := cmdutil.GetFlagString(cmd, "output") == "name"
 	schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
 	if err != nil {
@@ -256,6 +258,13 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *App
 		}
 
 		if !dryRun {
+			annotationMap, err := info.Mapping.MetadataAccessor.Annotations(info.Object)
+			if err != nil {
+				return err
+			}
+			if _, ok := annotationMap[annotations.LastAppliedConfigAnnotation]; !ok {
+				fmt.Fprintf(errOut, warningNoLastAppliedConfigAnnotation)
+			}
 			overwrite := cmdutil.GetFlagBool(cmd, "overwrite")
 			helper := resource.NewHelper(info.Client, info.Mapping)
 			patcher := &patcher{

pkg/kubectl/cmd/apply_test.go

@@ -41,9 +41,10 @@ import (
 
 func TestApplyExtraArgsFail(t *testing.T) {
 	buf := bytes.NewBuffer([]byte{})
+	errBuf := bytes.NewBuffer([]byte{})
 
 	f, _, _, _ := cmdtesting.NewAPIFactory()
-	c := NewCmdApply(f, buf)
+	c := NewCmdApply(f, buf, errBuf)
 	if validateApplyArgs(c, []string{"rc"}) == nil {
 		t.Fatalf("unexpected non-error")
 	}
@@ -77,6 +78,20 @@ func readBytesFromFile(t *testing.T, filename string) []byte {
 	return data
 }
 
+func readReplicationController(t *testing.T, filenameRC string) (string, []byte) {
+	rcObj := readReplicationControllerFromFile(t, filenameRC)
+	metaAccessor, err := meta.Accessor(rcObj)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rcBytes, err := runtime.Encode(testapi.Default.Codec(), rcObj)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return metaAccessor.GetName(), rcBytes
+}
+
 func readReplicationControllerFromFile(t *testing.T, filename string) *api.ReplicationController {
 	data := readBytesFromFile(t, filename)
 	rc := api.ReplicationController{}
@@ -177,6 +192,50 @@ func walkMapPath(t *testing.T, start map[string]interface{}, path []string) map[
 	return finish
 }
 
+func TestApplyObjectWithoutAnnotation(t *testing.T) {
+	initTestErrorHandler(t)
+	nameRC, rcBytes := readReplicationController(t, filenameRC)
+	pathRC := "/namespaces/test/replicationcontrollers/" + nameRC
+
+	f, tf, _, ns := cmdtesting.NewAPIFactory()
+	tf.Printer = &testPrinter{}
+	tf.Client = &fake.RESTClient{
+		NegotiatedSerializer: ns,
+		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+			switch p, m := req.URL.Path, req.Method; {
+			case p == pathRC && m == "GET":
+				bodyRC := ioutil.NopCloser(bytes.NewReader(rcBytes))
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bodyRC}, nil
+			case p == pathRC && m == "PATCH":
+				bodyRC := ioutil.NopCloser(bytes.NewReader(rcBytes))
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bodyRC}, nil
+			default:
+				t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
+				return nil, nil
+			}
+		}),
+	}
+	tf.Namespace = "test"
+	tf.ClientConfig = defaultClientConfig()
+	buf := bytes.NewBuffer([]byte{})
+	errBuf := bytes.NewBuffer([]byte{})
+
+	cmd := NewCmdApply(f, buf, errBuf)
+	cmd.Flags().Set("filename", filenameRC)
+	cmd.Flags().Set("output", "name")
+	cmd.Run(cmd, []string{})
+
+	// uses the name from the file, not the response
+	expectRC := "replicationcontroller/" + nameRC + "\n"
+	expectWarning := warningNoLastAppliedConfigAnnotation
+	if errBuf.String() != expectWarning {
+		t.Fatalf("unexpected non-warning: %s\nexpected: %s", errBuf.String(), expectWarning)
+	}
+	if buf.String() != expectRC {
+		t.Fatalf("unexpected output: %s\nexpected: %s", buf.String(), expectRC)
+	}
+}
+
 func TestApplyObject(t *testing.T) {
 	initTestErrorHandler(t)
 	nameRC, currentRC := readAndAnnotateReplicationController(t, filenameRC)
@@ -203,8 +262,9 @@ func TestApplyObject(t *testing.T) {
 	}
 	tf.Namespace = "test"
 	buf := bytes.NewBuffer([]byte{})
+	errBuf := bytes.NewBuffer([]byte{})
 
-	cmd := NewCmdApply(f, buf)
+	cmd := NewCmdApply(f, buf, errBuf)
 	cmd.Flags().Set("filename", filenameRC)
 	cmd.Flags().Set("output", "name")
 	cmd.Run(cmd, []string{})
@@ -254,8 +314,9 @@ func TestApplyRetry(t *testing.T) {
 	}
 	tf.Namespace = "test"
 	buf := bytes.NewBuffer([]byte{})
+	errBuf := bytes.NewBuffer([]byte{})
 
-	cmd := NewCmdApply(f, buf)
+	cmd := NewCmdApply(f, buf, errBuf)
 	cmd.Flags().Set("filename", filenameRC)
 	cmd.Flags().Set("output", "name")
 	cmd.Run(cmd, []string{})
@@ -297,8 +358,9 @@ func TestApplyNonExistObject(t *testing.T) {
 	}
 	tf.Namespace = "test"
 	buf := bytes.NewBuffer([]byte{})
+	errBuf := bytes.NewBuffer([]byte{})
 
-	cmd := NewCmdApply(f, buf)
+	cmd := NewCmdApply(f, buf, errBuf)
 	cmd.Flags().Set("filename", filenameRC)
 	cmd.Flags().Set("output", "name")
 	cmd.Run(cmd, []string{})
@@ -353,8 +415,9 @@ func testApplyMultipleObjects(t *testing.T, asList bool) {
 	}
 	tf.Namespace = "test"
 	buf := bytes.NewBuffer([]byte{})
+	errBuf := bytes.NewBuffer([]byte{})
 
-	cmd := NewCmdApply(f, buf)
+	cmd := NewCmdApply(f, buf, errBuf)
 	if asList {
 		cmd.Flags().Set("filename", filenameRCSVC)
 	} else {

pkg/kubectl/cmd/cmd.go

@@ -281,7 +281,7 @@ func NewKubectlCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob
 		{
 			Message: "Advanced Commands:",
 			Commands: []*cobra.Command{
-				NewCmdApply(f, out),
+				NewCmdApply(f, out, err),
 				NewCmdPatch(f, out),
 				NewCmdReplace(f, out),
 				NewCmdConvert(f, out),