Skip to content

Clarify or drop the Deleting phase arm in the lifecycle groups authorizer #4513

Open

Description

@mjudeikis-bot

What exists today

pkg/authorization/lifecycle_groups_authorizer.go:76 (added in #4507):

case corev1alpha1.LogicalClusterPhaseTerminating, corev1alpha1.LogicalClusterPhaseDeleting:
    if isRegisteredTerminator(attr.GetUser().GetGroups(), logicalCluster) {

isRegisteredTerminator requires the group's terminator to be present in Status.Terminators. The phase reconciler (pkg/reconciler/core/logicalcluster/logicalcluster_reconcile_phase.go:37-57) only moves a LogicalCluster to Deleting once Status.Terminators is empty. As written, a terminator-group request in Deleting therefore never matches, and the Deleting label suggests access that is never granted.

What should happen

Work out why Deleting is in the case list (it may be historical, e.g. the earlier phase-only check, or there may be a path I have not traced), then do one of:

  • drop Deleting from the case, or
  • keep it and add a comment explaining why, or
  • if a terminator is meant to keep access while it removes itself, change the check accordingly.

This is deliberately separate from #4507, as requested.

Links

Steps

  • Find out why Deleting was included (git history of the authorizer and phase reconciler)
  • Decide: drop the arm, keep it with a comment, or change isRegisteredTerminator semantics
  • Update lifecycle_groups_authorizer.go accordingly
  • Add or adjust a unit test covering a terminator-group request in Deleting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    • Status
      New

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions