Skip to content

Commit

Permalink
refactor(sdk)!: Renamed secretName to secretValueKey and secretPath t… (
Browse files Browse the repository at this point in the history
#1337)

* refactor(sdk)!: Renamed secretName to secretValueKey and secretPath to secretName

BREAKING CHANGE: Renamed secretName to secretValueKey and secretPath to secretName

Signed-off-by: Marc-Philippe Fuller <marc-philippe.fuller@intel.com>
  • Loading branch information
marcpfuller authored Mar 6, 2023
1 parent 23a06a5 commit 4c69509
Show file tree
Hide file tree
Showing 13 changed files with 144 additions and 170 deletions.
48 changes: 24 additions & 24 deletions internal/app/configurable.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ const (
BatchThreshold = "batchthreshold"
TimeInterval = "timeinterval"
HeaderName = "headername"
SecretPath = "secretpath"
SecretName = "secretname"
SecretValueKey = "secretvaluekey"
BrokerAddress = "brokeraddress"
ClientID = "clientid"
KeepAlive = "keepalive"
Expand Down Expand Up @@ -292,35 +292,35 @@ func (app *Configurable) Encrypt(parameters map[string]string) interfaces.AppFun
return nil
}

secretPath := parameters[SecretPath]
secretName := parameters[SecretName]
secretValueKey := parameters[SecretValueKey]
encryptionKey := parameters[EncryptionKey]

// SecretPath & SecretName are optional if EncryptionKey specified
// EncryptionKey is optional if SecretPath & SecretName are specified
// SecretName & SecretValueKey are optional if EncryptionKey specified
// EncryptionKey is optional if SecretName & SecretValueKey are specified

// If EncryptionKey not specified, then SecretPath & SecretName must be specified
if len(encryptionKey) == 0 && (len(secretPath) == 0 || len(secretName) == 0) {
app.lc.Errorf("Could not find '%s' or '%s' and '%s' in configuration", EncryptionKey, SecretPath, SecretName)
// If EncryptionKey not specified, then SecretName & SecretValueKey must be specified
if len(encryptionKey) == 0 && (len(secretName) == 0 || len(secretValueKey) == 0) {
app.lc.Errorf("Could not find '%s' or '%s' and '%s' in configuration", EncryptionKey, SecretName, SecretValueKey)
return nil
}

// SecretPath & SecretName both must be specified it one of them is.
if (len(secretPath) != 0 && len(secretName) == 0) || (len(secretPath) == 0 && len(secretName) != 0) {
app.lc.Errorf("'%s' and '%s' both must be set in configuration", SecretPath, SecretName)
// SecretName & SecretValueKey both must be specified it one of them is.
if (len(secretName) != 0 && len(secretValueKey) == 0) || (len(secretName) == 0 && len(secretValueKey) != 0) {
app.lc.Errorf("'%s' and '%s' both must be set in configuration", SecretName, SecretValueKey)
return nil
}

switch strings.ToLower(algorithm) {
case EncryptAES256:
if len(secretPath) > 0 && len(secretName) > 0 {
if len(secretName) > 0 && len(secretValueKey) > 0 {
protector := transforms.AESProtection{
SecretPath: secretPath,
SecretName: secretName,
SecretName: secretName,
SecretValueKey: secretValueKey,
}
return protector.Encrypt
}
app.lc.Error("secretPath / secretKey are required for AES 256 encryption")
app.lc.Error("secretName / secretValueKey are required for AES 256 encryption")
return nil
default:
app.lc.Errorf(
Expand Down Expand Up @@ -380,9 +380,9 @@ func (app *Configurable) MQTTExport(parameters map[string]string) interfaces.App
return nil
}

secretPath, ok := parameters[SecretPath]
secretName, ok := parameters[SecretName]
if !ok {
app.lc.Error("Could not find " + SecretPath)
app.lc.Error("Could not find " + SecretName)
return nil
}
authMode, ok := parameters[AuthMode]
Expand Down Expand Up @@ -441,7 +441,7 @@ func (app *Configurable) MQTTExport(parameters map[string]string) interfaces.App
QoS: byte(qos),
BrokerAddress: brokerAddress,
ClientId: clientID,
SecretPath: secretPath,
SecretName: secretName,
Topic: topic,
AuthMode: authMode,
}
Expand Down Expand Up @@ -734,20 +734,20 @@ func (app *Configurable) processHttpExportParameters(
result.URL = strings.TrimSpace(result.URL)
result.MimeType = strings.TrimSpace(result.MimeType)
result.HTTPHeaderName = strings.TrimSpace(parameters[HeaderName])
result.SecretPath = strings.TrimSpace(parameters[SecretPath])
result.SecretName = strings.TrimSpace(parameters[SecretName])
result.SecretValueKey = strings.TrimSpace(parameters[SecretValueKey])

if len(result.HTTPHeaderName) == 0 && len(result.SecretPath) != 0 && len(result.SecretName) != 0 {
if len(result.HTTPHeaderName) == 0 && len(result.SecretName) != 0 && len(result.SecretValueKey) != 0 {
return result, "",
fmt.Errorf("HTTPExport missing %s since %s & %s are specified", HeaderName, SecretPath, SecretName)
fmt.Errorf("HTTPExport missing %s since %s & %s are specified", HeaderName, SecretName, SecretValueKey)
}
if len(result.SecretPath) == 0 && len(result.HTTPHeaderName) != 0 && len(result.SecretName) != 0 {
if len(result.SecretName) == 0 && len(result.HTTPHeaderName) != 0 && len(result.SecretValueKey) != 0 {
return result, "",
fmt.Errorf("HTTPExport missing %s since %s & %s are specified", SecretPath, HeaderName, SecretName)
fmt.Errorf("HTTPExport missing %s since %s & %s are specified", SecretName, HeaderName, SecretValueKey)
}
if len(result.SecretName) == 0 && len(result.SecretPath) != 0 && len(result.HTTPHeaderName) != 0 {
if len(result.SecretValueKey) == 0 && len(result.SecretName) != 0 && len(result.HTTPHeaderName) != 0 {
return result, "",
fmt.Errorf("HTTPExport missing %s since %s & %s are specified", SecretName, SecretPath, HeaderName)
fmt.Errorf("HTTPExport missing %s since %s & %s are specified", SecretValueKey, SecretName, HeaderName)
}

return result, method, nil
Expand Down
52 changes: 26 additions & 26 deletions internal/app/configurable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ func TestHTTPExport(t *testing.T) {
testBadReturnInputData := "bogus"

testHeaderName := "My-Header"
testSecretPath := "/path"
testSecretName := "header"
testSecretName := "my-secret"
testSecretValueKey := "header"

tests := []struct {
Name string
Expand All @@ -178,32 +178,32 @@ func TestHTTPExport(t *testing.T) {
ContinueOnSendError *string
ReturnInputData *string
HeaderName *string
SecretPath *string
SecretName *string
SecretValueKey *string
ExpectValid bool
}{
{"Valid Post - ony required params", ExportMethodPost, &testUrl, &testMimeType, nil, nil, nil, nil, nil, nil, true},
{"Valid Post - w/o secrets", http.MethodPost, &testUrl, &testMimeType, &testPersistOnError, nil, nil, nil, nil, nil, true},
{"Valid Post - with secrets", ExportMethodPost, &testUrl, &testMimeType, nil, nil, nil, &testHeaderName, &testSecretPath, &testSecretName, true},
{"Valid Post - with all params", ExportMethodPost, &testUrl, &testMimeType, &testPersistOnError, &testContinueOnSendError, &testReturnInputData, &testHeaderName, &testSecretPath, &testSecretName, true},
{"Valid Post - with secrets", ExportMethodPost, &testUrl, &testMimeType, nil, nil, nil, &testHeaderName, &testSecretName, &testSecretValueKey, true},
{"Valid Post - with all params", ExportMethodPost, &testUrl, &testMimeType, &testPersistOnError, &testContinueOnSendError, &testReturnInputData, &testHeaderName, &testSecretName, &testSecretValueKey, true},
{"Invalid Post - no url", ExportMethodPost, nil, &testMimeType, nil, nil, nil, nil, nil, nil, false},
{"Invalid Post - no mimeType", ExportMethodPost, &testUrl, nil, nil, nil, nil, nil, nil, nil, false},
{"Invalid Post - bad persistOnError", ExportMethodPost, &testUrl, &testMimeType, &testBadPersistOnError, nil, nil, nil, nil, nil, false},
{"Invalid Post - missing headerName", ExportMethodPost, &testUrl, &testMimeType, &testPersistOnError, nil, nil, nil, &testSecretPath, &testSecretName, false},
{"Invalid Post - missing secretPath", ExportMethodPost, &testUrl, &testMimeType, &testPersistOnError, nil, nil, &testHeaderName, nil, &testSecretName, false},
{"Invalid Post - missing secretName", ExportMethodPost, &testUrl, &testMimeType, &testPersistOnError, nil, nil, &testHeaderName, &testSecretPath, nil, false},
{"Invalid Post - missing headerName", ExportMethodPost, &testUrl, &testMimeType, &testPersistOnError, nil, nil, nil, &testSecretName, &testSecretValueKey, false},
{"Invalid Post - missing secretName", ExportMethodPost, &testUrl, &testMimeType, &testPersistOnError, nil, nil, &testHeaderName, nil, &testSecretValueKey, false},
{"Invalid Post - missing secretValueKey", ExportMethodPost, &testUrl, &testMimeType, &testPersistOnError, nil, nil, &testHeaderName, &testSecretName, nil, false},
{"Valid Put - ony required params", ExportMethodPut, &testUrl, &testMimeType, nil, nil, nil, nil, nil, nil, true},
{"Valid Put - w/o secrets", ExportMethodPut, &testUrl, &testMimeType, &testPersistOnError, nil, nil, nil, nil, nil, true},
{"Valid Put - with secrets", http.MethodPut, &testUrl, &testMimeType, nil, nil, nil, &testHeaderName, &testSecretPath, &testSecretName, true},
{"Valid Put - with all params", ExportMethodPut, &testUrl, &testMimeType, &testPersistOnError, nil, nil, &testHeaderName, &testSecretPath, &testSecretName, true},
{"Valid Put - with secrets", http.MethodPut, &testUrl, &testMimeType, nil, nil, nil, &testHeaderName, &testSecretName, &testSecretValueKey, true},
{"Valid Put - with all params", ExportMethodPut, &testUrl, &testMimeType, &testPersistOnError, nil, nil, &testHeaderName, &testSecretName, &testSecretValueKey, true},
{"Invalid Put - no url", ExportMethodPut, nil, &testMimeType, nil, nil, nil, nil, nil, nil, false},
{"Invalid Put - no mimeType", ExportMethodPut, &testUrl, nil, nil, nil, nil, nil, nil, nil, false},
{"Invalid Put - bad persistOnError", ExportMethodPut, &testUrl, &testMimeType, &testBadPersistOnError, nil, nil, nil, nil, nil, false},
{"Invalid Put - bad continueOnSendError", ExportMethodPut, &testUrl, &testMimeType, nil, &testBadContinueOnSendError, nil, nil, nil, nil, false},
{"Invalid Put - bad returnInputData", ExportMethodPut, &testUrl, &testMimeType, nil, nil, &testBadReturnInputData, nil, nil, nil, false},
{"Invalid Put - missing headerName", ExportMethodPut, &testUrl, &testMimeType, &testPersistOnError, nil, nil, nil, &testSecretPath, &testSecretName, false},
{"Invalid Put - missing secretPath", ExportMethodPut, &testUrl, &testMimeType, &testPersistOnError, nil, nil, &testHeaderName, nil, &testSecretName, false},
{"Invalid Put - missing secretName", ExportMethodPut, &testUrl, &testMimeType, &testPersistOnError, nil, nil, &testHeaderName, &testSecretPath, nil, false},
{"Invalid Put - missing headerName", ExportMethodPut, &testUrl, &testMimeType, &testPersistOnError, nil, nil, nil, &testSecretName, &testSecretValueKey, false},
{"Invalid Put - missing secretName", ExportMethodPut, &testUrl, &testMimeType, &testPersistOnError, nil, nil, &testHeaderName, nil, &testSecretValueKey, false},
{"Invalid Put - missing secretValueKey", ExportMethodPut, &testUrl, &testMimeType, &testPersistOnError, nil, nil, &testHeaderName, &testSecretName, nil, false},
}

for _, test := range tests {
Expand Down Expand Up @@ -235,14 +235,14 @@ func TestHTTPExport(t *testing.T) {
params[HeaderName] = *test.HeaderName
}

if test.SecretPath != nil {
params[SecretPath] = *test.SecretPath
}

if test.SecretName != nil {
params[SecretName] = *test.SecretName
}

if test.SecretValueKey != nil {
params[SecretValueKey] = *test.SecretValueKey
}

transform := configurable.HTTPExport(params)
assert.Equal(t, test.ExpectValid, transform != nil)
})
Expand Down Expand Up @@ -327,7 +327,7 @@ func TestMQTTExport(t *testing.T) {
params := make(map[string]string)
params[BrokerAddress] = "mqtt://broker:8883"
params[Topic] = "topic"
params[SecretPath] = "/path"
params[SecretName] = "my-secret"
params[ClientID] = "clientid"
params[Qos] = "0"
params[Retain] = "true"
Expand Down Expand Up @@ -375,11 +375,11 @@ func TestEncrypt(t *testing.T) {
configurable := Configurable{lc: lc}

tests := []struct {
Name string
Algorithm string
SecretPath string
SecretName string
ExpectNil bool
Name string
Algorithm string
SecretName string
SecretValueKey string
ExpectNil bool
}{
{"AES256 - Bad - No secrets ", EncryptAES256, "", "", true},
{"AES256 - good - secrets", EncryptAES256, uuid.NewString(), uuid.NewString(), false},
Expand All @@ -391,12 +391,12 @@ func TestEncrypt(t *testing.T) {
if len(testCase.Algorithm) > 0 {
params[Algorithm] = testCase.Algorithm
}
if len(testCase.SecretPath) > 0 {
params[SecretPath] = testCase.SecretPath
}
if len(testCase.SecretName) > 0 {
params[SecretName] = testCase.SecretName
}
if len(testCase.SecretValueKey) > 0 {
params[SecretValueKey] = testCase.SecretValueKey
}

transform := configurable.Encrypt(params)
assert.Equal(t, testCase.ExpectNil, transform == nil)
Expand Down
6 changes: 3 additions & 3 deletions internal/common/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,10 @@ type ExternalMqttConfig struct {
Retain bool
// SkipCertVerify indicates if the certificate verification should be skipped
SkipCertVerify bool
// SecretPath is the name of the path in secret provider to retrieve your secrets
SecretPath string
// SecretName is the name of the secret in secret provider to retrieve your secrets
SecretName string
// AuthMode indicates what to use when connecting to the broker. Options are "none", "cacert" , "usernamepassword", "clientcert".
// If a CA Cert exists in the SecretPath then it will be used for all modes except "none".
// If a CA Cert exists in the secret data for the SecretName then it will be used for all modes except "none".
AuthMode string
// RetryDuration indicates how long (in seconds) to wait timing out on the MQTT client creation
RetryDuration int
Expand Down
8 changes: 4 additions & 4 deletions internal/controller/rest/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ func (c *Controller) AddSecret(writer http.ResponseWriter, request *http.Request
return
}

path, secret := c.prepareSecret(secretRequest)
secretName, secret := c.prepareSecret(secretRequest)

if err := c.secretProvider.StoreSecret(path, secret); err != nil {
if err := c.secretProvider.StoreSecret(secretName, secret); err != nil {
c.sendError(writer, request, errors.KindServerError, "Storing secret failed", err, secretRequest.RequestId)
return
}
Expand Down Expand Up @@ -175,7 +175,7 @@ func (c *Controller) prepareSecret(request commonDtos.SecretRequest) (string, ma
secretsKV[secret.Key] = secret.Value
}

path := strings.TrimSpace(request.SecretName)
secretName := strings.TrimSpace(request.SecretName)

return path, secretsKV
return secretName, secretsKV
}
2 changes: 1 addition & 1 deletion internal/trigger/mqtt/mqtt.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func createMqttClient(sp messaging.SecretDataProvider, lc logger.LoggingClient,
sp,
lc,
config.AuthMode,
config.SecretPath,
config.SecretName,
config.SkipCertVerify,
)
mqttClient, err := mqttFactory.Create(opts)
Expand Down
29 changes: 0 additions & 29 deletions internal/webserver/types.go

This file was deleted.

13 changes: 7 additions & 6 deletions pkg/secure/mqttfactory.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import (
"crypto/tls"
"crypto/x509"
"errors"
"github.com/eclipse/paho.mqtt.golang"

mqtt "github.com/eclipse/paho.mqtt.golang"
"github.com/edgexfoundry/go-mod-bootstrap/v3/bootstrap/messaging"
"github.com/edgexfoundry/go-mod-core-contracts/v3/clients/logger"
)
Expand All @@ -29,17 +30,17 @@ type MqttFactory struct {
sp messaging.SecretDataProvider
logger logger.LoggingClient
authMode string
secretPath string
secretName string
opts *mqtt.ClientOptions
skipCertVerify bool
}

func NewMqttFactory(sp messaging.SecretDataProvider, log logger.LoggingClient, mode string, path string, skipVerify bool) MqttFactory {
func NewMqttFactory(sp messaging.SecretDataProvider, log logger.LoggingClient, mode string, secretName string, skipVerify bool) MqttFactory {
return MqttFactory{
sp: sp,
logger: log,
authMode: mode,
secretPath: path,
secretName: secretName,
skipCertVerify: skipVerify,
}
}
Expand All @@ -53,13 +54,13 @@ func (factory MqttFactory) Create(opts *mqtt.ClientOptions) (mqtt.Client, error)
factory.opts = opts

//get the secrets from the secret provider and populate the struct
secretData, err := messaging.GetSecretData(factory.authMode, factory.secretPath, factory.sp)
secretData, err := messaging.GetSecretData(factory.authMode, factory.secretName, factory.sp)
if err != nil {
return nil, err
}
//ensure that the authmode selected has the required secret values
if secretData != nil {
err = messaging.ValidateSecretData(factory.authMode, factory.secretPath, secretData)
err = messaging.ValidateSecretData(factory.authMode, factory.secretName, secretData)
if err != nil {
return nil, err
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/secure/mqttfactory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@
package secure

import (
mqtt "github.com/eclipse/paho.mqtt.golang"
"os"
"testing"

mqtt "github.com/eclipse/paho.mqtt.golang"

bootstrapContainer "github.com/edgexfoundry/go-mod-bootstrap/v3/bootstrap/container"
"github.com/edgexfoundry/go-mod-bootstrap/v3/bootstrap/messaging"
"github.com/edgexfoundry/go-mod-bootstrap/v3/di"
Expand Down Expand Up @@ -56,13 +57,13 @@ func TestMain(m *testing.M) {

func TestNewMqttFactory(t *testing.T) {
expectedMode := "none"
expectedPath := "myPath"
expectedSecretName := "mySecretName"
expectedSkipVerify := true
target := NewMqttFactory(secretDataProvider, lc, expectedMode, expectedPath, expectedSkipVerify)
target := NewMqttFactory(secretDataProvider, lc, expectedMode, expectedSecretName, expectedSkipVerify)

assert.NotNil(t, target.logger)
assert.Equal(t, expectedMode, target.authMode)
assert.Equal(t, expectedPath, target.secretPath)
assert.Equal(t, expectedSecretName, target.secretName)
assert.Equal(t, expectedSkipVerify, target.skipCertVerify)
assert.Nil(t, target.opts)

Expand Down
Loading

0 comments on commit 4c69509

Please sign in to comment.