From ed0cff7fbaa7132999d11ac397e8259274dde50a Mon Sep 17 00:00:00 2001
From: AliceProxy <alicewasko@datawire.io>
Date: Fri, 24 Jun 2022 12:23:58 -0700
Subject: [PATCH 01/11] updte release notees for 3.0

Signed-off-by: AliceProxy <alicewasko@datawire.io>
---
 CHANGELOG.md          | 10 +++++-----
 docs/releaseNotes.yml |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3cdb7dc91b..6a5467cb84 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -72,15 +72,15 @@ it will be removed; but as it won't be user-visible this isn't considered a brea
 
 ## RELEASE NOTES
 
-## [3.0.0] TBD
+## [3.0.0] June 27, 2022
 [3.0.0]: https://github.com/emissary-ingress/emissary/compare/v2.3.1...v3.0.0
 
 ### Emissary-ingress and Ambassador Edge Stack
 
-- Change: The envoy version included in Emissary-ingress has been upgraded from 1.17 to latest patch
-  release of 1.22. This provides $produceName$ with the latest security patches, performances
-  enhancments, and features offered by the envoy proxy. One notable change that will effect users is
-  the removal of support for V2 tranport protocol. See below for more information.
+- Change: The envoy version included in Emissary-ingress has been upgraded from 1.17 to the latest
+  patch release of 1.22. This provides Emissary-ingress with the latest security patches,
+  performances enhancments, and features offered by the envoy proxy. One notable change that will
+  effect users is the removal of support for V2 tranport protocol. See below for more information.
 
 - Change: Emissary-ingress can no longer be made to configure Envoy using the v2 xDS configuration
   API; it now always uses the v3 xDS API to configure Envoy.  This change should be mostly invisible
diff --git a/docs/releaseNotes.yml b/docs/releaseNotes.yml
index c99e33c442..f782c28fd0 100644
--- a/docs/releaseNotes.yml
+++ b/docs/releaseNotes.yml
@@ -33,13 +33,13 @@
 changelog: https://github.com/emissary-ingress/emissary/blob/$branch$/CHANGELOG.md
 items:
   - version: 3.0.0
-    date: 'TBD'
+    date: '2022-06-27'
     notes:
       - title: Envoy upgraded to 1.22
         type: change
         body: >-
-          The envoy version included in $productName$ has been upgraded from 1.17 to latest patch
-          release of 1.22. This provides $produceName$ with the latest security patches, performances enhancments,
+          The envoy version included in $productName$ has been upgraded from 1.17 to the latest patch
+          release of 1.22. This provides $productName$ with the latest security patches, performances enhancments,
           and features offered by the envoy proxy. One notable change that will effect users is the removal
           of support for V2 tranport protocol. See below for more information.
         docs: https://www.envoyproxy.io/docs/envoy/latest/version_history/v1.22/v1.22.0

From 4d28d13f49b8264a7e3b9141a40705c61d211c07 Mon Sep 17 00:00:00 2001
From: Luke Shumaker <lukeshu@datawire.io>
Date: Fri, 24 Jun 2022 14:26:25 -0600
Subject: [PATCH 02/11] cleanup: cmd/entrypoint: ID, not Id

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
 cmd/entrypoint/consul.go     | 2 +-
 cmd/entrypoint/endpoints.go  | 4 ++--
 cmd/entrypoint/entrypoint.go | 2 +-
 cmd/entrypoint/env.go        | 6 +++---
 cmd/entrypoint/helpers.go    | 2 +-
 cmd/entrypoint/secrets.go    | 2 +-
 cmd/entrypoint/snapshot.go   | 4 ++--
 cmd/entrypoint/watcher.go    | 2 +-
 8 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/cmd/entrypoint/consul.go b/cmd/entrypoint/consul.go
