Merge pull request #293 from jeremmfr/issue-292

Fix #292: add missing information in some errors

CHANGELOG.md

@@ -5,6 +5,9 @@ ENHANCEMENTS:
 
 BUG FIXES:
 
+* module go-netconf updated to enhance RPCError display with the `error-path` and `error-info>bad-element` values if set (Fixes parts of #292)
+* r/`*`: fix missing identifier value in the errors `multiple blocks with the same identifier` (Fixes parts of #292)
+
 ## 1.21.0 (October 12, 2021)
 
 FEATURES:

go.mod

@@ -5,7 +5,7 @@ go 1.16
 require (
 	github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.8.0
-	github.com/jeremmfr/go-netconf v0.3.2
+	github.com/jeremmfr/go-netconf v0.4.0
 	github.com/jeremmfr/go-utils v0.3.0
 	github.com/jeremmfr/junosdecode v1.1.0
 	golang.org/x/crypto v0.0.0-20210921155107-089bfa567519

go.sum

@@ -203,8 +203,8 @@ github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
 github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
 github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A=
 github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo=
-github.com/jeremmfr/go-netconf v0.3.2 h1:9U55VX6sYCfy8JgjUxjmesoaon4cWa/jVVxFzQnTmAA=
-github.com/jeremmfr/go-netconf v0.3.2/go.mod h1:AHzLyFRHXf4z7N2KK5+UYjX1bljd23tfuNq+YYRr+nA=
+github.com/jeremmfr/go-netconf v0.4.0 h1:MYrbA5keH7bsBpgcTA0MVvIg6oiA2bgdsJkdE2F3I7Y=
+github.com/jeremmfr/go-netconf v0.4.0/go.mod h1:Nd1B4xd6miAKQzNxe0m3wddyLdAeKK+N04MOAiVhE0c=
 github.com/jeremmfr/go-utils v0.3.0 h1:f69G5Z6EnHO2bNEW+aeWB22P/4/plS1DZ2UZK7gd+WI=
 github.com/jeremmfr/go-utils v0.3.0/go.mod h1:K0lGadiSvg9OKGJnW4Bs3t18/VApp/6x2+BV93Sts2M=
 github.com/jeremmfr/junosdecode v1.1.0 h1:Os8QeOzyL+BPuDZJMjyJgz4QPOgA8EChgKB2Ih5wwCc=
@@ -325,7 +325,6 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh
 golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4=
 golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4=
-golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 h1:7I4JAnoQBe7ZtJcBaYHi5UtiO8tQHbUSXxL+pnGRANg=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=

junos/func_resource_bgp.go

@@ -574,11 +574,11 @@ func setBgpOptsFamily(setPrefix, familyType string, familyOptsList []interface{}
 		if bchk.StringInSlice(familyOptsM["nlri_type"].(string), familyNlriTypeList) {
 			switch familyType {
 			case evpnWord:
-				return fmt.Errorf("multiple family_evpn blocks with the same nlri_type")
+				return fmt.Errorf("multiple blocks family_evpn with the same nlri_type %s", familyOptsM["nlri_type"].(string))
 			case inetWord:
-				return fmt.Errorf("multiple family_inet blocks with the same nlri_type")
+				return fmt.Errorf("multiple blocks family_inet with the same nlri_type %s", familyOptsM["nlri_type"].(string))
 			case inet6Word:
-				return fmt.Errorf("multiple family_inet6 blocks with the same nlri_type")
+				return fmt.Errorf("multiple blocks family_inet6 with the same nlri_type %s", familyOptsM["nlri_type"].(string))
 			}
 		}
 		familyNlriTypeList = append(familyNlriTypeList, familyOptsM["nlri_type"].(string))

junos/netconf.go

@@ -14,7 +14,7 @@ import (
 )
 
 const (
-	warningSeverity string = "warning"
+	errorSeverity string = "error"
 
 	rpcCommand         = "<command format=\"text\">%s</command>"
 	rpcConfigStringSet = "<load-configuration action=\"set\" format=\"text\">" +
@@ -55,16 +55,9 @@ type netconfAuthMethod struct {
 	Ciphers        []string
 }
 
-type commitError struct {
-	Path     string `xml:"error-path"`
-	Element  string `xml:"error-info>bad-element"`
-	Message  string `xml:"error-message"`
-	Severity string `xml:"error-severity"`
-}
-
 type commitResults struct {
-	XMLName xml.Name      `xml:"commit-results"`
-	Errors  []commitError `xml:"rpc-error"`
+	XMLName xml.Name           `xml:"commit-results"`
+	Errors  []netconf.RPCError `xml:"rpc-error"`
 }
 
 // netconfNewSession establishes a new connection to a NetconfObject device that we will use
@@ -193,7 +186,7 @@ func (j *NetconfObject) netconfCommand(cmd string) (string, error) {
 	}
 	if reply.Errors != nil {
 		for _, m := range reply.Errors {
-			return "", errors.New(m.Message)
+			return "", errors.New(m.Error())
 		}
 	}
 	if reply.Data == "" || strings.Count(reply.Data, "") <= 2 {
@@ -214,7 +207,7 @@ func (j *NetconfObject) netconfCommandXML(cmd string) (string, error) {
 	}
 	if reply.Errors != nil {
 		for _, m := range reply.Errors {
-			return "", errors.New(m.Message)
+			return "", errors.New(m.Error())
 		}
 	}
 
@@ -298,10 +291,10 @@ func (j *NetconfObject) netconfCommit(logMessage string) (_warn []error, _err er
 	if reply.Errors != nil {
 		warnings := make([]error, 0)
 		for _, m := range reply.Errors {
-			if m.Severity != warningSeverity {
-				return warnings, errors.New(m.Message)
+			if m.Severity == errorSeverity {
+				return warnings, errors.New(m.Error())
 			}
-			warnings = append(warnings, errors.New(m.Message))
+			warnings = append(warnings, errors.New(m.Error()))
 		}
 
 		return warnings, nil
@@ -317,15 +310,10 @@ func (j *NetconfObject) netconfCommit(logMessage string) (_warn []error, _err er
 		if errs.Errors != nil {
 			warnings := make([]error, 0)
 			for _, m := range errs.Errors {
-				if m.Severity != warningSeverity {
-					message := fmt.Sprintf("[%s]\n    %s\nError: %s",
-						strings.Trim(m.Path, "[\r\n]"),
-						strings.Trim(m.Element, "[\r\n]"),
-						strings.Trim(m.Message, "[\r\n]"))
-
-					return []error{}, errors.New(message)
+				if m.Severity == errorSeverity {
+					return []error{}, errors.New(m.Error())
 				}
-				warnings = append(warnings, errors.New(m.Message))
+				warnings = append(warnings, errors.New(m.Error()))
 			}
 
 			return warnings, nil

junos/resource_chassis_cluster.go

@@ -365,7 +365,7 @@ func setChassisCluster(d *schema.ResourceData, m interface{}, jnprSess *NetconfO
 		for _, v2 := range redundancyGroup["interface_monitor"].([]interface{}) {
 			interfaceMonitor := v2.(map[string]interface{})
 			if bchk.StringInSlice(interfaceMonitor["name"].(string), interfaceMonitorNameList) {
-				return fmt.Errorf("multiple interface_monitor blocks with the same name")
+				return fmt.Errorf("multiple blocks interface_monitor with the same name %s", interfaceMonitor["name"].(string))
 			}
 			interfaceMonitorNameList = append(interfaceMonitorNameList, interfaceMonitor["name"].(string))
 			configSet = append(configSet, setChassisluster+"redundancy-group "+strconv.Itoa(i)+

junos/resource_eventoptions_destination.go

@@ -255,7 +255,7 @@ func setEventoptionsDestination(d *schema.ResourceData, m interface{}, jnprSess
 	for _, v := range d.Get("archive_site").([]interface{}) {
 		archiveSite := v.(map[string]interface{})
 		if bchk.StringInSlice(archiveSite["url"].(string), archiveSiteURLList) {
-			return fmt.Errorf("multiple archive_site blocks with the same url")
+			return fmt.Errorf("multiple blocks archive_site with the same url %s", archiveSite["url"].(string))
 		}
 		archiveSiteURLList = append(archiveSiteURLList, archiveSite["url"].(string))
 		configSet = append(configSet, setPrefix+"archive-sites \""+archiveSite["url"].(string)+"\"")

junos/resource_eventoptions_policy.go

@@ -691,7 +691,7 @@ func setEventoptionsPolicy(d *schema.ResourceData, m interface{}, jnprSess *Netc
 		for _, v2 := range then["event_script"].([]interface{}) {
 			eventScript := v2.(map[string]interface{})
 			if bchk.StringInSlice(eventScript["filename"].(string), eventScriptFilenameList) {
-				return fmt.Errorf("multiple event_script blocks with the same filename")
+				return fmt.Errorf("multiple blocks event_script with the same filename %s", eventScript["filename"].(string))
 			}
 			eventScriptFilenameList = append(eventScriptFilenameList, eventScript["filename"].(string))
 			setPrefixThenEventScript := setPrefix + "then event-script \"" + eventScript["filename"].(string) + "\" "
@@ -700,7 +700,7 @@ func setEventoptionsPolicy(d *schema.ResourceData, m interface{}, jnprSess *Netc
 			for _, v3 := range eventScript["arguments"].([]interface{}) {
 				arguments := v3.(map[string]interface{})
 				if bchk.StringInSlice(arguments["name"].(string), argumentsNameList) {
-					return fmt.Errorf("multiple arguments blocks with the same name")
+					return fmt.Errorf("multiple blocks arguments with the same name %s", arguments["name"].(string))
 				}
 				argumentsNameList = append(argumentsNameList, arguments["name"].(string))
 				configSet = append(configSet, setPrefixThenEventScript+
@@ -785,7 +785,8 @@ func setEventoptionsPolicy(d *schema.ResourceData, m interface{}, jnprSess *Netc
 			setPrefixThenUpload := setPrefix + "then upload filename \"" + upload["filename"].(string) + "\" " +
 				"destination \"" + upload["destination"].(string) + "\" "
 			if bchk.StringInSlice(setPrefixThenUpload, uploadFileDestList) {
-				return fmt.Errorf("multiple upload blocks with the same filename and destination")
+				return fmt.Errorf("multiple blocks upload with the same filename %s and destination %s",
+					upload["filename"].(string), upload["destination"].(string))
 			}
 			uploadFileDestList = append(uploadFileDestList, setPrefixThenUpload)
 			configSet = append(configSet, setPrefixThenUpload)
@@ -813,7 +814,8 @@ func setEventoptionsPolicy(d *schema.ResourceData, m interface{}, jnprSess *Netc
 		setAttriMatch := setPrefix + "attributes-match \"" + attriMatch["from"].(string) + "\" " +
 			attriMatch["compare"].(string) + " \"" + attriMatch["to"].(string) + "\""
 		if bchk.StringInSlice(setAttriMatch, attriMatchList) {
-			return fmt.Errorf("multiple attributes_match blocks with the same from, compare and to")
+			return fmt.Errorf("multiple blocks attributes_match with the same from %s, compare %s and to %s",
+				attriMatch["from"].(string), attriMatch["compare"].(string), attriMatch["to"].(string))
 		}
 		attriMatchList = append(attriMatchList, setAttriMatch)
 		configSet = append(configSet, setAttriMatch)
@@ -822,7 +824,7 @@ func setEventoptionsPolicy(d *schema.ResourceData, m interface{}, jnprSess *Netc
 	for _, v := range d.Get("within").([]interface{}) {
 		within := v.(map[string]interface{})
 		if bchk.IntInSlice(within["time_interval"].(int), withinTimeInterval) {
-			return fmt.Errorf("multiple within blocks with the same time_interval")
+			return fmt.Errorf("multiple blocks within with the same time_interval %d", within["time_interval"].(int))
 		}
 		withinTimeInterval = append(withinTimeInterval, within["time_interval"].(int))
 		setPrefixWithin := setPrefix + "within " + strconv.Itoa(within["time_interval"].(int)) + " "

junos/resource_firewall_filter.go

@@ -466,7 +466,7 @@ func setFirewallFilter(d *schema.ResourceData, m interface{}, jnprSess *NetconfO
 	for _, v := range d.Get("term").([]interface{}) {
 		term := v.(map[string]interface{})
 		if bchk.StringInSlice(term["name"].(string), termNameList) {
-			return fmt.Errorf("multiple term blocks with the same name")
+			return fmt.Errorf("multiple blocks term with the same name %s", term["name"].(string))
 		}
 		termNameList = append(termNameList, term["name"].(string))
 		setPrefixTerm := setPrefix + " term " + term["name"].(string)

junos/resource_forwardingoptions_sampling_instance.go

@@ -952,7 +952,7 @@ func setForwardingoptionsSamplingInstanceOutput(
 	for _, vFS := range output["flow_server"].([]interface{}) {
 		flowServer := vFS.(map[string]interface{})
 		if bchk.StringInSlice(flowServer["hostname"].(string), flowServerHostnameList) {
-			return fmt.Errorf("multiple flow_server blocks with the same hostname")
+			return fmt.Errorf("multiple blocks flow_server with the same hostname %s", flowServer["hostname"].(string))
 		}
 		flowServerHostnameList = append(flowServerHostnameList, flowServer["hostname"].(string))
 		setPrefixFlowServer := setPrefix + "flow-server " + flowServer["hostname"].(string) + " "
@@ -1021,7 +1021,7 @@ func setForwardingoptionsSamplingInstanceOutput(
 	for _, vIF := range output["interface"].([]interface{}) {
 		interFace := vIF.(map[string]interface{})
 		if bchk.StringInSlice(interFace["name"].(string), interfaceNameList) {
-			return fmt.Errorf("multiple interface blocks with the same name")
+			return fmt.Errorf("multiple blocks interface with the same name %s", interFace["name"].(string))
 		}
 		interfaceNameList = append(interfaceNameList, interFace["name"].(string))
 		setPrefixInterface := setPrefix + "interface " + interFace["name"].(string) + " "

junos/resource_group_dual_system.go

@@ -399,7 +399,8 @@ func setGroupDualSystem(d *schema.ResourceData, m interface{}, jnprSess *Netconf
 		for _, v2 := range interfaceFxp0["family_inet_address"].([]interface{}) {
 			familyInetAddress := v2.(map[string]interface{})
 			if bchk.StringInSlice(familyInetAddress["cidr_ip"].(string), familyInetAddressCIDRIPList) {
-				return fmt.Errorf("multiple family_inet_address blocks with the same cidr_ip")
+				return fmt.Errorf("multiple blocks family_inet_address with the same cidr_ip %s",
+					familyInetAddress["cidr_ip"].(string))
 			}
 			familyInetAddressCIDRIPList = append(familyInetAddressCIDRIPList, familyInetAddress["cidr_ip"].(string))
 			configSet = append(configSet, setPrefix+"interfaces fxp0 unit 0 family inet address "+
@@ -421,7 +422,8 @@ func setGroupDualSystem(d *schema.ResourceData, m interface{}, jnprSess *Netconf
 		for _, v2 := range interfaceFxp0["family_inet6_address"].([]interface{}) {
 			familyInet6Address := v2.(map[string]interface{})
 			if bchk.StringInSlice(familyInet6Address["cidr_ip"].(string), familyInet6AddressCIDRIPList) {
-				return fmt.Errorf("multiple family_inet6_address blocks with the same cidr_ip")
+				return fmt.Errorf("multiple blocks family_inet6_address with the same cidr_ip %s",
+					familyInet6Address["cidr_ip"].(string))
 			}
 			familyInet6AddressCIDRIPList = append(familyInet6AddressCIDRIPList, familyInet6Address["cidr_ip"].(string))
 			configSet = append(configSet, setPrefix+"interfaces fxp0 unit 0 family inet6 address "+
@@ -446,7 +448,7 @@ func setGroupDualSystem(d *schema.ResourceData, m interface{}, jnprSess *Netconf
 		for _, v2 := range routingOptions["static_route"].([]interface{}) {
 			staticRoute := v2.(map[string]interface{})
 			if bchk.StringInSlice(staticRoute["destination"].(string), staticRouteDestList) {
-				return fmt.Errorf("multiple static_route blocks with the same destination")
+				return fmt.Errorf("multiple blocks static_route with the same destination %s", staticRoute["destination"].(string))
 			}
 			staticRouteDestList = append(staticRouteDestList, staticRoute["destination"].(string))
 			for _, v3 := range staticRoute["next_hop"].([]interface{}) {

junos/resource_interface_logical.go

@@ -1347,10 +1347,12 @@ func setFamilyAddress(inetAddress map[string]interface{}, setPrefix string, fami
 		addressMap := address.(map[string]interface{})
 		if bchk.StringInSlice(addressMap["cidr_ip"].(string), addressCIDRIPList) {
 			if family == inetWord {
-				return configSet, fmt.Errorf("multiple family_inet blocks with the same cidr_ip")
+				return configSet, fmt.Errorf("multiple blocks family_inet with the same cidr_ip %s",
+					addressMap["cidr_ip"].(string))
 			}
 			if family == inet6Word {
-				return configSet, fmt.Errorf("multiple family_inet6 blocks with the same cidr_ip")
+				return configSet, fmt.Errorf("multiple blocks family_inet6 with the same cidr_ip %s",
+					addressMap["cidr_ip"].(string))
 			}
 		}
 		addressCIDRIPList = append(addressCIDRIPList, addressMap["cidr_ip"].(string))
@@ -1375,7 +1377,8 @@ func setFamilyAddress(inetAddress map[string]interface{}, setPrefix string, fami
 				return configSet, fmt.Errorf("ConflictsWith no_accept_data and accept_data")
 			}
 			if bchk.IntInSlice(vrrpGroupMap["identifier"].(int), vrrpGroupIDList) {
-				return configSet, fmt.Errorf("multiple vrrp_group blocks with the same identifier")
+				return configSet, fmt.Errorf("multiple blocks vrrp_group with the same identifier %d",
+					vrrpGroupMap["identifier"].(int))
 			}
 			vrrpGroupIDList = append(vrrpGroupIDList, vrrpGroupMap["identifier"].(int))
 			var setNameAddVrrp string
@@ -1440,7 +1443,8 @@ func setFamilyAddress(inetAddress map[string]interface{}, setPrefix string, fami
 			for _, trackInterface := range vrrpGroupMap["track_interface"].([]interface{}) {
 				trackInterfaceMap := trackInterface.(map[string]interface{})
 				if bchk.StringInSlice(trackInterfaceMap["interface"].(string), trackInterfaceList) {
-					return configSet, fmt.Errorf("multiple track_interface blocks with the same interface")
+					return configSet, fmt.Errorf("multiple blocks track_interface with the same interface %s",
+						trackInterfaceMap["interface"].(string))
 				}
 				trackInterfaceList = append(trackInterfaceList, trackInterfaceMap["interface"].(string))
 				configSet = append(configSet, setNameAddVrrp+" track interface "+trackInterfaceMap["interface"].(string)+
@@ -1450,7 +1454,8 @@ func setFamilyAddress(inetAddress map[string]interface{}, setPrefix string, fami
 			for _, trackRoute := range vrrpGroupMap["track_route"].([]interface{}) {
 				trackRouteMap := trackRoute.(map[string]interface{})
 				if bchk.StringInSlice(trackRouteMap["route"].(string), trackRouteList) {
-					return configSet, fmt.Errorf("multiple track_route blocks with the same interface")
+					return configSet, fmt.Errorf("multiple blocks track_route with the same interface %s",
+						trackRouteMap["route"].(string))
 				}
 				trackRouteList = append(trackRouteList, trackRouteMap["route"].(string))
 				configSet = append(configSet, setNameAddVrrp+" track route "+trackRouteMap["route"].(string)+

junos/resource_ospf_area.go

@@ -314,7 +314,7 @@ func setOspfArea(d *schema.ResourceData, m interface{}, jnprSess *NetconfObject)
 	for _, v := range d.Get("interface").([]interface{}) {
 		ospfInterface := v.(map[string]interface{})
 		if bchk.StringInSlice(ospfInterface["name"].(string), interfaceNameList) {
-			return fmt.Errorf("multiple interface blocks with the same name")
+			return fmt.Errorf("multiple blocks interface with the same name %s", ospfInterface["name"].(string))
 		}
 		interfaceNameList = append(interfaceNameList, ospfInterface["name"].(string))
 		setPrefixInterface := setPrefix + "interface " + ospfInterface["name"].(string) + " "

junos/resource_policyoptions_as_path_group.go

@@ -251,7 +251,7 @@ func setPolicyoptionsAsPathGroup(d *schema.ResourceData, m interface{}, jnprSess
 	for _, v := range d.Get("as_path").([]interface{}) {
 		asPath := v.(map[string]interface{})
 		if bchk.StringInSlice(asPath["name"].(string), asPathNameList) {
-			return fmt.Errorf("multiple as_path blocks with the same name")
+			return fmt.Errorf("multiple blocks as_path with the same name %s", asPath["name"].(string))
 		}
 		asPathNameList = append(asPathNameList, asPath["name"].(string))
 		configSet = append(configSet, setPrefix+

junos/resource_policyoptions_policy_statement.go

@@ -673,7 +673,7 @@ func setPolicyStatement(d *schema.ResourceData, m interface{}, jnprSess *Netconf
 	for _, term := range d.Get("term").([]interface{}) {
 		termMap := term.(map[string]interface{})
 		if bchk.StringInSlice(termMap["name"].(string), termNameList) {
-			return fmt.Errorf("multiple term blocks with the same name")
+			return fmt.Errorf("multiple blocks term with the same name %s", termMap["name"].(string))
 		}
 		termNameList = append(termNameList, termMap["name"].(string))
 		setPrefixTerm := setPrefix + " term " + termMap["name"].(string)
@@ -947,7 +947,8 @@ func setPolicyStatementOptsThen(setPrefix string, opts map[string]interface{}) (
 		community := v.(map[string]interface{})
 		setCommunityActVal := "community " + community["action"].(string) + " " + community["value"].(string)
 		if bchk.StringInSlice(setCommunityActVal, communityList) {
-			return configSet, fmt.Errorf("multiple community blocks with the same action and value")
+			return configSet, fmt.Errorf("multiple blocks community with the same action %s and value %s",
+				community["action"].(string), community["value"].(string))
 		}
 		communityList = append(communityList, setCommunityActVal)
 		configSet = append(configSet, setPrefixThen+setCommunityActVal)