Skip to content

Commit

Permalink
refactor: Making use of validation v10 lib to simplify code (#966)
Browse files Browse the repository at this point in the history
  • Loading branch information
dadrus authored Oct 11, 2023
1 parent 3c7538e commit c5e25ac
Show file tree
Hide file tree
Showing 44 changed files with 640 additions and 639 deletions.
6 changes: 3 additions & 3 deletions internal/rules/endpoint/api_key_strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ import (
)

type APIKeyStrategy struct {
In string `mapstructure:"in"`
Name string `mapstructure:"name"`
Value string `mapstructure:"value"`
In string `mapstructure:"in" validate:"required,oneof=cookie header query"`
Name string `mapstructure:"name" validate:"required"`
Value string `mapstructure:"value" validate:"required"`
}

func (c *APIKeyStrategy) Apply(_ context.Context, req *http.Request) error {
Expand Down
4 changes: 2 additions & 2 deletions internal/rules/endpoint/basic_auth_strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import (
)

type BasicAuthStrategy struct {
User string `mapstructure:"user"`
Password string `mapstructure:"password"`
User string `mapstructure:"user" validate:"required"`
Password string `mapstructure:"password" validate:"required"`
}

func (c *BasicAuthStrategy) Apply(_ context.Context, req *http.Request) error {
Expand Down
6 changes: 3 additions & 3 deletions internal/rules/endpoint/client_credentials_strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ import (
const defaultCacheLeeway = 15

type ClientCredentialsStrategy struct {
ClientID string `mapstructure:"client_id"`
ClientSecret string `mapstructure:"client_secret"`
TokenURL string `mapstructure:"token_url" validate:"required,url"`
ClientID string `mapstructure:"client_id" validate:"required"`
ClientSecret string `mapstructure:"client_secret" validate:"required"`
Scopes []string `mapstructure:"scopes"`
TokenURL string `mapstructure:"token_url"`
}

func (c *ClientCredentialsStrategy) Apply(ctx context.Context, req *http.Request) error {
Expand Down
17 changes: 1 addition & 16 deletions internal/rules/endpoint/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import (
)

type Endpoint struct {
URL string `mapstructure:"url"`
URL string `mapstructure:"url" validate:"required,url"`
Method string `mapstructure:"method"`
Retry *Retry `mapstructure:"retry"`
AuthStrategy AuthenticationStrategy `mapstructure:"auth"`
Expand All @@ -54,21 +54,6 @@ type Retry struct {
MaxDelay time.Duration `mapstructure:"max_delay"`
}

func (e Endpoint) Validate() error {
if len(e.URL) == 0 {
return errorchain.
NewWithMessage(heimdall.ErrConfiguration, "endpoint requires url to be set")
}

// this ensures that user info, scheme and host cannot be templated
if _, err := url.Parse(e.URL); err != nil {
return errorchain.NewWithMessage(heimdall.ErrConfiguration,
"failed to parse endpoint URL").CausedBy(err)
}

return nil
}

func (e Endpoint) CreateClient(peerName string) *http.Client {
client := &http.Client{
Transport: otelhttp.NewTransport(
Expand Down
47 changes: 0 additions & 47 deletions internal/rules/endpoint/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,53 +36,6 @@ import (
"github.com/dadrus/heimdall/internal/x"
)

func TestEndpointValidate(t *testing.T) {
t.Parallel()

for _, tc := range []struct {
uc string
endpoint Endpoint
assert func(t *testing.T, err error)
}{
{
uc: "endpoint without required URL attribute",
endpoint: Endpoint{Method: "GET"},
assert: func(t *testing.T, err error) {
t.Helper()

require.Error(t, err)
assert.ErrorIs(t, err, heimdall.ErrConfiguration)
assert.Contains(t, err.Error(), "requires url")
},
},
{
uc: "endpoint with malformed URL",
endpoint: Endpoint{URL: "{{ .Values.foo }}://foo.bar"},
assert: func(t *testing.T, err error) {
t.Helper()

require.Error(t, err)
assert.ErrorIs(t, err, heimdall.ErrConfiguration)
assert.Contains(t, err.Error(), "failed to parse")
},
},
{
uc: "endpoint with required URL attribute",
endpoint: Endpoint{URL: "http://foo.bar"},
assert: func(t *testing.T, err error) {
t.Helper()

assert.NoError(t, err)
},
},
} {
t.Run("case="+tc.uc, func(t *testing.T) {
// THEN
tc.assert(t, tc.endpoint.Validate())
})
}
}

func TestEndpointCreateClient(t *testing.T) {
t.Parallel()

Expand Down
101 changes: 14 additions & 87 deletions internal/rules/endpoint/mapstructure_decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/mitchellh/mapstructure"

"github.com/dadrus/heimdall/internal/heimdall"
"github.com/dadrus/heimdall/internal/validation"
"github.com/dadrus/heimdall/internal/x/errorchain"
)

Expand Down Expand Up @@ -73,107 +74,33 @@ func DecodeAuthenticationStrategyHookFunc() mapstructure.DecodeHookFunc {

switch typed["type"] {
case "basic_auth":
return decodeBasicAuthStrategy(typed["config"])
return decodeStrategy("basic_auth", &BasicAuthStrategy{}, typed["config"])
case "api_key":
return decodeAPIKeyStrategy(typed["config"])
return decodeStrategy("api_key", &APIKeyStrategy{}, typed["config"])
case "client_credentials":
return decodeClientCredentialsStrategy(typed["config"])
return decodeStrategy("client_credentials", &ClientCredentialsStrategy{}, typed["config"])
default:
return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration,
"unsupported authentication type: '%s'", typed["type"])
}
}
}

func decodeClientCredentialsStrategy(config any) (AuthenticationStrategy, error) {
var strategy ClientCredentialsStrategy

func decodeStrategy[S AuthenticationStrategy](name string, strategy S, config any) (AuthenticationStrategy, error) {
if config == nil {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"client-credentials strategy requires 'config' property to be set")
}

err := mapstructure.Decode(config, &strategy)
if err != nil {
return nil, err
}

if len(strategy.ClientID) == 0 {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"client-credentials strategy requires 'client_id' property to be set")
}

if len(strategy.ClientSecret) == 0 {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"client-credentials strategy requires 'client_secret' property to be set")
}

if len(strategy.TokenURL) == 0 {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"client-credentials strategy requires 'token_url' property to be set")
}

return &strategy, nil
}

func decodeAPIKeyStrategy(config any) (AuthenticationStrategy, error) {
var strategy APIKeyStrategy

if config == nil {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"api-key strategy requires 'config' property to be set")
}

err := mapstructure.Decode(config, &strategy)
if err != nil {
return nil, err
}

if len(strategy.Name) == 0 {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"api-key strategy requires 'name' property to be set")
}

if len(strategy.Value) == 0 {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"api-key strategy requires 'value' property to be set")
}

if len(strategy.In) == 0 {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"api-key strategy requires 'in' property to be set")
}

if strategy.In != "header" && strategy.In != "cookie" && strategy.In != "query" {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"api-key strategy requires 'in' property to be set to either 'header', 'cookie', or 'query'")
}

