From fca732694864cb6167f7e0a1327eb583906a1cfb Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Mon, 26 Nov 2018 10:36:38 +1000 Subject: [PATCH] a few changes for review --- .../modules/windows/win_psrepository.ps1 | 95 ++----- .../modules/windows/win_psrepository.py | 19 +- .../win_psrepository/defaults/main.yml | 7 +- .../targets/win_psrepository/meta/main.yml | 2 + .../targets/win_psrepository/tasks/main.yml | 27 +- .../targets/win_psrepository/tasks/tests.yml | 240 ++++++++++-------- 6 files changed, 183 insertions(+), 207 deletions(-) create mode 100644 test/integration/targets/win_psrepository/meta/main.yml diff --git a/lib/ansible/modules/windows/win_psrepository.ps1 b/lib/ansible/modules/windows/win_psrepository.ps1 index 2276742ae89..7aec08fb24d 100644 --- a/lib/ansible/modules/windows/win_psrepository.ps1 +++ b/lib/ansible/modules/windows/win_psrepository.ps1 @@ -14,90 +14,45 @@ $check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "b $name = Get-AnsibleParam -obj $params -name "name" -type "str" -failifempty $true $source = Get-AnsibleParam -obj $params -name "source" -type "str" $state = Get-AnsibleParam -obj $params -name "state" -type "str" -default "present" -validateset "present", "absent" -$installationpolicy = Get-AnsibleParam -obj $params -name "installation_policy" -type "str" -default "trusted" -validateset "trusted", "untrusted" +$installationpolicy = Get-AnsibleParam -obj $params -name "installation_policy" -type "str" -validateset "trusted", "untrusted" $result = @{"changed" = $false} -Function Install-Repository { - Param( - [Parameter(Mandatory=$true)] - [String]$Name, - [Parameter(Mandatory=$true)] - [String]$SourceLocation, - [String]$InstallationPolicy, - [Bool]$CheckMode - ) - - try { - if (-not $CheckMode) { - Register-PSRepository -Name $Name -SourceLocation $SourceLocation -InstallationPolicy $InstallationPolicy +$Repo = Get-PSRepository -Name $name -ErrorAction Ignore +if ($state -eq "present") { + if ($null -eq $Repo){ + if ($null -eq $installationpolicy) { + $installationpolicy = "trusted" + } + if (-not $check_mode) { + Register-PSRepository -Name $name -SourceLocation $source -InstallationPolicy $installationpolicy } $result.changed = $true } - catch { - $ErrorMessage = "Problems adding $($Name) repository: $($_.Exception.Message)" - Fail-Json $result $ErrorMessage - } - -} + else { + $changed_properties = @{} -Function Remove-Repository { - Param( - [Parameter(Mandatory=$true)] - [String]$Name, - [Bool]$CheckMode - ) - $Repo = Get-PSRepository -Name $Name -ErrorAction Ignore + if ($Repo.SourceLocation -ne $source) { + $changed_properties.SourceLocation = $source + } - if ( $null -ne $Repo) { + if ($null -ne $installationpolicy -and $Repo.InstallationPolicy -ne $installationpolicy) { + $changed_properties.InstallationPolicy = $installationpolicy + } - try { - if (-not $CheckMode) { - Unregister-PSRepository -Name $Name + if ($changed_properties.Count -gt 0) { + if (-not $check_mode) { + Set-PSRepository -Name $name @changed_properties } $result.changed = $true } - catch { - $ErrorMessage = "Problems removing $($Name) repository: $($_.Exception.Message)" - Fail-Json $result $ErrorMessage - } - } -} - -Function Set-Repository { - Param( - [Parameter(Mandatory=$true)] - [String]$Name, - [String]$InstallationPolicy, - [Bool]$CheckMode - ) - try { - if (-not $CheckMode) { - Set-PSRepository -Name $Name -InstallationPolicy $InstallationPolicy - } - $result.changed = $true - } - catch { - $ErrorMessage = "Problems changing the InstallationPolicy for $($Name) repository: $($_.Exception.Message)" - Fail-Json $result $ErrorMessage } } - -if ($state -eq "present") { - - $Repo = Get-PSRepository -Name $name -ErrorAction Ignore - - if ($null -eq $Repo){ - Install-Repository -Name $name -SourceLocation $source -InstallationPolicy $installationpolicy -CheckMode $check_mode - } - else { - if ( $Repo.InstallationPolicy -ne $installationpolicy ) { - Set-Repository -Name $name -InstallationPolicy $installationpolicy -CheckMode $check_mode - } +elseif ($state -eq "absent" -and $null -ne $Repo) { + if (-not $check_mode) { + Unregister-PSRepository -Name $name } -} -elseif ($state -eq "absent") { - Remove-Repository -Name $name -CheckMode $check_mode + $result.changed = $true } -Exit-Json $result +Exit-Json -obj $result diff --git a/lib/ansible/modules/windows/win_psrepository.py b/lib/ansible/modules/windows/win_psrepository.py index 55684630661..8a664129d2d 100644 --- a/lib/ansible/modules/windows/win_psrepository.py +++ b/lib/ansible/modules/windows/win_psrepository.py @@ -18,7 +18,7 @@ module: win_psrepository version_added: "2.8" short_description: Adds, removes or updates a Windows PowerShell repository. description: - - This module helps to install, remove and update Windows PowerShell repository on Windows-based systems. + - This module helps to add, remove and update Windows PowerShell repository on Windows-based systems. options: name: description: @@ -27,32 +27,35 @@ options: source: description: - Specifies the URI for discovering and installing modules from this repository. - URI can be a NuGet server feed (most common situation), HTTP, HTTPS, FTP or local folder location. + - A URI can be a NuGet server feed (most common situation), HTTP, HTTPS, FTP or file location. state: description: - - If C(present) a new repository is added or existing updated. + - If C(present) a new repository is added or updated. - If C(absent) a repository is removed. choices: [ absent, present ] default: present installation_policy: description: - - Set's the C(InstallationPolicy) of a repository. + - Sets the C(InstallationPolicy) of a repository. + - Will default to C(trusted) when creating a new repository. choices: [ trusted, untrusted ] - default: trusted notes: - The PowerShellGet module (version 1.6.0 or newer) and the NuGet package provider (version 2.8.5.201 or newer) are required. - To update the NuGet package provider use the command `Install-PackageProvider -Name NuGet -RequiredVersion 2.8.5.201 -Force`. - - You can't use M(win_psrepository) to re-register (add) removed PSGallery, use the command `Register-PSRepository -Default` instead. + - See the examples on how to update the NuGet package provider. + - You can't use M(win_psrepository) to re-register (add) removed PSGallery, use the command C(Register-PSRepository -Default) instead. author: - Wojciech Sciesinski (@it-praktyk) ''' EXAMPLES = r''' --- +- name: Ensure the required NuGet package provider version is installed + win_shell: Install-PackageProvider -Name NuGet -RequiredVersion 2.8.5.201 -Force + - name: Add a PowerShell module and register a repository win_psrepository: name: MyRepository - url: https://myrepo.com + source: https://myrepo.com state: present - name: Remove a PowerShell repository diff --git a/test/integration/targets/win_psrepository/defaults/main.yml b/test/integration/targets/win_psrepository/defaults/main.yml index 11857f9a62a..58e1d90ca3f 100644 --- a/test/integration/targets/win_psrepository/defaults/main.yml +++ b/test/integration/targets/win_psrepository/defaults/main.yml @@ -1,5 +1,4 @@ --- -PSGallery_url: https://www.powershellgallery.com/api/v2 - -repository_name: MyGet -repository_sourcelocation: https://www.myget.org/F/powershellgetdemo/api/v2 \ No newline at end of file +repository_name: My Get +repository_sourcelocation: https://www.myget.org/F/powershellgetdemo/api/v2 +repository_sourcelocation2: '{{ win_output_dir }}' diff --git a/test/integration/targets/win_psrepository/meta/main.yml b/test/integration/targets/win_psrepository/meta/main.yml new file mode 100644 index 00000000000..bdea853d75a --- /dev/null +++ b/test/integration/targets/win_psrepository/meta/main.yml @@ -0,0 +1,2 @@ +dependencies: +- prepare_win_tests diff --git a/test/integration/targets/win_psrepository/tasks/main.yml b/test/integration/targets/win_psrepository/tasks/main.yml index 4112820dd18..4667bbc7fd7 100644 --- a/test/integration/targets/win_psrepository/tasks/main.yml +++ b/test/integration/targets/win_psrepository/tasks/main.yml @@ -25,24 +25,17 @@ Install-PackageProvider -Name NuGet -MinimumVersion 2.8.5.201 -Force } - - name: set initial state - win_shell: "{{ item }}" - with_items: - - 'Set-PSRepository -Name PSGallery -InstallationPolicy Untrusted -ErrorAction Ignore' - - 'Unregister-PSRepository -Name MyGet -ErrorAction Ignore' + - name: ensure test repository is deleted + win_psrepository: + name: '{{ repository_name }}' + state: absent - name: run all tests - include: tests.yml - vars: - in_check_mode: no + include_tasks: tests.yml - - name: set initial state - win_shell: "{{ item }}" - with_items: - - 'Set-PSRepository -Name PSGallery -InstallationPolicy Untrusted -ErrorAction Ignore' - - 'Unregister-PSRepository -Name MyGet -ErrorAction Ignore' + always: - - name: run all tests - import_tasks: tests.yml - vars: - in_check_mode: yes + - name: ensure test repository is deleted after tests run + win_psrepository: + name: '{{ repository_name }}' + state: absent diff --git a/test/integration/targets/win_psrepository/tasks/tests.yml b/test/integration/targets/win_psrepository/tasks/tests.yml index f6e67245a8e..5dc80fa9af7 100644 --- a/test/integration/targets/win_psrepository/tasks/tests.yml +++ b/test/integration/targets/win_psrepository/tasks/tests.yml @@ -5,172 +5,196 @@ --- -- name: "check setting of InstallationPolicy - check mode: {{ in_check_mode }}" +- name: check adding of repository defaults - check mode win_psrepository: - name: PSGallery - source: "{{ PSGallery_url }}" - installation_policy: trusted - check_mode: "{{ in_check_mode }}" - register: change_installation_policy_1 - -- name: "get result of setting of InstallationPolicy - check mode: {{ in_check_mode }}" - win_shell: '(Get-PSRepository -Name PSGallery).InstallationPolicy' + name: "{{ repository_name }}" + source: "{{ repository_sourcelocation }}" + state: present + check_mode: True + register: adding_repository_check + +- name: get result of adding repository defaults - check mode + win_shell: (Get-PSRepository -Name {{ repository_name | quote }} -ErrorAction ignore | Measure-Object).Count changed_when: false - register: result_change_installation_policy_1 + register: result_adding_repository_check -- name: "test setting of InstallationPolicy - check mode: {{ in_check_mode }}" +- name: test adding repository defaults - check mode assert: that: - - "change_installation_policy_1 is changed" - - "'{{ result_change_installation_policy_1.stdout | trim }}' == 'Trusted'" - when: not in_check_mode + - adding_repository_check is changed + - result_adding_repository_check.stdout_lines[0] == '0' + +- name: check adding repository defaults + win_psrepository: + name: "{{ repository_name }}" + source: "{{ repository_sourcelocation }}" + state: present + register: adding_repository + +- name: get result of adding repository defaults + win_shell: | + $repo = Get-PSRepository -Name {{ repository_name | quote }} + ($repo | Measure-Object).Count + $repo.SourceLocation + $repo.InstallationPolicy + register: result_adding_repository -- name: "test setting of InstallationPolicy - check mode: {{ in_check_mode }}" +- name: test adding repository defaults assert: that: - - "change_installation_policy_1 is changed" - - "'{{ result_change_installation_policy_1.stdout | trim }}' == 'Untrusted'" - when: in_check_mode + - adding_repository is changed + - result_adding_repository.stdout_lines[0] == '1' + - result_adding_repository.stdout_lines[1] == repository_sourcelocation + - result_adding_repository.stdout_lines[2] == 'Trusted' -- name: "check idempotency setting of InstallationPolicy - check mode: {{ in_check_mode }}" +- name: check adding repository defaults - idempotent win_psrepository: - name: PSGallery - source: "{{ PSGallery_url }}" - installation_policy: trusted - register: change_installation_policy_2 - when: not in_check_mode + name: "{{ repository_name }}" + source: "{{ repository_sourcelocation }}" + state: present + register: adding_repository_again -- name: "test idempotency setting of InstallationPolicy - check mode: {{ in_check_mode }}" +- name: test check adding repository defaults - idempotent assert: that: - - "change_installation_policy_2 is not changed" - when: not in_check_mode + - adding_repository_again is not changed -- name: "check adding of repository 1 - check mode: {{ in_check_mode }}" +- name: change InstallationPolicy - check mode win_psrepository: name: "{{ repository_name }}" source: "{{ repository_sourcelocation }}" - state: present - check_mode: "{{ in_check_mode }}" - register: adding_repository_1 + installation_policy: untrusted + check_mode: True + register: change_installation_policy_check -- name: "get result of adding of repository 1 - check mode: {{ in_check_mode }}" - win_shell: | - $repo = Get-PSRepository -Name {{ repository_name | quote }} -ErrorAction Ignore - ($repo | Measure-Object).Count - $repo.SourceLocation - $repo.InstallationPolicy +- name: get result of change InstallationPolicy - check mode + win_shell: '(Get-PSRepository -Name {{ repository_name | quote }}).InstallationPolicy' changed_when: false - register: result_adding_repository_1 + register: result_change_installation_policy_check -- name: "test adding of repository 1 - check mode: {{ in_check_mode }}" +- name: test change InstallationPolicy - check mode assert: that: - - "adding_repository_1 is changed" - - "'{{ result_adding_repository_1.stdout_lines[0] }}' == '1'" - - "'{{ result_adding_repository_1.stdout_lines[1] }}' == '{{ repository_sourcelocation }}'" - - "'{{ result_adding_repository_1.stdout_lines[2] }}' == 'Trusted'" - when: not in_check_mode + - change_installation_policy_check is changed + - result_change_installation_policy_check.stdout | trim == 'Trusted' + +- name: change InstallationPolicy + win_psrepository: + name: "{{ repository_name }}" + source: "{{ repository_sourcelocation }}" + installation_policy: untrusted + register: change_installation_policy -- name: "test adding of repository 1 - check mode: {{ in_check_mode }}" +- name: get result of change InstallationPolicy + win_shell: '(Get-PSRepository -Name {{ repository_name | quote }}).InstallationPolicy' + changed_when: false + register: result_change_installation_policy + +- name: test change InstallationPolicy assert: that: - - "adding_repository_1 is changed" - - "'{{ result_adding_repository_1.stdout_lines[0] }}' == '0'" - when: in_check_mode + - change_installation_policy is changed + - result_change_installation_policy.stdout | trim == 'Untrusted' -- name: "check idempotency adding of repository - check mode: {{ in_check_mode }}" +- name: change InstallationPolicy - idempotent win_psrepository: name: "{{ repository_name }}" source: "{{ repository_sourcelocation }}" + installation_policy: untrusted + register: change_installation_policy_again + +- name: test change InstallationPolicy - idempotent + assert: + that: + - change_installation_policy_again is not changed + +- name: change source - check mode + win_psrepository: + name: "{{ repository_name }}" + source: "{{ repository_sourcelocation2 }}" state: present - register: adding_repository_2 - when: not in_check_mode + check_mode: True + register: change_source_check + +- name: get result of change source - check mode + win_shell: | + $repo = Get-PSRepository -Name {{ repository_name | quote }} + $repo.SourceLocation + $repo.InstallationPolicy + changed_when: False + register: result_change_source_check -- name: "test idempotency adding of repository - check mode: {{ in_check_mode }}" +- name: test change source - check mode assert: that: - - "adding_repository_2 is not changed" - when: not in_check_mode + - change_source_check is changed + - result_change_source_check.stdout_lines[0] == repository_sourcelocation + - result_change_source_check.stdout_lines[1] == 'Untrusted' -- name: "check removing of repository - check mode: {{ in_check_mode }}" +- name: change source win_psrepository: name: "{{ repository_name }}" - state: absent - register: removing_repository_1 - when: not in_check_mode + source: "{{ repository_sourcelocation2 }}" + state: present + register: change_source -- name: "get result removing of repository - check mode: {{ in_check_mode }}" - win_shell: '((Get-PSRepository -Name {{ repository_name }} -ErrorAction Ignore | Measure-Object).Count -eq 0)' - changed_when: false - register: result_removing_repository_1 - when: not in_check_mode +- name: get result of change source + win_shell: | + $repo = Get-PSRepository -Name {{ repository_name | quote }} + $repo.SourceLocation + $repo.InstallationPolicy + changed_when: False + register: result_change_source -- name: "test removing of repository - check mode: {{ in_check_mode }}" +- name: test change source assert: that: - - "removing_repository_1 is changed" - - "'{{ result_removing_repository_1.stdout | trim }}' == 'True'" - when: not in_check_mode + - change_source is changed + - result_change_source.stdout_lines[0] == repository_sourcelocation2 + - result_change_source.stdout_lines[1] == 'Untrusted' -- name: "check removing of repository - check mode: {{ in_check_mode }}" +- name: remove repository - check mode win_psrepository: - name: "PSGallery" + name: "{{ repository_name }}" state: absent - check_mode: "{{ in_check_mode }}" - register: removing_repository_2 - when: in_check_mode + check_mode: True + register: removing_repository_check -- name: "get result removing of repository - check mode: {{ in_check_mode }}" - win_shell: '((Get-PSRepository -Name PSGallery -ErrorAction Ignore | Measure-Object).Count -eq 1)' +- name: get result of remove repository - check mode + win_shell: '(Get-PSRepository -Name {{ repository_name | quote }} -ErrorAction Ignore | Measure-Object).Count' changed_when: false - register: result_removing_repository_2 - when: in_check_mode + register: result_removing_repository_check -- name: "test removing of repository - check mode: {{ in_check_mode }}" +- name: test remove repository - check mode assert: that: - - "removing_repository_2 is changed" - - "'{{ result_removing_repository_2.stdout | trim }}' == 'True'" - when: in_check_mode + - removing_repository_check is changed + - result_removing_repository_check.stdout | trim == '1' -- name: "check idempotency removing of repository - check mode: {{ in_check_mode }}" +- name: remove repository win_psrepository: name: "{{ repository_name }}" state: absent - register: removing_repository_3 - when: not in_check_mode + register: removing_repository + +- name: get result of remove repository + win_shell: '(Get-PSRepository -Name {{ repository_name | quote }} -ErrorAction Ignore | Measure-Object).Count' + changed_when: false + register: result_removing_repository -- name: "test idempotency removing of repository - check mode: {{ in_check_mode }}" +- name: test remove repository assert: that: - - "removing_repository_3 is not changed" - when: not in_check_mode + - removing_repository is changed + - result_removing_repository.stdout | trim == '0' -- name: "check adding of repository - 3 - check mode: {{ in_check_mode }}" +- name: remove repository - idempotent win_psrepository: name: "{{ repository_name }}" - source: "{{ repository_sourcelocation }}" - installation_policy: untrusted - state: present - check_mode: "{{ in_check_mode }}" - register: adding_repository_3 - -- name: "get result of adding of repository 3 - check mode: {{ in_check_mode }}" - win_shell: | - $repo = Get-PSRepository -Name {{ repository_name|quote }} -ErrorAction Ignore - ($repo | Measure-Object).Count - $repo.SourceLocation - $repo.InstallationPolicy - changed_when: false - register: result_adding_repository_3 - when: not in_check_mode + state: absent + register: remove_repository_again -- name: "test adding of repository - check mode: {{ in_check_mode }}" +- name: test remove repository - idempotent assert: that: - - "adding_repository_3 is changed" - - "'{{ result_adding_repository_3.stdout_lines[0] }}' == '1'" - - "'{{ result_adding_repository_3.stdout_lines[1] }}' == '{{ repository_sourcelocation }}'" - - "'{{ result_adding_repository_3.stdout_lines[2] }}' == 'Untrusted'" - when: not in_check_mode + - remove_repository_again is not changed