connect/proxy: don't require proxy ID

command/connect/proxy/proxy.go

@@ -10,6 +10,7 @@ import (
 	"os"
 
 	proxyAgent "github.com/hashicorp/consul/agent/proxy"
+	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/command/flags"
 	proxyImpl "github.com/hashicorp/consul/connect/proxy"
 
@@ -112,22 +113,17 @@ func (c *cmd) Run(args []string) int {
 		return 1
 	}
 
-	var p *proxyImpl.Proxy
+	// Get the proper configuration watcher
-	if c.cfgFile != "" {
+	cfgWatcher, err := c.configWatcher(client)
-		c.UI.Info("Configuring proxy locally from " + c.cfgFile)
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error preparing configuration: %s", err))
+		return 1
+	}
 
-		p, err = proxyImpl.NewFromConfigFile(client, c.cfgFile, c.logger)
+	p, err := proxyImpl.New(client, cfgWatcher, c.logger)
-		if err != nil {
+	if err != nil {
-			c.UI.Error(fmt.Sprintf("Failed configuring from file: %s", err))
+		c.UI.Error(fmt.Sprintf("Failed initializing proxy: %s", err))
-			return 1
+		return 1
-		}
-
-	} else {
-		p, err = proxyImpl.New(client, c.proxyID, c.logger)
-		if err != nil {
-			c.UI.Error(fmt.Sprintf("Failed configuring from agent: %s", err))
-			return 1
-		}
 	}
 
 	// Hook the shutdownCh up to close the proxy
@@ -151,6 +147,26 @@ func (c *cmd) Run(args []string) int {
 	return 0
 }
 
+func (c *cmd) configWatcher(client *api.Client) (proxyImpl.ConfigWatcher, error) {
+	// Manual configuration file is specified.
+	if c.cfgFile != "" {
+		cfg, err := proxyImpl.ParseConfigFile(c.cfgFile)
+		if err != nil {
+			return nil, err
+		}
+		return proxyImpl.NewStaticConfigWatcher(cfg), nil
+	}
+
+	// Use the configured proxy ID
+	if c.proxyID == "" {
+		return nil, fmt.Errorf(
+			"-service or -proxy-id must be specified so that proxy can " +
+				"configure itself.")
+	}
+
+	return proxyImpl.NewAgentConfigWatcher(client, c.proxyID, c.logger)
+}
+
 func (c *cmd) Synopsis() string {
 	return synopsis
 }

connect/proxy/config.go

@@ -19,18 +19,16 @@ import (
 // different locations (e.g. command line, agent config endpoint, agent
 // certificate endpoints).
 type Config struct {
-	// ProxyID is the identifier for this proxy as registered in Consul. It's only
-	// guaranteed to be unique per agent.
-	ProxyID string `json:"proxy_id" hcl:"proxy_id"`
-
 	// Token is the authentication token provided for queries to the local agent.
 	Token string `json:"token" hcl:"token"`
 
-	// ProxiedServiceID is the identifier of the service this proxy is representing.
+	// ProxiedServiceName is the name of the service this proxy is representing.
-	ProxiedServiceID string `json:"proxied_service_id" hcl:"proxied_service_id"`
+	// This is the service _name_ and not the service _id_. This allows the
-
+	// proxy to represent services not present in the local catalog.
+	//
 	// ProxiedServiceNamespace is the namespace of the service this proxy is
 	// representing.
+	ProxiedServiceName      string `json:"proxied_service_name" hcl:"proxied_service_name"`
 	ProxiedServiceNamespace string `json:"proxied_service_namespace" hcl:"proxied_service_namespace"`
 
 	// PublicListener configures the mTLS listener.
@@ -39,28 +37,34 @@ type Config struct {
 	// Upstreams configures outgoing proxies for remote connect services.
 	Upstreams []UpstreamConfig `json:"upstreams" hcl:"upstreams"`
 
-	// DevCAFile allows passing the file path to PEM encoded root certificate
+	// DevCAFile, DevServiceCertFile, and DevServiceKeyFile allow configuring
-	// bundle to be used in development instead of the ones supplied by Connect.
+	// the certificate information from a static file. This is only for testing
-	DevCAFile string `json:"dev_ca_file" hcl:"dev_ca_file"`
+	// purposes. All or none must be specified.
-
+	DevCAFile          string `json:"dev_ca_file" hcl:"dev_ca_file"`
-	// DevServiceCertFile allows passing the file path to PEM encoded service
-	// certificate (client and server) to be used in development instead of the
-	// ones supplied by Connect.
 	DevServiceCertFile string `json:"dev_service_cert_file" hcl:"dev_service_cert_file"`
+	DevServiceKeyFile  string `json:"dev_service_key_file" hcl:"dev_service_key_file"`
+}
 
-	// DevServiceKeyFile allows passing the file path to PEM encoded service
+// Service returns the *connect.Service structure represented by this config.
-	// private key to be used in development instead of the ones supplied by
+func (c *Config) Service(client *api.Client, logger *log.Logger) (*connect.Service, error) {
-	// Connect.
+	// If we aren't in dev mode, then we return the configured service.
-	DevServiceKeyFile string `json:"dev_service_key_file" hcl:"dev_service_key_file"`
+	if c.DevCAFile == "" {
+		return connect.NewServiceWithLogger(c.ProxiedServiceName, client, logger)
+	}
+
+	// Dev mode
+	return connect.NewDevServiceFromCertFiles(c.ProxiedServiceName,
+		logger, c.DevCAFile, c.DevServiceCertFile, c.DevServiceKeyFile)
 }
 
 // PublicListenerConfig contains the parameters needed for the incoming mTLS
 // listener.
 type PublicListenerConfig struct {
 	// BindAddress is the host/IP the public mTLS listener will bind to.
+	//
+	// BindPort is the port the public listener will bind to.
 	BindAddress string `json:"bind_address" hcl:"bind_address" mapstructure:"bind_address"`
-
+	BindPort    int    `json:"bind_port" hcl:"bind_port" mapstructure:"bind_port"`
-	BindPort int `json:"bind_port" hcl:"bind_port" mapstructure:"bind_port"`
 
 	// LocalServiceAddress is the host:port for the proxied application. This
 	// should be on loopback or otherwise protected as it's plain TCP.
@@ -265,9 +269,8 @@ func (w *AgentConfigWatcher) handler(blockVal watch.BlockingParamVal,
 
 	// Create proxy config from the response
 	cfg := &Config{
-		ProxyID: w.proxyID,
 		// Token should be already setup in the client
-		ProxiedServiceID:        resp.TargetServiceID,
+		ProxiedServiceName:      resp.TargetServiceName,
 		ProxiedServiceNamespace: "default",
 	}
 

connect/proxy/config_test.go

@@ -19,9 +19,8 @@ func TestParseConfigFile(t *testing.T) {
 	require.Nil(t, err)
 
 	expect := &Config{
-		ProxyID:                 "foo",
 		Token:                   "11111111-2222-3333-4444-555555555555",
-		ProxiedServiceID:        "web",
+		ProxiedServiceName:      "web",
 		ProxiedServiceNamespace: "default",
 		PublicListener: PublicListenerConfig{
 			BindAddress:           "127.0.0.1",
@@ -117,6 +116,7 @@ func TestUpstreamResolverFromClient(t *testing.T) {
 
 func TestAgentConfigWatcher(t *testing.T) {
 	a := agent.NewTestAgent("agent_smith", "")
+	defer a.Shutdown()
 
 	client := a.Client()
 	agent := client.Agent()
@@ -153,8 +153,7 @@ func TestAgentConfigWatcher(t *testing.T) {
 	cfg := testGetConfigValTimeout(t, w, 500*time.Millisecond)
 
 	expectCfg := &Config{
-		ProxyID:                 w.proxyID,
+		ProxiedServiceName:      "web",
-		ProxiedServiceID:        "web",
 		ProxiedServiceNamespace: "default",
 		PublicListener: PublicListenerConfig{
 			BindAddress:           "10.10.10.10",

connect/proxy/proxy.go

@@ -11,7 +11,6 @@ import (
 
 // Proxy implements the built-in connect proxy.
 type Proxy struct {
-	proxyID    string
 	client     *api.Client
 	cfgWatcher ConfigWatcher
 	stopChan   chan struct{}
@@ -19,51 +18,17 @@ type Proxy struct {
 	service    *connect.Service
 }
 
-// NewFromConfigFile returns a Proxy instance configured just from a local file.
+// New returns a proxy with the given configuration source.
-// This is intended mostly for development and bypasses the normal mechanisms
+//
-// for fetching config and certificates from the local agent.
+// The ConfigWatcher can be used to update the configuration of the proxy.
-func NewFromConfigFile(client *api.Client, filename string,
+// Whenever a new configuration is detected, the proxy will reconfigure itself.
-	logger *log.Logger) (*Proxy, error) {
+func New(client *api.Client, cw ConfigWatcher, logger *log.Logger) (*Proxy, error) {
-	cfg, err := ParseConfigFile(filename)
+	return &Proxy{
-	if err != nil {
-		return nil, err
-	}
-
-	service, err := connect.NewDevServiceFromCertFiles(cfg.ProxiedServiceID,
-		logger, cfg.DevCAFile, cfg.DevServiceCertFile,
-		cfg.DevServiceKeyFile)
-	if err != nil {
-		return nil, err
-	}
-
-	p := &Proxy{
-		proxyID:    cfg.ProxyID,
-		client:     client,
-		cfgWatcher: NewStaticConfigWatcher(cfg),
-		stopChan:   make(chan struct{}),
-		logger:     logger,
-		service:    service,
-	}
-	return p, nil
-}
-
-// New returns a Proxy with the given id, consuming the provided (configured)
-// agent. It is ready to Run().
-func New(client *api.Client, proxyID string, logger *log.Logger) (*Proxy, error) {
-	cw, err := NewAgentConfigWatcher(client, proxyID, logger)
-	if err != nil {
-		return nil, err
-	}
-	p := &Proxy{
-		proxyID:    proxyID,
 		client:     client,
 		cfgWatcher: cw,
 		stopChan:   make(chan struct{}),
 		logger:     logger,
-		// Can't load service yet as we only have the proxy's ID not the service's
+	}, nil
-		// until initial config fetch happens.
-	}
-	return p, nil
 }
 
 // Serve the proxy instance until a fatal error occurs or proxy is closed.
@@ -80,8 +45,7 @@ func (p *Proxy) Serve() error {
 				// Initial setup
 
 				// Setup Service instance now we know target ID etc
-				service, err := connect.NewServiceWithLogger(newCfg.ProxiedServiceID,
+				service, err := cfg.Service(p.client, p.logger)
-					p.client, p.logger)
 				if err != nil {
 					return err
 				}

connect/proxy/testdata/config-kitchensink.hcl

@@ -1,9 +1,8 @@
 # Example proxy config with everything specified
 
-proxy_id = "foo"
 token = "11111111-2222-3333-4444-555555555555"
 
-proxied_service_id = "web"
+proxied_service_name = "web"
 proxied_service_namespace = "default"
 
 # Assumes running consul in dev mode from the repo root...

connect/service.go

@@ -27,16 +27,12 @@ import (
 // service has been delivered valid certificates. Once built, document that here
 // too.
 type Service struct {
-	// serviceID is the unique ID for this service in the agent-local catalog.
+	// service is the name (not ID) for the Consul service. This is used to request
-	// This is often but not always the service name. This is used to request
+	// Connect metadata.
-	// Connect metadata. If the service with this ID doesn't exist on the local
+	service string
-	// agent no error will be returned and the Service will retry periodically.
-	// This allows service startup and registration to happen in either order
-	// without coordination since they might be performed by separate processes.
-	serviceID string
 
 	// client is the Consul API client. It must be configured with an appropriate
-	// Token that has `service:write` policy on the provided ServiceID. If an
+	// Token that has `service:write` policy on the provided service. If an
 	// insufficient token is provided, the Service will abort further attempts to
 	// fetch certificates and print a loud error message. It will not Close() or
 	// kill the process since that could lead to a crash loop in every service if
@@ -74,13 +70,13 @@ func NewService(serviceID string, client *api.Client) (*Service, error) {
 }
 
 // NewServiceWithLogger starts the service with a specified log.Logger.
-func NewServiceWithLogger(serviceID string, client *api.Client,
+func NewServiceWithLogger(serviceName string, client *api.Client,
 	logger *log.Logger) (*Service, error) {
 	s := &Service{
-		serviceID: serviceID,
+		service: serviceName,
-		client:    client,
+		client:  client,
-		logger:    logger,
+		logger:  logger,
-		tlsCfg:    newDynamicTLSConfig(defaultTLSConfig()),
+		tlsCfg:  newDynamicTLSConfig(defaultTLSConfig()),
 	}
 
 	// Set up root and leaf watches
@@ -94,8 +90,8 @@ func NewServiceWithLogger(serviceID string, client *api.Client,
 	s.rootsWatch.HybridHandler = s.rootsWatchHandler
 
 	p, err = watch.Parse(map[string]interface{}{
-		"type":       "connect_leaf",
+		"type":    "connect_leaf",
-		"service_id": s.serviceID,
+		"service": s.service,
 	})
 	if err != nil {
 		return nil, err
@@ -123,12 +119,12 @@ func NewDevServiceFromCertFiles(serviceID string, logger *log.Logger,
 
 // NewDevServiceWithTLSConfig creates a Service using static TLS config passed.
 // It's mostly useful for testing.
-func NewDevServiceWithTLSConfig(serviceID string, logger *log.Logger,
+func NewDevServiceWithTLSConfig(serviceName string, logger *log.Logger,
 	tlsCfg *tls.Config) (*Service, error) {
 	s := &Service{
-		serviceID: serviceID,
+		service: serviceName,
-		logger:    logger,
+		logger:  logger,
-		tlsCfg:    newDynamicTLSConfig(tlsCfg),
+		tlsCfg:  newDynamicTLSConfig(tlsCfg),
 	}
 	return s, nil
 }
@@ -144,7 +140,7 @@ func NewDevServiceWithTLSConfig(serviceID string, logger *log.Logger,
 // error during renewal. The listener will be able to accept connections again
 // once connectivity is restored provided the client's Token is valid.
 func (s *Service) ServerTLSConfig() *tls.Config {
-	return s.tlsCfg.Get(newServerSideVerifier(s.client, s.serviceID))
+	return s.tlsCfg.Get(newServerSideVerifier(s.client, s.service))
 }
 
 // Dial connects to a remote Connect-enabled server. The passed Resolver is used

connect/tls.go

@@ -112,9 +112,9 @@ func verifyServerCertMatchesURI(certs []*x509.Certificate,
 
 // newServerSideVerifier returns a verifierFunc that wraps the provided
 // api.Client to verify the TLS chain and perform AuthZ for the server end of
-// the connection. The service name provided is used as the target serviceID
+// the connection. The service name provided is used as the target service name
 // for the Authorization.
-func newServerSideVerifier(client *api.Client, serviceID string) verifierFunc {
+func newServerSideVerifier(client *api.Client, serviceName string) verifierFunc {
 	return func(tlsCfg *tls.Config, rawCerts [][]byte) error {
 		leaf, err := verifyChain(tlsCfg, rawCerts, false)
 		if err != nil {
@@ -142,14 +142,7 @@ func newServerSideVerifier(client *api.Client, serviceID string) verifierFunc {
 
 		// Perform AuthZ
 		req := &api.AgentAuthorizeParams{
-			// TODO(banks): this is jank, we have a serviceID from the Service setup
+			Target:           serviceName,
-			// but this needs to be a service name as the target. For now we are
-			// relying on them usually being the same but this will break when they
-			// are not. We either need to make Authorize endpoint optionally accept
-			// IDs somehow or rethink this as it will require fetching the service
-			// name sometime ahead of accepting requests (maybe along with TLS certs?)
-			// which feels gross and will take extra plumbing to expose it to here.
-			Target:           serviceID,
 			ClientCertURI:    certURI.URI().String(),
 			ClientCertSerial: connect.HexString(leaf.SerialNumber.Bytes()),
 		}

watch/funcs.go

@@ -258,8 +258,8 @@ func connectRootsWatch(params map[string]interface{}) (WatcherFunc, error) {
 func connectLeafWatch(params map[string]interface{}) (WatcherFunc, error) {
 	// We don't support stale since certs are cached locally in the agent.
 
-	var serviceID string
+	var serviceName string
-	if err := assignValue(params, "service_id", &serviceID); err != nil {
+	if err := assignValue(params, "service", &serviceName); err != nil {
 		return nil, err
 	}
 
@@ -268,7 +268,7 @@ func connectLeafWatch(params map[string]interface{}) (WatcherFunc, error) {
 		opts := makeQueryOptionsWithContext(p, false)
 		defer p.cancelFunc()
 
-		leaf, meta, err := agent.ConnectCALeaf(serviceID, &opts)
+		leaf, meta, err := agent.ConnectCALeaf(serviceName, &opts)
 		if err != nil {
 			return nil, nil, err
 		}

watch/funcs_test.go

@@ -602,7 +602,7 @@ func TestConnectLeafWatch(t *testing.T) {
 
 	//invoke := makeInvokeCh()
 	invoke := make(chan error)
-	plan := mustParse(t, `{"type":"connect_leaf", "service_id":"web"}`)
+	plan := mustParse(t, `{"type":"connect_leaf", "service":"web"}`)
 	plan.Handler = func(idx uint64, raw interface{}) {
 		if raw == nil {
 			return // ignore