Skip to content

Commit 943f7a7

Browse files
jshahriddhijshahriddhi
committed
Only allow local xds bind address
1 parent e2c2ec9 commit 943f7a7

File tree

11 files changed

+50
-322
lines changed

11 files changed

+50
-322
lines changed

‎pkg/consuldp/bootstrap.go‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,12 @@ func (cdp *ConsulDataplane) bootstrapConfig(ctx context.Context) ([]byte, error)
6868
Datacenter: rsp.Datacenter,
6969
}
7070

71-
if cdp.localXDSServer != nil && cdp.localXDSServer.enabled {
72-
if cdp.localXDSServer.listenerNetwork == "unix" {
73-
args.GRPC.AgentSocket = cdp.localXDSServer.listenerAddress
74-
} else {
75-
xdsServerFullAddr := strings.Split(cdp.localXDSServer.listenerAddress, ":")
76-
args.GRPC.AgentAddress = xdsServerFullAddr[0]
77-
args.GRPC.AgentPort = xdsServerFullAddr[1]
78-
}
71+
if cdp.xdsServer.listenerNetwork == "unix" {
72+
args.GRPC.AgentSocket = cdp.xdsServer.listenerAddress
73+
} else {
74+
xdsServerFullAddr := strings.Split(cdp.xdsServer.listenerAddress, ":")
75+
args.GRPC.AgentAddress = xdsServerFullAddr[0]
76+
args.GRPC.AgentPort = xdsServerFullAddr[1]
7977
}
8078

8179
var bootstrapConfig bootstrap.BootstrapConfig

‎pkg/consuldp/bootstrap_test.go‎

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func TestBootstrapConfig(t *testing.T) {
5656
Telemetry: &TelemetryConfig{
5757
UseCentralConfig: false,
5858
},
59-
XDSServer: &XDSServer{BindAddress: "1.2.3.4", BindPort: xdsBindPort},
59+
XDSServer: &XDSServer{BindAddress: "127.0.0.1", BindPort: xdsBindPort},
6060
},
6161
&pbdataplane.GetEnvoyBootstrapParamsResponse{
6262
Service: "web",
@@ -82,7 +82,7 @@ func TestBootstrapConfig(t *testing.T) {
8282
Telemetry: &TelemetryConfig{
8383
UseCentralConfig: true,
8484
},
85-
XDSServer: &XDSServer{BindAddress: "1.2.3.4", BindPort: xdsBindPort},
85+
XDSServer: &XDSServer{BindAddress: "127.0.0.1", BindPort: xdsBindPort},
8686
},
8787
&pbdataplane.GetEnvoyBootstrapParamsResponse{
8888
Service: "web",
@@ -110,40 +110,14 @@ func TestBootstrapConfig(t *testing.T) {
110110
Telemetry: &TelemetryConfig{
111111
UseCentralConfig: false,
112112
},
113-
XDSServer: &XDSServer{BindAddress: "1.2.3.4", BindPort: xdsBindPort},
114-
},
115-
&pbdataplane.GetEnvoyBootstrapParamsResponse{
116-
Service: "web",
117-
NodeName: nodeName,
118-
},
119-
},
120-
"local-xds-server": {
121-
&Config{
122-
Consul: &ConsulConfig{
123-
GRPCPort: 1234,
124-
},
125-
Service: &ServiceConfig{
126-
ServiceID: "web-proxy",
127-
NodeName: nodeName,
128-
},
129-
Envoy: &EnvoyConfig{
130-
AdminBindAddress: "127.0.0.1",
131-
AdminBindPort: 19000,
132-
},
133-
Telemetry: &TelemetryConfig{
134-
UseCentralConfig: false,
135-
},
136113
XDSServer: &XDSServer{BindAddress: "127.0.0.1", BindPort: xdsBindPort},
137114
},
138115
&pbdataplane.GetEnvoyBootstrapParamsResponse{
139116
Service: "web",
140117
NodeName: nodeName,
141-
Config: makeStruct(map[string]any{
142-
"envoy_dogstatsd_url": "this-should-not-appear-in-generated-config",
143-
}),
144118
},
145119
},
146-
"local-unix-socket-xds-server": {
120+
"unix-socket-xds-server": {
147121
&Config{
148122
Consul: &ConsulConfig{
149123
GRPCPort: 1234,
@@ -188,12 +162,10 @@ func TestBootstrapConfig(t *testing.T) {
188162
dpServiceClient: client,
189163
}
190164

191-
if checkLocalXDSServer(tc.cfg.XDSServer.BindAddress) {
192-
if strings.HasPrefix(tc.cfg.XDSServer.BindAddress, "unix://") {
193-
dp.localXDSServer = &localXDSServer{enabled: true, listenerAddress: socketPath, listenerNetwork: "unix"}
194-
} else {
195-
dp.localXDSServer = &localXDSServer{enabled: true, listenerAddress: fmt.Sprintf("127.0.0.1:%d", xdsBindPort)}
196-
}
165+
if strings.HasPrefix(tc.cfg.XDSServer.BindAddress, "unix://") {
166+
dp.xdsServer = &xdsServer{listenerAddress: socketPath, listenerNetwork: "unix"}
167+
} else {
168+
dp.xdsServer = &xdsServer{listenerAddress: fmt.Sprintf("127.0.0.1:%d", xdsBindPort)}
197169
}
198170

199171
bsCfg, err := dp.bootstrapConfig(ctx)

‎pkg/consuldp/consul_dataplane.go‎

Lines changed: 11 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ type consulServer struct {
3030
grpcClientConn *grpc.ClientConn
3131
}
3232

33-
type localXDSServer struct {
34-
enabled bool
33+
type xdsServer struct {
3534
listener net.Listener
3635
listenerAddress string
3736
listenerNetwork string
@@ -45,7 +44,7 @@ type ConsulDataplane struct {
4544
cfg *Config
4645
consulServer *consulServer
4746
dpServiceClient pbdataplane.DataplaneServiceClient
48-
localXDSServer *localXDSServer
47+
xdsServer *xdsServer
4948
}
5049

5150
// NewConsulDP creates a new instance of ConsulDataplane
@@ -65,9 +64,8 @@ func NewConsulDP(cfg *Config) (*ConsulDataplane, error) {
6564
})
6665

6766
return &ConsulDataplane{
68-
logger: logger,
69-
cfg: cfg,
70-
localXDSServer: &localXDSServer{enabled: false},
67+
logger: logger,
68+
cfg: cfg,
7169
}, nil
7270
}
7371

@@ -93,8 +91,8 @@ func validateConfig(cfg *Config) error {
9391
return errors.New("logging settings not specified")
9492
case cfg.XDSServer.BindAddress == "":
9593
return errors.New("envoy xDS bind address not specified")
96-
case cfg.XDSServer.BindPort == 0 && !checkLocalXDSServer(cfg.XDSServer.BindAddress):
97-
return errors.New("envoy xDS bind port not specified")
94+
case !strings.HasPrefix(cfg.XDSServer.BindAddress, "unix://") && cfg.XDSServer.BindAddress != "127.0.0.1" && cfg.XDSServer.BindAddress != "localhost":
95+
return errors.New("non-local xDS bind address not allowed")
9896
}
9997
return nil
10098
}
@@ -133,28 +131,6 @@ func (cdp *ConsulDataplane) setConsulServerSupportedFeatures(ctx context.Context
133131
return nil
134132
}
135133

136-
// checkLocalXDSServer checks if the specified xds bind address is local.
137-
func checkLocalXDSServer(xdsBindAddr string) bool {
138-
if strings.HasPrefix(xdsBindAddr, "unix://") || xdsBindAddr == "127.0.0.1" || xdsBindAddr == "localhost" {
139-
return true
140-
}
141-
return false
142-
}
143-
144-
// checkAndEnableLocalXDSServer checks if the specified xds bind address is local.
145-
// If local, it enables the flag to configure consul-dataplane (later on in the setup process)
146-
// with a gRPC server to serve envoy xDS requests.
147-
// If not local, the flag remains turned off and it is assumed the xDS requests will be served
148-
// by a remote gRPC server.
149-
// Potential TODO: Explicitly allow specifying an option (xds.disable?) to disable configuring
150-
// consul-dataplane as the xDS server in case xDS requests can be served on another port locally
151-
// (example: consul server process on localhost).
152-
func (cdp *ConsulDataplane) checkAndEnableLocalXDSServer() {
153-
if checkLocalXDSServer(cdp.cfg.XDSServer.BindAddress) {
154-
cdp.localXDSServer.enabled = true
155-
}
156-
}
157-
158134
func (cdp *ConsulDataplane) Run(ctx context.Context) error {
159135
cdp.logger.Info("started consul-dataplane process")
160136

@@ -187,16 +163,12 @@ func (cdp *ConsulDataplane) Run(ctx context.Context) error {
187163
return fmt.Errorf("failed to set supported features: %w", err)
188164
}
189165

190-
cdp.checkAndEnableLocalXDSServer()
191-
192-
if cdp.localXDSServer.enabled {
193-
err = cdp.setupXDSServer()
194-
if err != nil {
195-
return err
196-
}
197-
go cdp.startXDSServer()
198-
defer cdp.stopXDSServer()
166+
err = cdp.setupXDSServer()
167+
if err != nil {
168+
return err
199169
}
170+
go cdp.startXDSServer()
171+
defer cdp.stopXDSServer()
200172

201173
cfg, err := cdp.bootstrapConfig(ctx)
202174
if err != nil {

‎pkg/consuldp/consul_dataplane_test.go‎

Lines changed: 2 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,11 @@ func TestNewConsulDPError(t *testing.T) {
126126
expectErr: "envoy xDS bind address not specified",
127127
},
128128
{
129-
name: "missing xds bind port when address not local",
129+
name: "non-local xds bind address",
130130
modFn: func(c *Config) {
131-
c.XDSServer.BindPort = 0
132131
c.XDSServer.BindAddress = "1.2.3.4"
133132
},
134-
expectErr: "envoy xDS bind port not specified",
133+
expectErr: "non-local xDS bind address not allowed",
135134
},
136135
}
137136
for _, tc := range testCases {
@@ -214,47 +213,3 @@ func TestSetConsulServerSupportedFeaturesError(t *testing.T) {
214213
require.ErrorContains(t, consulDP.setConsulServerSupportedFeatures(context.Background()), "failure getting supported consul-dataplane features")
215214
require.Empty(t, consulDP.consulServer.supportedFeatures)
216215
}
217-
218-
func TestCheckAndEnableLocalXDSServer(t *testing.T) {
219-
type testCase struct {
220-
name string
221-
modFn func(*Config)
222-
expectLocalXDSServer bool
223-
}
224-
225-
testCases := []testCase{
226-
{
227-
name: "localhost ip",
228-
modFn: func(c *Config) { c.XDSServer.BindAddress = "127.0.0.1" },
229-
expectLocalXDSServer: true,
230-
},
231-
{
232-
name: "localhost name",
233-
modFn: func(c *Config) { c.XDSServer.BindAddress = "localhost" },
234-
expectLocalXDSServer: true,
235-
},
236-
{
237-
name: "unix socket",
238-
modFn: func(c *Config) { c.XDSServer.BindAddress = "unix:///var/run/xds.sock" },
239-
expectLocalXDSServer: true,
240-
},
241-
{
242-
name: "remote ip",
243-
modFn: func(c *Config) {
244-
c.XDSServer.BindAddress = "1.2.3.4"
245-
c.XDSServer.BindPort = 1234
246-
},
247-
expectLocalXDSServer: false,
248-
},
249-
}
250-
for _, tc := range testCases {
251-
t.Run(tc.name, func(t *testing.T) {
252-
cfg := validConfig()
253-
tc.modFn(cfg)
254-
cdp, err := NewConsulDP(cfg)
255-
require.NoError(t, err)
256-
cdp.checkAndEnableLocalXDSServer()
257-
require.Equal(t, tc.expectLocalXDSServer, cdp.localXDSServer.enabled)
258-
})
259-
}
260-
}

‎pkg/consuldp/testdata/TestBootstrapConfig/basic.golden‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
"endpoint": {
4545
"address": {
4646
"socket_address": {
47-
"address": "1.2.3.4",
47+
"address": "127.0.0.1",
4848
"port_value": 1234
4949
}
5050
}

‎pkg/consuldp/testdata/TestBootstrapConfig/central-telemetry-config.golden‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
"endpoint": {
4545
"address": {
4646
"socket_address": {
47-
"address": "1.2.3.4",
47+
"address": "127.0.0.1",
4848
"port_value": 1234
4949
}
5050
}

0 commit comments

Comments
 (0)