return &strategy, nil
}

func decodeBasicAuthStrategy(config any) (AuthenticationStrategy, error) {
var strategy BasicAuthStrategy

if config == nil {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"basic-auth strategy requires 'config' property to be set")
}

err := mapstructure.Decode(config, &strategy)
if err != nil {
return nil, err
return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration,
"'%s' strategy requires 'config' property to be set", name)
}

if len(strategy.User) == 0 {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"basic-auth strategy requires 'user' property to be set")
if err := mapstructure.Decode(config, strategy); err != nil {
return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration,
"failed to unmarshal '%s' strategy config", name).CausedBy(err)
}

if len(strategy.Password) == 0 {
return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration,
"basic-auth strategy requires 'password' property to be set")
if err := validation.ValidateStruct(strategy); err != nil {
return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration,
"failed validating '%s' strategy config", name).CausedBy(err)
}

return &strategy, nil
return strategy, nil
}
18 changes: 9 additions & 9 deletions internal/rules/endpoint/mapstructure_decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ auth:
t.Helper()

assert.Error(t, err)
assert.ErrorContains(t, err, "'user' property to be set")
assert.ErrorContains(t, err, "'user' is a required field")
},
},
{
Expand All @@ -84,7 +84,7 @@ auth:
t.Helper()

assert.Error(t, err)
assert.ErrorContains(t, err, "'password' property to be set")
assert.ErrorContains(t, err, "'password' is a required field")
},
},
{
Expand Down Expand Up @@ -215,7 +215,7 @@ auth:
t.Helper()

assert.Error(t, err)
assert.ErrorContains(t, err, "to either 'header', 'cookie', or 'query'")
assert.ErrorContains(t, err, "'in' must be one of [cookie header query]")
},
},
{
Expand All @@ -231,7 +231,7 @@ auth:
t.Helper()

assert.Error(t, err)
assert.ErrorContains(t, err, "'in' property to be set")
assert.ErrorContains(t, err, "'in' is a required field")
},
},
{
Expand All @@ -247,7 +247,7 @@ auth:
t.Helper()

assert.Error(t, err)
assert.ErrorContains(t, err, "'name' property to be set")
assert.ErrorContains(t, err, "'name' is a required field")
},
},
{
Expand All @@ -263,7 +263,7 @@ auth:
t.Helper()

assert.Error(t, err)
assert.ErrorContains(t, err, "'value' property to be set")
assert.ErrorContains(t, err, "'value' is a required field")
},
},
{
Expand Down Expand Up @@ -376,7 +376,7 @@ auth:
t.Helper()

assert.Error(t, err)
assert.ErrorContains(t, err, "'client_id' property to be set")
assert.ErrorContains(t, err, "'client_id' is a required field")
},
},
{
Expand All @@ -392,7 +392,7 @@ auth:
t.Helper()

assert.Error(t, err)
assert.ErrorContains(t, err, "'client_secret' property to be set")
assert.ErrorContains(t, err, "'client_secret' is a required field")
},
},
{
Expand All @@ -408,7 +408,7 @@ auth:
t.Helper()

assert.Error(t, err)
assert.ErrorContains(t, err, "'token_url' property to be set")
assert.ErrorContains(t, err, "'token_url' is a required field")
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func newAnonymousAuthenticator(id string, rawConfig map[string]any) (*anonymousA

if err := decodeConfig(rawConfig, &auth); err != nil {
return nil, errorchain.
NewWithMessage(heimdall.ErrConfiguration, "failed to decode anonymous authenticator config").
NewWithMessage(heimdall.ErrConfiguration, "failed decoding 'anonymous' authenticator config").
CausedBy(err)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestCreateAnonymousAuthenticator(t *testing.T) {

require.Error(t, err)
assert.ErrorIs(t, err, heimdall.ErrConfiguration)
assert.Contains(t, err.Error(), "failed to decode")
assert.Contains(t, err.Error(), "failed decoding")
},
},
} {
Expand Down Expand Up @@ -140,7 +140,7 @@ func TestCreateAnonymousAuthenticatorFromPrototype(t *testing.T) {

assert.Error(t, err)
assert.ErrorIs(t, err, heimdall.ErrConfiguration)
assert.Contains(t, err.Error(), "failed to decode")
assert.Contains(t, err.Error(), "failed decoding")
},
},
} {
Expand Down
Loading

0 comments on commit c5e25ac

Please sign in to comment.