fix shape of istio plugin

Author: lgadban

projects/gateway2/Makefile

@@ -14,7 +14,7 @@ test:
 test-full:
 	go test -ldflags=$(LDFLAGS) -count=1 ./...
 
-# internal target used by controller_suite_test.go
+# internal target used by ./controller/controller_suite_test.go & ./setup/ggv2setup_test.go
 envtest:
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)"
 

projects/gateway2/extensions2/plugins/istio/plugin.go

@@ -8,6 +8,7 @@ import (
 
 	structpb "github.com/golang/protobuf/ptypes/struct"
 	"google.golang.org/protobuf/types/known/anypb"
+	"istio.io/istio/pkg/kube/krt"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	envoy_config_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
@@ -58,6 +59,8 @@ func (i IstioSettings) Equals(in any) bool {
 var _ ir.PolicyIR = &IstioSettings{}
 
 func NewPlugin(ctx context.Context, commoncol *common.CommonCollections) extensionsplug.Plugin {
+	p := plugin{}
+
 	// TODO: if plumb settings from gw class; then they should be in the new translation pass
 	// the problem is that they get applied to an upstream, and currently we don't have access to the gateway
 	// when translating upstreams. if we want we can add the gateway to the context of PerClientProcessUpstream
@@ -67,23 +70,22 @@ func NewPlugin(ctx context.Context, commoncol *common.CommonCollections) extensi
 		EnableIstioIntegration:      commoncol.Settings.IstioIntegration,
 		EnableIstioSidecarOnGateway: sidecarEnabled,
 	}
-	p := plugin{
-		settings: istioSettings,
-	}
 
 	return extensionsplug.Plugin{
 		ContributesPolicies: map[schema.GroupKind]extensionsplug.PolicyPlugin{
 			VirtualIstioGK: {
 				Name:            "istio",
 				ProcessUpstream: p.processUpstream,
+				GlobalPolicies: func(_ krt.HandlerContext, _ extensionsplug.AttachmentPoints) ir.PolicyIR {
+					// return static settings which do not change post plugin creation
+					return istioSettings
+				},
 			},
 		},
 	}
 }
 
-type plugin struct {
-	settings IstioSettings
-}
+type plugin struct{}
 
 func isDisabledForUpstream(_ ir.Upstream) bool {
 	// return in.GetDisableIstioAutoMtls().GetValue()
@@ -100,24 +102,27 @@ func doesClusterHaveSslConfigPresent(_ *envoy_config_cluster_v3.Cluster) bool {
 	return false
 }
 
-func (p plugin) processUpstream(ctx context.Context, _ ir.PolicyIR, in ir.Upstream, out *envoy_config_cluster_v3.Cluster) {
+func (p plugin) processUpstream(ctx context.Context, ir ir.PolicyIR, in ir.Upstream, out *envoy_config_cluster_v3.Cluster) {
 	var socketmatches []*envoy_config_cluster_v3.Cluster_TransportSocketMatch
 
+	st, ok := ir.(IstioSettings)
+	if !ok {
+		return
+	}
 	// Istio automtls will only be applied when:
 	// 1) automtls is enabled on the settings
 	// 2) the upstream has not disabled auto mtls
 	// 3) the upstream has no sslConfig
-	//if p.settings.GetGloo().GetIstioOptions().GetEnableAutoMtls().GetValue() && !in.GetDisableIstioAutoMtls().GetValue() && sslConfig == nil {
-	if p.settings.EnableAutoMTLS && !isDisabledForUpstream(in) && !doesClusterHaveSslConfigPresent(out) {
+	if st.EnableAutoMTLS && !isDisabledForUpstream(in) && !doesClusterHaveSslConfigPresent(out) {
 		// Istio automtls config is not applied if istio integration is disabled on the helm chart.
 		// When istio integration is disabled via istioSds.enabled=false, there is no sds or istio-proxy sidecar present
-		if !p.settings.EnableIstioIntegration {
+		if !st.EnableIstioIntegration {
 			contextutils.LoggerFrom(ctx).Desugar().Error("Istio integration must be enabled to use auto mTLS. Enable integration with istioIntegration.enabled=true")
 		} else {
 			// Note: If EnableIstioSidecarOnGateway is enabled, Istio automtls will not be able to generate the endpoint
 			// metadata from the Pod to match the transport socket match. We will still translate the transport socket match
 			// configuration. EnableIstioSidecarOnGateway should be removed as part of: https://github.com/solo-io/solo-projects/issues/5743
-			if p.settings.EnableIstioSidecarOnGateway {
+			if st.EnableIstioSidecarOnGateway {
 				contextutils.LoggerFrom(ctx).Desugar().Warn("Istio sidecar injection (istioIntegration.EnableIstioSidecarOnGateway) should be disabled for Istio automtls mode")
 			}
 

projects/gateway2/setup/ggv2setup_test.go

@@ -21,7 +21,6 @@ import (
 	envoylistener "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
 	envoy_config_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
 	envoyhttp "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
-	cache "github.com/envoyproxy/go-control-plane/pkg/cache/v3"
 	jsonpb "google.golang.org/protobuf/encoding/protojson"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -111,7 +110,12 @@ func init() {
 func TestScenarios(t *testing.T) {
 	proxy_syncer.UseDetailedUnmarshalling = true
 	writer.set(t)
-	os.Setenv("POD_NAMESPACE", "gwtest")
+
+	os.Setenv("POD_NAMESPACE", "gwtest") // TODO: is this still needed?
+	// set global settings env vars; current ggv2setup_tests all assume these are set to true
+	os.Setenv("KGW_ISTIOINTEGRATION", "true")
+	os.Setenv("KGW_ENABLEAUTOMTLS", "true")
+
 	testEnv := &envtest.Environment{
 		CRDDirectoryPaths: []string{
 			filepath.Join("..", "crds"),
@@ -213,19 +217,27 @@ func TestScenarios(t *testing.T) {
 				// that we get test pollution.
 				// once we change it to only include the ones in the proxy, we can re-enable this
 				//				t.Parallel()
-				testScenario(t, ctx, setupOpts.KrtDebugger, snapCache, client, xdsPort, fullpath)
+				testScenario(t, ctx, setupOpts.KrtDebugger, client, xdsPort, fullpath)
 
 			})
 		}
 	}
 }
 
-func testScenario(t *testing.T, ctx context.Context, kdbg *krt.DebugHandler,
-	snapCache cache.SnapshotCache, client istiokube.CLIClient, xdsPort int, f string) {
+func testScenario(
+	t *testing.T,
+	ctx context.Context,
+	kdbg *krt.DebugHandler,
+	client istiokube.CLIClient,
+	xdsPort int,
+	f string,
+) {
 	fext := filepath.Ext(f)
 	fpre := strings.TrimSuffix(f, fext)
-	fout := fpre + "-out" + fext
+	t.Logf("running scenario for test file: %s", f)
+
 	// read the out file
+	fout := fpre + "-out" + fext
 	write := false
 	ya, err := os.ReadFile(fout)
 	// if not exist
@@ -535,14 +547,16 @@ func (x *xdsDump) Compare(t *testing.T, other xdsDump) {
 	for _, c := range x.Clusters {
 		clusterset[c.Name] = c
 	}
-	for _, c := range other.Clusters {
-		otherc := clusterset[c.Name]
-		if otherc == nil {
-			t.Errorf("cluster %v not found", c.Name)
+	for _, otherc := range other.Clusters {
+		ourc := clusterset[otherc.Name]
+		if ourc == nil {
+			t.Errorf("cluster %v not found", otherc.Name)
 			continue
 		}
-		if !proto.Equal(c, otherc) {
-			t.Errorf("cluster %v not equal", c.Name)
+		if !proto.Equal(otherc, ourc) {
+			t.Errorf("cluster %v not equal", otherc.Name)
+			t.Errorf("got: %s", ourc.String())
+			t.Errorf("expected: %s", otherc.String())
 		}
 	}
 	listenerset := map[string]*envoylistener.Listener{}