New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add win_mssql_query module: Module for query MSSQL databases #43774

Open
wants to merge 7 commits into
base: devel
from

Conversation

Projects
None yet
6 participants
@dlazz
Contributor

dlazz commented Aug 7, 2018

SUMMARY

This new module allows user to query MSSQL Server databases

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

win_mssql_query

ANSIBLE VERSION
2.7
ADDITIONAL INFORMATION

Example: List all the databases on a SQL Server

tasks:
    - name: get databases
      win_mssql_query:
        query: "SELECT name FROM sys.databases" 
        server_instance: localhost
        server_instance_user: sa
        server_instance_password: "{{ sa }}"
      register: databases

    - debug:
        msg: "{{ item.name }}"
      loop: "{{ databases.output }}"
      when: databases.output != []

Output:


PLAY [win_mssql_query] *************************************************************************************************************************************************************************

TASK [Gathering Facts] *************************************************************************************************************************************************************************
ok: [ansible]

TASK [get databases] ***************************************************************************************************************************************************************************
changed: [ansible-test] => {"changed": true, "output": [{"name": "master"}, {"name": "tempdb"}, {"name": "model"}, {"name": "msdb"}]}

TASK [debug] ***********************************************************************************************************************************************************************************
ok: [ansible] => (item={'name': 'master'}) => {
    "msg": "master"
}
ok: [ansible] => (item={'name': 'tempdb'}) => {
    "msg": "tempdb"
}
ok: [ansible] => (item={'name': 'model'}) => {
    "msg": "model"
}
ok: [ansible] => (item={'name': 'msdb'}) => {
    "msg": "msdb"
}

PLAY RECAP *************************************************************************************************************************************************************************************
ansible               : ok=3    changed=1    unreachable=0    failed=0
@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Contributor

ansibot commented Aug 7, 2018

The test ansible-test sanity --test integration-aliases [explain] failed with 1 error:

test/integration/targets/win_mssql_query/aliases:0:0: missing alias `shippable/windows/group[1-4]` or `unsupported`

The test ansible-test sanity --test pslint [explain] failed with 1 error:

lib/ansible/modules/windows/win_mssql_query.ps1:67:56: PSAvoidUsingConvertToSecureStringWithPlainText File 'win_mssql_query.ps1' uses ConvertTo-SecureString with plaintext. This will expose secure information. Encrypted standard strings should be used instead.

click here for bot help

@ansibot

This comment has been minimized.

Contributor

ansibot commented Aug 7, 2018

@ansibot

This comment was marked as resolved.

Contributor

ansibot commented Aug 7, 2018

The test ansible-test sanity --test pslint [explain] failed with 2 errors:

lib/ansible/modules/windows/win_mssql_query.ps1:67:56: PSAvoidUsingConvertToSecureStringWithPlainText File 'win_mssql_query.ps1' uses ConvertTo-SecureString with plaintext. This will expose secure information. Encrypted standard strings should be used instead.
test/sanity/pslint/ignore.txt:66:1: A102 Remove since "lib/ansible/modules/windows/win_mssql_query.ps1" passes "PSAvoidUsingPlainTextForPassword" test

click here for bot help

@dagwieers

This comment has been minimized.

Member

dagwieers commented Aug 16, 2018

Beware that the deadline for getting new features/modules accepted in Ansible v2.7 is nearing, it is set to either 2018-08-23 or 2018-08-30. If you are blocked, or you need feedback, please discuss on IRC channel #ansible-windows or add a comment to the Windows Working Group meeting agenda to get it resolved.

@ansibot ansibot added the stale_ci label Aug 16, 2018

@jhawkesworth

This comment has been minimized.

Contributor

jhawkesworth commented Aug 22, 2018

This would be a nice module to have, but I think you could give it a basic kind of idempotence without too much difficulty.
You could add 2 new parameters 'creates' and 'removes' which would take sql queries to run. If the 'creates' query returns any results, then the main query will not be run (again). In a similar way you could alternatively specify a 'removes' query which would work the opposite way: If the 'removes' query returns any results, then the main query would be run. This would let you ensure that rows were not present in the database.

@dlazz

This comment has been minimized.

Contributor

dlazz commented Aug 22, 2018

can you give me some example?

@jborean93

Looks great, just a few comments but thanks for the work you've done so far.

Fail-Json -obj $result -message "Error: $($_.exception.message)"
}
if ($res) {
Switch ($res.GetType()|Select-Object -ExpandProperty Name) {

This comment has been minimized.

@jborean93

jborean93 Sep 27, 2018

Contributor

You might be better off doing if ($res -is [Array]) and handle that accordingly. Having codepaths based on the string of the type seems wrong to me.

This comment has been minimized.

@dlazz

dlazz Sep 27, 2018

Contributor

I don't like it too, but the problem here is that if ($res -is [Array]) returns true whether is an Object[] or a DataRow.

This comment has been minimized.

@jborean93

jborean93 Oct 4, 2018

Contributor

That's interesting DataRow doesn't show any indication that it is an Array. You could also try $res.GetType().IsArray and iterate from there.

Should we be only getting the first entry if the value is Object[]? It seems like if it is an array we would be returning all DataRows in that array, e.g. make $result.output a list of DataRow values.

This comment has been minimized.

@dlazz

dlazz Oct 4, 2018

Contributor

Sorry, I was wrong :(

$res = Invoke-Sqlcmd -ServerInstance mysql -Query "Select top 2 name from sys.databases"
Is Array: $($res -is [Array])
Is Array: True
$res = Invoke-Sqlcmd -ServerInstance mysql -Query "Select top 1 name from sys.databases"
Is Array: $($res -is [Array])
Is Array: False

So I think that the code above could become:

if ($res) {
    if ($res -is [Array]){
        $Columns = $res[0].Table.Columns.Caption
    } 
    else {
        $Columns = $res.Table.Columns.Caption
    }
    $output = @($res |Select-Object -Property $Columns)
}
DOCUMENTATION = r'''
---
module: win_mssql_query
version_added: '2.7'

This comment has been minimized.

@jborean93

jborean93 Sep 27, 2018

Contributor

Bump to 2.8

server_instance:
description:
- Name of the sql server instance to query.
default: computername

This comment has been minimized.

@jborean93

jborean93 Sep 27, 2018

Contributor

Keep out the default, add a new entry to the description that says - If not specified, the remote host will be used.

- Name of a sql user with sufficient privileges.
server_instance_password:
description:
- Password for server_instance_user

This comment has been minimized.

@jborean93

jborean93 Sep 27, 2018

Contributor

When referencing another option, enclose that in I(), e.g. I(server_instance_user)

- database where the query has to be executed
default: master
author:
- Daniele Lazzari

This comment has been minimized.

@jborean93

jborean93 Sep 27, 2018

Contributor

Please put your github user id in brackets after the name. This is used so the Ansible bot can ping you on issues and pull requests for this module.

- Daniele Lazzari
notes:
- If the user that runs ansible has the appropriated permission on the db,
- you don't need to set C(server_instance_user) and C(server_instance_password).

This comment has been minimized.

@jborean93

jborean93 Sep 27, 2018

Contributor

No need to put this in a new list entry, just put you in the newline where it is but remove -. You may want to add in that CredSSP, Kerb with cred delgation or become must be used for implicit auth to work.

@@ -0,0 +1,72 @@
#!powershell
# -*- coding: utf-8 -*-

This comment has been minimized.

@jborean93

jborean93 Sep 27, 2018

Contributor

This isn't needed in PowerShell.

$output = @()
try {
if (!($CheckMode)) {
if (!($ServerInstanceUser) -or !($ServerInstancePassword)) {

This comment has been minimized.

@jborean93

jborean93 Sep 27, 2018

Contributor

Consider using splatting here instead;

$sql_params = @{
    ServerInstance = $ServerInstance
    Database = $Database
    Query = $Query
    QueryTimeout = $QueryTimeout
}
if ($ServerInstanceUser -and $ServerInstancePassword) {
    $SQLCredential = New-Object TypeName System.Management.Automation.PSCredential ($ServerInstanceUser, $ServerInstancePassword)
    $sql_params["Credential"] = $SQLCredential
}

Invoke-SqlCmd @sql_params

The benefit to this is you only call Invoke-SqlCmd in one spot, also it's a lot easier to deal with if statements when you aren't checking the false result.

This comment has been minimized.

@dlazz

dlazz Sep 27, 2018

Contributor

Excellent, I didn't know it was possible, I was looking for a solution like this and I'll definitely do it.

@dlazz

This comment has been minimized.

Contributor

dlazz commented Sep 27, 2018

@jborean93 thank you very much for your advices! I will make all the changes you have suggestad as soon as I can!

@dlazz dlazz force-pushed the dlazz:win_mssql_query branch from 099cf8f to 776e053 Nov 21, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Nov 21, 2018

The test ansible-test sanity --test pslint [explain] failed with 3 errors:

lib/ansible/modules/windows/win_mssql_query.ps1:31:17: PSUseDeclaredVarsMoreThanAssignments The variable 'SQLCredentials' is assigned but never used.
test/sanity/pslint/ignore.txt:45:1: A102 Remove since "lib/ansible/modules/windows/win_mapped_drive.ps1" passes "PSAvoidUsingConvertToSecureStringWithPlainText" test
test/sanity/pslint/ignore.txt:46:1: A102 Remove since "lib/ansible/modules/windows/win_mssql_query.ps1" passes "PSAvoidUsingPlainTextForPassword" test

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/windows/win_mssql_query.ps1:0:0: E102 Interpreter line is not "#!powershell"

click here for bot help

@ansibot ansibot added the ci_verified label Nov 21, 2018

@ansibot ansibot added the stale_ci label Dec 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment