Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Order of return values was reversed #5357

Merged
merged 1 commit into from
Oct 22, 2016

Conversation

abadger
Copy link
Contributor

@abadger abadger commented Oct 22, 2016

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

apt_key.py

ANSIBLE VERSION
devel 2.2
SUMMARY

My previous fix to apt_key reversed the order of return values and therefore compared the wrong length of ids. This PR fixes that.

Copy link

@ypid ypid left a comment

Choose a reason for hiding this comment

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

LGTM thanks! Fixes: #5237

@abadger abadger merged commit 1182d1f into ansible:devel Oct 22, 2016
@abadger abadger deleted the apt-key-reversed-order branch October 22, 2016 16:55
@abadger abadger added this to the 2.2.0 milestone Oct 22, 2016
@abadger
Copy link
Contributor Author

abadger commented Oct 22, 2016

Thanks for testing @ypid! Sorry that I broke it immediately after fixing it before.

If you want to work on tests for apt_key, we have both integration and unit tests for modules. The integration tests are just ansible roles and live here: https://github.com/ansible/ansible/tree/devel/test/integration/targets You can look at how the apt tests are set up for ideas on how to setup the apt_key tests.

unittests live here: https://github.com/ansible/ansible/tree/devel/test/units/modules/core I've mostly used those for very algorithmic code. (The function that splits up key_id into 8, 16, and everything segments would be something easily tested via the unittests).

@abadger
Copy link
Contributor Author

abadger commented Oct 22, 2016

Merged to devel and stable-2.2 branches. This should be in the 2.2.0 release.

@ypid
Copy link

ypid commented Oct 22, 2016

@abadger Thanks for your work. I will look into unit testing when I come around to submit a new Ansible module but I can not promise it. For now, i pass, sorry.

abadger added a commit that referenced this pull request Oct 24, 2016
apt_key has some handling of key ids that can be security concerns.

See this issue:
#5237

and this pair of PRs:
#5353
#5357
abadger added a commit that referenced this pull request Oct 24, 2016
apt_key has some handling of key ids that can be security concerns.

See this issue:
#5237

and this pair of PRs:
#5353
#5357
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants