From 58b934133dac51ccac160779b4c3e709385dcde8 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Mon, 17 May 2021 16:01:32 -0400 Subject: [PATCH] hcs-1936: Prepare for adding license auto-retrieval to auto-config in enterprise --- .changelog/10248.txt | 7 +++++ agent/agent.go | 4 +-- agent/auto-config/auto_config.go | 30 ++++++++++++++----- agent/auto-config/auto_config_oss.go | 11 +++++++ agent/auto-config/auto_config_oss_test.go | 11 +++++++ agent/auto-config/auto_config_test.go | 22 +++++++------- agent/auto-config/auto_encrypt.go | 4 +-- agent/auto-config/auto_encrypt_test.go | 2 +- agent/auto-config/config.go | 3 ++ agent/auto-config/config_oss.go | 11 +++++++ agent/auto-config/mock_oss_test.go | 18 ++++++++++++ agent/auto-config/mock_test.go | 32 ++++++++++++++------- agent/consul/client.go | 5 ---- agent/consul/config.go | 3 -- agent/setup.go | 27 +++++++++++------ agent/setup_oss.go | 7 +++++ sdk/testutil/testlog.go | 8 +++++- test/integration/connect/envoy/run-tests.sh | 10 +++++++ 18 files changed, 161 insertions(+), 54 deletions(-) create mode 100644 .changelog/10248.txt create mode 100644 agent/auto-config/auto_config_oss.go create mode 100644 agent/auto-config/auto_config_oss_test.go create mode 100644 agent/auto-config/config_oss.go create mode 100644 agent/auto-config/mock_oss_test.go diff --git a/.changelog/10248.txt b/.changelog/10248.txt new file mode 100644 index 000000000..54ae19dab --- /dev/null +++ b/.changelog/10248.txt @@ -0,0 +1,7 @@ +```release-note:breaking-change +licensing: **(Enterprise Only)** Consul Enterprise has removed support for temporary licensing. All server agents must have a valid license at startup and client agents must have a license at startup or be able to retrieve one from the servers. +``` + +```release-note:breaking-change +licensing: **(Enterprise Only)** Consul Enterprise client agents now require a valid non-anonymous ACL token for retrieving their license from the servers. Additionally client agents rely on the value of the `start_join` and `retry_join` configurations for determining the servers to query for the license. Therefore one must be set to use license auto-retrieval. +``` \ No newline at end of file diff --git a/agent/agent.go b/agent/agent.go index 19e6a01bd..6f85f502b 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -457,9 +457,7 @@ func (a *Agent) Start(ctx context.Context) error { return fmt.Errorf("Failed to load TLS configurations after applying auto-config settings: %w", err) } - // we cannot use the context passed into this method as that context will be cancelled after the - // agent finishes starting up which would cause the license manager to stop - if err := a.startLicenseManager(&lib.StopChannelContext{StopCh: a.shutdownCh}); err != nil { + if err := a.startLicenseManager(ctx); err != nil { return err } diff --git a/agent/auto-config/auto_config.go b/agent/auto-config/auto_config.go index 11f1c7bf9..f3eedb7eb 100644 --- a/agent/auto-config/auto_config.go +++ b/agent/auto-config/auto_config.go @@ -92,6 +92,10 @@ func New(config Config) (*AutoConfig, error) { } } + if err := config.EnterpriseConfig.validateAndFinalize(); err != nil { + return nil, err + } + return &AutoConfig{ acConfig: config, logger: logger, @@ -126,13 +130,8 @@ func (ac *AutoConfig) ReadConfig() (*config.RuntimeConfig, error) { // The context passed in can be used to cancel the retrieval of the initial configuration // like when receiving a signal during startup. func (ac *AutoConfig) InitialConfiguration(ctx context.Context) (*config.RuntimeConfig, error) { - if ac.config == nil { - config, err := ac.ReadConfig() - if err != nil { - return nil, err - } - - ac.config = config + if err := ac.maybeLoadConfig(); err != nil { + return nil, err } switch { @@ -180,6 +179,23 @@ func (ac *AutoConfig) InitialConfiguration(ctx context.Context) (*config.Runtime } } +// maybeLoadConfig will read the Consul configuration using the +// provided config loader if and only if the config field of +// the struct is nil. When it does this it will fill in that +// field. If the config field already is non-nil then this +// is a noop. +func (ac *AutoConfig) maybeLoadConfig() error { + if ac.config == nil { + config, err := ac.ReadConfig() + if err != nil { + return err + } + + ac.config = config + } + return nil +} + // introToken is responsible for determining the correct intro token to use // when making the initial AutoConfig.InitialConfiguration RPC request. func (ac *AutoConfig) introToken() (string, error) { diff --git a/agent/auto-config/auto_config_oss.go b/agent/auto-config/auto_config_oss.go new file mode 100644 index 000000000..1ce93c6e7 --- /dev/null +++ b/agent/auto-config/auto_config_oss.go @@ -0,0 +1,11 @@ +// +build !consulent + +package autoconf + +// AutoConfigEnterprise has no fields in OSS +type AutoConfigEnterprise struct{} + +// newAutoConfigEnterprise initializes the enterprise AutoConfig struct +func newAutoConfigEnterprise(config Config) AutoConfigEnterprise { + return AutoConfigEnterprise{} +} diff --git a/agent/auto-config/auto_config_oss_test.go b/agent/auto-config/auto_config_oss_test.go new file mode 100644 index 000000000..2d6092f37 --- /dev/null +++ b/agent/auto-config/auto_config_oss_test.go @@ -0,0 +1,11 @@ +// +build !consulent + +package autoconf + +import ( + "testing" +) + +func newEnterpriseConfig(t *testing.T) EnterpriseConfig { + return EnterpriseConfig{} +} diff --git a/agent/auto-config/auto_config_test.go b/agent/auto-config/auto_config_test.go index 918acf0b5..c4b40606e 100644 --- a/agent/auto-config/auto_config_test.go +++ b/agent/auto-config/auto_config_test.go @@ -136,11 +136,12 @@ func TestNew(t *testing.T) { Loader: func(source config.Source) (result config.LoadResult, err error) { return config.LoadResult{}, nil }, - DirectRPC: newMockDirectRPC(t), - Tokens: newMockTokenStore(t), - Cache: newMockCache(t), - TLSConfigurator: newMockTLSConfigurator(t), - ServerProvider: newMockServerProvider(t), + DirectRPC: newMockDirectRPC(t), + Tokens: newMockTokenStore(t), + Cache: newMockCache(t), + TLSConfigurator: newMockTLSConfigurator(t), + ServerProvider: newMockServerProvider(t), + EnterpriseConfig: newEnterpriseConfig(t), } if tcase.modify != nil { @@ -211,18 +212,15 @@ func setupRuntimeConfig(t *testing.T) *configLoader { } func TestInitialConfiguration_disabled(t *testing.T) { - loader := setupRuntimeConfig(t) - loader.addConfigHCL(` + mcfg := newMockedConfig(t) + mcfg.loader.addConfigHCL(` primary_datacenter = "primary" auto_config = { enabled = false } `) - conf := newMockedConfig(t).Config - conf.Loader = loader.Load - - ac, err := New(conf) + ac, err := New(mcfg.Config) require.NoError(t, err) require.NotNil(t, ac) @@ -230,7 +228,7 @@ func TestInitialConfiguration_disabled(t *testing.T) { require.NoError(t, err) require.NotNil(t, cfg) require.Equal(t, "primary", cfg.PrimaryDatacenter) - require.NoFileExists(t, filepath.Join(*loader.opts.FlagValues.DataDir, autoConfigFileName)) + require.NoFileExists(t, filepath.Join(*mcfg.loader.opts.FlagValues.DataDir, autoConfigFileName)) } func TestInitialConfiguration_cancelled(t *testing.T) { diff --git a/agent/auto-config/auto_encrypt.go b/agent/auto-config/auto_encrypt.go index f2bf424aa..d42e25d47 100644 --- a/agent/auto-config/auto_encrypt.go +++ b/agent/auto-config/auto_encrypt.go @@ -43,7 +43,7 @@ func (ac *AutoConfig) autoEncryptInitialCertsOnce(ctx context.Context, csr, key } var resp structs.SignedResponse - servers, err := ac.autoEncryptHosts() + servers, err := ac.joinHosts() if err != nil { return nil, err } @@ -69,7 +69,7 @@ func (ac *AutoConfig) autoEncryptInitialCertsOnce(ctx context.Context, csr, key return nil, fmt.Errorf("No servers successfully responded to the auto-encrypt request") } -func (ac *AutoConfig) autoEncryptHosts() ([]string, error) { +func (ac *AutoConfig) joinHosts() ([]string, error) { // use servers known to gossip if there are any if ac.acConfig.ServerProvider != nil { if srv := ac.acConfig.ServerProvider.FindLANServer(); srv != nil { diff --git a/agent/auto-config/auto_encrypt_test.go b/agent/auto-config/auto_encrypt_test.go index a9d05c111..c92736ed6 100644 --- a/agent/auto-config/auto_encrypt_test.go +++ b/agent/auto-config/auto_encrypt_test.go @@ -182,7 +182,7 @@ func TestAutoEncrypt_hosts(t *testing.T) { }, } - hosts, err := ac.autoEncryptHosts() + hosts, err := ac.joinHosts() if tcase.err != "" { testutil.RequireErrorContains(t, err, tcase.err) } else { diff --git a/agent/auto-config/config.go b/agent/auto-config/config.go index 457cf4136..090b30dcc 100644 --- a/agent/auto-config/config.go +++ b/agent/auto-config/config.go @@ -104,4 +104,7 @@ type Config struct { // agent token as well as getting notifications when that token is updated. // This field is required. Tokens TokenStore + + // EnterpriseConfig is the embedded specific enterprise configurations + EnterpriseConfig } diff --git a/agent/auto-config/config_oss.go b/agent/auto-config/config_oss.go new file mode 100644 index 000000000..876d090d5 --- /dev/null +++ b/agent/auto-config/config_oss.go @@ -0,0 +1,11 @@ +// +build !consulent + +package autoconf + +// EnterpriseConfig stub - only populated in Consul Enterprise +type EnterpriseConfig struct{} + +// finalize is a noop for OSS +func (_ *EnterpriseConfig) validateAndFinalize() error { + return nil +} diff --git a/agent/auto-config/mock_oss_test.go b/agent/auto-config/mock_oss_test.go new file mode 100644 index 000000000..da227484f --- /dev/null +++ b/agent/auto-config/mock_oss_test.go @@ -0,0 +1,18 @@ +// +build !consulent + +package autoconf + +import ( + "testing" +) + +// mockedEnterpriseConfig is pretty much just a stub in OSS +// It does contain an enterprise config for compatibility +// purposes but that in and of itself is just a stub. +type mockedEnterpriseConfig struct { + EnterpriseConfig +} + +func newMockedEnterpriseConfig(t *testing.T) *mockedEnterpriseConfig { + return &mockedEnterpriseConfig{} +} diff --git a/agent/auto-config/mock_test.go b/agent/auto-config/mock_test.go index d828e6a84..8d63a05b4 100644 --- a/agent/auto-config/mock_test.go +++ b/agent/auto-config/mock_test.go @@ -218,20 +218,25 @@ func (m *mockTokenStore) StopNotify(notifier token.Notifier) { type mockedConfig struct { Config - directRPC *mockDirectRPC - serverProvider *mockServerProvider - cache *mockCache - tokens *mockTokenStore - tlsCfg *mockTLSConfigurator + loader *configLoader + directRPC *mockDirectRPC + serverProvider *mockServerProvider + cache *mockCache + tokens *mockTokenStore + tlsCfg *mockTLSConfigurator + enterpriseConfig *mockedEnterpriseConfig } func newMockedConfig(t *testing.T) *mockedConfig { + loader := setupRuntimeConfig(t) directRPC := newMockDirectRPC(t) serverProvider := newMockServerProvider(t) mcache := newMockCache(t) tokens := newMockTokenStore(t) tlsCfg := newMockTLSConfigurator(t) + entConfig := newMockedEnterpriseConfig(t) + // I am not sure it is well defined behavior but in testing it // out it does appear like Cleanup functions can fail tests // Adding in the mock expectations assertions here saves us @@ -248,18 +253,23 @@ func newMockedConfig(t *testing.T) *mockedConfig { return &mockedConfig{ Config: Config{ - DirectRPC: directRPC, - ServerProvider: serverProvider, - Cache: mcache, - Tokens: tokens, - TLSConfigurator: tlsCfg, - Logger: testutil.Logger(t), + Loader: loader.Load, + DirectRPC: directRPC, + ServerProvider: serverProvider, + Cache: mcache, + Tokens: tokens, + TLSConfigurator: tlsCfg, + Logger: testutil.Logger(t), + EnterpriseConfig: entConfig.EnterpriseConfig, }, + loader: loader, directRPC: directRPC, serverProvider: serverProvider, cache: mcache, tokens: tokens, tlsCfg: tlsCfg, + + enterpriseConfig: entConfig, } } diff --git a/agent/consul/client.go b/agent/consul/client.go index 78db6fe72..d71c81519 100644 --- a/agent/consul/client.go +++ b/agent/consul/client.go @@ -159,11 +159,6 @@ func NewClient(config *Config, deps Deps) (*Client, error) { go c.monitorACLMode() } - if err := c.startEnterprise(); err != nil { - c.Shutdown() - return nil, err - } - return c, nil } diff --git a/agent/consul/config.go b/agent/consul/config.go index 259c865af..b8dea5eca 100644 --- a/agent/consul/config.go +++ b/agent/consul/config.go @@ -350,9 +350,6 @@ type Config struct { // a Consul server is now up and known about. ServerUp func() - // Shutdown callback is used to trigger a full Consul shutdown - Shutdown func() - // UserEventHandler callback can be used to handle incoming // user events. This function should not block. UserEventHandler func(serf.UserEvent) diff --git a/agent/setup.go b/agent/setup.go index c35a54c6c..04ff50975 100644 --- a/agent/setup.go +++ b/agent/setup.go @@ -110,21 +110,30 @@ func NewBaseDeps(configLoader ConfigLoader, logOut io.Writer) (BaseDeps, error) d.Router = router.NewRouter(d.Logger, cfg.Datacenter, fmt.Sprintf("%s.%s", cfg.NodeName, cfg.Datacenter), builder) - acConf := autoconf.Config{ - DirectRPC: d.ConnPool, - Logger: d.Logger, - Loader: configLoader, - ServerProvider: d.Router, - TLSConfigurator: d.TLSConfigurator, - Cache: d.Cache, - Tokens: d.Tokens, + // this needs to happen prior to creating auto-config as some of the dependencies + // must also be passed to auto-config + d, err = initEnterpriseBaseDeps(d, cfg) + if err != nil { + return d, err } + + acConf := autoconf.Config{ + DirectRPC: d.ConnPool, + Logger: d.Logger, + Loader: configLoader, + ServerProvider: d.Router, + TLSConfigurator: d.TLSConfigurator, + Cache: d.Cache, + Tokens: d.Tokens, + EnterpriseConfig: initEnterpriseAutoConfig(d.EnterpriseDeps), + } + d.AutoConfig, err = autoconf.New(acConf) if err != nil { return d, err } - return initEnterpriseBaseDeps(d, cfg) + return d, nil } // grpcLogInitOnce because the test suite will call NewBaseDeps in many tests and diff --git a/agent/setup_oss.go b/agent/setup_oss.go index d16494133..7bda56a17 100644 --- a/agent/setup_oss.go +++ b/agent/setup_oss.go @@ -3,7 +3,9 @@ package agent import ( + autoconf "github.com/hashicorp/consul/agent/auto-config" "github.com/hashicorp/consul/agent/config" + "github.com/hashicorp/consul/agent/consul" ) // initEnterpriseBaseDeps is responsible for initializing the enterprise dependencies that @@ -11,3 +13,8 @@ import ( func initEnterpriseBaseDeps(d BaseDeps, _ *config.RuntimeConfig) (BaseDeps, error) { return d, nil } + +// initEnterpriseAutoConfig is responsible for setting up auto-config for enterprise +func initEnterpriseAutoConfig(_ consul.EnterpriseDeps) autoconf.EnterpriseConfig { + return autoconf.EnterpriseConfig{} +} diff --git a/sdk/testutil/testlog.go b/sdk/testutil/testlog.go index a47298391..fa0bd93db 100644 --- a/sdk/testutil/testlog.go +++ b/sdk/testutil/testlog.go @@ -23,6 +23,7 @@ func LoggerWithOutput(t TestingTB, output io.Writer) hclog.InterceptLogger { } var sendTestLogsToStdout = os.Getenv("NOLOGBUFFER") == "1" +var testLogOnlyFailed = os.Getenv("TEST_LOGGING_ONLY_FAILED") == "1" // NewLogBuffer returns an io.Writer which buffers all writes. When the test // ends, t.Failed is checked. If the test has failed or has been run in verbose @@ -30,13 +31,18 @@ var sendTestLogsToStdout = os.Getenv("NOLOGBUFFER") == "1" // // Set the env var NOLOGBUFFER=1 to disable buffering, resulting in all log // output being written immediately to stdout. +// +// Typically log output is written either for failed tests or when go test +// is running with the verbose flag (-v) set. Setting TEST_LOGGING_ONLY_FAILED=1 +// will prevent logs being output when the verbose flag is set if the test +// case is successful. func NewLogBuffer(t TestingTB) io.Writer { if sendTestLogsToStdout { return os.Stdout } buf := &logBuffer{buf: new(bytes.Buffer)} t.Cleanup(func() { - if t.Failed() || testing.Verbose() { + if t.Failed() || (!testLogOnlyFailed && testing.Verbose()) { buf.Lock() defer buf.Unlock() buf.buf.WriteTo(os.Stdout) diff --git a/test/integration/connect/envoy/run-tests.sh b/test/integration/connect/envoy/run-tests.sh index 81646f10f..f536e9ed2 100755 --- a/test/integration/connect/envoy/run-tests.sh +++ b/test/integration/connect/envoy/run-tests.sh @@ -142,6 +142,15 @@ function start_consul { '-p=9502:8502' ) fi + + license="${CONSUL_LICENSE:-}" + # load the consul license so we can pass it into the consul + # containers as an env var in the case that this is a consul + # enterprise test + if test -z "$license" -a -n "${CONSUL_LICENSE_PATH:-}" + then + license=$(cat $CONSUL_LICENSE_PATH) + fi # Run consul and expose some ports to the host to make debugging locally a # bit easier. @@ -151,6 +160,7 @@ function start_consul { $WORKDIR_SNIPPET \ --hostname "consul-${DC}" \ --network-alias "consul-${DC}" \ + -e "CONSUL_LICENSE=$license" \ ${ports[@]} \ consul-dev \ agent -dev -datacenter "${DC}" \