index ec03d66b97..22cb835927 100644
--- a/cmd/entrypoint/consul.go
+++ b/cmd/entrypoint/consul.go
@@ -325,7 +325,7 @@ func watchConsul(
 
 	w.Watch(func(endpoints consulwatch.Endpoints, e error) {
 		if endpoints.Id == "" {
-			// For Ambassador, overwrite the Id with the resolver's datacenter -- the
+			// For Ambassador, overwrite the ID with the resolver's datacenter -- the
 			// Consul watcher doesn't actually hand back the DC, and we need it.
 			endpoints.Id = resolver.Spec.Datacenter
 		}
diff --git a/cmd/entrypoint/endpoints.go b/cmd/entrypoint/endpoints.go
index acefde1682..b43d242db0 100644
--- a/cmd/entrypoint/endpoints.go
+++ b/cmd/entrypoint/endpoints.go
@@ -78,7 +78,7 @@ func (eri *endpointRoutingInfo) reconcileEndpointWatches(ctx context.Context, s
 			if _, isInvalid := a.(*kates.Unstructured); isInvalid {
 				continue
 			}
-			if include(GetAmbId(ctx, a)) {
+			if include(GetAmbID(ctx, a)) {
 				eri.checkResourcePhase1(ctx, a, "annotation")
 			}
 		}
@@ -128,7 +128,7 @@ func (eri *endpointRoutingInfo) reconcileEndpointWatches(ctx context.Context, s
 			if _, isInvalid := a.(*kates.Unstructured); isInvalid {
 				continue
 			}
-			if include(GetAmbId(ctx, a)) {
+			if include(GetAmbID(ctx, a)) {
 				eri.checkResourcePhase2(ctx, a, "annotation")
 			}
 		}
diff --git a/cmd/entrypoint/entrypoint.go b/cmd/entrypoint/entrypoint.go
index c303e8d69b..58d0029ac1 100644
--- a/cmd/entrypoint/entrypoint.go
+++ b/cmd/entrypoint/entrypoint.go
@@ -221,7 +221,7 @@ func Main(ctx context.Context, Version string, args ...string) error {
 }
 
 func clusterIDFromRootID(rootID string) string {
-	clusterUrl := fmt.Sprintf("d6e_id://%s/%s", rootID, GetAmbassadorId())
+	clusterUrl := fmt.Sprintf("d6e_id://%s/%s", rootID, GetAmbassadorID())
 	uid := uuid.NewSHA1(uuid.NameSpaceURL, []byte(clusterUrl))
 
 	return strings.ToLower(uid.String())
diff --git a/cmd/entrypoint/env.go b/cmd/entrypoint/env.go
index a62017ce2c..07f205fa21 100644
--- a/cmd/entrypoint/env.go
+++ b/cmd/entrypoint/env.go
@@ -19,7 +19,7 @@ func GetAgentService() string {
 	return ""
 }
 
-func GetAmbassadorId() string {
+func GetAmbassadorID() string {
 	id := os.Getenv("AMBASSADOR_ID")
 	if id != "" {
 		return id
@@ -67,7 +67,7 @@ func GetEnvoyBootstrapFile() string {
 	return env("ENVOY_BOOTSTRAP_FILE", path.Join(GetAmbassadorConfigBaseDir(), "bootstrap-ads.json"))
 }
 
-func GetEnvoyBaseId() string {
+func GetEnvoyBaseID() string {
 	return env("AMBASSADOR_ENVOY_BASE_ID", "0")
 }
 
@@ -149,7 +149,7 @@ func isDebug(name string) bool {
 }
 
 func GetEnvoyFlags() []string {
-	result := []string{"-c", GetEnvoyBootstrapFile(), "--base-id", GetEnvoyBaseId()}
+	result := []string{"-c", GetEnvoyBootstrapFile(), "--base-id", GetEnvoyBaseID()}
 	svc := GetAgentService()
 	if svc != "" {
 		result = append(result, "--drain-time-s", "1")
diff --git a/cmd/entrypoint/helpers.go b/cmd/entrypoint/helpers.go
index 12de3c4679..912ec2e0c0 100644
--- a/cmd/entrypoint/helpers.go
+++ b/cmd/entrypoint/helpers.go
@@ -78,7 +78,7 @@ func include(id amb.AmbassadorID) bool {
 
 	// It's not "_automatic_", so we have to actually do the work. Grab
 	// our AmbassadorID...
-	me := GetAmbassadorId()
+	me := GetAmbassadorID()
 
 	// ...force an empty AmbassadorID to "default", per the documentation...
 	if len(id) == 0 {
diff --git a/cmd/entrypoint/secrets.go b/cmd/entrypoint/secrets.go
index ce8ad8bc4f..d402ae53cc 100644
--- a/cmd/entrypoint/secrets.go
+++ b/cmd/entrypoint/secrets.go
@@ -190,7 +190,7 @@ func ReconcileSecrets(ctx context.Context, sh *SnapshotHolder) error {
 			if _, isInvalid := a.(*kates.Unstructured); isInvalid {
 				continue
 			}
-			if include(GetAmbId(ctx, a)) {
+			if include(GetAmbID(ctx, a)) {
 				resources = append(resources, a)
 			}
 		}
diff --git a/cmd/entrypoint/snapshot.go b/cmd/entrypoint/snapshot.go
index 7d172492fc..46791b9dae 100644
--- a/cmd/entrypoint/snapshot.go
+++ b/cmd/entrypoint/snapshot.go
@@ -19,8 +19,8 @@ func NewKubernetesSnapshot() *snapshotTypes.KubernetesSnapshot {
 	return a
 }
 
-// GetAmbId extracts the AmbassadorId from the kubernetes resource.
-func GetAmbId(ctx context.Context, resource kates.Object) amb.AmbassadorID {
+// GetAmbID extracts the AmbassadorID from the kubernetes resource.
+func GetAmbID(ctx context.Context, resource kates.Object) amb.AmbassadorID {
 	switch r := resource.(type) {
 	case *amb.Host:
 		var id amb.AmbassadorID
diff --git a/cmd/entrypoint/watcher.go b/cmd/entrypoint/watcher.go
index 9d0eefc654..aea4fb6b7e 100644
--- a/cmd/entrypoint/watcher.go
+++ b/cmd/entrypoint/watcher.go
@@ -44,7 +44,7 @@ func WatchAllTheThings(
 	interestingTypes := GetInterestingTypes(ctx, serverTypeList)
 	queries := GetQueries(ctx, interestingTypes)
 
-	ambassadorMeta := getAmbassadorMeta(GetAmbassadorId(), clusterID, version, client)
+	ambassadorMeta := getAmbassadorMeta(GetAmbassadorID(), clusterID, version, client)
 
 	// **** SETUP DONE for the Kubernetes Watcher
 

From 94266f202956049d9f9a7c5c1acaf822411d3262 Mon Sep 17 00:00:00 2001
From: Luke Shumaker <lukeshu@datawire.io>
Date: Fri, 24 Jun 2022 14:29:16 -0600
Subject: [PATCH 03/11] cleanup: cmd/entrypoint: Don't duplicate code from
 AmbassadorID.Matches

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
 cmd/entrypoint/consul.go                    | 12 ++++----
 cmd/entrypoint/endpoints.go                 | 18 ++++++------
 cmd/entrypoint/helpers.go                   | 31 ---------------------
 cmd/entrypoint/secrets.go                   | 10 ++++---
 pkg/api/getambassador.io/v3alpha1/common.go |  5 ++++
 5 files changed, 28 insertions(+), 48 deletions(-)

diff --git a/cmd/entrypoint/consul.go b/cmd/entrypoint/consul.go
index 22cb835927..11315596ad 100644
--- a/cmd/entrypoint/consul.go
+++ b/cmd/entrypoint/consul.go
@@ -20,16 +20,18 @@ type consulMapping struct {
 }
 
 func ReconcileConsul(ctx context.Context, consulWatcher *consulWatcher, s *snapshotTypes.KubernetesSnapshot) error {
+	envAmbID := GetAmbassadorID()
+
 	var mappings []consulMapping
 	for _, list := range s.Annotations {
 		for _, a := range list {
 			switch m := a.(type) {
 			case *amb.Mapping:
-				if include(m.Spec.AmbassadorID) {
+				if m.Spec.AmbassadorID.Matches(envAmbID) {
 					mappings = append(mappings, consulMapping{Service: m.Spec.Service, Resolver: m.Spec.Resolver})
 				}
 			case *amb.TCPMapping:
-				if include(m.Spec.AmbassadorID) {
+				if m.Spec.AmbassadorID.Matches(envAmbID) {
 					mappings = append(mappings, consulMapping{Service: m.Spec.Service, Resolver: m.Spec.Resolver})
 				}
 			}
@@ -38,19 +40,19 @@ func ReconcileConsul(ctx context.Context, consulWatcher *consulWatcher, s *snaps
 
 	var resolvers []*amb.ConsulResolver
 	for _, cr := range s.ConsulResolvers {
-		if include(cr.Spec.AmbassadorID) {
+		if cr.Spec.AmbassadorID.Matches(envAmbID) {
 			resolvers = append(resolvers, cr)
 		}
 	}
 
 	for _, m := range s.Mappings {
-		if include(m.Spec.AmbassadorID) {
+		if m.Spec.AmbassadorID.Matches(envAmbID) {
 			mappings = append(mappings, consulMapping{Service: m.Spec.Service, Resolver: m.Spec.Resolver})
 		}
 	}
 
 	for _, tm := range s.TCPMappings {
-		if include(tm.Spec.AmbassadorID) {
+		if tm.Spec.AmbassadorID.Matches(envAmbID) {
 			mappings = append(mappings, consulMapping{Service: tm.Spec.Service, Resolver: tm.Spec.Resolver})
 		}
 	}
diff --git a/cmd/entrypoint/endpoints.go b/cmd/entrypoint/endpoints.go
index b43d242db0..b20cf58b55 100644
--- a/cmd/entrypoint/endpoints.go
+++ b/cmd/entrypoint/endpoints.go
@@ -63,6 +63,8 @@ func newEndpointRoutingInfo() endpointRoutingInfo {
 }
 
 func (eri *endpointRoutingInfo) reconcileEndpointWatches(ctx context.Context, s *snapshotTypes.KubernetesSnapshot) {
+	envAmbID := GetAmbassadorID()
+
 	// Reset our state except for the previous endpoint watches. We keep them so we can detect if
 	// the set of things we are interested in has changed.
 	eri.resolverTypes = map[string]ResolverType{}
@@ -78,7 +80,7 @@ func (eri *endpointRoutingInfo) reconcileEndpointWatches(ctx context.Context, s
 			if _, isInvalid := a.(*kates.Unstructured); isInvalid {
 				continue
 			}
-			if include(GetAmbID(ctx, a)) {
+			if GetAmbID(ctx, a).Matches(envAmbID) {
 				eri.checkResourcePhase1(ctx, a, "annotation")
 			}
 		}
@@ -89,25 +91,25 @@ func (eri *endpointRoutingInfo) reconcileEndpointWatches(ctx context.Context, s
 	// need to test every resource, and no need to walk over things we're not
 	// interested in.
 	for _, m := range s.Modules {
-		if include(m.Spec.AmbassadorID) {
+		if m.Spec.AmbassadorID.Matches(envAmbID) {
 			eri.checkModule(ctx, m, "CRD")
 		}
 	}
 
 	for _, r := range s.KubernetesServiceResolvers {
-		if include(r.Spec.AmbassadorID) {
+		if r.Spec.AmbassadorID.Matches(envAmbID) {
 			eri.saveResolver(ctx, r.GetName(), KubernetesServiceResolver, "CRD")
 		}
 	}
 
 	for _, r := range s.KubernetesEndpointResolvers {
-		if include(r.Spec.AmbassadorID) {
+		if r.Spec.AmbassadorID.Matches(envAmbID) {
 			eri.saveResolver(ctx, r.GetName(), KubernetesEndpointResolver, "CRD")
 		}
 	}
 
 	for _, r := range s.ConsulResolvers {
-		if include(r.Spec.AmbassadorID) {
+		if r.Spec.AmbassadorID.Matches(envAmbID) {
 			eri.saveResolver(ctx, r.GetName(), ConsulResolver, "CRD")
 		}
 	}
@@ -128,20 +130,20 @@ func (eri *endpointRoutingInfo) reconcileEndpointWatches(ctx context.Context, s
 			if _, isInvalid := a.(*kates.Unstructured); isInvalid {
 				continue
 			}
-			if include(GetAmbID(ctx, a)) {
+			if GetAmbID(ctx, a).Matches(envAmbID) {
 				eri.checkResourcePhase2(ctx, a, "annotation")
 			}
 		}
 	}
 
 	for _, m := range s.Mappings {
-		if include(m.Spec.AmbassadorID) {
+		if m.Spec.AmbassadorID.Matches(envAmbID) {
 			eri.checkMapping(ctx, m, "CRD")
 		}
 	}
 
 	for _, t := range s.TCPMappings {
-		if include(t.Spec.AmbassadorID) {
+		if t.Spec.AmbassadorID.Matches(envAmbID) {
 			eri.checkTCPMapping(ctx, t, "CRD")
 		}
 	}
diff --git a/cmd/entrypoint/helpers.go b/cmd/entrypoint/helpers.go
index 912ec2e0c0..f9944996b0 100644
--- a/cmd/entrypoint/helpers.go
+++ b/cmd/entrypoint/helpers.go
@@ -7,7 +7,6 @@ import (
 	"strings"
 
 	"github.com/datawire/dlib/dexec"
-	amb "github.com/emissary-ingress/emissary/v3/pkg/api/getambassador.io/v3alpha1"
 )
 
 func envbool(name string) bool {
@@ -64,33 +63,3 @@ func convert(in interface{}, out interface{}) error {
 
 	return nil
 }
-
-// Should we pay attention to a given AmbassadorID set?
-//
-// XXX Yes, amb.AmbassadorID is a singular name for a plural type. Sigh.
-func include(id amb.AmbassadorID) bool {
-	// We always pay attention to the "_automatic_" ID -- it gives us a
-	// to easily always include certain configuration resources for Edge
-	// Stack.
-	if len(id) == 1 && id[0] == "_automatic_" {
-		return true
-	}
-
-	// It's not "_automatic_", so we have to actually do the work. Grab
-	// our AmbassadorID...
-	me := GetAmbassadorID()
-
-	// ...force an empty AmbassadorID to "default", per the documentation...
-	if len(id) == 0 {
-		id = amb.AmbassadorID{"default"}
-	}
-
-	// ...and then see if our AmbassadorID is in the list.
-	for _, name := range id {
-		if me == name {
-			return true
-		}
-	}
-
-	return false
-}
diff --git a/cmd/entrypoint/secrets.go b/cmd/entrypoint/secrets.go
index d402ae53cc..0e835974c4 100644
--- a/cmd/entrypoint/secrets.go
+++ b/cmd/entrypoint/secrets.go
@@ -175,6 +175,8 @@ func checkSecret(
 // since we don't want to send secrets to Ambassador unless we're
 // using them, since any secret we send will be saved to disk.
 func ReconcileSecrets(ctx context.Context, sh *SnapshotHolder) error {
+	envAmbID := GetAmbassadorID()
+
 	// Start by building up a list of all the K8s objects that are
 	// allowed to mention secrets. Note that we vet the ambassador_id
 	// for all of these before putting them on the list.
@@ -190,7 +192,7 @@ func ReconcileSecrets(ctx context.Context, sh *SnapshotHolder) error {
 			if _, isInvalid := a.(*kates.Unstructured); isInvalid {
 				continue
 			}
-			if include(GetAmbID(ctx, a)) {
+			if GetAmbID(ctx, a).Matches(envAmbID) {
 				resources = append(resources, a)
 			}
 		}
@@ -203,19 +205,19 @@ func ReconcileSecrets(ctx context.Context, sh *SnapshotHolder) error {
 		if len(h.Spec.AmbassadorID) > 0 {
 			id = h.Spec.AmbassadorID
 		}
-		if include(id) {
+		if id.Matches(envAmbID) {
 			resources = append(resources, h)
 		}
 	}
 
 	// TLSContexts, Modules, and Ingresses are all straightforward.
 	for _, t := range sh.k8sSnapshot.TLSContexts {
-		if include(t.Spec.AmbassadorID) {
+		if t.Spec.AmbassadorID.Matches(envAmbID) {
 			resources = append(resources, t)
 		}
 	}
 	for _, m := range sh.k8sSnapshot.Modules {
-		if include(m.Spec.AmbassadorID) {
+		if m.Spec.AmbassadorID.Matches(envAmbID) {
 			resources = append(resources, m)
 		}
 	}
diff --git a/pkg/api/getambassador.io/v3alpha1/common.go b/pkg/api/getambassador.io/v3alpha1/common.go
index fef03eac2f..ef40afd824 100644
--- a/pkg/api/getambassador.io/v3alpha1/common.go
+++ b/pkg/api/getambassador.io/v3alpha1/common.go
@@ -130,6 +130,11 @@ func (aid AmbassadorID) Matches(envVar string) bool {
 		if item == envVar {
 			return true
 		}
+		if item == "_automatic_" {
+			// We always pay attention to the "_automatic_" ID -- it gives us a to
+			// easily always include certain configuration resources for Edge Stack.
+			return true
+		}
 	}
 	return false
 }

From d6db89bbd4fcf2405694a8e19c57cfd0c1571855 Mon Sep 17 00:00:00 2001
From: Luke Shumaker <lukeshu@datawire.io>
Date: Fri, 24 Jun 2022 14:43:46 -0600
Subject: [PATCH 04/11] cleanup: cmd/entrypoint: Begone with the
 AMBASSADOR_DEMO_MODE env-var

The only thing that used it was syntheticauth, but syntheticauth only runs
from inside of WatchAllTheThings(), which we only call if !demoMode.

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
 cmd/entrypoint/entrypoint.go    | 4 ----
 cmd/entrypoint/syntheticauth.go | 4 ----
 2 files changed, 8 deletions(-)

diff --git a/cmd/entrypoint/entrypoint.go b/cmd/entrypoint/entrypoint.go
index 58d0029ac1..ee58604a5f 100644
--- a/cmd/entrypoint/entrypoint.go
+++ b/cmd/entrypoint/entrypoint.go
@@ -77,8 +77,6 @@ import (
 // manager (e.g. kubernetes) is expected to take note and restart if
 // appropriate.
 
-const envAmbassadorDemoMode string = "AMBASSADOR_DEMO_MODE"
-
 func Main(ctx context.Context, Version string, args ...string) error {
 	// Setup logging according to AES_LOG_LEVEL
 	busy.SetLogLevel(logutil.DefaultLogLevel)
@@ -105,8 +103,6 @@ func Main(ctx context.Context, Version string, args ...string) error {
 		// Demo mode!
 		dlog.Infof(ctx, "DEMO MODE")
 		demoMode = true
-		// Set an environment variable so that other parts of the code can check if demo mode is active (mainly used for disabling synthetic authservice injection)
-		os.Setenv(envAmbassadorDemoMode, "true")
 	}
 
 	clusterID := GetClusterID(ctx)
diff --git a/cmd/entrypoint/syntheticauth.go b/cmd/entrypoint/syntheticauth.go
index 35b6b71db5..2e4c0f9051 100644
--- a/cmd/entrypoint/syntheticauth.go
+++ b/cmd/entrypoint/syntheticauth.go
@@ -42,10 +42,6 @@ func ReconcileAuthServices(ctx context.Context, sh *SnapshotHolder, deltas *[]*k
 	} else if !isEdgeStack {
 		return nil
 	}
-	// We also dont want to do anything with AuthServices if the Docker demo mode is running
-	if envbool("AMBASSADOR_DEMO_MODE") {
-		return nil
-	}
 
 	// Construct a synthetic AuthService to be injected if we dont find any valid AuthServices
 	injectSyntheticAuth := true

From 74e42a4d41229018fc58962bea909dc82aa41d46 Mon Sep 17 00:00:00 2001
From: Luke Shumaker <lukeshu@datawire.io>
Date: Fri, 24 Jun 2022 21:51:31 -0600
Subject: [PATCH 05/11] cleanup: cmd/entrypoint: Don't bother manually cleaning
 up testing.T.Setenv

The point of testing.T.Setenv over os.Setenv is that the test framework
cleans it up for you.

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
 .../testutil_fake_filtersecrets_test.go       |  2 --
 .../testutil_fake_syntheticauth_test.go       | 24 -------------------
 2 files changed, 26 deletions(-)

diff --git a/cmd/entrypoint/testutil_fake_filtersecrets_test.go b/cmd/entrypoint/testutil_fake_filtersecrets_test.go
index ffe85de16e..8a78d27117 100644
--- a/cmd/entrypoint/testutil_fake_filtersecrets_test.go
+++ b/cmd/entrypoint/testutil_fake_filtersecrets_test.go
@@ -67,7 +67,6 @@ type: Opaque
 			snap, err := f.GetSnapshot(hasSecret("namespaced-secret", "bar"))
 			require.NoError(t, err)
 			assert.NotNil(t, snap)
-			t.Setenv("EDGE_STACK", "")
 		})
 	}
 }
@@ -112,7 +111,6 @@ type: Opaque
 			snap, err := f.GetSnapshot(hasSecret("namespaced-secret", "foo"))
 			require.NoError(t, err)
 			assert.NotNil(t, snap)
-			t.Setenv("EDGE_STACK", "")
 		})
 	}
 }
diff --git a/cmd/entrypoint/testutil_fake_syntheticauth_test.go b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
index b2353f21bb..8227909874 100644
--- a/cmd/entrypoint/testutil_fake_syntheticauth_test.go
+++ b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
@@ -72,8 +72,6 @@ spec:
 
 	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
 	assert.NotNil(t, envoyConfig)
-
-	t.Setenv("EDGE_STACK", "")
 }
 
 // Tests the synthetic auth generation when a valid AuthService is created as a getambassador.io/v2 resource
@@ -123,8 +121,6 @@ spec:
 
 	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
 	assert.NotNil(t, envoyConfig)
-
-	t.Setenv("EDGE_STACK", "")
 }
 
 // This tests with a provided AuthService that has no protocol_version (which defaults to v2)
@@ -174,8 +170,6 @@ spec:
 
 	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
 	assert.NotNil(t, envoyConfig)
-
-	t.Setenv("EDGE_STACK", "")
 }
 
 // This tests with a provided AuthService that has no protocol_version (which defaults to v2)
@@ -225,8 +219,6 @@ spec:
 
 	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
 	assert.NotNil(t, envoyConfig)
-
-	t.Setenv("EDGE_STACK", "")
 }
 
 // Tests the synthetic auth generation when an invalid AuthService is created as a getambassador.io/v2 resource
@@ -278,8 +270,6 @@ spec:
 
 	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
 	assert.NotNil(t, envoyConfig)
-
-	t.Setenv("EDGE_STACK", "")
 }
 
 // Tests the synthetic auth generation when an invalid AuthService is created as a getambassador.io/v2 resource
@@ -330,9 +320,6 @@ spec:
 
 	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
 	assert.NotNil(t, envoyConfig)
-
-	t.Setenv("EDGE_STACK", "")
-
 }
 
 // Tests the synthetic auth generation when an invalid AuthService (because the protocol_version is invalid for the supported enums)
@@ -383,8 +370,6 @@ spec:
 
 	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
 	assert.NotNil(t, envoyConfig)
-
-	t.Setenv("EDGE_STACK", "")
 }
 
 // Tests the synthetic auth generation when an invalid AuthService is created and edited several times in succession.
@@ -474,8 +459,6 @@ spec:
 
 	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
 	assert.NotNil(t, envoyConfig)
-
-	t.Setenv("EDGE_STACK", "")
 }
 
 // Tests the synthetic auth generation by first creating an invalid AuthService and confirming that the synthetic AuthService gets injected.
@@ -570,9 +553,6 @@ spec:
 
 	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
 	assert.NotNil(t, envoyConfig)
-
-	t.Setenv("EDGE_STACK", "")
-
 }
 
 // This AuthService points at 127.0.0.1:8500, but it does not have protocol_version: v3. It also has additional fields set.
@@ -631,8 +611,6 @@ spec:
 
 	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
 	assert.NotNil(t, envoyConfig)
-
-	t.Setenv("EDGE_STACK", "")
 }
 
 // This AuthService does not point at 127.0.0.1:8500, we leave it alone rather than adding a synthetic one
@@ -686,8 +664,6 @@ spec:
 
 	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
 	assert.NotNil(t, envoyConfig)
-
-	t.Setenv("EDGE_STACK", "")
 }
 
 // When deciding if we need to inject a synthetic AuthService or not, we need to be able to reliably determine if that

From 5ebabc2337392c6337ef6077ac36730b2cb0efd2 Mon Sep 17 00:00:00 2001
From: Luke Shumaker <lukeshu@datawire.io>
Date: Fri, 24 Jun 2022 22:08:31 -0600
Subject: [PATCH 06/11] cleanup:
 cmd/entrypoint/testutil_fake_syntheticauth_test.go: Fuss with comments

Wrap them per .editorconfig:max_line_length.  Fuss with punctuation and
capitalization.

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
 .../testutil_fake_syntheticauth_test.go       | 248 ++++++++++--------
 1 file changed, 142 insertions(+), 106 deletions(-)

diff --git a/cmd/entrypoint/testutil_fake_syntheticauth_test.go b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
index 8227909874..8f6c25bc6a 100644
--- a/cmd/entrypoint/testutil_fake_syntheticauth_test.go
+++ b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
@@ -13,7 +13,8 @@ import (
 	"github.com/emissary-ingress/emissary/v3/pkg/snapshot/v1"
 )
 
-// This predicate is used to check k8s snapshots for an AuthService matching the provided name and namespace
+// This predicate is used to check k8s snapshots for an AuthService matching the provided name and
+// namespace.
 func HasAuthService(namespace, name string) func(snapshot *snapshot.Snapshot) bool {
 	return func(snapshot *snapshot.Snapshot) bool {
 		for _, m := range snapshot.Kubernetes.AuthServices {
@@ -25,8 +26,8 @@ func HasAuthService(namespace, name string) func(snapshot *snapshot.Snapshot) bo
 	}
 }
 
-// Tests the synthetic auth generation when a valid AuthService is created
-// This authservice has protocol_Version: v3 and should not be replaced by the synthetic AuthService
+// Tests the synthetic auth generation when a valid AuthService is created.  This AuthService has
+// `protocol_version: v3` and should not be replaced by the synthetic AuthService.
 func TestSyntheticAuthValid(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
@@ -47,9 +48,9 @@ spec:
 	assert.NoError(t, err)
 	f.Flush()
 
-	// Use the predicate above to check that the snapshot contains the AuthService defined above
-	// The AuthService has protocol_Version: v3 so it should not be removed/replaced by the synthetic AuthService
-	// injected by syntheticauth.go
+	// Use the predicate above to check that the snapshot contains the AuthService defined
+	// above.  The AuthService has `protocol_version: v3` so it should not be removed/replaced
+	// by the synthetic AuthService injected by syntheticauth.go
 	snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
@@ -58,8 +59,9 @@ spec:
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are harder to check since they always have the same name)
-	// the namespace for this extauthz cluster should be foo (since that is the namespace of the valid AuthService above)
+	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
+	// harder to check since they always have the same name).  The namespace for this extauthz
+	// cluster should be foo (since that is the namespace of the valid AuthService above).
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
 	}
@@ -70,12 +72,14 @@ spec:
 	})
 	require.NoError(t, err)
 
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
+	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
+	// defined.
 	assert.NotNil(t, envoyConfig)
 }
 
-// Tests the synthetic auth generation when a valid AuthService is created as a getambassador.io/v2 resource
-// This authservice has protocol_Version: v3 and should not be replaced by the synthetic AuthService
+// Tests the synthetic auth generation when a valid AuthService is created as a getambassador.io/v2
+// resource.  This AuthService has `protocol_version: v3` and should not be replaced by the
+// synthetic AuthService.
 func TestSyntheticAuthValidV2(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
@@ -96,9 +100,9 @@ spec:
 	assert.NoError(t, err)
 	f.Flush()
 
-	// Use the predicate above to check that the snapshot contains the AuthService defined above
-	// The AuthService has protocol_Version: v3 so it should not be removed/replaced by the synthetic AuthService
-	// injected by syntheticauth.go
+	// Use the predicate above to check that the snapshot contains the AuthService defined
+	// above.  The AuthService has `protocol_version: v3` so it should not be removed/replaced
+	// by the synthetic AuthService injected by syntheticauth.go
 	snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
@@ -107,8 +111,9 @@ spec:
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are harder to check since they always have the same name)
-	// the namespace for this extauthz cluster should be foo (since that is the namespace of the valid AuthService above)
+	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
+	// harder to check since they always have the same name).  The namespace for this extauthz
+	// cluster should be foo (since that is the namespace of the valid AuthService above).
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
 	}
@@ -119,12 +124,13 @@ spec:
 	})
 	require.NoError(t, err)
 
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
+	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
+	// defined.
 	assert.NotNil(t, envoyConfig)
 }
 
-// This tests with a provided AuthService that has no protocol_version (which defaults to v2)
-// The synthetic AuthService should be created instead
+// This tests with a provided AuthService that has no protocol_version (which defaults to v2).  The
+// synthetic AuthService should be created instead.
 func TestSyntheticAuthReplace(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
@@ -144,20 +150,21 @@ spec:
 	assert.NoError(t, err)
 	f.Flush()
 
-	// Use the predicate above to check that the snapshot contains the AuthService defined above
-	// The AuthService does not have protocol_Version: v3 so it should be removed and replaced by the synthetic AuthService
-	// injected by syntheticauth.go
+	// Use the predicate above to check that the snapshot contains the AuthService defined
+	// above.  The AuthService does not have `protocol_version: v3` so it should be removed and
+	// replaced by the synthetic AuthService injected by syntheticauth.go
 	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
-	// The snapshot should only have the synthetic AuthService and not the one defined above
+	// The snapshot should only have the synthetic AuthService and not the one defined above.
 	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are harder to check since they always have the same name)
-	// the namespace for this extauthz cluster should be default (since that is the namespace of the synthetic AuthService)
+	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
+	// harder to check since they always have the same name).  The namespace for this extauthz
+	// cluster should be default (since that is the namespace of the synthetic AuthService).
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
 	}
@@ -168,12 +175,13 @@ spec:
 	})
 	require.NoError(t, err)
 
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
+	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
+	// defined.
 	assert.NotNil(t, envoyConfig)
 }
 
-// This tests with a provided AuthService that has no protocol_version (which defaults to v2)
-// The synthetic AuthService should be created instead
+// This tests with a provided AuthService that has no protocol_version (which defaults to v2).  The
+// synthetic AuthService should be created instead.
 func TestSyntheticAuthReplaceV2(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
@@ -193,20 +201,21 @@ spec:
 	assert.NoError(t, err)
 	f.Flush()
 
-	// Use the predicate above to check that the snapshot contains the AuthService defined above
-	// The AuthService does not have protocol_Version: v3 so it should be removed and replaced by the synthetic AuthService
-	// injected by syntheticauth.go
+	// Use the predicate above to check that the snapshot contains the AuthService defined
+	// above.  The AuthService does not have `protocol_version: v3` so it should be removed and
+	// replaced by the synthetic AuthService injected by syntheticauth.go
 	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
-	// The snapshot should only have the synthetic AuthService and not the one defined above
+	// The snapshot should only have the synthetic AuthService and not the one defined above.
 	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are harder to check since they always have the same name)
-	// the namespace for this extauthz cluster should be default (since that is the namespace of the synthetic AuthService)
+	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
+	// harder to check since they always have the same name).  The namespace for this extauthz
+	// cluster should be default (since that is the namespace of the synthetic AuthService).
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
 	}
@@ -217,13 +226,15 @@ spec:
 	})
 	require.NoError(t, err)
 
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
+	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
+	// defined.
 	assert.NotNil(t, envoyConfig)
 }
 
-// Tests the synthetic auth generation when an invalid AuthService is created as a getambassador.io/v2 resource
-// This authservice has protocol_Version: v3 and should not be replaced by the synthetic AuthService even though it has a bogus value
-// because the bogus field will be dropped when it is loaded and we will be left with a Valid AuthService
+// Tests the synthetic auth generation when an invalid AuthService is created.  This AuthService has
+// `protocol_version: v3` and should not be replaced by the synthetic AuthService even though it has
+// a bogus value because the bogus field will be dropped when it is loaded and we will be left with
+// a valid AuthService.
 func TestSyntheticAuthBogusField(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
@@ -245,9 +256,9 @@ spec:
 	assert.NoError(t, err)
 	f.Flush()
 
-	// Use the predicate above to check that the snapshot contains the AuthService defined above
-	// The AuthService has protocol_Version: v3 so it should not be removed/replaced by the synthetic AuthService
-	// injected by syntheticauth.go
+	// Use the predicate above to check that the snapshot contains the AuthService defined
+	// above.  The AuthService has `protocol_version: v3` so it should not be removed/replaced by
+	// the synthetic AuthService injected by syntheticauth.go
 	snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
@@ -256,8 +267,9 @@ spec:
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are harder to check since they always have the same name)
-	// the namespace for this extauthz cluster should be foo (since that is the namespace of the valid AuthService above)
+	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
+	// harder to check since they always have the same name).  The namespace for this extauthz
+	// cluster should be foo (since that is the namespace of the valid AuthService above).
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
 	}
@@ -268,12 +280,14 @@ spec:
 	})
 	require.NoError(t, err)
 
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
+	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
+	// defined.
 	assert.NotNil(t, envoyConfig)
 }
 
-// Tests the synthetic auth generation when an invalid AuthService is created as a getambassador.io/v2 resource
-// This authservice has protocol_Version: v3 and should be replaced by the synthetic AuthService because it contains a bogus field and is not valid.
+// Tests the synthetic auth generation when an invalid AuthService is created as a
+// getambassador.io/v2 resource.  This AuthService has `protocol_version: v3` and should be replaced
+// by the synthetic AuthService because it contains a bogus field and is not valid.
 func TestSyntheticAuthBogusFieldV2(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
@@ -295,9 +309,9 @@ spec:
 	assert.NoError(t, err)
 	f.Flush()
 
-	// Use the predicate above to check that the snapshot contains the AuthService defined above
-	// The AuthService has protocol_Version: v3 so it should not be removed/replaced by the synthetic AuthService
-	// injected by syntheticauth.go
+	// Use the predicate above to check that the snapshot contains the AuthService defined
+	// above.  The AuthService has `protocol_version: v3` so it should not be removed/replaced
+	// by the synthetic AuthService injected by syntheticauth.go
 	snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
@@ -306,8 +320,9 @@ spec:
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are harder to check since they always have the same name)
-	// the namespace for this extauthz cluster should be foo (since that is the namespace of the valid AuthService above)
+	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
+	// harder to check since they always have the same name).  The namespace for this extauthz
+	// cluster should be foo (since that is the namespace of the valid AuthService above).
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
 	}
@@ -318,12 +333,14 @@ spec:
 	})
 	require.NoError(t, err)
 
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
+	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
+	// defined.
 	assert.NotNil(t, envoyConfig)
 }
 
-// Tests the synthetic auth generation when an invalid AuthService (because the protocol_version is invalid for the supported enums)
-// This AuthService should be tossed out an the synthetic AuthService should be injected
+// Tests the synthetic auth generation when an invalid AuthService (because the protocol_version is
+// invalid for the supported enums).  This AuthService should be tossed out an the synthetic
+// AuthService should be injected.
 func TestSyntheticAuthInvalidProtocolVer(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
@@ -345,19 +362,21 @@ spec:
 	assert.NoError(t, err)
 	f.Flush()
 
-	// Use the predicate above to check that the snapshot contains the Synthetic AuthService
-	// The AuthService has protocol_Version: v3, but it has a bogus field so it should not be validated and instead we inject the synthetic authservice
+	// Use the predicate above to check that the snapshot contains the synthetic AuthService.
+	// The AuthService has `protocol_version: v3`, but it has a bogus field so it should not be
+	// validated and instead we inject the synthetic AuthService.
 	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
-	// The snapshot should only have the synthetic AuthService and not the one defined above
+	// The snapshot should only have the synthetic AuthService and not the one defined above.
 	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
-	// Check for an ext_authz cluster name matching the synthetic AuthService.
-	// the namespace for this extauthz cluster should be default (since that is the namespace of the synthetic AuthService)
+	// Check for an ext_authz cluster name matching the synthetic AuthService.  The namespace
+	// for this extauthz cluster should be default (since that is the namespace of the synthetic
+	// AuthService).
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
 	}
@@ -368,14 +387,16 @@ spec:
 	})
 	require.NoError(t, err)
 
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
+	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
+	// defined.
 	assert.NotNil(t, envoyConfig)
 }
 
