Repository: grpc/grpc-go
File: internal/xds/xdsclient/pool.go (Go)

Code before:
	p.mu.Lock()
	defer p.mu.Unlock()
	config, err := p.getConfiguration()
	if err != nil {
		return nil, fmt.Errorf("xds: failed to read xDS bootstrap config from env vars:  %v", err)
	}
	if config == nil {
		return nil, fmt.Errorf("failed to read xDS bootstrap config from env vars: bootstrap environment variables (%q or %q) not defined", envconfig.XDSBootstrapFileNameEnv, envconfig.XDSBootstrapFileContentEnv)
	}
	return config, nil
}

// NewClientWithConfig returns an xDS client with the given name from the pool.
// If the client doesn't already exist, it creates a new xDS client using the
// provided config and adds it to the pool. The provided config takes precedence
// over any config passed during pool creation. This should be used when a
// non-default config is required. If the provided config is nil, it attempts
// to load the configuration from the environment variables.
//
// The second return value represents a close function which the caller is
// expected to invoke once they are done using the client.  It is safe for the
// caller to invoke this close function multiple times.
func (p *Pool) NewClientWithConfig(name string, metricsRecorder estats.MetricsRecorder, config *bootstrap.Config) (XDSClient, func(), error) {
	if config == nil {
		var err error
		config, err = p.getBootstrapConfiguration()
		if err != nil {
			return nil, nil, err
		}
	}
	return p.newRefCounted(name, metricsRecorder, defaultWatchExpiryTimeout, config)
}

// NewClient returns an xDS client with the given name from the pool. If the
// client doesn't already exist, it loads the configuration from the
// environment variables, creates a new xDS client and adds it to the pool.
//
// The second return value represents a close function which the caller is
// expected to invoke once they are done using the client.  It is safe for the
// caller to invoke this close function multiple times.
func (p *Pool) NewClient(name string, metricsRecorder estats.MetricsRecorder) (XDSClient, func(), error) {
	config, err := p.getBootstrapConfiguration()
	if err != nil {
		return nil, nil, err
	}
	return p.newRefCounted(name, metricsRecorder, defaultWatchExpiryTimeout, config)
}

// NewClientForTesting returns an xDS client configured with the provided
// options from the pool. If the client doesn't already exist, it creates a new
// xDS client and adds it to the pool.

Review comment (suggestion):
This and the similar code down below don't handle the case where `p.getBootstrapConfiguration()` could return `nil, nil`, which is exactly why I think it makes sense to change `bootstrap.GetConfiguration` to return an error in that case instead.

Code after:
	p.mu.Lock()
	defer p.mu.Unlock()
	config, err := p.getConfiguration()
	if err != nil {
		return nil, fmt.Errorf("xds: failed to read xDS bootstrap config:  %v", err)
	}
	return config, nil
}

// getConfig returns the provided config if it is non-nil. Otherwise, it
// delegates to getBootstrapConfiguration to retrieve the configuration.
func (p *Pool) getConfig(config *bootstrap.Config) (*bootstrap.Config, error) {
	if config != nil {
		return config, nil
	}
	return p.getBootstrapConfiguration()
}

// NewClientWithConfig returns an xDS client with the given name from the pool.
// If the client doesn't already exist, it creates a new xDS client using the
// provided config and adds it to the pool. The provided config takes precedence
// over any config passed during pool creation. This should be used when a
// non-default config is required. If the provided config is nil, it attempts
// to load the configuration provided at pool creation time.
//
// The second return value represents a close function which the caller is
// expected to invoke once they are done using the client.  It is safe for the
// caller to invoke this close function multiple times.
func (p *Pool) NewClientWithConfig(name string, metricsRecorder estats.MetricsRecorder, config *bootstrap.Config) (XDSClient, func(), error) {
	config, err := p.getConfig(config)
	if err != nil {
		return nil, nil, err
	}
	return p.newRefCounted(name, metricsRecorder, defaultWatchExpiryTimeout, config)
}

// NewClient returns an xDS client with the given name from the pool. If the
// client doesn't already exist, it loads the configuration and creates a new
// xDS client and adds it to the pool.
//
// The second return value represents a close function which the caller is
// expected to invoke once they are done using the client.  It is safe for the
// caller to invoke this close function multiple times.
func (p *Pool) NewClient(name string, metricsRecorder estats.MetricsRecorder) (XDSClient, func(), error) {
	config, err := p.getConfig(nil)
	if err != nil {
		return nil, nil, err
	}
	return p.newRefCounted(name, metricsRecorder, defaultWatchExpiryTimeout, config)
}