Skip to content

Commit 5f419c1

Browse files
jwttyjwtty
committed
fix comments
1 parent 909862a commit 5f419c1

6 files changed

Lines changed: 70 additions & 46 deletions

File tree

‎pkg/utils/azure.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
Copyright 2025 The KubeFleet Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package utils
18+
19+
import (
20+
"slices"
21+
22+
authenticationv1 "k8s.io/api/authentication/v1"
23+
)
24+
25+
// This file contains constants and utility functions related to Azure-specific logic.
26+
// We define these in a separate file to avoid potential merge conflicts from the KubeFleet repository when backporting changes.
27+
const (
28+
// ReconcileLabelKey is the label key added to resources that should be reconciled.
29+
// The value indicates the reason the resource should be reconciled.
30+
ReconcileLabelKey = "fleet.azure.com/reconcile"
31+
// ReconcileLabelValue is the label value paired with ReconcileLabelKey,
32+
// indicating the resource is managed and should be reconciled.
33+
ReconcileLabelValue = "managed"
34+
35+
// AKSServiceUserName is the username whose deployment requests should be mutated.
36+
AKSServiceUserName = "aksService"
37+
// SystemMastersGroup is the Kubernetes group representing cluster administrators.
38+
SystemMastersGroup = "system:masters"
39+
)
40+
41+
// IsAKSService reports whether the user is the aksService user with
42+
// system:masters group membership.
43+
func IsAKSService(userInfo authenticationv1.UserInfo) bool {
44+
return userInfo.Username == AKSServiceUserName && slices.Contains(userInfo.Groups, SystemMastersGroup)
45+
}
46+
47+
// HasReconcileLabel reports whether the given labels contain the fleet reconcile
48+
// label key with a non-empty value.
49+
func HasReconcileLabel(labels map[string]string) bool {
50+
return labels[ReconcileLabelKey] != ""
51+
}

‎pkg/utils/common.go‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,6 @@ const (
6060
MutatingPathFmt = "/mutate-%s-%s-%s"
6161
lessGroupsStringFormat = "groups: %v"
6262
moreGroupsStringFormat = "groups: [%s, %s, %s,......]"
63-
64-
// ReconcileLabelKey is the label key added to resources that should be reconciled.
65-
// The value indicates the reason the resource should be reconciled.
66-
ReconcileLabelKey = "fleet.azure.com/reconcile"
67-
// ReconcileLabelValue is the label value paired with ReconcileLabelKey,
68-
// indicating the resource is managed and should be reconciled.
69-
ReconcileLabelValue = "managed"
7063
)
7164

