feat(file): FORMAT CHANGE: update import id, so it matches the resour…

…ce's format: `<node_name>/<datastore_id>:<content_type>/<file>` (#543) * feat(file): FORMAT CHANGE: update **import id**, so it matches the resource's format: `<node_name>/<datastore_id>:<content_type>/<file>` * better example
bpg · Sep 4, 2023 · 7ace07d · 7ace07d
1 parent 0233053
commit 7ace07d
Show file tree

Hide file tree

Showing 3 changed files with 140 additions and 41 deletions.
diff --git a/docs/resources/virtual_environment_file.md b/docs/resources/virtual_environment_file.md
@@ -99,8 +99,12 @@ created as another temporary file).
 ## Import
 
 Instances can be imported using the `node_name`, `datastore_id`, `content_type`
-and the `file_name`, e.g.,
+and the `file_name` in the following format:
+```
+<node_name>:<datastore_id>/<content_type>/<file_name>
+```
 
+Example:
 ```bash
-$ terraform import proxmox_virtual_environment_file.cloud_config pve/local/snippets/example.cloud-config.yaml
+$ terraform import proxmox_virtual_environment_file.cloud_config pve/local:snippets/example.cloud-config.yaml
 ```
diff --git a/proxmoxtf/resource/file.go b/proxmoxtf/resource/file.go
@@ -205,21 +205,26 @@ func File() *schema.Resource {
 		UpdateContext: fileUpdate,
 		Importer: &schema.ResourceImporter{
 			StateContext: func(ctx context.Context, d *schema.ResourceData, i interface{}) ([]*schema.ResourceData, error) {
-				node, datastore, volumeID, err := fileParseImportID(d.Id())
+				node, volID, err := fileParseImportID(d.Id())
 				if err != nil {
 					return nil, err
 				}
 
-				d.SetId(volumeID)
+				d.SetId(volID.String())
 
 				err = d.Set(mkResourceVirtualEnvironmentFileNodeName, node)
 				if err != nil {
-					return nil, fmt.Errorf("failed setting state during import: %w", err)
+					return nil, fmt.Errorf("failed setting 'node_name' in state during import: %w", err)
 				}
 
-				err = d.Set(mkResourceVirtualEnvironmentFileDatastoreID, datastore)
+				err = d.Set(mkResourceVirtualEnvironmentFileDatastoreID, volID.datastoreID)
 				if err != nil {
-					return nil, fmt.Errorf("failed setting state during import: %w", err)
+					return nil, fmt.Errorf("failed setting 'datastore_id' in state during import: %w", err)
+				}
+
+				err = d.Set(mkResourceVirtualEnvironmentFileContentType, volID.contentType)
+				if err != nil {
+					return nil, fmt.Errorf("failed setting 'content_type' in state during import: %w", err)
 				}
 
 				return []*schema.ResourceData{d}, nil
@@ -228,14 +233,59 @@ func File() *schema.Resource {
 	}
 }
 
-func fileParseImportID(id string) (string, string, string, error) {
-	parts := strings.SplitN(id, "/", 4)
+type fileVolumeID struct {
+	datastoreID string
+	contentType string
+	fileName    string
+}
+
+func (v fileVolumeID) String() string {
+	return fmt.Sprintf("%s:%s/%s", v.datastoreID, v.contentType, v.fileName)
+}
+
+// fileParseVolumeID parses a volume ID in the format datastore_id:content_type/file_name.
+func fileParseVolumeID(id string) (fileVolumeID, error) {
+	parts := strings.SplitN(id, ":", 2)
+
+	if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
+		return fileVolumeID{}, fmt.Errorf("unexpected format of ID (%s), expected datastore_id:content_type/file_name", id)
+	}
+
+	datastoreID := parts[0]
+
+	parts = strings.SplitN(parts[1], "/", 2)
+
+	if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
+		return fileVolumeID{}, fmt.Errorf("unexpected format of ID (%s), expected datastore_id:content_type/file_name", id)
+	}
+
+	contentType := parts[0]
+	fileName := parts[1]
+
+	return fileVolumeID{
+		datastoreID: datastoreID,
+		contentType: contentType,
+		fileName:    fileName,
+	}, nil
+}
+
+// fileParseImportID parses an import ID in the format node/datastore_id:content_type/file_name.
+func fileParseImportID(id string) (string, fileVolumeID, error) {
+	parts := strings.SplitN(id, "/", 2)
 
-	if len(parts) != 4 || parts[0] == "" || parts[1] == "" || parts[2] == "" || parts[3] == "" {
-		return "", "", "", fmt.Errorf("unexpected format of ID (%s), expected node/datastore_id/content_type/file_name", id)
+	if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
+		return "", fileVolumeID{},
+			fmt.Errorf("unexpected format of ID (%s), expected node/datastore_id:content_type/file_name", id)
 	}
 
-	return parts[0], parts[1], strings.Join(parts[2:], "/"), nil
+	node := parts[0]
+
+	volID, err := fileParseVolumeID(parts[1])
+	if err != nil {
+		return "", fileVolumeID{}, err
+	}
+
+	return node, volID, nil
 }
 
 func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
@@ -471,18 +521,18 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag
 		return diag.FromErr(err)
 	}
 
