Merge pull request #33 from fabi200123/add-new-extra-specs

Add DiskType, DisplayDevice, ServiceAccounts extra-specs options

README.md

@@ -104,10 +104,18 @@ To this end, this provider supports the following extra specs schema:
     "type": "object",
     "description": "Schema defining supported extra specs for the Garm GCP Provider",
     "properties": {
+        "display_device": {
+            "type": "boolean",
+            "description": "Enable the display device on the VM."
+        },
         "disksize": {
             "type": "integer",
             "description": "The size of the root disk in GB. Default is 127 GB."
         },
+        "disktype": {
+            "type": "string",
+            "description": "The type of the disk. Default is pd-standard."
+        },
         "network_id": {
             "type": "string",
             "description": "The name of the network attached to the instance."
@@ -134,6 +142,13 @@ To this end, this provider supports the following extra specs schema:
                 "type": "string"
             }
         },
+        "service_accounts": {
+            "type": "array",
+            "description": "A list of service accounts to be attached to the instance",
+            "items": {
+                "$ref": "#/$defs/ServiceAccount"
+            }
+        },
         "source_snapshot": {
             "type": "string",
             "description": "The source snapshot to create this disk."
@@ -169,17 +184,22 @@ An example of extra specs json would look like this:
 
 ```bash
 {
+    "display_device": true,
     "disksize": 255,
+    "disktype": "projects/garm-testing/zones/europe-west1/diskTypes/pd-ssd",
     "network_id": "projects/garm-testing/global/networks/garm-2",
     "subnetwork_id": "projects/garm-testing/regions/europe-west1/subnetworks/garm",
     "nic_type": "VIRTIO_NET",
     "custom_labels": {"environment":"production","project":"myproject"},
     "network_tags": ["web-server", "production"],
+    "service_accounts": [{"email":"[email protected]", "scopes":["https://www.googleapis.com/auth/devstorage.read_only", "https://www.googleapis.com/auth/logging.write"]}],
     "source_snapshot": "projects/garm-testing/global/snapshots/garm-snapshot",
     "ssh_keys": ["username1:ssh_key1", "username2:ssh_key2"]
 }
 ```
 
+**NOTE**: Using the `service_accounts` extra specs when creating instances **introduces certain risks that must be carefully managed**. **Service accounts** grant access to specific resources, and if improperly configured, they can expose sensitive data or allow unauthorized actions. Misconfigured permissions or overly broad scopes can lead to privilege escalation, enabling attackers or unintended users to access critical resources. It's essential to follow the principle of least privilege, ensuring that service accounts only have the necessary permissions for their intended tasks. Regular audits and proper key management are also crucial to safeguard access and prevent potential security vulnerabilities.
+
 **NOTE**: The `custom_labels` and `network_tags` must meet the [GCP requirements for labels](https://cloud.google.com/compute/docs/labeling-resources#requirements) and the [GCP requirements for network tags](https://cloud.google.com/vpc/docs/add-remove-network-tags#restrictions)!
 
 **NOTE**: The `ssh_keys` add the option to [connect to an instance via SSH](https://cloud.google.com/compute/docs/instances/ssh) (either Linux or Windows). After you added the key as `username:ssh_public_key`, you can use the `private_key` to connect to the Linux/Windows instance via `ssh -i private_rsa username@instance_ip`. For **Windows** instances, the provider installs on the instance `google-compute-engine-ssh` and `enables ssh` if a `ssh_key` is added to extra-specs.

internal/client/gcp.go

@@ -135,7 +135,10 @@ func (g *GcpCli) CreateInstance(ctx context.Context, spec *spec.RunnerSpec) (*co
 	inst := &computepb.Instance{
 		Name:        proto.String(name),
 		MachineType: proto.String(util.GetMachineType(g.cfg.Zone, spec.BootstrapParams.Flavor)),
-		Disks:       generateBootDisk(spec.DiskSize, spec.BootstrapParams.Image, spec.SourceSnapshot),
+		Disks:       generateBootDisk(spec.DiskSize, spec.BootstrapParams.Image, spec.SourceSnapshot, spec.DiskType, spec.CustomLabels),
+		DisplayDevice: &computepb.DisplayDevice{
+			EnableDisplay: proto.Bool(spec.DisplayDevice),
+		},
 		NetworkInterfaces: []*computepb.NetworkInterface{
 			{
 				Network: proto.String(g.cfg.NetworkID),
@@ -169,6 +172,7 @@ func (g *GcpCli) CreateInstance(ctx context.Context, spec *spec.RunnerSpec) (*co
 		Tags: &computepb.Tags{
 			Items: spec.NetworkTags,
 		},
+		ServiceAccounts: spec.ServiceAccounts,
 	}
 
 	if !g.cfg.ExternalIPAccess {
@@ -314,19 +318,24 @@ func selectStartupScript(osType params.OSType) string {
 	}
 }
 
-func generateBootDisk(diskSize int64, image, snapshot string) []*computepb.AttachedDisk {
+func generateBootDisk(diskSize int64, image, snapshot string, diskType string, customLabels map[string]string) []*computepb.AttachedDisk {
 	disk := []*computepb.AttachedDisk{
 		{
 			Boot: proto.Bool(true),
 			InitializeParams: &computepb.AttachedDiskInitializeParams{
 				DiskSizeGb:     proto.Int64(diskSize),
+				Labels:         customLabels,
 				SourceImage:    proto.String(image),
 				SourceSnapshot: proto.String(snapshot),
 			},
 			AutoDelete: proto.Bool(true),
 		},
 	}
 
+	if diskType != "" {
+		disk[0].InitializeParams.DiskType = proto.String(diskType)
+	}
+
 	if snapshot != "" {
 		disk[0].InitializeParams.SourceImage = nil
 	}

internal/spec/spec.go

@@ -21,6 +21,7 @@ import (
 	"maps"
 	"regexp"
 
+	"cloud.google.com/go/compute/apiv1/computepb"
 	"github.com/cloudbase/garm-provider-common/cloudconfig"
 	"github.com/cloudbase/garm-provider-common/params"
 	"github.com/cloudbase/garm-provider-common/util"
@@ -127,15 +128,18 @@ func (e *extraSpecs) Validate() error {
 }
 
 type extraSpecs struct {
-	DiskSize        int64             `json:"disksize,omitempty" jsonschema:"description=The size of the root disk in GB. Default is 127 GB."`
-	NetworkID       string            `json:"network_id,omitempty" jsonschema:"description=The name of the network attached to the instance."`
-	SubnetworkID    string            `json:"subnetwork_id,omitempty" jsonschema:"description=The name of the subnetwork attached to the instance."`
-	NicType         string            `json:"nic_type,omitempty" jsonschema:"description=The type of the network interface card. Default is VIRTIO_NET."`
-	CustomLabels    map[string]string `json:"custom_labels,omitempty" jsonschema:"description=Custom labels to apply to the instance. Each label is a key-value pair where both key and value are strings."`
-	NetworkTags     []string          `json:"network_tags,omitempty" jsonschema:"description=A list of network tags to be attached to the instance"`
-	SourceSnapshot  string            `json:"source_snapshot,omitempty" jsonschema:"description=The source snapshot to create this disk."`
-	SSHKeys         []string          `json:"ssh_keys,omitempty" jsonschema:"description=A list of SSH keys to be added to the instance. The format is USERNAME:SSH_KEY"`
-	EnableBootDebug *bool             `json:"enable_boot_debug,omitempty" jsonschema:"description=Enable boot debug on the VM."`
+	DiskSize        int64                       `json:"disksize,omitempty" jsonschema:"description=The size of the root disk in GB. Default is 127 GB."`
+	DiskType        string                      `json:"disktype,omitempty" jsonschema:"description=The type of the disk. Default is pd-standard."`
+	DisplayDevice   bool                        `json:"display_device,omitempty" jsonschema:"description=Enable the display device on the VM."`
+	NetworkID       string                      `json:"network_id,omitempty" jsonschema:"description=The name of the network attached to the instance."`
+	SubnetworkID    string                      `json:"subnetwork_id,omitempty" jsonschema:"description=The name of the subnetwork attached to the instance."`
+	NicType         string                      `json:"nic_type,omitempty" jsonschema:"description=The type of the network interface card. Default is VIRTIO_NET."`
+	CustomLabels    map[string]string           `json:"custom_labels,omitempty" jsonschema:"description=Custom labels to apply to the instance. Each label is a key-value pair where both key and value are strings."`
+	NetworkTags     []string                    `json:"network_tags,omitempty" jsonschema:"description=A list of network tags to be attached to the instance"`
+	ServiceAccounts []*computepb.ServiceAccount `json:"service_accounts,omitempty" jsonschema:"description=A list of service accounts to be attached to the instance"`
+	SourceSnapshot  string                      `json:"source_snapshot,omitempty" jsonschema:"description=The source snapshot to create this disk."`
+	SSHKeys         []string                    `json:"ssh_keys,omitempty" jsonschema:"description=A list of SSH keys to be added to the instance. The format is USERNAME:SSH_KEY"`
+	EnableBootDebug *bool                       `json:"enable_boot_debug,omitempty" jsonschema:"description=Enable boot debug on the VM."`
 	// The Cloudconfig struct from common package
 	cloudconfig.CloudConfigSpec
 }
@@ -182,9 +186,12 @@ type RunnerSpec struct {
 	SubnetworkID    string
 	ControllerID    string
 	NicType         string
+	DisplayDevice   bool
 	DiskSize        int64
+	DiskType        string
 	CustomLabels    map[string]string
 	NetworkTags     []string
+	ServiceAccounts []*computepb.ServiceAccount
 	SourceSnapshot  string
 	SSHKeys         string
 	EnableBootDebug bool
@@ -197,9 +204,15 @@ func (r *RunnerSpec) MergeExtraSpecs(extraSpecs *extraSpecs) {
 	if extraSpecs.SubnetworkID != "" {
 		r.SubnetworkID = extraSpecs.SubnetworkID
 	}
+	if extraSpecs.DisplayDevice {
+		r.DisplayDevice = extraSpecs.DisplayDevice
+	}
 	if extraSpecs.DiskSize > 0 {
 		r.DiskSize = extraSpecs.DiskSize
 	}
+	if extraSpecs.DiskType != "" {
+		r.DiskType = extraSpecs.DiskType
+	}
 	if extraSpecs.NicType != "" {
 		r.NicType = extraSpecs.NicType
 	}
@@ -209,6 +222,9 @@ func (r *RunnerSpec) MergeExtraSpecs(extraSpecs *extraSpecs) {
 	if len(extraSpecs.NetworkTags) > 0 {
 		r.NetworkTags = extraSpecs.NetworkTags
 	}
+	if len(extraSpecs.ServiceAccounts) > 0 {
+		r.ServiceAccounts = extraSpecs.ServiceAccounts
+	}
 	if extraSpecs.SourceSnapshot != "" {
 		r.SourceSnapshot = extraSpecs.SourceSnapshot
 	}

internal/spec/spec_test.go

@@ -20,8 +20,10 @@ import (
 	"fmt"
 	"testing"
 
+	"cloud.google.com/go/compute/apiv1/computepb"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/proto"
 )
 
 func TestJsonSchemaValidation(t *testing.T) {
@@ -33,28 +35,46 @@ func TestJsonSchemaValidation(t *testing.T) {
 		{
 			name: "Full specs",
 			input: json.RawMessage(`{
+				"display_device": true,
 				"disksize": 127,
+				"disktype": "pd-ssd",
 				"network_id": "default",
 				"subnetwork_id": "default",
 				"nic_type": "VIRTIO_NET",
 				"custom_labels": {
 					"example_label": "example_value"
 				},
 				"network_tags": ["example_tag"],
+				"service_accounts": [{"email": "email", "scopes": ["scope"]}],
+				"service_accounts": [{"email": "email", "scopes": ["scope", "scope2"]}, {"email": "email2", "scopes": ["scope2"]}],
 				"source_snapshot": "snapshot-id",
 				"ssh_keys": ["ssh-key", "ssh-key2"],
 				"enable_boot_debug": true,
 				"runner_install_template": "IyEvYmluL2Jhc2gKZWNobyBJbnN0YWxsaW5nIHJ1bm5lci4uLg==", "pre_install_scripts": {"setup.sh": "IyEvYmluL2Jhc2gKZWNobyBTZXR1cCBzY3JpcHQuLi4="}, "extra_context": {"key": "value"}
 				}`),
 			errString: "",
 		},
+		{
+			name: "Specs just with display_device",
+			input: json.RawMessage(`{
+				"display_device": true
+			}`),
+			errString: "",
+		},
 		{
 			name: "Specs just with disksize",
 			input: json.RawMessage(`{
 				"disksize": 127
 			}`),
 			errString: "",
 		},
+		{
+			name: "Specs just with disktype",
+			input: json.RawMessage(`{
+				"disktype": "projects/garm-testing/zones/europe-west1/diskTypes/pd-ssd"
+			}`),
+			errString: "",
+		},
 		{
 			name: "Specs just with network_id",
 			input: json.RawMessage(`{
@@ -92,6 +112,13 @@ func TestJsonSchemaValidation(t *testing.T) {
 			}`),
 			errString: "",
 		},
+		{
+			name: "Specs just with service_accounts",
+			input: json.RawMessage(`{
+				"service_accounts": [{"email": "email", "scopes": ["scope"]}]
+			}`),
+			errString: "",
+		},
 		{
 			name: "Specs just with source_snapshot",
 			input: json.RawMessage(`{
@@ -138,13 +165,27 @@ func TestJsonSchemaValidation(t *testing.T) {
 			}`),
 			errString: "",
 		},
+		{
+			name: "Invalid input for display_device - wrong data type",
+			input: json.RawMessage(`{
+				"display_device": "true"
+			}`),
+			errString: "schema validation failed: [display_device: Invalid type. Expected: boolean, given: string]",
+		},
 		{
 			name: "Invalid input for disksize - wrong data type",
 			input: json.RawMessage(`{
 				"disksize": "127"
 			}`),
 			errString: "schema validation failed: [disksize: Invalid type. Expected: integer, given: string]",
 		},
+		{
+			name: "Invalid input for disktype - wrong data type",
+			input: json.RawMessage(`{
+				"disktype": 127
+			}`),
+			errString: "schema validation failed: [disktype: Invalid type. Expected: string, given: integer]",
+		},
 		{
 			name: "Invalid input for nic_type - wrong data type",
 			input: json.RawMessage(`{
@@ -166,6 +207,13 @@ func TestJsonSchemaValidation(t *testing.T) {
 			}`),
 			errString: "schema validation failed: [network_tags: Invalid type. Expected: array, given: string]",
 		},
+		{
+			name: "Invalid input for service_accounts - wrong data type",
+			input: json.RawMessage(`{
+				"service_accounts": "email"
+			}`),
+			errString: "schema validation failed: [service_accounts: Invalid type. Expected: array, given: string]",
+		},
 		{
 			name: "Invalid input for ssh_keys - wrong data type",
 			input: json.RawMessage(`{
@@ -243,12 +291,20 @@ func TestMergeExtraSpecs(t *testing.T) {
 		{
 			name: "ValidExtraSpecs",
 			extraSpecs: &extraSpecs{
-				NetworkID:       "projects/garm-testing/global/networks/garm-2",
-				SubnetworkID:    "projects/garm-testing/regions/europe-west1/subnetworks/garm",
-				DiskSize:        100,
-				NicType:         "VIRTIO_NET",
-				CustomLabels:    map[string]string{"key1": "value1"},
-				NetworkTags:     []string{"tag1", "tag2"},
+				NetworkID:     "projects/garm-testing/global/networks/garm-2",
+				SubnetworkID:  "projects/garm-testing/regions/europe-west1/subnetworks/garm",
+				DisplayDevice: true,
+				DiskSize:      100,
+				DiskType:      "projects/garm-testing/zones/europe-west1/diskTypes/pd-ssd",
+				NicType:       "VIRTIO_NET",
+				CustomLabels:  map[string]string{"key1": "value1"},
+				NetworkTags:   []string{"tag1", "tag2"},
+				ServiceAccounts: []*computepb.ServiceAccount{
+					{
+						Email:  proto.String("email"),
+						Scopes: []string{"scope"},
+					},
+				},
 				SourceSnapshot:  "projects/garm-testing/global/snapshots/garm-snapshot",
 				SSHKeys:         []string{"ssh-key1", "ssh-key2"},
 				EnableBootDebug: &enable_boot_debug,
@@ -265,7 +321,9 @@ func TestMergeExtraSpecs(t *testing.T) {
 			spec := &RunnerSpec{
 				NetworkID:      "default-network",
 				SubnetworkID:   "default-subnetwork",
+				DisplayDevice:  true,
 				DiskSize:       50,
+				DiskType:       "projects/garm-testing/zones/europe-west1/diskTypes/pd-ssd",
 				NicType:        "Standard",
 				CustomLabels:   map[string]string{"key2": "value2"},
 				NetworkTags:    []string{"tag3", "tag4"},
@@ -287,6 +345,11 @@ func TestMergeExtraSpecs(t *testing.T) {
 					assert.Equal(t, tt.extraSpecs.DiskSize, spec.DiskSize, "expected DiskSize to be %d, got %d", tt.extraSpecs.DiskSize, spec.DiskSize)
 				}
 			}
+			if tt.extraSpecs.DiskType != "" {
+				if spec.DiskType != tt.extraSpecs.DiskType {
+					assert.Equal(t, tt.extraSpecs.DiskType, spec.DiskType, "expected DiskType to be %s, got %s", tt.extraSpecs.DiskType, spec.DiskType)
+				}
+			}
 			if tt.extraSpecs.NicType != "" {
 				if spec.NicType != tt.extraSpecs.NicType {
 					assert.Equal(t, tt.extraSpecs.NicType, spec.NicType, "expected NicType to be %s, got %s", tt.extraSpecs.NicType, spec.NicType)