1
0
mirror of https://github.com/ansible/awx.git synced 2024-10-27 09:25:10 +03:00

Merge pull request #6568 from AlanCoding/whoops_not_changed

Do not set changed=True if the object did not change

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul
This commit is contained in:
softwarefactory-project-zuul[bot] 2020-04-10 00:16:02 +00:00 committed by GitHub
commit eeab4b90a5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 220 additions and 48 deletions

View File

@ -44,6 +44,7 @@ class TowerModule(AnsibleModule):
cookie_jar = CookieJar()
authenticated = False
config_name = 'tower_cli.cfg'
ENCRYPTED_STRING = "$encrypted$"
def __init__(self, argument_spec, **kwargs):
args = dict(
@ -574,6 +575,45 @@ class TowerModule(AnsibleModule):
else:
self.exit_json(**self.json_output)
def _encrypted_changed_warning(self, field, old, warning=False):
if not warning:
return
self.warn(
'The field {0} of {1} {2} has encrypted data and may inaccurately report task is changed.'.format(
field, old.get('type', 'unknown'), old.get('id', 'unknown')
))
@staticmethod
def has_encrypted_values(obj):
"""Returns True if JSON-like python content in obj has $encrypted$
anywhere in the data as a value
"""
if isinstance(obj, dict):
for val in obj.values():
if TowerModule.has_encrypted_values(val):
return True
elif isinstance(obj, list):
for val in obj:
if TowerModule.has_encrypted_values(val):
return True
elif obj == TowerModule.ENCRYPTED_STRING:
return True
return False
def objects_could_be_different(self, old, new, field_set=None, warning=False):
if field_set is None:
field_set = set(fd for fd in new.keys() if fd not in ('modified', 'related', 'summary_fields'))
for field in field_set:
new_field = new.get(field, None)
old_field = old.get(field, None)
if old_field != new_field:
return True # Something doesn't match
elif self.has_encrypted_values(new_field) or field not in new:
# case of 'field not in new' - user password write-only field that API will not display
self._encrypted_changed_warning(field, old, warning=warning)
return True
return False
def update_if_needed(self, existing_item, new_item, on_update=None, associations=None):
# This will exit from the module on its own
# If the method successfully updates an item and on_update param is defined,
@ -601,22 +641,17 @@ class TowerModule(AnsibleModule):
self.fail_json(msg="Unable to process update of item due to missing data {0}".format(ke))
# Check to see if anything within the item requires the item to be updated
needs_update = False
for field in new_item:
existing_field = existing_item.get(field, None)
new_field = new_item.get(field, None)
# If the two items don't match and we are not comparing '' to None
if existing_field != new_field and not (existing_field in (None, '') and new_field == ''):
# Something doesn't match so let's update it
needs_update = True
break
needs_patch = self.objects_could_be_different(existing_item, new_item)
# If we decided the item needs to be updated, update it
self.json_output['id'] = item_id
if needs_update:
if needs_patch:
response = self.patch_endpoint(item_url, **{'data': new_item})
if response['status_code'] == 200:
self.json_output['changed'] = True
# compare apples-to-apples, old API data to new API data
# but do so considering the fields given in parameters
self.json_output['changed'] = self.objects_could_be_different(
existing_item, response['json'], field_set=new_item.keys(), warning=True)
elif 'json' in response and '__all__' in response['json']:
self.fail_json(msg=response['json']['__all__'])
else:

View File

@ -123,9 +123,10 @@ def main():
# These will be passed into the create/updates
credential_type_params = {
'name': new_name if new_name else name,
'kind': kind,
'managed_by_tower': False,
}
if kind:
credential_type_params['kind'] = kind
if module.params.get('description'):
credential_type_params['description'] = module.params.get('description')
if module.params.get('inputs'):

View File

@ -450,6 +450,8 @@ def main():
existing_spec = module.get_endpoint(spec_endpoint)['json']
if new_spec != existing_spec:
module.json_output['changed'] = True
if existing_item and module.has_encrypted_values(existing_spec):
module._encrypted_changed_warning('survey_spec', existing_item, warning=True)
on_change = update_survey
if state == 'absent':

View File

@ -209,6 +209,8 @@ def main():
existing_spec = module.get_endpoint(spec_endpoint)
if new_spec != existing_spec:
module.json_output['changed'] = True
if existing_item and module.has_encrypted_values(existing_spec):
module._encrypted_changed_warning('survey_spec', existing_item, warning=True)
on_change = update_survey
if state == 'absent':

View File

@ -241,12 +241,12 @@ def silence_deprecation():
"""The deprecation warnings are stored in a global variable
they will create cross-test interference. Use this to turn them off.
"""
with mock.patch('ansible.module_utils.basic.AnsibleModule.deprecate'):
yield
with mock.patch('ansible.module_utils.basic.AnsibleModule.deprecate') as this_mock:
yield this_mock
@pytest.fixture
def silence_warning():
"""Warnings use global variable, same as deprecations."""
with mock.patch('ansible.module_utils.basic.AnsibleModule.warn'):
yield
with mock.patch('ansible.module_utils.basic.AnsibleModule.warn') as this_mock:
yield this_mock

View File

@ -81,30 +81,6 @@ def test_create_vault_credential(run_module, admin_user, organization, silence_d
assert result['id'] == cred.pk
@pytest.mark.django_db
def test_create_custom_credential_type(run_module, admin_user, silence_deprecation):
# Example from docs
result = run_module('tower_credential_type', dict(
name='Nexus',
description='Credentials type for Nexus',
kind='cloud',
inputs={"fields": [{"id": "server", "type": "string", "default": "", "label": ""}], "required": []},
injectors={'extra_vars': {'nexus_credential': 'test'}},
state='present',
validate_certs='false'
), admin_user)
assert not result.get('failed', False), result.get('msg', result)
assert result.get('changed'), result
ct = CredentialType.objects.get(name='Nexus')
assert result['name'] == 'Nexus'
assert result['id'] == ct.pk
assert ct.inputs == {"fields": [{"id": "server", "type": "string", "default": "", "label": ""}], "required": []}
assert ct.injectors == {'extra_vars': {'nexus_credential': 'test'}}
@pytest.mark.django_db
def test_ct_precedence_over_kind(run_module, admin_user, organization, cred_type, silence_deprecation):
result = run_module('tower_credential', dict(
@ -155,16 +131,15 @@ def test_missing_credential_type(run_module, admin_user, organization):
@pytest.mark.django_db
def test_make_use_of_custom_credential_type(run_module, admin_user, cred_type):
Organization.objects.create(name='test-org')
def test_make_use_of_custom_credential_type(run_module, organization, admin_user, cred_type):
result = run_module('tower_credential', dict(
name='Galaxy Token for Steve',
organization='test-org',
organization=organization.name,
credential_type=cred_type.name,
inputs={'token': '7rEZK38DJl58A7RxA6EC7lLvUHbBQ1'},
state='present'
inputs={'token': '7rEZK38DJl58A7RxA6EC7lLvUHbBQ1'}
), admin_user)
assert not result.get('failed', False), result.get('msg', result)
assert result.get('changed', False), result
cred = Credential.objects.get(name='Galaxy Token for Steve')
assert cred.credential_type_id == cred_type.id
@ -174,3 +149,30 @@ def test_make_use_of_custom_credential_type(run_module, admin_user, cred_type):
assert result['name'] == "Galaxy Token for Steve"
assert result['id'] == cred.pk
@pytest.mark.django_db
def test_secret_field_write_twice(run_module, organization, admin_user, cred_type, silence_warning):
val1 = '7rEZK38DJl58A7RxA6EC7lLvUHbBQ1'
result = run_module('tower_credential', dict(
name='Galaxy Token for Steve',
organization=organization.name,
credential_type=cred_type.name,
inputs={'token': val1}
), admin_user)
assert not result.get('failed', False), result.get('msg', result)
Credential.objects.get(id=result['id']).inputs['token'] == val1
val2 = '7rEZ238DJl5837rxA6xxxlLvUHbBQ1'
result = run_module('tower_credential', dict(
name='Galaxy Token for Steve',
organization=organization.name,
credential_type=cred_type.name,
inputs={'token': val2}
), admin_user)
assert not result.get('failed', False), result.get('msg', result)
Credential.objects.get(id=result['id']).inputs['token'] == val2
assert result.get('changed'), result

View File

@ -0,0 +1,49 @@
from __future__ import (absolute_import, division, print_function)
__metaclass__ = type
import pytest
from awx.main.models import CredentialType
@pytest.mark.django_db
def test_create_custom_credential_type(run_module, admin_user, silence_deprecation):
# Example from docs
result = run_module('tower_credential_type', dict(
name='Nexus',
description='Credentials type for Nexus',
kind='cloud',
inputs={"fields": [{"id": "server", "type": "string", "default": "", "label": ""}], "required": []},
injectors={'extra_vars': {'nexus_credential': 'test'}},
state='present',
), admin_user)
assert not result.get('failed', False), result.get('msg', result)
assert result.get('changed'), result
ct = CredentialType.objects.get(name='Nexus')
assert result['name'] == 'Nexus'
assert result['id'] == ct.pk
assert ct.inputs == {"fields": [{"id": "server", "type": "string", "default": "", "label": ""}], "required": []}
assert ct.injectors == {'extra_vars': {'nexus_credential': 'test'}}
@pytest.mark.django_db
def test_changed_false_with_api_changes(run_module, admin_user):
result = run_module('tower_credential_type', dict(
name='foo',
kind='cloud',
inputs={"fields": [{"id": "env_value", "label": "foo", "default": "foo"}]},
injectors={'env': {'TEST_ENV_VAR': '{{ env_value }}'}},
), admin_user)
assert not result.get('failed', False), result.get('msg', result)
assert result.get('changed'), result
result = run_module('tower_credential_type', dict(
name='foo',
inputs={"fields": [{"id": "env_value", "label": "foo", "default": "foo"}]},
injectors={'env': {'TEST_ENV_VAR': '{{ env_value }}'}},
), admin_user)
assert not result.get('failed', False), result.get('msg', result)
assert not result.get('changed'), result

View File

@ -112,7 +112,7 @@ def test_job_template_with_survey_spec(run_module, admin_user, project, inventor
assert result.get('changed', False), result
jt = JobTemplate.objects.get(pk=result['id'])
# assert jt.survey_spec == survey_spec
assert jt.survey_spec == survey_spec
prior_ct = ActivityStream.objects.count()
result = run_module('tower_job_template', dict(
@ -130,3 +130,38 @@ def test_job_template_with_survey_spec(run_module, admin_user, project, inventor
assert jt.survey_spec == survey_spec
assert ActivityStream.objects.count() == prior_ct
@pytest.mark.django_db
def test_job_template_with_survey_encrypted_default(run_module, admin_user, project, inventory, silence_warning):
spec = {
"spec": [
{
"index": 0,
"question_name": "my question?",
"default": "very_secret_value",
"variable": "myvar",
"type": "password",
"required": False
}
],
"description": "test",
"name": "test"
}
for i in range(2):
result = run_module('tower_job_template', dict(
name='foo',
playbook='helloworld.yml',
project=project.name,
inventory=inventory.name,
survey_spec=spec,
survey_enabled=True
), admin_user)
assert not result.get('failed', False), result.get('msg', result)
assert result.get('changed', False), result # not actually desired, but assert for sanity
silence_warning.assert_called_once_with(
"The field survey_spec of job_template {0} has encrypted data and "
"may inaccurately report task is changed.".format(result['id'])
)

View File

@ -85,7 +85,6 @@ def test_invalid_notification_configuration(run_module, admin_user, organization
@pytest.mark.django_db
@pytest.mark.xfail(reason='Handling API validation changes w.r.t. changed status is an open item')
def test_deprecated_to_modern_no_op(run_module, admin_user, organization):
nt_config = {
'url': 'http://www.example.com/hook',

View File

@ -0,0 +1,47 @@
from __future__ import (absolute_import, division, print_function)
__metaclass__ = type
import pytest
from unittest import mock
from awx.main.models import User
@pytest.fixture
def mock_auth_stuff():
"""Some really specific session-related stuff is done for changing or setting
passwords, so we will just avoid that here.
"""
with mock.patch('awx.api.serializers.update_session_auth_hash'):
yield
@pytest.mark.django_db
def test_create_user(run_module, admin_user, mock_auth_stuff):
result = run_module('tower_user', dict(
username='Bob',
password='pass4word'
), admin_user)
assert not result.get('failed', False), result.get('msg', result)
assert result.get('changed'), result
user = User.objects.get(id=result['id'])
assert user.username == 'Bob'
@pytest.mark.django_db
def test_password_no_op_warning(run_module, admin_user, mock_auth_stuff, silence_warning):
for i in range(2):
result = run_module('tower_user', dict(
username='Bob',
password='pass4word'
), admin_user)
assert not result.get('failed', False), result.get('msg', result)
assert result.get('changed') # not actually desired, but assert for sanity
silence_warning.assert_called_once_with(
"The field password of user {0} has encrypted data and "
"may inaccurately report task is changed.".format(result['id'])
)