(feat): General improvements to code base

cmd/ebs-bootstrap.go

@@ -52,6 +52,7 @@ func main() {
 		config.NewResizeThresholdValidator(),
 		config.NewDeviceValidator(lds),
 		config.NewMountPointValidator(),
+		config.NewMountOptionsValidator(),
 		config.NewOwnerValidator(uos),
 	}
 	for _, v := range validators {

configs/ubuntu.yml

@@ -3,7 +3,6 @@ devices:
     fs: xfs
     label: external-vol
     mountPoint: /mnt/app
-    mountOptions: defaults
     group: ubuntu
     user: ubuntu
     permissions: 0644

internal/backend/device.go

@@ -78,7 +78,7 @@ func (db *LinuxDeviceBackend) Mount(bd *model.BlockDevice, target string, option
 }
 
 func (db *LinuxDeviceBackend) Remount(bd *model.BlockDevice, target string, options model.MountOptions) action.Action {
-	return db.Mount(bd, target, options.Remount(true))
+	return db.Mount(bd, target, options.Remount())
 }
 
 func (db *LinuxDeviceBackend) Umount(bd *model.BlockDevice) action.Action {

internal/backend/resize.go

@@ -10,14 +10,17 @@ import (
 )
 
 type BlockDeviceMetrics struct {
-	FileSystemSize  int
-	BlockDeviceSize int
+	FileSystemSize  uint64
+	BlockDeviceSize uint64
 }
 
 func (bdm *BlockDeviceMetrics) ShouldResize(threshold float64) bool {
-	// Minimum File System Size (mfss)
-	mfss := float64(bdm.BlockDeviceSize) * (threshold / 100)
-	return float64(bdm.FileSystemSize) < mfss
+	// Handle edge case for dividing by zero
+	if bdm.BlockDeviceSize == 0 {
+		return false
+	}
+	utilisation := (float64(bdm.FileSystemSize) / float64(bdm.BlockDeviceSize)) * 100
+	return utilisation < threshold
 }
 
 type DeviceResizeBackend interface {

internal/config/config.go

@@ -17,12 +17,14 @@ const (
 	// set to 99.999% to account for occasional overhead that reduces the effective storage
 	// that can be used by the filesystem
 	DefaultResizeThreshold = 99.999
+	DefaultMountOptions    = model.MountOptions("defaults")
 )
 
 type Flag struct {
 	Config          string
 	Mode            string
 	Remount         bool
+	MountOptions    string
 	ResizeFs        bool
 	ResizeThreshold float64
 }
@@ -42,17 +44,19 @@ type Device struct {
 }
 
 type Defaults struct {
-	Mode            model.Mode `yaml:"mode"`
-	Remount         bool       `yaml:"remount"`
-	ResizeFs        bool       `yaml:"resizeFs"`
-	ResizeThreshold float64    `yaml:"resizeThreshold"`
+	Mode            model.Mode         `yaml:"mode"`
+	Remount         bool               `yaml:"remount"`
+	MountOptions    model.MountOptions `yaml:"mountOptions"`
+	ResizeFs        bool               `yaml:"resizeFs"`
+	ResizeThreshold float64            `yaml:"resizeThreshold"`
 }
 
 type Overrides struct {
-	Mode            model.Mode `yaml:"mode"`
-	Remount         bool       `yaml:"remount"`
-	ResizeFs        bool       `yaml:"resizeFs"`
-	ResizeThreshold float64    `yaml:"resizeThreshold"`
+	Mode            model.Mode         `yaml:"mode"`
+	Remount         bool               `yaml:"remount"`
+	MountOptions    model.MountOptions `yaml:"mountOptions"`
+	ResizeFs        bool               `yaml:"resizeFs"`
+	ResizeThreshold float64            `yaml:"resizeThreshold"`
 }
 
 // We don't export "overrides" as this is an attribute that is used
@@ -104,6 +108,7 @@ func parseFlags(program string, args []string) (*Flag, error) {
 	flags.StringVar(&f.Config, "config", "/etc/ebs-bootstrap/config.yml", "path to config file")
 	flags.StringVar(&f.Mode, "mode", "", "override for mode")
 	flags.BoolVar(&f.Remount, "remount", false, "override for remount")
+	flags.StringVar(&f.MountOptions, "mount-options", "", "override for mount options")
 	flags.BoolVar(&f.ResizeFs, "resize-fs", false, "override for resize filesystem")
 	flags.Float64Var(&f.ResizeThreshold, "resize-threshold", 0, "override for resize threshold")
 
@@ -119,6 +124,7 @@ func parseFlags(program string, args []string) (*Flag, error) {
 func (c *Config) setOverrides(f *Flag) *Config {
 	c.overrides.Mode = model.Mode(f.Mode)
 	c.overrides.Remount = f.Remount
+	c.overrides.MountOptions = model.MountOptions(f.MountOptions)
 	c.overrides.ResizeFs = f.ResizeFs
 	c.overrides.ResizeThreshold = f.ResizeThreshold
 	return c
@@ -149,6 +155,23 @@ func (c *Config) GetRemount(name string) bool {
 	return c.overrides.Remount || c.Defaults.Remount || cd.Remount
 }
 
+func (c *Config) GetMountOptions(name string) model.MountOptions {
+	cd, found := c.Devices[name]
+	if !found {
+		return DefaultMountOptions
+	}
+	if len(c.overrides.MountOptions) != 0 {
+		return c.overrides.MountOptions
+	}
+	if len(cd.MountOptions) != 0 {
+		return cd.MountOptions
+	}
+	if len(c.Defaults.MountOptions) != 0 {
+		return c.Defaults.MountOptions
+	}
+	return DefaultMountOptions
+}
+
 func (c *Config) GetResizeFs(name string) bool {
 	cd, found := c.Devices[name]
 	if !found {

internal/config/validator.go

@@ -3,6 +3,7 @@ package config
 import (
 	"fmt"
 	"path"
+	"strings"
 
 	"github.com/reecetech/ebs-bootstrap/internal/model"
 	"github.com/reecetech/ebs-bootstrap/internal/service"
@@ -101,6 +102,40 @@ func (apv *MountPointValidator) Validate(c *Config) error {
 	return nil
 }
 
+type MountOptionsValidator struct{}
+
+func NewMountOptionsValidator() *MountOptionsValidator {
+	return &MountOptionsValidator{}
+}
+
+func (mov *MountOptionsValidator) Validate(c *Config) error {
+	mo := string(c.Defaults.MountOptions)
+	if err := mov.validate(mo); err != nil {
+		return fmt.Errorf("🔴 '%s' is not a supported global mode as %s", mo, err)
+	}
+	mo = string(c.overrides.MountOptions)
+	if err := mov.validate(mo); err != nil {
+		return fmt.Errorf("🔴 '%s' is not a supported overrides mode as %s", mo, err)
+	}
+	for name, device := range c.Devices {
+		mo := string(device.MountOptions)
+		if err := mov.validate(mo); err != nil {
+			return fmt.Errorf("🔴 %s: '%s' is not a supported mode as %s", name, mo, err)
+		}
+	}
+	return nil
+}
+
+func (mov *MountOptionsValidator) validate(mo string) error {
+	if strings.Contains(mo, "remount") {
+		return fmt.Errorf("it prevents unmounted devices from being mounted")
+	}
+	if strings.Contains(mo, "bind") {
+		return fmt.Errorf("bind mounts are not supported for block devices")
+	}
+	return nil
+}
+
 type OwnerValidator struct {
 	ownerService service.OwnerService
 }
@@ -137,10 +172,10 @@ func NewResizeThresholdValidator() *ResizeThresholdValidator {
 
 func (rtv *ResizeThresholdValidator) Validate(c *Config) error {
 	if c.Defaults.ResizeThreshold < 0 || c.Defaults.ResizeThreshold > 100 {
-		return fmt.Errorf("🔴 '%g' must be a floating point between 0 and 100", c.Defaults.ResizeThreshold)
+		return fmt.Errorf("🔴 '%g' (default) must be a floating point between 0 and 100", c.Defaults.ResizeThreshold)
 	}
 	if c.overrides.ResizeThreshold < 0 || c.overrides.ResizeThreshold > 100 {
-		return fmt.Errorf("🔴 '%g' must be a floating point between 0 and 100", c.overrides.ResizeThreshold)
+		return fmt.Errorf("🔴 '%g' (overrides) must be a floating point between 0 and 100", c.overrides.ResizeThreshold)
 	}
 	for name, device := range c.Devices {
 		if device.ResizeThreshold < 0 || device.ResizeThreshold > 100 {

internal/layer/mount.go

@@ -43,9 +43,10 @@ func (fdl *MountDeviceLayer) Modify(c *config.Config) ([]action.Action, error) {
 			return nil, fmt.Errorf("🔴 %s: Can not mount a device with no file system", bd.Name)
 		}
 		mode := c.GetMode(name)
+		mo := c.GetMountOptions(name)
 		if bd.MountPoint == cd.MountPoint {
 			if c.GetRemount(name) {
-				a := fdl.deviceBackend.Remount(bd, cd.MountPoint, cd.MountOptions).SetMode(mode)
+				a := fdl.deviceBackend.Remount(bd, cd.MountPoint, mo).SetMode(mode)
 				actions = append(actions, a)
 			}
 		} else {
@@ -64,7 +65,7 @@ func (fdl *MountDeviceLayer) Modify(c *config.Config) ([]action.Action, error) {
 				a := fdl.deviceBackend.Umount(bd).SetMode(mode)
 				actions = append(actions, a)
 			}
-			a := fdl.deviceBackend.Mount(bd, cd.MountPoint, cd.MountOptions).SetMode(mode)
+			a := fdl.deviceBackend.Mount(bd, cd.MountPoint, mo).SetMode(mode)
 			actions = append(actions, a)
 		}
 	}

internal/layer/resize.go

@@ -1,8 +1,6 @@
 package layer
 
 import (
-	"fmt"
-
 	"github.com/reecetech/ebs-bootstrap/internal/action"
 	"github.com/reecetech/ebs-bootstrap/internal/backend"
 	"github.com/reecetech/ebs-bootstrap/internal/config"
@@ -50,18 +48,6 @@ func (fdl *ResizeDeviceLayer) Modify(c *config.Config) ([]action.Action, error)
 }
 
 func (fdl *ResizeDeviceLayer) Validate(c *config.Config) error {
-	for name := range c.Devices {
-		if !c.GetResizeFs(name) {
-			continue
-		}
-		metrics, err := fdl.deviceResizeBackend.GetBlockDeviceMetrics(name)
-		if err != nil {
-			return err
-		}
-		if rt := c.GetResizeThreshold(name); metrics.ShouldResize(rt) {
-			return fmt.Errorf("🔴 %s: Failed to resize file system. File System=%d Block Device=%d (bytes)", name, metrics.FileSystemSize, metrics.BlockDeviceSize)
-		}
-	}
 	return nil
 }
 

internal/model/device.go

@@ -15,29 +15,13 @@ type BlockDevice struct {
 
 type MountOptions string
 
-func (mop MountOptions) String() string {
-	if mop == "" {
-		return "defaults"
-	}
-	return string(mop)
-}
-
-func (mop MountOptions) Remount(isEnabled bool) MountOptions {
-	mops := strings.Split(mop.String(), ",")
+func (mop MountOptions) Remount() MountOptions {
+	mops := strings.Split(string(mop), ",")
 	// Return index of remount option (if it exists)
 	// Return -1 if not found
 	index := slices.Index(mops, "remount")
-
-	if isEnabled && index < 0 {
-		// Add remount option if enabled and mount options
-		// do not already contain remount option
+	if index < 0 {
 		mops = append(mops, "remount")
-	} else if !isEnabled && index >= 0 {
-		// Remove remount option if disabled and mount options
-		// already contains remount option
-		copy(mops[index:], mops[index+1:]) // Shift a[i+1:] left one index
-		mops[len(mops)-1] = ""             // Erase last element (write zero value).
-		mops = mops[:len(mops)-1]          // Truncate slice.
 	}
 	return MountOptions(strings.Join(mops, ","))
 }

internal/model/file.go

@@ -26,10 +26,18 @@ type File struct {
 
 type FilePermissions uint32
 
+// It is useful to be able to convert FilePermissions back into the fs.FileMode
+// type which is expected by Go standard libraries
 func (p FilePermissions) Perm() fs.FileMode {
 	return fs.FileMode(p)
 }
 
+// Linux File Permission bits are typically represented as octals: e.g 0755.
+// Some users may feel comfortable representing file permission bits as decimals:
+// e.g 755. While the latter is not considered an octal, lets not punish them
+// for a behaviour that has been ingrained by tools like chmod.
+// `strconv.ParseUint` has the ability to force the intepreation of a string as a base-8
+// unsigned integer
 func (p *FilePermissions) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	var ps string
 	if err := unmarshal(&ps); err != nil {

internal/service/device.go

@@ -12,7 +12,7 @@ import (
 // Device Service Interface [START]
 
 type DeviceService interface {
-	GetSize(name string) (int, error) // bytes
+	GetSize(name string) (uint64, error) // bytes
 	GetBlockDevices() ([]string, error)
 	GetBlockDevice(name string) (*model.BlockDevice, error)
 	Mount(source string, target string, fs model.FileSystem, options model.MountOptions) error
@@ -43,15 +43,15 @@ func NewLinuxDeviceService(rc utils.RunnerFactory) *LinuxDeviceService {
 	}
 }
 
-func (du *LinuxDeviceService) GetSize(name string) (int, error) {
+func (du *LinuxDeviceService) GetSize(name string) (uint64, error) {
 	r := du.RunnerFactory.Select(utils.BlockDev)
 	output, err := r.Command("--getsize64", name)
 	if err != nil {
-		return -1, err
+		return 0, err
 	}
-	b, err := strconv.Atoi(output)
+	b, err := strconv.ParseUint(output, 10, 64)
 	if err != nil {
-		return 0, nil
+		return 0, fmt.Errorf("🔴 Failed to cast block device size to unsigned 64-bit integer")
 	}
 	return b, nil
 }
@@ -103,7 +103,7 @@ func (du *LinuxDeviceService) GetBlockDevice(name string) (*model.BlockDevice, e
 
 func (du *LinuxDeviceService) Mount(source string, target string, fs model.FileSystem, options model.MountOptions) error {
 	r := du.RunnerFactory.Select(utils.Mount)
-	_, err := r.Command(source, "-t", string(fs), "-o", options.String(), target)
+	_, err := r.Command(source, "-t", string(fs), "-o", string(options), target)
 	if err != nil {
 		return err
 	}