Set password to sqlcmd with env variable for security #153
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
[citest] |
richm
reviewed
Dec 14, 2022
tasks/main.yml
Outdated
|
|
||
| - name: Check if the set password matches the existing password | ||
| command: "{{ __mssql_sqlcmd_login_cmd }} -Q 'SELECT @@VERSION'" | ||
| environment: | ||
| - SQLCMDPASSWORD: "{{ mssql_password | quote }}" |
There was a problem hiding this comment.
does it need to be quoted? not sure how environment variables work in ansible, but if you do something like in python os.environ["SQLCMDPASSWORD"] = somevalue then somevalue does not have to be shell quoted
There was a problem hiding this comment.
Good catch, this should not be shell quoted. I fixed that.
spetrosi
added a commit
to spetrosi/mssql
that referenced
this pull request
Dec 21, 2022
[1.3.0] - 2022-12-21 -------------------- - Add support for SQL Server 2022 (linux-system-roles#148) - Set mssql_version to null by default and require users to specify it - Set mssql_version if user didn't and if SQL Server package exists - Set mssql_version to none if it is not set and no current ver exists - Add workarounds for known issues in SQL Server 2022 - Use delay=3 timeout=40 in wait_for module to avoid unreachable server - Fix creating a read-only cluster and setting db_names to empty list (linux-system-roles#152) - Fix a bug when listener were created on mssql_ha_ag_cluster_type=none - Fix a bug when setting mssql_ha_db_names to empty list didn't work - With sqlcmd, set password with env variable instead of -P for security (linux-system-roles#153) - weekly-ci: do not create a new PR every time - python version depends on platform; upgrade checkout, setup-python; support py311 [citest skip] (linux-system-roles#142) - The python version used now requires a corresponding os version e.g. python 2.7 and python 3.6 are no longer supported on ubuntu-latest - must use 20.04. Update the python matrix to include the os to use as well. - Use checkout@v3 and setup-python@v4 - python 3.11 stable is now supported by setup-python - Add `push` action for status reporting on role main page if missing - Use `docker` for ansible-test if not already doing that - Set __mssql_single_node_test to be false when not set (linux-system-roles#143) - Cleanup tests for vault (linux-system-roles#151) - delete a repeating task added by mistake - use the string name instead of the number for noqa - In clean up playbook also remove repo files - Add tests_idempotency_* to no-vault-variables.txt - Define different test passwords consistently - Incorporate tests_powershell to tests_idempotency - Add tests_input_sql_file_2017 - Remove redundant no_log in tests/tasks/ - Add missing input_sql_file_2017 to no-vault-variables Signed-off-by: Sergei Petrosian <spetrosi@redhat.com>
spetrosi
added a commit
that referenced
this pull request
Dec 21, 2022
[1.3.0] - 2022-12-21 -------------------- - Add support for SQL Server 2022 (#148) - Set mssql_version to null by default and require users to specify it - Set mssql_version if user didn't and if SQL Server package exists - Set mssql_version to none if it is not set and no current ver exists - Add workarounds for known issues in SQL Server 2022 - Use delay=3 timeout=40 in wait_for module to avoid unreachable server - Fix creating a read-only cluster and setting db_names to empty list (#152) - Fix a bug when listener were created on mssql_ha_ag_cluster_type=none - Fix a bug when setting mssql_ha_db_names to empty list didn't work - With sqlcmd, set password with env variable instead of -P for security (#153) - weekly-ci: do not create a new PR every time - python version depends on platform; upgrade checkout, setup-python; support py311 [citest skip] (#142) - The python version used now requires a corresponding os version e.g. python 2.7 and python 3.6 are no longer supported on ubuntu-latest - must use 20.04. Update the python matrix to include the os to use as well. - Use checkout@v3 and setup-python@v4 - python 3.11 stable is now supported by setup-python - Add `push` action for status reporting on role main page if missing - Use `docker` for ansible-test if not already doing that - Set __mssql_single_node_test to be false when not set (#143) - Cleanup tests for vault (#151) - delete a repeating task added by mistake - use the string name instead of the number for noqa - In clean up playbook also remove repo files - Add tests_idempotency_* to no-vault-variables.txt - Define different test passwords consistently - Incorporate tests_powershell to tests_idempotency - Add tests_input_sql_file_2017 - Remove redundant no_log in tests/tasks/ - Add missing input_sql_file_2017 to no-vault-variables Signed-off-by: Sergei Petrosian <spetrosi@redhat.com> Signed-off-by: Sergei Petrosian <spetrosi@redhat.com>
spetrosi
added a commit
that referenced
this pull request
Feb 16, 2023
[1.3.0] - 2023-02-16 -------------------- - Add support for SQL Server 2022 (#148) - Set mssql_version to null by default and require users to specify it - Set mssql_version if user didn't and if SQL Server package exists - Set mssql_version to none if it is not set and no current ver exists - Add workarounds for known issues in SQL Server 2022 - Use delay=3 timeout=40 in wait_for module to avoid unreachable server - Imrpove performance by intputting multiple SQL files with loop internaly (#116) - Make it possible to input multiple file with loop internally - Rename task file and vars for clarity - Make regex for files extension search more strict - Make mode consistent between template and copy tasks - Add support for configuring asynchronous replicas (#121) - Add support for configuring asynchronous replicas - Add __mssql_ha_replica_types variables for code readability - Add test for error when primary is not defined - Use the certificate role to create the cert and the key (#125) - Introduce a variable mssql_tls_certificates to set the certificate_requests. - Add the test case to test_tls_2019.yml - Apply basename to mssql_tls_certificates.name In case a full path of a relative path is set to mssql_tls_certificates.name, just get the basename part of the name and pass it to certificate_requests to create the private key and the cert in /etc/pki/tls where the setype is cert_t and the certificate role has the permission to create the files. - Allow *_input_sql_files vars to take lists and strings (#124) - Add support for read-scale always on clusters (#134) - Add support for read-scale always on clusters - Improve tmp file names and logging for sqlcmd_input_file - Add __mssql_single_node_test as a workaround for single-node tests - Rename mssql_ha_listener_port to mssql_ha_endpoint_port (#166) As per feedback from Microsoft, mssql_ha_listener_port should be called mssql_ha_endpoint_port, as that port is used when creating endpoint for replication between primary and secondary replica. Listener is a term used for AG listener associated with AG which is used to route client connection to primary replica (or read only secondary replica based on configuration and request type). And listener uses tcp_port, hence the confusion. - Restructure README files to split it into available scenarios (#161) - Add AD integration functionality (#159) - Add mssql version to set up - Add clean up for realm to clean_up_mssql.yml - Remove setting passwords with environment: Set password inline for security because setting passwords with environment: reveals the value when running playbooks with high verbosity - Add collection-requirements.yml - Set the mssql_password variable to default null after test verification - Fix creating a read-only cluster and setting db_names to empty list (#152) - Fix a bug when listener were created on mssql_ha_ag_cluster_type=none - Fix a bug when setting mssql_ha_db_names to empty list didn't work - With sqlcmd, set password with env variable instead of -P for security (#153) - Identify the current primary replica and configure ag on it (#113) Previously, the role configured AG on the server that has the `mssql_ha_replica_type: primary` variable set. However, in the case of fail over the primary replica moves to a different server. With this change, the role identifies the current primary server and configures AG on it. - Group input_sql_file.yml tasks to improve performance - Make test work in CI and when testing against multiple hosts manually - Check if primary is available prior to configuring HA (#117) - Previously, in the case that the primary node failed before the role run tasks to configure for high availability, the role failed unexpectedly. Now, the role fails with an error message that primary node is not available. - Add no_log true to tasks listing credentials on output (#119) For sqlcmd_input_sql_file add tasks to block to print the output of sqlcmd in case it fails. It is now required because the task itself has no_log true. - Set __mssql_single_node_test to be false when not set (#143) Remove redundant empty line in weekly CI job - Add a note about not supporting direct upgrade 2017>2022 (#157) - Fixes for AD integration functionality (#172) - Add configuring DNS vars to ad_integration - Install sshpass on client for AD testing - Print errors for tasks with no_log: true for visibility - Add tests/requirements.txt for reqs on Python modules during testing - Set up MSSQL in a separate block to catch errors - Call the ad_integration role with FQDN (#175) - weekly-ci: do not create a new PR every time - python version depends on platform; upgrade checkout, setup-python; support py311 [citest skip] (#142) - The python version used now requires a corresponding os version e.g. python 2.7 and python 3.6 are no longer supported on ubuntu-latest - must use 20.04. Update the python matrix to include the os to use as well. - Use checkout@v3 and setup-python@v4 - python 3.11 stable is now supported by setup-python - Add `push` action for status reporting on role main page if missing - Use `docker` for ansible-test if not already doing that - Set __mssql_single_node_test to be false when not set (#143) - Cleanup tests for vault (#151) - delete a repeating task added by mistake - use the string name instead of the number for noqa - In clean up playbook also remove repo files - Add tests_idempotency_* to no-vault-variables.txt - Define different test passwords consistently - Incorporate tests_powershell to tests_idempotency - Add tests_input_sql_file_2017 - Remove redundant no_log in tests/tasks/ - Add missing input_sql_file_2017 to no-vault-variables - Move all ha-related tasks under a single block to clear code (#118) - Add support for CI testing with ansible_vault Excluding tests that re-define variables because CI provides encrypted variables with env var and they take the highest precedence - Fix Reload service daemon task taking 30 min (#120) - Remove support for Fedora 36 (#123) mssql-server package does not support Fedora at all but it works on Fedora <36. Once Microsoft adds mssql-server package for RHEL 9 it should work on Fedora 36 too. - Clean up role code (#126) - Replace `str` with `string` in README for consistency - Remove flush_handlers from tests because role does it each invocation - add github action for weekly ci (#127) - weekly-ci: do not create a new PR every time - python version depends on platform; upgrade checkout, setup-python; support py311 [citest skip] (#142) The python version used now requires a corresponding os version e.g. python 2.7 and python 3.6 are no longer supported on ubuntu-latest - must use 20.04. Update the python matrix to include the os to use as well. - Use checkout@v3 and setup-python@v4 - python 3.11 stable is now supported by setup-python - Add `push` action for status reporting on role main page if missing - Use `docker` for ansible-test if not already doing that - ansible-lint 6.x fixes (#162) The big one is that ansible-lint doesn't like templates in `name` strings except at the end. In general, Ansible does not like having templated variables in `name` values because it makes it harder to grep the source to find a log message in the source. The other ones are jinja spacing cleanup, use of `true`/`false` instead of `yes`/`no`, and various other cleanup. - Add check for non-inclusive language (#158) - Cleanup non-inclusive words. - Add a check for usage of terms and language that is considered non-inclusive. We are using the woke tool for this with a wordlist that can be found at https://github.com/linux-system-roles/tox-lsr/blob/main/src/tox_lsr/config_files/woke.yml - Create separate github actions for various checks; get rid of monolithic tox.yml Using separate github actions, and especially the official github actions which generally have support for in-line comments, should help greatly with readability and troubleshooting test results. - skip no-changelog errors because it searches changelog in .collection galaxy[no-changelog]: No changelog found. Please add a changelog file. Refer to the galaxy.md file for more info. .collection/galaxy.yml:1 - Skip no-changed-when check in clean_up.yml (#171) - add contents: write permission for branch push - Need `contents: write` permission for branch push for weekly ci job Signed-off-by: Rich Megginson <rmeggins@redhat.com> - Remove shellcheck github action (#173) Remove shellcheck github action since there are no shell scripts in the role.
spetrosi
added a commit
that referenced
this pull request
Feb 16, 2023
[1.3.0] - 2023-02-16 -------------------- - Add support for SQL Server 2022 (#148) - Set mssql_version to null by default and require users to specify it - Set mssql_version if user didn't and if SQL Server package exists - Set mssql_version to none if it is not set and no current ver exists - Add workarounds for known issues in SQL Server 2022 - Use delay=3 timeout=40 in wait_for module to avoid unreachable server - Imrpove performance by intputting multiple SQL files with loop internaly (#116) - Make it possible to input multiple file with loop internally - Rename task file and vars for clarity - Make regex for files extension search more strict - Make mode consistent between template and copy tasks - Add support for configuring asynchronous replicas (#121) - Add support for configuring asynchronous replicas - Add __mssql_ha_replica_types variables for code readability - Add test for error when primary is not defined - Use the certificate role to create the cert and the key (#125) - Introduce a variable mssql_tls_certificates to set the certificate_requests. - Add the test case to test_tls_2019.yml - Apply basename to mssql_tls_certificates.name In case a full path of a relative path is set to mssql_tls_certificates.name, just get the basename part of the name and pass it to certificate_requests to create the private key and the cert in /etc/pki/tls where the setype is cert_t and the certificate role has the permission to create the files. - Allow *_input_sql_files vars to take lists and strings (#124) - Add support for read-scale always on clusters (#134) - Add support for read-scale always on clusters - Improve tmp file names and logging for sqlcmd_input_file - Add __mssql_single_node_test as a workaround for single-node tests - Rename mssql_ha_listener_port to mssql_ha_endpoint_port (#166) As per feedback from Microsoft, mssql_ha_listener_port should be called mssql_ha_endpoint_port, as that port is used when creating endpoint for replication between primary and secondary replica. Listener is a term used for AG listener associated with AG which is used to route client connection to primary replica (or read only secondary replica based on configuration and request type). And listener uses tcp_port, hence the confusion. - Restructure README files to split it into available scenarios (#161) - Add AD integration functionality (#159) - Add mssql version to set up - Add clean up for realm to clean_up_mssql.yml - Remove setting passwords with environment: Set password inline for security because setting passwords with environment: reveals the value when running playbooks with high verbosity - Add collection-requirements.yml - Set the mssql_password variable to default null after test verification - Fix creating a read-only cluster and setting db_names to empty list (#152) - Fix a bug when listener were created on mssql_ha_ag_cluster_type=none - Fix a bug when setting mssql_ha_db_names to empty list didn't work - With sqlcmd, set password with env variable instead of -P for security (#153) - Identify the current primary replica and configure ag on it (#113) Previously, the role configured AG on the server that has the `mssql_ha_replica_type: primary` variable set. However, in the case of fail over the primary replica moves to a different server. With this change, the role identifies the current primary server and configures AG on it. - Group input_sql_file.yml tasks to improve performance - Make test work in CI and when testing against multiple hosts manually - Check if primary is available prior to configuring HA (#117) - Previously, in the case that the primary node failed before the role run tasks to configure for high availability, the role failed unexpectedly. Now, the role fails with an error message that primary node is not available. - Add no_log true to tasks listing credentials on output (#119) For sqlcmd_input_sql_file add tasks to block to print the output of sqlcmd in case it fails. It is now required because the task itself has no_log true. - Set __mssql_single_node_test to be false when not set (#143) Remove redundant empty line in weekly CI job - Add a note about not supporting direct upgrade 2017>2022 (#157) - Fixes for AD integration functionality (#172) - Add configuring DNS vars to ad_integration - Install sshpass on client for AD testing - Print errors for tasks with no_log: true for visibility - Add tests/requirements.txt for reqs on Python modules during testing - Set up MSSQL in a separate block to catch errors - Call the ad_integration role with FQDN (#175) - weekly-ci: do not create a new PR every time - python version depends on platform; upgrade checkout, setup-python; support py311 [citest skip] (#142) - The python version used now requires a corresponding os version e.g. python 2.7 and python 3.6 are no longer supported on ubuntu-latest - must use 20.04. Update the python matrix to include the os to use as well. - Use checkout@v3 and setup-python@v4 - python 3.11 stable is now supported by setup-python - Add `push` action for status reporting on role main page if missing - Use `docker` for ansible-test if not already doing that - Set __mssql_single_node_test to be false when not set (#143) - Cleanup tests for vault (#151) - delete a repeating task added by mistake - use the string name instead of the number for noqa - In clean up playbook also remove repo files - Add tests_idempotency_* to no-vault-variables.txt - Define different test passwords consistently - Incorporate tests_powershell to tests_idempotency - Add tests_input_sql_file_2017 - Remove redundant no_log in tests/tasks/ - Add missing input_sql_file_2017 to no-vault-variables - Move all ha-related tasks under a single block to clear code (#118) - Add support for CI testing with ansible_vault Excluding tests that re-define variables because CI provides encrypted variables with env var and they take the highest precedence - Fix Reload service daemon task taking 30 min (#120) - Remove support for Fedora 36 (#123) mssql-server package does not support Fedora at all but it works on Fedora <36. Once Microsoft adds mssql-server package for RHEL 9 it should work on Fedora 36 too. - Clean up role code (#126) - Replace `str` with `string` in README for consistency - Remove flush_handlers from tests because role does it each invocation - add github action for weekly ci (#127) - weekly-ci: do not create a new PR every time - python version depends on platform; upgrade checkout, setup-python; support py311 [citest skip] (#142) The python version used now requires a corresponding os version e.g. python 2.7 and python 3.6 are no longer supported on ubuntu-latest - must use 20.04. Update the python matrix to include the os to use as well. - Use checkout@v3 and setup-python@v4 - python 3.11 stable is now supported by setup-python - Add `push` action for status reporting on role main page if missing - Use `docker` for ansible-test if not already doing that - ansible-lint 6.x fixes (#162) The big one is that ansible-lint doesn't like templates in `name` strings except at the end. In general, Ansible does not like having templated variables in `name` values because it makes it harder to grep the source to find a log message in the source. The other ones are jinja spacing cleanup, use of `true`/`false` instead of `yes`/`no`, and various other cleanup. - Add check for non-inclusive language (#158) - Cleanup non-inclusive words. - Add a check for usage of terms and language that is considered non-inclusive. We are using the woke tool for this with a wordlist that can be found at https://github.com/linux-system-roles/tox-lsr/blob/main/src/tox_lsr/config_files/woke.yml - Create separate github actions for various checks; get rid of monolithic tox.yml Using separate github actions, and especially the official github actions which generally have support for in-line comments, should help greatly with readability and troubleshooting test results. - skip no-changelog errors because it searches changelog in .collection galaxy[no-changelog]: No changelog found. Please add a changelog file. Refer to the galaxy.md file for more info. .collection/galaxy.yml:1 - Skip no-changed-when check in clean_up.yml (#171) - add contents: write permission for branch push - Need `contents: write` permission for branch push for weekly ci job Signed-off-by: Rich Megginson <rmeggins@redhat.com> - Remove shellcheck github action (#173) Remove shellcheck github action since there are no shell scripts in the role.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.