add oc adm prune role command to replace the existing reaper

[deads2k]

oc adm prune role accepts input like oc delete does, but does not remove the role itself. It removes the bindings.

If we follow this pattern, I would anticipate doing the same for future ones like oc adm prune user.

I think I prefer something like oc reap <resource string>. It works better with the "normal" command flow of doing generic things and it makes for an easy transition since you just oc reap <whatever flags>, then oc delete <whatever flags>.

@liggitt You asked for it this way. How about oc reap instead?

callers to `oc delete` that are deleting users, groups, roles, or cluster roles should first call `oc adm prune auth` with the resources.  That will remove deps before deletion.  `oc delete` will no longer clean up dependent resources in a future release.

[deads2k]

We spoke. We'll move toward oc adm prune auth <resource string> that ignores unknowns and reaps what it knows without deleting the objects themselves.

[enj]

@deads2k remind me why we are moving away from reapers / why we cannot use GC for this?

This deletes bindings that reference a role correct?

@openshift/sig-security

[liggitt]

why we are moving away from reapers

reapers started upstream as a client-side substitute for GC, and did whole-object dependent removal. they are being replaced upstream by server-side GC.

why we cannot use GC for this?

we made use of the reaper hook to do partial update of dependent objects. GC cannot reach into referencing objects and modify them arbitrarily

[enj]

we made use of the reaper hook to do partial update of dependent objects. GC cannot reach into referencing objects and modify them arbitrarily

Right, but does something prevent us from setting ownerReferences in new role bindings? That way if you delete a role all the role bindings get GCed?

[deads2k]

Right, but does something prevent us from setting ownerReferences in new role bindings? That way if you delete a role all the role bindings get GCed?

A possibly reasonable improvement that could be made upstream for this. Since GC expanded for cluster scoped resources, for cluster scoped ones too. How it gets added isn't obvious and it changes how manifests are handled for cluster-admins.

For users and groups it won't work. This doesn't expand our surface area and doesn't preclude future improvements, but does protect us from upstream developments.

[deads2k]

/retest

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k

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

The pull request process is described here

Needs approval from an approver in each of these files:

• contrib/completions/OWNERS [deads2k]
• docs/man/OWNERS [deads2k]
• hack/OWNERS [deads2k]
• pkg/authorization/OWNERS [deads2k]
• pkg/oc/OWNERS [deads2k]
• pkg/user/OWNERS [deads2k]
• test/cmd/OWNERS [deads2k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[deads2k]

/assign @soltysh

We'll want this to provide a transition for people using delete on these resources. Check the release not and find the write spot to update in the docs.

[soltysh]

@deads2k the idea is legit, but I'd really see the new pruners follow the patterns from the old ones, rather than using the reapers one. I'm worried that this might lead to others copying the wrong approach. Can be done as a followup, I can pick it up if you want me to.

[deads2k]

@deads2k the idea is legit, but I'd really see the new pruners follow the patterns from the old ones, rather than using the reapers one. I'm worried that this might lead to others copying the wrong approach. Can be done as a followup, I can pick it up if you want me to

I don't understand what you mean. I don't think we'll end up with more of these, hence the non-generic name. The CLI contract is backwards from a "normal" pruner.

[deads2k]

/retest

[deads2k]

Spoke on a voice call. A refactor can wait until after the 1.11 rebase to avoid extra constraints

[soltysh]

Yup to what David said above.

/lgtm

[soltysh]

/lgtm

[deads2k]

/retest

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[deads2k]

/retest

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.