Fix envoy bootstrap logic to not append multiple self_admin clusters

command/connect/envoy/bootstrap_config.go

@@ -11,6 +11,10 @@ import (
 	"text/template"
 )
 
+const (
+	selfAdminName = "self_admin"
+)
+
 // BootstrapConfig is the set of keys we care about in a Connect.Proxy.Config
 // map. Note that this only includes config keys that affects Envoy bootstrap
 // generation. For Envoy config keys that affect runtime xDS behavior see
@@ -70,7 +74,7 @@ type BootstrapConfig struct {
 	// liveness of an Envoy instance when no other listeners are garaunteed to be
 	// configured, as is the case with ingress gateways.
 	//
-	// Not that we do not allow this to be configured via the service
+	// Note that we do not allow this to be configured via the service
 	// definition config map currently.
 	ReadyBindAddr string `mapstructure:"-"`
 
@@ -405,7 +409,7 @@ func (c *BootstrapConfig) generateListenerConfig(args *BootstrapTplArgs, bindAdd
 	}
 
 	clusterJSON := `{
-		"name": "self_admin",
+		"name": "` + selfAdminName + `",
 		"connect_timeout": "5s",
 		"type": "STATIC",
 		"http_protocol_options": {},
@@ -476,14 +480,43 @@ func (c *BootstrapConfig) generateListenerConfig(args *BootstrapTplArgs, bindAdd
 		]
 	}`
 
-	if args.StaticClustersJSON != "" {
-		clusterJSON = ",\n" + clusterJSON
+	// Make sure we do not append the same cluster multiple times, as that will
+	// cause envoy startup to fail.
+	selfAdminClusterExists, err := containsSelfAdminCluster(args.StaticClustersJSON)
+	if err != nil {
+		return err
+	}
+
+	if args.StaticClustersJSON == "" {
+		args.StaticClustersJSON = clusterJSON
+	} else if !selfAdminClusterExists {
+		args.StaticClustersJSON += ",\n" + clusterJSON
 	}
-	args.StaticClustersJSON += clusterJSON
 
 	if args.StaticListenersJSON != "" {
 		listenerJSON = ",\n" + listenerJSON
 	}
 	args.StaticListenersJSON += listenerJSON
 	return nil
 }
+
+func containsSelfAdminCluster(clustersJSON string) (bool, error) {
+	clusterNames := []struct {
+		Name string
+	}{}
+
+	// StaticClustersJSON is defined as a comma-separated list of clusters, so we
+	// need to wrap it in JSON array brackets
+	err := json.Unmarshal([]byte("["+clustersJSON+"]"), &clusterNames)
+	if err != nil {
+		return false, fmt.Errorf("failed to parse static clusters: %s", err)
+	}
+
+	for _, cluster := range clusterNames {
+		if cluster.Name == selfAdminName {
+			return true, nil
+		}
+	}
+
+	return false, nil
+}

command/connect/envoy/bootstrap_config_test.go

@@ -598,6 +598,31 @@ func TestBootstrapConfig_ConfigureArgs(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "ready-bind-addr-and-prometheus-and-stats",
+			input: BootstrapConfig{
+				ReadyBindAddr:      "0.0.0.0:4444",
+				PrometheusBindAddr: "0.0.0.0:9000",
+				StatsBindAddr:      "0.0.0.0:9000",
+			},
+			baseArgs: BootstrapTplArgs{
+				AdminBindAddress: "127.0.0.1",
+				AdminBindPort:    "19000",
+			},
+			wantArgs: BootstrapTplArgs{
+				AdminBindAddress: "127.0.0.1",
+				AdminBindPort:    "19000",
+				// Should add a static cluster for the self-proxy to admin
+				StaticClustersJSON: expectedSelfAdminCluster,
+				// Should add a static http listener too
+				StaticListenersJSON: strings.Join(
+					[]string{expectedPromListener, expectedStatsListener, expectedReadyListener},
+					", ",
+				),
+				StatsConfigJSON: defaultStatsConfigJSON,
+			},
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {