Skip to content

Commit b6394a0

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 b6394a0

File tree

4 files changed

+312
-0
lines changed

4 files changed

+312
-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: "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: 72 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,8 @@ 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"
3337
)
3438

3539
var crdNames = []string{
@@ -75,6 +79,7 @@ func TestGatewayAPI(t *testing.T) {
7579
t.Run("testGatewayAPIResources", testGatewayAPIResources)
7680
t.Run("testGatewayAPIObjects", testGatewayAPIObjects)
7781
t.Run("testGatewayAPIIstioInstallation", testGatewayAPIIstioInstallation)
82+
t.Run("testGatewayAPIResourcesProtection", testGatewayAPIResourcesProtection)
7883
}
7984

8085
// testGatewayAPIResources tests that Gateway API Custom Resource Definitions are available.
@@ -141,6 +146,60 @@ func testGatewayAPIObjects(t *testing.T) {
141146
}
142147
}
143148

149+
// testGatewayAPIResourcesProtection verifies that the ingress operator's Validating Admission Policy
150+
// denies admission requests attempting to modify Gateway API CRDs on behalf of a user
151+
// who is not the ingress operator's service account.
152+
func testGatewayAPIResourcesProtection(t *testing.T) {
153+
t.Helper()
154+
155+
var testCRDs []*apiextensionsv1.CustomResourceDefinition
156+
for _, name := range crdNames {
157+
testCRDs = append(testCRDs, buildGWAPICRDFromName(name))
158+
}
159+
expectedErrMsg := "modifications to Gateway API Custom Resource Definitions may only be made by the Ingress Operator"
160+
161+
// Verify that GatewayAPI CRD creation is forbidden.
162+
for i := range testCRDs {
163+
if err := kclient.Create(context.Background(), testCRDs[i]); err != nil {
164+
if !strings.Contains(err.Error(), expectedErrMsg) {
165+
t.Errorf("unexpected error received while creating CRD %q: %v", testCRDs[i].Name, err)
166+
}
167+
} else {
168+
t.Errorf("admission error is expected while creating CRD %q but not received", testCRDs[i].Name)
169+
}
170+
}
171+
172+
// Verify that GatewayAPI CRD update is forbidden.
173+
for i := range testCRDs {
174+
crdName := types.NamespacedName{Name: testCRDs[i].Name}
175+
crd := &apiextensionsv1.CustomResourceDefinition{}
176+
if err := kclient.Get(context.Background(), crdName, crd); err != nil {
177+
t.Errorf("failed to get %q CRD: %v", crdName.Name, err)
178+
continue
179+
}
180+
crd.Spec = testCRDs[i].Spec
181+
crd.Spec.Conversion = testCRDs[i].Spec.Conversion
182+
if err := kclient.Update(context.Background(), crd); err != nil {
183+
if !strings.Contains(err.Error(), expectedErrMsg) {
184+
t.Errorf("unexpected error received while updating CRD %q: %v", testCRDs[i].Name, err)
185+
}
186+
} else {
187+
t.Errorf("admission error is expected while updating CRD %q but not received", testCRDs[i].Name)
188+
}
189+
}
190+
191+
// Verify that GatewayAPI CRD deletion is forbidden.
192+
for i := range testCRDs {
193+
if err := kclient.Delete(context.Background(), testCRDs[i]); err != nil {
194+
if !strings.Contains(err.Error(), expectedErrMsg) {
195+
t.Errorf("unexpected error received while deleting CRD %q: %v", testCRDs[i].Name, err)
196+
}
197+
} else {
198+
t.Errorf("admission error is expected while deleting CRD %q but not received", testCRDs[i].Name)
199+
}
200+
}
201+
}
202+
144203
// ensureCRDs tests that the Gateway API custom resource definitions exist.
145204
func ensureCRDs(t *testing.T) {
146205
t.Helper()
@@ -156,6 +215,19 @@ func ensureCRDs(t *testing.T) {
156215
// deleteCRDs deletes Gateway API custom resource definitions.
157216
func deleteCRDs(t *testing.T) {
158217
t.Helper()
218+
219+
vm := newVAPManager(t, gwapiCRDVAPName)
220+
// Remove the ingress operator's Validating Admission Policy (VAP)
221+
// which prevents modifications of Gateway API CRDs
222+
// by anything other than the ingress operator.
223+
if err, recoverFn := vm.disable(); err != nil {
224+
defer recoverFn()
225+
t.Fatalf("failed to disable vap: %v", err)
226+
}
227+
// Put back the VAP to ensure that it does not prevent
228+
// the ingress operator from managing Gateway API CRDs.
229+
defer vm.enable()
230+
159231
for _, crdName := range crdNames {
160232
err := deleteExistingCRD(t, crdName)
161233
if err != nil {

test/e2e/util_gatewayapi_test.go

Lines changed: 191 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"
@@ -43,6 +44,10 @@ const (
4344
openshiftIstiodDeploymentName = "istiod-openshift-gateway"
4445
// openshiftSMCPName holds the expected OSSM ServiceMeshControlPlane name
4546
openshiftSMCPName = "openshift-gateway"
47+
// cvoNamespace is the namespace of cluster version operator (CVO).
48+
cvoNamespace = "openshift-cluster-version"
49+
// cvoDeploymentName is the name of cluster version operator's deployment.
50+
cvoDeploymentName = "cluster-version-operator"
4651
)
4752

4853
// updateIngressOperatorRole updates the ingress-operator cluster role with cluster-admin privilege.
@@ -149,6 +154,53 @@ func deleteExistingCRD(t *testing.T, crdName string) error {
149154
return nil
150155
}
151156

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

327+
// buildGWAPICRDFromName initializes the GatewayAPI CRD deducing most of its required fields from the given name.
328+
func buildGWAPICRDFromName(name string) *apiextensionsv1.CustomResourceDefinition {
329+
var (
330+
plural = strings.Split(name, ".")[0]
331+
group, _ = strings.CutPrefix(name, plural+".")
332+
scope = apiextensionsv1.NamespaceScoped
333+
// removing trailing "s"
334+
singular = plural[0 : len(plural)-1]
335+
kind string
336+
)
337+
338+
switch plural {
339+
case "gatewayclasses":
340+
singular = "gatewayclass"
341+
kind = "GatewayClass"
342+
scope = apiextensionsv1.ClusterScoped
343+
case "gateways":
344+
kind = "Gateway"
345+
case "httproutes":
346+
kind = "HTTPRoute"
347+
case "referencegrants":
348+
kind = "ReferenceGrant"
349+
}
350+
351+
return &apiextensionsv1.CustomResourceDefinition{
352+
ObjectMeta: metav1.ObjectMeta{
353+
Name: plural + "." + group,
354+
Annotations: map[string]string{
355+
"api-approved.kubernetes.io": "https://github.com/kubernetes-sigs/gateway-api/pull/2466",
356+
},
357+
},
358+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
359+
Group: group,
360+
Names: apiextensionsv1.CustomResourceDefinitionNames{
361+
Singular: singular,
362+
Plural: plural,
363+
Kind: kind,
364+
},
365+
Scope: scope,
366+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
367+
{
368+
Name: "v1",
369+
Storage: true,
370+
Served: true,
371+
Schema: &apiextensionsv1.CustomResourceValidation{
372+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
373+
Type: "object",
374+
},
375+
},
376+
},
377+
{
378+
Name: "v1beta1",
379+
Storage: false,
380+
Served: true,
381+
Schema: &apiextensionsv1.CustomResourceValidation{
382+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
383+
Type: "object",
384+
},
385+
},
386+
},
387+
},
388+
},
389+
}
390+
}
391+
275392
// assertSubscription checks if the Subscription of the given name exists and returns an error if not.
276393
func assertSubscription(t *testing.T, namespace, subName string) error {
277394
t.Helper()
@@ -651,3 +768,77 @@ func assertDNSRecord(t *testing.T, recordName types.NamespacedName) error {
651768
})
652769
return err
653770
}
771+
772+
// assertVAP checks if the VAP of the given name exists, and returns an error if not.
773+
func assertVAP(t *testing.T, name string) error {
774+
t.Helper()
775+
vap := &admissionregistrationv1.ValidatingAdmissionPolicy{}
776+
return wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 1*time.Minute, false, func(context context.Context) (bool, error) {
777+
if err := kclient.Get(context, types.NamespacedName{Name: name}, vap); err != nil {
778+
t.Logf("failed to get vap %q: %v, retrying...", name, err)
779+
return false, nil
780+
}
781+
return true, nil
782+
})
783+
}
784+
785+
// scaleDeployment scales the deployment with the given name to the specified number of replicas.
786+
func scaleDeployment(t *testing.T, namespace, name string, replicas int32) error {
787+
t.Helper()
788+
789+
nsName := types.NamespacedName{Namespace: namespace, Name: name}
790+
return wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 30*time.Second, false, func(context context.Context) (bool, error) {
791+
depl := &appsv1.Deployment{}
792+
if err := kclient.Get(context, nsName, depl); err != nil {
793+
t.Logf("failed to get deployment %q: %v, retrying...", nsName, err)
794+
return false, nil
795+
}
796+
if *depl.Spec.Replicas != replicas {
797+
depl.Spec.Replicas = &replicas
798+
if err := kclient.Update(context, depl); err != nil {
799+
t.Logf("failed to update deployment %q: %v, retrying...", nsName, err)
800+
return false, nil
801+
}
802+
}
803+
t.Logf("scaled deployment %q to %d replica(s)", nsName, replicas)
804+
return true, nil
805+
})
806+
}
807+
808+
// vapManager helps to disable the VAP resource which is managed by CVO.
809+
type vapManager struct {
810+
t *testing.T
811+
name string
812+
}
813+
814+
// newVAPManager returns a new instance of VAPManager.
815+
func newVAPManager(t *testing.T, vapName string) *vapManager {
816+
return &vapManager{
817+
t: t,
818+
name: vapName,
819+
}
820+
}
821+
822+
// disable scales down CVO and removes the VAP resource.
823+
func (m *VAPManager) disable() (error, func()) {
824+
if err := scaleDeployment(m.t, cvoNamespace, cvoDeploymentName, 0); err != nil {
825+
return fmt.Errorf("failed to scale down cvo: %w", err), func() { /*scale down didn't work, nothing to do*/ }
826+
}
827+
if err := deleteExistingVAP(m.t, m.name); err != nil {
828+
return fmt.Errorf("failed to delete vap %q: %w", m.name, err), func() {
829+
if err := scaleDeployment(m.t, cvoNamespace, cvoDeploymentName, 1); err != nil {
830+
m.t.Errorf("failed to scale up cvo: %v", err)
831+
}
832+
}
833+
}
834+
return nil, nil
835+
}
836+
837+
// Enable scales up CVO and waits until the VAP is recreated.
838+
func (m *vapManager) enable() {
839+
if err := scaleDeployment(m.t, cvoNamespace, cvoDeploymentName, 1); err != nil {
840+
m.t.Errorf("failed to scale up cvo: %v", err)
841+
} else if err := assertVAP(m.t, m.name); err != nil {
842+
m.t.Errorf("failed to find vap %q: %v", m.name, err)
843+
}
844+
}

0 commit comments

Comments
 (0)