Skip to content

Commit 9c5672f

Browse files
jwttyjwtty
committed
resolve comments
1 parent 5f419c1 commit 9c5672f

4 files changed

Lines changed: 120 additions & 17 deletions

File tree

‎pkg/webhook/pod/pod_validating_webhook.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (v *podValidator) Handle(_ context.Context, req admission.Request) admissio
6565
return admission.Errored(http.StatusBadRequest, err)
6666
}
6767
if !utils.IsReservedNamespace(pod.Namespace) && !utils.HasReconcileLabel(pod.Labels) {
68-
klog.V(2).InfoS(deniedPodResource, "user", req.UserInfo.Username, "groups", req.UserInfo.Groups, "operation", req.Operation, "GVK", req.RequestKind, "subResource", req.SubResource, "namespacedName", namespacedName)
68+
klog.V(2).InfoS(deniedPodResource, "user", req.UserInfo.Username, "groups", req.UserInfo.Groups, "operation", req.Operation, "GVK", req.RequestKind, "subResource", req.SubResource, "namespacedName", namespacedName, "hasReconcileLabel", utils.HasReconcileLabel(pod.Labels))
6969
return admission.Denied(fmt.Sprintf(podDeniedFormat, pod.Namespace, pod.Name))
7070
}
7171
}

‎pkg/webhook/replicaset/replicaset_validating_webhook.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func (v *replicaSetValidator) Handle(_ context.Context, req admission.Request) a
6464
return admission.Errored(http.StatusBadRequest, err)
6565
}
6666
if !utils.IsReservedNamespace(rs.Namespace) && !utils.HasReconcileLabel(rs.Labels) {
67-
klog.V(2).InfoS(deniedReplicaSetResource, "user", req.UserInfo.Username, "groups", req.UserInfo.Groups, "operation", req.Operation, "GVK", req.RequestKind, "subResource", req.SubResource, "namespacedName", namespacedName)
67+
klog.V(2).InfoS(deniedReplicaSetResource, "user", req.UserInfo.Username, "groups", req.UserInfo.Groups, "operation", req.Operation, "GVK", req.RequestKind, "subResource", req.SubResource, "namespacedName", namespacedName, "hasReconcileLabel", utils.HasReconcileLabel(rs.Labels))
6868
return admission.Denied(fmt.Sprintf(replicaSetDeniedFormat, rs.Namespace, rs.Name))
6969
}
7070
}

‎test/e2e/fleet_guard_rail_test.go‎

Lines changed: 115 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"k8s.io/apimachinery/pkg/util/intstr"
3636
utilrand "k8s.io/apimachinery/pkg/util/rand"
3737
"k8s.io/utils/ptr"
38+
"sigs.k8s.io/controller-runtime/pkg/client"
3839

3940
clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1"
4041
placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
@@ -1477,12 +1478,14 @@ var _ = Describe("fleet deployment webhook tests for validating and mutating dep
14771478
Expect(notMasterUser.Create(ctx, deploy)).Should(Succeed())
14781479

14791480
// Verify the reconcile label was NOT injected.
1480-
var d appsv1.Deployment
1481-
Expect(hubClient.Get(ctx, types.NamespacedName{Name: deploy.Name, Namespace: deploy.Namespace}, &d)).Should(Succeed())
1482-
Expect(d.Labels).ShouldNot(HaveKey(utils.ReconcileLabelKey),
1483-
"deployment metadata should NOT have reconcile label for regular user")
1484-
Expect(d.Spec.Template.Labels).ShouldNot(HaveKey(utils.ReconcileLabelKey),
1485-
"pod template should NOT have reconcile label for regular user")
1481+
Eventually(func(g Gomega) {
1482+
var d appsv1.Deployment
1483+
g.Expect(hubClient.Get(ctx, types.NamespacedName{Name: deploy.Name, Namespace: deploy.Namespace}, &d)).Should(Succeed())
1484+
g.Expect(d.Labels).ShouldNot(HaveKey(utils.ReconcileLabelKey),
1485+
"deployment metadata should NOT have reconcile label for regular user")
1486+
g.Expect(d.Spec.Template.Labels).ShouldNot(HaveKey(utils.ReconcileLabelKey),
1487+
"pod template should NOT have reconcile label for regular user")
1488+
}, eventuallyDuration, eventuallyInterval).Should(Succeed())
14861489
})
14871490
})
14881491

