Merge pull request #127302 from cici37/costFG

Promote cost related feature gate to default true

pkg/features/versioned_kube_features.go

@@ -296,10 +296,12 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 
 	genericfeatures.StrictCostEnforcementForVAP: {
 		{Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Beta},
+		{Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 	},
 
 	genericfeatures.StrictCostEnforcementForWebhooks: {
 		{Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Beta},
+		{Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 	},
 
 	genericfeatures.StructuredAuthenticationConfiguration: {

staging/src/k8s.io/apiserver/pkg/features/kube_features.go

@@ -408,10 +408,12 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 
 	StrictCostEnforcementForVAP: {
 		{Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Beta},
+		{Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 	},
 
 	StrictCostEnforcementForWebhooks: {
 		{Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Beta},
+		{Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 	},
 
 	StructuredAuthenticationConfiguration: {

test/featuregates_linter/test_data/versioned_feature_list.yaml

@@ -1146,12 +1146,20 @@
     lockToDefault: false
     preRelease: Beta
     version: "1.30"
+  - default: true
+    lockToDefault: true
+    preRelease: GA
+    version: "1.32"
 - name: StrictCostEnforcementForWebhooks
   versionedSpecs:
   - default: false
     lockToDefault: false
     preRelease: Beta
     version: "1.30"
+  - default: true
+    lockToDefault: true
+    preRelease: GA
+    version: "1.32"
 - name: StructuredAuthenticationConfiguration
   versionedSpecs:
   - default: false

test/integration/apiserver/admissionwebhook/match_conditions_test.go

@@ -22,6 +22,7 @@ import (
 	"crypto/x509"
 	"encoding/json"
 	"io"
+	"k8s.io/apimachinery/pkg/util/version"
 	"net/http"
 	"net/http/httptest"
 	"strconv"
@@ -111,7 +112,6 @@ func newMatchConditionHandler(recorder *admissionRecorder) http.Handler {
 
 // TestMatchConditions tests ValidatingWebhookConfigurations and MutatingWebhookConfigurations that validates different cases of matchCondition fields
 func TestMatchConditions(t *testing.T) {
-	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.StrictCostEnforcementForWebhooks, false)
 	fail := admissionregistrationv1.Fail
 	ignore := admissionregistrationv1.Ignore
 
@@ -289,35 +289,6 @@ func TestMatchConditions(t *testing.T) {
 				matchConditionsTestPod("test2", "default"),
 			},
 		},
-		{
-			name: "without strict cost enforcement: Authz check does not exceed per call limit",
-			matchConditions: []admissionregistrationv1.MatchCondition{
-				{
-					Name:       "test1",
-					Expression: "authorizer.group('').resource('pods').name('test1').check('create').allowed() && authorizer.group('').resource('pods').name('test1').check('create').allowed() && authorizer.group('').resource('pods').name('test1').check('create').allowed()",
-				},
-			},
-			pods: []*corev1.Pod{
-				matchConditionsTestPod("test1", "kube-system"),
-			},
-			matchedPods: []*corev1.Pod{
-				matchConditionsTestPod("test1", "kube-system"),
-			},
-			failPolicy:     &fail,
-			expectErrorPod: false,
-		},
-		{
-			name:            "without strict cost enforcement: Authz check does not exceed overall cost limit",
-			matchConditions: generateMatchConditionsWithAuthzCheck(8, "authorizer.group('').resource('pods').name('test1').check('create').allowed() && authorizer.group('').resource('pods').name('test1').check('create').allowed()"),
-			pods: []*corev1.Pod{
-				matchConditionsTestPod("test1", "kube-system"),
-			},
-			matchedPods: []*corev1.Pod{
-				matchConditionsTestPod("test1", "kube-system"),
-			},
-			failPolicy:     &fail,
-			expectErrorPod: false,
-		},
 	}
 
 	roots := x509.NewCertPool()
@@ -447,7 +418,325 @@ func TestMatchConditions(t *testing.T) {
 			}()
 
 			// wait until new webhook is called the first time
-			if err := wait.PollImmediate(time.Millisecond*5, wait.ForeverTestTimeout, func() (bool, error) {
+			if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*5, wait.ForeverTestTimeout, true, func(_ context.Context) (bool, error) {
+				_, err = client.CoreV1().Pods(markerNs).Patch(context.TODO(), marker.Name, types.JSONPatchType, []byte("[]"), metav1.PatchOptions{})
+				select {
+				case <-upCh:
+					return true, nil
+				default:
+					t.Logf("Waiting for webhook to become effective, getting marker object: %v", err)
+					return false, nil
+				}
+			}); err != nil {
+				t.Fatal(err)
+			}
+
+			for _, pod := range testcase.pods {
+				_, err := client.CoreV1().Pods(pod.Namespace).Create(context.TODO(), pod, dryRunCreate)
+				if !testcase.expectErrorPod && err != nil {
+					t.Fatalf("unexpected error creating test pod: %v", err)
+				} else if testcase.expectErrorPod && err == nil {
+					t.Fatal("expected error creating pods")
+				} else if testcase.expectErrorPod && err != nil && !strings.Contains(err.Error(), testcase.errMessage) {
+					t.Fatalf("expected error message includes: %v, but get %v", testcase.errMessage, err)
+				}
+			}
+
+			if len(recorder.requests) != len(testcase.matchedPods) {
+				t.Errorf("unexpected requests %v, expected %v", recorder.requests, testcase.matchedPods)
+			}
+
+			for i, request := range recorder.requests {
+				if request.Name != testcase.matchedPods[i].Name {
+					t.Errorf("unexpected pod name %v, expected %v", request.Name, testcase.matchedPods[i].Name)
+				}
+				if request.Namespace != testcase.matchedPods[i].Namespace {
+					t.Errorf("unexpected pod namespace %v, expected %v", request.Namespace, testcase.matchedPods[i].Namespace)
+				}
+			}
+
+			// Reset and rerun against mutating webhook configuration
+			// TODO: private helper function for validation after creating vwh or mwh
+			upCh = recorder.Reset()
+			err = client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(context.TODO(), validatingcfg.GetName(), metav1.DeleteOptions{})
+			if err != nil {
+				t.Fatal(err)
+			} else {
+				vhwHasBeenCleanedUp = true
+			}
+
+			mutatingwebhook := &admissionregistrationv1.MutatingWebhookConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "admission.integration.test",
+				},
+				Webhooks: []admissionregistrationv1.MutatingWebhook{
+					{
+						Name: "admission.integration.test",
+						Rules: []admissionregistrationv1.RuleWithOperations{{
+							Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.Create},
+							Rule: admissionregistrationv1.Rule{
+								APIGroups:   []string{""},
+								APIVersions: []string{"v1"},
+								Resources:   []string{"pods"},
+							},
+						}},
+						ClientConfig: admissionregistrationv1.WebhookClientConfig{
+							URL:      &endpoint,
+							CABundle: localhostCert,
+						},
+						// ignore pods in the marker namespace
+						NamespaceSelector: &metav1.LabelSelector{
+							MatchExpressions: []metav1.LabelSelectorRequirement{
+								{
+									Key:      corev1.LabelMetadataName,
+									Operator: metav1.LabelSelectorOpNotIn,
+									Values:   []string{"marker"},
+								},
+							}},
+						FailurePolicy:           testcase.failPolicy,
+						SideEffects:             &noSideEffects,
+						AdmissionReviewVersions: []string{"v1"},
+						MatchConditions:         testcase.matchConditions,
+					},
+					{
+						Name: "admission.integration.test.marker",
+						Rules: []admissionregistrationv1.RuleWithOperations{{
+							Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll},
+							Rule:       admissionregistrationv1.Rule{APIGroups: []string{""}, APIVersions: []string{"v1"}, Resources: []string{"pods"}},
+						}},
+						ClientConfig: admissionregistrationv1.WebhookClientConfig{
+							URL:      &markerEndpoint,
+							CABundle: localhostCert,
+						},
+						NamespaceSelector: &metav1.LabelSelector{MatchLabels: map[string]string{
+							corev1.LabelMetadataName: "marker",
+						}},
+						ObjectSelector:          &metav1.LabelSelector{MatchLabels: map[string]string{"marker": "true"}},
+						FailurePolicy:           testcase.failPolicy,
+						SideEffects:             &noSideEffects,
+						AdmissionReviewVersions: []string{"v1"},
+					},
+				},
+			}
+
+			mutatingcfg, err := client.AdmissionregistrationV1().MutatingWebhookConfigurations().Create(context.TODO(), mutatingwebhook, metav1.CreateOptions{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer func() {
+				err := client.AdmissionregistrationV1().MutatingWebhookConfigurations().Delete(context.TODO(), mutatingcfg.GetName(), metav1.DeleteOptions{})
+				if err != nil {
+					t.Fatal(err)
+				}
+			}()
+
+			// wait until new webhook is called the first time
+			if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*5, wait.ForeverTestTimeout, true, func(_ context.Context) (bool, error) {
+				_, err = client.CoreV1().Pods(markerNs).Patch(context.TODO(), marker.Name, types.JSONPatchType, []byte("[]"), metav1.PatchOptions{})
+				select {
+				case <-upCh:
+					return true, nil
+				default:
+					t.Logf("Waiting for webhook to become effective, getting marker object: %v", err)
+					return false, nil
+				}
+			}); err != nil {
+				t.Fatal(err)
+			}
+
+			for _, pod := range testcase.pods {
+				_, err = client.CoreV1().Pods(pod.Namespace).Create(context.TODO(), pod, dryRunCreate)
+				if testcase.expectErrorPod == false && err != nil {
+					t.Fatalf("unexpected error creating test pod: %v", err)
+				} else if testcase.expectErrorPod == true && err == nil {
+					t.Fatal("expected error creating pods")
+				}
+			}
+
+			if len(recorder.requests) != len(testcase.matchedPods) {
+				t.Errorf("unexpected requests %v, expected %v", recorder.requests, testcase.matchedPods)
+			}
+
+			for i, request := range recorder.requests {
+				if request.Name != testcase.matchedPods[i].Name {
+					t.Errorf("unexpected pod name %v, expected %v", request.Name, testcase.matchedPods[i].Name)
+				}
+				if request.Namespace != testcase.matchedPods[i].Namespace {
+					t.Errorf("unexpected pod namespace %v, expected %v", request.Namespace, testcase.matchedPods[i].Namespace)
+				}
+			}
+		})
+	}
+}
+
+func TestMatchConditionsWithoutStrictCostEnforcement(t *testing.T) {
+	testcases := []struct {
+		name            string
+		matchConditions []admissionregistrationv1.MatchCondition
+		pods            []*corev1.Pod
+		matchedPods     []*corev1.Pod
+		expectErrorPod  bool
+		failPolicy      *admissionregistrationv1.FailurePolicyType
+		errMessage      string
+	}{
+		{
+			name: "without strict cost enforcement: Authz check does not exceed per call limit",
+			matchConditions: []admissionregistrationv1.MatchCondition{
+				{
+					Name:       "test1",
+					Expression: "authorizer.group('').resource('pods').name('test1').check('create').allowed() && authorizer.group('').resource('pods').name('test1').check('create').allowed() && authorizer.group('').resource('pods').name('test1').check('create').allowed()",
+				},
+			},
+			pods: []*corev1.Pod{
+				matchConditionsTestPod("test1", "kube-system"),
+			},
+			matchedPods: []*corev1.Pod{
+				matchConditionsTestPod("test1", "kube-system"),
+			},
+			expectErrorPod: false,
+		},
+		{
+			name:            "without strict cost enforcement: Authz check does not exceed overall cost limit",
+			matchConditions: generateMatchConditionsWithAuthzCheck(8, "authorizer.group('').resource('pods').name('test1').check('create').allowed() && authorizer.group('').resource('pods').name('test1').check('create').allowed()"),
+			pods: []*corev1.Pod{
+				matchConditionsTestPod("test1", "kube-system"),
+			},
+			matchedPods: []*corev1.Pod{
+				matchConditionsTestPod("test1", "kube-system"),
+			},
+			expectErrorPod: false,
+		},
+	}
+
+	roots := x509.NewCertPool()
+	if !roots.AppendCertsFromPEM(localhostCert) {
+		t.Fatal("Failed to append Cert from PEM")
+	}
+	cert, err := tls.X509KeyPair(localhostCert, localhostKey)
+	if err != nil {
+		t.Fatalf("Failed to build cert with error: %+v", err)
+	}
+
+	recorder := &admissionRecorder{requests: []*admissionv1.AdmissionRequest{}}
+
+	webhookServer := httptest.NewUnstartedServer(newMatchConditionHandler(recorder))
+	webhookServer.TLS = &tls.Config{
+		RootCAs:      roots,
+		Certificates: []tls.Certificate{cert},
+	}
+	webhookServer.StartTLS()
+	defer webhookServer.Close()
+
+	dryRunCreate := metav1.CreateOptions{
+		DryRun: []string{metav1.DryRunAll},
+	}
+
+	for _, testcase := range testcases {
+		t.Run(testcase.name, func(t *testing.T) {
+			upCh := recorder.Reset()
+			featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, version.MustParse("1.31"))
+			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.StrictCostEnforcementForWebhooks, false)
+			server, err := apiservertesting.StartTestServer(t, &apiservertesting.TestServerInstanceOptions{EmulationVersion: "1.31"}, []string{
+				"--disable-admission-plugins=ServiceAccount",
+			}, framework.SharedEtcd())
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer server.TearDownFn()
+
+			config := server.ClientConfig
+
+			client, err := clientset.NewForConfig(config)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			// Write markers to a separate namespace to avoid cross-talk
+			markerNs := "marker"
+			_, err = client.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: markerNs}}, metav1.CreateOptions{})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			// Create a marker object to use to check for the webhook configurations to be ready.
+			marker, err := client.CoreV1().Pods(markerNs).Create(context.TODO(), newMarkerPod(markerNs), metav1.CreateOptions{})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			endpoint := webhookServer.URL
+			markerEndpoint := webhookServer.URL + "/marker"
+			validatingwebhook := &admissionregistrationv1.ValidatingWebhookConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "admission.integration.test",
+				},
+				Webhooks: []admissionregistrationv1.ValidatingWebhook{
+					{
+						Name: "admission.integration.test",
+						Rules: []admissionregistrationv1.RuleWithOperations{{
+							Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.Create},
+							Rule: admissionregistrationv1.Rule{
+								APIGroups:   []string{""},
+								APIVersions: []string{"v1"},
+								Resources:   []string{"pods"},
+							},
+						}},
+						ClientConfig: admissionregistrationv1.WebhookClientConfig{
+							URL:      &endpoint,
+							CABundle: localhostCert,
+						},
+						// ignore pods in the marker namespace
+						NamespaceSelector: &metav1.LabelSelector{
+							MatchExpressions: []metav1.LabelSelectorRequirement{
+								{
+									Key:      corev1.LabelMetadataName,
+									Operator: metav1.LabelSelectorOpNotIn,
+									Values:   []string{"marker"},
+								},
+							}},
+						FailurePolicy:           testcase.failPolicy,
+						SideEffects:             &noSideEffects,
+						AdmissionReviewVersions: []string{"v1"},
+						MatchConditions:         testcase.matchConditions,
+					},
+					{
+						Name: "admission.integration.test.marker",
+						Rules: []admissionregistrationv1.RuleWithOperations{{
+							Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll},
+							Rule:       admissionregistrationv1.Rule{APIGroups: []string{""}, APIVersions: []string{"v1"}, Resources: []string{"pods"}},
+						}},
+						ClientConfig: admissionregistrationv1.WebhookClientConfig{
+							URL:      &markerEndpoint,
+							CABundle: localhostCert,
+						},
+						NamespaceSelector: &metav1.LabelSelector{MatchLabels: map[string]string{
+							corev1.LabelMetadataName: "marker",
+						}},
+						ObjectSelector:          &metav1.LabelSelector{MatchLabels: map[string]string{"marker": "true"}},
+						FailurePolicy:           testcase.failPolicy,
+						SideEffects:             &noSideEffects,
+						AdmissionReviewVersions: []string{"v1"},
+					},
+				},
+			}
+
+			validatingcfg, err := client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Create(context.TODO(), validatingwebhook, metav1.CreateOptions{})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			vhwHasBeenCleanedUp := false
+			defer func() {
+				if !vhwHasBeenCleanedUp {
+					err := client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(context.TODO(), validatingcfg.GetName(), metav1.DeleteOptions{})
+					if err != nil {
+						t.Fatal(err)
+					}
+				}
+			}()
+
+			// wait until new webhook is called the first time
+			if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*5, wait.ForeverTestTimeout, true, func(_ context.Context) (bool, error) {
 				_, err = client.CoreV1().Pods(markerNs).Patch(context.TODO(), marker.Name, types.JSONPatchType, []byte("[]"), metav1.PatchOptions{})
 				select {
 				case <-upCh:
@@ -876,7 +1165,7 @@ func TestMatchConditionsWithStrictCostEnforcement(t *testing.T) {
 			}()
 
 			// wait until new webhook is called the first time
-			if err := wait.PollImmediate(time.Millisecond*5, wait.ForeverTestTimeout, func() (bool, error) {
+			if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*5, wait.ForeverTestTimeout, true, func(_ context.Context) (bool, error) {
 				_, err = client.CoreV1().Pods(markerNs).Patch(context.TODO(), marker.Name, types.JSONPatchType, []byte("[]"), metav1.PatchOptions{})
 				select {
 				case <-upCh:

test/integration/apiserver/cel/validatingadmissionpolicy_test.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"k8s.io/apimachinery/pkg/util/version"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -2234,6 +2235,7 @@ func Test_CostLimitForValidation(t *testing.T) {
 func Test_CostLimitForValidationWithFeatureDisabled(t *testing.T) {
 	resetPolicyRefreshInterval := generic.SetPolicyRefreshIntervalForTests(policyRefreshInterval)
 	defer resetPolicyRefreshInterval()
+	featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, version.MustParse("1.31"))
 	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.StrictCostEnforcementForVAP, false)
 	server, err := apiservertesting.StartTestServer(t, &apiservertesting.TestServerInstanceOptions{EmulationVersion: "1.31"}, []string{
 		"--enable-admission-plugins", "ValidatingAdmissionPolicy",