add reconcile command to kubectl auth #51636

deads2k · 2017-08-30T16:10:20Z

This pull exposes the RBAC reconcile commands through kubectl auth reconcile -f FILE. When passed a file which contains RBAC roles, rolebindings, clusterroles, or clusterrolebindings, it will compute covers and add the missing rules.

The logic required to properly "apply" rbac permissions is more complicated that a json merge since you have to compute logical covers operations between rule sets. This means that we cannot use kubectl apply to update rbac roles without risking breaking old clients (like controllers).

To solve this problem, RBAC created reconcile functions to use during startup for "stock" roles. We want to offer this power to users who are running their own controllers and extension servers.

This is an intersection between @kubernetes/sig-auth-misc and @kubernetes/sig-cli-misc

enj · 2017-08-30T16:23:16Z

@deads2k I assume we plan to expose these in some manner in Openshift?

cc @simo5

deads2k · 2017-08-30T16:26:06Z

@deads2k I assume we plan to expose these in some manner in Openshift?

cc @simo5

Nothing special. oc already wraps kubectl. This is needed to make RBAC upgrades whole for kube consumers.

fabianofranz · 2017-08-30T17:15:36Z

/release-note

liggitt · 2017-08-30T17:30:37Z

pkg/kubectl/cmd/auth/reconcile.go

+		ContinueOnError().
+		NamespaceParam(namespace).DefaultNamespace().
+		FilenameParam(enforceNamespace, options).
+		ResourceTypeOrNameArgs(false, args...).


ResourceTypeOrNameArgs lets them specify a resource name that is fetched from the server? what would that do for reconcile?

ResourceTypeOrNameArgs lets them specify a resource name that is fetched from the server? what would that do for reconcile?

nothing since args are forced to be zero length. I'll remove that bit though.

ericchiang · 2017-08-30T16:53:04Z

pkg/kubectl/cmd/auth/reconcile.go

+				},
+			}
+			result, err := reconcileOptions.Run()
+			shallowInfoCopy.Object = result.Role.GetObject()


When the error is non-nil can result be nil?

When the error is non-nil can result be nil?

Maybe. I'll gate it.

ericchiang · 2017-08-30T18:01:21Z

pkg/kubectl/cmd/auth/reconcile.go

+
+var (
+	reconcileLong = templates.LongDesc(`
+		Reconciles rules for RBAC Role, RoleBinding, ClusterRole, and ClusterRole binding objects`)


Would this always be used instead of kubectl apply, or are there different cases when one is preferred instead of another? Can we add that to the description?

Would this always be used instead of kubectl apply, or are there different cases when one is preferred instead of another? Can we add that to the description?

I can't think of a reason to use apply with rbac resources. It rarely does what you want. Maybe in the bare creation case. I can add it.

deads2k · 2017-08-30T19:13:21Z

comments addressed

fabianofranz · 2017-08-30T19:34:26Z

pkg/kubectl/cmd/auth/reconcile.go

+
+	cmdutil.AddPrinterFlags(cmd)
+	usage := "identifying the resource to reconcile."
+	cmdutil.AddFilenameOptionFlags(cmd, fileOptions, usage)


cmd.MarkFlagRequired("filename") since it is required.

cmd.MarkFlagRequired("filename") since it is required.

done

fabianofranz · 2017-08-30T19:36:47Z

pkg/kubectl/cmd/auth/reconcile.go

+	}
+
+	cmd := &cobra.Command{
+		Use:     "reconcile -f [FILE|URL|DIRECTORY]",


We've been using the usage like reconcile -f FILENAME in other places (create, convert, etc).

We've been using the usage like reconcile -f FILENAME in other places (create, convert, etc).

done

fabianofranz · 2017-08-30T19:41:45Z

pkg/kubectl/cmd/auth/reconcile.go

+		}
+
+		// shallowInfoCopy this is used to later twiddle the Object for printing
+		// we really need more straightforward printing options


@juanvallejo fyi

fabianofranz · 2017-08-30T19:43:31Z

pkg/kubectl/cmd/auth/reconcile.go

+			return nil
+
+		default:
+			// skip ignored resources


nit, worth to glog.V(n) print the ignored ones?

nit, worth to glog.V(n) print the ignored ones?

done

deads2k · 2017-08-30T20:04:16Z

