Process attachment payload message for EBS attach (aws#3911)

* Process attachment payload message for EBS attach

* Add one more test case

* Update agent

* Update amazonebs to AmazonElasticBlockStorage

* Save attachmentType

ecs-agent/acs/session/attach_resource_responder.go

@@ -91,6 +91,7 @@ func (r *attachResourceResponder) handleAttachMessage(message *ecsacs.ConfirmAtt
 			Status:               status.AttachmentNone,
 		},
 		AttachmentProperties: attachmentProperties,
+		AttachmentType:       aws.StringValue(message.Attachment.AttachmentType),
 	})
 
 	// Send ACK.
@@ -180,9 +181,23 @@ func validateAttachmentAndReturnProperties(message *ecsacs.ConfirmAttachmentMess
 		attachmentProperties[name] = value
 	}
 
-	err = resource.ValidateResource(attachmentProperties)
+	// For "AmazonElasticBlockStorage" used by the EBS attach, ACS is using attachmentType to indicate its attachment type.
+	attachmentType := aws.StringValue(message.Attachment.AttachmentType)
+	if attachmentType == resource.AmazonElasticBlockStorage {
+		err = resource.ValidateRequiredProperties(
+			attachmentProperties,
+			resource.GetVolumeSpecificPropertiesForEBSAttach(),
+		)
+		if err != nil {
+			return nil, errors.Wrap(err, "resource attachment validation by attachment type failed")
+		}
+		return attachmentProperties, nil
+	}
+
+	// For "EphemeralStorage" and "ElasticBlockStorage", ACS is using resourceType to indicate its attachment type.
+	err = resource.ValidateResourceByResourceType(attachmentProperties)
 	if err != nil {
-		return nil, errors.Wrap(err, "resource attachment validation error")
+		return nil, errors.Wrap(err, "resource attachment validation by resource type failed ")
 	}
 
 	return attachmentProperties, nil

ecs-agent/acs/session/attach_resource_responder_test.go

