Skip to content

Commit

Permalink
move parameter checks into New method
Browse files Browse the repository at this point in the history
Seems like it'd be better to return them sooner rather than later.
  • Loading branch information
eikenb committed Feb 28, 2023
1 parent e4ff0f4 commit aa22e59
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 15 deletions.
2 changes: 1 addition & 1 deletion agent/connect/ca/provider_vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ func configureVaultAuthMethod(authMethod *structs.VaultAuthMethod) (VaultAuthent
case VaultAuthMethodTypeGCP:
return NewGCPAuthClient(authMethod)
case VaultAuthMethodTypeAliCloud:
return NewAliCloudAuthClient(authMethod), nil
return NewAliCloudAuthClient(authMethod)
case VaultAuthMethodTypeKubernetes:
// For the Kubernetes Auth method, we will try to read the JWT token
// from the default service account file location if jwt was not provided.
Expand Down
21 changes: 11 additions & 10 deletions agent/connect/ca/provider_vault_auth_alicloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,22 @@ import (
"github.com/hashicorp/vault-plugin-auth-alicloud/tools"
)

func NewAliCloudAuthClient(authMethod *structs.VaultAuthMethod) *VaultAuthClient {
func NewAliCloudAuthClient(authMethod *structs.VaultAuthMethod) (*VaultAuthClient, error) {
if _, ok := authMethod.Params["role"].(string); !ok {
return nil, fmt.Errorf("role is required for AliCloud login")
}
if _, ok := authMethod.Params["region"].(string); !ok {
return nil, fmt.Errorf("region is required for AliCloud login")
}
client := NewVaultAPIAuthClient(authMethod, "")
client.LoginDataGen = AliLoginDataGen
return client
return client, nil
}

func AliLoginDataGen(authMethod *structs.VaultAuthMethod) (map[string]any, error) {
role, ok := authMethod.Params["role"].(string)
if !ok {
return nil, fmt.Errorf("role is required for AliCloud login")
}
region, ok := authMethod.Params["region"].(string)
if !ok {
return nil, fmt.Errorf("region is required for AliCloud login")
}
// validity of these params is checked above in New..
role := authMethod.Params["role"].(string)
region := authMethod.Params["region"].(string)
// Credentials can be provided either explicitly via env vars,
// or we will try to derive them from instance metadata.
credentialChain := []providers.Provider{
Expand Down
6 changes: 3 additions & 3 deletions agent/connect/ca/provider_vault_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,13 +355,13 @@ func TestVaultCAProvider_AliCloudAuthClient(t *testing.T) {

for name, c := range cases {
t.Run(name, func(t *testing.T) {
auth := NewAliCloudAuthClient(c.authMethod)
require.NotNil(t, auth)
encodedData, err := auth.LoginDataGen(c.authMethod)
auth, err := NewAliCloudAuthClient(c.authMethod)
if c.expErr != nil {
require.Equal(t, err, c.expErr)
return
}
require.NotNil(t, auth)
encodedData, err := auth.LoginDataGen(c.authMethod)
require.NoError(t, err)

// identity_request_headers (json encoded headers)
Expand Down
2 changes: 1 addition & 1 deletion agent/connect/ca/provider_vault_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestVaultCAProvider_configureVaultAuthMethod(t *testing.T) {
expError string
hasLDG bool
}{
"alicloud": {expLoginPath: "auth/alicloud/login", hasLDG: true},
"alicloud": {expLoginPath: "auth/alicloud/login", params: map[string]any{"role": "test-role", "region": "test-region"}, hasLDG: true},
"approle": {expLoginPath: "auth/approle/login"},
"aws": {expLoginPath: "auth/aws/login", params: map[string]interface{}{"type": "iam"}, hasLDG: true},
"azure": {expLoginPath: "auth/azure/login"},
Expand Down

0 comments on commit aa22e59

Please sign in to comment.