comments addressed.

fabianofranz · 2017-08-30T21:10:42Z

LGTM

ericchiang · 2017-08-30T21:21:45Z

/test pull-kubernetes-e2e-gce-gpu

deads2k · 2017-08-31T01:36:05Z

/retest

deads2k · 2017-08-31T12:36:57Z

/approve no-issue

k8s-github-robot · 2017-08-31T12:37:55Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deads2k
We suggest the following additional approver: smarterclayton

Assign the PR to them by writing /assign @smarterclayton in a comment when ready.

Associated issue requirement bypassed by: deads2k

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

docs/OWNERS
~~hack/OWNERS~~ [deads2k]
~~pkg/kubectl/OWNERS~~ [deads2k]
~~pkg/registry/OWNERS~~ [deads2k]
~~test/fixtures/pkg/kubectl/OWNERS~~ [deads2k]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

deads2k · 2017-08-31T12:52:26Z

docs are generated, labeled.

k8s-github-robot · 2017-09-03T02:26:24Z

Automatic merge from submit-queue (batch tested with PRs 50832, 51119, 51636, 48921, 51712)

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 30, 2017

k8s-github-robot assigned smarterclayton and fejta Aug 30, 2017

k8s-github-robot added the release-note-label-needed label Aug 30, 2017

deads2k assigned fabianofranz Aug 30, 2017

deads2k force-pushed the cli-01-reconcile branch from 084b956 to 271049e Compare August 30, 2017 16:11

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Aug 30, 2017

liggitt reviewed Aug 30, 2017

View reviewed changes

ericchiang reviewed Aug 30, 2017

View reviewed changes

deads2k force-pushed the cli-01-reconcile branch from 271049e to f52f7d3 Compare August 30, 2017 19:13

k8s-github-robot added the kind/old-docs label Aug 30, 2017

fabianofranz reviewed Aug 30, 2017

View reviewed changes

deads2k force-pushed the cli-01-reconcile branch from f52f7d3 to d769e2c Compare August 30, 2017 20:03

add reconcile command to kubectl auth

aa63750

deads2k force-pushed the cli-01-reconcile branch from d769e2c to aa63750 Compare August 30, 2017 20:04

deads2k added approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Aug 31, 2017

deads2k added this to the v1.8 milestone Sep 1, 2017

k8s-github-robot merged commit a3aac42 into kubernetes:master Sep 3, 2017

enj mentioned this pull request Sep 8, 2017

Reconciliation differences between upstream and origin openshift/origin#15412

Closed

deads2k deleted the cli-01-reconcile branch July 3, 2018 18:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add reconcile command to kubectl auth #51636

add reconcile command to kubectl auth #51636

deads2k commented Aug 30, 2017

enj commented Aug 30, 2017

deads2k commented Aug 30, 2017

fabianofranz commented Aug 30, 2017

liggitt Aug 30, 2017

deads2k Aug 30, 2017

ericchiang Aug 30, 2017

deads2k Aug 30, 2017

ericchiang Aug 30, 2017

deads2k Aug 30, 2017

deads2k commented Aug 30, 2017

fabianofranz Aug 30, 2017

deads2k Aug 30, 2017

fabianofranz Aug 30, 2017

deads2k Aug 30, 2017

fabianofranz Aug 30, 2017

fabianofranz Aug 30, 2017

deads2k Aug 30, 2017

deads2k commented Aug 30, 2017

fabianofranz commented Aug 30, 2017

ericchiang commented Aug 30, 2017

deads2k commented Aug 31, 2017

deads2k commented Aug 31, 2017

k8s-github-robot commented Aug 31, 2017

deads2k commented Aug 31, 2017

k8s-github-robot commented Sep 3, 2017

add reconcile command to kubectl auth #51636

add reconcile command to kubectl auth #51636

Conversation

deads2k commented Aug 30, 2017

enj commented Aug 30, 2017

deads2k commented Aug 30, 2017

fabianofranz commented Aug 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Aug 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Aug 30, 2017

fabianofranz commented Aug 30, 2017

ericchiang commented Aug 30, 2017

deads2k commented Aug 31, 2017

deads2k commented Aug 31, 2017

k8s-github-robot commented Aug 31, 2017

deads2k commented Aug 31, 2017

k8s-github-robot commented Sep 3, 2017