diff --git a/cloudstack/resource_cloudstack_configuration.go b/cloudstack/resource_cloudstack_configuration.go index 5a4950f0..4c32ad8c 100644 --- a/cloudstack/resource_cloudstack_configuration.go +++ b/cloudstack/resource_cloudstack_configuration.go @@ -163,10 +163,7 @@ func resourceCloudStackConfigurationCreate(d *schema.ResourceData, meta interfac d.SetId(v.(string)) } - resourceCloudStackConfigurationUpdate(d, meta) - - return nil - + return resourceCloudStackConfigurationUpdate(d, meta) } func resourceCloudStackConfigurationUpdate(d *schema.ResourceData, meta interface{}) error { @@ -201,9 +198,7 @@ func resourceCloudStackConfigurationUpdate(d *schema.ResourceData, meta interfac return err } - resourceCloudStackConfigurationRead(d, meta) - - return nil + return resourceCloudStackConfigurationRead(d, meta) } func resourceCloudStackConfigurationDelete(d *schema.ResourceData, meta interface{}) error { diff --git a/cloudstack/resource_cloudstack_instance.go b/cloudstack/resource_cloudstack_instance.go index 06c84f42..a14aa1ba 100644 --- a/cloudstack/resource_cloudstack_instance.go +++ b/cloudstack/resource_cloudstack_instance.go @@ -153,7 +153,7 @@ func resourceCloudStackInstance() *schema.Resource { ForceNew: true, }, - "keypair": &schema.Schema{ + "keypair": { Type: schema.TypeString, Optional: true, ConflictsWith: []string{"keypairs"}, diff --git a/cloudstack/resource_cloudstack_network.go b/cloudstack/resource_cloudstack_network.go index e7329f82..c1dc9f4c 100644 --- a/cloudstack/resource_cloudstack_network.go +++ b/cloudstack/resource_cloudstack_network.go @@ -20,6 +20,7 @@ package cloudstack import ( + "context" "fmt" "log" "net" @@ -59,6 +60,7 @@ func resourceCloudStackNetwork() *schema.Resource { Importer: &schema.ResourceImporter{ State: importStatePassthrough, }, + CustomizeDiff: resourceCloudStackNetworkCustomizeDiff, Schema: map[string]*schema.Schema{ "name": { @@ -72,9 +74,23 @@ func resourceCloudStackNetwork() *schema.Resource { Computed: true, }, + "type": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) { + v := val.(string) + if v != "L2" && v != "L3" { + errs = append(errs, fmt.Errorf("%q must be either 'L2' or 'L3', got: %s", key, v)) + } + return + }, + }, + "cidr": { Type: schema.TypeString, - Required: true, + Optional: true, ForceNew: true, }, @@ -158,6 +174,41 @@ func resourceCloudStackNetwork() *schema.Resource { } } +func resourceCloudStackNetworkCustomizeDiff(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + var t string + if v, ok := d.GetOk("type"); ok { + t = v.(string) + } + cidr := d.Get("cidr").(string) + + // Default type if user didn't set it + if t == "" { + if cidr != "" { + if err := d.SetNew("type", "L3"); err != nil { + return err + } + t = "L3" + } else { + if err := d.SetNew("type", "L2"); err != nil { + return err + } + t = "L2" + } + } + + // Enforce combinations; also clear stray cidr for L2 to avoid drift + if t == "L3" && cidr == "" { + return fmt.Errorf("cidr is required when type is L3") + } + if t == "L2" && cidr != "" { + if err := d.SetNew("cidr", ""); err != nil { + return err + } + } + + return nil +} + func resourceCloudStackNetworkCreate(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) @@ -190,23 +241,31 @@ func resourceCloudStackNetworkCreate(d *schema.ResourceData, meta interface{}) e return err } - m, err := parseCIDR(d, no.Specifyipranges) - if err != nil { - return err - } + // Check if this is an L2 network + networkType := d.Get("type").(string) + if networkType == "L2" { + // For L2 networks, we don't set IP-related parameters + // The network offering should handle VLAN-only configuration + } else { + // For L3 networks, parse CIDR and set IP parameters + m, err := parseCIDR(d, no.Specifyipranges) + if err != nil { + return err + } - // Set the needed IP config - p.SetGateway(m["gateway"]) - p.SetNetmask(m["netmask"]) + // Set the needed IP config + p.SetGateway(m["gateway"]) + p.SetNetmask(m["netmask"]) - // Only set the start IP if we have one - if startip, ok := m["startip"]; ok { - p.SetStartip(startip) - } + // Only set the start IP if we have one + if startip, ok := m["startip"]; ok { + p.SetStartip(startip) + } - // Only set the end IP if we have one - if endip, ok := m["endip"]; ok { - p.SetEndip(endip) + // Only set the end IP if we have one + if endip, ok := m["endip"]; ok { + p.SetEndip(endip) + } } // Set the network domain if we have one @@ -301,11 +360,20 @@ func resourceCloudStackNetworkRead(d *schema.ResourceData, meta interface{}) err d.Set("name", n.Name) d.Set("display_text", n.Displaytext) - d.Set("cidr", n.Cidr) - d.Set("gateway", n.Gateway) d.Set("network_domain", n.Networkdomain) d.Set("vpc_id", n.Vpcid) + // Normalize: different ACS versions omit/empty fields + if strings.TrimSpace(n.Cidr) == "" { + d.Set("type", "L2") + d.Set("cidr", "") + d.Set("gateway", "") + } else { + d.Set("type", "L3") + d.Set("cidr", n.Cidr) + d.Set("gateway", n.Gateway) + } + if n.Aclid == "" { n.Aclid = none } @@ -439,7 +507,19 @@ func resourceCloudStackNetworkDelete(d *schema.ResourceData, meta interface{}) e func parseCIDR(d *schema.ResourceData, specifyiprange bool) (map[string]string, error) { m := make(map[string]string, 4) + // Check if this is an L2 network + networkType := d.Get("type").(string) + if networkType == "L2" { + // For L2 networks, we don't need CIDR parsing + // Just return empty values for IP-related fields + return m, nil + } + cidr := d.Get("cidr").(string) + if cidr == "" { + return nil, fmt.Errorf("cidr is required for L3 networks") + } + ip, ipnet, err := net.ParseCIDR(cidr) if err != nil { return nil, fmt.Errorf("Unable to parse cidr %s: %s", cidr, err) diff --git a/cloudstack/resource_cloudstack_network_parse_test.go b/cloudstack/resource_cloudstack_network_parse_test.go new file mode 100644 index 00000000..6bd17149 --- /dev/null +++ b/cloudstack/resource_cloudstack_network_parse_test.go @@ -0,0 +1,88 @@ +// +// 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" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func TestParseCIDR(t *testing.T) { + networkResource := resourceCloudStackNetwork() + + t.Run("L2 network should return empty map", func(t *testing.T) { + config := map[string]interface{}{ + "type": "L2", + } + + resourceData := schema.TestResourceDataRaw(t, networkResource.Schema, config) + + result, err := parseCIDR(resourceData, false) + if err != nil { + t.Errorf("Expected no error for L2 network, got: %v", err) + } + if result == nil { + t.Error("Expected non-nil result for L2 network") + } + if len(result) != 0 { + t.Errorf("Expected empty map for L2 network, got: %v", result) + } + }) + + t.Run("L3 network with valid CIDR should parse correctly", func(t *testing.T) { + config := map[string]interface{}{ + "type": "L3", + "cidr": "10.0.0.0/16", + } + + resourceData := schema.TestResourceDataRaw(t, networkResource.Schema, config) + + result, err := parseCIDR(resourceData, true) + if err != nil { + t.Errorf("Expected no error for L3 network with valid CIDR, got: %v", err) + } + if result == nil { + t.Error("Expected non-nil result for L3 network") + } + if result["gateway"] == "" { + t.Error("Expected gateway to be set") + } + if result["netmask"] == "" { + t.Error("Expected netmask to be set") + } + }) + + t.Run("L3 network without CIDR should return error", func(t *testing.T) { + config := map[string]interface{}{ + "type": "L3", + } + + resourceData := schema.TestResourceDataRaw(t, networkResource.Schema, config) + + _, err := parseCIDR(resourceData, true) + if err == nil { + t.Error("Expected error for L3 network without CIDR, but got none") + } + if err != nil && err.Error() != "cidr is required for L3 networks" { + t.Errorf("Expected specific error message, got: %v", err) + } + }) +} diff --git a/cloudstack/resource_cloudstack_network_test.go b/cloudstack/resource_cloudstack_network_test.go index 0b650ace..47b6389a 100644 --- a/cloudstack/resource_cloudstack_network_test.go +++ b/cloudstack/resource_cloudstack_network_test.go @@ -165,6 +165,26 @@ func TestAccCloudStackNetwork_importProject(t *testing.T) { }) } +func TestAccCloudStackNetwork_L2(t *testing.T) { + var network cloudstack.Network + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetwork_L2, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkExists( + "cloudstack_network.foo", &network), + testAccCheckCloudStackNetworkL2Attributes(&network), + ), + }, + }, + }) +} + func testAccCheckCloudStackNetworkExists( n string, network *cloudstack.Network) resource.TestCheckFunc { return func(s *terraform.State) error { @@ -244,6 +264,27 @@ func testAccCheckCloudStackNetworkVPCAttributes( } } +func testAccCheckCloudStackNetworkL2Attributes( + network *cloudstack.Network) resource.TestCheckFunc { + return func(s *terraform.State) error { + + if network.Name != "terraform-l2-network" { + return fmt.Errorf("Bad name: %s", network.Name) + } + + if network.Displaytext != "terraform-l2-network" { + return fmt.Errorf("Bad display name: %s", network.Displaytext) + } + + // L2 networks should not have a CIDR + if network.Cidr != "" { + return fmt.Errorf("L2 network should not have CIDR, got: %s", network.Cidr) + } + + return nil + } +} + func testAccCheckCloudStackNetworkDestroy(s *terraform.State) error { cs := testAccProvider.Meta().(*cloudstack.CloudStackClient) @@ -377,3 +418,13 @@ resource "cloudstack_network" "foo" { acl_id = cloudstack_network_acl.bar.id zone = cloudstack_vpc.foo.zone }` + +const testAccCloudStackNetwork_L2 = ` +resource "cloudstack_network" "foo" { + name = "terraform-l2-network" + display_text = "terraform-l2-network" + type = "L2" + network_offering = "DefaultSharedNetworkOffering" + zone = "Sandbox-simulator" + vlan = 100 +}` diff --git a/cloudstack/resource_cloudstack_network_validation_test.go b/cloudstack/resource_cloudstack_network_validation_test.go new file mode 100644 index 00000000..1306b11f --- /dev/null +++ b/cloudstack/resource_cloudstack_network_validation_test.go @@ -0,0 +1,72 @@ +// +// 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" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func TestResourceCloudStackNetworkSchema(t *testing.T) { + networkResource := resourceCloudStackNetwork() + + // Test that required fields exist + t.Run("Schema should have type field", func(t *testing.T) { + if typeField, ok := networkResource.Schema["type"]; !ok { + t.Error("Schema should have 'type' field") + } else { + if typeField.Type != schema.TypeString { + t.Errorf("Type field should be TypeString, got: %v", typeField.Type) + } + if typeField.Required { + t.Error("Type field should not be required") + } + if typeField.Optional != true { + t.Error("Type field should be optional") + } + if typeField.Computed != true { + t.Error("Type field should be computed") + } + if typeField.Default != nil { + t.Errorf("Type field should not have a default (it's computed), got: %v", typeField.Default) + } + } + }) + + t.Run("Schema should have cidr field as optional", func(t *testing.T) { + if cidrField, ok := networkResource.Schema["cidr"]; !ok { + t.Error("Schema should have 'cidr' field") + } else { + if cidrField.Required { + t.Error("CIDR field should not be required") + } + if cidrField.Optional != true { + t.Error("CIDR field should be optional") + } + } + }) + + t.Run("Schema should have CustomizeDiff", func(t *testing.T) { + if networkResource.CustomizeDiff == nil { + t.Error("Resource should have CustomizeDiff function") + } + }) +} diff --git a/website/docs/r/network.html.markdown b/website/docs/r/network.html.markdown index d83ebb1a..be2f1b59 100644 --- a/website/docs/r/network.html.markdown +++ b/website/docs/r/network.html.markdown @@ -12,17 +12,30 @@ Creates a network. ## Example Usage -Basic usage: +Basic L3 network usage: ```hcl resource "cloudstack_network" "default" { name = "test-network" + type = "L3" cidr = "10.0.0.0/16" network_offering = "Default Network" zone = "zone-1" } ``` +L2 VLAN-only network usage: + +```hcl +resource "cloudstack_network" "l2_network" { + name = "l2-network" + type = "L2" + network_offering = "L2 Network Offering" + zone = "zone-1" + vlan = 100 +} +``` + ## Argument Reference The following arguments are supported: @@ -31,8 +44,9 @@ The following arguments are supported: * `display_text` - (Optional) The display text of the network. -* `cidr` - (Required) The CIDR block for the network. Changing this forces a new - resource to be created. +* `type` - (Optional) The type of network. Valid values are "L2" for VLAN-only networks and "L3" for IP-based networks. Defaults to "L3". + +* `cidr` - (Optional) The CIDR block for the network. Required when `type` is "L3", not allowed when `type` is "L2". Changing this forces a new resource to be created. * `gateway` - (Optional) Gateway that will be provided to the instances in this network. Defaults to the first usable IP in the range.