fix(provider): parsing PVE version reported by API (#2115)

Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com>

fwprovider/test/datasource_version_test.go

@@ -10,10 +10,13 @@ package test
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 	"testing"
 
+	"github.com/hashicorp/go-version"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/terraform"
 )
 
 func TestAccDatasourceVersion(t *testing.T) {
@@ -28,17 +31,74 @@ func TestAccDatasourceVersion(t *testing.T) {
 			{
 				Config: `data "proxmox_virtual_environment_version" "test" {}`,
 				Check: resource.ComposeAggregateTestCheckFunc(
-					resource.TestCheckResourceAttr(datasourceName, "release", "8.4"),
-					resource.TestCheckResourceAttrSet(datasourceName, "repository_id"),
-					resource.TestCheckResourceAttrWith(datasourceName, "version", func(value string) error {
-						if strings.HasPrefix(value, "8.4") {
-							return nil
-						}
-						return fmt.Errorf("version %s does not start with 8.4", value)
-					}),
 					resource.TestCheckResourceAttrSet(datasourceName, "id"),
+					resource.TestCheckResourceAttrSet(datasourceName, "repository_id"),
+					resource.TestCheckResourceAttrWith(datasourceName, "release", validateReleaseVersion),
+					resource.TestCheckResourceAttrWith(datasourceName, "version", validateFullVersion),
+					validateVersionReleaseConsistency(datasourceName),
 				),
 			},
 		},
 	})
 }
+
+// validateReleaseVersion validates that the release field matches the expected pattern (e.g., "8.4", "9.0").
+func validateReleaseVersion(value string) error {
+	releasePattern := regexp.MustCompile(`^[0-9]+\.[0-9]+$`)
+	if !releasePattern.MatchString(value) {
+		return fmt.Errorf("release %q does not match expected pattern (major.minor)", value)
+	}
+
+	// Ensure it's at least the minimum supported version
+	releaseVer, err := version.NewVersion(value + ".0") // Add patch version for comparison
+	if err != nil {
+		return fmt.Errorf("failed to parse release version %q: %w", value, err)
+	}
+
+	minVersion := version.Must(version.NewVersion("8.0.0"))
+	if releaseVer.LessThan(minVersion) {
+		return fmt.Errorf("release version %q is below minimum supported version 8.0", value)
+	}
+
+	return nil
+}
+
+// validateFullVersion validates that the version field is a valid semantic version.
+func validateFullVersion(value string) error {
+	if strings.TrimSpace(value) == "" {
+		return fmt.Errorf("version cannot be empty")
+	}
+
+	_, err := version.NewVersion(value)
+	if err != nil {
+		return fmt.Errorf("version %q is not a valid semantic version: %w", value, err)
+	}
+
+	return nil
+}
+
+// validateVersionReleaseConsistency returns a TestCheckFunc that validates version and release consistency.
+func validateVersionReleaseConsistency(resourceName string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[resourceName]
+		if !ok {
+			return fmt.Errorf("resource %s not found", resourceName)
+		}
+
+		release := rs.Primary.Attributes["release"]
+		version := rs.Primary.Attributes["version"]
+
+		if release == "" {
+			return fmt.Errorf("release attribute is empty")
+		}
+		if version == "" {
+			return fmt.Errorf("version attribute is empty")
+		}
+
+		if !strings.HasPrefix(version, release) {
+			return fmt.Errorf("version %q does not start with release %q", version, release)
+		}
+
+		return nil
+	}
+}

proxmox/version/version_types.go

@@ -8,6 +8,7 @@ package version
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/hashicorp/go-version"
 )