-// Tests the synthetic auth generation when an invalid AuthService is created and edited several times in succession.
-// After the config is edited several times, we should see that the final result is our provided valid AuthService.
-// There should not be any duplicate AuthService resources, and the synthetic AuthService that gets created when the first
-// Invalid AuthService is applied should be removed when the final edit makes it a valid AuthService.
+// Tests the synthetic auth generation when an invalid AuthService is created and edited several
+// times in succession.  After the config is edited several times, we should see that the final
+// result is our provided valid AuthService.  There should not be any duplicate AuthService
+// resources, and the synthetic AuthService that gets created when the first invalid AuthService is
+// applied should be removed when the final edit makes it a valid AuthService.
 func TestSyntheticAuthChurn(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
@@ -433,20 +454,21 @@ spec:
 `)
 	assert.NoError(t, err)
 
-	// Use the predicate above to check that the snapshot contains the AuthService defined above
-	// The AuthService has protocol_Version: v3 so it should not be removed/replaced by the synthetic AuthService
-	// injected by syntheticauth.go
+	// Use the predicate above to check that the snapshot contains the AuthService defined
+	// above.  The AuthService has `protocol_version: v3` so it should not be removed/replaced
+	// by the synthetic AuthService injected by syntheticauth.go
 	snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
-	// The snapshot should only have the synthetic AuthService and not the one defined above
+	// The snapshot should only have the synthetic AuthService and not the one defined above.
 	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are harder to check since they always have the same name)
-	// the namespace for this extauthz cluster should be foo (since that is the namespace of the valid AuthService above)
+	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
+	// harder to check since they always have the same name).  The namespace for this extauthz
+	// cluster should be foo (since that is the namespace of the valid AuthService above)
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
 	}
@@ -461,15 +483,16 @@ spec:
 	assert.NotNil(t, envoyConfig)
 }
 
-// Tests the synthetic auth generation by first creating an invalid AuthService and confirming that the synthetic AuthService gets injected.
-// Afterwards, a valid AuthService is applied and we expect the synthetic AuthService to be removed in favor of the new valid AuthService.
+// Tests the synthetic auth generation by first creating an invalid AuthService and confirming that
+// the synthetic AuthService gets injected.  Afterwards, a valid AuthService is applied and we
+// expect the synthetic AuthService to be removed in favor of the new valid AuthService.
 func TestSyntheticAuthInjectAndRemove(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
 	f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
 	f.AutoFlush(true)
 
-	// This will cause a synthethic authservice to be injected
+	// This will cause a synthethic AuthService to be injected.
 	err := f.UpsertYAML(`
 ---
 apiVersion: getambassador.io/v3alpha1
@@ -484,19 +507,21 @@ spec:
 `)
 	assert.NoError(t, err)
 
-	// Use the predicate above to check that the snapshot contains the Synthetic AuthService
-	// The AuthService has protocol_Version: v3, but it has a bogus field so it should not be validated and instead we inject the synthetic authservice
+	// Use the predicate above to check that the snapshot contains the synthetic AuthService.
+	// The AuthService has `protocol_version: v3`, but it has a bogus field so it should not be
+	// validated and instead we inject the synthetic AuthService.
 	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
-	// The snapshot should only have the synthetic AuthService and not the one defined above
+	// The snapshot should only have the synthetic AuthService and not the one defined above.
 	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
 	// We should only have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
-	// Check for an ext_authz cluster name matching the synthetic AuthService.
-	// the namespace for this extauthz cluster should be default (since that is the namespace of the synthetic AuthService)
+	// Check for an ext_authz cluster name matching the synthetic AuthService.  The namespace
+	// for this extauthz cluster should be default (since that is the namespace of the synthetic
+	// AuthService).
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
 	}
@@ -507,13 +532,15 @@ spec:
 	})
 	require.NoError(t, err)
 
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
+	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
+	// defined.
 	assert.NotNil(t, envoyConfig)
 
 	t.Setenv("EDGE_STACK", "")
 
-	// Updating the yaml for that AuthService to include protocol_version: v3 should make it valid and then
-	// Remove our synthetic AuthService and allow the now valid AuthService to be used.
+	// Updating the yaml for that AuthService to include `protocol_version: v3` should make it
+	// valid and then remove our synthetic AuthService and allow the now valid AuthService to be
+	// used.
 	err = f.UpsertYAML(`
 ---
 apiVersion: getambassador.io/v3alpha1
@@ -528,9 +555,9 @@ spec:
 `)
 	assert.NoError(t, err)
 
-	// Use the predicate above to check that the snapshot contains the AuthService defined above
-	// The AuthService has protocol_Version: v3 so it should not be removed/replaced by the synthetic AuthService
-	// injected by syntheticauth.go
+	// Use the predicate above to check that the snapshot contains the AuthService defined
+	// above.  The AuthService has `protocol_version: v3` so it should not be removed/replaced
+	// by the synthetic AuthService injected by syntheticauth.go
 	snap, err = f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
@@ -539,8 +566,9 @@ spec:
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are harder to check since they always have the same name)
-	// the namespace for this extauthz cluster should be foo (since that is the namespace of the valid AuthService above)
+	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
+	// harder to check since they always have the same name).  The namespace for this extauthz
+	// cluster should be foo (since that is the namespace of the valid AuthService above).
 	isAuthCluster = func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
 	}
@@ -551,12 +579,14 @@ spec:
 	})
 	require.NoError(t, err)
 
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
+	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
+	// defined.
 	assert.NotNil(t, envoyConfig)
 }
 
-// This AuthService points at 127.0.0.1:8500, but it does not have protocol_version: v3. It also has additional fields set.
-// The correct action is to create a SyntheticAuth copy of this AuthService with the same fields but with protocol_version: v3
+// This AuthService points at 127.0.0.1:8500, but it does not have `protocol_version: v3`.  It also
+// has additional fields set.  The correct action is to create a SyntheticAuth copy of this
+// AuthService with the same fields but with `protocol_version: v3`.
 func TestSyntheticAuthCopyFields(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
@@ -578,10 +608,10 @@ spec:
 	assert.NoError(t, err)
 	f.Flush()
 
-	// Use the predicate above to check that the snapshot contains the Synthetic AuthService
-	// The AuthService has protocol_Version: v3, but it is missing the protocol_version: v3 field.
-	// We expect the synthetic AuthService to be injected, but later we will check that the synthetic AuthService has
-	// Our custom timeout_ms field
+	// Use the predicate above to check that the snapshot contains the synthetic AuthService.
+	// The AuthService has `protocol_version: v3`, but it is missing the `protocol_version: v3`
+	// field.  We expect the synthetic AuthService to be injected, but later we will check that
+	// the synthetic AuthService has Our custom timeout_ms field.
 	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
@@ -591,14 +621,16 @@ spec:
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
-	// Even though it is the synthetic AuthService, we should have the custom timeout_ms and v3 protocol version
+	// Even though it is the synthetic AuthService, we should have the custom timeout_ms and v3
+	// protocol version.
 	for _, authService := range snap.Kubernetes.AuthServices {
 		assert.Equal(t, int64(12345), authService.Spec.Timeout.Duration.Milliseconds())
 		assert.Equal(t, "v3", authService.Spec.ProtocolVersion)
 	}
 
-	// Check for an ext_authz cluster name matching the synthetic AuthService.
-	// the namespace for this extauthz cluster should be default (since that is the namespace of the synthetic AuthService)
+	// Check for an ext_authz cluster name matching the synthetic AuthService.  The namespace
+	// for this extauthz cluster should be default (since that is the namespace of the synthetic
+	// AuthService).
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
 	}
@@ -609,11 +641,13 @@ spec:
 	})
 	require.NoError(t, err)
 
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
+	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
+	// defined.
 	assert.NotNil(t, envoyConfig)
 }
 
-// This AuthService does not point at 127.0.0.1:8500, we leave it alone rather than adding a synthetic one
+// This AuthService does not point at 127.0.0.1:8500, we leave it alone rather than adding a
+// synthetic one.
 func TestSyntheticAuthCustomAuthService(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
@@ -635,9 +669,9 @@ spec:
 	assert.NoError(t, err)
 	f.Flush()
 
-	// Use the predicate above to check that the snapshot contains the AuthService defined above
-	// The AuthService has protocol_Version: v3 so it should not be removed/replaced by the synthetic AuthService
-	// injected by syntheticauth.go
+	// Use the predicate above to check that the snapshot contains the AuthService defined
+	// above.  The AuthService has `protocol_version: v3` so it should not be removed/replaced
+	// by the synthetic AuthService injected by syntheticauth.go
 	snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
@@ -650,8 +684,9 @@ spec:
 		assert.Equal(t, "dummy-service", authService.Spec.AuthService)
 	}
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are harder to check since they always have the same name)
-	// the namespace for this extauthz cluster should be foo (since that is the namespace of the valid AuthService above)
+	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
+	// harder to check since they always have the same name).  the namespace for this extauthz
+	// cluster should be foo (since that is the namespace of the valid AuthService above).
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
 		return strings.Contains(c.Name, "cluster_extauth_dummy_service_foo")
 	}
@@ -662,12 +697,13 @@ spec:
 	})
 	require.NoError(t, err)
 
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was defined
+	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
+	// defined.
 	assert.NotNil(t, envoyConfig)
 }
 
-// When deciding if we need to inject a synthetic AuthService or not, we need to be able to reliably determine if that
-// AuthService points at a localhost:8500 or not
+// When deciding if we need to inject a synthetic AuthService or not, we need to be able to reliably
+// determine if that AuthService points at a localhost:8500 or not.
 func TestIsLocalhost8500(t *testing.T) {
 	t.Parallel()
 

From 7293a53a81544f3cae8e34bdcdacffba9da85150 Mon Sep 17 00:00:00 2001
From: Luke Shumaker <lukeshu@datawire.io>
Date: Mon, 27 Jun 2022 10:31:16 -0600
Subject: [PATCH 07/11] cleanup: cmd/entrypoint: Indent some tests in prep for
 the next commit

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
 .../testutil_fake_syntheticauth_test.go       | 408 ++++++++++--------
 1 file changed, 226 insertions(+), 182 deletions(-)

diff --git a/cmd/entrypoint/testutil_fake_syntheticauth_test.go b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
index 8f6c25bc6a..1640629edc 100644
--- a/cmd/entrypoint/testutil_fake_syntheticauth_test.go
+++ b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
@@ -29,11 +29,13 @@ func HasAuthService(namespace, name string) func(snapshot *snapshot.Snapshot) bo
 // Tests the synthetic auth generation when a valid AuthService is created.  This AuthService has
 // `protocol_version: v3` and should not be replaced by the synthetic AuthService.
 func TestSyntheticAuthValid(t *testing.T) {
-	t.Setenv("EDGE_STACK", "true")
+	if true {
+		if true {
+			t.Setenv("EDGE_STACK", "true")
 
-	f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
+			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
 
-	err := f.UpsertYAML(`
+			err := f.UpsertYAML(`
 ---
 apiVersion: getambassador.io/v3alpha1
 kind: AuthService
@@ -45,47 +47,54 @@ spec:
   protocol_version: "v3"
   proto: "grpc"
 `)
-	assert.NoError(t, err)
-	f.Flush()
-
-	// Use the predicate above to check that the snapshot contains the AuthService defined
-	// above.  The AuthService has `protocol_version: v3` so it should not be removed/replaced
-	// by the synthetic AuthService injected by syntheticauth.go
-	snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
-	assert.NoError(t, err)
-	assert.NotNil(t, snap)
+			assert.NoError(t, err)
+			f.Flush()
+
+			// Use the predicate above to check that the snapshot contains the
+			// AuthService defined above.  The AuthService has `protocol_version: v3` so
+			// it should not be removed/replaced by the synthetic AuthService injected
+			// by syntheticauth.go
+			snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
+			assert.NoError(t, err)
+			assert.NotNil(t, snap)
+
+			assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
+			// In edge-stack we should only ever have 1 AuthService.
+			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+
+			// Check for an ext_authz cluster name matching the provided AuthService
+			// (Http_Filters are harder to check since they always have the same name).
+			// The namespace for this extauthz cluster should be foo (since that is the
+			// namespace of the valid AuthService above).
+			isAuthCluster := func(c *v3cluster.Cluster) bool {
+				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
+			}
 
-	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
-	// In edge-stack we should only ever have 1 AuthService.
-	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+			// Grab the next Envoy config that has an Edge Stack auth cluster on
+			// 127.0.0.1:8500
+			envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
+				return FindCluster(envoy, isAuthCluster) != nil
+			})
+			require.NoError(t, err)
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
-	// harder to check since they always have the same name).  The namespace for this extauthz
-	// cluster should be foo (since that is the namespace of the valid AuthService above).
-	isAuthCluster := func(c *v3cluster.Cluster) bool {
-		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
+			// Make sure an Envoy Config containing a extauth cluster for the
+			// AuthService that was defined.
+			assert.NotNil(t, envoyConfig)
+		}
 	}
