test(configuration): add some additional coverage (#5485)

Signed-off-by: James Elliott <james-d-elliott@users.noreply.github.com>

internal/configuration/decode_hooks.go

@@ -634,11 +634,11 @@ func StringToPasswordDigestHookFunc() mapstructure.DecodeHookFuncType {
 
 		var result *schema.PasswordDigest
 
+		if dataStr != "" {
 			if !strings.HasPrefix(dataStr, "$") {
 				dataStr = fmt.Sprintf(plaintext.EncodingFmt, plaintext.AlgIdentifierPlainText, dataStr)
 			}
 
-		if dataStr != "" {
 			if result, err = schema.DecodePasswordDigest(dataStr); err != nil {
 				return nil, fmt.Errorf(errFmtDecodeHookCouldNotParse, dataStr, prefixType, expectedType.String(), err)
 			}

internal/configuration/decode_hooks_test.go

@@ -4,8 +4,10 @@ import (
 	"bytes"
 	"crypto/ecdsa"
 	"crypto/rsa"
+	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
+	"fmt"
 	"math"
 	"net/mail"
 	"net/url"
@@ -786,24 +788,6 @@ func TestStringToRegexpFuncPointers(t *testing.T) {
 }
 
 func TestStringToAddressHookFunc(t *testing.T) {
-	mustParseAddress := func(a string) (addr schema.Address) {
-		addrs, err := schema.NewAddress(a)
-		if err != nil {
-			panic(err)
-		}
-
-		return *addrs
-	}
-
-	mustParseAddressPtr := func(a string) (addr *schema.Address) {
-		addr, err := schema.NewAddress(a)
-		if err != nil {
-			panic(err)
-		}
-
-		return addr
-	}
-
 	testCases := []struct {
 		name     string
 		have     any
@@ -814,13 +798,13 @@ func TestStringToAddressHookFunc(t *testing.T) {
 		{
 			name:     "ShouldDecodeNonPtr",
 			have:     "tcp://0.0.0.0:2020",
-			expected: mustParseAddress("tcp://0.0.0.0:2020"),
+			expected: MustParseAddress("tcp://0.0.0.0:2020"),
 			decode:   true,
 		},
 		{
 			name:     "ShouldDecodePtr",
 			have:     "tcp://0.0.0.0:2020",
-			expected: mustParseAddressPtr("tcp://0.0.0.0:2020"),
+			expected: MustParseAddressPtr("tcp://0.0.0.0:2020"),
 			decode:   true,
 		},
 		{
@@ -872,6 +856,90 @@ func TestStringToAddressHookFunc(t *testing.T) {
 			err:      "could not decode 'tcp://&!@^#*&!@#&*@!:2020' to a schema.Address: could not parse string 'tcp://&!@^#*&!@#&*@!:2020' as address: expected format is [<scheme>://]<hostname>[:<port>]: parse \"tcp://&!@^\": invalid character \"^\" in host name",
 			decode:   false,
 		},
+		{
+			name:     "ShouldDecodeTCP",
+			have:     "tcp://127.0.0.1",
+			expected: schema.AddressTCP{Address: MustParseAddress("tcp://127.0.0.1")},
+			err:      "",
+			decode:   true,
+		},
+		{
+			name:     "ShouldDecodeTCPPtr",
+			have:     "tcp://127.0.0.1",
+			expected: &schema.AddressTCP{Address: MustParseAddress("tcp://127.0.0.1")},
+			err:      "",
+			decode:   true,
+		},
+		{
+			name:     "ShouldDecodeUDP",
+			have:     "udp://127.0.0.1",
+			expected: schema.AddressUDP{Address: MustParseAddress("udp://127.0.0.1")},
+			err:      "",
+			decode:   true,
+		},
+		{
+			name:     "ShouldDecodeUDPPtr",
+			have:     "udp://127.0.0.1",
+			expected: &schema.AddressUDP{Address: MustParseAddress("udp://127.0.0.1")},
+			err:      "",
+			decode:   true,
+		},
+		{
+			name:     "ShouldDecodeLDAP",
+			have:     "ldap://127.0.0.1",
+			expected: schema.AddressLDAP{Address: MustParseAddress("ldap://127.0.0.1")},
+			err:      "",
+			decode:   true,
+		},
+		{
+			name:     "ShouldDecodeLDAPPtr",
+			have:     "ldap://127.0.0.1",
+			expected: &schema.AddressLDAP{Address: MustParseAddress("ldap://127.0.0.1")},
+			err:      "",
+			decode:   true,
+		},
+		{
+			name:     "ShouldDecodeSMTP",
+			have:     "smtp://127.0.0.1",
+			expected: schema.AddressSMTP{Address: MustParseAddress("smtp://127.0.0.1")},
+			err:      "",
+			decode:   true,
+		},
+		{
+			name:     "ShouldDecodeSMTPPtr",
+			have:     "smtp://127.0.0.1",
+			expected: &schema.AddressSMTP{Address: MustParseAddress("smtp://127.0.0.1")},
+			err:      "",
+			decode:   true,
+		},
+		{
+			name:     "ShouldFailDecodeTCP",
+			have:     "@@@@@@@",
+			expected: schema.AddressTCP{Address: MustParseAddress("tcp://127.0.0.1")},
+			err:      "could not decode '@@@@@@@' to a schema.AddressTCP: error validating the address: the url 'tcp://%40%40%40%40%40%40@' appears to have user info but this is not valid for addresses",
+			decode:   false,
+		},
+		{
+			name:     "ShouldFailDecodeUDP",
+			have:     "@@@@@@@",
+			expected: schema.AddressUDP{Address: MustParseAddress("udp://127.0.0.1")},
+			err:      "could not decode '@@@@@@@' to a schema.AddressUDP: error validating the address: the url 'udp://%40%40%40%40%40%40@' appears to have user info but this is not valid for addresses",
+			decode:   false,
+		},
+		{
+			name:     "ShouldFailDecodeLDAP",
+			have:     "@@@@@@@",
+			expected: schema.AddressLDAP{Address: MustParseAddress("ldap://127.0.0.1")},
+			err:      "could not decode '@@@@@@@' to a schema.AddressLDAP: error validating the address: the url 'ldaps://%40%40%40%40%40%40@' appears to have user info but this is not valid for addresses",
+			decode:   false,
+		},
+		{
+			name:     "ShouldFailDecodeSMTP",
+			have:     "@@@@@@@",
+			expected: schema.AddressSMTP{Address: MustParseAddress("smtp://127.0.0.1")},
+			err:      "could not decode '@@@@@@@' to a schema.AddressSMTP: error validating the address: the url 'smtp://%40%40%40%40%40%40@' appears to have user info but this is not valid for addresses",
+			decode:   false,
+		},
 	}
 
 	hook := configuration.StringToAddressHookFunc()
@@ -1095,6 +1163,160 @@ func TestStringToX509CertificateHookFunc(t *testing.T) {
 	}
 }
 
+func TestStringToPasswordDigestHookFunc(t *testing.T) {
+	var nilvalue *schema.PasswordDigest
+
+	testCases := []struct {
+		name     string
+		have     any
+		expected any
+		err      string
+		decode   bool
+	}{
+		{
+			"ShouldParse",
+			"$plaintext$example",
+			MustParsePasswordDigest("$plaintext$example"),
+			"",
+			true,
+		},
+		{
+			"ShouldParsePtr",
+			"$plaintext$example",
+			MustParsePasswordDigestPtr("$plaintext$example"),
+			"",
+			true,
+		},
+		{
+			"ShouldNotParseUnknown",
+			"$abc$example",
+			schema.PasswordDigest{},
+			"could not decode '$abc$example' to a schema.PasswordDigest: provided encoded hash has an invalid identifier: the identifier 'abc' is unknown to the decoder",
+			false,
+		},
+		{
+			"ShouldNotParseWrongType",
+			"$abc$example",
+			schema.TLSVersion{},
+			"",
+			false,
+		},
+		{
+			"ShouldNotParseWrongTypePtr",
+			"$abc$example",
+			&schema.TLSVersion{},
+			"",
+			false,
+		},
+		{
+			"ShouldNotParseEmptyString",
+			"",
+			schema.PasswordDigest{},
+			"could not decode an empty value to a schema.PasswordDigest: must have a non-empty value",
+			false,
+		},
+		{
+			"ShouldParseEmptyStringPtr",
+			"",
+			nilvalue,
+			"",
+			true,
+		},
+	}
+
+	hook := configuration.StringToPasswordDigestHookFunc()
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual, err := hook(reflect.TypeOf(tc.have), reflect.TypeOf(tc.expected), tc.have)
+			if tc.err != "" {
+				assert.EqualError(t, err, tc.err)
+
+				if !tc.decode {
+					assert.Nil(t, actual)
+				}
+			} else {
+				assert.NoError(t, err)
+
+				if tc.decode {
+					assert.Equal(t, tc.expected, actual)
+				} else {
+					assert.Equal(t, tc.have, actual)
+				}
+			}
+		})
+	}
+}
+
+func TestStringToTLSVersionHookFunc(t *testing.T) {
+	testCases := []struct {
+		name     string
+		have     any
+		expected any
+		err      string
+		decode   bool
+	}{
+		{
+			"ShouldParseTLS1.3",
+			"TLS1.3",
+			schema.TLSVersion{Value: tls.VersionTLS13},
+			"",
+			true,
+		},
+		{
+			"ShouldParseTLS1.3PTR",
+			"TLS1.3",
+			&schema.TLSVersion{Value: tls.VersionTLS13},
+			"",
+			true,
+		},
+		{
+			"ShouldParseTLS1.2",
+			"TLS1.2",
+			schema.TLSVersion{Value: tls.VersionTLS12},
+			"",
+			true,
+		},
+		{
+			"ShouldNotParseInt",
+			1,
+			&schema.TLSVersion{},
+			"",
+			false,
+		},
+		{
+			"ShouldNotParseNonVersion",
+			"1",
+			&schema.TLSVersion{},
+			"could not decode '1' to a *schema.TLSVersion: supplied tls version isn't supported",
+			false,
+		},
+	}
+
+	hook := configuration.StringToTLSVersionHookFunc()
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual, err := hook(reflect.TypeOf(tc.have), reflect.TypeOf(tc.expected), tc.have)
+			if tc.err != "" {
+				assert.EqualError(t, err, tc.err)
+
+				if !tc.decode {
+					assert.Nil(t, actual)
+				}
+			} else {
+				assert.NoError(t, err)
+
+				if tc.decode {
+					assert.Equal(t, tc.expected, actual)
+				} else {
+					assert.Equal(t, tc.have, actual)
+				}
+			}
+		})
+	}
+}
+
 func TestStringToX509CertificateChainHookFunc(t *testing.T) {
 	var nilkey *schema.X509CertificateChain
 
@@ -1411,3 +1633,42 @@ var (
 	testZero   int32
 	testString = ""
 )
+
+func MustParseAddress(input string) schema.Address {
+	address, err := schema.NewAddress(input)
+	if err != nil {
+		panic(err)
+	}
+
+	fmt.Println(address.String())
+	addr := *address
+
+	return addr
+}
+
+func MustParseAddressPtr(input string) *schema.Address {
+	address, err := schema.NewAddress(input)
+	if err != nil {
+		panic(err)
+	}
+
+	return address
+}
+
+func MustParsePasswordDigest(input string) schema.PasswordDigest {
+	digest, err := schema.DecodePasswordDigest(input)
+	if err != nil {
+		panic(err)
+	}
+
+	return *digest
+}
+
+func MustParsePasswordDigestPtr(input string) *schema.PasswordDigest {
+	digest, err := schema.DecodePasswordDigest(input)
+	if err != nil {
+		panic(err)
+	}
+
+	return digest
+}

internal/configuration/koanf_callbacks_test.go

@@ -6,6 +6,7 @@ import (
 	"runtime"
 	"testing"
 
+	"github.com/spf13/pflag"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -125,3 +126,85 @@ func TestKoanfSecretCallbackShouldErrorOnFSError(t *testing.T) {
 	assert.Len(t, val.Warnings(), 0)
 	assert.EqualError(t, val.Errors()[0], fmt.Sprintf("secrets: error loading secret path %s into key 'theme': file permission error occurred: open %s: permission denied", secret, secret))
 }
+
+func TestKoanfCommandLineWithMappingCallback(t *testing.T) {
+	testCases := []struct {
+		name          string
+		have          []string
+		flagName      string
+		flagValue     string
+		mapped        string
+		valid         bool
+		unchanged     bool
+		expectedName  string
+		expectedValue any
+	}{
+		{
+			"ShouldDecodeStandard",
+			[]string{"--commands", "abc"},
+			"commands",
+			"",
+			"command.another",
+			false,
+			false,
+			"command.another",
+			"abc",
+		},
+		{
+			"ShouldSkipUnchangedKey",
+			[]string{},
+			"commands",
+			"abc",
+			"command.another",
+			false,
+			false,
+			"",
+			nil,
+		},
+		{
+			"ShouldLookupNormalizedKey",
+			[]string{"--log.file-path", "abc"},
+			"log.file-path",
+			"",
+			"",
+			true,
+			false,
+			"log.file_path",
+			"abc",
+		},
+		{
+			"ShouldReturnUnmodified",
+			[]string{"--commands", "abc"},
+			"commands",
+			"",
+			"",
+			false,
+			false,
+			"",
+			nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			flagset := pflag.NewFlagSet("test", pflag.ContinueOnError)
+
+			flagset.String(tc.flagName, tc.flagValue, "")
+
+			assert.NoError(t, flagset.Parse(tc.have))
+
+			mapper := map[string]string{}
+
+			if tc.mapped != "" {
+				mapper[tc.flagName] = tc.mapped
+			}
+
+			callback := koanfCommandLineWithMappingCallback(mapper, tc.valid, tc.unchanged)
+
+			actualName, actualValue := callback(flagset.Lookup(tc.flagName))
+
+			assert.Equal(t, tc.expectedName, actualName)
+			assert.Equal(t, tc.expectedValue, actualValue)
+		})
+	}
+}

internal/configuration/schema/types_test.go

@@ -268,6 +268,21 @@ func TestX509CertificateChain(t *testing.T) {
 	assert.Regexp(t, regexp.MustCompile(`^certificate #1 in chain is invalid before 13569465600 but the time is \d+$`), err.Error())
 }
 
+func TestPasswordDigest_IsPlainText(t *testing.T) {
+	digest, err := DecodePasswordDigest("$plaintext$exam")
+	assert.NoError(t, err)
+	assert.True(t, digest.IsPlainText())
+
+	digest = &PasswordDigest{}
+
+	assert.False(t, digest.IsPlainText())
+
+	digest, err = DecodePasswordDigest("$pbkdf2-sha512$310000$c8p78n7pUMln0jzvd4aK4Q$JNRBzwAo0ek5qKn50cFzzvE9RXV88h1wJn5KGiHrD0YKtZaR/nCb2CJPOsKaPK0hjf.9yHxzQGZziziccp6Yng")
+	assert.NoError(t, err)
+
+	assert.False(t, digest.IsPlainText())
+}
+
 func MustParseX509CertificateChain(data string) *X509CertificateChain {
 	chain, err := NewX509CertificateChain(data)
 	if err != nil {

internal/configuration/types.go

@@ -21,7 +21,7 @@ type FileSource struct {
 	filters []FileFilter
 }
 
-// EnvironmentSource is a configuration configuration.Source which loads values from the environment.
+// EnvironmentSource is a configuration.Source which loads values from the environment.
 type EnvironmentSource struct {
 	koanf     *koanf.Koanf
 	prefix    string