Skip to content

(MODULES-3256)(MODULES-2323)(MODULES-2554)(MODULES-3083) Fix sqlserver::login resource #182

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

Merged
merged 6 commits into from
Aug 16, 2016
Merged

(MODULES-3256)(MODULES-2323)(MODULES-2554)(MODULES-3083) Fix sqlserver::login resource #182

merged 6 commits into from
Aug 16, 2016

Conversation

glennsarti
Copy link
Contributor

This PR includes the following changes, all to do with the sqlserver::login resource

(MODULES-3256) Fix create/delete of Windows based logins
(MODULES-2323) Fix deleting an SQL login
(MODULES-2554) Fix remove database
(MODULES-3256) Refactor sqlserver_login_spec acceptance test
(MODULES-3256) Fix creating a login resource with additional attributes
(MODULES-3083) Fix modifying server roles for an existing login

Previously the module would fail to create and delete windows based logins due
to the TSQL scripts querying the `SQL_LOGINS` table.  This table only lists SQL
logins, not Windows logins.  Instead the `SERVER_PRINCIPALS` table is used to
determine login existance and correctness.

This commit also modifies how Windows Group logins are disabled.  Groups can
not be disabled, they can only be denied to connect to the database.  This
requires the use the GRAND/REVOKE server permissions table.  This commit detects
the state of the login using the `server_permissions` table.
@glennsarti glennsarti changed the title (MODULES-3256)(MODULES-2323)(MODULES-2554)(MODULES-3083) Fix sqlserver::resource {WIP}(MODULES-3256)(MODULES-2323)(MODULES-2554)(MODULES-3083) Fix sqlserver::resource Aug 16, 2016
@glennsarti glennsarti changed the title {WIP}(MODULES-3256)(MODULES-2323)(MODULES-2554)(MODULES-3083) Fix sqlserver::resource {WIP}(MODULES-3256)(MODULES-2323)(MODULES-2554)(MODULES-3083) Fix sqlserver::login resource Aug 16, 2016
@glennsarti glennsarti changed the title {WIP}(MODULES-3256)(MODULES-2323)(MODULES-2554)(MODULES-3083) Fix sqlserver::login resource (MODULES-3256)(MODULES-2323)(MODULES-2554)(MODULES-3083) Fix sqlserver::login resource Aug 16, 2016
def create_login_manifest (testcase,login_name,login_password,options = {})
case testcase
when 'SQL_LOGIN user'
login_type = 'SQL_LOGIN'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whitespace question - should this be a full tab under when ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure...typically I do

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Looks like currently its just one space instead of tab

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh I see. I'll fix that up

Previously when attempting to remove a login from an MS SQL Server, a TSQL error
was thrown.  This is due to the `GO` statement in the template.  `GO` is not a
TSQL statement, it is actually a command interpretted by a client, such as
sqlcmd.exe.  This commit changes the template to use only TSQL statements and
uses the `DROP LOGIN` command as the `sp_droplogin` stored procedure has been
deprecated.

This commit re-instates the login deletion after each login acceptance test.

Ref: https://msdn.microsoft.com/en-us/library/ms189767.aspx
Previously when attempting to remove a database from an MS SQL Server, a TSQL
error was thrown.  This is due to the `GO` statement in the template.  `GO` is
not a TSQL statement, it is actually a command interpretted by a TSQL client,
such as sqlcmd.exe.  This commit changes the database delete template to use
only TSQL statements and terminate with a semi-colon.  This change also
reinstates the database deletion in the login_spec and database_spec acceptance
tests.
Previously the acceptance test for the sqlserver::login resource was only
testing a portion of the functionality.  This commit refactors the test suite
and adds the following tests;

- Adds Windows user and group logins to the test suite
- Removes test fixtures for login, database and windows principals on completion
- Parameterizes the manifest generation
- Tests creating a resource as well as modifying an existing resource
- Tests deleting a sqlserver::login resource
- Tests disabling a resource
- Fixes testing errors where it was using a disabled account for db access
- Added function to query the database as `sa` account
Previously when creating a login in a SQL Server it would take to puppet runs
to complete the configuration.  This is due to the ordering of actions in the
`create/login.sql.rb` template.  It would only modify a login if it already
existed.  This commit changes the logic of the script to create the login if it
does not exist and then modify the login with the correct attributes.
Previously when modifying the assigned server roles for a login it would
generate a TSQL error.  This was due to not escaping the role names correctly
with square brackets.  This commit modifies the login template and adds
acceptance tests for this scenario.
@glennsarti
Copy link
Contributor Author

@ferventcoder Fixed typos in the commit messages.

@ferventcoder ferventcoder merged commit 482dc49 into puppetlabs:master Aug 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants