From 8cee920f72c5a6534f0d560abc5edd109ab27aec Mon Sep 17 00:00:00 2001 From: Leonard Goodell Date: Tue, 25 Apr 2023 17:58:28 -0700 Subject: [PATCH 1/2] feat: Add -d/--dev common command-line flag to put service in Dev Mode Dev Mode is used when running service locally with the other services running in Docker (aka hybrid mode). This causes all the `Host` settings pulled from common configuration to be overridden with the value `localhost`. closes #509 Signed-off-by: Leonard Goodell --- bootstrap/config/config.go | 29 +++++++++++++++++++++++++ bootstrap/config/configmock_test.go | 7 +----- bootstrap/flags/flags.go | 11 ++++++++++ bootstrap/handlers/clients.go | 11 ++++++---- bootstrap/handlers/clients_test.go | 27 ++++++++++++----------- bootstrap/handlers/messaging.go | 2 +- bootstrap/handlers/messaging_test.go | 2 +- bootstrap/handlers/metrics_test.go | 4 +++- bootstrap/registration/registry_test.go | 4 ++-- config/types.go | 9 ++++---- 10 files changed, 74 insertions(+), 32 deletions(-) diff --git a/bootstrap/config/config.go b/bootstrap/config/config.go index 5731a7c4..61702038 100644 --- a/bootstrap/config/config.go +++ b/bootstrap/config/config.go @@ -253,6 +253,35 @@ func (cp *Processor) Process( // Now that configuration has been loaded and overrides applied the log level can be set as configured. err = cp.lc.SetLogLevel(serviceConfig.GetLogLevel()) + if cp.flags.InDevMode() { + // Dev mode is for when running service with Config Provider in hybrid mode (all other service running in Docker). + // All the host values are set to the docker names in the common configuration, so must be overridden here with "localhost" + host := "localhost" + config := serviceConfig.GetBootstrap() + + if config.Service != nil { + config.Service.Host = host + } + + if config.MessageBus != nil { + config.MessageBus.Host = host + } + + if config.Registry != nil { + config.Registry.Host = host + } + + if config.Database != nil { + config.Database.Host = host + } + + if config.Clients != nil { + for _, client := range config.Clients { + client.Host = host + } + } + } + return err } diff --git a/bootstrap/config/configmock_test.go b/bootstrap/config/configmock_test.go index 795869b8..024331cc 100644 --- a/bootstrap/config/configmock_test.go +++ b/bootstrap/config/configmock_test.go @@ -64,8 +64,7 @@ func (c *ConfigurationMockStruct) UpdateWritableFromRaw(rawWritable interface{}) func (c *ConfigurationMockStruct) GetBootstrap() config.BootstrapConfiguration { return config.BootstrapConfiguration{ - Service: c.transformToBootstrapServiceInfo(), - Registry: c.Registry, + Registry: &c.Registry, } } @@ -85,10 +84,6 @@ func (c *ConfigurationMockStruct) GetTelemetryInfo() *config.TelemetryInfo { return &c.Writable.Telemetry } -func (c *ConfigurationMockStruct) transformToBootstrapServiceInfo() config.ServiceInfo { - return config.ServiceInfo{} -} - func (c *ConfigurationMockStruct) GetWritablePtr() any { return &c.Writable } diff --git a/bootstrap/flags/flags.go b/bootstrap/flags/flags.go index 2af6fe10..24665807 100644 --- a/bootstrap/flags/flags.go +++ b/bootstrap/flags/flags.go @@ -30,6 +30,7 @@ const ( type Common interface { OverwriteConfig() bool UseRegistry() bool + InDevMode() bool ConfigProviderUrl() string Profile() string ConfigDirectory() string @@ -45,6 +46,7 @@ type Default struct { additionalUsage string overwriteConfig bool useRegistry bool + devMode bool configProviderUrl string commonConfig string profile string @@ -97,6 +99,8 @@ func (d *Default) Parse(arguments []string) { d.FlagSet.StringVar(&d.configDir, "cd", "", "") d.FlagSet.BoolVar(&d.useRegistry, "registry", false, "") d.FlagSet.BoolVar(&d.useRegistry, "r", false, "") + d.FlagSet.BoolVar(&d.devMode, "dev", false, "") + d.FlagSet.BoolVar(&d.devMode, "d", false, "") d.FlagSet.Usage = d.helpCallback @@ -117,6 +121,11 @@ func (d *Default) UseRegistry() bool { return d.useRegistry } +// InDevMode returns whether running in dev mode or not +func (d *Default) InDevMode() bool { + return d.devMode +} + // ConfigProviderUrl returns the url for the Configuration Provider, if one was specified. func (d *Default) ConfigProviderUrl() string { return d.configProviderUrl @@ -163,6 +172,8 @@ func (d *Default) helpCallback() { " -p, --profile Indicate configuration profile other than default\n"+ " -cd, --configDir Specify local configuration directory\n"+ " -r, --registry Indicates service should use Registry.\n"+ + " -d, --dev Indicates service to run in developer mode which causes Host configuration values to be overridden.\n"+ + " with `localhost`. This is so that it will run with other services running in Docker (aka hybrid mode)\n"+ "%s\n"+ "Common Options:\n"+ " -h, --help Show this message\n", diff --git a/bootstrap/handlers/clients.go b/bootstrap/handlers/clients.go index 95510cfa..954de443 100644 --- a/bootstrap/handlers/clients.go +++ b/bootstrap/handlers/clients.go @@ -37,12 +37,15 @@ import ( // ClientsBootstrap contains data to boostrap the configured clients type ClientsBootstrap struct { - registry registry.Client + registry registry.Client + inDevMode bool } // NewClientsBootstrap is a factory method that returns the initialized "ClientsBootstrap" receiver struct. -func NewClientsBootstrap() *ClientsBootstrap { - return &ClientsBootstrap{} +func NewClientsBootstrap(devMode bool) *ClientsBootstrap { + return &ClientsBootstrap{ + inDevMode: devMode, + } } // BootstrapHandler fulfills the BootstrapHandler contract. @@ -156,7 +159,7 @@ func (cb *ClientsBootstrap) BootstrapHandler( } func (cb *ClientsBootstrap) getClientUrl(serviceKey string, defaultUrl string, startupTimer startup.Timer, lc logger.LoggingClient) (string, error) { - if cb.registry == nil { + if cb.registry == nil || cb.inDevMode { lc.Infof("Using REST for '%s' clients @ %s", serviceKey, defaultUrl) return defaultUrl, nil } diff --git a/bootstrap/handlers/clients_test.go b/bootstrap/handlers/clients_test.go index 6ecf07ec..33de3726 100644 --- a/bootstrap/handlers/clients_test.go +++ b/bootstrap/handlers/clients_test.go @@ -173,33 +173,34 @@ func TestClientsBootstrapHandler(t *testing.T) { for _, test := range tests { t.Run(test.Name, func(t *testing.T) { - clients := make(map[string]config.ClientInfo) + clients := make(map[string]*config.ClientInfo) if test.CoreDataClientInfo != nil { - clients[common.CoreDataServiceKey] = *test.CoreDataClientInfo + clients[common.CoreDataServiceKey] = test.CoreDataClientInfo } if test.CommandClientInfo != nil { - clients[common.CoreCommandServiceKey] = *test.CommandClientInfo + clients[common.CoreCommandServiceKey] = test.CommandClientInfo } if test.MetadataClientInfo != nil { - clients[common.CoreMetaDataServiceKey] = *test.MetadataClientInfo + clients[common.CoreMetaDataServiceKey] = test.MetadataClientInfo } if test.NotificationClientInfo != nil { - clients[common.SupportNotificationsServiceKey] = *test.NotificationClientInfo + clients[common.SupportNotificationsServiceKey] = test.NotificationClientInfo } if test.SchedulerClientInfo != nil { - clients[common.SupportSchedulerServiceKey] = *test.SchedulerClientInfo + clients[common.SupportSchedulerServiceKey] = test.SchedulerClientInfo } bootstrapConfig := config.BootstrapConfiguration{ - Service: config.ServiceInfo{ + Service: &config.ServiceInfo{ RequestTimeout: "30s", }, - Clients: clients, + Clients: clients, + MessageBus: &config.MessageBusInfo{}, } configMock := &mocks.Configuration{} @@ -223,7 +224,7 @@ func TestClientsBootstrapHandler(t *testing.T) { }, }) - actualResult := NewClientsBootstrap().BootstrapHandler(context.Background(), &sync.WaitGroup{}, startupTimer, dic) + actualResult := NewClientsBootstrap(false).BootstrapHandler(context.Background(), &sync.WaitGroup{}, startupTimer, dic) require.Equal(t, actualResult, test.ExpectedResult) if test.ExpectedResult == false { return @@ -308,13 +309,13 @@ func TestCommandMessagingClientErrors(t *testing.T) { mockMessaging := &messagingMocks.MessageClient{} - clients := make(map[string]config.ClientInfo) - clients[common.CoreCommandServiceKey] = config.ClientInfo{ + clients := make(map[string]*config.ClientInfo) + clients[common.CoreCommandServiceKey] = &config.ClientInfo{ UseMessageBus: true, } bootstrapConfig := config.BootstrapConfiguration{ - Service: config.ServiceInfo{ + Service: &config.ServiceInfo{ RequestTimeout: test.TimeoutDuration, }, Clients: clients, @@ -340,7 +341,7 @@ func TestCommandMessagingClientErrors(t *testing.T) { }) startupTimer := startup.NewTimer(1, 1) - actualResult := NewClientsBootstrap().BootstrapHandler(context.Background(), &sync.WaitGroup{}, startupTimer, dic) + actualResult := NewClientsBootstrap(false).BootstrapHandler(context.Background(), &sync.WaitGroup{}, startupTimer, dic) require.False(t, actualResult) mockLogger.AssertNumberOfCalls(t, "Errorf", 1) diff --git a/bootstrap/handlers/messaging.go b/bootstrap/handlers/messaging.go index db2182e9..5156ab36 100644 --- a/bootstrap/handlers/messaging.go +++ b/bootstrap/handlers/messaging.go @@ -47,7 +47,7 @@ func MessagingBootstrapHandler(ctx context.Context, wg *sync.WaitGroup, startupT } // Make sure the MessageBus password is not leaked into the Service Config that can be retrieved via the /config endpoint - messageBusInfo := deepCopy(messageBus) + messageBusInfo := deepCopy(*messageBus) if len(messageBusInfo.AuthMode) > 0 && !strings.EqualFold(strings.TrimSpace(messageBusInfo.AuthMode), boostrapMessaging.AuthModeNone) { diff --git a/bootstrap/handlers/messaging_test.go b/bootstrap/handlers/messaging_test.go index 7dbea1e1..a36b8d57 100644 --- a/bootstrap/handlers/messaging_test.go +++ b/bootstrap/handlers/messaging_test.go @@ -84,7 +84,7 @@ func TestBootstrapHandler(t *testing.T) { providerMock.On("GetSecret", test.MessageBus.SecretName).Return(usernameSecretData, nil) configMock := &mocks.Configuration{} configMock.On("GetBootstrap").Return(config.BootstrapConfiguration{ - MessageBus: test.MessageBus, + MessageBus: &test.MessageBus, }) dic.Update(di.ServiceConstructorMap{ diff --git a/bootstrap/handlers/metrics_test.go b/bootstrap/handlers/metrics_test.go index 1ba012ff..d975a64b 100644 --- a/bootstrap/handlers/metrics_test.go +++ b/bootstrap/handlers/metrics_test.go @@ -50,7 +50,9 @@ func TestServiceMetrics_BootstrapHandler(t *testing.T) { mockMessagingClient := &mocks.MessageClient{} mockConfiguration := &mocks2.Configuration{} - mockConfiguration.On("GetBootstrap").Return(config.BootstrapConfiguration{}) + mockConfiguration.On("GetBootstrap").Return(config.BootstrapConfiguration{ + MessageBus: &config.MessageBusInfo{}, + }) dic := di.NewContainer(di.ServiceConstructorMap{ container.LoggingClientInterfaceName: func(get di.Get) interface{} { diff --git a/bootstrap/registration/registry_test.go b/bootstrap/registration/registry_test.go index 7ca682e1..5c4a221f 100644 --- a/bootstrap/registration/registry_test.go +++ b/bootstrap/registration/registry_test.go @@ -81,8 +81,8 @@ func (ut unitTestConfiguration) UpdateWritableFromRaw(_ interface{}) bool { func (ut unitTestConfiguration) GetBootstrap() config.BootstrapConfiguration { return config.BootstrapConfiguration{ - Service: ut.Service, - Registry: ut.Registry, + Service: &ut.Service, + Registry: &ut.Registry, } } diff --git a/config/types.go b/config/types.go index d4ddf89e..02d9ec35 100644 --- a/config/types.go +++ b/config/types.go @@ -226,11 +226,12 @@ type InsecureSecretsInfo struct { // BootstrapConfiguration defines the configuration elements required by the bootstrap. type BootstrapConfiguration struct { - Clients map[string]ClientInfo - Service ServiceInfo + Clients map[string]*ClientInfo + Service *ServiceInfo Config ConfigProviderInfo - Registry RegistryInfo - MessageBus MessageBusInfo + Registry *RegistryInfo + MessageBus *MessageBusInfo + Database *Database ExternalMQTT ExternalMQTTInfo } From fd80cfe282a85a4d1aaa4ca5978b790f5bec7316 Mon Sep 17 00:00:00 2001 From: Leonard Goodell Date: Wed, 26 Apr 2023 08:34:43 -0700 Subject: [PATCH 2/2] fix: Addressed PR feedback about using pointers for all bootstrap objects Signed-off-by: Leonard Goodell --- bootstrap/config/config.go | 2 +- bootstrap/handlers/clients.go | 165 +++++++++++++++-------------- bootstrap/handlers/clients_test.go | 8 +- config/types.go | 9 +- 4 files changed, 94 insertions(+), 90 deletions(-) diff --git a/bootstrap/config/config.go b/bootstrap/config/config.go index 61702038..74b35672 100644 --- a/bootstrap/config/config.go +++ b/bootstrap/config/config.go @@ -276,7 +276,7 @@ func (cp *Processor) Process( } if config.Clients != nil { - for _, client := range config.Clients { + for _, client := range *config.Clients { client.Host = host } } diff --git a/bootstrap/handlers/clients.go b/bootstrap/handlers/clients.go index 954de443..f38b4be4 100644 --- a/bootstrap/handlers/clients.go +++ b/bootstrap/handlers/clients.go @@ -63,98 +63,99 @@ func (cb *ClientsBootstrap) BootstrapHandler( cb.registry = container.RegistryFrom(dic.Get) jwtSecretProvider := secret.NewJWTSecretProvider(container.SecretProviderExtFrom(dic.Get)) - for serviceKey, serviceInfo := range config.GetBootstrap().Clients { - var url string - var err error - - if !serviceInfo.UseMessageBus { - url, err = cb.getClientUrl(serviceKey, serviceInfo.Url(), startupTimer, lc) - if err != nil { - lc.Error(err.Error()) - return false - } - } + if config.GetBootstrap().Clients != nil { + for serviceKey, serviceInfo := range *config.GetBootstrap().Clients { + var url string + var err error - switch serviceKey { - case common.CoreDataServiceKey: - dic.Update(di.ServiceConstructorMap{ - container.EventClientName: func(get di.Get) interface{} { - return clients.NewEventClient(url, jwtSecretProvider) - }, - }) - case common.CoreMetaDataServiceKey: - dic.Update(di.ServiceConstructorMap{ - container.DeviceClientName: func(get di.Get) interface{} { - return clients.NewDeviceClient(url, jwtSecretProvider) - }, - container.DeviceServiceClientName: func(get di.Get) interface{} { - return clients.NewDeviceServiceClient(url, jwtSecretProvider) - }, - container.DeviceProfileClientName: func(get di.Get) interface{} { - return clients.NewDeviceProfileClient(url, jwtSecretProvider) - }, - container.ProvisionWatcherClientName: func(get di.Get) interface{} { - return clients.NewProvisionWatcherClient(url, jwtSecretProvider) - }, - }) - - case common.CoreCommandServiceKey: - var client interfaces.CommandClient - - if serviceInfo.UseMessageBus { - // TODO: Move following outside loop when multiple messaging based clients exist - messageClient := container.MessagingClientFrom(dic.Get) - if messageClient == nil { - lc.Errorf("Unable to create Command client using MessageBus: %s", "MessageBus Client was not created") + if !serviceInfo.UseMessageBus { + url, err = cb.getClientUrl(serviceKey, serviceInfo.Url(), startupTimer, lc) + if err != nil { + lc.Error(err.Error()) return false } + } - // TODO: Move following outside loop when multiple messaging based clients exist - timeout, err := time.ParseDuration(config.GetBootstrap().Service.RequestTimeout) - if err != nil { - lc.Errorf("Unable to parse Service.RequestTimeout as a time duration: %v", err) - return false + switch serviceKey { + case common.CoreDataServiceKey: + dic.Update(di.ServiceConstructorMap{ + container.EventClientName: func(get di.Get) interface{} { + return clients.NewEventClient(url, jwtSecretProvider) + }, + }) + case common.CoreMetaDataServiceKey: + dic.Update(di.ServiceConstructorMap{ + container.DeviceClientName: func(get di.Get) interface{} { + return clients.NewDeviceClient(url, jwtSecretProvider) + }, + container.DeviceServiceClientName: func(get di.Get) interface{} { + return clients.NewDeviceServiceClient(url, jwtSecretProvider) + }, + container.DeviceProfileClientName: func(get di.Get) interface{} { + return clients.NewDeviceProfileClient(url, jwtSecretProvider) + }, + container.ProvisionWatcherClientName: func(get di.Get) interface{} { + return clients.NewProvisionWatcherClient(url, jwtSecretProvider) + }, + }) + + case common.CoreCommandServiceKey: + var client interfaces.CommandClient + + if serviceInfo.UseMessageBus { + // TODO: Move following outside loop when multiple messaging based clients exist + messageClient := container.MessagingClientFrom(dic.Get) + if messageClient == nil { + lc.Errorf("Unable to create Command client using MessageBus: %s", "MessageBus Client was not created") + return false + } + + // TODO: Move following outside loop when multiple messaging based clients exist + timeout, err := time.ParseDuration(config.GetBootstrap().Service.RequestTimeout) + if err != nil { + lc.Errorf("Unable to parse Service.RequestTimeout as a time duration: %v", err) + return false + } + + baseTopic := config.GetBootstrap().MessageBus.GetBaseTopicPrefix() + client = clientsMessaging.NewCommandClient(messageClient, baseTopic, timeout) + + lc.Infof("Using messaging for '%s' clients", serviceKey) + } else { + client = clients.NewCommandClient(url, jwtSecretProvider) } - baseTopic := config.GetBootstrap().MessageBus.GetBaseTopicPrefix() - client = clientsMessaging.NewCommandClient(messageClient, baseTopic, timeout) + dic.Update(di.ServiceConstructorMap{ + container.CommandClientName: func(get di.Get) interface{} { + return client + }, + }) + + case common.SupportNotificationsServiceKey: + dic.Update(di.ServiceConstructorMap{ + container.NotificationClientName: func(get di.Get) interface{} { + return clients.NewNotificationClient(url, jwtSecretProvider) + }, + container.SubscriptionClientName: func(get di.Get) interface{} { + return clients.NewSubscriptionClient(url, jwtSecretProvider) + }, + }) + + case common.SupportSchedulerServiceKey: + dic.Update(di.ServiceConstructorMap{ + container.IntervalClientName: func(get di.Get) interface{} { + return clients.NewIntervalClient(url, jwtSecretProvider) + }, + container.IntervalActionClientName: func(get di.Get) interface{} { + return clients.NewIntervalActionClient(url, jwtSecretProvider) + }, + }) + + default: - lc.Infof("Using messaging for '%s' clients", serviceKey) - } else { - client = clients.NewCommandClient(url, jwtSecretProvider) } - - dic.Update(di.ServiceConstructorMap{ - container.CommandClientName: func(get di.Get) interface{} { - return client - }, - }) - - case common.SupportNotificationsServiceKey: - dic.Update(di.ServiceConstructorMap{ - container.NotificationClientName: func(get di.Get) interface{} { - return clients.NewNotificationClient(url, jwtSecretProvider) - }, - container.SubscriptionClientName: func(get di.Get) interface{} { - return clients.NewSubscriptionClient(url, jwtSecretProvider) - }, - }) - - case common.SupportSchedulerServiceKey: - dic.Update(di.ServiceConstructorMap{ - container.IntervalClientName: func(get di.Get) interface{} { - return clients.NewIntervalClient(url, jwtSecretProvider) - }, - container.IntervalActionClientName: func(get di.Get) interface{} { - return clients.NewIntervalActionClient(url, jwtSecretProvider) - }, - }) - - default: - } } - return true } diff --git a/bootstrap/handlers/clients_test.go b/bootstrap/handlers/clients_test.go index 33de3726..626fbf86 100644 --- a/bootstrap/handlers/clients_test.go +++ b/bootstrap/handlers/clients_test.go @@ -173,7 +173,7 @@ func TestClientsBootstrapHandler(t *testing.T) { for _, test := range tests { t.Run(test.Name, func(t *testing.T) { - clients := make(map[string]*config.ClientInfo) + clients := make(config.ClientsCollection) if test.CoreDataClientInfo != nil { clients[common.CoreDataServiceKey] = test.CoreDataClientInfo @@ -199,7 +199,7 @@ func TestClientsBootstrapHandler(t *testing.T) { Service: &config.ServiceInfo{ RequestTimeout: "30s", }, - Clients: clients, + Clients: &clients, MessageBus: &config.MessageBusInfo{}, } @@ -309,7 +309,7 @@ func TestCommandMessagingClientErrors(t *testing.T) { mockMessaging := &messagingMocks.MessageClient{} - clients := make(map[string]*config.ClientInfo) + clients := make(config.ClientsCollection) clients[common.CoreCommandServiceKey] = &config.ClientInfo{ UseMessageBus: true, } @@ -318,7 +318,7 @@ func TestCommandMessagingClientErrors(t *testing.T) { Service: &config.ServiceInfo{ RequestTimeout: test.TimeoutDuration, }, - Clients: clients, + Clients: &clients, } configMock := &mocks.Configuration{} diff --git a/config/types.go b/config/types.go index 02d9ec35..dd35ea52 100644 --- a/config/types.go +++ b/config/types.go @@ -224,15 +224,18 @@ type InsecureSecretsInfo struct { SecretData map[string]string } +// ClientsCollection is a collection of Client information for communicating to dependent clients. +type ClientsCollection map[string]*ClientInfo + // BootstrapConfiguration defines the configuration elements required by the bootstrap. type BootstrapConfiguration struct { - Clients map[string]*ClientInfo + Clients *ClientsCollection Service *ServiceInfo - Config ConfigProviderInfo + Config *ConfigProviderInfo Registry *RegistryInfo MessageBus *MessageBusInfo Database *Database - ExternalMQTT ExternalMQTTInfo + ExternalMQTT *ExternalMQTTInfo } // MessageBusInfo provides parameters related to connecting to the EdgeX MessageBus