Skip to content

Commit

Permalink
adding filesystem type validation
Browse files Browse the repository at this point in the history
  • Loading branch information
mye956 committed Oct 10, 2023
1 parent a294fcb commit fc6760d
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 7 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions ecs-agent/acs/session/attach_resource_responder.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,10 @@ func validateAttachmentAndReturnProperties(message *ecsacs.ConfirmAttachmentMess
if err != nil {
return nil, errors.Wrap(err, "resource attachment validation by attachment type failed")
}
err = resource.ValidateFileSystemType(attachmentProperties[resource.FileSystemKey])
if err != nil {
return nil, errors.Wrap(err, "resource attachment validation by attachment type failed")
}
return attachmentProperties, nil
}

Expand Down
27 changes: 21 additions & 6 deletions ecs-agent/acs/session/attach_resource_responder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ var (
Name: aws.String(resource.DeviceNameKey),
Value: aws.String("device1"),
},
{
Name: aws.String(resource.FileSystemKey),
Value: aws.String(""),
},
}

testAttachmentProperties = []*ecsacs.AttachmentProperty{
Expand Down Expand Up @@ -202,12 +206,6 @@ func testValidateAttachmentAndReturnPropertiesWithAttachmentType(t *testing.T) {
_, 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
})
}

Expand Down Expand Up @@ -244,6 +242,23 @@ func testValidateAttachmentAndReturnPropertiesWithAttachmentType(t *testing.T) {
}
require.True(t, verified, "Missing required property: %s", requiredProperty)
}

for _, property := range confirmAttachmentMessageCopy.Attachment.AttachmentProperties {
if aws.StringValue(property.Name) == resource.FileSystemKey {
originalPropertyValue := property.Value
property.Value = aws.String("SomeFilesystemType")
_, err = validateAttachmentAndReturnProperties(&confirmAttachmentMessageCopy)
require.Error(t, err)
property.Value = originalPropertyValue

originalPropertyValue = property.Value
property.Value = aws.String("")
_, err = validateAttachmentAndReturnProperties(&confirmAttachmentMessageCopy)
require.Error(t, err)
property.Value = originalPropertyValue
}
}

}

// TestResourceAckHappyPath tests the happy path for a typical ConfirmAttachmentMessage and confirms expected
Expand Down
2 changes: 1 addition & 1 deletion ecs-agent/api/resource/ebs_discovery_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (api *EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID s
err = fmt.Errorf("%w; failed to run lsblk %v", err, string(output))
return "", err
}
logger.Debug(fmt.Sprintf("lsblk output: %s", string(output)))
// logger.Debug(fmt.Sprintf("lsblk output: %s", string(output)))
err = json.Unmarshal(output, &lsblkOut)
if err != nil {
err = fmt.Errorf("%w; failed to unmarshal string: %v", err, string(output))
Expand Down
10 changes: 10 additions & 0 deletions ecs-agent/api/resource/resource_attachment.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ const (
FileSystemKey = "fileSystem"
)

var (
AllowedFSTypes = map[string]bool{
"xfs": true,
"ext2": true,
"ext3": true,
"ext4": true,
"ntfs": true,
}
)

// getCommonProperties returns the common properties as used for validating a resource.
func getCommonProperties() (commonProperties []string) {
commonProperties = []string{
Expand Down
7 changes: 7 additions & 0 deletions ecs-agent/api/resource/resource_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,10 @@ func ValidateRequiredProperties(actualProperties map[string]string, requiredProp
}
return nil
}

func ValidateFileSystemType(filesystemType string) error {

This comment has been minimized.

Copy link
@xxx0624

xxx0624 Oct 10, 2023

Contributor

Can we have some comments about this optional property?

if !AllowedFSTypes[filesystemType] {

This comment has been minimized.

Copy link
@xxx0624

xxx0624 Oct 10, 2023

Contributor

if fileSystemType is not empty && !AllowedFSTypes[filesystemType] ?

This comment has been minimized.

Copy link
@mye956

mye956 Oct 10, 2023

Author Contributor

yep yep, haven't pushed that change yet :)

return errors.Errorf("invalid file system type: %s", filesystemType)
}
return nil
}

0 comments on commit fc6760d

Please sign in to comment.