Skip to content

Commit

Permalink
Use config for windows named pipe path
Browse files Browse the repository at this point in the history
  • Loading branch information
brycekahle committed Nov 15, 2024
1 parent 8c1d827 commit 9df73e5
Show file tree
Hide file tree
Showing 16 changed files with 38 additions and 95 deletions.
1 change: 0 additions & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,6 @@
/pkg/process/net/ @DataDog/universal-service-monitoring @DataDog/Networks
/pkg/process/net/common_windows.go @DataDog/windows-agent
/pkg/process/net/windows_pipe.go @DataDog/windows-kernel-integrations
/pkg/process/net/windows_pipe_testutil.go @DataDog/windows-kernel-integrations
/pkg/proto/datadog/remoteconfig/ @DataDog/remote-config
/pkg/proto/pbgo/ # do not notify anyone
/pkg/proto/pbgo/trace @DataDog/agent-apm
Expand Down
2 changes: 2 additions & 0 deletions cmd/agent/subcommands/flare/command_other_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
"github.com/DataDog/datadog-agent/pkg/config/model"
)

const systemProbeTestPipeName = ""

// NewSystemProbeTestServer starts a new mock server to handle System Probe requests.
func NewSystemProbeTestServer(_ http.Handler) (*httptest.Server, error) {
// Linux still uses a port-based system-probe, it does not need a dedicated system probe server
Expand Down
20 changes: 9 additions & 11 deletions cmd/agent/subcommands/flare/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ type commandTestSuite struct {

func (c *commandTestSuite) SetupSuite() {
t := c.T()
c.sysprobeSocketPath = path.Join(t.TempDir(), "sysprobe.sock")
if runtime.GOOS == "windows" {
c.sysprobeSocketPath = systemProbeTestPipeName
} else {
c.sysprobeSocketPath = path.Join(t.TempDir(), "sysprobe.sock")
}
}

// startTestServers starts test servers from a clean state to ensure no cache responses are used.
Expand Down Expand Up @@ -125,11 +129,9 @@ func (c *commandTestSuite) TestReadProfileData() {
mockConfig.SetWithoutSource("process_config.expvar_port", port)
mockConfig.SetWithoutSource("security_agent.expvar_port", port)

mockSysProbeConfig := configmock.NewSystemProbe(t)
mockSysProbeConfig.SetWithoutSource("system_probe_config.enabled", true)
if runtime.GOOS == "windows" {
mockSysProbeConfig.SetWithoutSource("system_probe_config.sysprobe_socket", u.Host)
} else {
if runtime.GOOS != "darwin" {
mockSysProbeConfig := configmock.NewSystemProbe(t)
mockSysProbeConfig.SetWithoutSource("system_probe_config.enabled", true)
mockSysProbeConfig.SetWithoutSource("system_probe_config.sysprobe_socket", c.sysprobeSocketPath)
}

Expand Down Expand Up @@ -197,11 +199,7 @@ func (c *commandTestSuite) TestReadProfileDataNoTraceAgent() {

mockSysProbeConfig := configmock.NewSystemProbe(t)
mockSysProbeConfig.SetWithoutSource("system_probe_config.enabled", true)
if runtime.GOOS == "windows" {
mockSysProbeConfig.SetWithoutSource("system_probe_config.sysprobe_socket", u.Host)
} else {
mockSysProbeConfig.SetWithoutSource("system_probe_config.sysprobe_socket", c.sysprobeSocketPath)
}
mockSysProbeConfig.SetWithoutSource("system_probe_config.sysprobe_socket", c.sysprobeSocketPath)

data, err := readProfileData(10)
require.Error(t, err)
Expand Down
14 changes: 2 additions & 12 deletions cmd/agent/subcommands/flare/command_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,13 @@ import (
const (
// SystemProbeTestPipeName is the test named pipe for system-probe
systemProbeTestPipeName = `\\.\pipe\dd_system_probe_flare_test`

// systemProbeTestPipeSecurityDescriptor has a DACL that allows Everyone access for these tests.
systemProbeTestPipeSecurityDescriptor = "D:PAI(A;;FA;;;WD)"
)

// NewSystemProbeTestServer starts a new mock server to handle System Probe requests.
func NewSystemProbeTestServer(handler http.Handler) (*httptest.Server, error) {
server := httptest.NewUnstartedServer(handler)

// Override the named pipe path for tests to avoid conflicts with the locally installed Datadog agent.
processNet.OverrideSystemProbeNamedPipeConfig(
systemProbeTestPipeName,
systemProbeTestPipeSecurityDescriptor)

conn, err := processNet.NewSystemProbeListener("")
conn, err := processNet.NewSystemProbeListener(systemProbeTestPipeName)
if err != nil {
return nil, err
}
Expand All @@ -52,9 +44,7 @@ func InjectConnectionFailures(mockSysProbeConfig model.Config, mockConfig model.
// Exercise a connection failure for a Windows system probe named pipe client by
// making them use a bad path.
// The system probe http server must be setup before this override.
processNet.OverrideSystemProbeNamedPipeConfig(
`\\.\pipe\dd_system_probe_test_bad`,
systemProbeTestPipeSecurityDescriptor)
mockSysProbeConfig.SetWithoutSource("system_probe_config.sysprobe_socket", `\\.\pipe\dd_system_probe_test_bad`)

// The security-agent connection is expected to fail too in this test, but
// by enabling system probe, a port will be provided to it (security agent).
Expand Down
11 changes: 3 additions & 8 deletions cmd/system-probe/api/client/client_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,20 @@ const (
idleConnTimeout = 5 * time.Second
)

var (
// SystemProbePipeName is the production named pipe for system probe
SystemProbePipeName = `\\.\pipe\dd_system_probe`
)

// DialContextFunc returns a function to be used in http.Transport.DialContext for connecting to system-probe.
// The result will be OS-specific.
func DialContextFunc(_ string) func(context.Context, string, string) (net.Conn, error) {
func DialContextFunc(namedPipePath string) func(context.Context, string, string) (net.Conn, error) {
return func(_ context.Context, _, _ string) (net.Conn, error) {
// Go clients do not immediately close (named pipe) connections when done,
// they keep connections idle for a while. Make sure the idle time
// is not too high and the timeout is generous enough for pending connections.
var timeout = 30 * time.Second

namedPipe, err := winio.DialPipe(SystemProbePipeName, &timeout)
namedPipe, err := winio.DialPipe(namedPipePath, &timeout)
if err != nil {
// This important error may not get reported upstream, making connection failures
// very difficult to diagnose. Explicitly log the error here too for diagnostics.
var namedPipeErr = fmt.Errorf("error connecting to named pipe %q: %s", SystemProbePipeName, err)
var namedPipeErr = fmt.Errorf("error connecting to named pipe %q: %s", namedPipePath, err)
log.Error(namedPipeErr.Error())
return nil, namedPipeErr
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/system-probe/config/adjust.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"sync"

"github.com/DataDog/datadog-agent/pkg/config/model"
"github.com/DataDog/datadog-agent/pkg/config/setup"
"github.com/DataDog/datadog-agent/pkg/util/log"
)

Expand Down Expand Up @@ -40,7 +41,7 @@ func Adjust(cfg model.Config) {
cfg.Set(netNS("enabled"), true, model.SourceAgentRuntime)
}

validateString(cfg, spNS("sysprobe_socket"), defaultSystemProbeAddress, ValidateSocketAddress)
validateString(cfg, spNS("sysprobe_socket"), setup.DefaultSystemProbeAddress, ValidateSocketAddress)

deprecateBool(cfg, spNS("allow_precompiled_fallback"), spNS("allow_prebuilt_fallback"))
allowPrebuiltEbpfFallback(cfg)
Expand Down
3 changes: 0 additions & 3 deletions cmd/system-probe/config/config_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ import (
)

const (
// defaultSystemProbeAddress is the default unix socket path to be used for connecting to the system probe
defaultSystemProbeAddress = "/opt/datadog-agent/run/sysprobe.sock"

defaultConfigDir = "/etc/datadog-agent"
)

Expand Down
9 changes: 4 additions & 5 deletions cmd/system-probe/config/config_unsupported.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,18 @@
package config

import (
"fmt"
"errors"

"github.com/DataDog/datadog-agent/pkg/config/model"
)

const (
defaultConfigDir = ""
defaultSystemProbeAddress = ""
defaultConfigDir = ""
)

// ValidateSocketAddress validates that the sysprobe socket config option is of the correct format.
// ValidateSocketAddress is not supported on this platform
func ValidateSocketAddress(sockPath string) error {
return fmt.Errorf("system-probe unsupported")
return errors.New("system-probe unsupported")
}

// ProcessEventDataStreamSupported returns true if process event data stream is supported
Expand Down
9 changes: 3 additions & 6 deletions cmd/system-probe/config/config_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,13 @@ package config

import (
"fmt"
"net"
"strings"

"github.com/DataDog/datadog-agent/pkg/config/model"
"github.com/DataDog/datadog-agent/pkg/util/winutil"
)

const (
// defaultSystemProbeAddress is the default address to be used for connecting to the system probe
defaultSystemProbeAddress = "localhost:3333"

// ServiceName is the service name used for the system-probe
ServiceName = "datadog-system-probe"
)
Expand All @@ -36,8 +33,8 @@ func init() {

// ValidateSocketAddress validates that the sysprobe socket config option is of the correct format.
func ValidateSocketAddress(sockAddress string) error {
if _, _, err := net.SplitHostPort(sockAddress); err != nil {
return fmt.Errorf("socket address must be of the form 'host:port'")
if !strings.HasPrefix(sockAddress, `\\.\pipe\`) {
return fmt.Errorf(`named pipe must be of the form '\\.\pipe\<pipename>'`)
}
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,12 @@ import (
)

const (
// SystemProbeTestPipeName is the test named pipe for system-probe
// systemProbeTestPipeName is the test named pipe for system-probe
systemProbeTestPipeName = `\\.\pipe\dd_system_probe_wincrash_test`

// systemProbeTestPipeSecurityDescriptor has a DACL that allows Everyone access for these tests.
systemProbeTestPipeSecurityDescriptor = "D:PAI(A;;FA;;;WD)"
)

func createSystemProbeListener() (l net.Listener, close func()) {
process_net.OverrideSystemProbeNamedPipeConfig(
systemProbeTestPipeName,
systemProbeTestPipeSecurityDescriptor)

// No socket address. Windows uses a fixed name pipe
conn, err := process_net.NewSystemProbeListener("")
conn, err := process_net.NewSystemProbeListener(systemProbeTestPipeName)
if err != nil {
panic(err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/setup/config_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ const (
DefaultProcessAgentLogFile = "/opt/datadog-agent/logs/process-agent.log"
// DefaultOTelAgentLogFile is the default otel-agent log file
DefaultOTelAgentLogFile = "/opt/datadog-agent/logs/otel-agent.log"
// defaultSystemProbeAddress is the default unix socket path to be used for connecting to the system probe
defaultSystemProbeAddress = "/opt/datadog-agent/run/sysprobe.sock"
// DefaultSystemProbeAddress is the default unix socket path to be used for connecting to the system probe
DefaultSystemProbeAddress = "/opt/datadog-agent/run/sysprobe.sock"
// defaultEventMonitorAddress is the default unix socket path to be used for connecting to the event monitor
defaultEventMonitorAddress = "/opt/datadog-agent/run/event-monitor.sock"
defaultSystemProbeLogFilePath = "/opt/datadog-agent/logs/system-probe.log"
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/setup/config_nix.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ var (
)

var (
// defaultSystemProbeAddress is the default unix socket path to be used for connecting to the system probe
defaultSystemProbeAddress = filepath.Join(InstallPath, "run/sysprobe.sock")
// DefaultSystemProbeAddress is the default unix socket path to be used for connecting to the system probe
DefaultSystemProbeAddress = filepath.Join(InstallPath, "run/sysprobe.sock")
// defaultEventMonitorAddress is the default unix socket path to be used for connecting to the event monitor
defaultEventMonitorAddress = filepath.Join(InstallPath, "run/event-monitor.sock")
// DefaultDDAgentBin the process agent's binary
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/setup/config_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ var (
DefaultProcessAgentLogFile = "C:\\ProgramData\\Datadog\\logs\\process-agent.log"
// DefaultOTelAgentLogFile is the default otel-agent log file
DefaultOTelAgentLogFile = "C:\\ProgramData\\Datadog\\logs\\otel-agent.log"
// defaultSystemProbeAddress is the default address to be used for connecting to the system probe
defaultSystemProbeAddress = "localhost:3333"
// DefaultSystemProbeAddress is the default address to be used for connecting to the system probe
DefaultSystemProbeAddress = `\\.\pipe\dd_system_probe`
// defaultEventMonitorAddress is the default address to be used for connecting to the event monitor
defaultEventMonitorAddress = "localhost:3335"
defaultSystemProbeLogFilePath = "c:\\programdata\\datadog\\logs\\system-probe.log"
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/setup/system_probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func InitSystemProbeConfig(cfg pkgconfigmodel.Config) {
cfg.BindEnvAndSetDefault(join(spNS, "external"), false, "DD_SYSTEM_PROBE_EXTERNAL")
cfg.SetKnown(join(spNS, "adjusted"))

cfg.BindEnvAndSetDefault(join(spNS, "sysprobe_socket"), defaultSystemProbeAddress, "DD_SYSPROBE_SOCKET")
cfg.BindEnvAndSetDefault(join(spNS, "sysprobe_socket"), DefaultSystemProbeAddress, "DD_SYSPROBE_SOCKET")
cfg.BindEnvAndSetDefault(join(spNS, "max_conns_per_message"), defaultConnsMessageBatchSize)

cfg.BindEnvAndSetDefault(join(spNS, "debug_port"), 0)
Expand Down
12 changes: 4 additions & 8 deletions pkg/process/net/windows_pipe.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import (
"net"

"github.com/Microsoft/go-winio"

"github.com/DataDog/datadog-agent/cmd/system-probe/api/client"
)

const (
Expand Down Expand Up @@ -54,15 +52,13 @@ func newPipeListener(namedPipeName string) (net.Listener, error) {
}

// NewSystemProbeListener sets up a named pipe listener for the system probe service.
func NewSystemProbeListener(_ string) (*WindowsPipeListener, error) {
// socketAddr not used

namedPipe, err := newPipeListener(client.SystemProbePipeName)
func NewSystemProbeListener(namedPipePath string) (*WindowsPipeListener, error) {
namedPipe, err := newPipeListener(namedPipePath)
if err != nil {
return nil, fmt.Errorf("error named pipe %s : %s", client.SystemProbePipeName, err)
return nil, fmt.Errorf("error named pipe %s : %s", namedPipePath, err)
}

return &WindowsPipeListener{namedPipe, client.SystemProbePipeName}, nil
return &WindowsPipeListener{namedPipe, namedPipePath}, nil
}

// GetListener will return underlying Listener's conn
Expand Down
23 changes: 0 additions & 23 deletions pkg/process/net/windows_pipe_testutil.go

This file was deleted.

0 comments on commit 9df73e5

Please sign in to comment.