fix(host): fix role_arn fallback bug (#82)
* Fix role arn fallthrough bug * add unit test * bump awsutil version * fix tests
This commit is contained in:
1 parent
ca7b5ff389
commit
1c87d178a2
5 files changed
+153
-10
No files matched your search
@@ -86,6 +86,15 @@ func (p *HostPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalo
|
||||
}
|
||||
}
|
||||
|
||||
// Validate assumed-role credentials at catalog-creation time so that an
|
||||
// invalid role_arn is rejected immediately rather than silently falling
|
||||
// back to ambient credentials.
|
||||
if credential.GetCredentialType(credState.CredentialsConfig) == credential.DynamicAWS {
|
||||
if err := credState.ValidateCreds(ctx); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
catalogState, err := newAwsCatalogPersistedState(
|
||||
append([]awsCatalogPersistedStateOption{
|
||||
withCredentials(credState),
|
||||
@@ -181,6 +190,15 @@ func (p *HostPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalo
|
||||
if err != nil {
|
||||
return nil, errors.BadRequestStatusf("error setting up new credential persisted state: %s", err)
|
||||
}
|
||||
|
||||
// Validate assumed-role credentials when the role_arn changes so that
|
||||
// an invalid ARN is caught before replacing the existing credential state.
|
||||
if credential.GetCredentialType(newCredState.CredentialsConfig) == credential.DynamicAWS {
|
||||
if err := newCredState.ValidateCreds(ctx); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
newCatalogState, err := newAwsCatalogPersistedState(
|
||||
append([]awsCatalogPersistedStateOption{
|
||||
withCredentials(newCredState),
|
||||
|
||||
@@ -16,6 +16,8 @@ import (
|
||||
"github.com/aws/aws-sdk-go-v2/service/ec2/types"
|
||||
"github.com/aws/aws-sdk-go-v2/service/iam"
|
||||
iamTypes "github.com/aws/aws-sdk-go-v2/service/iam/types"
|
||||
"github.com/aws/aws-sdk-go-v2/service/sts"
|
||||
stsTypes "github.com/aws/aws-sdk-go-v2/service/sts/types"
|
||||
"github.com/google/go-cmp/cmp"
|
||||
"github.com/hashicorp/boundary-plugin-aws/internal/credential"
|
||||
awserrors "github.com/hashicorp/boundary-plugin-aws/internal/errors"
|
||||
@@ -29,6 +31,38 @@ import (
|
||||
"google.golang.org/protobuf/testing/protocmp"
|
||||
"google.golang.org/protobuf/types/known/structpb"
|
||||
)
|
||||
// testRoleARN is a syntactically valid ARN used for role_arn test cases.
|
||||
// All STS calls against it are handled by injected mocks — no real AWS
|
||||
// account is contacted.
|
||||
const testRoleARN = "arn:aws:iam::123456789012:role/BoundaryTestRole"
|
||||
|
||||
// validAssumeRoleOutput returns a mock AssumeRoleOutput with synthetic
|
||||
// temporary credentials for use with WithSTSAPIFunc.
|
||||
func validAssumeRoleOutput() *sts.AssumeRoleOutput {
|
||||
return &sts.AssumeRoleOutput{
|
||||
Credentials: &stsTypes.Credentials{
|
||||
AccessKeyId: aws.String("ASIAmockassumedkey"),
|
||||
SecretAccessKey: aws.String("mockassumedsecret"),
|
||||
SessionToken: aws.String("mockassumedsessiontoken"),
|
||||
Expiration: aws.Time(time.Now().Add(time.Hour)),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// catalogAttrsStruct builds a catalog attributes structpb for the given
|
||||
// region. roleArn is written only when non-empty. disableRotation must be
|
||||
// set explicitly to avoid implicit rotation behaviour in tests.
|
||||
func catalogAttrsStruct(region, roleArn string, disableRotation bool) *structpb.Struct {
|
||||
fields := map[string]*structpb.Value{
|
||||
credential.ConstRegion: structpb.NewStringValue(region),
|
||||
credential.ConstDisableCredentialRotation: structpb.NewBoolValue(disableRotation),
|
||||
}
|
||||
if roleArn != "" {
|
||||
fields[credential.ConstRoleArn] = structpb.NewStringValue(roleArn)
|
||||
}
|
||||
return &structpb.Struct{Fields: fields}
|
||||
}
|
||||
|
||||
|
||||
func TestPluginOnCreateCatalogSuccess(t *testing.T) {
|
||||
tests := []struct {
|
||||
@@ -139,17 +173,13 @@ func TestPluginOnCreateCatalogSuccess(t *testing.T) {
|
||||
},
|
||||
},
|
||||
{
|
||||
// role_arn set, no static keys. AssumeRole mock succeeds.
|
||||
// Persisted secrets are empty — dynamic credentials are not stored.
|
||||
name: "usingAssumeRole",
|
||||
req: &pb.OnCreateCatalogRequest{
|
||||
Catalog: &hostcatalogs.HostCatalog{
|
||||
Attrs: &hostcatalogs.HostCatalog_Attributes{
|
||||
Attributes: &structpb.Struct{
|
||||
Fields: map[string]*structpb.Value{
|
||||
credential.ConstRegion: structpb.NewStringValue("us-east-1"),
|
||||
credential.ConstRoleArn: structpb.NewStringValue("arn:0123:test:rolearn"),
|
||||
credential.ConstDisableCredentialRotation: structpb.NewBoolValue(true),
|
||||
},
|
||||
},
|
||||
Attributes: catalogAttrsStruct("us-east-1", testRoleARN, true),
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -159,6 +189,13 @@ func TestPluginOnCreateCatalogSuccess(t *testing.T) {
|
||||
testMockEC2WithDescribeInstancesOutput(&ec2.DescribeInstancesOutput{}),
|
||||
)),
|
||||
},
|
||||
credOpts: []credential.AwsCredentialPersistedStateOption{
|
||||
credential.WithStateTestOpts([]awsutil.Option{
|
||||
awsutil.WithSTSAPIFunc(awsutil.NewMockSTS(
|
||||
awsutil.WithAssumeRoleOutput(validAssumeRoleOutput()),
|
||||
)),
|
||||
}),
|
||||
},
|
||||
expRsp: &pb.OnCreateCatalogResponse{Persisted: &pb.HostCatalogPersisted{Secrets: &structpb.Struct{}}},
|
||||
},
|
||||
}
|
||||
@@ -316,6 +353,20 @@ func TestPluginOnUpdateCatalogSuccess(t *testing.T) {
|
||||
testMockEC2WithDescribeInstancesOutput(&ec2.DescribeInstancesOutput{}),
|
||||
)),
|
||||
},
|
||||
credOpts: []credential.AwsCredentialPersistedStateOption{
|
||||
credential.WithStateTestOpts([]awsutil.Option{
|
||||
awsutil.WithSTSAPIFunc(awsutil.NewMockSTS(
|
||||
awsutil.WithAssumeRoleOutput(&sts.AssumeRoleOutput{
|
||||
Credentials: &stsTypes.Credentials{
|
||||
AccessKeyId: aws.String("ASIAfoobar"),
|
||||
SecretAccessKey: aws.String("secretkeyfoobar"),
|
||||
SessionToken: aws.String("sessiontokenfoobar"),
|
||||
Expiration: aws.Time(time.Now().Add(time.Hour)),
|
||||
},
|
||||
}),
|
||||
)),
|
||||
}),
|
||||
},
|
||||
expRsp: &pb.OnUpdateCatalogResponse{
|
||||
Persisted: &pb.HostCatalogPersisted{Secrets: &structpb.Struct{}},
|
||||
},
|
||||
@@ -422,6 +473,16 @@ func TestPluginOnUpdateCatalogSuccess(t *testing.T) {
|
||||
credOpts: []credential.AwsCredentialPersistedStateOption{
|
||||
credential.WithStateTestOpts([]awsutil.Option{
|
||||
awsutil.WithIAMAPIFunc(awsutil.NewMockIAM()),
|
||||
awsutil.WithSTSAPIFunc(awsutil.NewMockSTS(
|
||||
awsutil.WithAssumeRoleOutput(&sts.AssumeRoleOutput{
|
||||
Credentials: &stsTypes.Credentials{
|
||||
AccessKeyId: aws.String("ASIAfoobar"),
|
||||
SecretAccessKey: aws.String("secretkeyfoobar"),
|
||||
SessionToken: aws.String("sessiontokenfoobar"),
|
||||
Expiration: aws.Time(time.Now().Add(time.Hour)),
|
||||
},
|
||||
}),
|
||||
)),
|
||||
}),
|
||||
},
|
||||
expRsp: &pb.OnUpdateCatalogResponse{
|
||||
@@ -749,6 +810,35 @@ func TestPluginOnCreateCatalogErr(t *testing.T) {
|
||||
expectedErrContains: testDescribeInstancesError,
|
||||
expectedErrCode: codes.Unknown,
|
||||
},
|
||||
{
|
||||
// BUG FIX: role_arn set, no static keys. AssumeRole mock returns
|
||||
// an error. ValidateCreds now eagerly calls Retrieve() which forces
|
||||
// sts:AssumeRole, so the error surfaces here at creation time.
|
||||
// Previously this succeeded silently via the ambient credential chain.
|
||||
name: "role_arn AssumeRole error — correctly rejected",
|
||||
req: &pb.OnCreateCatalogRequest{
|
||||
Catalog: &hostcatalogs.HostCatalog{
|
||||
Attrs: &hostcatalogs.HostCatalog_Attributes{
|
||||
Attributes: catalogAttrsStruct("us-east-1", testRoleARN, true),
|
||||
},
|
||||
},
|
||||
},
|
||||
credOpts: []credential.AwsCredentialPersistedStateOption{
|
||||
credential.WithStateTestOpts([]awsutil.Option{
|
||||
awsutil.WithSTSAPIFunc(awsutil.NewMockSTS(
|
||||
awsutil.WithAssumeRoleError(errors.New("simulated AssumeRole failure")),
|
||||
)),
|
||||
}),
|
||||
},
|
||||
catalogOpts: []awsCatalogPersistedStateOption{
|
||||
withTestEC2APIFunc(newTestMockEC2(
|
||||
nil,
|
||||
testMockEC2WithDescribeInstancesOutput(&ec2.DescribeInstancesOutput{}),
|
||||
)),
|
||||
},
|
||||
expectedErrContains: "simulated AssumeRole failure",
|
||||
expectedErrCode: codes.Unknown,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
@@ -1037,6 +1127,16 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) {
|
||||
},
|
||||
},
|
||||
},
|
||||
// ValidateCreds is called for DynamicAWS credentials before the dry-run
|
||||
// DescribeInstances. Provide a mock STS that returns a successful AssumeRole
|
||||
// so the validate step passes and execution reaches the EC2 mock.
|
||||
credOpts: []credential.AwsCredentialPersistedStateOption{
|
||||
credential.WithStateTestOpts([]awsutil.Option{
|
||||
awsutil.WithSTSAPIFunc(awsutil.NewMockSTS(
|
||||
awsutil.WithAssumeRoleOutput(validAssumeRoleOutput()),
|
||||
)),
|
||||
}),
|
||||
},
|
||||
catalogOpts: []awsCatalogPersistedStateOption{
|
||||
withTestEC2APIFunc(newTestMockEC2(
|
||||
nil,
|
||||
|
||||
Reference in new issue
Block a user