Merge pull request #4306 from emissary-ingress/lukeshu/for-aes3

[v3.0.0] Fixes (mostly for Edge Stack)

Commit-by-commit:
 - (first 8 commits) Miscellaneous cleanup in `cmd/entrypoint`; not
   actually fixing anything.
 - (penultimate commit) Fix some problems with synthetic AuthService
   injection. This only affects Edge Stack.
 - (ultimate commit) Fix the JSON error response when an invalid
   AuthService is given. This affects both Edge Stack and Emissary.

The only commit that I think merits much discussion is the synthetic
AuthService injection one.  It's sort of a rewrite.  And I sort of
feel bad about that. I had a hard time following what it was doing
(can't fit big function in small Friday afternoon brain? can't fit big
function on small laptop screen? take your pick).  At first I was just
trying to factor out a `iterateOverAuthServices` function so that the
big chunk wouldn't need to be repeated twice.  And I ended up with
this.  At least one bug in the old code:

    var newDeltas []*kates.Delta
    *deltas = append(*deltas, &kates.Delta{
        TypeMeta:   syntheticAuth.TypeMeta,
        ObjectMeta: syntheticAuth.ObjectMeta,
        DeltaType:  kates.ObjectDelete,
    })
    *deltas = newDeltas

which results in `*deltas` always getting set to `nil`. But because I
couldn't follow the code, there might have been more bugs too.  My
version does have one change: When it sees an Edge Stack AuthService
(localhost:8500) with the wrong protocol version, it just overrides
the protocol version of that AuthService, rather than dropping that
AuthService and injecting a fully-synthetic one.

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>

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})
 		}
 	}
@@ -325,7 +327,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
 		}

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")
 		}
 	}

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)
@@ -221,7 +217,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())

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")

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
-}

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)
 		}
 	}

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