-	volumeID, di := fileGetVolumeID(d)
+	volID, di := fileGetVolumeID(d)
 	diags = append(diags, di...)
 	if diags.HasError() {
 		return diags
 	}
 
-	d.SetId(*volumeID)
+	d.SetId(volID.String())
 
 	diags = append(diags, fileRead(ctx, d, m)...)
 
 	if d.Id() == "" {
-		diags = append(diags, diag.Errorf("failed to read file from %q", *volumeID)...)
+		diags = append(diags, diag.Errorf("failed to read file from %q", volID.String())...)
 	}
 
 	return diags
@@ -586,18 +636,20 @@ func fileGetFileName(d *schema.ResourceData) (*string, error) {
 	return &sourceFileFileName, nil
 }
 
-func fileGetVolumeID(d *schema.ResourceData) (*string, diag.Diagnostics) {
+func fileGetVolumeID(d *schema.ResourceData) (fileVolumeID, diag.Diagnostics) {
 	fileName, err := fileGetFileName(d)
 	if err != nil {
-		return nil, diag.FromErr(err)
+		return fileVolumeID{}, diag.FromErr(err)
 	}
 
 	datastoreID := d.Get(mkResourceVirtualEnvironmentFileDatastoreID).(string)
 	contentType, diags := fileGetContentType(d)
 
-	volumeID := fmt.Sprintf("%s:%s/%s", datastoreID, *contentType, *fileName)
-
-	return &volumeID, diags
+	return fileVolumeID{
+		datastoreID: datastoreID,
+		contentType: *contentType,
+		fileName:    *fileName,
+	}, diags
 }
 
 func fileIsURL(d *schema.ResourceData) bool {

diff --git a/proxmoxtf/resource/file_test.go b/proxmoxtf/resource/file_test.go
@@ -7,10 +7,10 @@
 package resource
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
-	"github.com/stretchr/testify/require"
 
 	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/test"
 )
@@ -102,40 +102,83 @@ func TestFileSchema(t *testing.T) {
 	})
 }
 
-func Test_fileParseImportID(t *testing.T) {
+func Test_fileParseVolumeID(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		name                string
-		value               string
-		valid               bool
-		expectedNodeName    string
-		expectedDatastoreID string
-		expectedVolumeID    string
+		name    string
+		id      string
+		want    fileVolumeID
+		wantErr bool
 	}{
-		{"empty", "", false, "", "", ""},
-		{"missing slash", "invalid", false, "", "", ""},
-		{"missing parts", "invalid/invalid/invalid", false, "", "", ""},
-		{"valid", "node/datastore_id/content_type/file_name", true, "node", "datastore_id", "content_type/file_name"},
+		{"empty", "", fileVolumeID{}, true},
+		{"missing datastore", "iso/file.ido", fileVolumeID{}, true},
+		{"missing type", "local:/file.ido", fileVolumeID{}, true},
+		{"missing file", "local:iso", fileVolumeID{}, true},
+		{"missing file", "local:iso/", fileVolumeID{}, true},
+		{"valid", "local:iso/file.iso", fileVolumeID{
+			datastoreID: "local",
+			contentType: "iso",
+			fileName:    "file.iso",
+		}, false},
 	}
 
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			require := require.New(t)
+			got, err := fileParseVolumeID(tt.id)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("fileParseVolumeID() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("fileParseVolumeID() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_fileParseImportID(t *testing.T) {
+	t.Parallel()
 
-			nodeName, datastoreID, volumeID, err := fileParseImportID(tt.value)
+	tests := []struct {
+		name    string
+		id      string
+		node    string
+		volID   fileVolumeID
+		wantErr bool
+	}{
+		{"empty", "", "", fileVolumeID{}, true},
+		{"missing node", "local:iso/file.iso", "", fileVolumeID{}, true},
+		{"missing node 2", "/local:iso/file.iso", "", fileVolumeID{}, true},
+		{
+			"valid", "pve/local:iso/file.iso",
+			"pve",
+			fileVolumeID{
+				datastoreID: "local",
+				contentType: "iso",
+				fileName:    "file.iso",
+			},
+			false,
+		},
+	}
 
-			if !tt.valid {
-				require.Error(err)
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			node, volID, err := fileParseImportID(tt.id)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("fileParseImportID() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
-
-			require.Nil(err)
-			require.Equal(tt.expectedNodeName, nodeName)
-			require.Equal(tt.expectedDatastoreID, datastoreID)
-			require.Equal(tt.expectedVolumeID, volumeID)
+			if node != tt.node {
+				t.Errorf("fileParseImportID() got node = %v, want %v", node, tt.node)
+			}
+			if !reflect.DeepEqual(volID, tt.volID) {
+				t.Errorf("fileParseImportID() got volID = %v, want %v", volID, tt.volID)
+			}
 		})
 	}
 }