@@ -30,8 +31,8 @@ type ProxmoxVersion struct {
 }
 
 func (v *ProxmoxVersion) UnmarshalJSON(data []byte) error {
-	// Unmarshal the version string into a go-version Version object
-	ver, err := version.NewVersion(string(data))
+	// Unmarshal the version string into a go-version Version object, remove wrapping quotes if any
+	ver, err := version.NewVersion(strings.Trim(string(data), "\""))
 	if err != nil {
 		return fmt.Errorf("failed to parse version %q: %w", string(data), err)
 	}

proxmox/version/version_types_test.go

@@ -0,0 +1,221 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+package version
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/hashicorp/go-version"
+)
+
+func TestResponseBody_UnmarshalJSON(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name           string
+		jsonData       string
+		expectedError  bool
+		expectedResult *ResponseBody
+	}{
+		{
+			name: "valid response with version 9.0.4",
+			jsonData: `{
+				"data": {
+					"repoid": "39d8a4de7dfb2c40",
+					"release": "9.0",
+					"version": "9.0.4"
+				}
+			}`,
+			expectedError: false,
+			expectedResult: &ResponseBody{
+				Data: &ResponseData{
+					Console:      "",
+					Release:      "9.0",
+					RepositoryID: "39d8a4de7dfb2c40",
+					Version: ProxmoxVersion{
+						Version: *mustParseVersion("9.0.4"),
+					},
+				},
+			},
+		},
+		{
+			name: "valid response with semantic version",
+			jsonData: `{
+				"data": {
+					"repoid": "test123",
+					"release": "8.2",
+					"version": "8.2.1",
+					"console": "proxmox"
+				}
+			}`,
+			expectedError: false,
+			expectedResult: &ResponseBody{
+				Data: &ResponseData{
+					Console:      "proxmox",
+					Release:      "8.2",
+					RepositoryID: "test123",
+					Version: ProxmoxVersion{
+						Version: *mustParseVersion("8.2.1"),
+					},
+				},
+			},
+		},
+		{
+			name: "invalid version format",
+			jsonData: `{
+				"data": {
+					"repoid": "test123",
+					"release": "9.0",
+					"version": "invalid-version"
+				}
+			}`,
+			expectedError: true,
+		},
+		{
+			name: "missing data field",
+			jsonData: `{
+				"repoid": "test123",
+				"release": "9.0",
+				"version": "9.0.4"
+			}`,
+			expectedError: false,
+			expectedResult: &ResponseBody{
+				Data: nil,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			var result ResponseBody
+
+			err := json.Unmarshal([]byte(tt.jsonData), &result)
+
+			if tt.expectedError {
+				if err == nil {
+					t.Errorf("expected error but got none")
+				}
+
+				return
+			}
+
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+				return
+			}
+
+			if tt.expectedResult.Data == nil {
+				if result.Data != nil {
+					t.Errorf("expected nil data but got %+v", result.Data)
+				}
+
+				return
+			}
+
+			if result.Data == nil {
+				t.Errorf("expected data but got nil")
+				return
+			}
+
+			if result.Data.Console != tt.expectedResult.Data.Console {
+				t.Errorf("console mismatch: expected %q, got %q", tt.expectedResult.Data.Console, result.Data.Console)
+			}
+
+			if result.Data.Release != tt.expectedResult.Data.Release {
+				t.Errorf("release mismatch: expected %q, got %q", tt.expectedResult.Data.Release, result.Data.Release)
+			}
+
+			if result.Data.RepositoryID != tt.expectedResult.Data.RepositoryID {
+				t.Errorf("repository ID mismatch: expected %q, got %q", tt.expectedResult.Data.RepositoryID, result.Data.RepositoryID)
+			}
+
+			if result.Data.Version.String() != tt.expectedResult.Data.Version.String() {
+				t.Errorf("version mismatch: expected %q, got %q", tt.expectedResult.Data.Version.String(), result.Data.Version.String())
+			}
+		})
+	}
+}
+
+func TestProxmoxVersion_UnmarshalJSON(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name           string
+		jsonData       []byte
+		expectedError  bool
+		expectedResult string
+	}{
+		{
+			name:           "valid version with quotes",
+			jsonData:       []byte(`"9.0.4"`),
+			expectedError:  false,
+			expectedResult: "9.0.4",
+		},
+		{
+			name:           "valid version without quotes",
+			jsonData:       []byte(`9.0.4`),
+			expectedError:  true, // This should fail because it's invalid JSON for a string
+			expectedResult: "",
+		},
+		{
+			name:           "semantic version",
+			jsonData:       []byte(`"8.2.1"`),
+			expectedError:  false,
+			expectedResult: "8.2.1",
+		},
+		{
+			name:          "invalid version format",
+			jsonData:      []byte(`"not-a-version"`),
+			expectedError: true,
+		},
+		{
+			name:          "empty version",
+			jsonData:      []byte(`""`),
+			expectedError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			var pv ProxmoxVersion
+
+			err := json.Unmarshal(tt.jsonData, &pv)
+
+			if tt.expectedError {
+				if err == nil {
+					t.Errorf("expected error but got none")
+				}
+
+				return
+			}
+
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+				return
+			}
+
+			if pv.String() != tt.expectedResult {
+				t.Errorf("version mismatch: expected %q, got %q", tt.expectedResult, pv.String())
+			}
+		})
+	}
+}
+
+// Helper function to create version objects for testing.
+func mustParseVersion(v string) *version.Version {
+	parsed, err := version.NewVersion(v)
+	if err != nil {
+		panic(err)
+	}
+
+	return parsed
+}