From ccb5d93ca0a3d022f1d3757d9c37bff011741e73 Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Fri, 2 May 2025 11:29:32 -0400 Subject: [PATCH 1/6] Bump to OSSM 3.0.1 and Istio 1.24.4 Also, explicitly set ENABLE_GATEWAY_API_MANUAL_DEPLOYMENT to "false" on the Istio CR. For Istio 1.24.3, OSSM has a vendor override that sets this option[1]. However, for Istio 1.24.4, the option must be explicitly set. 1. https://github.com/openshift-service-mesh/sail-operator/blob/3bf27ee3c4fb4494ffe6028c7f72034c5a7a1e60/pkg/istiovalues/vendor_defaults.yaml#L11-L14 This commit resolves NE-2103. https://issues.redhat.com/browse/NE-2103 * cmd/ingress-operator/start.go (defaultGatewayAPIOperatorVersion): * manifests/02-deployment-ibm-cloud-managed.yaml (GATEWAY_API_OPERATOR_VERSION): * manifests/02-deployment.yaml (GATEWAY_API_OPERATOR_VERSION): Bump from OSSM v3.0.0 to v3.0.1. * pkg/operator/controller/gatewayclass/istio.go (desiredIstio): Bump from Istio v1.24.3 to v1.24.4. Set ENABLE_GATEWAY_API_MANUAL_DEPLOYMENT to "false". (cherry picked from commit 47d415264a24fd80c5ffb82793569919933d3031) --- cmd/ingress-operator/start.go | 2 +- manifests/02-deployment-ibm-cloud-managed.yaml | 2 +- manifests/02-deployment.yaml | 2 +- pkg/operator/controller/gatewayclass/istio.go | 7 ++++++- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/cmd/ingress-operator/start.go b/cmd/ingress-operator/start.go index e548062b8f..81c1bf2b41 100644 --- a/cmd/ingress-operator/start.go +++ b/cmd/ingress-operator/start.go @@ -34,7 +34,7 @@ const ( // that is mounted from configmap openshift-ingress-operator/trusted-ca. defaultTrustedCABundle = "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem" defaultGatewayAPIOperatorChannel = "stable" - defaultGatewayAPIOperatorVersion = "servicemeshoperator3.v3.0.0" + defaultGatewayAPIOperatorVersion = "servicemeshoperator3.v3.0.1" ) type StartOptions struct { diff --git a/manifests/02-deployment-ibm-cloud-managed.yaml b/manifests/02-deployment-ibm-cloud-managed.yaml index 9e2d14fce8..8903d6132a 100644 --- a/manifests/02-deployment-ibm-cloud-managed.yaml +++ b/manifests/02-deployment-ibm-cloud-managed.yaml @@ -53,7 +53,7 @@ spec: - name: GATEWAY_API_OPERATOR_CHANNEL value: stable - name: GATEWAY_API_OPERATOR_VERSION - value: servicemeshoperator3.v3.0.0 + value: servicemeshoperator3.v3.0.1 image: openshift/origin-cluster-ingress-operator:latest imagePullPolicy: IfNotPresent name: ingress-operator diff --git a/manifests/02-deployment.yaml b/manifests/02-deployment.yaml index ebbc5dbd73..8bda4436bb 100644 --- a/manifests/02-deployment.yaml +++ b/manifests/02-deployment.yaml @@ -82,7 +82,7 @@ spec: - name: GATEWAY_API_OPERATOR_CHANNEL value: stable - name: GATEWAY_API_OPERATOR_VERSION - value: servicemeshoperator3.v3.0.0 + value: servicemeshoperator3.v3.0.1 resources: requests: cpu: 10m diff --git a/pkg/operator/controller/gatewayclass/istio.go b/pkg/operator/controller/gatewayclass/istio.go index dea7772f52..5a48edb306 100644 --- a/pkg/operator/controller/gatewayclass/istio.go +++ b/pkg/operator/controller/gatewayclass/istio.go @@ -95,6 +95,11 @@ func desiredIstio(name types.NamespacedName, ownerRef metav1.OwnerReference) *sa // "multi-network gateways". This is an Istio feature that I // haven't really found any explanation for. "PILOT_MULTI_NETWORK_DISCOVER_GATEWAY_API": "false", + // Don't allow Istio's "manual deployment" feature, which would + // allow a gateway to specify an existing service. Only allow + // "automated deployment", meaning Istio creates a new load- + // balancer service for each gateway. + "ENABLE_GATEWAY_API_MANUAL_DEPLOYMENT": "false", } return &sailv1.Istio{ ObjectMeta: metav1.ObjectMeta{ @@ -151,7 +156,7 @@ func desiredIstio(name types.NamespacedName, ownerRef metav1.OwnerReference) *sa IngressControllerMode: sailv1.MeshConfigIngressControllerModeOff, }, }, - Version: "v1.24.3", + Version: "v1.24.4", }, } } From 7408cea3a28772128e04d67d586f4dc2b5c9b79f Mon Sep 17 00:00:00 2001 From: Aslak Knutsen Date: Tue, 1 Apr 2025 19:08:44 +0200 Subject: [PATCH 2/6] Enable Gateway only CA Bundles Configure Istio to inject the configmaps only into namespaces where gateways exist in order to avoid polluting the whole cluster. Set one new environment variable in the Istio CR: PILOT_ENABLE_GATEWAY_API_CA_CERT_ONLY In the future, we will set the Istio CR's trustBundleName global value to specify a custom configmap name. However, we cannot do that yet as the trustBundleName field only exists in OSSM 3.1. This commit is related to OSSM-9076. * pkg/operator/controller/gatewayclass/istio.go (desiredIstio): Set the new environment variable. * pkg/operator/controller/names.go (OpenShiftGatewayCARootCertName): New const for future use. Modified-by: Miciah Masters (cherry picked from commit 41d7addc3975ea0ca12ada2c368ef58311298921) --- pkg/operator/controller/gatewayclass/istio.go | 3 +++ pkg/operator/controller/names.go | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/pkg/operator/controller/gatewayclass/istio.go b/pkg/operator/controller/gatewayclass/istio.go index 5a48edb306..c972556a20 100644 --- a/pkg/operator/controller/gatewayclass/istio.go +++ b/pkg/operator/controller/gatewayclass/istio.go @@ -100,6 +100,9 @@ func desiredIstio(name types.NamespacedName, ownerRef metav1.OwnerReference) *sa // "automated deployment", meaning Istio creates a new load- // balancer service for each gateway. "ENABLE_GATEWAY_API_MANUAL_DEPLOYMENT": "false", + // Only create CA Bundle CM in namespaces where there are + // Gateway API Gateways + "PILOT_ENABLE_GATEWAY_API_CA_CERT_ONLY": "true", } return &sailv1.Istio{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/operator/controller/names.go b/pkg/operator/controller/names.go index 424375e8d6..7c9858a28a 100644 --- a/pkg/operator/controller/names.go +++ b/pkg/operator/controller/names.go @@ -68,6 +68,11 @@ const ( // gatewayclass that Istio creates when it is installed. OpenShiftDefaultGatewayClassName = "openshift-default" + // OpenShiftGatewayCARootCertName is the name of the configmap with the + // CA bundle that Istio creates for the Istio CR that this operator + // creates. + OpenShiftGatewayCARootCertName = "openshift-gw-ca-root-cert" + // IstioRevLabelKey is the key for the gateway label that Istio checks // for to determine whether it should reconcile that gateway. IstioRevLabelKey = "istio.io/rev" From 39c0861e37f7a5e554f554a4e2137dc422647274 Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Fri, 2 May 2025 11:48:01 -0400 Subject: [PATCH 3/6] desiredIstio: Don't copy labels or annotations Configure Istiod not to copy annotations or labels from gateways onto associated resources, such as the proxy deployment and load-balancer service for a gateway. This copying behavior is Istio-specific, not part of the Gateway API spec, and could be used to inject unsupported configuration. For example, an end-user could set a service annotation on the gateway in order to configure a load-balancer. Setting annotations on the gateway to configure the load-balancer would not be portable to other Gateway API implementations and would complicate product support. This commit is related to OSSM-8989. https://issues.redhat.com/browse/OSSM-8989 * pkg/operator/controller/gatewayclass/istio.go (desiredIstio): Set the "PILOT_ENABLE_GATEWAY_API_COPY_LABELS_ANNOTATIONS" to "false". (cherry picked from commit 05417af3686f5ef709706e1e46f70bbec769729c) --- pkg/operator/controller/gatewayclass/istio.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/operator/controller/gatewayclass/istio.go b/pkg/operator/controller/gatewayclass/istio.go index c972556a20..62babe43e8 100644 --- a/pkg/operator/controller/gatewayclass/istio.go +++ b/pkg/operator/controller/gatewayclass/istio.go @@ -103,6 +103,13 @@ func desiredIstio(name types.NamespacedName, ownerRef metav1.OwnerReference) *sa // Only create CA Bundle CM in namespaces where there are // Gateway API Gateways "PILOT_ENABLE_GATEWAY_API_CA_CERT_ONLY": "true", + // Don't copy labels or annotations from gateways to resources + // that Istiod creates for that gateway. This is an Istio- + // specific behavior which might not be supported by other + // Gateway API implementations and that could allow the end-user + // to inject unsupported configuration, for example using + // service annotations. + "PILOT_ENABLE_GATEWAY_API_COPY_LABELS_ANNOTATIONS": "false", } return &sailv1.Istio{ ObjectMeta: metav1.ObjectMeta{ From 6ec264fa189ffd98a689c5f3143ebc676c381eaf Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Fri, 2 May 2025 12:42:16 -0400 Subject: [PATCH 4/6] desiredIstio: Delete old controller-mode setting Delete the obsolete PILOT_ENABLE_GATEWAY_CONTROLLER_MODE environment variable from the Istiod configuration. This environment variable is no longer recognized in OSSM 3, and the variable has been superseded by EnhancedResourceScoping. * pkg/operator/controller/gatewayclass/istio.go (desiredIstio): Delete PILOT_ENABLE_GATEWAY_CONTROLLER_MODE. (cherry picked from commit e05309ee67247113e1ba70a95092691a559a9df8) --- pkg/operator/controller/gatewayclass/istio.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/operator/controller/gatewayclass/istio.go b/pkg/operator/controller/gatewayclass/istio.go index 62babe43e8..b6ac368d09 100644 --- a/pkg/operator/controller/gatewayclass/istio.go +++ b/pkg/operator/controller/gatewayclass/istio.go @@ -83,11 +83,6 @@ func desiredIstio(name types.NamespacedName, ownerRef metav1.OwnerReference) *sa // then our Istiod instance might try to reconcile gateways // belonging to an unrelated Istiod instance. "PILOT_GATEWAY_API_DEFAULT_GATEWAYCLASS_NAME": controller.OpenShiftDefaultGatewayClassName, - // Watch Gateway API and Kubernetes resources in all namespaces, - // but ignore Istio resources that don't match our label - // selector. (We do not specify the label selector, so this - // causes Istio to ignore all Istio resources.) - "PILOT_ENABLE_GATEWAY_CONTROLLER_MODE": "true", // Only reconcile resources that are associated with // gatewayclasses that have our controller name. "PILOT_GATEWAY_API_CONTROLLER_NAME": controller.OpenShiftGatewayClassControllerName, From fe3dd8ad5b23f3f5c6a218247c37ebf702b54d5d Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Fri, 2 May 2025 16:21:33 -0400 Subject: [PATCH 5/6] assertDNSRecord: Increase timeout to 10m Increase the timeout in assertDNSRecord for polling for the DNSRecord CR from 1 minute to 10 minutes. The cloud provider can easily take over a minute to provision the load balancer, and the operator cannot create the DNSRecord CR before the load balancer has been provisioned and assigned a host name or address. Consequently, the polling loop could easily reach the 1-minute timeout just on account of the time that it takes to provision the load balancer. * test/e2e/util_gatewayapi_test.go (assertDNSRecord): Increase timeout for the DNSRecord CR polling loop from 1m to 10m. (cherry picked from commit 2f7da77fd531a8c780d0e65f56b50ebe51706593) --- test/e2e/util_gatewayapi_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/util_gatewayapi_test.go b/test/e2e/util_gatewayapi_test.go index c3cf2c24e2..5fc8c61d85 100644 --- a/test/e2e/util_gatewayapi_test.go +++ b/test/e2e/util_gatewayapi_test.go @@ -1035,7 +1035,7 @@ func assertDNSRecord(t *testing.T, recordName types.NamespacedName) error { t.Helper() dnsRecord := &v1.DNSRecord{} - err := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 1*time.Minute, false, func(context context.Context) (bool, error) { + err := wait.PollUntilContextTimeout(context.Background(), 10*time.Second, 10*time.Minute, false, func(context context.Context) (bool, error) { if err := kclient.Get(context, recordName, dnsRecord); err != nil { t.Logf("Failed to get DNSRecord %v: %v; retrying...", recordName, err) return false, nil From d30becadd43680ea9b02db670d433db398aa3f79 Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Fri, 2 May 2025 16:29:55 -0400 Subject: [PATCH 6/6] testGatewayAPIManualDeployment: Increase timeout Increase the timeout for polling the gateway, and dump the gateway if the test fails. * test/e2e/gateway_api_test.go (testGatewayAPIManualDeployment): Increase the timeout for polling the gateway from 1m to 5m. Dump the gateway if the test fails. (cherry picked from commit f1e445da0c864fc0c15ed3f90b7f3f2f6483a014) --- test/e2e/gateway_api_test.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/e2e/gateway_api_test.go b/test/e2e/gateway_api_test.go index 0bb591dd99..8d41ad7dc6 100644 --- a/test/e2e/gateway_api_test.go +++ b/test/e2e/gateway_api_test.go @@ -15,6 +15,7 @@ import ( iov1 "github.com/openshift/api/operatoringress/v1" operatorclient "github.com/openshift/cluster-ingress-operator/pkg/operator/client" operatorcontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller" + util "github.com/openshift/cluster-ingress-operator/pkg/util" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -259,6 +260,14 @@ func testGatewayAPIManualDeployment(t *testing.T) { t.Fatalf("Failed to create gateway %v: %v", gatewayName, err) } t.Cleanup(func() { + if t.Failed() { + t.Logf("Dumping gateway %q...", gatewayName) + var gateway gatewayapiv1.Gateway + if err := kclient.Get(context.Background(), gatewayName, &gateway); err != nil { + t.Errorf("Failed to get gateway %v: %v", gatewayName, err) + } + t.Log(util.ToYaml(gateway)) + } if err := kclient.Delete(context.Background(), &gateway); err != nil { if !errors.IsNotFound(err) { t.Errorf("Failed to delete gateway %v: %v", gatewayName, err) @@ -266,7 +275,7 @@ func testGatewayAPIManualDeployment(t *testing.T) { } }) - interval, timeout := 5*time.Second, 1*time.Minute + interval, timeout := 5*time.Second, 5*time.Minute t.Logf("Polling for up to %v to verify that the gateway is accepted...", timeout) if err := wait.PollUntilContextTimeout(context.Background(), interval, timeout, false, func(context context.Context) (bool, error) { if err := kclient.Get(context, gatewayName, &gateway); err != nil {