Skip to content

Commit

Permalink
fix logic of running dns proxy because it relied on some ambient cont…
Browse files Browse the repository at this point in the history 
…ext to be set.
  • Loading branch information
@jmurret
jmurret committed Jul 22, 2024
1 parent aba18ac commit 1234b0c
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 32 deletions.
42 changes: 21 additions & 21 deletions pkg/consuldp/bootstrap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ const (
)

// bootstrapConfig generates the Envoy bootstrap config in JSON format.
func (cdp *ConsulDataplane) bootstrapConfig(ctx context.Context) (*bootstrap.BootstrapConfig, []byte, error) {
func (cdp *ConsulDataplane) getBootstrapParams(ctx context.Context) (*pbdataplane.GetEnvoyBootstrapParamsResponse, error) {
svc := cdp.cfg.Proxy
envoy := cdp.cfg.Envoy

req := &pbdataplane.GetEnvoyBootstrapParamsRequest{
ServiceId: svc.ProxyID,
Expand All @@ -50,16 +49,17 @@ func (cdp *ConsulDataplane) bootstrapConfig(ctx context.Context) (*bootstrap.Boo

rsp, err := cdp.dpServiceClient.GetEnvoyBootstrapParams(ctx, req)
if err != nil {
return nil, nil, fmt.Errorf("failed to get envoy bootstrap params: %w", err)
return nil, fmt.Errorf("failed to get envoy bootstrap params: %w", err)
}

// store the final resolved service for others to use.
cdp.resolvedProxyConfig = ProxyConfig{
NodeName: rsp.NodeName,
ProxyID: cdp.cfg.Proxy.ProxyID,
Namespace: rsp.Namespace,
Partition: rsp.Partition,
}
return rsp, nil
}

// bootstrapConfig generates the Envoy bootstrap config in JSON format.
func (cdp *ConsulDataplane) bootstrapConfig(
bootstrapParams *pbdataplane.GetEnvoyBootstrapParamsResponse) (*bootstrap.BootstrapConfig, []byte, error) {
svc := cdp.cfg.Proxy
envoy := cdp.cfg.Envoy

prom := cdp.cfg.Telemetry.Prometheus
args := &bootstrap.BootstrapTplArgs{
Expand All @@ -68,26 +68,26 @@ func (cdp *ConsulDataplane) bootstrapConfig(ctx context.Context) (*bootstrap.Boo
AgentPort: strconv.Itoa(cdp.cfg.XDSServer.BindPort),
AgentTLS: false,
},
ProxyCluster: rsp.Service,
ProxyCluster: bootstrapParams.Service,
ProxyID: svc.ProxyID,
NodeName: rsp.NodeName,
ProxySourceService: rsp.Service,
AdminAccessLogConfig: rsp.AccessLogs,
NodeName: bootstrapParams.NodeName,
ProxySourceService: bootstrapParams.Service,
AdminAccessLogConfig: bootstrapParams.AccessLogs,
AdminAccessLogPath: defaultAdminAccessLogsPath,
AdminBindAddress: envoy.AdminBindAddress,
AdminBindPort: strconv.Itoa(envoy.AdminBindPort),
LocalAgentClusterName: localClusterName,
Namespace: rsp.Namespace,
Partition: rsp.Partition,
Datacenter: rsp.Datacenter,
Namespace: bootstrapParams.Namespace,
Partition: bootstrapParams.Partition,
Datacenter: bootstrapParams.Datacenter,
PrometheusCertFile: prom.CertFile,
PrometheusKeyFile: prom.KeyFile,
PrometheusScrapePath: prom.ScrapePath,
}

if rsp.Identity != "" {
args.ProxyCluster = rsp.Identity
args.ProxySourceService = rsp.Identity
if bootstrapParams.Identity != "" {
args.ProxyCluster = bootstrapParams.Identity
args.ProxySourceService = bootstrapParams.Identity
}

if cdp.xdsServer.listenerNetwork == "unix" {
Expand Down Expand Up @@ -116,7 +116,7 @@ func (cdp *ConsulDataplane) bootstrapConfig(ctx context.Context) (*bootstrap.Boo
}

if cdp.cfg.Telemetry.UseCentralConfig {
if err := mapstructure.WeakDecode(rsp.Config.AsMap(), &bootstrapConfig); err != nil {
if err := mapstructure.WeakDecode(bootstrapParams.Config.AsMap(), &bootstrapConfig); err != nil {
return nil, nil, fmt.Errorf("failed parsing Proxy.Config: %w", err)
}

Expand Down
13 changes: 11 additions & 2 deletions pkg/consuldp/bootstrap_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,23 @@ func TestBootstrapConfig(t *testing.T) {
dp.xdsServer = &xdsServer{listenerAddress: fmt.Sprintf("127.0.0.1:%d", xdsBindPort)}
}

_, bsCfg, err := dp.bootstrapConfig(ctx)
params, err := dp.getBootstrapParams(ctx)
require.NoError(t, err)

_, bsCfg, err := dp.bootstrapConfig(params)
require.NoError(t, err)

golden(t, bsCfg)
validateBootstrapConfig(t, bsCfg)

if tc.resolvedProxyConfig != nil {
require.Equal(t, *tc.resolvedProxyConfig, dp.resolvedProxyConfig)
proxyCfg := ProxyConfig{
NodeName: params.NodeName,
ProxyID: dp.cfg.Proxy.ProxyID,
Namespace: params.Namespace,
Partition: params.Partition,
}
require.Equal(t, *tc.resolvedProxyConfig, proxyCfg)
}
})
}
Expand Down
26 changes: 17 additions & 9 deletions pkg/consuldp/consul_dataplane.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ type ConsulDataplane struct {
aclToken string
metricsConfig *metricsConfig
lifecycleConfig *lifecycleConfig

resolvedProxyConfig ProxyConfig
}

// NewConsulDP creates a new instance of ConsulDataplane
Expand Down Expand Up @@ -97,6 +95,8 @@ func validateConfig(cfg *Config) error {
return errors.New("non-local xDS bind address not allowed")
case cfg.Mode == ModeTypeSidecar && cfg.DNSServer.Port != -1 && !net.ParseIP(cfg.DNSServer.BindAddr).IsLoopback():
return errors.New("non-local DNS proxy bind address not allowed when running as a sidecar")
case cfg.Mode == ModeTypeDNSProxy && cfg.Proxy != nil && !(cfg.Proxy.Namespace == "" || cfg.Proxy.Namespace == "default"):
return errors.New("namespace must be empty or set to 'default' when running in dns-proxy mode")
}

creds := cfg.Consul.Credentials
Expand Down Expand Up @@ -180,8 +180,15 @@ func (cdp *ConsulDataplane) Run(ctx context.Context) error {
cdp.dpServiceClient = pbdataplane.NewDataplaneServiceClient(state.GRPCConn)

doneCh := make(chan error)
bootstrapParams, err := cdp.getBootstrapParams(ctx)
if err != nil {
cdp.logger.Error("failed to get bootstrap params", "error", err)
return fmt.Errorf("failed to get bootstrap config: %w", err)
}
cdp.logger.Debug("generated envoy bootstrap params", "params", bootstrapParams)

// start up DNS server
if err = cdp.startDNSProxy(ctx); err != nil {
if err = cdp.startDNSProxy(ctx, cdp.cfg.DNSServer, bootstrapParams); err != nil {
cdp.logger.Error("failed to start the dns proxy", "error", err)
return err
}
Expand All @@ -203,7 +210,7 @@ func (cdp *ConsulDataplane) Run(ctx context.Context) error {
}
go cdp.startXDSServer(ctx)

bootstrapCfg, cfg, err := cdp.bootstrapConfig(ctx)
bootstrapCfg, cfg, err := cdp.bootstrapConfig(bootstrapParams)
if err != nil {
cdp.logger.Error("failed to get bootstrap config", "error", err)
return fmt.Errorf("failed to get bootstrap config: %w", err)
Expand Down Expand Up @@ -267,16 +274,17 @@ func (cdp *ConsulDataplane) Run(ctx context.Context) error {
return <-doneCh
}

func (cdp *ConsulDataplane) startDNSProxy(ctx context.Context) error {
func (cdp *ConsulDataplane) startDNSProxy(ctx context.Context,
dnsConfig *DNSServerConfig, bootstrapParams *pbdataplane.GetEnvoyBootstrapParamsResponse) error {
dnsClientInterface := pbdns.NewDNSServiceClient(cdp.serverConn)

dnsServer, err := dns.NewDNSServer(dns.DNSServerParams{
BindAddr: cdp.cfg.DNSServer.BindAddr,
Port: cdp.cfg.DNSServer.Port,
BindAddr: dnsConfig.BindAddr,
Port: dnsConfig.Port,
Client: dnsClientInterface,
Logger: cdp.logger,
Partition: cdp.resolvedProxyConfig.Partition,
Namespace: cdp.resolvedProxyConfig.Namespace,
Partition: bootstrapParams.Partition,
Namespace: bootstrapParams.Namespace,
Token: cdp.aclToken,
})
if err == dns.ErrServerDisabled {
Expand Down
8 changes: 8 additions & 0 deletions pkg/consuldp/consul_dataplane_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,14 @@ func TestNewConsulDPError(t *testing.T) {
},
expectErr: "bearer token (or path to a file containing a bearer token) is required for login",
},
{
name: "dns-proxy mode - namespace set to non empty or default value",
mode: ModeTypeDNSProxy,
modFn: func(c *Config) {
c.Proxy.Namespace = "test"
},
expectErr: "namespace must be empty or set to 'default' when running in dns-proxy mode",
},
}

testCases = append(testCases, dnsProxyTestCases...)
Expand Down

0 comments on commit 1234b0c

Please sign in to comment.