Fix peerdns attribute

The way we setup `peerdns` isn't correct. We don't want to require both
that the `proto` be `"dhcp"` and `"dhcpv6"`. That doesn't even make
sense. And the fact that we can say that makes even less sense.

In order to show the failure, we add a failing test. We then implement a
fix by adding a validator that disjoins multiple other validators. The
test is effectively what we want in reality, so it passing here should
give good confidence that we can do what we want.

It's unfortunate that we have to write this validator. Hopefully, it's
an oversight and not intentional. We don't want to maintain this
indefinitely. See
hashicorp/terraform-plugin-framework-validators#122
for the upstream issue.

Branch: joneshf/failing-peerdns-test
Pull-Request: #110

openwrt/internal/lucirpcglue/attribute.go

@@ -3,6 +3,7 @@ package lucirpcglue
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/hashicorp/terraform-plugin-framework-validators/helpers/validatordiag"
 	"github.com/hashicorp/terraform-plugin-framework/attr"
@@ -30,13 +31,56 @@ const (
 )
 
 var (
+	_ validator.Bool = anyValidatorBool{}
+
 	_ validator.Bool   = requiresAttribute[any]{}
 	_ validator.Int64  = requiresAttribute[any]{}
 	_ validator.List   = requiresAttribute[any]{}
 	_ validator.Set    = requiresAttribute[any]{}
 	_ validator.String = requiresAttribute[any]{}
 )
 
+// AnyBool returns a validator which ensures that any configured attribute value passes at least one of the given validators.
+func AnyBool(validators ...validator.Bool) validator.Bool {
+	return anyValidatorBool{
+		validators: validators,
+	}
+}
+
+type anyValidatorBool struct {
+	validators []validator.Bool
+}
+
+func (v anyValidatorBool) Description(ctx context.Context) string {
+	var descriptions []string
+
+	for _, subValidator := range v.validators {
+		descriptions = append(descriptions, subValidator.Description(ctx))
+	}
+
+	return fmt.Sprintf("Value must satisfy at least one of the validations: %s", strings.Join(descriptions, " + "))
+}
+
+func (v anyValidatorBool) MarkdownDescription(ctx context.Context) string {
+	return v.Description(ctx)
+}
+
+func (v anyValidatorBool) ValidateBool(ctx context.Context, req validator.BoolRequest, resp *validator.BoolResponse) {
+	for _, subValidator := range v.validators {
+		validateResp := &validator.BoolResponse{}
+
+		subValidator.ValidateBool(ctx, req, validateResp)
+
+		if !validateResp.Diagnostics.HasError() {
+			resp.Diagnostics = validateResp.Diagnostics
+
+			return
+		}
+
+		resp.Diagnostics.Append(validateResp.Diagnostics...)
+	}
+}
+
 type AttributeExistence int
 
 func (e AttributeExistence) ToComputed() bool {

openwrt/internal/network/interface.go

@@ -224,13 +224,15 @@ var (
 		ResourceExistence: lucirpcglue.NoValidation,
 		UpsertRequest:     lucirpcglue.UpsertRequestOptionBool(interfaceModelGetPeerDNS, interfacePeerDNSAttribute, interfacePeerDNSUCIOption),
 		Validators: []validator.Bool{
-			lucirpcglue.RequiresAttributeEqualString(
-				path.MatchRoot(interfaceProtocolAttribute),
-				interfaceProtocolDHCP,
-			),
-			lucirpcglue.RequiresAttributeEqualString(
-				path.MatchRoot(interfaceProtocolAttribute),
-				interfaceProtocolDHCPV6,
+			lucirpcglue.AnyBool(
+				lucirpcglue.RequiresAttributeEqualString(
+					path.MatchRoot(interfaceProtocolAttribute),
+					interfaceProtocolDHCP,
+				),
+				lucirpcglue.RequiresAttributeEqualString(
+					path.MatchRoot(interfaceProtocolAttribute),
+					interfaceProtocolDHCPV6,
+				),
 			),
 		},
 	}

openwrt/internal/network/interface_acceptance_test.go

@@ -134,3 +134,89 @@ resource "openwrt_network_interface" "testing" {
 		updateAndReadResource,
 	)
 }
+
+func TestNetworkInterfaceResourcePeerDNSWithDHCPAcceptance(t *testing.T) {
+	t.Parallel()
+
+	ctx := context.Background()
+	providerBlock := acceptancetest.RunOpenWrtServerWithProviderBlock(
+		ctx,
+		*dockerPool,
+		t,
+	)
+
+	step := resource.TestStep{
+		Config: fmt.Sprintf(`
+%s
+
+resource "openwrt_network_interface" "testing" {
+	device = "br-testing"
+	dns = [
+		"9.9.9.9",
+		"1.1.1.1",
+	]
+	id = "testing"
+	peerdns = false
+	proto = "dhcp"
+}
+`,
+			providerBlock,
+		),
+		Check: resource.ComposeAggregateTestCheckFunc(
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "id", "testing"),
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "device", "br-testing"),
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "dns.0", "9.9.9.9"),
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "dns.1", "1.1.1.1"),
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "peerdns", "false"),
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "proto", "dhcp"),
+		),
+	}
+
+	acceptancetest.TerraformSteps(
+		t,
+		step,
+	)
+}
+
+func TestNetworkInterfaceResourcePeerDNSWithDHCPV6Acceptance(t *testing.T) {
+	t.Parallel()
+
+	ctx := context.Background()
+	providerBlock := acceptancetest.RunOpenWrtServerWithProviderBlock(
+		ctx,
+		*dockerPool,
+		t,
+	)
+
+	step := resource.TestStep{
+		Config: fmt.Sprintf(`
+%s
+
+resource "openwrt_network_interface" "testing" {
+	device = "br-testing"
+	dns = [
+		"9.9.9.9",
+		"1.1.1.1",
+	]
+	id = "testing"
+	peerdns = false
+	proto = "dhcpv6"
+}
+`,
+			providerBlock,
+		),
+		Check: resource.ComposeAggregateTestCheckFunc(
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "id", "testing"),
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "device", "br-testing"),
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "dns.0", "9.9.9.9"),
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "dns.1", "1.1.1.1"),
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "peerdns", "false"),
+			resource.TestCheckResourceAttr("openwrt_network_interface.testing", "proto", "dhcpv6"),
+		),
+	}
+
+	acceptancetest.TerraformSteps(
+		t,
+		step,
+	)
+}