From 5dc14fbd1fc862787034ae2a051185b13f21b372 Mon Sep 17 00:00:00 2001 From: Ramgopal Nagaboina Date: Wed, 2 Sep 2026 13:18:09 -0400 Subject: [PATCH 1/4] fix: reject unsupported hypervisor values in host resource validation The hypervisor field accepted unsupported values at plan time because the ValidateFunc used sort.SearchStrings as a found-check when it returns an insertion index. Replace it with validation.StringInSlice, which rejects any value not in the supported list. Keeps the unit test covering accepted and rejected values. Signed-off-by: Ramgopal Nagaboina --- cloudstack/resource_cloudstack_host.go | 13 +------ .../resource_cloudstack_host_unit_test.go | 39 +++++++++++++++++++ 2 files changed, 41 insertions(+), 11 deletions(-) create mode 100644 cloudstack/resource_cloudstack_host_unit_test.go diff --git a/cloudstack/resource_cloudstack_host.go b/cloudstack/resource_cloudstack_host.go index 5f656392..e7641d5e 100644 --- a/cloudstack/resource_cloudstack_host.go +++ b/cloudstack/resource_cloudstack_host.go @@ -23,12 +23,12 @@ import ( "errors" "fmt" "log" - "sort" "strings" "time" "github.com/apache/cloudstack-go/v2/cloudstack" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) func resourceCloudStackHost() *schema.Resource { @@ -50,16 +50,7 @@ func resourceCloudStackHost() *schema.Resource { "hypervisor": { Type: schema.TypeString, Required: true, - ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) { - validHypervisors := []string{"xenserver", "kvm", "vmware", "baremetal", "simulator"} - - sort.Strings(validHypervisors) - - if sort.SearchStrings(validHypervisors, v.(string)) >= len(validHypervisors) { - errors = append(errors, fmt.Errorf("%q must be one of %v", k, validHypervisors)) - } - return - }, + ValidateFunc: validation.StringInSlice([]string{"xenserver", "kvm", "vmware", "baremetal", "simulator"}, false), ForceNew: true, }, "pod_id": { diff --git a/cloudstack/resource_cloudstack_host_unit_test.go b/cloudstack/resource_cloudstack_host_unit_test.go new file mode 100644 index 00000000..b0eeba8d --- /dev/null +++ b/cloudstack/resource_cloudstack_host_unit_test.go @@ -0,0 +1,39 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package cloudstack + +import "testing" + +func TestResourceCloudStackHostHypervisorValidation(t *testing.T) { + validate := resourceCloudStackHost().Schema["hypervisor"].ValidateFunc + + valid := []string{"xenserver", "kvm", "vmware", "baremetal", "simulator"} + for _, v := range valid { + if _, errs := validate(v, "hypervisor"); len(errs) != 0 { + t.Errorf("supported hypervisor %q should be accepted, got errors: %v", v, errs) + } + } + + // Values that are not in the supported list must be rejected at plan time. + invalid := []string{"foo", "docker", "esxi", "kvm2", ""} + for _, v := range invalid { + if _, errs := validate(v, "hypervisor"); len(errs) == 0 { + t.Errorf("unsupported hypervisor %q should be rejected, but validation accepted it", v) + } + } +} From 5608a17025db88cf47158dcfe38cdd7694b9376c Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Fri, 4 Sep 2026 09:54:46 +0530 Subject: [PATCH 2/4] gofmt --- cloudstack/resource_cloudstack_host.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cloudstack/resource_cloudstack_host.go b/cloudstack/resource_cloudstack_host.go index e7641d5e..de486353 100644 --- a/cloudstack/resource_cloudstack_host.go +++ b/cloudstack/resource_cloudstack_host.go @@ -48,10 +48,10 @@ func resourceCloudStackHost() *schema.Resource { }, Schema: map[string]*schema.Schema{ "hypervisor": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, ValidateFunc: validation.StringInSlice([]string{"xenserver", "kvm", "vmware", "baremetal", "simulator"}, false), - ForceNew: true, + ForceNew: true, }, "pod_id": { Type: schema.TypeString, From 6a4c4162d7d838e1589eef277aedaebebcf33166 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Fri, 4 Sep 2026 11:04:35 +0530 Subject: [PATCH 3/4] fix hypervisor casing to match CloudStack enum; merge unit test into acceptance test file --- cloudstack/resource_cloudstack_host.go | 2 +- cloudstack/resource_cloudstack_host_test.go | 19 +++++++++ .../resource_cloudstack_host_unit_test.go | 39 ------------------- 3 files changed, 20 insertions(+), 40 deletions(-) delete mode 100644 cloudstack/resource_cloudstack_host_unit_test.go diff --git a/cloudstack/resource_cloudstack_host.go b/cloudstack/resource_cloudstack_host.go index de486353..a33626b9 100644 --- a/cloudstack/resource_cloudstack_host.go +++ b/cloudstack/resource_cloudstack_host.go @@ -50,7 +50,7 @@ func resourceCloudStackHost() *schema.Resource { "hypervisor": { Type: schema.TypeString, Required: true, - ValidateFunc: validation.StringInSlice([]string{"xenserver", "kvm", "vmware", "baremetal", "simulator"}, false), + ValidateFunc: validation.StringInSlice([]string{"XenServer", "KVM", "VMware", "Hyperv", "BareMetal", "Simulator", "Ovm3"}, false), ForceNew: true, }, "pod_id": { diff --git a/cloudstack/resource_cloudstack_host_test.go b/cloudstack/resource_cloudstack_host_test.go index 25b53652..3c67339e 100644 --- a/cloudstack/resource_cloudstack_host_test.go +++ b/cloudstack/resource_cloudstack_host_test.go @@ -29,6 +29,25 @@ import ( "github.com/hashicorp/terraform-plugin-testing/terraform" ) +func TestResourceCloudStackHostHypervisorValidation(t *testing.T) { + validate := resourceCloudStackHost().Schema["hypervisor"].ValidateFunc + + valid := []string{"XenServer", "KVM", "VMware", "Hyperv", "BareMetal", "Simulator", "Ovm3"} + for _, v := range valid { + if _, errs := validate(v, "hypervisor"); len(errs) != 0 { + t.Errorf("supported hypervisor %q should be accepted, got errors: %v", v, errs) + } + } + + // Values that are not in the supported list, or that differ only in case, must be rejected at plan time. + invalid := []string{"foo", "docker", "esxi", "kvm2", "", "kvm", "simulator", "XENSERVER"} + for _, v := range invalid { + if _, errs := validate(v, "hypervisor"); len(errs) == 0 { + t.Errorf("unsupported hypervisor %q should be rejected, but validation accepted it", v) + } + } +} + func TestAccCloudStackHost_basic(t *testing.T) { var h cloudstack.Host resource.Test(t, resource.TestCase{ diff --git a/cloudstack/resource_cloudstack_host_unit_test.go b/cloudstack/resource_cloudstack_host_unit_test.go deleted file mode 100644 index b0eeba8d..00000000 --- a/cloudstack/resource_cloudstack_host_unit_test.go +++ /dev/null @@ -1,39 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package cloudstack - -import "testing" - -func TestResourceCloudStackHostHypervisorValidation(t *testing.T) { - validate := resourceCloudStackHost().Schema["hypervisor"].ValidateFunc - - valid := []string{"xenserver", "kvm", "vmware", "baremetal", "simulator"} - for _, v := range valid { - if _, errs := validate(v, "hypervisor"); len(errs) != 0 { - t.Errorf("supported hypervisor %q should be accepted, got errors: %v", v, errs) - } - } - - // Values that are not in the supported list must be rejected at plan time. - invalid := []string{"foo", "docker", "esxi", "kvm2", ""} - for _, v := range invalid { - if _, errs := validate(v, "hypervisor"); len(errs) == 0 { - t.Errorf("unsupported hypervisor %q should be rejected, but validation accepted it", v) - } - } -} From 51a199e7791fa2284d7ecc4579a9c6793d707712 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Fri, 4 Sep 2026 17:34:19 +0530 Subject: [PATCH 4/4] make hypervisor validation case-insensitive to match CloudStack server-side matching --- cloudstack/resource_cloudstack_host.go | 2 +- cloudstack/resource_cloudstack_host_test.go | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cloudstack/resource_cloudstack_host.go b/cloudstack/resource_cloudstack_host.go index a33626b9..61e81946 100644 --- a/cloudstack/resource_cloudstack_host.go +++ b/cloudstack/resource_cloudstack_host.go @@ -50,7 +50,7 @@ func resourceCloudStackHost() *schema.Resource { "hypervisor": { Type: schema.TypeString, Required: true, - ValidateFunc: validation.StringInSlice([]string{"XenServer", "KVM", "VMware", "Hyperv", "BareMetal", "Simulator", "Ovm3"}, false), + ValidateFunc: validation.StringInSlice([]string{"XenServer", "KVM", "VMware", "Hyperv", "BareMetal", "Simulator", "Ovm3"}, true), ForceNew: true, }, "pod_id": { diff --git a/cloudstack/resource_cloudstack_host_test.go b/cloudstack/resource_cloudstack_host_test.go index 3c67339e..c0774d82 100644 --- a/cloudstack/resource_cloudstack_host_test.go +++ b/cloudstack/resource_cloudstack_host_test.go @@ -32,15 +32,16 @@ import ( func TestResourceCloudStackHostHypervisorValidation(t *testing.T) { validate := resourceCloudStackHost().Schema["hypervisor"].ValidateFunc - valid := []string{"XenServer", "KVM", "VMware", "Hyperv", "BareMetal", "Simulator", "Ovm3"} + // CloudStack's HypervisorType.getType() lowercases the input before lookup, so matching + // is case-insensitive server-side; the validator must accept any casing accordingly. + valid := []string{"XenServer", "KVM", "VMware", "Hyperv", "BareMetal", "Simulator", "Ovm3", "kvm", "simulator", "XENSERVER"} for _, v := range valid { if _, errs := validate(v, "hypervisor"); len(errs) != 0 { t.Errorf("supported hypervisor %q should be accepted, got errors: %v", v, errs) } } - // Values that are not in the supported list, or that differ only in case, must be rejected at plan time. - invalid := []string{"foo", "docker", "esxi", "kvm2", "", "kvm", "simulator", "XENSERVER"} + invalid := []string{"foo", "docker", "esxi", "kvm2", ""} for _, v := range invalid { if _, errs := validate(v, "hypervisor"); len(errs) == 0 { t.Errorf("unsupported hypervisor %q should be rejected, but validation accepted it", v)