Skip to content

Commit 837858e

Browse files
committed
NE-1953: Add Validating Admission Policy for Gateway API CRDs
This commit introduces a validating admission policy that restricts modifications to CRDs from the "gateway.networking.k8s.io" group, allowing only the cluster ingress operator's service account to make changes. This ensures that the core OpenShift retains exclusive ownership of the Gateway API implementation, preventing modifications from any add-ons (whether third-party or Red Hat-owned). The policy and its corresponding binding will be managed by CVO.
1 parent e80fa46 commit 837858e

File tree

4 files changed

+352
-0
lines changed

4 files changed

+352
-0
lines changed
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
apiVersion: admissionregistration.k8s.io/v1
2+
kind: ValidatingAdmissionPolicyBinding
3+
metadata:
4+
name: openshift-ingress-operator-gatewayapi-crd-admission
5+
annotations:
6+
capability.openshift.io/name: Ingress
7+
include.release.openshift.io/ibm-cloud-managed: "true"
8+
include.release.openshift.io/self-managed-high-availability: "true"
9+
include.release.openshift.io/single-node-developer: "true"
10+
release.openshift.io/feature-set: "TechPreviewNoUpgrade,CustomNoUpgrade"
11+
spec:
12+
policyName: openshift-ingress-operator-gatewayapi-crd-admission
13+
validationActions: [Deny]
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
apiVersion: admissionregistration.k8s.io/v1
2+
kind: ValidatingAdmissionPolicy
3+
metadata:
4+
name: openshift-ingress-operator-gatewayapi-crd-admission
5+
annotations:
6+
capability.openshift.io/name: Ingress
7+
include.release.openshift.io/ibm-cloud-managed: "true"
8+
include.release.openshift.io/self-managed-high-availability: "true"
9+
include.release.openshift.io/single-node-developer: "true"
10+
release.openshift.io/feature-set: "TechPreviewNoUpgrade,CustomNoUpgrade"
11+
spec:
12+
matchConstraints:
13+
# Consider only requests to CRD resources.
14+
resourceRules:
15+
- apiGroups: ["apiextensions.k8s.io"]
16+
apiVersions: ["v1"]
17+
operations: ["CREATE","UPDATE","DELETE"]
18+
resources: ["customresourcedefinitions"]
19+
matchConditions:
20+
# Consider only request to Gateway API CRDs.
21+
- name: "check-only-gateway-api-crds"
22+
# When the operation is DELETE, the "object" variable is null.
23+
expression: "(request.operation == 'DELETE' ? oldObject : object).spec.group == 'gateway.networking.k8s.io'"
24+
# Validations are evaluated in the the order of their declaration.
25+
validations:
26+
# Verify that the request was sent by the ingress operator's service account.
27+
- expression: "has(request.userInfo.username) && (request.userInfo.username == 'system:serviceaccount:openshift-ingress-operator:ingress-operator')"
28+
message: "Gateway API Custom Resource Definitions are managed by the Ingress Operator and may not be modified"
29+
reason: Forbidden
30+
# Verify that the request was sent from a pod. The presence of both "node" and "pod" claims implies that the token is bound to a pod object.
31+
# Ref: https://kubernetes.io/docs/reference/access-authn-authz/service-accounts-admin/#schema-for-service-account-private-claims.
32+
- expression: "has(request.userInfo.extra) && ('authentication.kubernetes.io/node-name' in request.userInfo.extra) && ('authentication.kubernetes.io/pod-name' in request.userInfo.extra)"
33+
message: "this user must have a \"authentication.kubernetes.io/node-name\" and \"authentication.kubernetes.io/pod-name\" claims"
34+
reason: Forbidden
35+
# Fail the admission if any validation evaluates to false.
36+
failurePolicy: Fail

test/e2e/gateway_api_test.go

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,20 @@ import (
1010
"testing"
1111

1212
"github.com/openshift/api/features"
13+
operatorclient "github.com/openshift/cluster-ingress-operator/pkg/operator/client"
1314
operatorcontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller"
1415
"github.com/openshift/cluster-ingress-operator/pkg/operator/controller/gatewayclass"
1516

1617
corev1 "k8s.io/api/core/v1"
18+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1719
"k8s.io/apimachinery/pkg/api/errors"
1820
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
21+
"k8s.io/apimachinery/pkg/types"
1922
"k8s.io/apiserver/pkg/storage/names"
23+
"k8s.io/client-go/rest"
2024

25+
"sigs.k8s.io/controller-runtime/pkg/client"
26+
"sigs.k8s.io/controller-runtime/pkg/client/config"
2127
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
2228
)
2329

@@ -30,6 +36,8 @@ const (
3036
expectedCatalogSourceNamespace = "openshift-marketplace"
3137
// The test gateway name used in multiple places.
3238
testGatewayName = "test-gateway"
39+
// gwapiCRDVAPName is the name of the ingress operator's Validating Admission Policy (VAP).
40+
gwapiCRDVAPName = "openshift-ingress-operator-gatewayapi-crd-admission"
3341
)
3442

3543
var crdNames = []string{
@@ -84,6 +92,7 @@ func TestGatewayAPI(t *testing.T) {
8492
} else {
8593
t.Log("Gateway API Controller not enabled, skipping testGatewayAPIObjects and testGatewayAPIIstioInstallation")
8694
}
95+
t.Run("testGatewayAPIResourcesProtection", testGatewayAPIResourcesProtection)
8796
}
8897

