Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Remove deprecated environment variables and related code #718

Merged
merged 2 commits into from
Feb 25, 2021
Merged

refactor: Remove deprecated environment variables and related code #718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ffc6c7
refactor: Rename MqttBroker configuration to ExternalMqtt
Feb 25, 2021
e44cd7f
refactor: Remove deprecated environment variables and related code
Feb 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 0 additions & 49 deletions appsdk/sdk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@ import (
"errors"
"fmt"
nethttp "net/http"
"net/url"
"os"
"os/signal"
"reflect"
"strconv"
"strings"
"sync"
"syscall"
Expand Down Expand Up @@ -63,13 +61,8 @@ import (
const (
// ProfileSuffixPlaceholder is used to create unique names for profiles
ProfileSuffixPlaceholder = "<profile>"
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"
Expand Down Expand Up @@ -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{} {
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
32 changes: 10 additions & 22 deletions appsdk/sdk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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",
Expand All @@ -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,
Expand Down Expand Up @@ -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()

Expand Down
6 changes: 3 additions & 3 deletions internal/common/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type ConfigurationStruct struct {
// MessageBus
MessageBus types.MessageBusConfig
// MqttBroker
MqttBroker MqttBrokerConfig
ExternalMqtt ExternalMqttConfig
// Binding
Binding BindingInfo
// ApplicationSettings
Expand Down Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions internal/trigger/mqtt/mqtt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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()
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion internal/webserver/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down