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

Improve Templates deletion validation #355

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eromanova
Copy link
Member

@eromanova eromanova commented Sep 19, 2024

The following template deletion rules are validated:

  1. ClusterTemplate or ServiceTemplate can't be removed if it is in use by ManagedCluster
  2. ClusterTemplate or ServiceTemplate can't be removed if the template is managed by TemplateManagement
  3. ProviderTemplate can't be removed if it's a core provider or enabled in Management spec.providers

Closes #329

@eromanova eromanova self-assigned this Sep 19, 2024
@eromanova eromanova force-pushed the template-validation branch 2 times, most recently from 176cb64 to f27b9ce Compare September 23, 2024 10:31
@eromanova eromanova marked this pull request as ready for review September 23, 2024 10:32
@eromanova eromanova force-pushed the template-validation branch 4 times, most recently from 8fa5584 to 5d46cee Compare September 26, 2024 13:45
internal/webhook/template_webhook.go Outdated Show resolved Hide resolved
internal/webhook/template_webhook.go Outdated Show resolved Hide resolved
@eromanova eromanova marked this pull request as draft October 16, 2024 16:42
@eromanova eromanova marked this pull request as ready for review October 18, 2024 12:11
@eromanova eromanova changed the title Allow Templates deletion by the controller only Improve Templates deletion validation Oct 18, 2024
api/v1alpha1/common.go Outdated Show resolved Hide resolved
internal/webhook/template_webhook.go Outdated Show resolved Hide resolved
internal/webhook/userinfo.go Outdated Show resolved Hide resolved
@eromanova eromanova marked this pull request as draft October 21, 2024 13:07
@eromanova eromanova marked this pull request as ready for review October 21, 2024 18:55
@eromanova eromanova marked this pull request as draft October 24, 2024 09:21
@eromanova eromanova force-pushed the template-validation branch 2 times, most recently from ee7c734 to 995c17f Compare October 25, 2024 11:16
@eromanova eromanova marked this pull request as ready for review October 25, 2024 12:15
@eromanova eromanova force-pushed the template-validation branch 4 times, most recently from f210ccb to aa3e359 Compare November 4, 2024 10:50
@eromanova eromanova force-pushed the template-validation branch 2 times, most recently from 5c08a8c to 220b985 Compare November 12, 2024 08:54
@eromanova
Copy link
Member Author

eromanova commented Nov 12, 2024

@Kshatrix Could you please retake a look when you have time? Tests are failing in the CI env due to #624 but succeeded when run locally. Thanks

@eromanova eromanova marked this pull request as draft November 21, 2024 11:43
The following rules are validated:
1. ClusterTemplate or ServiceTemplate can't be removed if it is in use by
   ManagedCluster or TemplateChain
2. ClusterTemplate or ServiceTemplate can't be removed if the template is
   managed by the TemplateManagement
3. ProviderTemplate can't be removed if it's a core provider or enabled in
   Management spec.providers
@@ -231,9 +231,6 @@ var _ = Describe("Template Chain Controller", func() {

Eventually(k8sClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, chain, clusterTemplateChainResource).Should(HaveOccurred())
}
for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ctTemplates["test"], ctTemplates["ct0"], ctTemplates["ct2"]} {
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing the templates' cleanup since the webhook now denies these requests. Templates have owner reference set and should be automatically deleted by the garbage collector. The owner reference existence is checked below.

@eromanova eromanova marked this pull request as ready for review November 21, 2024 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[admission] Restrict manual removal of Templates managed by HMC
2 participants