fix: use fleet-/kube- prefix checks in fleet guard rail

[Arvindthiru]

Description of your changes

Currently the fleet guard rail prevents users from creating namespaces with fleet/kube prefix instead of fleet-/kube- prefix.

This PR fixes that issue and I moved some code around to reuse code.

Eg: fleettest, kubetest namespaces will be prevented from being created which is not ideal

Fixes #

I have:

• Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

[kaito-pr-agent]

Title

(Describe updated until commit 3e0f15d)

fix: use fleet-/kube- prefix checks in fleet guard rail

---

User description

Description of your changes

Currently the fleet guard rail prevents users from creating namespaces with fleet/kube prefix instead of fleet-/kube- prefix.

This PR fixes that issue and I moved some code around to reuse code.

Eg: fleettest, kubetest namespaces will be prevented from being created which is not ideal

Fixes #

I have:

• Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

---

PR Type

Enhancement

---

Description

• Added function IsFleetMemberNamespace to check if a namespace starts with fleet-member-.

• Updated namespace validation logic to use IsFleetMemberNamespace and improved logging messages.

• Refactored test cases to include new scenarios for non-reserved namespaces.

---

Changes walkthrough 📝

	Relevant files
Enhancement	common.go Refactor namespace validation logic                                            pkg/utils/common.go Introduced fleetMemberNamespacePrefix constant. Created IsFleetMemberNamespace function. Updated namespace-related constants and functions. +15/-9    fleetresourcehandler_webhook.go Improve namespace validation and logging                                  pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go Updated namespace validation logic to use IsFleetMemberNamespace. Improved logging messages for various namespace operations. Refactored test cases to include new scenarios. +19/-17
Tests	fleetresourcehandler_webhook_test.go Add test cases for non-reserved namespaces                              pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go Added test cases for non-reserved namespaces. +58/-5

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[kaito-pr-agent]

PR Reviewer Guide 🔍

(Review updated until commit 3e0f15d)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ No major issues detected

[kaito-pr-agent]

PR Code Suggestions ✨

Latest suggestions up to 6c32d3e
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Improve log message readability Improve log message readability. pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go [140] -klog.V(3).InfoS(allowedMessageMemberCluster, +klog.V(3).InfoS("Allowed creation/deletion of upstream member cluster resource", "user", req.UserInfo.Username, "groups", req.UserInfo.Groups, "operation", req.Operation, "kind", req.RequestKind.Kind, "subResource", req.SubResource, "namespacedName", types.NamespacedName{Name: req.Name, Namespace: req.Namespace}) Suggestion importance[1-10]: 7 __ Why: This suggestion improves the readability of the log message by providing a clearer description of what is being logged. It increases the maintainability of the code.	Medium
	Improve test case names Ensure test case names accurately reflect their purpose. pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go [704] -wantResponse: admission.Allowed(allowedMessageFleetReservedNamespacedResource), +wantResponse: admission.Allowed("Allowed operation on fleet reserved namespace"), Suggestion importance[1-10]: 7 __ Why: Improving test case names makes it easier to understand the purpose of each test case, which enhances the overall readability and maintainability of the tests.	Medium

---

Previous suggestions

Suggestions up to commit 468009c

Category	Suggestion	Impact
General	Enhance log message clarity Improve log message readability. pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go [140] -klog.V(3).InfoS(allowedMessageMemberCluster, +klog.V(3).InfoS("Allowed creation/deletion of upstream member cluster resource", "user", req.UserInfo.Username, "groups", req.UserInfo.Groups, "operation", req.Operation, "kind", req.RequestKind.Kind, "subResource", req.SubResource, "namespacedName", types.NamespacedName{Name: req.Name, Namespace: req.Namespace}) Suggestion importance[1-10]: 4 __ Why: The suggestion improves the readability of the log message, but it's a minor improvement.	Low
	Remove redundant comment Simplify the function by removing unnecessary comments. pkg/utils/common.go [596] +func IsFleetMemberNamespace(namespace string) bool { + return strings.HasPrefix(namespace, fleetMemberNamespacePrefix) +} - Suggestion importance[1-10]: 2 __ Why: The suggestion is not actionable and only asks to remove a comment, which is not necessary.	Low
	Maintain consistency Ensure consistent naming convention. pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go [990] +wantResponse: admission.Allowed(allowedMessageNonReservedNamespace), - Suggestion importance[1-10]: 2 __ Why: The suggestion is not actionable and only asks to maintain consistency, which is not necessary.	Low

[kaito-pr-agent]

Persistent review updated to latest commit 6c32d3e

[Copilot]

Pull Request Overview

This pull request refactors the fleet resource webhook and test logic to enforce the use of consistent fleet- and kube- prefixed namespace names while updating log messages accordingly.

• Updated test cases to validate allowed operations based on the new namespace prefix rules.
• Refactored webhook code to use utility functions (IsFleetMemberNamespace and IsReservedNamespace) for prefix checks.
• Updated constants and formatting in the common utilities to support fleet member namespace naming.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go	Added and renamed test cases to verify behavior with updated namespace prefix logic.
pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go	Replaced direct prefix checks with utility functions and updated logging messages to use defined constants.
pkg/utils/common.go	Introduced a new constant for fleet member namespace and added a helper function for its check.

Comments suppressed due to low confidence (2)

pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go:177

• [nitpick] Consider renaming the constant 'allowedMessageNonReservedNamespace' to something like 'allowedMessageForNonReservedNamespace' to better reflect its purpose, improving readability in the logs.

klog.V(3).InfoS(allowedMessageNonReservedNamespace,

pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go:976

• [nitpick] For consistency, consider reviewing and standardizing the phrasing used in test case descriptions across non-reserved namespace tests.

"allow any user to modify non-reserved namespace": {

[Copilot]

Pull Request Overview

This pull request fixes the prefix check logic for fleet and kube resources by replacing hard-coded string checks with helper functions and updating the corresponding log and admission messages. Key changes include:

• Adding new test cases for member cluster and namespace handling scenarios.
• Refactoring webhook logic to use centralized helper functions from pkg/utils/common.go for prefix checks.
• Updating log and admission response messages to use well-defined constants.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go	Added test cases and updated expected admission responses using constants.
pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go	Replaced inline prefix checks with helper functions and updated log messages.
pkg/utils/common.go	Introduced a new helper function for detecting fleet member namespaces and updated prefix constants.

Comments suppressed due to low confidence (2)

pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go:981

• [nitpick] Test cases use 'testUser' here while other tests use 'test-user'; consider standardizing the username format for consistency.

Username: "testUser",

pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go:161

• [nitpick] The log message constant 'allowedMessageFleetReservedNamespacedResource' is slightly verbose; consider simplifying or clarifying the message text for better readability.

klog.V(3).InfoS(allowedMessageFleetReservedNamespacedResource,

[michaelawyu]

A few minor comments, PTAL

[kaito-pr-agent]

Persistent review updated to latest commit 3e0f15d

[michaelawyu]

LGTM ;)