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.

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

NET-10480 - fix setting namespace and partition setting in dns-proxy mode #593

Merged
merged 3 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 22 additions & 22 deletions pkg/consuldp/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@ const (
defaultAdminAccessLogsPath = os.DevNull
)

// bootstrapConfig generates the Envoy bootstrap config in JSON format.
func (cdp *ConsulDataplane) bootstrapConfig(ctx context.Context) (*bootstrap.BootstrapConfig, []byte, error) {
// getBootstrapParams makes a call using the service client to get the bootstrap params for eventually getting the Envoy bootstrap config.
func (cdp *ConsulDataplane) getBootstrapParams(ctx context.Context) (*pbdataplane.GetEnvoyBootstrapParamsResponse, error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

dns proxy requires bootstrap params to set the namespace and partition.....but it does not require the bootstrap config itself. So we are splitting this into two functions, so that startDNSProxy() can get require bootstrap params in its signature rather than just assuming it is already set on the struct.

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) {
Copy link
Member Author

Choose a reason for hiding this comment

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

A prior call will already have gotten bootstrap params for this call as well as startDNSProxy()

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
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
43 changes: 30 additions & 13 deletions pkg/consuldp/consul_dataplane.go
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,30 +180,46 @@ func (cdp *ConsulDataplane) Run(ctx context.Context) error {
cdp.dpServiceClient = pbdataplane.NewDataplaneServiceClient(state.GRPCConn)

doneCh := make(chan error)
// start up DNS server
if err = cdp.startDNSProxy(ctx); err != nil {
cdp.logger.Error("failed to start the dns proxy", "error", err)
return err
}

// if running as DNS PRoxy, xDS Server and Envoy are disabled, so
// return before configuring them.
if cdp.cfg.Mode == ModeTypeDNSProxy {
// start up DNS server with the configuration from the consul-dataplane flags / environment variables since
// envoy bootstrapping is bypassed.
if err = cdp.startDNSProxy(ctx, cdp.cfg.DNSServer, cdp.cfg.Proxy.Namespace, cdp.cfg.Proxy.Partition); err != nil {
cdp.logger.Error("failed to start the dns proxy", "error", err)
return err
}
// Wait for context to be done in a more simplified goroutine dns-proxy mode.
go func() {
<-ctx.Done()
doneCh <- nil
}()
return <-doneCh
}

// Configure xDS and Envoy configuration continues here when running in sidecar mode.
cdp.logger.Info("configuring xDS and Envoy")
err = cdp.setupXDSServer()
if err != nil {
return err
}
go cdp.startXDSServer(ctx)

bootstrapCfg, cfg, err := cdp.bootstrapConfig(ctx)
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 with envoy bootstrap params.
if err = cdp.startDNSProxy(ctx, cdp.cfg.DNSServer, bootstrapParams.Namespace, bootstrapParams.Partition); err != nil {
cdp.logger.Error("failed to start the dns proxy", "error", err)
return err
}

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 +283,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, namespace, partition string) 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: partition,
Namespace: namespace,
Token: cdp.aclToken,
})
if err == dns.ErrServerDisabled {
Expand Down
8 changes: 8 additions & 0 deletions pkg/consuldp/consul_dataplane_test.go
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
6 changes: 6 additions & 0 deletions pkg/dns/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ func (d *DNSServer) Start(ctx context.Context) error {
d.lock.Lock()
defer d.lock.Unlock()

d.logger.Debug("starting DNS proxy", "partition", d.partition, "namespace", d.namespace)

if d.running {
return ErrServerRunning
}
Expand Down Expand Up @@ -215,6 +217,8 @@ func (d *DNSServer) queryConsulAndRespondUDP(buf []byte, addr net.Addr) {
"x-consul-token", d.token,
)

logger.Debug("querying through udp", "partition", d.partition, "namespace", d.namespace)

resp, err := d.client.Query(ctx, req)
if err != nil {
logger.Error("error resolving consul request", "error", err)
Expand Down Expand Up @@ -301,6 +305,8 @@ func (d *DNSServer) proxyTCPAcceptedConn(ctx context.Context, conn net.Conn, cli
"x-consul-token", d.token,
)

logger.Debug("querying through tcp", "partition", d.partition, "namespace", d.namespace)

resp, err := client.Query(ctx, req)
if err != nil {
logger.Error("error resolving consul request", "error", err)
Expand Down
Loading