🧹 Cleanup unused block devices code, fix bool flag, extract detection (…

…#4740) * 🧹 Cleanup unused block devices code, fix bool flag, extract detection logic in a fn. Signed-off-by: Preslav <[email protected]> * tests for partitions. Signed-off-by: Preslav <[email protected]> * always specify device name for aws/gcp. Signed-off-by: Preslav <[email protected]> * Fix device connection. Signed-off-by: Preslav <[email protected]> --------- Signed-off-by: Preslav <[email protected]>
mondoohq · Oct 29, 2024 · cc0224e · cc0224e
1 parent a478cc3
commit cc0224e
Show file tree

Hide file tree

Showing 11 changed files with 255 additions and 568 deletions.
diff --git a/providers/aws/connection/awsec2ebsconn/provider.go b/providers/aws/connection/awsec2ebsconn/provider.go
@@ -156,6 +156,8 @@ func NewAwsEbsConnection(id uint32, conf *inventory.Config, asset *inventory.Ass
 	asset.PlatformIds = []string{conf.PlatformId}
 	c.deviceLocation = volLocation
 
+	// this indicates to the device connection which device to mount
+	conf.Options["device-name"] = c.deviceLocation
 	deviceConn, err := device.NewDeviceConnection(id, &inventory.Config{
 		Type:       "device",
 		PlatformId: conf.PlatformId,
@@ -190,11 +192,6 @@ func (c *AwsEbsConnection) Close() {
 	if c.DeviceProvider != nil {
 		c.DeviceProvider.Close()
 	}
-	// we seem to be losing all the connection info we
-	// had when we started by the time we get here.
-	// we should figure out what is happening
-	// for now, reassemble the info needed from the asset
-	// connection options
 	ctx := context.Background()
 	err := c.DetachVolumeFromInstance(ctx, c.scanVolumeInfo)
 	if err != nil {

diff --git a/providers/gcp/connection/gcpinstancesnapshot/provider.go b/providers/gcp/connection/gcpinstancesnapshot/provider.go
@@ -116,7 +116,7 @@ func NewGcpSnapshotConnection(id uint32, conf *inventory.Config, asset *inventor
 	// setup disk image so and attach it to the instance
 	var diskUrl string
 	mi := mountInfo{
-		deviceName: "cnspec",
+		deviceName: "/dev/sdh",
 	}
 	switch target.TargetType {
 	case "instance":
@@ -191,7 +191,8 @@ func NewGcpSnapshotConnection(id uint32, conf *inventory.Config, asset *inventor
 	if err != nil {
 		return nil, err
 	}
-
+	// this indicates to the device connection which device to mount
+	conf.Options["device-name"] = mi.deviceName
 	errorHandler := func() {
 		// use different err variable to ensure it does not overshadow the real error
 		dErr := sc.detachDisk(scanner.projectID, scanner.zone, scanner.instanceName, mi.deviceName)

diff --git a/providers/os/config/config.go b/providers/os/config/config.go
@@ -290,7 +290,7 @@ var Config = plugin.Provider{
 				},
 				{
 					Long:   "mount-all-partitions",
-					Type:   plugin.FlagType_String,
+					Type:   plugin.FlagType_Bool,
 					Desc:   "Mount all partitions of the block device",
 					Option: plugin.FlagOption_Hidden,
 				},

diff --git a/providers/os/connection/device/device_connection.go b/providers/os/connection/device/device_connection.go
@@ -14,6 +14,7 @@ import (
 	"go.mondoo.com/cnquery/v11/providers-sdk/v1/plugin"
 	"go.mondoo.com/cnquery/v11/providers/os/connection/device/linux"
 	"go.mondoo.com/cnquery/v11/providers/os/connection/device/windows"
+	"go.mondoo.com/cnquery/v11/providers/os/connection/snapshot"
 	"go.mondoo.com/cnquery/v11/utils/stringx"
 
 	"go.mondoo.com/cnquery/v11/providers/os/connection/fs"
@@ -58,7 +59,7 @@ func NewDeviceConnection(connId uint32, conf *inventory.Config, asset *inventory
 		return nil, err
 	}
 	if len(blocks) == 0 {
-		return nil, errors.New("internal> no blocks found")
+		return nil, errors.New("device connection> internal: blocks found")
 	}
 
 	res := &DeviceConnection{
@@ -71,82 +72,28 @@ func NewDeviceConnection(connId uint32, conf *inventory.Config, asset *inventory
 		conf.Options = make(map[string]string)
 	}
 
+	if len(asset.IdDetector) == 0 {
+		asset.IdDetector = []string{ids.IdDetector_Hostname, ids.IdDetector_SshHostkey}
+	}
+
 	// we iterate over all the blocks and try to run OS detection on each one of them
 	// we only return one asset, if we find the right block (e.g. the one with the root FS)
-	for i := range blocks {
-		block := blocks[i]
-		log.Debug().Str("name", block.Name).Str("type", block.FsType).Msg("identified partition for mounting")
-
-		scanDir, err := manager.Mount(block)
-		if err != nil {
-			log.Error().Err(err).Msg("unable to complete mount step")
-			res.Close()
-			return nil, err
+	for _, block := range blocks {
+		fsConn, scanDir, err := tryDetectAsset(connId, block, manager, conf, asset)
+		if scanDir != "" {
+			res.MountedDirs = append(res.MountedDirs, scanDir)
 		}
-		res.MountedDirs = append(res.MountedDirs, scanDir)
-
-		// create and initialize fs provider
-		conf.Options["path"] = scanDir
-		fsConn, err := fs.NewConnection(connId, &inventory.Config{
-			Path:       scanDir,
-			PlatformId: conf.PlatformId,
-			Options:    conf.Options,
-			Type:       "fs",
-			Record:     conf.Record,
-		}, asset)
 		if err != nil {
-			res.Close()
-			return nil, err
-		}
-
-		if asset.Platform != nil {
-			log.Debug().Msg("device connection> platform already detected")
-			// Edge case: asset platform is provided from the inventory
-			if res.FileSystemConnection == nil {
-				res.FileSystemConnection = fsConn
-			}
-			continue
-		}
-
-		p, ok := detector.DetectOS(fsConn)
-		if !ok {
-			log.Debug().
-				Str("block", block.Name).
-				Msg("device connection> cannot detect os")
-			continue
-		}
-
-		if len(asset.IdDetector) == 0 {
-			asset.IdDetector = []string{ids.IdDetector_Hostname, ids.IdDetector_SshHostkey}
-		}
-		res.FileSystemConnection = fsConn
-
-		fingerprint, p, err := id.IdentifyPlatform(fsConn, &plugin.ConnectReq{}, p, asset.IdDetector)
-		if err != nil {
-			log.Debug().Err(err).Msg("device connection> failed to identify platform from device")
-			asset.Platform = nil
-		}
-
-		if err == nil {
-			log.Debug().Str("scan_dir", scanDir).Msg("device connection> detected platform from device")
-			if asset.Name == "" {
-				asset.Name = fingerprint.Name
-			}
-			asset.PlatformIds = append(asset.PlatformIds, fingerprint.PlatformIDs...)
-			asset.IdDetector = fingerprint.ActiveIdDetectors
-			asset.Platform = p
-			asset.Id = conf.Type
+			log.Error().Err(err).Msg("partition did not return an asset, continuing")
+		} else {
+			res.FileSystemConnection = fsConn
 		}
 	}
 
+	// if none of the blocks returned a platform that we could detect, we return an error
 	if asset.Platform == nil {
 		res.Close()
-		return nil, errors.New("failed to detect OS")
-	}
-
-	if res.FileSystemConnection == nil {
-		res.Close()
-		return nil, errors.New("failed to create fs connection")
+		return nil, errors.New("device connection> no platform detected")
 	}
 
 	// allow injecting platform ids into the device connection. we cannot always know the asset that's being scanned, e.g.
@@ -210,3 +157,51 @@ func (p *DeviceConnection) FileInfo(path string) (shared.FileInfoDetails, error)
 func (p *DeviceConnection) Conf() *inventory.Config {
 	return p.FileSystemConnection.Conf
 }
+
+// tryDetectAsset tries to detect the OS on a given block device
+func tryDetectAsset(connId uint32, partition *snapshot.PartitionInfo, manager DeviceManager, conf *inventory.Config, asset *inventory.Asset) (*fs.FileSystemConnection, string, error) {
+	log.Debug().Str("name", partition.Name).Str("type", partition.FsType).Msg("mounting partition")
+	scanDir, err := manager.Mount(partition)
+	if err != nil {
+		log.Error().Err(err).Msg("unable to complete mount step")
+		return nil, "", err
+	}
+
+	// create and initialize fs provider
+	conf.Options["path"] = scanDir
+	fsConn, err := fs.NewConnection(connId, &inventory.Config{
+		Path:       scanDir,
+		PlatformId: conf.PlatformId,
+		Options:    conf.Options,
+		Type:       "fs",
+		Record:     conf.Record,
+	}, asset)
+	if err != nil {
+		return nil, scanDir, err
+	}
+
+	p, ok := detector.DetectOS(fsConn)
+	if !ok {
+		log.Debug().
+			Str("partition", partition.Name).
+			Msg("device connection> cannot detect os")
+		return nil, scanDir, errors.New("cannot detect os")
+	}
+
+	fingerprint, p, err := id.IdentifyPlatform(fsConn, &plugin.ConnectReq{}, p, asset.IdDetector)
+	if err != nil {
+		log.Debug().Err(err).Msg("device connection> failed to identify platform from device")
+		return nil, scanDir, err
+	}
+	log.Debug().Str("scan_dir", scanDir).Msg("device connection> detected platform from device")
+	asset.Platform = p
+	if asset.Name == "" {
+		asset.Name = fingerprint.Name
+	}
+	asset.PlatformIds = append(asset.PlatformIds, fingerprint.PlatformIDs...)
+	asset.IdDetector = fingerprint.ActiveIdDetectors
+	asset.Platform = p
+	asset.Id = conf.Type
+
+	return fsConn, scanDir, nil
+}
diff --git a/providers/os/connection/device/linux/device_manager.go b/providers/os/connection/device/linux/device_manager.go
@@ -91,6 +91,11 @@ func validateOpts(opts map[string]string) error {
 	if lun != "" && deviceName != "" {
 		return errors.New("both lun and device name provided")
 	}
+
+	if lun == "" && deviceName == "" {
+		return errors.New("either lun or device name must be provided")
+	}
+
 	if deviceName == "" && mountAll {
 		return errors.New("mount-all-partitions requires a device name")
 	}
@@ -134,18 +139,6 @@ func (c *LinuxDeviceManager) identifyViaDeviceName(deviceName string, mountAll b
 		return nil, err
 	}
 
-	// if we don't have a device name we can just return the first non-boot, non-mounted partition.
-	// this is a best-guess approach
-	if deviceName == "" {
-		// TODO: we should rename/simplify this method
-		pi, err := blockDevices.GetUnnamedBlockEntry()
-		if err != nil {
-			return nil, err
-		}
-		return []*snapshot.PartitionInfo{pi}, nil
-	}
-
-	// if we have a specific device we're looking for we can just ask only for that
 	device, err := blockDevices.FindDevice(deviceName)
 	if err != nil {
 		return nil, err