diff --git a/changelogs/fragments/win_xml_refactor.yaml b/changelogs/fragments/win_xml_refactor.yaml
new file mode 100644
index 0000000000..6cbab6e9c0
--- /dev/null
+++ b/changelogs/fragments/win_xml_refactor.yaml
@@ -0,0 +1,5 @@
+minor_changes:
+ - win_xml - Behaviour change, module now processes all nodes specified by xpath, not just first encountered.
+ - win_xml - Behaviour change, fragment no longer required when processing element type nodes and state=absent.
+ - win_xml - Some output messages worded differently now the module uses a generic method to save changes.
+ - win_xml - Added 'count' module parameter which will return number of nodes matched by xpath if set to yes/true
diff --git a/lib/ansible/modules/windows/win_xml.ps1 b/lib/ansible/modules/windows/win_xml.ps1
index 47ea6e2703..d051a3f67d 100644
--- a/lib/ansible/modules/windows/win_xml.ps1
+++ b/lib/ansible/modules/windows/win_xml.ps1
@@ -80,6 +80,22 @@ function Compare-XmlDocs($actual, $expected) {
}
}
+
+function Save-ChangedXml($xmlorig, $result, $message, $check_mode, $backup) {
+ $result.changed = $true
+ if (-Not $check_mode) {
+ if ($backup) {
+ $result.backup_file = Backup-File -path $dest -WhatIf:$check_mode
+ # Ensure backward compatibility (deprecate in future)
+ $result.backup = $result.backup_file
+ }
+ $xmlorig.Save($dest)
+ $result.msg = $message
+ } else {
+ $result.msg += " check mode"
+ }
+}
+
$params = Parse-Args $args -supports_check_mode $true
$check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "bool" -default $false
@@ -87,12 +103,13 @@ $debug_level = Get-AnsibleParam -obj $params -name "_ansible_verbosity" -type "i
$debug = $debug_level -gt 2
$dest = Get-AnsibleParam $params "path" -type "path" -FailIfEmpty $true -aliases "dest", "file"
-$fragment = Get-AnsibleParam $params "fragment" -type "str" -FailIfEmpty $true -aliases "xmlstring"
+$fragment = Get-AnsibleParam $params "fragment" -type "str" -aliases "xmlstring"
$xpath = Get-AnsibleParam $params "xpath" -type "str" -FailIfEmpty $true
-$backup = Get-AnsibleParam $params "backup" -type "bool" -default $false
+$backup = Get-AnsibleParam $params "backup" -type "bool" -Default $false
$type = Get-AnsibleParam $params "type" -type "str" -Default "element" -ValidateSet "element", "attribute", "text"
$attribute = Get-AnsibleParam $params "attribute" -type "str" -FailIfEmpty ($type -eq "attribute")
$state = Get-AnsibleParam $params "state" -type "str" -Default "present"
+$count = Get-AnsibleParam $params "count" -type "bool" -Default $false
$result = @{
changed = $false
@@ -117,121 +134,132 @@ $localname = $xmlorig.DocumentElement.LocalName
$namespaceMgr.AddNamespace($xmlorig.$localname.SchemaInfo.Prefix, $namespace)
+$nodeList = $xmlorig.SelectNodes($xpath, $namespaceMgr)
+$nodeListCount = $nodeList.get_Count()
+if ($count) {
+ $result.count = $nodeListCount
+ if (-not $fragment) {
+ Exit-Json $result
+ }
+}
+## Exit early if xpath did not match any nodes
+if ($nodeListCount -eq 0) {
+ $result.msg = "The supplied xpath did not match any nodes. If this is unexpected, check your xpath is valid for the xml file at supplied path."
+ Exit-Json $result
+}
+
+$changed = $false
+$result.msg = "not changed"
+
if ($type -eq "element") {
- $xmlchild = $null
- Try {
- $xmlchild = [xml]$fragment
- } Catch {
- Fail-Json $result "Failed to parse fragment as XML: $($_.Exception.Message)"
- }
-
- $child = $xmlorig.CreateElement($xmlchild.get_DocumentElement().get_Name(), $xmlorig.get_DocumentElement().get_NamespaceURI())
- Copy-Xml -dest $child -src $xmlchild.DocumentElement -xmlorig $xmlorig
-
- $node = $xmlorig.SelectSingleNode($xpath, $namespaceMgr)
- if ($node.get_NodeType() -eq "Document") {
- $node = $node.get_DocumentElement()
- }
- $elements = $node.get_ChildNodes()
- [bool]$present = $false
- [bool]$changed = $false
- if ($elements.get_Count()) {
- if ($debug) {
- $err = @()
- $result.err = {$err}.Invoke()
- }
- foreach ($element in $elements) {
- try {
- Compare-XmlDocs $child $element
- $present = $true
- break
- } catch {
+ if ($state -eq "absent") {
+ foreach ($node in $nodeList) {
+ # there are some nodes that match xpath, delete without comparing them to fragment
+ if (-Not $check_mode) {
+ $removedNode = $node.get_ParentNode().RemoveChild($node)
+ $changed = $true
if ($debug) {
- $result.err.Add($_.Exception.ToString())
+ $result.removed += $result.removed + $removedNode.get_OuterXml()
}
}
}
- if (!$present -and ($state -eq "present")) {
- [void]$node.AppendChild($child)
- $result.msg = "xml added"
- $changed = $true
- } elseif ($present -and ($state -eq "absent")) {
- [void]$node.RemoveChild($element)
- $result.msg = "xml removed"
- $changed = $true
+ } else { # state -eq 'present'
+ $xmlfragment = $null
+ Try {
+ $xmlfragment = [xml]$fragment
+ } Catch {
+ Fail-Json $result "Failed to parse fragment as XML: $($_.Exception.Message)"
}
- } else {
- if ($state -eq "present") {
- [void]$node.AppendChild($child)
- $result.msg = "xml added"
- $changed = $true
- }
- }
- if ($changed) {
- if ($backup) {
- $result.backup_file = Backup-File -path $dest -WhatIf:$check_mode
- # Ensure backward compatibility (deprecate in future)
- $result.backup = $result.backup_file
+ foreach ($node in $nodeList) {
+ $candidate = $xmlorig.CreateElement($xmlfragment.get_DocumentElement().get_Name(), $xmlorig.get_DocumentElement().get_NamespaceURI())
+ Copy-Xml -dest $candidate -src $xmlfragment.DocumentElement -xmlorig $xmlorig
+
+ if ($node.get_NodeType() -eq "Document") {
+ $node = $node.get_DocumentElement()
+ }
+ $elements = $node.get_ChildNodes()
+ [bool]$present = $false
+ [bool]$changed = $false
+ $element_count = $elements.get_Count()
+ $nstatus = "node: " + $node.get_Value() + " element: " + $elements.get_OuterXml() + " Element count is $element_count"
+ Add-Warning $result $nstatus
+ if ($elements.get_Count()) {
+ if ($debug) {
+ $err = @()
+ $result.err = {$err}.Invoke()
+ }
+ foreach ($element in $elements) {
+ $estatus = "element is " + $element.get_OuterXml()
+ Add-Warning $result $estatus
+ try {
+ Compare-XmlDocs $candidate $element
+ $present = $true
+ break
+ } catch {
+ if ($debug) {
+ $result.err.Add($_.Exception.ToString())
+ }
+ }
+ }
+ if (-Not $present -and ($state -eq "present")) {
+ [void]$node.AppendChild($candidate)
+ $result.msg = $result.msg + "xml added "
+ $changed = $true
+ }
+ }
}
- if (-not $check_mode) {
- $xmlorig.Save($dest)
- }
- $result.changed = $true
- } else {
- $result.msg = "not changed"
}
} elseif ($type -eq "text") {
- $node = $xmlorig.SelectSingleNode($xpath, $namespaceMgr)
- [bool]$add = ($node.get_InnerText() -ne $fragment)
- if ($add) {
- if ($backup) {
- $result.backup_file = Backup-File -path $dest -WhatIf:$check_mode
- # Ensure backward compatibility (deprecate in future)
- $result.backup = $result.backup_file
+ foreach ($node in $nodeList) {
+ if ($node.get_InnerText() -ne $fragment) {
+ $node.set_InnerText($fragment)
+ $changed = $true
}
- $node.set_InnerText($fragment)
- if (-not $check_mode) {
- $xmlorig.Save($dest)
- }
- $result.changed = $true
- $result.msg = "text changed"
- } else {
- $result.msg = "not changed"
}
} elseif ($type -eq "attribute") {
- $node = $xmlorig.SelectSingleNode($xpath, $namespaceMgr)
- [bool]$add = !$node.HasAttribute($attribute) -Or ($node.$attribute -ne $fragment)
- if ($add -And ($state -eq "present")) {
- if ($backup) {
- $result.backup_file = Backup-File -path $dest -WhatIf:$check_mode
- # Ensure backward compatibility (deprecate in future)
- $result.backup = $result.backup_file
+ foreach ($node in $nodeList) {
+ if ($state -eq 'present') {
+ if ($node.NodeType -eq 'Attribute') {
+ if ($node.Name -eq $attribute -and $node.Value -ne $fragment ) {
+ # this is already the attribute with the right name, so just set its value to match fragment
+ $node.Value = $fragment
+ $changed = $true
+ }
+ } else { # assume NodeType is Element
+ if ($node.$attribute -ne $fragment) {
+ if (!$node.HasAttribute($attribute)) { # add attribute to Element if missing
+ $node.SetAttributeNode($attribute, $xmlorig.get_DocumentElement().get_NamespaceURI())
+ }
+ #set the attribute into the element
+ $node.SetAttribute($attribute, $fragment)
+ $changed = $true
+ }
+ }
+ } elseif ($state -eq 'absent') {
+ if ($node.NodeType -eq 'Attribute') {
+ $attrNode = [System.Xml.XmlAttribute]$node
+ $parent = $attrNode.OwnerElement
+ $parent.RemoveAttribute($attribute)
+ $changed = $true
+ } else { # element node processing
+ if ($node.Name -eq $attribute ) { # note not caring about the state of 'fragment' at this point
+ $node.RemoveAttribute($attribute)
+ $changed = $true
+ }
+ }
+ } else {
+ Add-Warning $result "Unexpected state when processing attribute $($attribute), add was $add, state was $state"
}
- if (!$node.HasAttribute($attribute)) {
- $node.SetAttributeNode($attribute, $xmlorig.get_DocumentElement().get_NamespaceURI())
- }
- $node.SetAttribute($attribute, $fragment)
- if (-not $check_mode) {
- $xmlorig.Save($dest)
- }
- $result.changed = $true
- $result.msg = "text changed"
- } elseif (!$add -And ($state -eq "absent")) {
- if ($backup) {
- $result.backup_file = Backup-File -path $dest -WhatIf:$check_mode
- # Ensure backward compatibility (deprecate in future)
- $result.backup = $result.backup_file
- }
- $node.RemoveAttribute($attribute)
- if (-not $check_mode) {
- $xmlorig.Save($dest)
- }
- $result.changed = $true
- $result.msg = "text changed"
- } else {
- $result.msg = "not changed"
}
}
+if ($changed) {
+ if ($state -eq "absent") {
+ $summary = "$type removed"
+ } else {
+ $summary = "$type changed"
+ }
+ Save-ChangedXml -xmlorig $xmlorig -result $result -message $summary -check_mode $check_mode -backup $backup
+}
Exit-Json $result
diff --git a/lib/ansible/modules/windows/win_xml.py b/lib/ansible/modules/windows/win_xml.py
index 3f24c09a58..a503df81d8 100644
--- a/lib/ansible/modules/windows/win_xml.py
+++ b/lib/ansible/modules/windows/win_xml.py
@@ -15,28 +15,23 @@ DOCUMENTATION = r'''
---
module: win_xml
version_added: "2.7"
-short_description: Add XML fragment to an XML parent
+short_description: Manages XML file content on Windows hosts
description:
- - Adds XML fragments formatted as strings to existing XML on remote servers.
+ - Manages XML nodes, attributes and text, using xpath to select which xml nodes need to be managed.
+ - XML fragments, formatted as strings, are used to specify the desired state of a part or parts of XML files on remote Windows servers.
- For non-Windows targets, use the M(xml) module instead.
options:
- path:
+ attribute:
description:
- - The path of remote servers XML.
- type: path
- required: true
- aliases: [ dest, file ]
- fragment:
- description:
- - The string representation of the XML fragment to be added.
+ - The attribute name if the type is 'attribute'.
+ - Required if C(type=attribute).
type: str
- required: true
- aliases: [ xmlstring ]
- xpath:
+ count:
description:
- - The node of the remote server XML where the fragment will go.
- type: str
- required: true
+ - When set to C(yes), return the number of nodes matched by I(xpath).
+ type: bool
+ default: false
+ version_added: 2.9
backup:
description:
- Determine whether a backup should be created.
@@ -44,20 +39,49 @@ options:
so you can get the original file back if you somehow clobbered it incorrectly.
type: bool
default: no
+ fragment:
+ description:
+ - The string representation of the XML fragment expected at xpath. Since ansible 2.9 not required when I(state=absent), or when I(count=yes).
+ type: str
+ required: false
+ aliases: [ xmlstring ]
+ path:
+ description:
+ - Path to the file to operate on.
+ type: path
+ required: true
+ aliases: [ dest, file ]
+ state:
+ description:
+ - Set or remove the nodes (or attributes) matched by I(xpath).
+ type: str
+ default: present
+ choices: [ present, absent ]
+ version_added: 2.9
type:
description:
- - The type of XML you are working with.
+ - The type of XML node you are working with.
type: str
required: yes
default: element
choices: [ attribute, element, text ]
- attribute:
+ xpath:
description:
- - The attribute name if the type is 'attribute'.
- - Required if C(type=attribute).
+ - Xpath to select the node or nodes to operate on.
type: str
+ required: true
author:
- Richard Levenberg (@richardcs)
+ - Jon Hawkesworth (@jhawkesworth)
+notes:
+ - Only supports operating on xml elements, attributes and text.
+ - Namespace, processing-instruction, command and document node types cannot be modified with this module.
+seealso:
+ - module: xml
+ description: XML manipulation for Posix hosts.
+ - name: w3shools XPath tutorial
+ description: A useful tutorial on XPath
+ link: https://www.w3schools.com/xml/xpath_intro.asp
'''
EXAMPLES = r'''
@@ -74,6 +98,32 @@ EXAMPLES = r'''
attribute: 'sslEnabledProtocols'
fragment: 'TLSv1,TLSv1.1,TLSv1.2'
type: attribute
+
+- name: remove debug configuration nodes from nlog.conf
+ win_xml:
+ path: C:\IISApplication\nlog.conf
+ xpath: /nlog/rules/logger[@name="debug"]/descendant::*
+ state: absent
+
+- name: count configured connectors in Tomcat's server.xml
+ win_xml:
+ path: C:\Tomcat\conf\server.xml
+ xpath: //Server/Service/Connector
+ count: yes
+ register: connector_count
+
+- name: show connector count
+ debug:
+ msg="Connector count is {{connector_count.count}}"
+
+- name: ensure all lang=en attributes to lang=nl
+ win_xml:
+ path: C:\Data\Books.xml
+ xpath: //@[lang="en"]
+ attribute: lang
+ fragment: nl
+ type: attribute
+
'''
RETURN = r'''
@@ -82,6 +132,11 @@ backup_file:
returned: if backup=yes
type: str
sample: C:\Path\To\File.txt.11540.20150212-220915.bak
+count:
+ description: Number of nodes matched by xpath.
+ returned: if count=yes
+ type: int
+ sample: 33
msg:
description: What was done.
returned: always
diff --git a/test/integration/targets/win_xml/files/books.xml b/test/integration/targets/win_xml/files/books.xml
new file mode 100644
index 0000000000..e38ee15d4e
--- /dev/null
+++ b/test/integration/targets/win_xml/files/books.xml
@@ -0,0 +1,10 @@
+
+
+
+ A Great Book
+ Best Book Ever
+ Worst Book Ever
+ Another Book
+ Worst Book Ever Two
+
+
diff --git a/test/integration/targets/win_xml/tasks/main.yml b/test/integration/targets/win_xml/tasks/main.yml
index 77a9b9aa63..d3d98693ff 100644
--- a/test/integration/targets/win_xml/tasks/main.yml
+++ b/test/integration/targets/win_xml/tasks/main.yml
@@ -142,3 +142,166 @@
assert:
that:
- sha1_checksum.stat.checksum == 'de86f79b409383447cf4cf112b20af8ffffcfdbf'
+
+# features added ansible 2.8
+# count
+
+- name: count logger nodes in log4j.xml
+ win_xml:
+ path: "{{ win_output_dir }}\\log4j.xml"
+ xpath: //logger
+ count: yes
+ register: logger_node_count
+
+- name: verify node count
+ assert:
+ that:
+ - logger_node_count.count == 5
+
+# multiple attribute change
+- name: ensure //logger/level value attributes are set to debug
+ win_xml:
+ path: "{{ win_output_dir }}\\log4j.xml"
+ xpath: '//logger/level[@value="error"]'
+ type: attribute
+ attribute: value
+ fragment: debug
+ count: yes
+ register: logger_level_value_attrs
+
+- name: verify //logger/level value attributes
+ assert:
+ that:
+ - logger_level_value_attrs.count == 4
+ - logger_level_value_attrs.changed == true
+ - logger_level_value_attrs.msg == 'attribute changed'
+
+- name: ensure //logger/level value attributes are set to debug (idempotency)
+ win_xml:
+ path: "{{ win_output_dir }}\\log4j.xml"
+ xpath: '//logger/level[@value="error"]'
+ type: attribute
+ attribute: value
+ fragment: debug
+ count: yes
+ register: logger_level_value_attrs_again
+
+- name: verify //logger/level value attributes again (idempotency)
+ assert:
+ that:
+ - logger_level_value_attrs_again.count == 0
+ - logger_level_value_attrs_again.changed == false
+ - logger_level_value_attrs_again.msg == 'The supplied xpath did not match any nodes. If this is unexpected, check your xpath is valid for the xml file at supplied path.'
+
+# multiple text nodes
+- name: ensure test books.xml is present
+ win_copy:
+ src: books.xml
+ dest: '{{ win_output_dir }}\books.xml'
+
+- name: demonstrate multi text replace by replacing all title text elements
+ win_xml:
+ path: '{{ win_output_dir }}\books.xml'
+ xpath: //works/title
+ type: text
+ fragment: _TITLE_TEXT_REMOVED_BY_WIN_XML_MODULE_
+ count: yes
+ register: multi_text
+
+- name: verify multi text change
+ assert:
+ that:
+ - multi_text.changed == true
+ - multi_text.count == 5
+ - multi_text.msg == 'text changed'
+
+- name: demonstrate multi text replace by replacing all title text elements again (idempotency)
+ win_xml:
+ path: '{{ win_output_dir }}\books.xml'
+ xpath: //works/title
+ type: text
+ fragment: _TITLE_TEXT_REMOVED_BY_WIN_XML_MODULE_
+ count: yes
+ register: multi_text_again
+
+- name: verify multi text again change (idempotency)
+ assert:
+ that:
+ - multi_text_again.changed == false
+ - multi_text_again.count == 5
+ - multi_text_again.msg == 'not changed'
+
+# multiple element
+
+#- name: ensure a fresh test books.xml is present
+# win_copy:
+# src: books.xml
+# dest: '{{ win_output_dir }}\books.xml'
+
+- name: demonstrate multi element should append new information element from fragment
+ win_xml:
+ path: '{{ win_output_dir }}\books.xml'
+ xpath: //works/title
+ type: element
+ fragment: This element added by ansible
+ count: yes
+ register: multi_element
+
+- name: verify multi element
+ assert:
+ that:
+ - multi_element.changed == true
+ - multi_element.count == 5
+ - multi_element.msg == 'element changed'
+
+- name: demonstrate multi element unchanged (idempotency)
+ win_xml:
+ path: '{{ win_output_dir }}\books.xml'
+ xpath: //works/title
+ type: element
+ fragment: This element added by ansible
+ count: yes
+ register: multi_element_again
+
+- name: verify multi element again (idempotency)
+ assert:
+ that:
+ - multi_element_again.changed == false
+ - multi_element_again.count == 5
+ - multi_element_again.msg == 'not changed'
+
+# multiple attributes on differing parent nodes
+
+- name: ensure all attribute lang=nl
+ win_xml:
+ path: '{{ win_output_dir }}\books.xml'
+ xpath: //@lang
+ type: attribute
+ attribute: lang
+ fragment: nl
+ count: yes
+ register: multi_attr
+
+- name: verify multi attribute
+ assert:
+ that:
+ - multi_attr.changed == true
+ - multi_attr.count == 6
+ - multi_attr.msg == 'attribute changed'
+
+- name: ensure all attribute lang=nl (idempotency)
+ win_xml:
+ path: '{{ win_output_dir }}\books.xml'
+ xpath: //@lang
+ type: attribute
+ attribute: lang
+ fragment: nl
+ count: yes
+ register: multi_attr_again
+
+- name: verify multi attribute (idempotency)
+ assert:
+ that:
+ - multi_attr_again.changed == false
+ - multi_attr_again.count == 6
+ - multi_attr_again.msg == 'not changed'