diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go index ec491e17279..e4d9156a07e 100644 --- a/pkg/features/versioned_kube_features.go +++ b/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: { diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index 50ac6930b2e..0c951076b32 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/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: { diff --git a/test/featuregates_linter/test_data/versioned_feature_list.yaml b/test/featuregates_linter/test_data/versioned_feature_list.yaml index ba9819c01f1..35567e7b2a4 100644 --- a/test/featuregates_linter/test_data/versioned_feature_list.yaml +++ b/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 diff --git a/test/integration/apiserver/admissionwebhook/match_conditions_test.go b/test/integration/apiserver/admissionwebhook/match_conditions_test.go index fc5d0c7f9ad..f35076dd59b 100644 --- a/test/integration/apiserver/admissionwebhook/match_conditions_test.go +++ b/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: diff --git a/test/integration/apiserver/cel/validatingadmissionpolicy_test.go b/test/integration/apiserver/cel/validatingadmissionpolicy_test.go index 6231c06c533..bf618da56d3 100644 --- a/test/integration/apiserver/cel/validatingadmissionpolicy_test.go +++ b/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",