@@ -38,6 +38,29 @@ const (
 )
 
 var (
+	testAttachmentPropertiesForEBSAttach = []*ecsacs.AttachmentProperty{
+		{
+			Name:  aws.String(resource.SourceVolumeHostPathKey),
+			Value: aws.String("taskarn-vol-id"),
+		},
+		{
+			Name:  aws.String(resource.VolumeIdKey),
+			Value: aws.String("id1"),
+		},
+		{
+			Name:  aws.String(resource.VolumeSizeGibKey),
+			Value: aws.String("size1"),
+		},
+		{
+			Name:  aws.String(resource.VolumeNameKey),
+			Value: aws.String("name"),
+		},
+		{
+			Name:  aws.String(resource.DeviceNameKey),
+			Value: aws.String("device1"),
+		},
+	}
+
 	testAttachmentProperties = []*ecsacs.AttachmentProperty{
 		{
 			Name:  aws.String(resource.FargateResourceIdName),
@@ -83,46 +106,47 @@ func TestValidateAttachResourceMessage(t *testing.T) {
 	defer ctrl.Finish()
 
 	_, err := validateAttachResourceMessage(nil)
-
 	require.Error(t, err)
 
+	// Verify the Attachment is required.
 	confirmAttachmentMessageCopy := *testConfirmAttachmentMessage
 	confirmAttachmentMessageCopy.Attachment = nil
-
 	_, err = validateAttachResourceMessage(&confirmAttachmentMessageCopy)
-
 	require.Error(t, err)
 
+	// Verify the MessageId is required.
 	confirmAttachmentMessageCopy = *testConfirmAttachmentMessage
 	confirmAttachmentMessageCopy.MessageId = aws.String("")
-
 	_, err = validateAttachResourceMessage(&confirmAttachmentMessageCopy)
-
 	require.Error(t, err)
 
+	// Verify the ClusterArn is required.
 	confirmAttachmentMessageCopy = *testConfirmAttachmentMessage
 	confirmAttachmentMessageCopy.ClusterArn = aws.String("")
-
 	_, err = validateAttachResourceMessage(&confirmAttachmentMessageCopy)
-
 	require.Error(t, err)
 
+	// Verify the ContainerInstanceArn is required.
 	confirmAttachmentMessageCopy = *testConfirmAttachmentMessage
 	confirmAttachmentMessageCopy.ContainerInstanceArn = aws.String("")
-
 	_, err = validateAttachResourceMessage(&confirmAttachmentMessageCopy)
-
 	require.Error(t, err)
 
+	// Verify the AttachmentArn is required and uses correct format.
 	confirmAttachmentMessageCopy = *testConfirmAttachmentMessage
 	confirmAttachmentMessageCopy.Attachment.AttachmentArn = aws.String("incorrectArn")
-
 	_, err = validateAttachResourceMessage(&confirmAttachmentMessageCopy)
-
 	require.Error(t, err)
 }
 
 func TestValidateAttachmentAndReturnProperties(t *testing.T) {
+	t.Run("with no attachment type", testValidateAttachmentAndReturnPropertiesWithoutAttachmentType)
+	t.Run("with attachment type", testValidateAttachmentAndReturnPropertiesWithAttachmentType)
+}
+
+// testValidateAttachmentAndReturnPropertiesWithoutAttachmentType verifies all required properties for either
+// resource type: "EphemeralStorage" or "ElasticBlockStorage".
+func testValidateAttachmentAndReturnPropertiesWithoutAttachmentType(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
@@ -157,6 +181,71 @@ func TestValidateAttachmentAndReturnProperties(t *testing.T) {
 	}
 }
 
+// testValidateAttachmentAndReturnPropertiesWithAttachmentType verifies all required properties for attachment
+// type: "AmazonElasticBlockStorage".
+func testValidateAttachmentAndReturnPropertiesWithAttachmentType(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	// Verify the AttachmentArn is required and uses the correct format.
+	confirmAttachmentMessageCopy := *testConfirmAttachmentMessage
+	confirmAttachmentMessageCopy.Attachment.AttachmentArn = aws.String("incorrectArn")
+	_, err := validateAttachmentAndReturnProperties(&confirmAttachmentMessageCopy)
+	require.Error(t, err)
+	confirmAttachmentMessageCopy.Attachment.AttachmentArn = aws.String(testAttachmentArn)
+
+	// Verify the property name & property value must be non-empty.
+	for _, property := range confirmAttachmentMessageCopy.Attachment.AttachmentProperties {
+		t.Run(property.String(), func(t *testing.T) {
+			originalPropertyName := property.Name
+			property.Name = aws.String("")
+			_, err := validateAttachmentAndReturnProperties(&confirmAttachmentMessageCopy)
+			require.Error(t, err)
+			property.Name = originalPropertyName
+
+			originalPropertyValue := property.Value
+			property.Value = aws.String("")
+			_, err = validateAttachmentAndReturnProperties(&confirmAttachmentMessageCopy)
+			require.Error(t, err)
+			property.Value = originalPropertyValue
+		})
+	}
+
+	// Reset attachment to be a good one with invalid attachment type.
+	confirmAttachmentMessageCopy.Attachment.AttachmentType = aws.String("invalid-type")
+	_, err = validateAttachmentAndReturnProperties(&confirmAttachmentMessageCopy)
+	require.NoError(t, err)
+
+	// Reset attachment to be a good one with valid attachment type.
+	confirmAttachmentMessageCopy.Attachment.AttachmentType = aws.String("AmazonElasticBlockStorage")
+	confirmAttachmentMessageCopy.Attachment.AttachmentProperties = testAttachmentPropertiesForEBSAttach
+
+	// Verify all required properties for the attachment for EBS attach are present.
+	requiredProperties := []string{
+		"volumeId",
+		"volumeSizeGib",
+		"deviceName",
+		"sourceVolumeHostPath",
+		"volumeName",
+	}
+	for _, requiredProperty := range requiredProperties {
+		verified := false
+		for _, property := range confirmAttachmentMessageCopy.Attachment.AttachmentProperties {
+			if requiredProperty == aws.StringValue(property.Name) {
+				originalPropertyName := property.Name
+				property.Name = aws.String("")
+				_, err := validateAttachmentAndReturnProperties(&confirmAttachmentMessageCopy)
+				require.Error(t, err)
+				property.Name = originalPropertyName
+
+				verified = true
+				break
+			}
+		}
+		require.True(t, verified, "Missing required property: %s", requiredProperty)
+	}
+}
+
 // TestResourceAckHappyPath tests the happy path for a typical ConfirmAttachmentMessage and confirms expected
 // ACK request is made
 func TestResourceAckHappyPath(t *testing.T) {

ecs-agent/api/resource/ebs_discovery.go

@@ -28,6 +28,8 @@ const (
 )
 
 var (
+	// When confirming an EBS volume is attached to a host, if the expected volume ID does not
+	// match the volume ID found on the host, this error is returned.
 	ErrInvalidVolumeID = errors.New("EBS volume IDs do not match")
 )