subnet: fix default values for dhcp_options_id, route_table_id, security_list_ids (#151)

Default values in schema differ from the defaults provided by API, so any configuration not specifying them is broken. Any configuration which does specify them should be unchanged.

- resource_obmcs_core_subnet.go: make dhcp_options_id, route_table_id, security_list_ids required
- resource_obmcs_core_subnet_test.go: rewrite
- provider_test.go, data_source_obmcs_core_subnet_test.go: add required attributes

Author: josephholsten

data_source_obmcs_core_subnet_test.go

@@ -87,6 +87,7 @@ resource "baremetal_core_subnet" "WebSubnetAD1" {
   vcn_id = "${baremetal_core_virtual_network.t.id}"
   route_table_id = "${baremetal_core_route_table.RouteForComplete.id}"
   security_list_ids = ["${baremetal_core_security_list.WebSubnet.id}"]
+  dhcp_options_id = ["${baremetal_core_virtual_network.t.default_dhcp_options_id}"]
 }
   `
 	s.Config += testProviderConfig()

docs/resources/core/subnet.md

@@ -5,15 +5,28 @@ Gets a list of subnets.
 ## Example Usage
 
 ```
+data "baremetal_identity_availability_domains" "ADs" {
+  compartment_id = "${var.compartment_id}"
+}
+
+resource "baremetal_core_virtual_network" "t" {
+  cidr_block     = "10.0.0.0/16"
+  compartment_id = "${var.compartment_id}"
+  display_name   = "network_name"
+}
+
 resource "baremetal_core_subnet" "t" {
-    availability_domain = "availabilitydomainid"
-    compartment_id = "compartmentid"
-    display_name = "display_name"
-    cidr_block = "10.10.10.0/24"
-    route_table_id = "routetableid"
-    vcn_id = "vcnid"
-    security_list_ids = ["slid1", "slid2"]
-    prohibit_public_ip_on_vnic = true
+  compartment_id = "${var.compartment_id}"
+
+  availability_domain = "${data.baremetal_identity_availability_domains.ADs.availability_domains.0.name}"
+  route_table_id      = "${baremetal_core_virtual_network.t.default_route_table_id}"
+  vcn_id              = "${baremetal_core_virtual_network.t.id}"
+  security_list_ids   = ["${baremetal_core_virtual_network.t.default_security_list_id}"]
+  dhcp_options_id     = "${baremetal_core_virtual_network.t.default_dhcp_options_id}"
+
+  display_name               = "display_name"
+  cidr_block                 = "10.10.10.0/24"
+  prohibit_public_ip_on_vnic = true
 }
 ```
 
@@ -22,30 +35,21 @@ resource "baremetal_core_subnet" "t" {
 The following arguments are supported:
 
 * `availability_domain` - (Required) The Availability Domain to contain the subnet.
-* `compartment_id` - (Required) The OCID of the compartment to contain the subnet.
 * `cidr_block` - (Required) The CIDR IP address range of the subnet.
+* `compartment_id` - (Required) The OCID of the compartment to contain the subnet.
+* `dhcp_options_id` - (Required) The OCID of the set of DHCP options the subnet will use.
+* `route_table_id` - (Required) The OCID of the route table the subnet will use.
+* `security_list_ids` - (Required) OCIDs for the security lists to associate with the subnet. Remember that security lists are associated at the subnet level, but the rules are applied to the individual VNICs in the subnet.
 * `vcn_id` - (Required) The OCID of the VCN to contain the subnet.
-* `dhcp_options_id` - (Optional) The OCID of the set of DHCP options the subnet will use. If you don't provide a value, the subnet will use the VCN's default set of DHCP options.
-* `display_name` - (Optional) The maximum number of items to return in a paginated "List" call.
-* `prohibit_public_ip_on_vnic` - (Optional) Whether VNICs within this subnet can have public IP. If it is allowed, VNICs created in the subnet will automatically be assigned public IP unless otherwise specified in the VNIC. If it is prohibited, VNICs in the subnet cannot have public IP address assigned. The default value is false if unspecified.
-* `route_table_id` - (Optional) The OCID of the route table the subnet will use. If you don't provide a value, the subnet will use the VCN's default route table.
-* `security_list_ids` - (Optional) OCIDs for the security lists to associate with the subnet. If you don't provide a value, the VCN's default security list will be associated with the subnet. Remember that security lists are associated at the subnet level, but the rules are applied to the individual VNICs in the subnet.
-
 
+* `dns_label` - (Optional) DNS label for the subnet, used in conjunction with the VNIC's hostname and VCN's DNS label to form a fully qualified domain name (FQDN) for each VNIC within this subnet (e.g., bminstance-1.subnet123.vcn1.oraclevcn.com). Must be an alphanumeric string that begins with a letter and is unique within the VCN. The value cannot be changed. The absence of this parameter means the Internet and VCN Resolver will not resolve hostnames of instances in this subnet.
+* `display_name` - (Optional) User-friendly name. Does not have to be unique, and it's changeable.
+* `prohibit_public_ip_on_vnic` - (Optional) Whether VNICs within this subnet can have public IP. If it is allowed, VNICs created in the subnet will automatically be assigned public IP unless otherwise specified in the VNIC. If it is prohibited, VNICs in the subnet cannot have public IP address assigned. The default value is false if unspecified.
 
 ## Attributes Reference
 
-* `availability_domain` - The subnet's Availability Domain.
-* `cidr_block` - The CIDR IP address block of the VCN.
-* `compartment_id` - The OCID of the compartment containing the VCN.
-* `dhcp_options_id` - The OCID for the VCN's default set of DHCP options.
-* `route_table_id` - The OCID for the VCN's default route table.
-* `security_list_ids` - OCIDs for the security lists to use for VNICs in this subnet.
-* `display_name` - A user-friendly name. Does not have to be unique, and it's changeable.
 * `id` - The subnet's Oracle ID (OCID).
-* `prohibit_public_ip_on_vnic` - Whether VNICs within this subnet can have public IPs. If it is allowed, VNICs created in the subnet will automatically be assigned public IP unless otherwise specified in the VNIC. If it is prohibited, VNICs in the subnet cannot have public IP address assigned. The default value is false if unspecified.
-* `vcn_id` - The OCID of the VCN the subnet is in.
 * `state` - The VCN's current state. [PROVISIONING, AVAILABLE, TERMINATING, TERMINATED]
 * `time_created` - The date and time the VCN was created.
 * `virtual_router_ip` - The IP address of the virtual router.
-* `virtual_router_mac` - The MAC address of the virtual router.
+* `virtual_router_mac` - The MAC address of the virtual router.

provider_test.go

@@ -99,12 +99,13 @@ resource "baremetal_core_security_list" "WebSubnet" {
 
 resource "baremetal_core_subnet" "WebSubnetAD1" {
   availability_domain = "${lookup(data.baremetal_identity_availability_domains.ADs.availability_domains[0],"name")}"
-  cidr_block = "10.0.1.0/24"
-  display_name = "WebSubnetAD1"
-  compartment_id = "${var.compartment_id}"
-  vcn_id = "${baremetal_core_virtual_network.t.id}"
-  route_table_id = "${baremetal_core_route_table.RouteForComplete.id}"
-  security_list_ids = ["${baremetal_core_security_list.WebSubnet.id}"]
+  cidr_block          = "10.0.1.0/24"
+  display_name        = "WebSubnetAD1"
+  compartment_id      = "${var.compartment_id}"
+  vcn_id              = "${baremetal_core_virtual_network.t.id}"
+  route_table_id      = "${baremetal_core_route_table.RouteForComplete.id}"
+  security_list_ids   = ["${baremetal_core_security_list.WebSubnet.id}"]
+  dhcp_options_id     = "${baremetal_core_virtual_network.t.default_dhcp_options_id}"
 }
 
 `

resource_obmcs_core_subnet.go

@@ -38,47 +38,50 @@ func SubnetResource() *schema.Resource {
 				Required: true,
 				ForceNew: true,
 			},
-			"route_table_id": {
+			"dhcp_options_id": {
 				Type:     schema.TypeString,
-				Optional: true,
+				Required: true,
 				ForceNew: true,
 			},
-			"vcn_id": {
+			"route_table_id": {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
 			},
 			"security_list_ids": {
 				Type:     schema.TypeSet,
-				Optional: true,
+				Required: true,
 				ForceNew: true,
 				Set:      schema.HashString,
 				Elem: &schema.Schema{
 					Type: schema.TypeString,
 				},
 			},
-			"display_name": {
-				Type:     schema.TypeString,
-				Optional: true,
-				Computed: true,
-			},
-
-			"dhcp_options_id": {
+			"vcn_id": {
 				Type:     schema.TypeString,
-				Optional: true,
-				Computed: true,
+				Required: true,
+				ForceNew: true,
 			},
+			// Optional
 			"dns_label": {
 				Type:     schema.TypeString,
 				Optional: true,
+				ForceNew: true,
 			},
-			"id": {
+			"display_name": {
 				Type:     schema.TypeString,
+				Optional: true,
 				Computed: true,
 			},
 			"prohibit_public_ip_on_vnic": {
 				Type:     schema.TypeBool,
 				Optional: true,
+				Default:  false,
+			},
+			// Computed
+			"id": {
+				Type:     schema.TypeString,
+				Computed: true,
 			},
 			"state": {
 				Type:     schema.TypeString,

resource_obmcs_core_subnet_test.go

@@ -3,92 +3,123 @@
 package main
 
 import (
+	"regexp"
 	"testing"
-	"time"
 
 	"github.com/MustWin/baremetal-sdk-go"
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
-
-	"github.com/stretchr/testify/suite"
 )
 
-type ResourceCoreSubnetTestSuite struct {
-	suite.Suite
-	Client       mockableClient
-	Provider     terraform.ResourceProvider
-	Providers    map[string]terraform.ResourceProvider
-	TimeCreated  baremetal.Time
-	Config       string
-	ResourceName string
-}
-
-func (s *ResourceCoreSubnetTestSuite) SetupTest() {
-	s.Client = GetTestProvider()
-
-	s.Provider = Provider(
+func TestResourceCoreSubnetCreate(t *testing.T) {
+	client := GetTestProvider()
+	provider := Provider(
 		func(d *schema.ResourceData) (interface{}, error) {
-			return s.Client, nil
+			return client, nil
 		},
 	)
 
-	s.Providers = map[string]terraform.ResourceProvider{
-		"baremetal": s.Provider,
-	}
-
-	s.TimeCreated = baremetal.Time{Time: time.Now()}
-
-	s.Config = subnetConfig
-	s.Config += testProviderConfig()
+	config := `
+data "baremetal_identity_availability_domains" "ADs" {
+  compartment_id = "${var.compartment_id}"
+}
 
-	s.ResourceName = "baremetal_core_subnet.WebSubnetAD1"
+resource "baremetal_core_virtual_network" "t" {
+  cidr_block     = "10.0.0.0/16"
+  compartment_id = "${var.compartment_id}"
+  display_name   = "network_name"
+}
 
+resource "baremetal_core_subnet" "s" {
+  availability_domain = "${data.baremetal_identity_availability_domains.ADs.availability_domains.0.name}"
+  compartment_id      = "${var.compartment_id}"
+  vcn_id              = "${baremetal_core_virtual_network.t.id}"
+  security_list_ids   = ["${baremetal_core_virtual_network.t.default_security_list_id}"]
+  route_table_id      = "${baremetal_core_virtual_network.t.default_route_table_id}"
+  dhcp_options_id     = "${baremetal_core_virtual_network.t.default_dhcp_options_id}"
+  cidr_block          = "10.0.2.0/24"
 }
+	`
+	config += testProviderConfig()
 
-func (s *ResourceCoreSubnetTestSuite) TestCreateResourceCoreSubnet() {
+	resourceName := "baremetal_core_subnet.s"
 
-	resource.UnitTest(s.T(), resource.TestCase{
-		Providers: s.Providers,
+	resource.UnitTest(t, resource.TestCase{
+		Providers: map[string]terraform.ResourceProvider{
+			"baremetal": provider,
+		},
 		Steps: []resource.TestStep{
 			{
 				ImportState:       true,
 				ImportStateVerify: true,
-				Config:            s.Config,
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttrSet(s.ResourceName, "availability_domain"),
-					resource.TestCheckResourceAttr(s.ResourceName, "display_name", "WebSubnetAD1"),
-					resource.TestCheckResourceAttrSet(s.ResourceName, "id"),
-					resource.TestCheckResourceAttr(s.ResourceName, "state", baremetal.ResourceAvailable),
-					resource.TestCheckResourceAttrSet(s.ResourceName, "time_created"),
+				Config:            config,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestMatchResourceAttr(resourceName, "compartment_id", regexp.MustCompile("ocid1\\.compartment\\.oc1\\..*")),
+					resource.TestCheckResourceAttrSet(resourceName, "availability_domain"),
+					resource.TestMatchResourceAttr(resourceName, "vcn_id", regexp.MustCompile("ocid1\\.vcn\\.oc1\\..*")),
+					resource.TestMatchResourceAttr(resourceName, "dhcp_options_id", regexp.MustCompile("ocid1\\.dhcpoptions\\.oc1\\..*")),
+					resource.TestMatchResourceAttr(resourceName, "route_table_id", regexp.MustCompile("ocid1\\.routetable\\.oc1\\..*")),
+					resource.TestCheckResourceAttr(resourceName, "security_list_ids.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "cidr_block", "10.0.2.0/24"),
+					resource.TestCheckResourceAttrSet(resourceName, "display_name"),
+					resource.TestCheckResourceAttr(resourceName, "dns_label", ""),
+					resource.TestCheckResourceAttr(resourceName, "prohibit_public_ip_on_vnic", "false"),
+					resource.TestMatchResourceAttr(resourceName, "id", regexp.MustCompile("ocid1\\.subnet\\.oc1\\..*")),
+					resource.TestCheckResourceAttr(resourceName, "state", baremetal.ResourceAvailable),
+					resource.TestCheckResourceAttrSet(resourceName, "time_created"),
+					resource.TestCheckResourceAttrSet(resourceName, "virtual_router_ip"),
+					resource.TestCheckResourceAttrSet(resourceName, "virtual_router_mac"),
 				),
 			},
 		},
 	})
 }
 
-func (s *ResourceCoreSubnetTestSuite) TestTerminateSubnet() {
-	if IsAccTest() {
-		s.T().Skip()
-	}
+func TestResourceCoreSubnetTerminate(t *testing.T) {
+	client := GetTestProvider()
+	provider := Provider(
+		func(d *schema.ResourceData) (interface{}, error) {
+			return client, nil
+		},
+	)
+	config := `
+data "baremetal_identity_availability_domains" "ADs" {
+  compartment_id = "${var.compartment_id}"
+}
+
+resource "baremetal_core_virtual_network" "t" {
+  cidr_block     = "10.0.0.0/16"
+  compartment_id = "${var.compartment_id}"
+  display_name   = "network_name"
+}
 
-	resource.UnitTest(s.T(), resource.TestCase{
-		Providers: s.Providers,
+resource "baremetal_core_subnet" "s" {
+  availability_domain = "${data.baremetal_identity_availability_domains.ADs.availability_domains.0.name}"
+  compartment_id      = "${var.compartment_id}"
+  vcn_id              = "${baremetal_core_virtual_network.t.id}"
+  security_list_ids   = ["${baremetal_core_virtual_network.t.default_security_list_id}"]
+  route_table_id      = "${baremetal_core_virtual_network.t.default_route_table_id}"
+  dhcp_options_id     = "${baremetal_core_virtual_network.t.default_dhcp_options_id}"
+  cidr_block          = "10.0.2.0/24"
+}
+	`
+	config += testProviderConfig()
+	resource.UnitTest(t, resource.TestCase{
+		Providers: map[string]terraform.ResourceProvider{
+			"baremetal": provider,
+		},
 		Steps: []resource.TestStep{
 			{
 				ImportState:       true,
 				ImportStateVerify: true,
-				Config:            s.Config,
+				Config:            config,
 			},
 			{
-				Config:  s.Config,
+				Config:  config,
 				Destroy: true,
 			},
 		},
 	})
 
 }
-
-func TestResourceCoreSubnetTestSuite(t *testing.T) {
-	suite.Run(t, new(ResourceCoreSubnetTestSuite))
-}