-
-	// Grab the next Envoy config that has an Edge Stack auth cluster on 127.0.0.1:8500
-	envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
-		return FindCluster(envoy, isAuthCluster) != nil
-	})
-	require.NoError(t, err)
-
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
-	// defined.
-	assert.NotNil(t, envoyConfig)
 }
 
 // Tests the synthetic auth generation when a valid AuthService is created as a getambassador.io/v2
 // resource.  This AuthService has `protocol_version: v3` and should not be replaced by the
 // synthetic AuthService.
 func TestSyntheticAuthValidV2(t *testing.T) {
-	t.Setenv("EDGE_STACK", "true")
+	if true {
+		if true {
+			t.Setenv("EDGE_STACK", "true")
 
-	f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
+			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
 
-	err := f.UpsertYAML(`
+			err := f.UpsertYAML(`
 ---
 apiVersion: getambassador.io/v2
 kind: AuthService
@@ -97,46 +106,53 @@ spec:
   protocol_version: "v3"
   proto: "grpc"
 `)
-	assert.NoError(t, err)
-	f.Flush()
+			assert.NoError(t, err)
+			f.Flush()
+
+			// Use the predicate above to check that the snapshot contains the
+			// AuthService defined above.  The AuthService has `protocol_version: v3` so
+			// it should not be removed/replaced by the synthetic AuthService injected
+			// by syntheticauth.go
+			snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
+			assert.NoError(t, err)
+			assert.NotNil(t, snap)
+
+			assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
+			// In edge-stack we should only ever have 1 AuthService.
+			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+
+			// Check for an ext_authz cluster name matching the provided AuthService
+			// (Http_Filters are harder to check since they always have the same name).
+			// The namespace for this extauthz cluster should be foo (since that is the
+			// namespace of the valid AuthService above).
+			isAuthCluster := func(c *v3cluster.Cluster) bool {
+				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
+			}
 
-	// Use the predicate above to check that the snapshot contains the AuthService defined
-	// above.  The AuthService has `protocol_version: v3` so it should not be removed/replaced
-	// by the synthetic AuthService injected by syntheticauth.go
-	snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
-	assert.NoError(t, err)
-	assert.NotNil(t, snap)
+			// Grab the next Envoy config that has an Edge Stack auth cluster on
+			// 127.0.0.1:8500
+			envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
+				return FindCluster(envoy, isAuthCluster) != nil
+			})
+			require.NoError(t, err)
 
-	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
-	// In edge-stack we should only ever have 1 AuthService.
-	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
-
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
-	// harder to check since they always have the same name).  The namespace for this extauthz
-	// cluster should be foo (since that is the namespace of the valid AuthService above).
-	isAuthCluster := func(c *v3cluster.Cluster) bool {
-		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
+			// Make sure an Envoy Config containing a extauth cluster for the
+			// AuthService that was defined.
+			assert.NotNil(t, envoyConfig)
+		}
 	}
-
-	// Grab the next Envoy config that has an Edge Stack auth cluster on 127.0.0.1:8500
-	envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
-		return FindCluster(envoy, isAuthCluster) != nil
-	})
-	require.NoError(t, err)
-
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
-	// defined.
-	assert.NotNil(t, envoyConfig)
 }
 
 // This tests with a provided AuthService that has no protocol_version (which defaults to v2).  The
 // synthetic AuthService should be created instead.
 func TestSyntheticAuthReplace(t *testing.T) {
-	t.Setenv("EDGE_STACK", "true")
+	if true {
+		if true {
+			t.Setenv("EDGE_STACK", "true")
 
-	f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
+			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
 
-	err := f.UpsertYAML(`
+			err := f.UpsertYAML(`
 ---
 apiVersion: getambassador.io/v3alpha1
 kind: AuthService
@@ -147,47 +163,55 @@ spec:
   auth_service: 127.0.0.1:8500
   proto: "grpc"
 `)
-	assert.NoError(t, err)
-	f.Flush()
-
-	// Use the predicate above to check that the snapshot contains the AuthService defined
-	// above.  The AuthService does not have `protocol_version: v3` so it should be removed and
-	// replaced by the synthetic AuthService injected by syntheticauth.go
-	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
-	assert.NoError(t, err)
-	assert.NotNil(t, snap)
+			assert.NoError(t, err)
+			f.Flush()
+
+			// Use the predicate above to check that the snapshot contains the
+			// AuthService defined above.  The AuthService does not have
+			// `protocol_version: v3` so it should be removed and replaced by the
+			// synthetic AuthService injected by syntheticauth.go
+			snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
+			assert.NoError(t, err)
+			assert.NotNil(t, snap)
+
+			// The snapshot should only have the synthetic AuthService and not the one
+			// defined above.
+			assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
+			// In edge-stack we should only ever have 1 AuthService.
+			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+
+			// Check for an ext_authz cluster name matching the provided AuthService
+			// (Http_Filters are harder to check since they always have the same name).
+			// The namespace for this extauthz cluster should be default (since that is
+			// the namespace of the synthetic AuthService).
+			isAuthCluster := func(c *v3cluster.Cluster) bool {
+				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
+			}
 
-	// The snapshot should only have the synthetic AuthService and not the one defined above.
-	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
-	// In edge-stack we should only ever have 1 AuthService.
-	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+			// Grab the next Envoy config that has an Edge Stack auth cluster on
+			// 127.0.0.1:8500
+			envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
+				return FindCluster(envoy, isAuthCluster) != nil
+			})
+			require.NoError(t, err)
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
-	// harder to check since they always have the same name).  The namespace for this extauthz
-	// cluster should be default (since that is the namespace of the synthetic AuthService).
-	isAuthCluster := func(c *v3cluster.Cluster) bool {
-		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
+			// Make sure an Envoy Config containing a extauth cluster for the
+			// AuthService that was defined.
+			assert.NotNil(t, envoyConfig)
+		}
 	}
-
-	// Grab the next Envoy config that has an Edge Stack auth cluster on 127.0.0.1:8500
-	envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
-		return FindCluster(envoy, isAuthCluster) != nil
-	})
-	require.NoError(t, err)
-
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
-	// defined.
-	assert.NotNil(t, envoyConfig)
 }
 
 // This tests with a provided AuthService that has no protocol_version (which defaults to v2).  The
 // synthetic AuthService should be created instead.
 func TestSyntheticAuthReplaceV2(t *testing.T) {
-	t.Setenv("EDGE_STACK", "true")
+	if true {
+		if true {
+			t.Setenv("EDGE_STACK", "true")
 
-	f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
+			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
 
-	err := f.UpsertYAML(`
+			err := f.UpsertYAML(`
 ---
 apiVersion: getambassador.io/v2
 kind: AuthService
@@ -198,37 +222,43 @@ spec:
   auth_service: 127.0.0.1:8500
   proto: "grpc"
 `)
-	assert.NoError(t, err)
-	f.Flush()
-
-	// Use the predicate above to check that the snapshot contains the AuthService defined
-	// above.  The AuthService does not have `protocol_version: v3` so it should be removed and
-	// replaced by the synthetic AuthService injected by syntheticauth.go
-	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
-	assert.NoError(t, err)
-	assert.NotNil(t, snap)
+			assert.NoError(t, err)
+			f.Flush()
+
+			// Use the predicate above to check that the snapshot contains the
+			// AuthService defined above.  The AuthService does not have
+			// `protocol_version: v3` so it should be removed and replaced by the
+			// synthetic AuthService injected by syntheticauth.go
+			snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
+			assert.NoError(t, err)
+			assert.NotNil(t, snap)
+
+			// The snapshot should only have the synthetic AuthService and not the one
+			// defined above.
+			assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
+			// In edge-stack we should only ever have 1 AuthService.
+			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+
+			// Check for an ext_authz cluster name matching the provided AuthService
+			// (Http_Filters are harder to check since they always have the same name).
+			// The namespace for this extauthz cluster should be default (since that is
+			// the namespace of the synthetic AuthService).
+			isAuthCluster := func(c *v3cluster.Cluster) bool {
+				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
+			}
 
-	// The snapshot should only have the synthetic AuthService and not the one defined above.
-	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
-	// In edge-stack we should only ever have 1 AuthService.
-	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+			// Grab the next Envoy config that has an Edge Stack auth cluster on
+			// 127.0.0.1:8500
+			envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
+				return FindCluster(envoy, isAuthCluster) != nil
+			})
+			require.NoError(t, err)
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
-	// harder to check since they always have the same name).  The namespace for this extauthz
-	// cluster should be default (since that is the namespace of the synthetic AuthService).
-	isAuthCluster := func(c *v3cluster.Cluster) bool {
-		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
+			// Make sure an Envoy Config containing a extauth cluster for the
+			// AuthService that was defined.
+			assert.NotNil(t, envoyConfig)
+		}
 	}
-
-	// Grab the next Envoy config that has an Edge Stack auth cluster on 127.0.0.1:8500
-	envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
-		return FindCluster(envoy, isAuthCluster) != nil
-	})
-	require.NoError(t, err)
-
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
-	// defined.
-	assert.NotNil(t, envoyConfig)
 }
 
 // Tests the synthetic auth generation when an invalid AuthService is created.  This AuthService has
@@ -236,11 +266,13 @@ spec:
 // a bogus value because the bogus field will be dropped when it is loaded and we will be left with
 // a valid AuthService.
 func TestSyntheticAuthBogusField(t *testing.T) {
-	t.Setenv("EDGE_STACK", "true")
+	if true {
+		if true {
+			t.Setenv("EDGE_STACK", "true")
 
-	f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
+			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
 
-	err := f.UpsertYAML(`
+			err := f.UpsertYAML(`
 ---
 apiVersion: getambassador.io/v3alpha1
 kind: AuthService
@@ -253,47 +285,54 @@ spec:
   proto: "grpc"
   bogus_field: "foo"
 `)
-	assert.NoError(t, err)
-	f.Flush()
+			assert.NoError(t, err)
+			f.Flush()
+
+			// Use the predicate above to check that the snapshot contains the
+			// AuthService defined above.  The AuthService has `protocol_version: v3` so
+			// it should not be removed/replaced by the synthetic AuthService injected
+			// by syntheticauth.go
+			snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
+			assert.NoError(t, err)
+			assert.NotNil(t, snap)
+
+			assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
+			// In edge-stack we should only ever have 1 AuthService.
+			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+
+			// Check for an ext_authz cluster name matching the provided AuthService
+			// (Http_Filters are harder to check since they always have the same name).
+			// The namespace for this extauthz cluster should be foo (since that is the
+			// namespace of the valid AuthService above).
+			isAuthCluster := func(c *v3cluster.Cluster) bool {
+				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
+			}
 
-	// Use the predicate above to check that the snapshot contains the AuthService defined
-	// above.  The AuthService has `protocol_version: v3` so it should not be removed/replaced by
-	// the synthetic AuthService injected by syntheticauth.go
-	snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
-	assert.NoError(t, err)
-	assert.NotNil(t, snap)
+			// Grab the next Envoy config that has an Edge Stack auth cluster on
+			// 127.0.0.1:8500
+			envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
+				return FindCluster(envoy, isAuthCluster) != nil
+			})
+			require.NoError(t, err)
 
-	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
-	// In edge-stack we should only ever have 1 AuthService.
-	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
-
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
-	// harder to check since they always have the same name).  The namespace for this extauthz
-	// cluster should be foo (since that is the namespace of the valid AuthService above).
-	isAuthCluster := func(c *v3cluster.Cluster) bool {
-		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
+			// Make sure an Envoy Config containing a extauth cluster for the
+			// AuthService that was defined.
+			assert.NotNil(t, envoyConfig)
+		}
 	}
-
-	// Grab the next Envoy config that has an Edge Stack auth cluster on 127.0.0.1:8500
-	envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
-		return FindCluster(envoy, isAuthCluster) != nil
-	})
-	require.NoError(t, err)
-
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
-	// defined.
-	assert.NotNil(t, envoyConfig)
 }
 
 // Tests the synthetic auth generation when an invalid AuthService is created as a
 // getambassador.io/v2 resource.  This AuthService has `protocol_version: v3` and should be replaced
 // by the synthetic AuthService because it contains a bogus field and is not valid.
 func TestSyntheticAuthBogusFieldV2(t *testing.T) {
-	t.Setenv("EDGE_STACK", "true")
+	if true {
+		if true {
+			t.Setenv("EDGE_STACK", "true")
 
-	f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
+			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
 
-	err := f.UpsertYAML(`
+			err := f.UpsertYAML(`
 ---
 apiVersion: getambassador.io/v2
 kind: AuthService
@@ -306,36 +345,41 @@ spec:
   proto: "grpc"
   bogus_field: "foo"
 `)
-	assert.NoError(t, err)
-	f.Flush()
-
-	// Use the predicate above to check that the snapshot contains the AuthService defined
-	// above.  The AuthService has `protocol_version: v3` so it should not be removed/replaced
-	// by the synthetic AuthService injected by syntheticauth.go
-	snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
-	assert.NoError(t, err)
-	assert.NotNil(t, snap)
+			assert.NoError(t, err)
+			f.Flush()
+
+			// Use the predicate above to check that the snapshot contains the
+			// AuthService defined above.  The AuthService has `protocol_version: v3` so
+			// it should not be removed/replaced by the synthetic AuthService injected
+			// by syntheticauth.go
+			snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
+			assert.NoError(t, err)
+			assert.NotNil(t, snap)
+
+			assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
+			// In edge-stack we should only ever have 1 AuthService.
+			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+
+			// Check for an ext_authz cluster name matching the provided AuthService
+			// (Http_Filters are harder to check since they always have the same name).
+			// The namespace for this extauthz cluster should be foo (since that is the
+			// namespace of the valid AuthService above).
+			isAuthCluster := func(c *v3cluster.Cluster) bool {
+				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
+			}
 
-	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
-	// In edge-stack we should only ever have 1 AuthService.
-	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+			// Grab the next Envoy config that has an Edge Stack auth cluster on
+			// 127.0.0.1:8500
+			envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
+				return FindCluster(envoy, isAuthCluster) != nil
+			})
+			require.NoError(t, err)
 
-	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
-	// harder to check since they always have the same name).  The namespace for this extauthz
-	// cluster should be foo (since that is the namespace of the valid AuthService above).
-	isAuthCluster := func(c *v3cluster.Cluster) bool {
-		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
+			// Make sure an Envoy Config containing a extauth cluster for the
+			// AuthService that was defined.
+			assert.NotNil(t, envoyConfig)
+		}
 	}
-
-	// Grab the next Envoy config that has an Edge Stack auth cluster on 127.0.0.1:8500
-	envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
-		return FindCluster(envoy, isAuthCluster) != nil
-	})
-	require.NoError(t, err)
-
-	// Make sure an Envoy Config containing a extauth cluster for the AuthService that was
-	// defined.
-	assert.NotNil(t, envoyConfig)
 }
 
 // Tests the synthetic auth generation when an invalid AuthService (because the protocol_version is

From a67d634628c59981a5b833d03f43212e4ee33a77 Mon Sep 17 00:00:00 2001
From: Luke Shumaker <lukeshu@datawire.io>
Date: Fri, 24 Jun 2022 22:22:58 -0600
Subject: [PATCH 08/11] cleanup: cmd/entrypoint: De-duplicate the synthetic
 auth tests

Some of the tests were awfully repetitive; parameterize them as subtests
to reduce duplication that I need to reat through.

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
 .../testutil_fake_syntheticauth_test.go       | 205 ++----------------
 1 file changed, 15 insertions(+), 190 deletions(-)

diff --git a/cmd/entrypoint/testutil_fake_syntheticauth_test.go b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
index 1640629edc..df926cad53 100644
--- a/cmd/entrypoint/testutil_fake_syntheticauth_test.go
+++ b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
@@ -29,74 +29,16 @@ func HasAuthService(namespace, name string) func(snapshot *snapshot.Snapshot) bo
 // Tests the synthetic auth generation when a valid AuthService is created.  This AuthService has
 // `protocol_version: v3` and should not be replaced by the synthetic AuthService.
 func TestSyntheticAuthValid(t *testing.T) {
-	if true {
-		if true {
+	for _, apiVersion := range []string{"v2", "v3alpha1"} {
+		apiVersion := apiVersion // capture loop variable
+		t.Run(apiVersion, func(t *testing.T) {
 			t.Setenv("EDGE_STACK", "true")
 
 			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
 
 			err := f.UpsertYAML(`
 ---
-apiVersion: getambassador.io/v3alpha1
-kind: AuthService
-metadata:
-  name: edge-stack-auth-test
-  namespace: foo
-spec:
-  auth_service: 127.0.0.1:8500
-  protocol_version: "v3"
-  proto: "grpc"
-`)
-			assert.NoError(t, err)
-			f.Flush()
-
-			// Use the predicate above to check that the snapshot contains the
-			// AuthService defined above.  The AuthService has `protocol_version: v3` so
-			// it should not be removed/replaced by the synthetic AuthService injected
-			// by syntheticauth.go
-			snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
-			assert.NoError(t, err)
-			assert.NotNil(t, snap)
-
-			assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
-			// In edge-stack we should only ever have 1 AuthService.
-			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
-
-			// Check for an ext_authz cluster name matching the provided AuthService
-			// (Http_Filters are harder to check since they always have the same name).
-			// The namespace for this extauthz cluster should be foo (since that is the
-			// namespace of the valid AuthService above).
-			isAuthCluster := func(c *v3cluster.Cluster) bool {
-				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
-			}
-
-			// Grab the next Envoy config that has an Edge Stack auth cluster on
-			// 127.0.0.1:8500
-			envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
-				return FindCluster(envoy, isAuthCluster) != nil
-			})
-			require.NoError(t, err)
-
-			// Make sure an Envoy Config containing a extauth cluster for the
-			// AuthService that was defined.
-			assert.NotNil(t, envoyConfig)
-		}
-	}
-}
-
-// Tests the synthetic auth generation when a valid AuthService is created as a getambassador.io/v2
-// resource.  This AuthService has `protocol_version: v3` and should not be replaced by the
-// synthetic AuthService.
-func TestSyntheticAuthValidV2(t *testing.T) {
-	if true {
-		if true {
-			t.Setenv("EDGE_STACK", "true")
-
-			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
-
-			err := f.UpsertYAML(`
----
-apiVersion: getambassador.io/v2
+apiVersion: getambassador.io/` + apiVersion + `
 kind: AuthService
 metadata:
   name: edge-stack-auth-test
@@ -139,81 +81,23 @@ spec:
 			// Make sure an Envoy Config containing a extauth cluster for the
 			// AuthService that was defined.
 			assert.NotNil(t, envoyConfig)
-		}
+		})
 	}
 }
 
 // This tests with a provided AuthService that has no protocol_version (which defaults to v2).  The
 // synthetic AuthService should be created instead.
 func TestSyntheticAuthReplace(t *testing.T) {
-	if true {
-		if true {
-			t.Setenv("EDGE_STACK", "true")
-
-			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
-
-			err := f.UpsertYAML(`
----
-apiVersion: getambassador.io/v3alpha1
-kind: AuthService
-metadata:
-  name: edge-stack-auth-test
-  namespace: foo
-spec:
-  auth_service: 127.0.0.1:8500
-  proto: "grpc"
-`)
-			assert.NoError(t, err)
-			f.Flush()
-
-			// Use the predicate above to check that the snapshot contains the
-			// AuthService defined above.  The AuthService does not have
-			// `protocol_version: v3` so it should be removed and replaced by the
-			// synthetic AuthService injected by syntheticauth.go
-			snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
-			assert.NoError(t, err)
-			assert.NotNil(t, snap)
-
-			// The snapshot should only have the synthetic AuthService and not the one
-			// defined above.
-			assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
-			// In edge-stack we should only ever have 1 AuthService.
-			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
-
-			// Check for an ext_authz cluster name matching the provided AuthService
-			// (Http_Filters are harder to check since they always have the same name).
-			// The namespace for this extauthz cluster should be default (since that is
-			// the namespace of the synthetic AuthService).
-			isAuthCluster := func(c *v3cluster.Cluster) bool {
-				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
-			}
-
-			// Grab the next Envoy config that has an Edge Stack auth cluster on
-			// 127.0.0.1:8500
-			envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
-				return FindCluster(envoy, isAuthCluster) != nil
-			})
-			require.NoError(t, err)
-
-			// Make sure an Envoy Config containing a extauth cluster for the
-			// AuthService that was defined.
-			assert.NotNil(t, envoyConfig)
-		}
-	}
-}
-
-// This tests with a provided AuthService that has no protocol_version (which defaults to v2).  The
-// synthetic AuthService should be created instead.
-func TestSyntheticAuthReplaceV2(t *testing.T) {
-	if true {
-		if true {
+	for _, apiVersion := range []string{"v2", "v3alpha1"} {
+		apiVersion := apiVersion // capture loop variable
+		t.Run(apiVersion, func(t *testing.T) {
 			t.Setenv("EDGE_STACK", "true")
 
 			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
 
 			err := f.UpsertYAML(`
 ---
-apiVersion: getambassador.io/v2
+apiVersion: getambassador.io/` + apiVersion + `
 kind: AuthService
 metadata:
   name: edge-stack-auth-test
@@ -257,7 +141,7 @@ spec:
 			// Make sure an Envoy Config containing a extauth cluster for the
 			// AuthService that was defined.
 			assert.NotNil(t, envoyConfig)
-		}
+		})
 	}
 }
 
@@ -266,75 +150,16 @@ spec:
 // a bogus value because the bogus field will be dropped when it is loaded and we will be left with
 // a valid AuthService.
 func TestSyntheticAuthBogusField(t *testing.T) {
-	if true {
-		if true {
-			t.Setenv("EDGE_STACK", "true")
-
-			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
-
-			err := f.UpsertYAML(`
----
-apiVersion: getambassador.io/v3alpha1
-kind: AuthService
-metadata:
-  name: edge-stack-auth-test
-  namespace: foo
-spec:
-  auth_service: 127.0.0.1:8500
-  protocol_version: "v3"
-  proto: "grpc"
-  bogus_field: "foo"
-`)
-			assert.NoError(t, err)
-			f.Flush()
-
-			// Use the predicate above to check that the snapshot contains the
-			// AuthService defined above.  The AuthService has `protocol_version: v3` so
-			// it should not be removed/replaced by the synthetic AuthService injected
-			// by syntheticauth.go
-			snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
-			assert.NoError(t, err)
-			assert.NotNil(t, snap)
-
-			assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
-			// In edge-stack we should only ever have 1 AuthService.
-			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
-
-			// Check for an ext_authz cluster name matching the provided AuthService
-			// (Http_Filters are harder to check since they always have the same name).
-			// The namespace for this extauthz cluster should be foo (since that is the
-			// namespace of the valid AuthService above).
-			isAuthCluster := func(c *v3cluster.Cluster) bool {
-				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
-			}
-
-			// Grab the next Envoy config that has an Edge Stack auth cluster on
-			// 127.0.0.1:8500
-			envoyConfig, err := f.GetEnvoyConfig(func(envoy *v3bootstrap.Bootstrap) bool {
-				return FindCluster(envoy, isAuthCluster) != nil
-			})
-			require.NoError(t, err)
-
-			// Make sure an Envoy Config containing a extauth cluster for the
-			// AuthService that was defined.
-			assert.NotNil(t, envoyConfig)
-		}
-	}
-}
-
-// Tests the synthetic auth generation when an invalid AuthService is created as a
-// getambassador.io/v2 resource.  This AuthService has `protocol_version: v3` and should be replaced
-// by the synthetic AuthService because it contains a bogus field and is not valid.
-func TestSyntheticAuthBogusFieldV2(t *testing.T) {
-	if true {
-		if true {
+	for _, apiVersion := range []string{"v2", "v3alpha1"} {
+		apiVersion := apiVersion // capture loop variable
+		t.Run(apiVersion, func(t *testing.T) {
 			t.Setenv("EDGE_STACK", "true")
 
 			f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil)
 
 			err := f.UpsertYAML(`
 ---
-apiVersion: getambassador.io/v2
+apiVersion: getambassador.io/` + apiVersion + `
 kind: AuthService
 metadata:
   name: edge-stack-auth-test
@@ -378,7 +203,7 @@ spec:
 			// Make sure an Envoy Config containing a extauth cluster for the
 			// AuthService that was defined.
 			assert.NotNil(t, envoyConfig)
-		}
+		})
 	}
 }
 

From 10fbc26d9e23bfcb7f6393b718fb7cd1570cb459 Mon Sep 17 00:00:00 2001
From: Luke Shumaker <lukeshu@datawire.io>
Date: Fri, 24 Jun 2022 22:41:46 -0600
Subject: [PATCH 09/11] cleanup:
 cmd/entrypoint/testutil_fake_syntheticauth_test.go: Check lengths first

Check that slice[0] even exists before checking things inside of slice[0]!

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
 .../testutil_fake_syntheticauth_test.go       | 28 +++++++++----------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/cmd/entrypoint/testutil_fake_syntheticauth_test.go b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
index df926cad53..31c34e1228 100644
--- a/cmd/entrypoint/testutil_fake_syntheticauth_test.go
+++ b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
@@ -59,9 +59,9 @@ spec:
 			assert.NoError(t, err)
 			assert.NotNil(t, snap)
 
-			assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
 			// In edge-stack we should only ever have 1 AuthService.
 			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+			assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
 
 			// Check for an ext_authz cluster name matching the provided AuthService
 			// (Http_Filters are harder to check since they always have the same name).
@@ -117,11 +117,11 @@ spec:
 			assert.NoError(t, err)
 			assert.NotNil(t, snap)
 
+			// In edge-stack we should only ever have 1 AuthService.
+			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 			// The snapshot should only have the synthetic AuthService and not the one
 			// defined above.
 			assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
-			// In edge-stack we should only ever have 1 AuthService.
-			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 
 			// Check for an ext_authz cluster name matching the provided AuthService
 			// (Http_Filters are harder to check since they always have the same name).
@@ -181,9 +181,9 @@ spec:
 			assert.NoError(t, err)
 			assert.NotNil(t, snap)
 
-			assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
 			// In edge-stack we should only ever have 1 AuthService.
 			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+			assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
 
 			// Check for an ext_authz cluster name matching the provided AuthService
 			// (Http_Filters are harder to check since they always have the same name).
@@ -238,10 +238,10 @@ spec:
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
-	// The snapshot should only have the synthetic AuthService and not the one defined above.
-	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+	// The snapshot should only have the synthetic AuthService and not the one defined above.
+	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
 
 	// Check for an ext_authz cluster name matching the synthetic AuthService.  The namespace
 	// for this extauthz cluster should be default (since that is the namespace of the synthetic
@@ -330,10 +330,10 @@ spec:
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
-	// The snapshot should only have the synthetic AuthService and not the one defined above.
-	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+	// The snapshot should only have the synthetic AuthService and not the one defined above.
+	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
 
 	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
 	// harder to check since they always have the same name).  The namespace for this extauthz
@@ -383,10 +383,10 @@ spec:
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
-	// The snapshot should only have the synthetic AuthService and not the one defined above.
-	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
 	// We should only have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+	// The snapshot should only have the synthetic AuthService and not the one defined above.
+	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
 
 	// Check for an ext_authz cluster name matching the synthetic AuthService.  The namespace
 	// for this extauthz cluster should be default (since that is the namespace of the synthetic
@@ -431,9 +431,9 @@ spec:
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
-	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
 
 	// Check for an ext_authz cluster name matching the provided AuthService (Http_Filters are
 	// harder to check since they always have the same name).  The namespace for this extauthz
@@ -485,10 +485,10 @@ spec:
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
-	// The snapshot should only have the synthetic AuthService
-	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+	// The snapshot should only have the synthetic AuthService
+	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
 
 	// Even though it is the synthetic AuthService, we should have the custom timeout_ms and v3
 	// protocol version.
@@ -545,9 +545,9 @@ spec:
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
-	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
+	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
 
 	for _, authService := range snap.Kubernetes.AuthServices {
 		assert.Equal(t, "dummy-service", authService.Spec.AuthService)

From 9d1a692241da0808a9c6f8d5527ecc01f42f7d99 Mon Sep 17 00:00:00 2001
From: Luke Shumaker <lukeshu@datawire.io>
Date: Fri, 24 Jun 2022 14:44:12 -0600
Subject: [PATCH 10/11] fix: cmd/entrypoint: Simplify and fix synthetic
 authservice injection

Maybe even fix a few bugs while we're at it?  It's easier for me to look
at the simple code and say there are no bugs than at the complex code and
say there are no bugs.

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
 cmd/entrypoint/syntheticauth.go               | 227 ++++++------------
 .../testutil_fake_syntheticauth_test.go       |  74 +++---
 2 files changed, 112 insertions(+), 189 deletions(-)

diff --git a/cmd/entrypoint/syntheticauth.go b/cmd/entrypoint/syntheticauth.go
index 2e4c0f9051..925c037932 100644
--- a/cmd/entrypoint/syntheticauth.go
+++ b/cmd/entrypoint/syntheticauth.go
@@ -2,184 +2,117 @@ package entrypoint
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/datawire/dlib/dlog"
 	"github.com/emissary-ingress/emissary/v3/pkg/api/getambassador.io/v3alpha1"
 	"github.com/emissary-ingress/emissary/v3/pkg/emissaryutil"
 	"github.com/emissary-ingress/emissary/v3/pkg/kates"
-	"github.com/emissary-ingress/emissary/v3/pkg/snapshot/v1"
 )
 
-// Iterates over the annotations in a snapshot to check if any AuthServices are present.
-func annotationsContainAuthService(annotations map[string]snapshot.AnnotationList) bool {
-	for _, list := range annotations {
-		for _, obj := range list {
-			switch obj.(type) {
-			case *v3alpha1.AuthService:
-				return true
-			default:
-				continue
-			}
-		}
-	}
-	return false
-}
-
 // Checks if the provided string is a loopback IP address with port 8500
 func IsLocalhost8500(svcStr string) bool {
 	_, hostname, port, err := emissaryutil.ParseServiceName(svcStr)
 	return err == nil && port == 8500 && emissaryutil.IsLocalhost(hostname)
 }
 
+func iterateOverAuthServices(sh *SnapshotHolder, cb func(
+	authService *v3alpha1.AuthService, // duh
+	name string, // name to unambiguously refer to the authService by; might be more complex than "name.namespace" if it's an annotation
+	parentName string, // name of the thing that the annotation is on (or empty if not an annotation)
+	idx int, // index of the authService; either in sh.k8sSnapshot.AuthServices or in sh.k8sSnapshot.Annotations[parentName]
+)) {
+	envAmbID := GetAmbassadorID()
+
+	for i, authService := range sh.k8sSnapshot.AuthServices {
+		if authService.Spec.AmbassadorID.Matches(envAmbID) {
+			name := authService.TypeMeta.Kind + "/" + authService.ObjectMeta.Name + "." + authService.ObjectMeta.Namespace
+			cb(authService, name, "", i)
+		}
+	}
+	for parentName, list := range sh.k8sSnapshot.Annotations {
+		for i, obj := range list {
+			if authService, ok := obj.(*v3alpha1.AuthService); ok && authService.Spec.AmbassadorID.Matches(envAmbID) {
+				name := fmt.Sprintf("%s#%d", parentName, i)
+				cb(authService, name, parentName, i)
+			}
+		}
+	}
+}
+
 // This is a gross hack to remove all AuthServices using protocol_version: v2 only when running Edge-Stack and then inject an
 // AuthService with protocol_version: v3 if needed. The purpose of this hack is to prevent Edge-Stack 2.3 from
 // using any other AuthService than the default one running as part of amb-sidecar and force the protocol version to v3.
 func ReconcileAuthServices(ctx context.Context, sh *SnapshotHolder, deltas *[]*kates.Delta) error {
 	// We only want to remove AuthServices if this is an instance of Edge-Stack
-	isEdgeStack, err := IsEdgeStack()
-	if err != nil {
-		return err
+	if isEdgeStack, err := IsEdgeStack(); err != nil {
+		return fmt.Errorf("ReconcileAuthServices: %w", err)
 	} else if !isEdgeStack {
 		return nil
 	}
 
-	// Construct a synthetic AuthService to be injected if we dont find any valid AuthServices
-	injectSyntheticAuth := true
-	syntheticAuth := &v3alpha1.AuthService{
-		TypeMeta: kates.TypeMeta{
-			Kind:       "AuthService",
-			APIVersion: "getambassador.io/v3alpha1",
-		},
-		ObjectMeta: kates.ObjectMeta{
-			Name:      "synthetic-edge-stack-auth",
-			Namespace: GetAmbassadorNamespace(),
-		},
-		Spec: v3alpha1.AuthServiceSpec{
-			AuthService:     "127.0.0.1:8500",
-			Proto:           "grpc",
-			ProtocolVersion: "v3",
-			AmbassadorID:    []string{"_automatic_"},
-		},
-	}
-
-	var authServices []*v3alpha1.AuthService
-	syntheticAuthExists := false
-	for _, authService := range sh.k8sSnapshot.AuthServices {
-		// check if the AuthService points at 127.0.0.1:8500 (edge-stack)
+	// using a name with underscores prevents it from colliding with anything real in the
+	// cluster--Kubernetes resources can't have underscores in their name.
+	const syntheticAuthServiceName = "synthetic_edge_stack_auth"
+
+	var (
+		numAuthServices  uint64
+		syntheticAuth    *v3alpha1.AuthService
+		syntheticAuthIdx int
+	)
+	iterateOverAuthServices(sh, func(authService *v3alpha1.AuthService, name, parentName string, i int) {
+		numAuthServices++
 		if IsLocalhost8500(authService.Spec.AuthService) {
-			// If it does point at localhost, make sure it is v3, otherwise we need to inject the synthetic AuthService
-			if authService.Spec.ProtocolVersion == "v3" {
-				injectSyntheticAuth = false
-				if authService.ObjectMeta.Name == "synthetic-edge-stack-auth" {
-					syntheticAuthExists = true
-				} else {
-					authServices = append(authServices, authService)
-				}
-			} else {
-				// In the event that there is an AuthService that does not have protocol_version: v3
-				// Then we use the spec of that AuthService as the Synthetic v3 AuthService we will inject later
-				syntheticAuth.Spec = authService.Spec
-				syntheticAuth.Spec.ProtocolVersion = "v3"
+			if parentName == "" && authService.ObjectMeta.Name == syntheticAuthServiceName {
+				syntheticAuth = authService
+				syntheticAuthIdx = i
 			}
-		} else {
-			// By default we keep any custom AuthServices that do not point at localhost
-			authServices = append(authServices, authService)
-			injectSyntheticAuth = false
-		}
-	}
-
-	// TODO if there are v3 authServices, still remove any that are not `v3`
-
-	// Also loop over the annotations and remove authservices that are not v3. We do
-	// this by looping over each entry in the annotations map, removing all the non-v3
-	// AuthService entries, and then removing any keys that end up with empty lists.
-
-	// OK. Loop over all the keys and their corrauthServicesesponding lists of annotations...
-	if annotationsContainAuthService(sh.k8sSnapshot.Annotations) {
-		for key, list := range sh.k8sSnapshot.Annotations {
-			// ...and build up our edited list of things.
-			editedList := snapshot.AnnotationList{}
-
-			for _, obj := range list {
-				switch annotationObj := obj.(type) {
-				case *v3alpha1.AuthService:
-					if IsLocalhost8500(annotationObj.Spec.AuthService) {
-						// If it does point at localhost, make sure it is v3, otherwise we need to inject the synthetic AuthService
-						if annotationObj.Spec.ProtocolVersion == "v3" {
-							injectSyntheticAuth = false
-							if annotationObj.ObjectMeta.Name == "synthetic-edge-stack-auth" {
-								syntheticAuthExists = true
-							} else {
-								authServices = append(authServices, annotationObj)
-								editedList = append(editedList, annotationObj)
-							}
-						} else {
-							// In the event that there is an AuthService that does not have protocol_version: v3
-							// Then we use the spec of that AuthService as the Synthetic v3 AuthService we will inject later
-							syntheticAuth.Spec = annotationObj.Spec
-							syntheticAuth.Spec.ProtocolVersion = "v3"
-						}
-					} else {
-						// By default we keep any custom AuthServices that do not point at localhost
-						authServices = append(authServices, annotationObj)
-						editedList = append(editedList, annotationObj)
-						injectSyntheticAuth = false
-					}
-				default:
-					// This isn't an AuthService at all, so we'll keep it.
-					editedList = append(editedList, annotationObj)
-				}
-			}
-
-			// Once here, is our editedList is empty?
-			if len(editedList) == 0 {
-				// Yes. Delete the whole key for this list.
-				delete(sh.k8sSnapshot.Annotations, key)
-			} else {
-				// Nope, not empty. Save the edited list.
-				sh.k8sSnapshot.Annotations[key] = editedList
+			if authService.Spec.ProtocolVersion != "v3" {
+				// Force the Edge Stack AuthService to be protocol_version=v3.  This
+				// is important so that <2.3 and >=2.3 installations can coexist.
+				// This is important, because for zero-downtime upgrades, they must
+				// coexist briefly while the new Deployment is getting rolled out.
+				dlog.Debugf(ctx, "ReconcileAuthServices: Forcing protocol_version=v3 on %s", name)
+				authService.Spec.ProtocolVersion = "v3"
 			}
 		}
-	}
-
-	if injectSyntheticAuth {
-		dlog.Debugf(ctx, "[WATCHER]: No valid AuthServices with protocol_version: v3 detected, injecting Synthetic AuthService")
-		// There are no valid AuthServices with protocol_version: v3. A synthetic one needs to be injected.
-		authServices = append(authServices, syntheticAuth)
-
-		// loop through the deltas and remove any AuthService deltas adding other AuthServices before the Synthetic delta is inserted
-		var newDeltas []*kates.Delta
-		for _, delta := range *deltas {
-			// Keep all the deltas that are not for AuthServices. The AuthService deltas can be kept as long as they are not an add delta.
-			if (delta.Kind != "AuthService") || (delta.Kind == "AuthService" && delta.DeltaType != kates.ObjectAdd) {
-				newDeltas = append(newDeltas, delta)
-			}
+	})
+
+	switch {
+	case numAuthServices == 0: // add the synthetic auth service
+		dlog.Debug(ctx, "ReconcileAuthServices: No user-provided AuthServices detected; injecting synthetic AuthService")
+		syntheticAuth = &v3alpha1.AuthService{
+			TypeMeta: kates.TypeMeta{
+				Kind:       "AuthService",
+				APIVersion: "getambassador.io/v3alpha1",
+			},
+			ObjectMeta: kates.ObjectMeta{
+				Name:      syntheticAuthServiceName,
+				Namespace: GetAmbassadorNamespace(),
+			},
+			Spec: v3alpha1.AuthServiceSpec{
+				AmbassadorID:    []string{GetAmbassadorID()},
+				AuthService:     "127.0.0.1:8500",
+				Proto:           "grpc",
+				ProtocolVersion: "v3",
+			},
 		}
-		newDeltas = append(newDeltas, &kates.Delta{
+		sh.k8sSnapshot.AuthServices = append(sh.k8sSnapshot.AuthServices, syntheticAuth)
+		*deltas = append(*deltas, &kates.Delta{
 			TypeMeta:   syntheticAuth.TypeMeta,
 			ObjectMeta: syntheticAuth.ObjectMeta,
 			DeltaType:  kates.ObjectAdd,
 		})
-
-		*deltas = newDeltas
-		sh.k8sSnapshot.AuthServices = authServices
-	} else if len(authServices) >= 1 {
-		// Write back the list of valid AuthServices.
-		sh.k8sSnapshot.AuthServices = authServices
-
-		// The synthetic AuthService needs to be removed since one or more valid AuthServices are present.
-		if syntheticAuthExists {
-			dlog.Debugf(ctx, "[WATCHER]: Valid AuthServices using protocol_version: v3 detected alongside the Synthetic AuthService, removing Synthetic...")
-			// One or more Valid AuthServices are present. The synthetic AuthService exists and needs to be removed now.
-			var newDeltas []*kates.Delta
-			*deltas = append(*deltas, &kates.Delta{
-				TypeMeta:   syntheticAuth.TypeMeta,
-				ObjectMeta: syntheticAuth.ObjectMeta,
-				DeltaType:  kates.ObjectDelete,
-			})
-
-			*deltas = newDeltas
-		}
+	case numAuthServices > 1 && syntheticAuth != nil: // remove the synthetic auth service
+		dlog.Debugf(ctx, "ReconcileAuthServices: %d user-provided AuthServices detected; removing synthetic AuthService", numAuthServices-1)
+		sh.k8sSnapshot.AuthServices = append(
+			sh.k8sSnapshot.AuthServices[:syntheticAuthIdx],
+			sh.k8sSnapshot.AuthServices[syntheticAuthIdx+1:]...)
+		*deltas = append(*deltas, &kates.Delta{
+			TypeMeta:   syntheticAuth.TypeMeta,
+			ObjectMeta: syntheticAuth.ObjectMeta,
+			DeltaType:  kates.ObjectDelete,
+		})
 	}
 
 	return nil
diff --git a/cmd/entrypoint/testutil_fake_syntheticauth_test.go b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
index 31c34e1228..9404cf1a6b 100644
--- a/cmd/entrypoint/testutil_fake_syntheticauth_test.go
+++ b/cmd/entrypoint/testutil_fake_syntheticauth_test.go
@@ -85,8 +85,8 @@ spec:
 	}
 }
 
-// This tests with a provided AuthService that has no protocol_version (which defaults to v2).  The
-// synthetic AuthService should be created instead.
+// This tests with a provided AuthService that has no protocol_version (which defaults to v2).  It
+// should get forcibly overridden to be v3.
 func TestSyntheticAuthReplace(t *testing.T) {
 	for _, apiVersion := range []string{"v2", "v3alpha1"} {
 		apiVersion := apiVersion // capture loop variable
@@ -109,26 +109,25 @@ spec:
 			assert.NoError(t, err)
 			f.Flush()
 
-			// Use the predicate above to check that the snapshot contains the
-			// AuthService defined above.  The AuthService does not have
-			// `protocol_version: v3` so it should be removed and replaced by the
-			// synthetic AuthService injected by syntheticauth.go
-			snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
+			// The AuthService does not have `protocol_version: v3` so it should be
+			// forcibly edited to say `protocol_version: v3` by syntheticauth.go
+			snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
 			assert.NoError(t, err)
 			assert.NotNil(t, snap)
 
 			// In edge-stack we should only ever have 1 AuthService.
 			assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
-			// The snapshot should only have the synthetic AuthService and not the one
-			// defined above.
-			assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
+			// The snapshot should only have the one defined above.
+			assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
+			// The protocol version should be forcibly set to v3.
+			assert.Equal(t, "v3", snap.Kubernetes.AuthServices[0].Spec.ProtocolVersion)
 
 			// Check for an ext_authz cluster name matching the provided AuthService
 			// (Http_Filters are harder to check since they always have the same name).
-			// The namespace for this extauthz cluster should be default (since that is
-			// the namespace of the synthetic AuthService).
+			// The namespace for this extauthz cluster should be "foo" (since that is
+			// the namespace of the AuthService).
 			isAuthCluster := func(c *v3cluster.Cluster) bool {
-				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
+				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
 			}
 
 			// Grab the next Envoy config that has an Edge Stack auth cluster on
@@ -187,8 +186,8 @@ spec:
 
 			// Check for an ext_authz cluster name matching the provided AuthService
 			// (Http_Filters are harder to check since they always have the same name).
-			// The namespace for this extauthz cluster should be foo (since that is the
-			// namespace of the valid AuthService above).
+			// The namespace for this extauthz cluster should be "foo" (since that is
+			// the namespace of the valid AuthService above).
 			isAuthCluster := func(c *v3cluster.Cluster) bool {
 				return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
 			}
@@ -224,7 +223,7 @@ metadata:
   namespace: foo
 spec:
   auth_service: 127.0.0.1:8500
-  protocol_version: "v4"
+  protocol_version: "vBogus"
   proto: "grpc"
   bogus_field: "foo"
 `)
@@ -234,14 +233,14 @@ spec:
 	// Use the predicate above to check that the snapshot contains the synthetic AuthService.
 	// The AuthService has `protocol_version: v3`, but it has a bogus field so it should not be
 	// validated and instead we inject the synthetic AuthService.
-	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
+	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic_edge_stack_auth"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 	// The snapshot should only have the synthetic AuthService and not the one defined above.
-	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
+	assert.Equal(t, "synthetic_edge_stack_auth", snap.Kubernetes.AuthServices[0].Name)
 
 	// Check for an ext_authz cluster name matching the synthetic AuthService.  The namespace
 	// for this extauthz cluster should be default (since that is the namespace of the synthetic
@@ -372,21 +371,21 @@ metadata:
 spec:
   auth_service: 127.0.0.1:8500
   proto: "grpc"
-  bogus_field: "foo"
+  protocol_version: "vBogus"
 `)
 	assert.NoError(t, err)
 
 	// Use the predicate above to check that the snapshot contains the synthetic AuthService.
-	// The AuthService has `protocol_version: v3`, but it has a bogus field so it should not be
-	// validated and instead we inject the synthetic AuthService.
-	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
+	// The user-provided AuthService is invalid and so it should be ignored and instead we
+	// inject the synthetic AuthService.
+	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic_edge_stack_auth"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
 	// We should only have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
 	// The snapshot should only have the synthetic AuthService and not the one defined above.
-	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
+	assert.Equal(t, "synthetic_edge_stack_auth", snap.Kubernetes.AuthServices[0].Name)
 
 	// Check for an ext_authz cluster name matching the synthetic AuthService.  The namespace
 	// for this extauthz cluster should be default (since that is the namespace of the synthetic
@@ -405,8 +404,6 @@ spec:
 	// defined.
 	assert.NotNil(t, envoyConfig)
 
-	t.Setenv("EDGE_STACK", "")
-
 	// Updating the yaml for that AuthService to include `protocol_version: v3` should make it
 	// valid and then remove our synthetic AuthService and allow the now valid AuthService to be
 	// used.
@@ -454,8 +451,8 @@ spec:
 }
 
 // This AuthService points at 127.0.0.1:8500, but it does not have `protocol_version: v3`.  It also
-// has additional fields set.  The correct action is to create a SyntheticAuth copy of this
-// AuthService with the same fields but with `protocol_version: v3`.
+// has additional fields set.  The correct action is to edit the AuthService to say
+// `protocol_version: v3`.
 func TestSyntheticAuthCopyFields(t *testing.T) {
 	t.Setenv("EDGE_STACK", "true")
 
@@ -477,31 +474,24 @@ spec:
 	assert.NoError(t, err)
 	f.Flush()
 
-	// Use the predicate above to check that the snapshot contains the synthetic AuthService.
-	// The AuthService has `protocol_version: v3`, but it is missing the `protocol_version: v3`
-	// field.  We expect the synthetic AuthService to be injected, but later we will check that
-	// the synthetic AuthService has Our custom timeout_ms field.
-	snap, err := f.GetSnapshot(HasAuthService("default", "synthetic-edge-stack-auth"))
+	// Use the predicate above to check that the snapshot contains the AuthService.
+	snap, err := f.GetSnapshot(HasAuthService("foo", "edge-stack-auth-test"))
 	assert.NoError(t, err)
 	assert.NotNil(t, snap)
 
 	// In edge-stack we should only ever have 1 AuthService.
 	assert.Equal(t, 1, len(snap.Kubernetes.AuthServices))
-	// The snapshot should only have the synthetic AuthService
-	assert.Equal(t, "synthetic-edge-stack-auth", snap.Kubernetes.AuthServices[0].Name)
-
-	// Even though it is the synthetic AuthService, we should have the custom timeout_ms and v3
-	// protocol version.
-	for _, authService := range snap.Kubernetes.AuthServices {
-		assert.Equal(t, int64(12345), authService.Spec.Timeout.Duration.Milliseconds())
-		assert.Equal(t, "v3", authService.Spec.ProtocolVersion)
-	}
+	// It should be that user-provided AuthService...
+	assert.Equal(t, "edge-stack-auth-test", snap.Kubernetes.AuthServices[0].Name)
+	assert.Equal(t, int64(12345), snap.Kubernetes.AuthServices[0].Spec.Timeout.Duration.Milliseconds())
+	// ... but with `protocol_version: v3` set.
+	assert.Equal(t, "v3", snap.Kubernetes.AuthServices[0].Spec.ProtocolVersion)
 
 	// Check for an ext_authz cluster name matching the synthetic AuthService.  The namespace
 	// for this extauthz cluster should be default (since that is the namespace of the synthetic
 	// AuthService).
 	isAuthCluster := func(c *v3cluster.Cluster) bool {
-		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_default")
+		return strings.Contains(c.Name, "cluster_extauth_127_0_0_1_8500_foo")
 	}
 
 	// Grab the next Envoy config that has an Edge Stack auth cluster on 127.0.0.1:8500

From e403ecd332e91c247e5760d74c28e34178aa0f88 Mon Sep 17 00:00:00 2001
From: Luke Shumaker <lukeshu@datawire.io>
Date: Fri, 24 Jun 2022 19:26:24 -0600
Subject: [PATCH 11/11] fix: v3httpfilter: Fix the error JSON

The important change here is that there was an extra trailing comma that
makes it be invalid JSON.  But also, I intended for it to be
pretty-formatted JSON, but there weren't any newlines.

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
 python/ambassador/envoy/v3/v3httpfilter.py | 10 +++++-----
 python/tests/kat/t_extauth.py              |  7 +++++++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/python/ambassador/envoy/v3/v3httpfilter.py b/python/ambassador/envoy/v3/v3httpfilter.py
index 08a537ab04..63acff77ac 100644
--- a/python/ambassador/envoy/v3/v3httpfilter.py
+++ b/python/ambassador/envoy/v3/v3httpfilter.py
@@ -237,11 +237,11 @@ def V3HTTPFilter_authv1(auth: IRAuth, v3config: 'V3Config'):
    request_handle:respond(
       {[":status"] = "500",
        ["content-type"] = "application/json"},
-      '{'..
-      '  "message": "the """+auth.rkey+""" AuthService is misconfigured; see the logs for more information",'..
-      '  "request_id": "'..request_handle:headers():get('x-request-id')..'",'..
-      '  "status_code": 500,'..
-      '}')
+      '{\\n'..
+      '  "message": "the """+auth.rkey+""" AuthService is misconfigured; see the logs for more information",\\n'..
+      '  "request_id": "'..request_handle:headers():get('x-request-id')..'",\\n'..
+      '  "status_code": 500\\n'..
+      '}\\n')
 end
 """,
             },
diff --git a/python/tests/kat/t_extauth.py b/python/tests/kat/t_extauth.py
index 3cfba27ab8..0e82e6bcdd 100644
--- a/python/tests/kat/t_extauth.py
+++ b/python/tests/kat/t_extauth.py
@@ -1004,6 +1004,13 @@ def queries(self):
 
     def check(self):
         if self.expected_protocol_version == 'invalid':
+            for i, result in enumerate(self.results):
+                # Verify the basic structure of the HTTP 500's JSON body.
+                assert result.json, f"self.results[{i}] does not have a JSON body"
+                assert result.json['status_code'] == 500, f"self.results[{i}] JSON body={repr(result.json)} does not have status_code=500"
+                assert result.json['request_id'], f"self.results[{i}] JSON body={repr(result.json)} does not have request_id"
+                assert self.path.k8s in result.json['message'], f"self.results[{i}] JSON body={repr(result.json)} does not have thing-containing-the-annotation-containing-the-AuthService name {repr(self.path.k8s)} in message"
+                assert 'AuthService' in result.json['message'], f"self.results[{i}] JSON body={repr(result.json)} does not have type 'AuthService' in message"
             return
 
         # [0] Verifies all request headers sent to the authorization server.