Skip to content
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

Prevent deletion of local cluster #551

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dharmit
Copy link

@dharmit dharmit commented Dec 5, 2024

Issue:

rancher/rancher#32745
rancher/rancher#48303

Problem

Presently, it's possible to delete the local cluster using:

$ k delete cluster.management.cattle.io local

Other ways the local cluster, one on which Rancher is deployed, could be corrupted are mentioned in rancher/rancher#48303.

Solution

  • Prevent local cluster deletion:

    $  k delete cluster.management.cattle.io local
    Error from server (BadRequest): admission webhook "rancher.cattle.io.clusters.management.cattle.io" denied the request: cannot delete the local cluster
  • Prevent deletion of local and fleet-local namespaces:

    $ k delete ns local
    Error from server (BadRequest): admission webhook "rancher.cattle.io.namespaces" denied the request: deletion of namespace "local" is not allowed
    
    $ k delete ns fleet-local
    Error from server (BadRequest): admission webhook "rancher.cattle.io.namespaces" denied the request: deletion of namespace "fleet-local" is not allowed

CheckList

  • Test
  • Docs

@dharmit dharmit requested a review from a team as a code owner December 5, 2024 12:59
@dharmit dharmit marked this pull request as draft December 5, 2024 13:01
@dharmit dharmit requested a review from a team December 6, 2024 06:43
Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that "local" already appears in that file, I would set it to a constant called localCluster, like in pkg/resolvers/grbRuleResolvers.go

@dharmit dharmit force-pushed the fix-r/r-32745 branch 4 times, most recently from 18fa0d4 to eae9a9d Compare December 31, 2024 07:18
@dharmit dharmit marked this pull request as ready for review December 31, 2024 07:19
It prevents deletion of both clusters.provisioning.cattle.io and
cluster.management.cattle.io of the name `local`.

Signed-off-by: Dharmit Shah <[email protected]>
Signed-off-by: Dharmit Shah <[email protected]>
Copy link
Contributor

@jakefhyde jakefhyde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but just wanted to confirm that the agent's CLUSTER_CLEANUP is unimpacted.

@jakefhyde jakefhyde self-requested a review January 8, 2025 17:39
@dharmit dharmit requested a review from ericpromislow January 9, 2025 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants