Add source port range to security list rules (another rebase) (#385)

Ported from PR #366.

Currently, TCP and UDP options can be specified with a single set of
max/min values, which correspond to the destination port range. This change
adds the ability to specify the source port range as well, issue #338.

Ideally, we should deprecate the existing max/min params and add a new
destination_port_range option next to the new source_port_range. Unfortunately,
a number of issue with nested schemas in Terraform are preventing support of both
of these params simultaneously. Instead, max/min params will remain, and source_port_range
will exist beside it.

This change also enables security lists with no rules.

Some updates are made to the docs, but the docs for the security list resource and data source
could really use a bigger update that is not included in this change.

Author: briangustafson

CHANGELOG.md

@@ -3,6 +3,11 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/).
 
+## Unreleased
+
+### Added
+- Support for security list rule source port ranges (#340). This can be specified in "tcp_options" and "udp_options" using "source_port_range".
+
 ## 2.0.4 - 2017-11-2
 
 ### Added

docs/datasources/core/security_lists.md

@@ -11,8 +11,6 @@ Gets a list of security lists. Each security list is a set of virtual firewall r
 ```
     data "oci_core_security_lists" "t" {
       compartment_id = "compartment_id"
-      limit = 1
-      page = "page"
       vcn_id = "vcn_id"
     }
 ```

docs/examples/networking/security_list/security_list.tf

@@ -9,9 +9,6 @@ variable "private_key_path" {}
 variable "compartment_ocid" {}
 variable "region" {}
 
-variable "vcn_ocid" {}
-
-
 provider "oci" {
   tenancy_ocid = "${var.tenancy_ocid}"
   user_ocid = "${var.user_ocid}"
@@ -20,13 +17,20 @@ provider "oci" {
   region = "${var.region}"
 }
 
+resource "oci_core_virtual_network" "ExampleVCN" {
+  cidr_block = "10.0.0.0/16"
+  dns_label = "examplevcn"
+  compartment_id = "${var.compartment_ocid}"
+  display_name = "ExampleVCN"
+}
+
 # Protocols are specified as protocol numbers.
 # http://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml
 
-resource "oci_core_security_list" "security_list1" {
+resource "oci_core_security_list" "ExampleSecurityList" {
   compartment_id = "${var.compartment_ocid}"
-  vcn_id = "${var.vcn_ocid}"
-  display_name = "security_list1"
+  vcn_id = "${oci_core_virtual_network.ExampleVCN.id}"
+  display_name = "ExampleSecurityList"
 
   // allow outbound tcp traffic on all ports
   egress_security_rules {
@@ -41,18 +45,24 @@ resource "oci_core_security_list" "security_list1" {
     stateless = true
 
     udp_options {
+      // These values correspond to the destination port range.
       "min" = 319
       "max" = 320
     }
   }
 
-  // allow inbound ssh traffic
+  // allow inbound ssh traffic from a specific port
   ingress_security_rules {
     protocol = "6" // tcp
     source = "0.0.0.0/0"
     stateless = false
 
     tcp_options {
+      source_port_range {
+        "min" = 100
+        "max" = 100
+      }
+      // These values correspond to the destination port range.
       "min" = 22
       "max" = 22
     }

docs/resources/core/security_list.md

@@ -38,8 +38,13 @@ resource "oci_core_security_list" "t" {
         stateless = true
 
         tcp_options {
-            "min" = 80
-            "max" = 82
+            source_port_range {
+                "min" = 100
+                "max" = 100
+            }
+            // These values correspond to the destination port range.
+            "min" = 22
+            "max" = 22
         }
     }
 
@@ -49,6 +54,8 @@ resource "oci_core_security_list" "t" {
         stateless = true
 
         udp_options {
+            // These values correspond to the destination port range.
+            // source_port_range may also be specified.
             "min" = 319
             "max" = 320
         }

main.go

@@ -5,6 +5,7 @@ package main
 import (
 	"github.com/hashicorp/terraform/plugin"
 	"github.com/hashicorp/terraform/terraform"
+
 	"github.com/oracle/terraform-provider-oci/provider"
 )
 

provider/core_security_list_resource.go

@@ -15,13 +15,31 @@ var transportSchema = &schema.Schema{
 	MaxItems: 1,
 	Elem: &schema.Resource{
 		Schema: map[string]*schema.Schema{
+			"source_port_range": {
+				Type:     schema.TypeList,
+				Optional: true,
+				MaxItems: 1,
+				MinItems: 1,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"max": {
+							Type:     schema.TypeInt,
+							Required: true,
+						},
+						"min": {
+							Type:     schema.TypeInt,
+							Required: true,
+						},
+					},
+				},
+			},
 			"max": {
 				Type:     schema.TypeInt,
-				Required: true,
+				Optional: true,
 			},
 			"min": {
 				Type:     schema.TypeInt,
-				Required: true,
+				Optional: true,
 			},
 		},
 	},
@@ -153,7 +171,7 @@ func SecurityListResource() *schema.Resource {
 			},
 			"egress_security_rules": {
 				Type:     schema.TypeList,
-				Required: true,
+				Optional: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"destination": {
@@ -181,7 +199,7 @@ func SecurityListResource() *schema.Resource {
 			},
 			"ingress_security_rules": {
 				Type:     schema.TypeList,
-				Required: true,
+				Optional: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"icmp_options": icmpSchema,
@@ -336,48 +354,20 @@ func (s *SecurityListResourceCrud) Update() (e error) {
 	}
 
 	s.Res, e = s.Client.UpdateSecurityList(s.D.Id(), opts)
+
 	return
 }
 
 func (s *SecurityListResourceCrud) SetData() {
 	s.D.Set("compartment_id", s.Res.CompartmentID)
 	s.D.Set("display_name", s.Res.DisplayName)
-
-	confEgressRules := []map[string]interface{}{}
-	for _, egressRule := range s.Res.EgressSecurityRules {
-		confEgressRule := map[string]interface{}{}
-		confEgressRule["destination"] = egressRule.Destination
-		confEgressRule = buildConfRule(
-			confEgressRule,
-			egressRule.Protocol,
-			egressRule.ICMPOptions,
-			egressRule.TCPOptions,
-			egressRule.UDPOptions,
-			&egressRule.IsStateless,
-		)
-		confEgressRules = append(confEgressRules, confEgressRule)
-	}
-	s.D.Set("egress_security_rules", confEgressRules)
-
-	confIngressRules := []map[string]interface{}{}
-	for _, ingressRule := range s.Res.IngressSecurityRules {
-		confIngressRule := map[string]interface{}{}
-		confIngressRule["source"] = ingressRule.Source
-		confIngressRule = buildConfRule(
-			confIngressRule,
-			ingressRule.Protocol,
-			ingressRule.ICMPOptions,
-			ingressRule.TCPOptions,
-			ingressRule.UDPOptions,
-			&ingressRule.IsStateless,
-		)
-		confIngressRules = append(confIngressRules, confIngressRule)
-	}
-	s.D.Set("ingress_security_rules", confIngressRules)
-
 	s.D.Set("state", s.Res.State)
 	s.D.Set("time_created", s.Res.TimeCreated.String())
 	s.D.Set("vcn_id", s.Res.VcnID)
+
+	confEgressRules, confIngressRules := buildConfRuleLists(s.Res)
+	s.D.Set("egress_security_rules", confEgressRules)
+	s.D.Set("ingress_security_rules", confIngressRules)
 }
 
 func (s *SecurityListResourceCrud) reset() (e error) {
@@ -453,69 +443,137 @@ func (s *SecurityListResourceCrud) buildICMPOptions(conf map[string]interface{})
 }
 
 func (s *SecurityListResourceCrud) buildTCPOptions(conf map[string]interface{}) (opts *baremetal.TCPOptions) {
-	l := conf["tcp_options"].([]interface{})
-	if len(l) > 0 {
-		confOpts := l[0].(map[string]interface{})
+	options := conf["tcp_options"].([]interface{})
+	if len(options) > 0 {
+		sourcePortRange, destinationPortRange := s.buildSourceAndDestinationPortRanges(options)
 		opts = &baremetal.TCPOptions{
-			baremetal.PortRange{
-				Max: uint64(confOpts["max"].(int)),
-				Min: uint64(confOpts["min"].(int)),
-			},
+			DestinationPortRange: destinationPortRange,
+			SourcePortRange:      sourcePortRange,
 		}
 	}
 	return
 }
 
 func (s *SecurityListResourceCrud) buildUDPOptions(conf map[string]interface{}) (opts *baremetal.UDPOptions) {
-	l := conf["udp_options"].([]interface{})
-	if len(l) > 0 {
-		confOpts := l[0].(map[string]interface{})
+	options := conf["udp_options"].([]interface{})
+	if len(options) > 0 {
+		sourcePortRange, destinationPortRange := s.buildSourceAndDestinationPortRanges(options)
 		opts = &baremetal.UDPOptions{
-			baremetal.PortRange{
-				Max: uint64(confOpts["max"].(int)),
-				Min: uint64(confOpts["min"].(int)),
-			},
+			DestinationPortRange: destinationPortRange,
+			SourcePortRange:      sourcePortRange,
 		}
 	}
 	return
 }
 
-func buildConfICMPOptions(opts *baremetal.ICMPOptions) (list []interface{}) {
-	confOpts := map[string]interface{}{
-		"code": int(opts.Code),
-		"type": int(opts.Type),
+func buildPortRange(conf []interface{}) (portRange *baremetal.PortRange) {
+	if len(conf) > 0 && conf[0] != nil {
+		mapConf := conf[0].(map[string]interface{})
+
+		max := mapConf["max"].(int)
+		min := mapConf["min"].(int)
+
+		// Max and Min default to 0, and that is not a valid port number, so we can assume that if
+		// the value is 0 then the user has not set the port number.
+		// Also, note that if either max or min is set, then the service will return an error if both are not
+		// set. However, we want to create the PortRange if either is set and let the service return the error.
+		if max != 0 || min != 0 {
+			portRange = &baremetal.PortRange{
+				Max: uint64(max),
+				Min: uint64(min),
+			}
+		}
 	}
-	return []interface{}{confOpts}
+	return
 }
 
-func buildConfTransportOptions(portRange baremetal.PortRange) (list []interface{}) {
-	confOpts := map[string]interface{}{
-		"max": int(portRange.Max),
-		"min": int(portRange.Min),
+func (s *SecurityListResourceCrud) buildSourceAndDestinationPortRanges(conf []interface{}) (sourcePortRange, destinationPortRange *baremetal.PortRange) {
+	if len(conf) > 0 && conf[0] != nil {
+		mapConf := conf[0].(map[string]interface{})
+		sourcePortRange = buildPortRange(mapConf["source_port_range"].([]interface{}))
+		destinationPortRange = buildPortRange(conf)
 	}
-	return []interface{}{confOpts}
+
+	return
+}
+
+// Used to build rule lists for SetData.
+func buildConfRuleLists(res *baremetal.SecurityList) (confEgressRules, confIngressRules []map[string]interface{}) {
+	for _, egressRule := range res.EgressSecurityRules {
+		confEgressRule := buildConfRule(
+			egressRule.Protocol,
+			egressRule.ICMPOptions,
+			egressRule.TCPOptions,
+			egressRule.UDPOptions,
+			&egressRule.IsStateless,
+		)
+		confEgressRule["destination"] = egressRule.Destination
+		confEgressRules = append(confEgressRules, confEgressRule)
+	}
+
+	for _, ingressRule := range res.IngressSecurityRules {
+		confIngressRule := buildConfRule(
+			ingressRule.Protocol,
+			ingressRule.ICMPOptions,
+			ingressRule.TCPOptions,
+			ingressRule.UDPOptions,
+			&ingressRule.IsStateless,
+		)
+		confIngressRule["source"] = ingressRule.Source
+		confIngressRules = append(confIngressRules, confIngressRule)
+	}
+
+	return
 }
 
+// Used to build rules for SetData.
 func buildConfRule(
-	confRule map[string]interface{},
 	protocol string,
 	icmpOpts *baremetal.ICMPOptions,
 	tcpOpts *baremetal.TCPOptions,
 	udpOpts *baremetal.UDPOptions,
 	stateless *bool,
-) map[string]interface{} {
+) (confRule map[string]interface{}) {
+	confRule = map[string]interface{}{}
 	confRule["protocol"] = protocol
 	if icmpOpts != nil {
 		confRule["icmp_options"] = buildConfICMPOptions(icmpOpts)
 	}
 	if tcpOpts != nil {
-		confRule["tcp_options"] = buildConfTransportOptions(tcpOpts.DestinationPortRange)
+		confRule["tcp_options"] = buildConfTransportOptions(tcpOpts.DestinationPortRange, tcpOpts.SourcePortRange)
 	}
 	if udpOpts != nil {
-		confRule["udp_options"] = buildConfTransportOptions(udpOpts.DestinationPortRange)
+		confRule["udp_options"] = buildConfTransportOptions(udpOpts.DestinationPortRange, udpOpts.SourcePortRange)
 	}
 	if stateless != nil {
 		confRule["stateless"] = *stateless
 	}
 	return confRule
 }
+
+// Used to build ICMP options for SetData.
+func buildConfICMPOptions(opts *baremetal.ICMPOptions) (list []interface{}) {
+	confOpts := map[string]interface{}{
+		"code": int(opts.Code),
+		"type": int(opts.Type),
+	}
+	return []interface{}{confOpts}
+}
+
+// Used to build TCP/UDP port ranges for SetData.
+func buildConfTransportOptions(destinationPortRange *baremetal.PortRange, sourcePortRange *baremetal.PortRange) (list []interface{}) {
+	confOpts := map[string]interface{}{}
+	if destinationPortRange != nil {
+		confOpts["max"] = int(destinationPortRange.Max)
+		confOpts["min"] = int(destinationPortRange.Min)
+	}
+
+	if sourcePortRange != nil {
+		confOpts["source_port_range"] = []interface{}{map[string]interface{}{
+			"max": int(sourcePortRange.Max),
+			"min": int(sourcePortRange.Min),
+		}}
+	}
+
+	return []interface{}{confOpts}
+}