Add rework dns forwarders and add localonly dns option to libvirt-net…

…work

I make a new DNS a sectrion of network, more like the upstream xml. Inside
that new DNS section I include forwarders, which were under the main object,
so this is a breaking change. But one that aligns with the upstream xml and
one which allows much easier future growth of DNS management.

I add 'localOnly' which tells dnsmasq to own all of the domain in question.
Instead of forwarding unknown names. This way you can point your local
DNS at the libvirt DNS for the domain in question, and the domain in
question will not forward unknown hosts back to your local DNS.

libvirt/resource_libvirt_network.go

@@ -18,6 +18,7 @@ const (
 	netModeNat      = "nat"
 	netModeRoute    = "route"
 	netModeBridge   = "bridge"
+	dnsPrefix       = "dns.0"
 )
 
 // a libvirt network resource
@@ -80,23 +81,39 @@ func resourceLibvirtNetwork() *schema.Resource {
 				Optional: true,
 				Required: false,
 			},
-			"dns_forwarder": {
+			"dns": {
 				Type:     schema.TypeList,
 				Optional: true,
 				ForceNew: true,
+				MaxItems: 1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
-						"address": {
-							Type:     schema.TypeString,
+						"local_only": {
+							Type:     schema.TypeBool,
+							Default:  false,
 							Optional: true,
 							Required: false,
-							ForceNew: true,
 						},
-						"domain": {
-							Type:     schema.TypeString,
+						"forwarders": {
+							Type:     schema.TypeList,
 							Optional: true,
-							Required: false,
 							ForceNew: true,
+							Elem: &schema.Resource{
+								Schema: map[string]*schema.Schema{
+									"address": {
+										Type:     schema.TypeString,
+										Optional: true,
+										Required: false,
+										ForceNew: true,
+									},
+									"domain": {
+										Type:     schema.TypeString,
+										Optional: true,
+										Required: false,
+										ForceNew: true,
+									},
+								},
+							},
 						},
 					},
 				},
@@ -185,8 +202,17 @@ func resourceLibvirtNetworkCreate(d *schema.ResourceData, meta interface{}) erro
 
 	networkDef := newNetworkDef()
 	networkDef.Name = d.Get("name").(string)
-	networkDef.Domain = &libvirtxml.NetworkDomain{
-		Name: d.Get("domain").(string),
+
+	if domain, ok := d.GetOk("domain"); ok {
+		networkDef.Domain = &libvirtxml.NetworkDomain{
+			Name: domain.(string),
+		}
+
+		if dnsLocalOnly, ok := d.GetOk(dnsPrefix + ".local_only"); ok {
+			if dnsLocalOnly.(bool) {
+				networkDef.Domain.LocalOnly = "yes" // this "boolean" must be "yes"|"no"
+			}
+		}
 	}
 
 	// use a bridge provided by the user, or create one otherwise (libvirt will assign on automatically when empty)
@@ -217,14 +243,14 @@ func resourceLibvirtNetworkCreate(d *schema.ResourceData, meta interface{}) erro
 		if err != nil {
 			return fmt.Errorf("Could not set DHCP from adresses '%s'", err)
 		}
-		if dnsForwardCount, ok := d.GetOk("dns_forwarder.#"); ok {
+		if dnsForwardCount, ok := d.GetOk(dnsPrefix + ".forwarders.#"); ok {
 			dns := libvirtxml.NetworkDNS{
 				Forwarders: []libvirtxml.NetworkDNSForwarder{},
 			}
 
 			for i := 0; i < dnsForwardCount.(int); i++ {
 				forward := libvirtxml.NetworkDNSForwarder{}
-				forwardPrefix := fmt.Sprintf("dns_forwarder.%d", i)
+				forwardPrefix := fmt.Sprintf(dnsPrefix+".forwarders.%d", i)
 				if address, ok := d.GetOk(forwardPrefix + ".address"); ok {
 					ip := net.ParseIP(address.(string))
 					if ip == nil {
@@ -333,6 +359,7 @@ func resourceLibvirtNetworkRead(d *schema.ResourceData, meta interface{}) error
 	// Domain as won't be present for bridged networks
 	if networkDef.Domain != nil {
 		d.Set("domain", networkDef.Domain.Name)
+		d.Set(dnsPrefix+".local_only", strings.ToLower(networkDef.Domain.LocalOnly) == "yes")
 	}
 
 	autostart, err := network.GetAutostart()
@@ -361,6 +388,17 @@ func resourceLibvirtNetworkRead(d *schema.ResourceData, meta interface{}) error
 		d.Set("addresses", addresses)
 	}
 
+	if networkDef.DNS != nil {
+		for i, forwarder := range networkDef.DNS.Forwarders {
+			key := fmt.Sprintf(dnsPrefix+".forwarders.%d", i)
+			if len(forwarder.Addr) > 0 {
+				d.Set(key+".address", forwarder.Addr)
+			}
+			if len(forwarder.Domain) > 0 {
+				d.Set(key+".domain", forwarder.Domain)
+			}
+		}
+	}
 	// TODO: get any other parameters from the network and save them
 
 	log.Printf("[DEBUG] Network ID %s successfully read", d.Id())

libvirt/resource_libvirt_network_test.go

@@ -2,13 +2,160 @@ package libvirt
 
 import (
 	"fmt"
+	"reflect"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/libvirt/libvirt-go"
+	"github.com/libvirt/libvirt-go-xml"
 )
 
+func getNetworkDef(s *terraform.State, name string) (*libvirtxml.Network, error) {
+	var network *libvirt.Network
+	rs, ok := s.RootModule().Resources[name]
+	if !ok {
+		return nil, fmt.Errorf("Not found: %s", name)
+	}
+	if rs.Primary.ID == "" {
+		return nil, fmt.Errorf("No libvirt network ID is set")
+	}
+	virConn := testAccProvider.Meta().(*Client).libvirt
+	network, err := virConn.LookupNetworkByUUIDString(rs.Primary.ID)
+	if err != nil {
+		return nil, err
+	}
+	networkDef, err := newDefNetworkfromLibvirt(network)
+	if err != nil {
+		return nil, fmt.Errorf("Error reading libvirt network XML description: %s", err)
+	}
+	return &networkDef, nil
+}
+
+func TestAccCheckLibvirtNetwork_LocalOnly(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckLibvirtNetworkDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(`
+				resource "libvirt_network" "test_net" {
+					name      = "networktest"
+					domain    = "k8s.local"
+					addresses = ["10.17.3.0/24"]
+					dns {
+						local_only = true
+					}
+				}`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("libvirt_network.test_net", "dns.0.local_only", "true"),
+					checkLocalOnly("libvirt_network.test_net", true),
+				),
+			},
+		},
+	})
+}
+
+func checkLocalOnly(name string, expectLocalOnly bool) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		networkDef, err := getNetworkDef(s, name)
+		if err != nil {
+			return err
+		}
+		if expectLocalOnly {
+			if networkDef.Domain == nil || networkDef.Domain.LocalOnly != "yes" {
+				return fmt.Errorf("networkDef.Domain.LocalOnly is not true")
+			}
+		} else {
+			if networkDef.Domain != nil && networkDef.Domain.LocalOnly != "no" {
+				return fmt.Errorf("networkDef.Domain.LocalOnly is true")
+			}
+		}
+		return nil
+	}
+}
+
+func TestAccCheckLibvirtNetwork_DNSForwarders(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckLibvirtNetworkDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(`
+				resource "libvirt_network" "test_net" {
+					name      = "networktest"
+					domain    = "k8s.local"
+					addresses = ["10.17.3.0/24"]
+					dns {
+						forwarders = [
+						  {
+						    address = "8.8.8.8",
+					          },
+						  {
+						    address = "10.10.0.67",
+						    domain = "my.domain.com",
+						  },
+						  {
+						    domain = "hello.com",
+						  },
+						]
+					}
+				}`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("libvirt_network.test_net", "dns.0.forwarders.#", "3"),
+					resource.TestCheckResourceAttr("libvirt_network.test_net", "dns.0.forwarders.0.address", "8.8.8.8"),
+					resource.TestCheckResourceAttr("libvirt_network.test_net", "dns.0.forwarders.1.address", "10.10.0.67"),
+					resource.TestCheckResourceAttr("libvirt_network.test_net", "dns.0.forwarders.1.domain", "my.domain.com"),
+					resource.TestCheckResourceAttr("libvirt_network.test_net", "dns.0.forwarders.2.domain", "hello.com"),
+					checkDNSForwarders("libvirt_network.test_net", []libvirtxml.NetworkDNSForwarder{
+						{
+							Addr: "8.8.8.8",
+						},
+						{
+							Addr:   "10.10.0.67",
+							Domain: "my.domain.com",
+						},
+						{
+							Domain: "hello.com",
+						},
+					}),
+				),
+			},
+		},
+	})
+}
+
+func checkDNSForwarders(name string, expected []libvirtxml.NetworkDNSForwarder) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		networkDef, err := getNetworkDef(s, name)
+		if err != nil {
+			return err
+		}
+		if networkDef.DNS == nil {
+			return fmt.Errorf("DNS block not found in networkDef")
+		}
+		actual := networkDef.DNS.Forwarders
+		if len(expected) != len(actual) {
+			return fmt.Errorf("len(expected): %d != len(actual): %d", len(expected), len(actual))
+		}
+		for _, e := range expected {
+			found := false
+			for _, a := range actual {
+				if reflect.DeepEqual(a, e) {
+					found = true
+					break
+				}
+			}
+			if !found {
+				return fmt.Errorf("Unable to find %v in %v", e, actual)
+			}
+		}
+		return nil
+	}
+}
+
 func networkExists(n string, network *libvirt.Network) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]

website/docs/r/network.markdown

@@ -34,12 +34,25 @@ resource "libvirt_network" "kube_network" {
   # (only necessary in "bridge" mode)
   # bridge = "br7"
 
-  # (Optional) one or more DNS forwarder entries.  One or both of
-  # "address" and "domain" must be specified.  The format is:
-  # dns_forwarder {
-  #   address = "my address"
-  #   domain = "my domain"
-  # }
+  # (Optional) DNS configuration
+  dns {
+    # (Optional, default false)
+    # true: DNS requests under this domain will only be resolved by the
+    # virtual network's own DNS server
+    # false: Unresolved requests will be forwarded to the host's
+    # upstream DNS server if the virtual network's DNS server does not
+    # have an answer.
+   local_only = true
+
+    # (Optional) one or more DNS forwarder entries.  One or both of
+    # "address" and "domain" must be specified.  The format is:
+    # forwarders = [
+    #   {
+    #     address = "my address"
+    #     domain = "my domain"
+    #   }
+    # ]
+  }
 }
 ```
 
@@ -76,40 +89,14 @@ The following arguments are supported:
 * `bridge` - (Optional) The bridge device defines the name of a bridge
    device which will be used to construct the virtual network (when not provided,
    it will be automatically obtained by libvirt in `none`, `nat` and `route` modes).
-*  `dns_forwarder` - (Optional) a DNS forwarder entry block.  You can have
-   one or mode of these blocks in your network definition.  You must specify one or
-   both of `address` and `domain`.  You can use either of the forms below to
-   specify dns_forwarders:
 * `autostart` - (Optional) Set to `true` to start the network on host boot up.
   If not specified `false` is assumed.
+* `dns` - (Optional) configuration of DNS specific settings for the network
 
-```hcl
-resource "libvirt_network" "my_network" {
-  ...
-  dns_forwarder {
-    address = "my address"
-  }
-  dns_forwarder {
-    address = "my address 1"
-    domain = "my domain"
-  }
-}
-```
+Inside of `dns` section the following argument are supported:
+* `local_only` - (Optional) true/false: true means 'do not forward unresolved requests for this domain to the part DNS server
+* `forwarders` - (Optional) Either `address`, `domain`, or both must be set
 
-```hcl
-resource "libvirt_network" "my_network" {
-  ...
-  dns_forwarder = [
-    {
-      address = "my address"
-    },
-    {
-      address = "my address 1"
-      domain = "my domain
-    }
-  ]
-}
-```
 
 * `dhcp` - (Optional) DHCP configuration. 
    You need to use it in conjuction with the adresses variable.