From 5ffc6c730a08ceec1433951d0b36389f0b334d2f Mon Sep 17 00:00:00 2001 From: lenny Date: Thu, 25 Feb 2021 13:18:46 -0700 Subject: [PATCH 1/2] refactor: Rename MqttBroker configuration to ExternalMqtt This name matches the corresponding Binding type of `external-mqtt` closes #673 BREAKING CHANGE: Configuration section name changed Signed-off-by: lenny --- internal/common/config.go | 6 +++--- internal/trigger/mqtt/mqtt.go | 8 ++++---- internal/webserver/server_test.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/common/config.go b/internal/common/config.go index a2a0033d6..6a0456885 100644 --- a/internal/common/config.go +++ b/internal/common/config.go @@ -51,7 +51,7 @@ type ConfigurationStruct struct { // MessageBus MessageBus types.MessageBusConfig // MqttBroker - MqttBroker MqttBrokerConfig + ExternalMqtt ExternalMqttConfig // Binding Binding BindingInfo // ApplicationSettings @@ -91,8 +91,8 @@ type BindingInfo struct { PublishTopic string } -// MqttBrokerConfig contains the MQTT broker configuration for MQTT Trigger -type MqttBrokerConfig struct { +// ExternalMqttConfig contains the MQTT broker configuration for MQTT Trigger +type ExternalMqttConfig struct { // Url contains the fully qualified URL to connect to the MQTT broker Url string // ClientId to connect to the broker with. diff --git a/internal/trigger/mqtt/mqtt.go b/internal/trigger/mqtt/mqtt.go index ede125046..d7b2b5b89 100644 --- a/internal/trigger/mqtt/mqtt.go +++ b/internal/trigger/mqtt/mqtt.go @@ -65,7 +65,7 @@ func NewTrigger( func (trigger *Trigger) Initialize(_ *sync.WaitGroup, _ context.Context, background <-chan types.MessageEnvelope) (bootstrap.Deferred, error) { // Convenience short cuts logger := trigger.edgeXClients.LoggingClient - brokerConfig := trigger.configuration.MqttBroker + brokerConfig := trigger.configuration.ExternalMqtt topics := trigger.configuration.Binding.SubscribeTopics logger.Info("Initializing MQTT Trigger") @@ -80,7 +80,7 @@ func (trigger *Trigger) Initialize(_ *sync.WaitGroup, _ context.Context, backgro brokerUrl, err := url.Parse(brokerConfig.Url) if err != nil { - return nil, fmt.Errorf("invalid MQTT Broker Url '%s': %s", trigger.configuration.MqttBroker.Url, err.Error()) + return nil, fmt.Errorf("invalid MQTT Broker Url '%s': %s", trigger.configuration.ExternalMqtt.Url, err.Error()) } opts := pahoMqtt.NewClientOptions() @@ -132,7 +132,7 @@ func (trigger *Trigger) onConnectHandler(mqttClient pahoMqtt.Client) { // Convenience short cuts logger := trigger.edgeXClients.LoggingClient topics := util.DeleteEmptyAndTrim(strings.FieldsFunc(trigger.configuration.Binding.SubscribeTopics, util.SplitComma)) - qos := trigger.configuration.MqttBroker.QoS + qos := trigger.configuration.ExternalMqtt.QoS for _, topic := range topics { if token := mqttClient.Subscribe(topic, qos, trigger.messageHandler); token.Wait() && token.Error() != nil { @@ -149,7 +149,7 @@ func (trigger *Trigger) onConnectHandler(mqttClient pahoMqtt.Client) { func (trigger *Trigger) messageHandler(client pahoMqtt.Client, message pahoMqtt.Message) { // Convenience short cuts logger := trigger.edgeXClients.LoggingClient - brokerConfig := trigger.configuration.MqttBroker + brokerConfig := trigger.configuration.ExternalMqtt topic := trigger.configuration.Binding.PublishTopic data := message.Payload() diff --git a/internal/webserver/server_test.go b/internal/webserver/server_test.go index 8422fb4eb..338a2e55a 100644 --- a/internal/webserver/server_test.go +++ b/internal/webserver/server_test.go @@ -110,7 +110,7 @@ func TestConfigureAndConfigRoute(t *testing.T) { rr := httptest.NewRecorder() webserver.router.ServeHTTP(rr, req) - expected := `{"Writable":{"LogLevel":"","Pipeline":{"ExecutionOrder":"","UseTargetTypeOfByteArray":false,"Functions":null},"StoreAndForward":{"Enabled":false,"RetryInterval":"","MaxRetryCount":0},"InsecureSecrets":null},"Registry":{"Host":"","Port":0,"Type":""},"Service":{"BootTimeout":"","CheckInterval":"","Host":"","HTTPSCert":"","HTTPSKey":"","ServerBindAddr":"","Port":0,"Protocol":"","StartupMsg":"","ReadMaxLimit":0,"Timeout":""},"MessageBus":{"PublishHost":{"Host":"","Port":0,"Protocol":""},"SubscribeHost":{"Host":"","Port":0,"Protocol":""},"Type":"","Optional":null},"MqttBroker":{"Url":"","ClientId":"","ConnectTimeout":"","AutoReconnect":false,"KeepAlive":0,"QoS":0,"Retain":false,"SkipCertVerify":false,"SecretPath":"","AuthMode":""},"Binding":{"Type":"","SubscribeTopics":"","PublishTopic":""},"ApplicationSettings":null,"Clients":null,"Database":{"Type":"","Host":"","Port":0,"Timeout":"","MaxIdle":0,"BatchSize":0},"SecretStore":{"Type":"","Host":"","Port":0,"Path":"","Protocol":"","Namespace":"","RootCaCertPath":"","ServerName":"","Authentication":{"AuthType":"","AuthToken":""},"AdditionalRetryAttempts":0,"RetryWaitPeriod":"","TokenFile":""}}` + "\n" + expected := `{"Writable":{"LogLevel":"","Pipeline":{"ExecutionOrder":"","UseTargetTypeOfByteArray":false,"Functions":null},"StoreAndForward":{"Enabled":false,"RetryInterval":"","MaxRetryCount":0},"InsecureSecrets":null},"Registry":{"Host":"","Port":0,"Type":""},"Service":{"BootTimeout":"","CheckInterval":"","Host":"","HTTPSCert":"","HTTPSKey":"","ServerBindAddr":"","Port":0,"Protocol":"","StartupMsg":"","ReadMaxLimit":0,"Timeout":""},"MessageBus":{"PublishHost":{"Host":"","Port":0,"Protocol":""},"SubscribeHost":{"Host":"","Port":0,"Protocol":""},"Type":"","Optional":null},"ExternalMqtt":{"Url":"","ClientId":"","ConnectTimeout":"","AutoReconnect":false,"KeepAlive":0,"QoS":0,"Retain":false,"SkipCertVerify":false,"SecretPath":"","AuthMode":""},"Binding":{"Type":"","SubscribeTopics":"","PublishTopic":""},"ApplicationSettings":null,"Clients":null,"Database":{"Type":"","Host":"","Port":0,"Timeout":"","MaxIdle":0,"BatchSize":0},"SecretStore":{"Type":"","Host":"","Port":0,"Path":"","Protocol":"","Namespace":"","RootCaCertPath":"","ServerName":"","Authentication":{"AuthType":"","AuthToken":""},"AdditionalRetryAttempts":0,"RetryWaitPeriod":"","TokenFile":""}}` + "\n" body := rr.Body.String() assert.Equal(t, expected, body) From e44cd7feed8e5350dd7671d94e3b4410b95d4e22 Mon Sep 17 00:00:00 2001 From: lenny Date: Thu, 25 Feb 2021 13:37:42 -0700 Subject: [PATCH 2/2] refactor: Remove deprecated environment variables and related code closes #337 BREAKING CHANGE: The following environment variables no longer supported: - `edgex_profile` (replaced by uppercase version) - `edgex_service` Signed-off-by: lenny --- appsdk/sdk.go | 49 ---------------------------------------------- appsdk/sdk_test.go | 32 ++++++++++-------------------- 2 files changed, 10 insertions(+), 71 deletions(-) diff --git a/appsdk/sdk.go b/appsdk/sdk.go index f0904099e..829fc7905 100644 --- a/appsdk/sdk.go +++ b/appsdk/sdk.go @@ -21,11 +21,9 @@ import ( "errors" "fmt" nethttp "net/http" - "net/url" "os" "os/signal" "reflect" - "strconv" "strings" "sync" "syscall" @@ -63,13 +61,8 @@ import ( const ( // ProfileSuffixPlaceholder is used to create unique names for profiles ProfileSuffixPlaceholder = "" - envV1Profile = "edgex_profile" // TODO: Remove for release v2.0.0 envProfile = "EDGEX_PROFILE" envServiceKey = "EDGEX_SERVICE_KEY" - envV1Service = "edgex_service" // deprecated TODO: Remove for release v2.0.0 - envServiceProtocol = "Service_Protocol" // Used for envV1Service processing TODO: Remove for release v2.0.0 - envServiceHost = "Service_Host" // Used for envV1Service processing TODO: Remove for release v2.0.0 - envServicePort = "Service_Port" // Used for envV1Service processing TODO: Remove for release v2.0.0 bindingTypeMessageBus = "MESSAGEBUS" bindingTypeEdgeXMessageBus = "EDGEX-MESSAGEBUS" @@ -377,14 +370,6 @@ func (sdk *AppFunctionsSDK) Initialize() error { sdk.LoggingClient.Info(fmt.Sprintf("Starting %s %s ", sdk.ServiceKey, internal.ApplicationVersion)) - // The use of the edgex_service environment variable (only used for App Services) has been deprecated - // and not included in the common bootstrap. Have to be handle here before calling into the common bootstrap - // so proper overrides are set. - // TODO: Remove for release v2.0.0 - if err := sdk.handleEdgexService(); err != nil { - return err - } - sdk.config = &common.ConfigurationStruct{} dic := di.NewContainer(di.ServiceConstructorMap{ container.ConfigurationName: func(get di.Get) interface{} { @@ -516,11 +501,6 @@ func (sdk *AppFunctionsSDK) setServiceKey(profile string) { // Have to handle environment override here before common bootstrap is used so it is passed the proper service key profileOverride := os.Getenv(envProfile) - if len(profileOverride) == 0 { - // V2 not set so try V1 - profileOverride = os.Getenv(envV1Profile) // TODO: Remove for release v2.0.0: - } - if len(profileOverride) > 0 { profile = profileOverride } @@ -533,32 +513,3 @@ func (sdk *AppFunctionsSDK) setServiceKey(profile string) { // No profile specified so remove the placeholder text sdk.ServiceKey = strings.Replace(sdk.ServiceKey, ProfileSuffixPlaceholder, "", 1) } - -// handleEdgexService checks to see if the "edgex_service" environment variable is set and if so creates appropriate config -// overrides from the URL parts. -// TODO: Remove for release v2.0.0 -func (sdk *AppFunctionsSDK) handleEdgexService() error { - if envValue := os.Getenv(envV1Service); envValue != "" { - u, err := url.Parse(envValue) - if err != nil { - return fmt.Errorf( - "failed to parse 'edgex_service' environment value '%s' as a URL: %s", - envValue, - err.Error()) - } - - _, err = strconv.ParseInt(u.Port(), 10, 0) - if err != nil { - return fmt.Errorf( - "failed to parse port from 'edgex_service' environment value '%s' as an integer: %s", - envValue, - err.Error()) - } - - os.Setenv(envServiceProtocol, u.Scheme) - os.Setenv(envServiceHost, u.Hostname()) - os.Setenv(envServicePort, u.Port()) - } - - return nil -} diff --git a/appsdk/sdk_test.go b/appsdk/sdk_test.go index a78fd729a..f03c2a28d 100644 --- a/appsdk/sdk_test.go +++ b/appsdk/sdk_test.go @@ -23,17 +23,18 @@ import ( "reflect" "testing" - "github.com/gorilla/mux" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/edgexfoundry/app-functions-sdk-go/v2/appcontext" "github.com/edgexfoundry/app-functions-sdk-go/v2/internal/common" "github.com/edgexfoundry/app-functions-sdk-go/v2/internal/runtime" triggerHttp "github.com/edgexfoundry/app-functions-sdk-go/v2/internal/trigger/http" "github.com/edgexfoundry/app-functions-sdk-go/v2/internal/trigger/messagebus" "github.com/edgexfoundry/app-functions-sdk-go/v2/internal/webserver" + "github.com/edgexfoundry/go-mod-core-contracts/v2/clients/logger" + + "github.com/gorilla/mux" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var lc logger.LoggingClient @@ -76,7 +77,7 @@ func TestAddBackgroundPublisher(t *testing.T) { } require.NotNil(t, pub.output, "publisher should have an output channel set") - require.NotNil(t, sdk.backgroundChannel, "sdk should have a background channel set for passing to trigger intitialization") + require.NotNil(t, sdk.backgroundChannel, "sdk should have a background channel set for passing to trigger initialization") // compare addresses since types will not match assert.Equal(t, fmt.Sprintf("%p", sdk.backgroundChannel), fmt.Sprintf("%p", pub.output), @@ -367,14 +368,6 @@ func TestSetServiceKey(t *testing.T) { originalServiceKey: "MyAppService-" + ProfileSuffixPlaceholder, expectedServiceKey: "MyAppService-mqtt-export", }, - { - name: "Profile specified with V1 override", - profile: "rules-engine", - profileEnvVar: envV1Profile, - profileEnvValue: "rules-engine-mqtt", - originalServiceKey: "MyAppService-" + ProfileSuffixPlaceholder, - expectedServiceKey: "MyAppService-rules-engine-mqtt", - }, { name: "Profile specified with V2 override", profile: "rules-engine", @@ -383,13 +376,6 @@ func TestSetServiceKey(t *testing.T) { originalServiceKey: "MyAppService-" + ProfileSuffixPlaceholder, expectedServiceKey: "MyAppService-rules-engine-redis", }, - { - name: "No profile specified with V1 override", - profileEnvVar: envV1Profile, - profileEnvValue: "sample", - originalServiceKey: "MyAppService-" + ProfileSuffixPlaceholder, - expectedServiceKey: "MyAppService-sample", - }, { name: "No profile specified with V2 override", profileEnvVar: envProfile, @@ -444,10 +430,12 @@ func TestSetServiceKey(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { if len(test.profileEnvVar) > 0 && len(test.profileEnvValue) > 0 { - os.Setenv(test.profileEnvVar, test.profileEnvValue) + err := os.Setenv(test.profileEnvVar, test.profileEnvValue) + require.NoError(t, err) } if len(test.serviceKeyEnvValue) > 0 { - os.Setenv(envServiceKey, test.serviceKeyEnvValue) + err := os.Setenv(envServiceKey, test.serviceKeyEnvValue) + require.NoError(t, err) } defer os.Clearenv()