diff --git a/agent/config/builder.go b/agent/config/builder.go index 6374cf2ce..92505724b 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -1896,18 +1896,15 @@ func uint64Val(v *uint64) uint64 { } // Expect an octal permissions string, e.g. 0644 -func (b *builder) unixPermissionsVal(name string, v *string) uint32 { - +func (b *builder) unixPermissionsVal(name string, v *string) string { if v == nil { - return 0 + return "" } - if strings.HasPrefix(*v, "0") { - if mode, err := strconv.ParseUint(*v, 0, 32); err == nil { - return uint32(mode) - } + if _, err := strconv.ParseUint(*v, 8, 32); err == nil { + return *v } b.err = multierror.Append(b.err, fmt.Errorf("%s: invalid mode: %s", name, *v)) - return 0 + return "0" } func (b *builder) portVal(name string, v *int) int { diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go index 332f27f18..2354f4211 100644 --- a/agent/config/builder_test.go +++ b/agent/config/builder_test.go @@ -175,6 +175,30 @@ func TestBuilder_BuildAndValidate_NodeName(t *testing.T) { } } +func TestBuilder_unixPermissionsVal(t *testing.T) { + + b, _ := newBuilder(LoadOpts{ + FlagValues: Config{ + NodeName: pString("foo"), + DataDir: pString("dir"), + }, + }) + + goodmode := "666" + badmode := "9666" + + patchLoadOptsShims(&b.opts) + require.NoError(t, b.err) + _ = b.unixPermissionsVal("local_bind_socket_mode", &goodmode) + require.NoError(t, b.err) + require.Len(t, b.Warnings, 0) + + _ = b.unixPermissionsVal("local_bind_socket_mode", &badmode) + require.NotNil(t, b.err) + require.Contains(t, b.err.Error(), "local_bind_socket_mode: invalid mode") + require.Len(t, b.Warnings, 0) +} + func patchLoadOptsShims(opts *LoadOpts) { if opts.hostname == nil { opts.hostname = func() (string, error) { diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index a3c02dc04..bd6f2bdf1 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2689,7 +2689,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { DestinationType: "service", DestinationName: "db2", LocalBindSocketPath: "/tmp/socketpath", - LocalBindSocketMode: 0644, + LocalBindSocketMode: "0644", }, }, }, diff --git a/agent/structs/connect_proxy_config.go b/agent/structs/connect_proxy_config.go index d46e797dc..15aaa8316 100644 --- a/agent/structs/connect_proxy_config.go +++ b/agent/structs/connect_proxy_config.go @@ -324,7 +324,8 @@ type Upstream struct { // These are exclusive with LocalBindAddress/LocalBindPort LocalBindSocketPath string `json:",omitempty" alias:"local_bind_socket_path"` - LocalBindSocketMode uint32 `json:",omitempty" alias:"local_bind_socket_mode"` + // This might be represented as an int, but because it's octal outputs can be a bit strange. + LocalBindSocketMode string `json:",omitempty" alias:"local_bind_socket_mode"` // Config is an opaque config that is specific to the proxy process being run. // It can be used to pass arbitrary configuration for this specific upstream @@ -355,7 +356,7 @@ func (t *Upstream) UnmarshalJSON(data []byte) (err error) { LocalBindPortSnake int `json:"local_bind_port"` LocalBindSocketPathSnake string `json:"local_bind_socket_path"` - LocalBindSocketModeSnake uint32 `json:"local_bind_socket_mode"` + LocalBindSocketModeSnake string `json:"local_bind_socket_mode"` MeshGatewaySnake MeshGatewayConfig `json:"mesh_gateway"` @@ -384,7 +385,7 @@ func (t *Upstream) UnmarshalJSON(data []byte) (err error) { if t.LocalBindSocketPath == "" { t.LocalBindSocketPath = aux.LocalBindSocketPathSnake } - if t.LocalBindSocketMode == 0 { + if t.LocalBindSocketMode == "" { t.LocalBindSocketMode = aux.LocalBindSocketModeSnake } if t.MeshGateway.Mode == "" { diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 280565bbc..17dc11a4d 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -558,11 +558,15 @@ func makePortListenerWithDefault(name, addr string, port int, trafficDirection e return makePortListener(name, addr, port, trafficDirection) } -// TODO markan INVESTIGATE sanitizing path name (path.filepath) clean/validate. (Maybe check if sanitizer alters things, then fail) -func makePipeListener(name, path string, mode uint32, trafficDirection envoy_core_v3.TrafficDirection) *envoy_listener_v3.Listener { +func makePipeListener(name, path string, mode_str string, trafficDirection envoy_core_v3.TrafficDirection) *envoy_listener_v3.Listener { + // We've already validated this, so it should not fail. + mode, err := strconv.ParseUint(mode_str, 0, 32) + if err != nil { + mode = 0 + } return &envoy_listener_v3.Listener{ Name: fmt.Sprintf("%s:%s", name, path), - Address: makePipeAddress(path, mode), + Address: makePipeAddress(path, uint32(mode)), TrafficDirection: trafficDirection, } } diff --git a/api/agent.go b/api/agent.go index 6263972ca..3a82a4198 100644 --- a/api/agent.go +++ b/api/agent.go @@ -408,7 +408,7 @@ type Upstream struct { LocalBindAddress string `json:",omitempty"` LocalBindPort int `json:",omitempty"` LocalBindSocketPath string `json:",omitempty"` - LocalBindSocketMode uint32 `json:",omitempty"` + LocalBindSocketMode string `json:",omitempty"` Config map[string]interface{} `json:",omitempty" bexpr:"-"` MeshGateway MeshGatewayConfig `json:",omitempty"` CentrallyConfigured bool `json:",omitempty" bexpr:"-"`