more WIP: lots of fun testing

plugin/plugin.go

@@ -17,6 +17,11 @@ import (
 	"google.golang.org/protobuf/types/known/structpb"
 )
 
+// rotationWaitTimeout controls the time we wait for credential rotation to
+// succeed. This is important to ensure that rotated credentials can be used
+// right away.
+const rotationWaitTimeout = time.Second * 30
+
 // AwsPlugin implements the HostPluginServiceServer interface for the
 // AWS plugin.
 type AwsPlugin struct {
@@ -74,6 +79,11 @@ func (p *AwsPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalog
 		if err := state.RotateCreds(); err != nil {
 			return nil, fmt.Errorf("error during credential rotation: %w", err)
 		}
+	} else {
+		// Simply validate if we aren't rotating.
+		if err := state.ValidateCreds(); err != nil {
+			return nil, fmt.Errorf("error during credential validation: %w", err)
+		}
 	}
 
 	persistedProto, err := state.ToProto()
@@ -456,6 +466,22 @@ func (s *awsCatalogPersistedState) ToProto() (*pb.HostCatalogPersisted, error) {
 	return &pb.HostCatalogPersisted{Data: data}, nil
 }
 
+func (s *awsCatalogPersistedState) ValidateCreds() error {
+	c, err := awsutil.NewCredentialsConfig(
+		awsutil.WithAccessKey(s.AccessKeyId),
+		awsutil.WithSecretKey(s.SecretAccessKey),
+	)
+	if err != nil {
+		return fmt.Errorf("error loading credentials: %w", err)
+	}
+
+	if _, err := c.GetCallerIdentity(); err != nil {
+		return fmt.Errorf("error validating credentials: %w", err)
+	}
+
+	return nil
+}
+
 func (s *awsCatalogPersistedState) RotateCreds() error {
 	c, err := awsutil.NewCredentialsConfig(
 		awsutil.WithAccessKey(s.AccessKeyId),
@@ -465,7 +491,7 @@ func (s *awsCatalogPersistedState) RotateCreds() error {
 		return fmt.Errorf("error loading credentials: %w", err)
 	}
 
-	if err := c.RotateKeys(); err != nil {
+	if err := c.RotateKeys(awsutil.WithValidityCheckTimeout(rotationWaitTimeout)); err != nil {
 		return fmt.Errorf("error rotating credentials: %w", err)
 	}
 

plugin/plugin_test.go

@@ -0,0 +1,349 @@
+package plugin
+
+import (
+	"context"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/aws/aws-sdk-go/aws/awserr"
+	pluginTesting "github.com/hashicorp/boundary-host-plugin-aws/testing"
+	"github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/hostcatalogs"
+	pb "github.com/hashicorp/boundary/sdk/pbs/plugin"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/types/known/structpb"
+)
+
+func TestPlugin(t *testing.T) {
+	region := os.Getenv("AWS_REGION")
+	if region == "" {
+		t.Skip("set AWS_REGION to use this test")
+	}
+	if envAccessKeyId := os.Getenv("AWS_ACCESS_KEY_ID"); envAccessKeyId == "" {
+		t.Skip("set AWS_ACCESS_KEY_ID to use this test")
+	}
+	if envSecretAccessKey := os.Getenv("AWS_SECRET_ACCESS_KEY"); envSecretAccessKey == "" {
+		t.Skip("set AWS_SECRET_ACCESS_KEY to use this test")
+	}
+
+	require := require.New(t)
+	tf, err := pluginTesting.NewTestTerraformer("testdata/basic")
+	require.NoError(err)
+	require.NotNil(tf)
+
+	err = tf.Deploy()
+	require.NoError(err)
+
+	defer func() {
+		if err := tf.Destroy(); err != nil {
+			t.Logf("WARNING: could not run Terraform destroy: %s", err)
+		}
+	}()
+
+	iamUserName, err := tf.GetOutputString("iam_user_name")
+	require.NoError(err)
+	require.NotZero(iamUserName)
+
+	iamUserArn, err := tf.GetOutputString("iam_user_arn")
+	require.NoError(err)
+	require.NotZero(iamUserArn)
+
+	iamAccessKeyId, err := tf.GetOutputString("iam_access_key_id")
+	require.NoError(err)
+	require.NotZero(iamAccessKeyId)
+
+	iamSecretAccessKey, err := tf.GetOutputString("iam_secret_access_key")
+	require.NoError(err)
+	require.NotZero(iamSecretAccessKey)
+
+	ec2InstanceIds, err := tf.GetOutputSlice("instance_ids")
+	require.NoError(err)
+	require.Len(ec2InstanceIds, 5)
+
+	ec2InstanceAddrs, err := tf.GetOutputMap("instance_addrs")
+	require.NoError(err)
+	require.Len(ec2InstanceAddrs, 5)
+
+	ec2InstanceTags, err := tf.GetOutputMap("instance_tags")
+	require.NoError(err)
+	require.Len(ec2InstanceTags, 5)
+
+	// Start the workflow now. Set up the host catalog. Note that this
+	// will cause the state to go out of drift above in the sense that
+	// the access key ID/secret access key will no longer be valid. We
+	// will assert this through the returned state.
+	p := new(AwsPlugin)
+	ctx := context.Background()
+
+	// ********************
+	// * OnCreateCatalog
+	// ********************
+	//
+	// Test rotation first.
+	iamAccessKeyId, iamSecretAccessKey = testPluginOnCreateCatalog(ctx, t, p, region, iamAccessKeyId, iamSecretAccessKey, true)
+	// Test non-rotation next.
+	iamAccessKeyId, iamSecretAccessKey = testPluginOnCreateCatalog(ctx, t, p, region, iamAccessKeyId, iamSecretAccessKey, false)
+
+	// ********************
+	// * OnUpdateCatalog
+	// ********************
+	//
+	// Test no-op non-rotation.
+	iamAccessKeyId, iamSecretAccessKey = testPluginOnCreateCatalog(ctx, t, p, region, iamAccessKeyId, iamSecretAccessKey, "", "", false, false)
+	// Switch to rotation.
+	iamAccessKeyId, iamSecretAccessKey = testPluginOnCreateCatalog(ctx, t, p, region, iamAccessKeyId, iamSecretAccessKey, "", "", false, true)
+	// Test no-op with rotation.
+	iamAccessKeyId, iamSecretAccessKey = testPluginOnCreateCatalog(ctx, t, p, region, iamAccessKeyId, iamSecretAccessKey, "", "", true, true)
+	// Switch credentials to other user.
+}
+
+func testPluginOnCreateCatalog(ctx context.Context, t *testing.T, p *AwsPlugin, region, accessKeyId, secretAccessKey string, rotate bool) (string, string) {
+	t.Helper()
+	require := require.New(t)
+
+	reqAttrs, err := structpb.NewStruct(map[string]interface{}{
+		constRegion:                    region,
+		constDisableCredentialRotation: !rotate,
+	})
+	require.NoError(err)
+	reqSecrets, err := structpb.NewStruct(map[string]interface{}{
+		constAccessKeyId:     accessKeyId,
+		constSecretAccessKey: secretAccessKey,
+	})
+	require.NoError(err)
+	request := &pb.OnCreateCatalogRequest{
+		Catalog: &hostcatalogs.HostCatalog{
+			Attributes: reqAttrs,
+			Secrets:    reqSecrets,
+		},
+	}
+	response, err := p.OnCreateCatalog(ctx, request)
+	require.NoError(err)
+	require.NotNil(response)
+	persisted := response.GetPersisted()
+	require.NotNil(persisted)
+	persistedData := persisted.GetData()
+	require.NotNil(persistedData)
+	persistedDataMap := persistedData.AsMap()
+	require.NotNil(persistedDataMap)
+	persistedAccessKeyId, ok := persistedDataMap[constAccessKeyId]
+	require.True(ok)
+	require.NotZero(persistedAccessKeyId)
+	if rotate {
+		require.NotEqual(accessKeyId, persistedAccessKeyId)
+	} else {
+		require.Equal(accessKeyId, persistedAccessKeyId)
+	}
+
+	persistedSecretAccessKey, ok := persistedDataMap[constSecretAccessKey]
+	require.True(ok)
+	require.NotZero(persistedSecretAccessKey)
+	if rotate {
+		require.NotEqual(secretAccessKey, persistedSecretAccessKey)
+	} else {
+		require.Equal(secretAccessKey, persistedSecretAccessKey)
+	}
+
+	persistedCredsLastRotatedTime, ok := persistedDataMap[constCredsLastRotatedTime]
+	require.True(ok)
+	if rotate {
+		require.NotZero(persistedCredsLastRotatedTime)
+	} else {
+		require.Zero(persistedCredsLastRotatedTime)
+	}
+
+	return persistedAccessKeyId.(string), persistedSecretAccessKey.(string)
+}
+
+func testPluginOnUpdateCatalog(
+	ctx context.Context, t *testing.T, p *AwsPlugin,
+	region, currentAccessKeyId, currentSecretAccessKey, newAccessKeyId, newSecretAccessKey string,
+	rotated, rotate bool,
+) (string, string) {
+	t.Helper()
+	require := require.New(t)
+
+	// Take a timestamp of the current time to get a point in time to
+	// reference, ensuring that we are updating credential rotation
+	// timestamps.
+	currentLastRotationTime = time.Now()
+
+	reqCurrentAttrs, err := structpb.NewStruct(map[string]interface{}{
+		constRegion:                    region,
+		constDisableCredentialRotation: !rotated,
+	})
+	reqNewAttrs, err := structpb.NewStruct(map[string]interface{}{
+		constRegion:                    region,
+		constDisableCredentialRotation: !rotate,
+	})
+	require.NoError(err)
+	var reqSecrets *structpb.Struct
+	if newAccessKeyId != "" && newSecretAccessKey != "" {
+		reqSecrets, err = structpb.NewStruct(map[string]interface{}{
+			constAccessKeyId:     newAccessKeyId,
+			constSecretAccessKey: newSecretAccessKey,
+		})
+	}
+	reqPersistedData, err := structpb.NewStruct(map[string]interface{}{
+		constAccessKeyId:     currentAccessKeyId,
+		constSecretAccessKey: currentSecretAccessKey,
+		constCredsLastRotatedTime: func() time.Time {
+			if rotated {
+				return currentLastRotationTime
+			}
+
+			return time.Time{}
+		}(),
+	})
+	require.NoError(err)
+	onUpdateCatalogRequest := &pb.OnUpdateCatalogRequest{
+		CurrentCatalog: &hostcatalogs.HostCatalog{
+			Attributes: reqCurrentAttrs,
+		},
+		NewCatalog: &hostcatalogs.HostCatalog{
+			Attributes: reqNewAttrs,
+			Secrets:    reqSecrets,
+		},
+		Persisted: &pb.HostCatalogPersisted{
+			Data: reqPersistedData,
+		},
+	}
+	response, err := p.OnCreateCatalog(ctx, request)
+	require.NoError(err)
+	require.NotNil(response)
+	persisted := response.GetPersisted()
+	require.NotNil(persisted)
+	persistedData := persisted.GetData()
+	require.NotNil(persistedData)
+	persistedDataMap := persistedData.AsMap()
+	require.NotNil(persistedDataMap)
+
+	// Complex checks based on the scenarios.
+	persistedAccessKeyId, ok := persistedDataMap[constAccessKeyId]
+	require.True(ok)
+	require.NotZero(persistedAccessKeyId)
+	persistedSecretAccessKey, ok := persistedDataMap[constSecretAccessKey]
+	require.True(ok)
+	require.NotZero(persistedSecretAccessKey)
+	persistedCredsLastRotatedTime, ok := persistedDataMap[constCredsLastRotatedTime]
+	require.True(ok)
+
+	// Our test scenarios are complex due the multi-dimensional nature
+	// of criteria, so we lay them out in a switch below.
+	switch {
+	case newAccessKeyId != "" && rotated && rotate:
+		// The new access key ID was provided, we had previously rotated
+		// the credentials before, and the new credential set is to be
+		// rotated as well. In this case, the old credentials should have
+		// been deleted, and the new credentials should have been rotated,
+		// hence, should not match the new credentials initially
+		// provided. Rotation time should be non-zero and updated.
+		require.CredentialsInvalid(t, currentAccessKeyId, currentSecretAccessKey)
+		require.NotEqual(persistedAccessKeyId, newAccessKeyId)
+		require.NotEqual(persistedSecretAccessKey, newSecretAccessKey)
+		require.NotZero(persistedCredsLastRotatedTime)
+		require.True(persistedCredsLastRotatedTime.After(currentCredsLastRotatedTime))
+
+	case newAccessKeyId != "" && rotated && !rotate:
+		// The new access key ID was provided, we had previously rotated
+		// the credentials before, and the new credential is *not*
+		// rotated. In this case, the old credentials should have
+		// been deleted, But the new credentials should have not been
+		// rotated, and hence should be the same. Rotation time should be
+		// zero.
+		require.CredentialsInvalid(t, currentAccessKeyId, currentSecretAccessKey)
+		require.Equal(persistedAccessKeyId, newAccessKeyId)
+		require.Equal(persistedSecretAccessKey, newSecretAccessKey)
+		require.Zero(persistedCredsLastRotatedTime)
+
+	case newAccessKeyId != "" && !rotated && rotate:
+		// The new access key ID was provided, we *have not* previously
+		// rotated the credentials, and the new credential set is to be
+		// rotated. In this case, the old credentials should have been
+		// left alone, and the new credentials should have been rotated,
+		// hence, should not match the new credentials initially
+		// provided. Rotation time should be non-zero, but updated.
+		require.CredentialsValid(t, currentAccessKeyId, currentSecretAccessKey)
+		require.NotEqual(persistedAccessKeyId, newAccessKeyId)
+		require.NotEqual(persistedSecretAccessKey, newSecretAccessKey)
+		require.NotZero(persistedCredsLastRotatedTime)
+		require.True(persistedCredsLastRotatedTime.After(currentCredsLastRotatedTime))
+
+	case newAccessKeyId != "" && !rotated && !rotate:
+		// The new access key ID was provided, but we have not rotated
+		// the credentials previously and we still don't plan on rotating
+		// them. In this case, the old credentials should still be valid,
+		// and the persisted ones should match the new ones provided.
+		// Rotation time should be zero.
+		require.CredentialsValid(t, currentAccessKeyId, currentSecretAccessKey)
+		require.Equal(persistedAccessKeyId, newAccessKeyId)
+		require.Equal(persistedSecretAccessKey, newSecretAccessKey)
+		require.Zero(persistedCredsLastRotatedTime)
+
+	case newAccessKeyId == "" && rotated && rotate:
+		// No new credentials have been provided, but we have previously
+		// rotated and are still rotating credentials. This is a no-op.
+		// Existing credentials should still be valid and match the ones
+		// persisted to state. Rotation time should be identical since
+		// no new rotation occurred.
+		require.CredentialsValid(t, currentAccessKeyId, currentSecretAccessKey)
+		require.Equal(persistedAccessKeyId, currentAccessKeyId)
+		require.Equal(persistedSecretAccessKey, currentSecretAccessKey)
+		require.NotZero(persistedCredsLastRotatedTime)
+		require.Equal(currentCredsLastRotatedTime, persistedCredsLastRotatedTime)
+
+	case newAccessKeyId == "" && rotated && !rotate:
+		// No new credentials have been provided, and we have previously
+		// rotated the credentials. This is actually an error.
+		//
+		// TODO: validate this scenario through unit testing. For now if
+		// we for some reason try to test this with this function, return
+		// an error.
+		require.FailNow("testing rotated-to-not-rotated scenario not implemented by this helper")
+
+	case newAccessKeyId == "" && !rotated && rotate:
+		// No new credentials have been provided, and while we did not
+		// rotate before, we want to switch to rotation. In this case,
+		// the existing persisted credentials should have been rotated,
+		// with a new non-zero timestamp.
+		require.CredentialsInvalid(t, currentAccessKeyId, currentSecretAccessKey)
+		require.NotEqual(persistedAccessKeyId, currentAccessKeyId)
+		require.NotEqual(persistedSecretAccessKey, currentSecretAccessKey)
+		require.NotZero(persistedCredsLastRotatedTime)
+
+	case newAccessKeyId == "" && !rotated && !rotate:
+		// No new credentials have been provided and we have not, nor do
+		// not, plan on rotating the credentials. This is a no-op.
+		// Existing credentials should still be valid and match the ones
+		// persisted to state. Rotation time should remain at zero.
+		require.CredentialsValid(t, currentAccessKeyId, currentSecretAccessKey)
+		require.Equal(persistedAccessKeyId, currentAccessKeyId)
+		require.Equal(persistedSecretAccessKey, currentSecretAccessKey)
+		require.Zero(persistedCredsLastRotatedTime)
+
+	default:
+		// Scenario was reached that was not covered by this function.
+		require.FailNow("unknown test scenario")
+	}
+
+	return persistedAccessKeyId.(string), persistedSecretAccessKey.(string)
+}
+
+func requireCredentailsInvalid(t *testing.T, accessKeyId, secretAccessKey string) {
+	t.Helper()
+	require := require.New(t)
+
+	err := c.GetCallerIdentity()
+	require.NotNil(err)
+	awsErr, ok := err.(awserr.Error)
+	require.True(ok)
+	require.Equal("InvalidClientTokenId", awsErr.Code())
+}
+
+func requireCredentailsvValid(t *testing.T, accessKeyId, secretAccessKey string) {
+	t.Helper()
+	require := require.New(t)
+
+	err := c.GetCallerIdentity()
+	require.NoError(err)
+}

plugin/testdata/basic/iam.tf

@@ -2,19 +2,27 @@ variable "iam_user_prefix" {
   default = "boundary-plugin-host-aws-test-iam-user"
 }
 
+variable "iam_user_count" {
+  default = 2
+}
+
 resource "random_id" "aws_iam_user_name" {
-  prefix      = var.iam_user_prefix
+  count       = var.iam_user_count
+  prefix      = "${var.iam_user_prefix}-${count.index}"
   byte_length = 4
 }
 
 resource "aws_iam_user" "user" {
-  name = random_id.aws_iam_user_name.dec
+  count         = var.iam_user_count
+  name          = random_id.aws_iam_user_name[count.index].dec
+  force_destroy = true
 
   tags = var.project_path_tags
 }
 
 resource "aws_iam_access_key" "user_initial_key" {
-  user = aws_iam_user.user.name
+  count = var.iam_user_count
+  user  = aws_iam_user.user[count.index].name
 }
 
 resource "aws_iam_policy" "ec2_describeinstances" {
@@ -39,12 +47,14 @@ EOF
 }
 
 resource "aws_iam_user_policy_attachment" "user_ec2_describeinstances_attachment" {
-  user       = aws_iam_user.user.name
+  count      = var.iam_user_count
+  user       = aws_iam_user.user[count.index].name
   policy_arn = aws_iam_policy.ec2_describeinstances.arn
 }
 
 resource "aws_iam_policy" "user_self_manage_policy" {
-  name = "BoundaryPluginHostAwsTestUserSelfManagePolicy"
+  count = var.iam_user_count
+  name  = "BoundaryPluginHostAwsTestUserSelfManagePolicy-${count.index}"
 
   policy = <<EOF
 {
@@ -57,7 +67,7 @@ resource "aws_iam_policy" "user_self_manage_policy" {
         "iam:CreateAccessKey"
       ],
       "Effect": "Allow",
-      "Resource": "${aws_iam_user.user.arn}"
+      "Resource": "${aws_iam_user.user[count.index].arn}"
     }
   ]
 }
@@ -67,24 +77,25 @@ EOF
 }
 
 resource "aws_iam_user_policy_attachment" "user_self_manage_policy_attachment" {
-  user       = aws_iam_user.user.name
-  policy_arn = aws_iam_policy.user_self_manage_policy.arn
+  count      = var.iam_user_count
+  user       = aws_iam_user.user[count.index].name
+  policy_arn = aws_iam_policy.user_self_manage_policy[count.index].arn
 }
 
-output "iam_user_name" {
-  value = aws_iam_user.user.name
+output "iam_user_names" {
+  value = aws_iam_user.user.*.name
 }
 
-output "iam_user_arn" {
-  value = aws_iam_user.user.arn
+output "iam_user_arns" {
+  value = aws_iam_user.user.*.arn
 }
 
-output "iam_access_key_id" {
-  value     = aws_iam_access_key.user_initial_key.id
+output "iam_access_key_ids" {
+  value     = aws_iam_access_key.user_initial_key.*.id
   sensitive = true
 }
 
 output "iam_secret_access_key" {
-  value     = aws_iam_access_key.user_initial_key.secret
+  value     = aws_iam_access_key.user_initial_key.*.secret
   sensitive = true
 }

plugin/testdata/basic/vpc_ec2.tf

@@ -60,6 +60,16 @@ resource "aws_instance" "instances" {
   tags = merge(var.instance_tags[count.index], var.project_path_tags)
 }
 
+output "instance_ids" {
+  value = aws_instance.instances.*.id
+}
+
+output "instance_addrs" {
+  value = {
+    for _, r in aws_instance.instances : r.id => r.private_ip
+  }
+}
+
 output "instance_tags" {
   value = {
     for _, r in aws_instance.instances : r.id => r.tags