From b28e2b862237773d00025078d9e8c121629673b6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 19 May 2018 00:11:51 -0700 Subject: [PATCH] connect/proxy: don't require proxy ID --- command/connect/proxy/proxy.go | 46 ++++++++++------ connect/proxy/config.go | 47 +++++++++-------- connect/proxy/config_test.go | 7 ++- connect/proxy/proxy.go | 52 +++---------------- connect/proxy/testdata/config-kitchensink.hcl | 3 +- connect/service.go | 36 ++++++------- connect/tls.go | 13 ++--- watch/funcs.go | 6 +-- watch/funcs_test.go | 2 +- 9 files changed, 91 insertions(+), 121 deletions(-) diff --git a/command/connect/proxy/proxy.go b/command/connect/proxy/proxy.go index b797177c5..83406e0fb 100644 --- a/command/connect/proxy/proxy.go +++ b/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 - if c.cfgFile != "" { - c.UI.Info("Configuring proxy locally from " + c.cfgFile) + // Get the proper configuration watcher + cfgWatcher, err := c.configWatcher(client) + if err != nil { + c.UI.Error(fmt.Sprintf("Error preparing configuration: %s", err)) + return 1 + } - p, err = proxyImpl.NewFromConfigFile(client, c.cfgFile, c.logger) - if err != nil { - c.UI.Error(fmt.Sprintf("Failed configuring from file: %s", err)) - 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 - } + p, err := proxyImpl.New(client, cfgWatcher, c.logger) + if err != nil { + c.UI.Error(fmt.Sprintf("Failed initializing proxy: %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 } diff --git a/connect/proxy/config.go b/connect/proxy/config.go index 840afa896..025809a7b 100644 --- a/connect/proxy/config.go +++ b/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. - ProxiedServiceID string `json:"proxied_service_id" hcl:"proxied_service_id"` - + // ProxiedServiceName is the name of the service this proxy is representing. + // 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 - // bundle to be used in development instead of the ones supplied by Connect. - 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. + // DevCAFile, DevServiceCertFile, and DevServiceKeyFile allow configuring + // the certificate information from a static file. This is only for testing + // purposes. All or none must be specified. + DevCAFile string `json:"dev_ca_file" hcl:"dev_ca_file"` 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 - // private key to be used in development instead of the ones supplied by - // Connect. - DevServiceKeyFile string `json:"dev_service_key_file" hcl:"dev_service_key_file"` +// Service returns the *connect.Service structure represented by this config. +func (c *Config) Service(client *api.Client, logger *log.Logger) (*connect.Service, error) { + // If we aren't in dev mode, then we return the configured service. + 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", } diff --git a/connect/proxy/config_test.go b/connect/proxy/config_test.go index 1473e8fea..87bb43c81 100644 --- a/connect/proxy/config_test.go +++ b/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, - ProxiedServiceID: "web", + ProxiedServiceName: "web", ProxiedServiceNamespace: "default", PublicListener: PublicListenerConfig{ BindAddress: "10.10.10.10", diff --git a/connect/proxy/proxy.go b/connect/proxy/proxy.go index 64e098825..fb382288f 100644 --- a/connect/proxy/proxy.go +++ b/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. -// This is intended mostly for development and bypasses the normal mechanisms -// for fetching config and certificates from the local agent. -func NewFromConfigFile(client *api.Client, filename string, - logger *log.Logger) (*Proxy, error) { - cfg, err := ParseConfigFile(filename) - 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, +// New returns a proxy with the given configuration source. +// +// The ConfigWatcher can be used to update the configuration of the proxy. +// Whenever a new configuration is detected, the proxy will reconfigure itself. +func New(client *api.Client, cw ConfigWatcher, logger *log.Logger) (*Proxy, error) { + return &Proxy{ 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 - // until initial config fetch happens. - } - return p, nil + }, 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, - p.client, p.logger) + service, err := cfg.Service(p.client, p.logger) if err != nil { return err } diff --git a/connect/proxy/testdata/config-kitchensink.hcl b/connect/proxy/testdata/config-kitchensink.hcl index fccfdffd0..de3472d0f 100644 --- a/connect/proxy/testdata/config-kitchensink.hcl +++ b/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... diff --git a/connect/service.go b/connect/service.go index af9fbfcb7..b00775c1c 100644 --- a/connect/service.go +++ b/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. - // This is often but not always the service name. This is used to request - // Connect metadata. If the service with this ID doesn't exist on the local - // 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 + // service is the name (not ID) for the Consul service. This is used to request + // Connect metadata. + service 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, - client: client, - logger: logger, - tlsCfg: newDynamicTLSConfig(defaultTLSConfig()), + service: serviceName, + client: client, + logger: logger, + 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", - "service_id": s.serviceID, + "type": "connect_leaf", + "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, - logger: logger, - tlsCfg: newDynamicTLSConfig(tlsCfg), + service: serviceName, + logger: logger, + 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 diff --git a/connect/tls.go b/connect/tls.go index 6f14cd787..db96eb3de 100644 --- a/connect/tls.go +++ b/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 - // 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, + Target: serviceName, ClientCertURI: certURI.URI().String(), ClientCertSerial: connect.HexString(leaf.SerialNumber.Bytes()), } diff --git a/watch/funcs.go b/watch/funcs.go index 3b1b854ed..a87fd63f4 100644 --- a/watch/funcs.go +++ b/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 - if err := assignValue(params, "service_id", &serviceID); err != nil { + var serviceName string + 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 } diff --git a/watch/funcs_test.go b/watch/funcs_test.go index b304a803f..9632ef206 100644 --- a/watch/funcs_test.go +++ b/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