Repository: hashicorp/nomad
File: command/agent/agent.go (Go)

Code before:
		}

		current.TLSConfig = newConfig.TLSConfig
		current.TLSConfig.KeyLoader = keyloader
		a.config = current
		return nil
	} else if newConfig.TLSConfig.IsEmpty() && !current.TLSConfig.IsEmpty() {
		a.logger.Warn("downgrading agent's existing TLS configuration to plaintext")
		fullUpdateTLSConfig()
	} else if !newConfig.TLSConfig.IsEmpty() && current.TLSConfig.IsEmpty() {
		a.logger.Info("upgrading from plaintext configuration to TLS")
		fullUpdateTLSConfig()
	}

	// Set agent config to the updated config
	a.config = current
	return nil
}

// GetConfig returns the current agent configuration. The Config should *not*
// be mutated directly. First call Config.Copy.
func (a *Agent) GetConfig() *Config {
	a.configLock.Lock()
	defer a.configLock.Unlock()

	return a.config
}

// GetMetricsSink returns the metrics sink.
func (a *Agent) GetMetricsSink() *metrics.InmemSink {
	return a.inmemSink
}

func (a *Agent) setupConsuls(cfgs []*config.ConsulConfig) error {

	isClient := false
	if a.config.Client != nil && a.config.Client.Enabled {
		isClient = true
	}

	a.consulServices = consul.NewServiceClientWrapper()
	consulProxies := map[string]*consul.ConnectProxies{}
	consulConfigEntries := map[string]consul.ConfigAPI{}

	for _, consulConfig := range cfgs {
		cluster := consulConfig.Name
		apiConf, err := consulConfig.ApiConfig()
		if err != nil {
			return err
		}


Review comment (refactor):
it's an unusual pattern for Go, no? I'd expect this to be a helper function that just returns an error and then make it up to the caller to decide how to handle an error. But since an error is not a blocker here I suppose this is fine?

Code after:
	// file to be reopened if necessary
	if err := a.auditor.Reopen(); err != nil {
		return err
	}

	fullUpdateTLSConfig := func() {
		// Completely reload the agent's TLS configuration (moving from non-TLS to
		// TLS, or vice versa)
		// This does not handle errors in loading the new TLS configuration
		current.TLSConfig = newConfig.TLSConfig.Copy()
	}

	// The metric reload function is used to stop/start the TLS metrics emitter
	// when we are reloading the TLS configuration. We need to do this in both a
	// full reload and a partial reload, so new certificates are picked up.
	//
	// A failure to initialize the TLS metrics emitter is not considered
	// terminal to the reload but is logged. This error can only occur when we
	// attempt to read the certificates, so it would indicate a problem with the
	// certificate files/permissions that would also impact the agent TLS
	// configuration as a whole. In fact, if we succeed in the ShouldReload
	// check, we should succeed here as that too reads the files.
	tlsMetricReloadFn := func() {
		tlsMetrics, err := newTLSMetrics(a.logger, newConfig.TLSConfig, nil)
		if err != nil {
			a.logger.Error("failed to initialize TLS metrics emitter", "error", err)
		} else {
			// We have successfully initialized the new TLS metrics emitter, so
			// we can stop the old one (if it exists) and start the new one.
			if a.tlsMetrics != nil {
				a.tlsMetrics.stop()
			}

			a.tlsMetrics = tlsMetrics
			a.tlsMetrics.start(newConfig.Telemetry.collectionInterval)
		}
	}

	if !current.TLSConfig.IsEmpty() && !newConfig.TLSConfig.IsEmpty() {
		// This is just a TLS configuration reload, we don't need to refresh
		// existing network connections

		// Reload the certificates on the keyloader and on success store the
		// updated TLS config. It is important to reuse the same keyloader
		// as this allows us to dynamically reload configurations not only
		// on the Agent but on the Server and Client too (they are
		// referencing the same keyloader).
		keyloader := current.TLSConfig.GetKeyLoader()
		_, err := keyloader.LoadKeyPair(newConfig.TLSConfig.CertFile, newConfig.TLSConfig.KeyFile)
		if err != nil {
			return err