8998
// testGatewayAPIResources tests that Gateway API Custom Resource Definitions are available.
@@ -167,6 +176,93 @@ func testGatewayAPIObjects(t *testing.T) {
167176
}
168177
}
169178

179+
// testGatewayAPIResourcesProtection verifies that the ingress operator's Validating Admission Policy
180+
// denies admission requests attempting to modify Gateway API CRDs on behalf of a user
181+
// who is not the ingress operator's service account.
182+
func testGatewayAPIResourcesProtection(t *testing.T) {
183+
t.Helper()
184+
185+
// Get kube client which impersonates ingress operator's service account.
186+
kubeConfig, err := config.GetConfig()
187+
if err != nil {
188+
t.Fatalf("failed to get kube config: %v", err)
189+
}
190+
kubeConfig.Impersonate = rest.ImpersonationConfig{
191+
UserName: "system:serviceaccount:openshift-ingress-operator:ingress-operator",
192+
}
193+
kubeClient, err := operatorclient.NewClient(kubeConfig)
194+
if err != nil {
195+
t.Fatalf("failed to to create kube client: %v", err)
196+
}
197+
198+
// Create test CRDs.
199+
var testCRDs []*apiextensionsv1.CustomResourceDefinition
200+
for _, name := range crdNames {
201+
testCRDs = append(testCRDs, buildGWAPICRDFromName(name))
202+
}
203+
204+
testCases := []struct {
205+
name string
206+
kclient client.Client
207+
expectedErrMsg string
208+
}{
209+
{
210+
name: "Ingress operator service account required",
211+
kclient: kclient,
212+
expectedErrMsg: "Gateway API Custom Resource Definitions are managed by the Ingress Operator and may not be modified",
213+
},
214+
{
215+
name: "Pod binding required",
216+
kclient: kubeClient,
217+
expectedErrMsg: "this user must have a \"authentication.kubernetes.io/node-name\" and \"authentication.kubernetes.io/pod-name\" claims",
218+
},
219+
}
220+
221+
for _, tc := range testCases {
222+
t.Run(tc.name, func(t *testing.T) {
223+
// Verify that GatewayAPI CRD creation is forbidden.
224+
for i := range testCRDs {
225+
if err := tc.kclient.Create(context.Background(), testCRDs[i]); err != nil {
226+
if !strings.Contains(err.Error(), tc.expectedErrMsg) {
227+
t.Errorf("unexpected error received while creating CRD %q: %v", testCRDs[i].Name, err)
228+
}
229+
} else {
230+
t.Errorf("admission error is expected while creating CRD %q but not received", testCRDs[i].Name)
231+
}
232+
}
233+
234+
// Verify that GatewayAPI CRD update is forbidden.
235+
for i := range testCRDs {
236+
crdName := types.NamespacedName{Name: testCRDs[i].Name}
237+
crd := &apiextensionsv1.CustomResourceDefinition{}
238+
if err := tc.kclient.Get(context.Background(), crdName, crd); err != nil {
239+
t.Errorf("failed to get %q CRD: %v", crdName.Name, err)
240+
continue
241+
}
242+
crd.Spec = testCRDs[i].Spec
243+
if err := tc.kclient.Update(context.Background(), crd); err != nil {
244+
if !strings.Contains(err.Error(), tc.expectedErrMsg) {
245+
t.Errorf("unexpected error received while updating CRD %q: %v", testCRDs[i].Name, err)
246+
}
247+
} else {
248+
t.Errorf("admission error is expected while updating CRD %q but not received", testCRDs[i].Name)
249+
}
250+
}
251+
252+
// Verify that GatewayAPI CRD deletion is forbidden.
253+
for i := range testCRDs {
254+
if err := tc.kclient.Delete(context.Background(), testCRDs[i]); err != nil {
255+
if !strings.Contains(err.Error(), tc.expectedErrMsg) {
256+
t.Errorf("unexpected error received while deleting CRD %q: %v", testCRDs[i].Name, err)
257+
}
258+
} else {
259+
t.Errorf("admission error is expected while deleting CRD %q but not received", testCRDs[i].Name)
260+
}
261+
}
262+
})
263+
}
264+
}
265+
170266
// ensureCRDs tests that the Gateway API custom resource definitions exist.
171267
func ensureCRDs(t *testing.T) {
172268
t.Helper()
@@ -182,6 +278,19 @@ func ensureCRDs(t *testing.T) {
182278
// deleteCRDs deletes Gateway API custom resource definitions.
183279
func deleteCRDs(t *testing.T) {
184280
t.Helper()
281+
282+
vm := newVAPManager(t, gwapiCRDVAPName)
283+
// Remove the ingress operator's Validating Admission Policy (VAP)
284+
// which prevents modifications of Gateway API CRDs
285+
// by anything other than the ingress operator.
286+
if err, recoverFn := vm.disable(); err != nil {
287+
defer recoverFn()
288+
t.Fatalf("failed to disable vap: %v", err)
289+
}
290+
// Put back the VAP to ensure that it does not prevent
291+
// the ingress operator from managing Gateway API CRDs.
292+
defer vm.enable()
293+
185294
for _, crdName := range crdNames {
186295
err := deleteExistingCRD(t, crdName)
187296
if err != nil {

0 commit comments

Comments
 (0)