Skip to content

Commit 1d36482

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 69cadab commit 1d36482

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{
@@ -75,6 +83,7 @@ func TestGatewayAPI(t *testing.T) {
7583
t.Run("testGatewayAPIResources", testGatewayAPIResources)
7684
t.Run("testGatewayAPIObjects", testGatewayAPIObjects)
7785
t.Run("testGatewayAPIIstioInstallation", testGatewayAPIIstioInstallation)
86+
t.Run("testGatewayAPIResourcesProtection", testGatewayAPIResourcesProtection)
7887
}
7988

8089
// testGatewayAPIResources tests that Gateway API Custom Resource Definitions are available.
@@ -158,6 +167,93 @@ func testGatewayAPIObjects(t *testing.T) {
158167
}
159168
}
160169

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

0 commit comments

Comments
 (0)