Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[agent] Serve API over Unix domain sockets #16884

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[agent] Serve API over Unix domain sockets #16884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6df76fd
Add api_socket config
angrycub Apr 13, 2023
449fa27
Update tests for new config
angrycub Apr 13, 2023
412d606
Enable socket listener
angrycub Apr 13, 2023
482299b
Add flags to agent for socket config
angrycub Apr 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions command/agent/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,10 @@ func (c *Command) readConfig() *Config {

// Make a new, empty config.
cmdConfig := &Config{
Client: &ClientConfig{},
Consul: &config.ConsulConfig{},
Ports: &Ports{},
APISocket: &SocketConfig{},
Client: &ClientConfig{},
Consul: &config.ConsulConfig{},
Ports: &Ports{},
Server: &ServerConfig{
ServerJoin: &ServerJoin{},
},
Expand Down Expand Up @@ -122,6 +123,12 @@ func (c *Command) readConfig() *Config {
flags.BoolVar(&cmdConfig.LogJson, "log-json", false, "")
flags.StringVar(&cmdConfig.NodeName, "node", "", "")

// API Socket Config options
flags.StringVar(&cmdConfig.APISocket.Path, "api-socket-path", "", "")
flags.StringVar(&cmdConfig.APISocket.User, "api-socket-user", "", "")
flags.StringVar(&cmdConfig.APISocket.Group, "api-socket-group", "", "")
flags.StringVar(&cmdConfig.APISocket.Mode, "api-socket-mode", "", "")
Comment on lines +127 to +130
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think api-unix-* makes more sense here (see below for discussion).


// Consul options
flags.StringVar(&cmdConfig.Consul.Auth, "consul-auth", "", "")
flags.Var((flaghelper.FuncBoolVar)(func(b bool) error {
Expand Down Expand Up @@ -349,10 +356,11 @@ func (c *Command) IsValidConfig(config, cmdConfig *Config) bool {

// Verify the paths are absolute.
dirs := map[string]string{
"data-dir": config.DataDir,
"plugin-dir": config.PluginDir,
"alloc-dir": config.Client.AllocDir,
"state-dir": config.Client.StateDir,
"data-dir": config.DataDir,
"plugin-dir": config.PluginDir,
"alloc-dir": config.Client.AllocDir,
"state-dir": config.Client.StateDir,
"api-socket-path": config.APISocket.Path,
}
for k, dir := range dirs {
if dir == "" {
Expand Down
64 changes: 64 additions & 0 deletions command/agent/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ type Config struct {
// Use normalizedAddrs if you need the host+port to bind to.
Addresses *Addresses `hcl:"addresses"`

// APISocket is used to configure a unix domain socket listener for the
// HTTP API
APISocket *SocketConfig `hcl:"api_socket"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pedant in me wants to name this api_unix_socket because it's the "unix" bit that makes this interesting. All addresses{} are all sockets. Using unix in the name would match Consul and Vault better as well.

That being said if you don't think anyone wants to type that many characters I will shutup. It is nbd. 😅


// normalizedAddr is set to the Address+Port by normalizeAddrs()
normalizedAddrs *NormalizedAddrs

Expand Down Expand Up @@ -1089,6 +1093,57 @@ func (n *NormalizedAddrs) Copy() *NormalizedAddrs {
return &nn
}

// SocketConfig contains the path and configuration for a unix domain socket
// listener.
type SocketConfig struct {
Path string `hcl:"path"`
User string `hcl:"user"`
Mode string `hcl:"mode"`
Group string `hcl:"group"`
Comment on lines +1100 to +1102
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Letting the administrator set user/group/mode is really interesting, in that I'd have expected us to use Nomad's own user/group and a restrictive mode. But this lets the administrator run a client as root and use a non-root user to talk to it (so long as you've got ACLs in place, I suppose).

This should definitely come with some notes about recommended usage in the docs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some prior art:

  • Vault has the bare minimum docs here.
  • Consul is probably a good one to copy (except for the lack of Windows support... I wonder if it just-started-working for them when Go+Windows added support).
  • Boundary is clearly an iteration on Vault's approach, and I don't think has anything relevant for us. It's interesting they appear to enable tls by default while Vault totally lacks tls for their unix socket. I think we can follow Vault here until somebody gives us a reason to do otherwise.

}

func (s *SocketConfig) Copy() *SocketConfig {
if s == nil {
return nil
}
ns := *s
return &ns
}

// Merge merges two SocketConfigs together, preferring values from the argument.
func (s *SocketConfig) Merge(b *SocketConfig) *SocketConfig {
if b == nil {
return s
}
var result SocketConfig
if s != nil {
result = *s
}
Comment on lines +1119 to +1121
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just declared Copy() above, let's use that instead of implementing it twice

if b.Path != "" {
result.Path = b.Path
}
if b.User != "" {
result.User = b.User
}
if b.Group != "" {
result.Group = b.Group
}
if b.Mode != "" {
result.Mode = b.Mode
}
Comment on lines +1122 to +1133
Copy link
Member

@shoenig shoenig Apr 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field merges can be 1-liners with https://github.com/hashicorp/nomad/blob/main/helper/funcs.go#L492-L500

return &result
}

// UnixSocketsConfig extracts user, group, and mode into a UnixSocketsConfig
// suitable for listenerutil.UnixSocketListener
func (s *SocketConfig) UnixSocketsConfig() *listenerutil.UnixSocketsConfig {
return &listenerutil.UnixSocketsConfig{
User: s.User,
Group: s.Group,
Mode: s.Mode,
}
}

// AdvertiseAddrs is used to control the addresses we advertise out for
// different network services. All are optional and default to BindAddr and
// their default Port.
Expand Down Expand Up @@ -1264,6 +1319,7 @@ func DefaultConfig() *Config {
},
Addresses: &Addresses{},
AdvertiseAddrs: &AdvertiseAddrs{},
APISocket: &SocketConfig{},
Consul: config.DefaultConsulConfig(),
Vault: config.DefaultVaultConfig(),
UI: config.DefaultUIConfig(),
Expand Down Expand Up @@ -1501,6 +1557,13 @@ func (c *Config) Merge(b *Config) *Config {
result.AdvertiseAddrs = result.AdvertiseAddrs.Merge(b.AdvertiseAddrs)
}

// Apply the api_socket config
if result.APISocket == nil && b.APISocket != nil {
result.APISocket = b.APISocket.Copy()
} else if b.APISocket != nil {
result.APISocket = result.APISocket.Merge(b.APISocket)
}

// Apply the Consul Configuration
if result.Consul == nil && b.Consul != nil {
result.Consul = b.Consul.Copy()
Expand Down Expand Up @@ -1576,6 +1639,7 @@ func (c *Config) Copy() *Config {
nc.Addresses = c.Addresses.Copy()
nc.normalizedAddrs = c.normalizedAddrs.Copy()
nc.AdvertiseAddrs = c.AdvertiseAddrs.Copy()
nc.APISocket = c.APISocket.Copy()
nc.Client = c.Client.Copy()
nc.Server = c.Server.Copy()
nc.ACL = c.ACL.Copy()
Expand Down
1 change: 1 addition & 0 deletions command/agent/config_parse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func ParseConfigFile(path string) (*Config, error) {

// parse
c := &Config{
APISocket: &SocketConfig{},
Client: &ClientConfig{
ServerJoin: &ServerJoin{},
TemplateConfig: &client.ClientTemplateConfig{
Expand Down
43 changes: 27 additions & 16 deletions command/agent/config_parse_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ var basicConfig = &Config{
RPC: "127.0.0.3",
Serf: "127.0.0.4",
},
APISocket: &SocketConfig{
Path: "/var/run/nomad.sock",
User: "nomad",
Group: "nomad",
Mode: "0600",
},
Client: &ClientConfig{
Enabled: true,
StateDir: "/tmp/client-state",
Expand Down Expand Up @@ -219,33 +225,33 @@ var basicConfig = &Config{
ClientServiceName: "nomad-client",
ClientHTTPCheckName: "nomad-client-http-health-check",
Addr: "127.0.0.1:9500",
AllowUnauthenticated: &trueValue,
AllowUnauthenticated: pointer.Of(true),
Token: "token1",
Auth: "username:pass",
EnableSSL: &trueValue,
VerifySSL: &trueValue,
EnableSSL: pointer.Of(true),
VerifySSL: pointer.Of(true),
CAFile: "/path/to/ca/file",
CertFile: "/path/to/cert/file",
KeyFile: "/path/to/key/file",
ServerAutoJoin: &trueValue,
ClientAutoJoin: &trueValue,
AutoAdvertise: &trueValue,
ChecksUseAdvertise: &trueValue,
ServerAutoJoin: pointer.Of(true),
ClientAutoJoin: pointer.Of(true),
AutoAdvertise: pointer.Of(true),
ChecksUseAdvertise: pointer.Of(true),
Timeout: 5 * time.Second,
TimeoutHCL: "5s",
},
Vault: &config.VaultConfig{
Addr: "127.0.0.1:9500",
AllowUnauthenticated: &trueValue,
AllowUnauthenticated: pointer.Of(true),
ConnectionRetryIntv: config.DefaultVaultConnectRetryIntv,
Enabled: &falseValue,
Enabled: pointer.Of(false),
Role: "test_role",
TLSCaFile: "/path/to/ca/file",
TLSCaPath: "/path/to/ca",
TLSCertFile: "/path/to/cert/file",
TLSKeyFile: "/path/to/key/file",
TLSServerName: "foobar",
TLSSkipVerify: &trueValue,
TLSSkipVerify: pointer.Of(true),
TaskTokenTTL: "1s",
Token: "12345",
},
Expand Down Expand Up @@ -280,16 +286,16 @@ var basicConfig = &Config{
},
},
Autopilot: &config.AutopilotConfig{
CleanupDeadServers: &trueValue,
CleanupDeadServers: pointer.Of(true),
ServerStabilizationTime: 23057 * time.Second,
ServerStabilizationTimeHCL: "23057s",
LastContactThreshold: 12705 * time.Second,
LastContactThresholdHCL: "12705s",
MaxTrailingLogs: 17849,
MinQuorum: 3,
EnableRedundancyZones: &trueValue,
DisableUpgradeMigration: &trueValue,
EnableCustomUpgrades: &trueValue,
EnableRedundancyZones: pointer.Of(true),
DisableUpgradeMigration: pointer.Of(true),
EnableCustomUpgrades: pointer.Of(true),
},
Plugins: []*config.PluginConfig{
{
Expand Down Expand Up @@ -532,6 +538,9 @@ func removeHelperAttributes(c *Config) *Config {
}

func (c *Config) addDefaults() {
if c.APISocket == nil {
c.APISocket = &SocketConfig{}
}
if c.Client == nil {
c.Client = &ClientConfig{}
}
Expand Down Expand Up @@ -635,7 +644,8 @@ var sample0 = &Config{
RPC: "host.example.com",
Serf: "host.example.com",
},
Client: &ClientConfig{ServerJoin: &ServerJoin{}},
APISocket: &SocketConfig{},
Client: &ClientConfig{ServerJoin: &ServerJoin{}},
Server: &ServerConfig{
Enabled: true,
BootstrapExpect: 3,
Expand Down Expand Up @@ -730,7 +740,8 @@ var sample1 = &Config{
RPC: "host.example.com",
Serf: "host.example.com",
},
Client: &ClientConfig{ServerJoin: &ServerJoin{}},
APISocket: &SocketConfig{},
Client: &ClientConfig{ServerJoin: &ServerJoin{}},
Server: &ServerConfig{
Enabled: true,
BootstrapExpect: 3,
Expand Down
72 changes: 72 additions & 0 deletions command/agent/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/hashicorp/nomad/helper/pointer"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/nomad/structs/config"
"github.com/shoenig/test/must"
"github.com/stretchr/testify/require"
)

Expand All @@ -44,6 +45,7 @@ func TestConfig_Merge(t *testing.T) {
Ports: &Ports{},
Addresses: &Addresses{},
AdvertiseAddrs: &AdvertiseAddrs{},
APISocket: &SocketConfig{},
Vault: &config.VaultConfig{},
Consul: &config.ConsulConfig{},
Sentinel: &config.SentinelConfig{},
Expand Down Expand Up @@ -386,6 +388,12 @@ func TestConfig_Merge(t *testing.T) {
RPC: "127.0.0.2",
Serf: "127.0.0.2",
},
APISocket: &SocketConfig{
Path: "/tmp/run/nomad.sock",
User: "nomad",
Group: "nomad",
Mode: "0600",
},
HTTPAPIResponseHeaders: map[string]string{
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Methods": "GET, POST, OPTIONS",
Expand Down Expand Up @@ -1578,3 +1586,67 @@ func TestParseMultipleIPTemplates(t *testing.T) {
})
}
}

func TestConfig_SocketConfig_Copy(t *testing.T) {
ci.Parallel(t)

a := &SocketConfig{
Path: "/var/run/nomad.sock",
User: "nomad",
Group: "wheel",
Mode: "0660",
}
b := a.Copy()
must.Eq(t, a, b)
}

func TestConfig_SocketConfig_Merge(t *testing.T) {
ci.Parallel(t)

sc1 := &SocketConfig{
Path: "/tmp/sc1.sock",
User: "nomad",
Group: "wheel",
Mode: "660",
}
tcs := []struct {
name string
a *SocketConfig
b *SocketConfig
expect *SocketConfig
}{
{
name: "nils",
expect: nil,
},
{
name: "nil_b",
a: sc1,
expect: sc1,
},
{
name: "nil_a",
b: sc1,
expect: sc1,
},
{
name: "overwrite_a.Path",
a: sc1,
b: &SocketConfig{Path: "/tc/sc2"},
expect: &SocketConfig{
Path: "/tc/sc2",
User: "nomad",
Group: "wheel",
Mode: "660",
},
},
}
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
tc := tc
ci.Parallel(t)
c := tc.a.Merge(tc.b)
must.Eq(t, tc.expect, c)
})
}
}
Loading