Skip to content

Commit

Permalink
Disable exposing metrics server on all interfaces (#3662) (#3670)
Browse files Browse the repository at this point in the history
This PR disables option to configure agent in a way that it will expose monitoring agent on all interfaces.
Empty value for agent.monitoring.http.host is now disabled and is replaced by localhost

Agents upgraded from <8.5 do have empty host due to change in defaults in 8.5. These agents are exposing metrics endpoint on all interfaces not on purpose.

Guard is implemented at the time of configuration unpacking and at the time of server creation to minimize future misbehavior with possible code changes

(cherry picked from commit 2afea75)

Co-authored-by: Michal Pristas <[email protected]>
  • Loading branch information
mergify[bot] and michalpristas authored Oct 30, 2023
1 parent c1de297 commit 2ff5f63
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 4 deletions.
5 changes: 5 additions & 0 deletions internal/pkg/agent/application/monitoring/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/elastic/elastic-agent-libs/config"
"github.com/elastic/elastic-agent-libs/monitoring"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/coordinator"
monitoringCfg "github.com/elastic/elastic-agent/internal/pkg/core/monitoring/config"
"github.com/elastic/elastic-agent/pkg/core/logger"
)

Expand All @@ -38,6 +39,10 @@ func NewServer(
log.Warnf("failed to create monitoring drop: %v", err)
}

if strings.TrimSpace(endpointConfig.Host) == "" {
endpointConfig.Host = monitoringCfg.DefaultHost
}

cfg, err := config.NewConfigFrom(endpointConfig)
if err != nil {
return nil, err
Expand Down
51 changes: 47 additions & 4 deletions internal/pkg/core/monitoring/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,20 @@

package config

import "time"
import (
"strings"
"time"

const defaultPort = 6791
const defaultNamespace = "default"
c "github.com/elastic/elastic-agent-libs/config"
)

const (
defaultPort = 6791
defaultNamespace = "default"

// DefaultHost is used when host is not defined or empty
DefaultHost = "localhost"
)

// MonitoringConfig describes a configuration of a monitoring
type MonitoringConfig struct {
Expand All @@ -33,6 +43,39 @@ type MonitoringHTTPConfig struct {
Buffer *BufferConfig `yaml:"buffer" config:"buffer"`
}

// Unpack reads a config object into the settings.
func (c *MonitoringHTTPConfig) Unpack(cfg *c.C) error {
// do not use MonitoringHTTPConfig, it will end up in a loop
tmp := struct {
Enabled bool `yaml:"enabled" config:"enabled"`
Host string `yaml:"host" config:"host"`
Port int `yaml:"port" config:"port" validate:"min=0,max=65535,nonzero"`
Buffer *BufferConfig `yaml:"buffer" config:"buffer"`
}{
Enabled: c.Enabled,
Host: c.Host,
Port: c.Port,
Buffer: c.Buffer,
}

if err := cfg.Unpack(&tmp); err != nil {
return err
}

if strings.TrimSpace(tmp.Host) == "" {
tmp.Host = DefaultHost
}

*c = MonitoringHTTPConfig{
Enabled: tmp.Enabled,
Host: tmp.Host,
Port: tmp.Port,
Buffer: tmp.Buffer,
}

return nil
}

// PprofConfig is a struct for the pprof enablement flag.
// It is a nil struct by default to allow the agent to use the a value that the user has injected into fleet.yml as the source of truth that is passed to beats
// TODO get this value from Kibana?
Expand All @@ -55,7 +98,7 @@ func DefaultConfig() *MonitoringConfig {
MonitorTraces: false,
HTTP: &MonitoringHTTPConfig{
Enabled: false,
Host: "localhost",
Host: DefaultHost,
Port: defaultPort,
},
Namespace: defaultNamespace,
Expand Down
51 changes: 51 additions & 0 deletions internal/pkg/core/monitoring/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,57 @@ import (
"github.com/elastic/elastic-agent/internal/pkg/config"
)

func TestHost(t *testing.T) {
testCases := []struct {
name string
config string
expectedHost string
}{
{"no host", `enabled: true
logs: true
metrics: true
http:
enabled: true`, DefaultHost},
{"empty host", `enabled: true
logs: true
metrics: true
http:
enabled: true
host: ""`, DefaultHost},
{"whitespace host", `enabled: true
logs: true
metrics: true
http:
enabled: true
host: " "`, DefaultHost},
{"default", `enabled: true
logs: true
metrics: true
http:
enabled: true
host: localhost`, DefaultHost},
{"custom host", `enabled: true
logs: true
metrics: true
http:
enabled: true
host: custom`, "custom"},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
c, err := config.NewConfigFrom(tc.config)
require.NoError(t, err, "failed to create config")

cfg := DefaultConfig()
err = c.Unpack(&cfg)
require.NoError(t, err, "failed to unpack config")

require.Equal(t, tc.expectedHost, cfg.HTTP.Host)
})
}
}

func TestAPMConfig(t *testing.T) {
tcs := map[string]struct {
in map[string]interface{}
Expand Down

0 comments on commit 2ff5f63

Please sign in to comment.