Skip to content

Commit eff1846

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 edf5e71 commit eff1846

File tree

4 files changed

+277
-0
lines changed

4 files changed

+277
-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"
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"
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: "modifications to Gateway API Custom Resource Definitions may only be made by the Ingress Operator"
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: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@ import (
1414
"github.com/openshift/cluster-ingress-operator/pkg/operator/controller/gatewayclass"
1515

1616
corev1 "k8s.io/api/core/v1"
17+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1718
"k8s.io/apimachinery/pkg/api/errors"
1819
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
20+
"k8s.io/apimachinery/pkg/types"
1921
"k8s.io/apiserver/pkg/storage/names"
2022

2123
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
@@ -30,6 +32,12 @@ const (
3032
expectedCatalogSourceNamespace = "openshift-marketplace"
3133
// The test gateway name used in multiple places.
3234
testGatewayName = "test-gateway"
35+
// gwapiCRDVAPName is the name of the ingress operator's Validating Admission Policy (VAP).
36+
gwapiCRDVAPName = "openshift-ingress-operator-gatewayapi-crd-admission"
37+
// cvoNamespace is the namespace of cluster version operator (CVO).
38+
cvoNamespace = "openshift-cluster-version"
39+
// cvoDeploymentName is the name of cluster version operator's deployment.
40+
cvoDeploymentName = "cluster-version-operator"
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.
@@ -141,6 +150,60 @@ func testGatewayAPIObjects(t *testing.T) {
141150
}
142151
}
143152

153+
// testGatewayAPIResourcesProtection verifies that the ingress operator's Validating Admission Policy
154+
// denies admission requests attempting to modify Gateway API CRDs on behalf of a user
155+
// who is not the ingress operator's service account.
156+
func testGatewayAPIResourcesProtection(t *testing.T) {
157+
t.Helper()
158+
159+
var testCRDs []*apiextensionsv1.CustomResourceDefinition
160+
for _, name := range crdNames {
161+
testCRDs = append(testCRDs, buildGWAPICRDFromName(name))
162+
}
163+
expectedErrMsg := "modifications to Gateway API Custom Resource Definitions may only be made by the Ingress Operator"
164+
165+
// Verify that GatewayAPI CRD creation is forbidden.
166+
for i := range testCRDs {
167+
if err := kclient.Create(context.Background(), testCRDs[i]); err != nil {
168+
if !strings.Contains(err.Error(), expectedErrMsg) {
169+
t.Errorf("unexpected error received while creating CRD %q: %v", testCRDs[i].Name, err)
170+
}
171+
} else {
172+
t.Errorf("admission error is expected while creating CRD %q but not received", testCRDs[i].Name)
173+
}
174+
}
175+
176+
// Verify that GatewayAPI CRD update is forbidden.
177+
for i := range testCRDs {
178+
crdName := types.NamespacedName{Name: testCRDs[i].Name}
179+
crd := &apiextensionsv1.CustomResourceDefinition{}
180+
if err := kclient.Get(context.Background(), crdName, crd); err != nil {
181+
t.Errorf("failed to get %q CRD: %v", crdName.Name, err)
182+
continue
183+
}
184+
crd.Spec = testCRDs[i].Spec
185+
crd.Spec.Conversion = testCRDs[i].Spec.Conversion
186+
if err := kclient.Update(context.Background(), crd); err != nil {
187+
if !strings.Contains(err.Error(), expectedErrMsg) {
188+
t.Errorf("unexpected error received while updating CRD %q: %v", testCRDs[i].Name, err)
189+
}
190+
} else {
191+
t.Errorf("admission error is expected while updating CRD %q but not received", testCRDs[i].Name)
192+
}
193+
}
194+
195+
// Verify that GatewayAPI CRD deletion is forbidden.
196+
for i := range testCRDs {
197+
if err := kclient.Delete(context.Background(), testCRDs[i]); err != nil {
198+
if !strings.Contains(err.Error(), expectedErrMsg) {
199+
t.Errorf("unexpected error received while deleting CRD %q: %v", testCRDs[i].Name, err)
200+
}
201+
} else {
202+
t.Errorf("admission error is expected while deleting CRD %q but not received", testCRDs[i].Name)
203+
}
204+
}
205+
}
206+
144207
// ensureCRDs tests that the Gateway API custom resource definitions exist.
145208
func ensureCRDs(t *testing.T) {
146209
t.Helper()
@@ -156,6 +219,24 @@ func ensureCRDs(t *testing.T) {
156219
// deleteCRDs deletes Gateway API custom resource definitions.
157220
func deleteCRDs(t *testing.T) {
158221
t.Helper()
222+
223+
// Remove the ingress operator's Validating Admission Policy
224+
// which prevents modifications of Gateway API CRDs
225+
// by anything other than the ingress operator.
226+
scaleDeployment(t, cvoNamespace, cvoDeploymentName, 0)
227+
if err := deleteVAP(t, gwapiCRDVAPName); err != nil {
228+
// Stop here because VAP will prevent the deletion of CRDs.
229+
t.Fatalf("failed to delete vap %q: %v", gwapiCRDVAPName, err)
230+
}
231+
defer func() {
232+
// Put the VAP back to ensure that it does not prevent
233+
// the ingress operator from managing Gateway API CRDs.
234+
scaleDeployment(t, cvoNamespace, cvoDeploymentName, 1)
235+
if err := assertVAP(t, gwapiCRDVAPName); err != nil {
236+
t.Errorf("failed to find vap %q: %v", gwapiCRDVAPName, err)
237+
}
238+
}()
239+
159240
for _, crdName := range crdNames {
160241
err := deleteExistingCRD(t, crdName)
161242
if err != nil {

test/e2e/util_gatewayapi_test.go

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
operatorcontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller"
2020
operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
2121

22+
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
2223
appsv1 "k8s.io/api/apps/v1"
2324
corev1 "k8s.io/api/core/v1"
2425
rbacv1 "k8s.io/api/rbac/v1"
@@ -149,6 +150,53 @@ func deleteExistingCRD(t *testing.T, crdName string) error {
149150
return nil
150151
}
151152

153+
// deleteVAP delete given Validating Admission Policy.
154+
func deleteVAP(t *testing.T, vapName string) error {
155+
t.Helper()
156+
157+
vap := &admissionregistrationv1.ValidatingAdmissionPolicy{}
158+
newVAP := &admissionregistrationv1.ValidatingAdmissionPolicy{}
159+
name := types.NamespacedName{Name: vapName}
160+
161+
// Retrieve the object to be deleted.
162+
if err := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 30*time.Second, false, func(context context.Context) (bool, error) {
163+
if err := kclient.Get(context, name, vap); err != nil {
164+
t.Logf("failed to get vap %q: %v, retrying ...", vapName, err)
165+
return false, nil
166+
}
167+
return true, nil
168+
}); err != nil {
169+
return fmt.Errorf("failed to get vap %q: %w", vapName, err)
170+
}
171+
172+
if err := kclient.Delete(context.Background(), vap); err != nil {
173+
return fmt.Errorf("failed to delete vap %q: %w", vapName, err)
174+
}
175+
176+
// Verify VAP was not recreated.
177+
if err := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 30*time.Second, false, func(ctx context.Context) (bool, error) {
178+
if err := kclient.Get(ctx, name, newVAP); err != nil {
179+
if kerrors.IsNotFound(err) {
180+
// VAP does not exist as expected.
181+
return true, nil
182+
}
183+
t.Logf("failed to get vap %q: %v, retrying ...", vapName, err)
184+
return false, nil
185+
}
186+
// Check if new VAP got recreated.
187+
if newVAP != nil && newVAP.UID != vap.UID {
188+
return true, fmt.Errorf("vap %q got recreated", vapName)
189+
}
190+
t.Logf("vap %q still exists, retrying ...", vapName)
191+
return false, nil
192+
}); err != nil {
193+
return fmt.Errorf("failed to verify deletion of vap %q: %v", vapName, err)
194+
}
195+
196+
t.Logf("deleted vap %q", vapName)
197+
return nil
198+
}
199+
152200
// createHttpRoute checks if the HTTPRoute can be created.
153201
// If it can't an error is returned.
154202
func createHttpRoute(namespace, routeName, parentNamespace, hostname, backendRefname string, gateway *gatewayapiv1.Gateway) (*gatewayapiv1.HTTPRoute, error) {
@@ -272,6 +320,71 @@ func buildHTTPRoute(routeName, namespace, parentgateway, parentNamespace, hostna
272320
}
273321
}
274322

323+
// buildGWAPICRDFromName initializes the GatewayAPI CRD deducing most of its required fields from the given name.
324+
func buildGWAPICRDFromName(name string) *apiextensionsv1.CustomResourceDefinition {
325+
var (
326+
plural = strings.Split(name, ".")[0]
327+
group, _ = strings.CutPrefix(name, plural+".")
328+
scope = apiextensionsv1.NamespaceScoped
329+
// removing trailing "s"
330+
singular = plural[0 : len(plural)-1]
331+
kind string
332+
)
333+
334+
switch plural {
335+
case "gatewayclasses":
336+
singular = "gatewayclass"
337+
kind = "GatewayClass"
338+
scope = apiextensionsv1.ClusterScoped
339+
case "gateways":
340+
kind = "Gateway"
341+
case "httproutes":
342+
kind = "HTTPRoute"
343+
case "referencegrants":
344+
kind = "ReferenceGrant"
345+
}
346+
347+
return &apiextensionsv1.CustomResourceDefinition{
348+
ObjectMeta: metav1.ObjectMeta{
349+
Name: plural + "." + group,
350+
Annotations: map[string]string{
351+
"api-approved.kubernetes.io": "https://github.com/kubernetes-sigs/gateway-api/pull/2466",
352+
},
353+
},
354+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
355+
Group: group,
356+
Names: apiextensionsv1.CustomResourceDefinitionNames{
357+
Singular: singular,
358+
Plural: plural,
359+
Kind: kind,
360+
},
361+
Scope: scope,
362+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
363+
{
364+
Name: "v1",
365+
Storage: true,
366+
Served: true,
367+
Schema: &apiextensionsv1.CustomResourceValidation{
368+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
369+
Type: "object",
370+
},
371+
},
372+
},
373+
{
374+
Name: "v1beta1",
375+
Storage: false,
376+
Served: true,
377+
Schema: &apiextensionsv1.CustomResourceValidation{
378+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
379+
Type: "object",
380+
},
381+
},
382+
},
383+
},
384+
},
385+
}
386+
}
387+
275388
// assertSubscription checks if the Subscription of the given name exists and returns an error if not.
276389
func assertSubscription(t *testing.T, namespace, subName string) error {
277390
t.Helper()
@@ -651,3 +764,37 @@ func assertDNSRecord(t *testing.T, recordName types.NamespacedName) error {
651764
})
652765
return err
653766
}
767+
768+
// assertVAP checks if the VAP of the given name exists, and returns an error if not.
769+
func assertVAP(t *testing.T, name string) error {
770+
t.Helper()
771+
vap := &admissionregistrationv1.ValidatingAdmissionPolicy{}
772+
return wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 1*time.Minute, false, func(context context.Context) (bool, error) {
773+
if err := kclient.Get(context, types.NamespacedName{Name: name}, vap); err != nil {
774+
t.Logf("failed to get vap %q: %v, retrying...", name, err)
775+
return false, nil
776+
}
777+
return true, nil
778+
})
779+
}
780+
781+
// scaleDeployment scales a deployment with the given name to the specified number of replicas.
782+
func scaleDeployment(t *testing.T, namespace, name string, replicas int32) error {
783+
t.Helper()
784+
785+
nsName := types.NamespacedName{Namespace: namespace, Name: name}
786+
return wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 30*time.Second, false, func(context context.Context) (bool, error) {
787+
depl := &appsv1.Deployment{}
788+
if err := kclient.Get(context, nsName, depl); err != nil {
789+
t.Logf("failed to get deployment %q: %v, retrying...", nsName, err)
790+
return false, nil
791+
}
792+
depl.Spec.Replicas = &replicas
793+
if err := kclient.Update(context, depl); err != nil {
794+
t.Logf("failed to update deployment %q: %v, retrying...", nsName, err)
795+
return false, nil
796+
}
797+
t.Logf("scaled deployment %q to %d replica(s)", nsName, replicas)
798+
return true, nil
799+
})
800+
}

0 commit comments

Comments
 (0)