From c1ed6d1033d2b72056b5d22769f0c38060b38046 Mon Sep 17 00:00:00 2001 From: Michi Mutsuzaki Date: Mon, 4 May 2020 15:11:04 -0700 Subject: [PATCH 1/3] hubble: Use a single string to configure the server address - Rename the option to `--hubble-listen-address` to make it clear that Hubble listens to at most one additional address. - Remove an info message that doesn't provide any addtional info: ``` level=info msg="Starting local Hubble server" address="unix:///var/run/cilium/hubble.sock" subsys=hubble level=info msg="Starting Hubble server" address=":4244" subsys=hubble - level=info msg="Starting gRPC server on listener" listener="unix:///var/run/cilium/hubble.sock" subsys=hubble - level=info msg="Starting gRPC server on listener" listener=":4244" subsys=hubble ``` - Use port 4244 in the documentation since this is the port that Hubble relay expects. Signed-off-by: Michi Mutsuzaki --- Documentation/cmdref/cilium-agent.md | 2 +- daemon/cmd/daemon_main.go | 4 ++-- daemon/cmd/hubble.go | 18 ++++++++---------- .../charts/config/templates/configmap.yaml | 8 ++------ install/kubernetes/cilium/values.yaml | 13 +++++-------- pkg/hubble/server/server.go | 1 - pkg/option/config.go | 12 ++++++------ 7 files changed, 24 insertions(+), 34 deletions(-) diff --git a/Documentation/cmdref/cilium-agent.md b/Documentation/cmdref/cilium-agent.md index 665080fdd430..f41d4c758d4f 100644 --- a/Documentation/cmdref/cilium-agent.md +++ b/Documentation/cmdref/cilium-agent.md @@ -99,7 +99,7 @@ cilium-agent [flags] --http-retry-timeout uint Time after which a forwarded but uncompleted request is retried (connection failures are retried immediately); defaults to 0 (never) --hubble-event-queue-size int Buffer size of the channel to receive monitor events. --hubble-flow-buffer-size int Maximum number of flows in Hubble's buffer. The actual buffer size gets rounded up to the next power of 2, e.g. 4095 => 4096 (default 4095) - --hubble-listen-addresses strings List of additional addresses for Hubble server to listen to + --hubble-listen-address string An additional address for Hubble server to listen to, e.g. ":4244" --hubble-metrics strings List of Hubble metrics to enable. --hubble-metrics-server string Address to serve Hubble metrics on. --hubble-socket-path string Set hubble's socket path to listen for connections (default "/var/run/cilium/hubble.sock") diff --git a/daemon/cmd/daemon_main.go b/daemon/cmd/daemon_main.go index 46dacc6e7bb7..234974cb955b 100644 --- a/daemon/cmd/daemon_main.go +++ b/daemon/cmd/daemon_main.go @@ -752,8 +752,8 @@ func init() { flags.String(option.HubbleSocketPath, defaults.HubbleSockPath, "Set hubble's socket path to listen for connections") option.BindEnv(option.HubbleSocketPath) - flags.StringSlice(option.HubbleListenAddresses, []string{}, "List of additional addresses for Hubble server to listen to") - option.BindEnv(option.HubbleListenAddresses) + flags.String(option.HubbleListenAddress, "", `An additional address for Hubble server to listen to, e.g. ":4244"`) + option.BindEnv(option.HubbleListenAddress) flags.Int(option.HubbleFlowBufferSize, 4095, "Maximum number of flows in Hubble's buffer. The actual buffer size gets rounded up to the next power of 2, e.g. 4095 => 4096") option.BindEnv(option.HubbleFlowBufferSize) diff --git a/daemon/cmd/hubble.go b/daemon/cmd/hubble.go index b3faa387ca1a..68da7f4bf8b5 100644 --- a/daemon/cmd/hubble.go +++ b/daemon/cmd/hubble.go @@ -84,13 +84,6 @@ func (d *Daemon) launchHubble() { logger.Info("Hubble server is disabled") return } - addresses := option.Config.HubbleListenAddresses - for _, address := range addresses { - // TODO: remove warning once mutual TLS has been implemented - if !strings.HasPrefix(address, "unix://") { - logger.WithField("address", address).Warn("Hubble server will be exposing its API insecurely on this address") - } - } payloadParser, err := parser.New(d, d, d, ipcache.IPIdentityCache, d) if err != nil { @@ -131,9 +124,14 @@ func (d *Daemon) launchHubble() { }() // configure another hubble instance that serve fewer gRPC services - if len(addresses) > 0 { + address := option.Config.HubbleListenAddress + if address != "" { + // TODO: remove warning once mutual TLS has been implemented + if !strings.HasPrefix(address, "unix://") { + logger.WithField("address", address).Warn("Hubble server will be exposing its API insecurely on this address") + } srv, err := server.NewServer(logger, - serveroption.WithListeners(addresses), + serveroption.WithListeners([]string{address}), serveroption.WithHealthService(), serveroption.WithObserverService(d.hubbleObserver), ) @@ -141,7 +139,7 @@ func (d *Daemon) launchHubble() { logger.WithError(err).Error("Failed to initialize Hubble server") return } - logger.WithField("addresses", addresses).Info("Starting Hubble server") + logger.WithField("address", address).Info("Starting Hubble server") if err := srv.Serve(); err != nil { logger.WithError(err).Error("Failed to start Hubble server") return diff --git a/install/kubernetes/cilium/charts/config/templates/configmap.yaml b/install/kubernetes/cilium/charts/config/templates/configmap.yaml index 80dfb096b4a4..ee4bcd4b9861 100644 --- a/install/kubernetes/cilium/charts/config/templates/configmap.yaml +++ b/install/kubernetes/cilium/charts/config/templates/configmap.yaml @@ -435,12 +435,8 @@ data: {{.}} {{- end }} {{- end }} - # A space separated list of additional addresses for Hubble server to listen to (e.g. ":50051 :50052"). -{{- if and .Values.global.hubble.ui.enabled (not (has "0.0.0.0:50051" .Values.global.hubble.listenAddresses)) }} - hubble-listen-addresses: {{ append .Values.global.hubble.listenAddresses "0.0.0.0:50051" | join " " | quote }} -{{- else }} - hubble-listen-addresses: {{ .Values.global.hubble.listenAddresses | join " " | quote }} -{{- end }} + # An additional address for Hubble server to listen to (e.g. ":4244"). + hubble-listen-address: {{ .Values.global.hubble.listenAddress | quote }} {{- end }} # A space separated list of iptables chains to disable when installing feeder rules. diff --git a/install/kubernetes/cilium/values.yaml b/install/kubernetes/cilium/values.yaml index 656af1125ba9..356cc3fdd9a6 100644 --- a/install/kubernetes/cilium/values.yaml +++ b/install/kubernetes/cilium/values.yaml @@ -432,16 +432,13 @@ global: enabled: false # Default unix domain socket path to listen to when Hubble is enabled. Default to "/var/run/cilium/hubble.sock". socketPath: /var/run/cilium/hubble.sock - # List of additional addresses to listen to, for example: + # An additional address to listen to, for example: # - # listenAddresses: - # - ":50051" - # - ":50052" + # listenAddress: ":4244" # - # You can specify the list of metrics from the helm CLI: - # - # --set global.hubble.listenAddresses={:50051,:50052} - listenAddresses: [] + # Set this field ":4244" if you are enabling hubble-relay, as it assumes that Hubble is listening + # on port 4244. + listenAddress: "" # Buffer size of the channel Hubble uses to receive monitor events. If this value is not set, # the queue size is set to the default monitor queue size. eventQueueSize: ~ diff --git a/pkg/hubble/server/server.go b/pkg/hubble/server/server.go index aa3d27924dca..b0a2506082eb 100644 --- a/pkg/hubble/server/server.go +++ b/pkg/hubble/server/server.go @@ -70,7 +70,6 @@ func (s *Server) Serve() error { s.initGRPCServer() for name, listener := range s.opts.Listeners { go func(name string, listener net.Listener) { - s.log.WithField("listener", name).Info("Starting gRPC server on listener") if err := s.srv.Serve(listener); err != nil { s.log.WithError(err).Error("failed to close grpc server") } diff --git a/pkg/option/config.go b/pkg/option/config.go index babc26af5453..79926d16ce26 100644 --- a/pkg/option/config.go +++ b/pkg/option/config.go @@ -725,8 +725,8 @@ const ( // HubbleSocketPath specifies the UNIX domain socket for Hubble server to listen to. HubbleSocketPath = "hubble-socket-path" - // HubbleListenAddresses specifies addresses for Hubble server to listen to. - HubbleListenAddresses = "hubble-listen-addresses" + // HubbleListenAddress specifies address for Hubble server to listen to. + HubbleListenAddress = "hubble-listen-address" // HubbleFlowBufferSize specifies the maximum number of flows in Hubble's buffer. HubbleFlowBufferSize = "hubble-flow-buffer-size" @@ -968,7 +968,7 @@ var HelpFlagSections = []FlagsSection{ Flags: []string{ EnableHubble, HubbleSocketPath, - HubbleListenAddresses, + HubbleListenAddress, HubbleFlowBufferSize, HubbleEventQueueSize, HubbleMetricsServer, @@ -1733,8 +1733,8 @@ type DaemonConfig struct { // HubbleSocketPath specifies the UNIX domain socket for Hubble server to listen to. HubbleSocketPath string - // HubbleListenAddresses specifies addresses for Hubble to listen to. - HubbleListenAddresses []string + // HubbleListenAddress specifies address for Hubble to listen to. + HubbleListenAddress string // HubbleFlowBufferSize specifies the maximum number of flows in Hubble's buffer. HubbleFlowBufferSize int @@ -2422,7 +2422,7 @@ func (c *DaemonConfig) Populate() { // Hubble options. c.EnableHubble = viper.GetBool(EnableHubble) c.HubbleSocketPath = viper.GetString(HubbleSocketPath) - c.HubbleListenAddresses = viper.GetStringSlice(HubbleListenAddresses) + c.HubbleListenAddress = viper.GetString(HubbleListenAddress) c.HubbleFlowBufferSize = viper.GetInt(HubbleFlowBufferSize) c.HubbleEventQueueSize = viper.GetInt(HubbleEventQueueSize) if c.HubbleEventQueueSize == 0 { From 26a98b70f2e40f335f73dd46d6ceb90fcb4efb41 Mon Sep 17 00:00:00 2001 From: Michi Mutsuzaki Date: Mon, 4 May 2020 16:01:39 -0700 Subject: [PATCH 2/3] hubble: Point hubble-ui to hubble-relay service - Configure hubble-ui to connect to hubble-relay:80. Now hubble-relay is responsible for retrieving flows from Cilium instances. - Remove hubble-grpc service. Signed-off-by: Michi Mutsuzaki --- .../cilium/charts/agent/templates/svc.yaml | 19 ------------------- .../hubble-ui/templates/deployment.yaml | 4 ++-- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/install/kubernetes/cilium/charts/agent/templates/svc.yaml b/install/kubernetes/cilium/charts/agent/templates/svc.yaml index 675612b9e1ae..c374e314c004 100644 --- a/install/kubernetes/cilium/charts/agent/templates/svc.yaml +++ b/install/kubernetes/cilium/charts/agent/templates/svc.yaml @@ -17,25 +17,6 @@ spec: selector: k8s-app: cilium {{- end }} -{{- if .Values.global.hubble.ui.enabled }} ---- -kind: Service -apiVersion: v1 -metadata: - name: hubble-grpc - namespace: {{ .Release.Namespace }} - labels: - k8s-app: hubble -spec: - type: ClusterIP - clusterIP: None - selector: - k8s-app: cilium - ports: - - targetPort: 50051 - protocol: TCP - port: 50051 -{{- end }} {{- if and .Values.global.hubble.metrics.enabled (.Values.global.hubble.metrics.serviceMonitor.enabled) }} --- kind: Service diff --git a/install/kubernetes/cilium/charts/hubble-ui/templates/deployment.yaml b/install/kubernetes/cilium/charts/hubble-ui/templates/deployment.yaml index 128da84b8fc6..711efe7c82d6 100644 --- a/install/kubernetes/cilium/charts/hubble-ui/templates/deployment.yaml +++ b/install/kubernetes/cilium/charts/hubble-ui/templates/deployment.yaml @@ -26,9 +26,9 @@ spec: - name: HUBBLE value: "true" - name: HUBBLE_SERVICE - value: "hubble-grpc.{{ .Release.Namespace }}.svc.{{ .Values.clusterDomain }}" + value: "hubble-relay.{{ .Release.Namespace }}.svc.{{ .Values.clusterDomain }}" - name: HUBBLE_PORT - value: "50051" + value: "80" ports: - containerPort: 12000 name: http From 5b6be3eabad42a295f691381af099b356efb98fa Mon Sep 17 00:00:00 2001 From: Michi Mutsuzaki Date: Tue, 5 May 2020 11:01:08 -0700 Subject: [PATCH 3/3] hubble: Remove serveroption.WithListeners Now that Hubble has 1 unix domain socket and at most 1 TCP socket to serve, we can simply use `With{UnixSocket,TCP}Listener` options. Signed-off-by: Michi Mutsuzaki --- daemon/cmd/hubble.go | 7 ++-- pkg/hubble/server/server.go | 20 ++++++------ pkg/hubble/server/serveroption/option.go | 41 ++++++------------------ 3 files changed, 22 insertions(+), 46 deletions(-) diff --git a/daemon/cmd/hubble.go b/daemon/cmd/hubble.go index 68da7f4bf8b5..4d8add612340 100644 --- a/daemon/cmd/hubble.go +++ b/daemon/cmd/hubble.go @@ -16,7 +16,6 @@ package cmd import ( "context" - "strings" "time" "github.com/cilium/cilium/api/v1/models" @@ -127,11 +126,9 @@ func (d *Daemon) launchHubble() { address := option.Config.HubbleListenAddress if address != "" { // TODO: remove warning once mutual TLS has been implemented - if !strings.HasPrefix(address, "unix://") { - logger.WithField("address", address).Warn("Hubble server will be exposing its API insecurely on this address") - } + logger.WithField("address", address).Warn("Hubble server will be exposing its API insecurely on this address") srv, err := server.NewServer(logger, - serveroption.WithListeners([]string{address}), + serveroption.WithTCPListener(address), serveroption.WithHealthService(), serveroption.WithObserverService(d.hubbleObserver), ) diff --git a/pkg/hubble/server/server.go b/pkg/hubble/server/server.go index b0a2506082eb..921fe7dcccc5 100644 --- a/pkg/hubble/server/server.go +++ b/pkg/hubble/server/server.go @@ -38,9 +38,7 @@ type Server struct { // NewServer creates a new hubble gRPC server. func NewServer(log *logrus.Entry, options ...serveroption.Option) (*Server, error) { - opts := serveroption.Options{ - Listeners: make(map[string]net.Listener), - } + opts := serveroption.Options{} for _, opt := range options { if err := opt(&opts); err != nil { return nil, fmt.Errorf("failed to apply option: %v", err) @@ -68,12 +66,16 @@ func (s *Server) initGRPCServer() { // listeners. Stop should be called to stop the server. func (s *Server) Serve() error { s.initGRPCServer() - for name, listener := range s.opts.Listeners { - go func(name string, listener net.Listener) { - if err := s.srv.Serve(listener); err != nil { - s.log.WithError(err).Error("failed to close grpc server") - } - }(name, listener) + for _, listener := range []net.Listener{s.opts.UnixSocketListener, s.opts.TCPListener} { + if listener != nil { + go func(listener net.Listener) { + if err := s.srv.Serve(listener); err != nil { + s.log.WithError(err). + WithField("address", listener.Addr().String()). + Error("failed to start grpc server") + } + }(listener) + } } return nil } diff --git a/pkg/hubble/server/serveroption/option.go b/pkg/hubble/server/serveroption/option.go index 4fc4147aa92f..085e48549a96 100644 --- a/pkg/hubble/server/serveroption/option.go +++ b/pkg/hubble/server/serveroption/option.go @@ -33,39 +33,16 @@ import ( // Options stores all the configuration values for the hubble server. type Options struct { - Listeners map[string]net.Listener - HealthService healthpb.HealthServer - ObserverService observerpb.ObserverServer - PeerService peerpb.PeerServer + TCPListener net.Listener + UnixSocketListener net.Listener + HealthService healthpb.HealthServer + ObserverService observerpb.ObserverServer + PeerService peerpb.PeerServer } // Option customizes then configuration of the hubble server. type Option func(o *Options) error -// WithListeners configures listeners. Addresses that are prefixed with -// 'unix://' are assumed to be UNIX domain sockets, in which case appropriate -// permissions are tentatively set and the group owner is set to socketGroup. -// Otherwise, the address is assumed to be TCP. -func WithListeners(addresses []string) Option { - return func(o *Options) error { - var opt Option - for _, address := range addresses { - if strings.HasPrefix(address, "unix://") { - opt = WithUnixSocketListener(address) - } else { - opt = WithTCPListener(address) - } - if err := opt(o); err != nil { - for _, l := range o.Listeners { - l.Close() - } - return err - } - } - return nil - } -} - // WithTCPListener configures a TCP listener with the address. func WithTCPListener(address string) Option { return func(o *Options) error { @@ -73,11 +50,11 @@ func WithTCPListener(address string) Option { if err != nil { return err } - if _, exist := o.Listeners[address]; exist { + if o.TCPListener != nil { socket.Close() return fmt.Errorf("listener already configured: %s", address) } - o.Listeners[address] = socket + o.TCPListener = socket return nil } } @@ -98,12 +75,12 @@ func WithUnixSocketListener(path string) Option { return err } } - if _, exist := o.Listeners[path]; exist { + if o.UnixSocketListener != nil { socket.Close() unix.Unlink(socketPath) return fmt.Errorf("listener already configured: %s", path) } - o.Listeners[path] = socket + o.UnixSocketListener = socket return nil } }