-
Notifications
You must be signed in to change notification settings - Fork 40.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add reconcile command to kubectl auth #51636
Conversation
084b956
to
271049e
Compare
/release-note |
pkg/kubectl/cmd/auth/reconcile.go
Outdated
ContinueOnError(). | ||
NamespaceParam(namespace).DefaultNamespace(). | ||
FilenameParam(enforceNamespace, options). | ||
ResourceTypeOrNameArgs(false, args...). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ResourceTypeOrNameArgs
lets them specify a resource name that is fetched from the server? what would that do for reconcile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
pkg/kubectl/cmd/auth/reconcile.go
Outdated
}, | ||
} | ||
result, err := reconcileOptions.Run() | ||
shallowInfoCopy.Object = result.Role.GetObject() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the error is non-nil can result
be nil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the error is non-nil can result be nil?
Maybe. I'll gate it.
pkg/kubectl/cmd/auth/reconcile.go
Outdated
|
||
var ( | ||
reconcileLong = templates.LongDesc(` | ||
Reconciles rules for RBAC Role, RoleBinding, ClusterRole, and ClusterRole binding objects`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
271049e
to
f52f7d3
Compare
comments addressed |
|
||
cmdutil.AddPrinterFlags(cmd) | ||
usage := "identifying the resource to reconcile." | ||
cmdutil.AddFilenameOptionFlags(cmd, fileOptions, usage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmd.MarkFlagRequired("filename")
since it is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmd.MarkFlagRequired("filename") since it is required.
done
pkg/kubectl/cmd/auth/reconcile.go
Outdated
} | ||
|
||
cmd := &cobra.Command{ | ||
Use: "reconcile -f [FILE|URL|DIRECTORY]", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been using the usage like reconcile -f FILENAME
in other places (create
, convert
, etc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been using the usage like reconcile -f FILENAME in other places (create, convert, etc).
done
} | ||
|
||
// shallowInfoCopy this is used to later twiddle the Object for printing | ||
// we really need more straightforward printing options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juanvallejo fyi
pkg/kubectl/cmd/auth/reconcile.go
Outdated
return nil | ||
|
||
default: | ||
// skip ignored resources |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, worth to glog.V(n)
print the ignored ones?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, worth to glog.V(n) print the ignored ones?
done
f52f7d3
to
d769e2c
Compare
d769e2c
to
aa63750
Compare
comments addressed. |
LGTM |
/test pull-kubernetes-e2e-gce-gpu |
/retest |
/approve no-issue |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deads2k Assign the PR to them by writing 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:
You can indicate your approval by writing |
docs are generated, labeled. |
Automatic merge from submit-queue (batch tested with PRs 50832, 51119, 51636, 48921, 51712) |
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