Merge pull request #29859 from kyrtapz/net_diag_perm_check

OCPBUGS-56698: Skip tests modifying cluster/network.config when it is not permitted

Author: openshift-merge-bot[bot]

test/extended/networking/network_diagnostics.go

@@ -16,6 +16,7 @@ import (
 	"k8s.io/kubernetes/test/e2e/framework"
 
 	exutil "github.com/openshift/origin/test/extended/util"
+	"k8s.io/kubernetes/test/e2e/framework/skipper"
 )
 
 const (
@@ -31,9 +32,16 @@ var _ = g.Describe("[sig-network][OCPFeatureGate:NetworkDiagnosticsConfig][Seria
 	oc := exutil.NewCLIWithoutNamespace("network-diagnostics")
 
 	g.BeforeAll(func(ctx context.Context) {
+		// Check if the test can write to cluster/network.config.openshift.io
+		hasAccess, err := hasNetworkConfigWriteAccess(oc)
+		o.Expect(err).NotTo(o.HaveOccurred())
+		if !hasAccess {
+			skipper.Skipf("The test is not permitted to modify the cluster/network.config.openshift.io resource")
+		}
+
 		// Reset and take ownership of the network diagnostics config
 		patch := []byte(`{"spec":{"networkDiagnostics":null}}`)
-		_, err := oc.AdminConfigClient().ConfigV1().Networks().Patch(ctx, clusterConfig, types.MergePatchType, patch, metav1.PatchOptions{FieldManager: fieldManager})
+		_, err = oc.AdminConfigClient().ConfigV1().Networks().Patch(ctx, clusterConfig, types.MergePatchType, patch, metav1.PatchOptions{FieldManager: fieldManager})
 		o.Expect(err).NotTo(o.HaveOccurred())
 	})
 

test/extended/networking/services.go

@@ -15,6 +15,7 @@ import (
 	admissionapi "k8s.io/pod-security-admission/api"
 
 	exutil "github.com/openshift/origin/test/extended/util"
+	"k8s.io/kubernetes/test/e2e/framework/skipper"
 )
 
 var _ = Describe("[sig-network] services", func() {
@@ -89,6 +90,13 @@ var _ = Describe("[sig-network] services", func() {
 
 	InIPv4ClusterContext(oc, func() {
 		It("ensures external ip policy is configured correctly on the cluster [apigroup:config.openshift.io] [Serial]", func() {
+			// Check if the test can write to cluster/network.config.openshift.io
+			hasAccess, err := hasNetworkConfigWriteAccess(oc)
+			Expect(err).NotTo(HaveOccurred())
+			if !hasAccess {
+				skipper.Skipf("The test is not permitted to modify the cluster/network.config.openshift.io resource")
+			}
+
 			namespace := oc.Namespace()
 			adminConfigClient := oc.AdminConfigClient()
 			k8sClient := oc.KubeClient()
@@ -97,7 +105,7 @@ var _ = Describe("[sig-network] services", func() {
 			By("create service of type load balancer with default cluster networks config")
 			serviceName := names.SimpleNameGenerator.GenerateName("svc-without-ext-ip")
 			By("check load balance service creation fails")
-			err := createWebserverLBService(k8sClient, namespace, serviceName, "", []string{"192.168.132.10"}, nil)
+			err = createWebserverLBService(k8sClient, namespace, serviceName, "", []string{"192.168.132.10"}, nil)
 			Expect(kapierrs.IsForbidden(err)).Should(Equal(true))
 
 			// Test external ip policy configured with allowedCIDRs. Make sure service
@@ -166,6 +174,13 @@ var _ = Describe("[sig-network] services", func() {
 
 	InBareMetalIPv4ClusterContext(oc, func() {
 		It("ensures external auto assign cidr is configured correctly on the cluster [apigroup:config.openshift.io] [Serial]", func() {
+			// Check if the test can write to cluster/network.config.openshift.io
+			hasAccess, err := hasNetworkConfigWriteAccess(oc)
+			Expect(err).NotTo(HaveOccurred())
+			if !hasAccess {
+				skipper.Skipf("The test is not permitted to modify the cluster/network.config.openshift.io resource")
+			}
+
 			namespace := oc.Namespace()
 			adminConfigClient := oc.AdminConfigClient()
 			k8sClient := oc.KubeClient()
@@ -175,7 +190,7 @@ var _ = Describe("[sig-network] services", func() {
 			By("create service of type load balancer with default cluster networks config")
 			serviceName := names.SimpleNameGenerator.GenerateName("svc-without-ext-ip-3")
 			By("check load balance service creation fails")
-			err := createWebserverLBService(k8sClient, namespace, serviceName, "", []string{"192.168.132.10"}, nil)
+			err = createWebserverLBService(k8sClient, namespace, serviceName, "", []string{"192.168.132.10"}, nil)
 			Expect(kapierrs.IsForbidden(err)).Should(Equal(true))
 
 			// Test external ip policy configured with both policy and auto assign cidr. Make sure service

test/extended/networking/util.go

@@ -30,6 +30,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/storage/names"
@@ -985,3 +986,21 @@ func getMachineConfigPoolByLabel(oc *exutil.CLI, mcSelectorLabel labels.Set) ([]
 	}
 	return pools, nil
 }
+
+// hasNetworkConfigWriteAccess determines if the admin client can patch the cluster/network.config.openshift.io object
+// by patching the resource in a dry-run mode(no changes are persisted).
+func hasNetworkConfigWriteAccess(oc *exutil.CLI) (bool, error) {
+	_, err := oc.AdminConfigClient().ConfigV1().Networks().Patch(context.TODO(),
+		clusterConfig,
+		types.MergePatchType,
+		[]byte(`{"spec":{"networkType": ""}}`),
+		metav1.PatchOptions{FieldManager: oc.Namespace(), DryRun: []string{metav1.DryRunAll}})
+
+	if err != nil {
+		if kapierrs.IsInvalid(err) || kapierrs.IsForbidden(err) {
+			return false, nil
+		}
+		return false, err
+	}
+	return true, nil
+}