Strip .meta.managedFields for kubectl edit command #91946

soltysh · 2020-06-09T13:00:47Z

What type of PR is this?
/kind feature
/sig cli
/priority backlog

What this PR does / why we need it:
With the introduction of .meta.managedFields it's cumbersome to edit any resource due to the size of that additional fields. This PR hides those fields, since they should not even be modified by consumers directly.

Which issue(s) this PR fixes:
Fixes #90066

Special notes for your reviewer:
/assign @sttts @apelisse

Does this PR introduce a user-facing change?:

Hide managedFields from kubectl edit command

lavalamp · 2020-06-09T16:01:10Z

This field is editable and we shouldn't make that impossible. Can we get a flag for not stripping it?

lavalamp · 2020-06-09T16:02:32Z

staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions.go

@@ -437,6 +439,22 @@ func (o *EditOptions) Run() error {
 	}
 }

+func stripManagedFields(obj runtime.Object) error {


I think technically this should work, however I am not sure it is future proof if we e.g. add another field format. I would greatly prefer it if you put them back before constructing the patch that will be sent to apiserver.

apelisse · 2020-06-09T16:40:53Z

@soltysh Discussion is going on about this general topic here: #90066

Would be nice if we had a more general solution.

soltysh · 2020-07-07T12:30:55Z

@apelisse @lavalamp updated per your comments, ptal

soltysh · 2020-07-07T14:58:38Z

/retest

lavalamp · 2020-07-07T16:07:22Z

staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions.go

+		if err != nil {
+			return err
+		}
+		mf := o.managedFields[metaObjs.GetUID()]


I recommend detecting if the user tried to add something here and failing with a message if so, e.g. "you can't use kubectl edit to edit the managed field list".

Yeah, there's a mergepatch.PreconditionFunc which I wanted to update and I forgot, updated.

lavalamp · 2020-07-07T16:08:01Z

staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions.go

@@ -437,6 +454,49 @@ func (o *EditOptions) Run() error {
 	}
 }