@@ -1495,12 +1498,112 @@ var _ = Describe("fleet deployment webhook tests for validating and mutating dep
14951498
Expect(sysMastersClient.Create(ctx, deploy)).Should(Succeed())
14961499

14971500
// Verify the reconcile label was NOT injected for reserved namespace.
1498-
var d appsv1.Deployment
1499-
Expect(hubClient.Get(ctx, types.NamespacedName{Name: deploy.Name, Namespace: deploy.Namespace}, &d)).Should(Succeed())
1500-
Expect(d.Labels).ShouldNot(HaveKey(utils.ReconcileLabelKey),
1501-
"deployment in kube-system should NOT have reconcile label auto-injected")
1502-
Expect(d.Spec.Template.Labels).ShouldNot(HaveKey(utils.ReconcileLabelKey),
1503-
"pod template in kube-system should NOT have reconcile label auto-injected")
1501+
Eventually(func(g Gomega) {
1502+
var d appsv1.Deployment
1503+
g.Expect(hubClient.Get(ctx, types.NamespacedName{Name: deploy.Name, Namespace: deploy.Namespace}, &d)).Should(Succeed())
1504+
g.Expect(d.Labels).ShouldNot(HaveKey(utils.ReconcileLabelKey),
1505+
"deployment in kube-system should NOT have reconcile label auto-injected")
1506+
g.Expect(d.Spec.Template.Labels).ShouldNot(HaveKey(utils.ReconcileLabelKey),
1507+
"pod template in kube-system should NOT have reconcile label auto-injected")
1508+
}, eventuallyDuration, eventuallyInterval).Should(Succeed())
1509+
})
1510+
})
1511+
1512+
Context("end-to-end deployment lifecycle with webhook enforcement", func() {
1513+
It("should block RS and Pod creation when non-aksService user creates a deployment", func() {
1514+
deployName := "test-deploy-e2e-noaks"
1515+
deploy := newDeployment(deployName, testNS, nil, nil)
1516+
DeferCleanup(func() {
1517+
_ = hubClient.Delete(ctx, deploy)
1518+
})
1519+
1520+
By("creating deployment as non-aksService user")
1521+
Expect(notMasterUser.Create(ctx, deploy)).Should(Succeed())
1522+
1523+
By("waiting for deployment to appear in the cache")
1524+
Eventually(func(g Gomega) {
1525+
var d appsv1.Deployment
1526+
g.Expect(hubClient.Get(ctx, types.NamespacedName{Name: deployName, Namespace: testNS}, &d)).Should(Succeed())
1527+
}, eventuallyDuration, eventuallyInterval).Should(Succeed())
1528+
1529+
By("verifying deployment exists but has no available replicas because RS creation is blocked")
1530+
Consistently(func(g Gomega) {
1531+
var d appsv1.Deployment
1532+
g.Expect(hubClient.Get(ctx, types.NamespacedName{Name: deployName, Namespace: testNS}, &d)).Should(Succeed())
1533+
g.Expect(d.Status.AvailableReplicas).Should(Equal(int32(0)),
1534+
"deployment should have 0 available replicas because RS/Pod creation is blocked by validating webhooks")
1535+
g.Expect(d.Status.ReadyReplicas).Should(Equal(int32(0)),
1536+
"deployment should have 0 ready replicas")
1537+
1538+
// Verify no ReplicaSets exist for this deployment.
1539+
var rsList appsv1.ReplicaSetList
1540+
g.Expect(hubClient.List(ctx, &rsList, client.InNamespace(testNS), client.MatchingLabels{"app": deployName})).Should(Succeed())
1541+
g.Expect(rsList.Items).Should(BeEmpty(),
1542+
"no ReplicaSets should exist because the RS validating webhook blocks creation without the reconcile label")
1543+
1544+
// Verify no Pods exist for this deployment.
1545+
var podList corev1.PodList
1546+
g.Expect(hubClient.List(ctx, &podList, client.InNamespace(testNS), client.MatchingLabels{"app": deployName})).Should(Succeed())
1547+
g.Expect(podList.Items).Should(BeEmpty(),
1548+
"no Pods should exist because the Pod validating webhook blocks creation without the reconcile label")
1549+
}, consistentlyDuration, consistentlyInterval).Should(Succeed())
1550+
})
1551+
1552+
It("should allow RS and Pod creation when aksService user creates a deployment", func() {
1553+
deployName := "test-deploy-e2e-aks"
1554+
deploy := newDeployment(deployName, testNS, nil, nil)
1555+
DeferCleanup(func() {
1556+
_ = hubClient.Delete(ctx, deploy)
1557+
})
1558+
1559+
By("creating deployment as aksService user (mutating webhook injects reconcile label)")
1560+
Expect(sysMastersClient.Create(ctx, deploy)).Should(Succeed())
1561+
1562+
By("verifying the mutating webhook injected the reconcile label")
1563+
Eventually(func(g Gomega) {
1564+
var d appsv1.Deployment
1565+
g.Expect(hubClient.Get(ctx, types.NamespacedName{Name: deployName, Namespace: testNS}, &d)).Should(Succeed())
1566+
g.Expect(d.Labels).Should(HaveKeyWithValue(utils.ReconcileLabelKey, utils.ReconcileLabelValue),
1567+
"deployment metadata should have reconcile label injected by mutating webhook")
1568+
g.Expect(d.Spec.Template.Labels).Should(HaveKeyWithValue(utils.ReconcileLabelKey, utils.ReconcileLabelValue),
1569+
"pod template should have reconcile label injected by mutating webhook")
1570+
}, eventuallyDuration, eventuallyInterval).Should(Succeed())
1571+
1572+
By("verifying ReplicaSet is created with the reconcile label")
1573+
Eventually(func(g Gomega) {
1574+
var rsList appsv1.ReplicaSetList
1575+
g.Expect(hubClient.List(ctx, &rsList, client.InNamespace(testNS), client.MatchingLabels{"app": deployName})).Should(Succeed())
1576+
g.Expect(rsList.Items).ShouldNot(BeEmpty(),
1577+
"at least one ReplicaSet should be created for the deployment")
1578+
1579+
rs := rsList.Items[0]
1580+
g.Expect(rs.Labels).Should(HaveKeyWithValue(utils.ReconcileLabelKey, utils.ReconcileLabelValue),
1581+
"ReplicaSet should inherit the reconcile label from the pod template")
1582+
}, workloadEventuallyDuration, eventuallyInterval).Should(Succeed())
1583+
1584+
By("verifying Pods are running with the reconcile label")
1585+
Eventually(func(g Gomega) {
1586+
var podList corev1.PodList
1587+
g.Expect(hubClient.List(ctx, &podList, client.InNamespace(testNS), client.MatchingLabels{"app": deployName})).Should(Succeed())
1588+
g.Expect(podList.Items).ShouldNot(BeEmpty(),
1589+
"at least one Pod should be created for the deployment")
1590+
1591+
pod := podList.Items[0]
1592+
g.Expect(pod.Labels).Should(HaveKeyWithValue(utils.ReconcileLabelKey, utils.ReconcileLabelValue),
1593+
"Pod should inherit the reconcile label from the pod template")
1594+
g.Expect(pod.Status.Phase).Should(Equal(corev1.PodRunning),
1595+
"Pod should be in Running phase")
1596+
}, workloadEventuallyDuration, eventuallyInterval).Should(Succeed())
1597+
1598+
By("verifying deployment reports available replicas")
1599+
Eventually(func(g Gomega) {
1600+
var d appsv1.Deployment
1601+
g.Expect(hubClient.Get(ctx, types.NamespacedName{Name: deployName, Namespace: testNS}, &d)).Should(Succeed())
1602+
g.Expect(d.Status.AvailableReplicas).Should(Equal(int32(1)),
1603+
"deployment should have 1 available replica")
1604+
g.Expect(d.Status.ReadyReplicas).Should(Equal(int32(1)),
1605+
"deployment should have 1 ready replica")
1606+
}, workloadEventuallyDuration, eventuallyInterval).Should(Succeed())
15041607
})
15051608
})
15061609
})

‎test/e2e/setup.sh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,11 @@ helm install hub-agent ../../charts/hub-agent/ \
148148
--set crdInstaller.image.pullPolicy=Never \
149149
--set namespace=fleet-system \
150150
--set logVerbosity=5 \
151-
--set replicaCount=3 \
152-
--set useCertManager=true \
151+
--set replicaCount=1 \
152+
--set useCertManager=false \
153153
--set webhookCertSecretName=fleet-webhook-server-cert \
154154
--set enableWebhook=true \
155-
--set enableWorkload=true \
155+
--set enableWorkload=false \
156156
--set webhookClientConnectionType=service \
157157
--set forceDeleteWaitTime="1m0s" \
158158
--set clusterUnhealthyThreshold="3m0s" \

0 commit comments

Comments
 (0)