7265
const (
@@ -511,12 +504,6 @@ func IsReservedNamespace(namespace string) bool {
511504
return strings.HasPrefix(namespace, fleetPrefix) || strings.HasPrefix(namespace, kubePrefix)
512505
}
513506

514-
// HasReconcileLabel reports whether the given labels contain the fleet reconcile
515-
// label key with a non-empty value.
516-
func HasReconcileLabel(labels map[string]string) bool {
517-
return labels[ReconcileLabelKey] != ""
518-
}
519-
520507
// IsFleetMemberNamespace indicates if an argued namespace is a fleet member namespace.
521508
func IsFleetMemberNamespace(namespace string) bool {
522509
return strings.HasPrefix(namespace, fleetMemberNamespacePrefix)

pkg/webhook/deployment/deployment_mutating_webhook.go renamed to pkg/webhook/deployment/mutating_webhook.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (m *deploymentMutator) Handle(_ context.Context, req admission.Request) adm
5858

5959
// Only mutate when the request was made by the aksService user with
6060
// system:masters group membership.
61-
if !isAKSService(req.UserInfo) {
61+
if !utils.IsAKSService(req.UserInfo) {
6262
return admission.Allowed("user is not aksService, no mutation needed")
6363
}
6464

pkg/webhook/deployment/deployment_mutating_webhook_test.go renamed to pkg/webhook/deployment/mutating_webhook_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,18 +58,18 @@ func TestHandle(t *testing.T) {
5858
decoder := admission.NewDecoder(scheme)
5959

6060
aksServiceUser := authenticationv1.UserInfo{
61-
Username: AKSServiceUserName,
62-
Groups: []string{SystemMastersGroup},
61+
Username: utils.AKSServiceUserName,
62+
Groups: []string{utils.SystemMastersGroup},
6363
}
6464

6565
aksServiceUserNoMasters := authenticationv1.UserInfo{
66-
Username: AKSServiceUserName,
66+
Username: utils.AKSServiceUserName,
6767
Groups: []string{"system:authenticated"},
6868
}
6969

7070
regularUser := authenticationv1.UserInfo{
7171
Username: "regular-user",
72-
Groups: []string{SystemMastersGroup, "system:authenticated"},
72+
Groups: []string{utils.SystemMastersGroup, "system:authenticated"},
7373
}
7474

7575
// Deployment with no existing labels.

pkg/webhook/deployment/deployment_validating_webhook.go renamed to pkg/webhook/deployment/validating_webhook.go

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,9 @@ import (
2525
"context"
2626
"fmt"
2727
"net/http"
28-
"slices"
2928

3029
admissionv1 "k8s.io/api/admission/v1"
3130
appsv1 "k8s.io/api/apps/v1"
32-
authenticationv1 "k8s.io/api/authentication/v1"
3331
"k8s.io/apimachinery/pkg/types"
3432
"k8s.io/klog/v2"
3533
"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -41,12 +39,6 @@ import (
4139

4240
const (
4341
deniedReconcileLabelFmt = "the %s label is reserved for aksService and cannot be set by user %q"
44-
45-
// AKSServiceUserName is the username whose deployment requests should be mutated.
46-
AKSServiceUserName = "aksService"
47-
48-
// SystemMastersGroup is the Kubernetes group representing cluster administrators.
49-
SystemMastersGroup = "system:masters"
5042
)
5143

5244
// ValidationPath is the webhook service path for validating Deployment resources.
@@ -91,7 +83,7 @@ func (v *deploymentValidator) Handle(_ context.Context, req admission.Request) a
9183

9284
// The reconcile label is present — only aksService with system:masters is
9385
// allowed to set it.
94-
if isAKSService(req.UserInfo) {
86+
if utils.IsAKSService(req.UserInfo) {
9587
klog.V(2).InfoS("aksService user allowed to set reconcile label",
9688
"namespacedName", namespacedName)
9789
return admission.Allowed("aksService user is allowed to set the reconcile label")
@@ -101,9 +93,3 @@ func (v *deploymentValidator) Handle(_ context.Context, req admission.Request) a
10193
"user", req.UserInfo.Username, "groups", req.UserInfo.Groups, "namespacedName", namespacedName)
10294
return admission.Denied(fmt.Sprintf(deniedReconcileLabelFmt, utils.ReconcileLabelKey, req.UserInfo.Username))
10395
}
104-
105-
// isAKSService reports whether the user is the aksService user with
106-
// system:masters group membership.
107-
func isAKSService(userInfo authenticationv1.UserInfo) bool {
108-
return userInfo.Username == AKSServiceUserName && slices.Contains(userInfo.Groups, SystemMastersGroup)
109-
}

pkg/webhook/deployment/deployment_validating_webhook_test.go renamed to pkg/webhook/deployment/validating_webhook_test.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,12 @@ func TestValidatingHandle(t *testing.T) {
5151
decoder := admission.NewDecoder(scheme)
5252

5353
aksServiceUser := authenticationv1.UserInfo{
54-
Username: AKSServiceUserName,
55-
Groups: []string{SystemMastersGroup},
54+
Username: utils.AKSServiceUserName,
55+
Groups: []string{utils.SystemMastersGroup},
5656
}
5757

5858
aksServiceUserNoMasters := authenticationv1.UserInfo{
59-
Username: AKSServiceUserName,
59+
Username: utils.AKSServiceUserName,
6060
Groups: []string{"system:authenticated"},
6161
}
6262

@@ -67,7 +67,7 @@ func TestValidatingHandle(t *testing.T) {
6767

6868
regularUserWithMasters := authenticationv1.UserInfo{
6969
Username: "regular-user",
70-
Groups: []string{SystemMastersGroup, "system:authenticated"},
70+
Groups: []string{utils.SystemMastersGroup, "system:authenticated"},
7171
}
7272

7373
// Deployment without the reconcile label.
@@ -357,51 +357,51 @@ func TestIsAKSService(t *testing.T) {
357357
}{
358358
"aksService with system:masters": {
359359
userInfo: authenticationv1.UserInfo{
360-
Username: AKSServiceUserName,
361-
Groups: []string{SystemMastersGroup},
360+
Username: utils.AKSServiceUserName,
361+
Groups: []string{utils.SystemMastersGroup},
362362
},
363363
want: true,
364364
},
365365
"aksService with system:masters and other groups": {
366366
userInfo: authenticationv1.UserInfo{
367-
Username: AKSServiceUserName,
368-
Groups: []string{"system:authenticated", SystemMastersGroup, "custom-group"},
367+
Username: utils.AKSServiceUserName,
368+
Groups: []string{"system:authenticated", utils.SystemMastersGroup, "custom-group"},
369369
},
370370
want: true,
371371
},
372372
"aksService without system:masters": {
373373
userInfo: authenticationv1.UserInfo{
374-
Username: AKSServiceUserName,
374+
Username: utils.AKSServiceUserName,
375375
Groups: []string{"system:authenticated"},
376376
},
377377
want: false,
378378
},
379379
"aksService with empty groups": {
380380
userInfo: authenticationv1.UserInfo{
381-
Username: AKSServiceUserName,
381+
Username: utils.AKSServiceUserName,
382382
Groups: []string{},
383383
},
384384
want: false,
385385
},
386386
"regular user with system:masters": {
387387
userInfo: authenticationv1.UserInfo{
388388
Username: "regular-user",
389-
Groups: []string{SystemMastersGroup},
389+
Groups: []string{utils.SystemMastersGroup},
390390
},
391391
want: false,
392392
},
393393
"empty username with system:masters": {
394394
userInfo: authenticationv1.UserInfo{
395395
Username: "",
396-
Groups: []string{SystemMastersGroup},
396+
Groups: []string{utils.SystemMastersGroup},
397397
},
398398
want: false,
399399
},
400400
}
401401

402402
for testName, tc := range testCases {
403403
t.Run(testName, func(t *testing.T) {
404-
got := isAKSService(tc.userInfo)
404+
got := utils.IsAKSService(tc.userInfo)
405405
if got != tc.want {
406406
t.Errorf("isAKSService(%+v) = %v, want %v", tc.userInfo, got, tc.want)
407407
}

0 commit comments

Comments
 (0)