+func (o *EditOptions) extractManagedFields(obj runtime.Object) error {


A test for extract / clear / restore would be good?

lavalamp · 2020-07-07T16:08:42Z

staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions.go

+	return metaObjs.GetUID(), mf, nil
+}
+
+func (o *EditOptions) reAddManagedFields(infos []*resource.Info) error {


nit: I suggest "restore" rather than "reAdd"

lavalamp · 2020-07-07T18:31:35Z

This looks to me like it does the right thing now. 👍

Maybe you want an official LGTM from someone a bit closer to the kubectl code though?

fejta-bot · 2020-07-07T18:56:54Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

apelisse · 2020-07-07T19:07:27Z

/lgtm

fejta-bot · 2020-07-08T00:30:53Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-07-08T05:45:55Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-07-08T09:15:49Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

soltysh · 2020-07-08T09:49:09Z

/hold
the verify failure is real

soltysh · 2020-07-08T10:19:24Z

/hold cancel
the gofmt was fixed

p0lyn0mial · 2020-07-08T10:22:02Z

/lgtm

soltysh · 2020-07-08T11:10:59Z

/milestone v1.19

apelisse · 2020-07-08T15:42:22Z

/retest

soltysh · 2020-07-08T20:42:35Z

/retest

fejta-bot · 2020-07-09T00:39:11Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

k8s-ci-robot assigned apelisse Jun 9, 2020

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Jun 9, 2020

k8s-ci-robot assigned sttts Jun 9, 2020

k8s-ci-robot requested review from deads2k and mengqiy June 9, 2020 13:01

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 9, 2020

lavalamp reviewed Jun 9, 2020

View reviewed changes

soltysh mentioned this pull request Jun 22, 2020

managedFields options are very verbose, making kubectl usage a bit #90066

Closed

soltysh force-pushed the strip_managed_fields branch from 6729ecf to 2774e92 Compare July 7, 2020 12:29

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 7, 2020

soltysh changed the title ~~WIP: Strip .meta.managedFields for kubectl edit command~~ Strip .meta.managedFields for kubectl edit command Jul 7, 2020

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2020

soltysh force-pushed the strip_managed_fields branch from 2774e92 to a2c6660 Compare July 7, 2020 12:30

lavalamp reviewed Jul 7, 2020

View reviewed changes

soltysh force-pushed the strip_managed_fields branch from a2c6660 to 86d2481 Compare July 7, 2020 18:25

k8s-ci-robot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 7, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 7, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 7, 2020

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 8, 2020

Strip .meta.managedFields for kubectl edit command

9e5c533

soltysh force-pushed the strip_managed_fields branch from c29c895 to 9e5c533 Compare July 8, 2020 10:17

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 8, 2020

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 8, 2020

k8s-ci-robot assigned p0lyn0mial Jul 8, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 8, 2020

k8s-ci-robot added this to the v1.19 milestone Jul 8, 2020

k8s-ci-robot merged commit f861a04 into kubernetes:master Jul 9, 2020

soltysh deleted the strip_managed_fields branch July 9, 2020 08:40

github-actions bot mentioned this pull request Jul 14, 2020

Week Ending July 12, 2020 dev-obs/actus#193

Open

vpnachev mentioned this pull request Jan 19, 2021

Hide managed field and last-applied-configuration in yaml editor. gardener/dashboard#935

Closed

soltysh mentioned this pull request Feb 5, 2021

KEP 2211: Omit managedFields server-side kubernetes/enhancements#2212

Closed

RainbowMango mentioned this pull request Apr 1, 2022

Error reported when executing kubectl edit cluster karmada-io/karmada#1571

Closed

henvic mentioned this pull request Nov 16, 2022

Kubectl edit: --show-managed-fields does nothing kubernetes/kubectl#1322

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Strip .meta.managedFields for kubectl edit command #91946

Strip .meta.managedFields for kubectl edit command #91946

soltysh commented Jun 9, 2020 •

edited

Loading

lavalamp commented Jun 9, 2020

lavalamp Jun 9, 2020

apelisse commented Jun 9, 2020

soltysh commented Jul 7, 2020

soltysh commented Jul 7, 2020

lavalamp Jul 7, 2020

soltysh Jul 7, 2020

lavalamp Jul 7, 2020

soltysh Jul 7, 2020

lavalamp Jul 7, 2020

soltysh Jul 7, 2020

lavalamp commented Jul 7, 2020

fejta-bot commented Jul 7, 2020

apelisse commented Jul 7, 2020

fejta-bot commented Jul 8, 2020

fejta-bot commented Jul 8, 2020

fejta-bot commented Jul 8, 2020

soltysh commented Jul 8, 2020

soltysh commented Jul 8, 2020

p0lyn0mial commented Jul 8, 2020

soltysh commented Jul 8, 2020

apelisse commented Jul 8, 2020

soltysh commented Jul 8, 2020

fejta-bot commented Jul 9, 2020

Strip .meta.managedFields for kubectl edit command #91946

Strip .meta.managedFields for kubectl edit command #91946

Conversation

soltysh commented Jun 9, 2020 • edited Loading

lavalamp commented Jun 9, 2020

lavalamp Jun 9, 2020

Choose a reason for hiding this comment

apelisse commented Jun 9, 2020

soltysh commented Jul 7, 2020

soltysh commented Jul 7, 2020

lavalamp Jul 7, 2020

Choose a reason for hiding this comment

soltysh Jul 7, 2020

Choose a reason for hiding this comment

lavalamp Jul 7, 2020

Choose a reason for hiding this comment

soltysh Jul 7, 2020

Choose a reason for hiding this comment

lavalamp Jul 7, 2020

Choose a reason for hiding this comment

soltysh Jul 7, 2020

Choose a reason for hiding this comment

lavalamp commented Jul 7, 2020

fejta-bot commented Jul 7, 2020

apelisse commented Jul 7, 2020

fejta-bot commented Jul 8, 2020

fejta-bot commented Jul 8, 2020

fejta-bot commented Jul 8, 2020

soltysh commented Jul 8, 2020

soltysh commented Jul 8, 2020

p0lyn0mial commented Jul 8, 2020

soltysh commented Jul 8, 2020

apelisse commented Jul 8, 2020

soltysh commented Jul 8, 2020

fejta-bot commented Jul 9, 2020

soltysh commented Jun 9, 2020 •

edited

Loading