Add ssl support to consul_kv lookup #42456
Merged
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
|
@MatrixCrawler this PR contains the following merge commits: Please rebase your branch to remove these commits. |
ansibot
added
affects_2.7
|
@MatrixCrawler this PR contains the following merge commits: Please rebase your branch to remove these commits. |
ansibot
added
the
needs_revision
bcoca
requested changes
Jul 9, 2018
| default: http | ||
| description: Whether to use http or https | ||
| version_added: '2.7' | ||
| verify: |
There was a problem hiding this comment.
normally we use validate_certs and client_cert as names for these options
There was a problem hiding this comment.
I would also use 'ini' entries to allow the user to optionally configure this in ansible.cfg
There was a problem hiding this comment.
@bcoca any hint wehre i can find an example for the ini implementation?
There was a problem hiding this comment.
redis is good example, https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/lookup/redis.py
in any case it is ts simple:
ini:
- section: consule_kv_lookup
key: validate_certs
There was a problem hiding this comment.
@bcoca added the ini keys
|
@MatrixCrawler This PR contains |
|
@MatrixCrawler this PR contains the following merge commits: Please rebase your branch to remove these commits. |
|
@MatrixCrawler this PR contains the following merge commits: Please rebase your branch to remove these commits. |
MatrixCrawler
force-pushed
the
feature/consul_lookup_ssl
branch
from
b659d39
to
b29e00e
Compare
ansibot
removed
merge_commit
ansibot
added
stale_ci
|
@bcoca Is there anythin i can do for this PR? |
bcoca
requested changes
Aug 9, 2018
| scheme: | ||
| default: http | ||
| description: Whether to use http or https | ||
| version_added: '2.7' |
There was a problem hiding this comment.
you describe scheme but give no way to set it?
There was a problem hiding this comment.
What do you mean by that? It can be set via the scheme param (line 131) or will be parsed from ANSIBLE_CONSUL_URL if one uses the environment variables
There was a problem hiding this comment.
@bcoca Can you please elaborate what the change request would be here? I still don't get it
There was a problem hiding this comment.
you have an option 'scheme' .. but the user has no way to set/change the option .. not via env var ini config .. anything
There was a problem hiding this comment.
@bcoca Ok, maybe i am overly tired or otherwise dense, but won't it be set via the kwargs?
For example lookup('consul_kv', 'my/key', host='10.10.10.10', port='2000', scheme='https')
In line 131 i parse the kwargs for scheme
In line 125 i parse the ANSIBLE_CONSUL_URL From the environment. Both results will be used in the consul-library call (line 126 and 134).
Sorry in advance if i am mistaken and just don't see my failure here
There was a problem hiding this comment.
forgot that part, still it seems that rest of options can be set in other ways also, why force user to always set in kwargs
There was a problem hiding this comment.
@bcoca Because i didn't want to introduce backwards-compatibility breaking changes.
host and port were to be set via kwargs prior to my PR, so i thought it would be best to just stick to the scheme here.
If i removed it fom the kwargs and introduced explicit args for that, all usages of the lookup plugin would have to be rewritten wouldn't it?
There was a problem hiding this comment.
@bcoca i gave it a thought. In which way would you imagine that variable to be set in an other way than via the kwargs?
There was a problem hiding this comment.
env var or ini entry, just like you do for host
There was a problem hiding this comment.
@bcoca But it will already be parsed from ANSIBLE_CONSUL_URL. Shall i omit this parsed results if ANSIBLE_CONSUL_SCHEME (just an example name) is set or always and rely on ANSIBLE_CONSUL_SCHEME is set?
ansibot
removed
the
stale_review
ansibot
removed
the
stale_ci
ansibot
added
stale_ci
ansibot
added
support:community
ansibot
removed
stale_ci
|
The test The test The test The test The test The test |
|
The test The test The test The test The test |
|
The test The test The test The test The test |
ansibot
removed
the
ci_verified
|
The test The test The test The test The test The test |
ansibot
removed
the
ci_verified
bcoca
approved these changes
Sep 27, 2018
ansibot
removed
the
needs_revision
MatrixCrawler
added a commit
to MatrixCrawler/ansible
that referenced
this pull request
Oct 4, 2018
* implemented lookup_consul_kv * added missing version_added for consul_url ini section * added default value for ANSIBLE_CONSUL_CLIENT_CERT and added some more documentation * removed trailing whitespaces * fixed indention * Fixes in Documentation * removed trailing whitespace * removed trailing whitespace (cherry picked from commit bacbd4e)
rowshi
added a commit
to rowshi/ansible
that referenced
this pull request
Oct 4, 2018
* fix wording for silencing a sensu client (ansible#46250) * module/systemd: fix logic determining if a service needs to be enabled (ansible#46245) * modules/systemd: fix logic: allow scope to default to 'system' Fix logic introduced in 7ea9094: if 'scope' param is not specified, it defaults to system, but the value of module.params['scope'] is None, not 'system' - so allow for that. * modules/systemd: fix logic: disabled means disabled Fix logic determining whether a service with both systemd and initd files is enabled or disabled. In situations where systemd thinks service is disabled, but rc.d symlinks mark it as enabled, this module wrongly assumes the service is enabled. Fix this logic: disabled means disabled Only when the output from does NOT include disabled, consider the status of rc.d symlinks. This essentially replicates the fixes done to the systemd handling in the "service" module in 3c89a21 Fixes ansible#22303 Fixes ansible#44409 * Corrected typo Corrected typo ansible_serach_path --> ansible_search_path +label: docsite_pr * added new module aci_access_port_block_to_access_port * changed documentation strings * be compliant to pep8 and pylint. Added integration tests. * added parameter leaf_port_blk_description in module and integration tests * added period in description. Changed integration test to work correctly. * Use unicode instead of bytes (ansible#46234) The stdout and stderr values returned from self._low_level_execute() are text, not bytes. This results in an error in Python 3 since str and bytes cannot be concatenated. Changing to unicode type allows this to work without error on Python 2 and Python 3. * Ensure _raw_params retain exact spaces This fixes a few issues related to multi-line YAML strings that eat whitespace and add whitespaces after newlines (that weren't there). * Add integration tests By checking the cmd result with the original we cause these tests to fail on older releases without this PR. * Add unit tests * Add less judgmental hint to use script + template * yes, we do run validate_modules in Shippable (ansible#46280) * Update playbooks_delegation.rst (ansible#46254) Fix example variable lookup. +label: docsite_pr * better error on empty playbook fixes ansible#45480 * Use bytes rather than native string for result (ansible#46281) This prevents a stack trace in Python 3 when the result is an empty file since the file is open in binary mode and a native string in Python 3 is str, not bytes. * Add support for OpenBSD (ansible#46147) * seboolean: Fix issue with changing persistent SELinux boolean values (ansible#22779) Previously, when the active SELinux value was the same as the desired value, regardless of the value of `persistent`, the module would simply exit successfully, assuming no need for update. This made the assumption that the active and persistent values should be the same, but that is not always the case. This modification treats both the active and persistent values separately when checking for the need to update. Note that the persistent update mode, however, will still update the active as well as the persistent value. If this is not desired, it is possible to separately toggle the active value alone. Other changes: * Make the check mode actually perform checks for changes * Organizes semanage commands into set of logical steps * docker_container: ambiguous parameter "stop_timeout" (ansible#43874) * docker_container: Honour stop_timeout when creating docker containers (ansible#43814) * Adjusting description to what actually happens. See docker-py changelog for 2.7.0: 'APIClient.stop will no longer override the stop_timeout value present in the container’s configuration.' * Add a test whether stop_timeout can be configured for the container. * Added changelog. * Integrate with comparisons (by default, ignore stop_timeout value for restarts; will be configurable with PR ansible#44789). * Fix config change code and tests (#2) * Improving wildcard test. * Using correct config. * feat: add scaleway security_group_rule management (ansible#45694) * feat: add scaleway security_group_rule management * homebrew_cask: Fixes ansible#38772 * Fix typo in ec2_vpnc_nat_gateway module (ansible#46304) - Added space between `format` and `in` on return item `create_time` * Add new member to botmeta * Add label pure_storage for all related files. Add team_purestorage macro * Add logstash_plugin example for lower heap size In a scenario where you've set your logstash heap size high, you may need to set your heap lower in order to execute logstash-plugin while logstash is still running. Here's an example of someone running into insufficient memory while executing logstash-plugin: https://discuss.elastic.co/t/logstash-plugin-and-jvm-options/99646 This scenario is probably fairly common, so it may be worth documenting as an example. +label: docsite_pr * Fix callout json plugin show global stats (ansible#43123) * fix callback Plugin json to support global stat set by set_stats module * refactor to not break compatiblity, add trailing comma on output dict * Remove sorted, as it not needed * refactor, to sivels better code * clean some code messup * add changelog fragment * added missing new line * fix pep8 stuff * Mention YAML as a AWS CloudFormation template language (ansible#46345) * avoid tests when ncclient is not installed * Add configuration to override location of ansible-connection (ansible#46128) * Add configuration to override location of ansible-connection * Check var or argv[0] before $PATH * Add the k8s label to the k8s files k8s label for inventory plugin, module_utils and modules * WIP: Check that union Jinja filter can be chained (ansible#46298) * Check that union Jinja filter can be chained * set filters: fix unexpected templating type error this error occurs with Jinja 2.10 since 32ec69d, for example when union filters are chained: $ ansible all -i localhost, -mdebug -a"msg={{ []|union([])|union([]) }}" localhost | FAILED! => { "msg": "Unexpected templating type error occurred on ({{ []|union([])|union([]) }}): unsupported operand type(s) for +: 'set' and 'list'" } * Use specified loopvar in include_role example loop * clean up doc - removed required: false (ansible#46357) * remove comments from multiplle yaml string that breaks tests for now * Use recursive_diff for kubernetes (ansible#45645) Move recursive_diff from cloudfront_distribution to common.dict_transformations and reuse it in k8s modules * New module: routeros_facts – collect facts from RouterOS devices (ansible#46114) * feat(routeros): implement routeros_facts module * fix(routeros): review by felixfontein * fix(routeros): review by ganeshrn * fix(routeros): review by felixfontein * vcenter_folder - fix error events being generated, incorrect parent folders (ansible#45951) * fix error events being generated when vmware folders exist * fix incorrect parent folders being identified (wrong type) * Ignore empty result of rabbitmqctl list_user_permissions (ansible#35598) Fix ansible#34863 * Fix typo in the author's github username. Ansibot is looking at the author's github username to query reviews for modules in the same namespace. * Correct invalid example in jenkins_script docs (ansible#46193) * Add ssl support to consul_kv lookup (ansible#42456) * implemented lookup_consul_kv * added missing version_added for consul_url ini section * added default value for ANSIBLE_CONSUL_CLIENT_CERT and added some more documentation * removed trailing whitespaces * fixed indention * Fixes in Documentation * removed trailing whitespace * removed trailing whitespace * Add examples for bind and unmount to mount module docs PR ansible#46375 * Updates release & maintenance version docs for 2.7 * Rebalance shippable/posix/ CI groups. * disable gather facts and update playbook output (ansible#46409) * disable facts gather and update playbook output * fix output * Move network utils that are used by AWS modules (ansible#45579) * Separate networking tools that may be used by modules outside of networking so changes to networking-only utilities don't trigger AWS integration tests * Add unit tests for moved network utils * Add comment to prevent imports from being mistakenly removed * Move to_bits as well * New option JUNIT_TASK_RELATIVE_PATH to output relative paths. (ansible#37274) * New option JUNIT_TASK_RELATIVE_PATH to output relative paths. It makes the output in Jenkins much more readable as the absolute path was taking the whole width of the screen, leaving no space for the actual message. A makedirs instead of mkdir slipped in as to make the creation of the output directory recursive, hope this is OK. * Add version_added field to doc set to 2.8 * Fix ansible-test multi-group smoke test handling. (ansible#46363) * Fix ansible-test smoke tests across groups. * Fix ansible-test list arg defaults. * Fix ansible-test require and exclude delegation. * Fix detection of Windows specific changes. * Add minimal Windows testing for Python 3.7. * Fix ansible-test encoding issues for exceptions. * Docs: Clean up of 'blockinfile' module docs (ansible#46331) * Docs: Clean up of 'blockinfile' module docs This is part of a series of module doc cleanups. * A few more fixes after review * win_exec: refactor PS exec runner (ansible#45334) * win_exec: refactor PS exec runner * more changes for PSCore compatibility * made some changes based on the recent review * split up module exec scripts for smaller payload * removed C# module support to focus on just error msg improvement * cleaned up c# test classifier code * Change with_*: loops to loop: loops. Fix some examples. (ansible#46373) * VMware: Add check mode support in vmware_host_config_facts (ansible#46272) * Only delete key from redis in-memory cache if present (ansible#35126) Fixes ansible#35120 : the redis cache plugin keeps key/value entries in an in-memory cache to avoid hitting the redis database each time. The problem is that a cache entry is only set when a value is get or set but it is always deleted when trying to delete a value. When the --flush-cache ansible-playbook option is used, the redis cache plugin is first asked to remove every entry corresponding to every hostname present in the inventory. As no value as been set/get so far, it then tries to delete an unexisting value from the cache and hence crashes with a KeyError exception. * Docsite: Add a new tool: Ansible Playbook Grapher (ansible#46435) Add Ansible Playbook Grapher: a tool create a graph representing your Ansible playbook tasks and roles. https://github.com/haidaraM/ansible-playbook-grapher +label: docsite_pr * fixed tests after EAP set to Stop globally, ci_complete * Docs: Cleanup selinux_permissive module docs (ansible#46351) Update the selinux_permissive instructions and add a comment to the provided example. Signed-off-by: Major Hayden <major@redhat.com> * Macports: Add upgrade parameter and replace update_ports with selfupdate (ansible#45049) * macports: Replace update_ports with selfupdate - Macports discourages use of `port sync` and recommends using `port selfupdate` instead. - Keep `update_cache` and `update_ports` as aliases. - No longer require the `name` parameter so that `selfupdate` can be used in a task by itself. * macports: Add upgrade parameter - New upgrade parameter which can be used to upgrade all outdated ports. * Add changelog fragment * Update asa_config.py (ansible#46448) <!--- Your description here --> +label: docsite_pr * Docs: Clean up of 'file' module docs (ansible#46327) This is part of a series of module doc cleanups. * Use default-test-container version 1.3.0. * Docs: iptables module cleanup (ansible#46368) Signed-off-by: Major Hayden <major@redhat.com> * Subscribing to notifications. (ansible#46372) * win_chocolatey: remove test packages after tests are run (ansible#46431) * ps-lint: ignore rules that are not relevant to Ansible (ansible#46376) * Docs: Clean up of 'assemble' module docs (ansible#46328) This is part of a series of module doc cleanups. * Docs: Cleanup make module docs (ansible#46422) Signed-off-by: Major Hayden <major@redhat.com> * Use proper index value with insertbefore on a one line file (ansible#46071) Add tests and changelog * Add file exists check in integration-aliases test. * remove unnecessary space (ansible#46462) kindly advised from jborean93 * Docs: Clean up 'win_service' module docs (ansible#46407) * Docs: Clean up 'win_service' module docs This is part of a series of module doc cleanups. * Fix typo pause vs paused * Use both YAML list syntaxes * Lookup plugin for rabbitmq (ansible#44070) * Adding a basic get lookup for rabbitmq. * Always return a list * If content type is JSON, make accessible via dict. * Fixed incorrect json.loads variable and missing raise * Change to document returned data * Fixed pep8 issues * Adding integration testing * Moving lookup intgration tests to new target * New rabbitmq lookup plugin (ansible#44070). * New rabbitmq lookup plugin (ansible#44070). * PR review feedback updates * Testing pika is installed * Minor mods to tests * Check if connection is already closed or closing * Updated tests and connection testing * PR review feedback updates * PR review include ValueError in AnsibleError output * Suggesting to use set_fact when using returned variable more than once. * Cleaned up some tests, added some notes and handling connection closure on some exceptions. * Removed finally statement and added some additional error handling. * Added some additional error handling. * PR review updates. * Additional integration tests and removing return in finally * Updated version * Changing back to running tests on ubuntu. * Additional tests * Running tests on Ubuntu only * Fixing syntax error * Fixing ingtegration tests and a string/byte issue * Removed non-required test and fixed BOTMETA * Trying to fix integration test failure on ubuntu1404 * Some issues occured when handling messages from the queue with to_native. Switching to to_text resolved the issues. * Renaming channel to queue (thanks dch). Disabling trusty tests. * Fix spelling error in subelements Filter example (ansible#46446) <!--- Your description here --> +label: docsite_pr * updates latest in docs conf.py (ansible#46400) * Docs: Clean up of 'template' module docs (ansible#46297) * Docs: Clean up of 'template' module docs * Changed influenced by review comments * add simple j2 example to vyos_config (ansible#46442) * Docs: Clean up 'win_reboot' module docs (ansible#46377) * Docs: Clean up 'win_reboot' module docs This is part of a series of module doc cleanups. * Remove hypothetical example * Docs: Clean up of 'fetch' module docs (ansible#46330) * Docs: Clean up of 'fetch' module docs This is part of a series of module doc cleanups. * Fixes as suggested by review * Make default_operation optional in netconf_config module (ansible#46333) Fixes ansible#46257 * As per netconf rfc default-operation value is optional, hence removing the default value for default_operation option. * Improve ansible-test environment checking between tests. (ansible#46459) * Add unified diff output to environment validation. This makes it easier to see where the environment changed. * Compare Python interpreters by version to pip shebangs. This helps expose cases where pip executables use a different Python interpreter than is expected. * Query `pip.__version__` instead of using `pip --version`. This is a much faster way to query the pip version. It also more closely matches how we invoke pip within ansible-test. * Remove redundant environment scan between tests. This reuses the environment scan from the end of the previous test as the basis for comparison during the next test. * VMware: Add check mode support to module vmware_host_acceptance (ansible#46260) * Improve module description * Add check mode support * VMware: Add check mode support to module vmware_host_firewall_facts (ansible#46265)
abadger
pushed a commit
that referenced
this pull request
Oct 9, 2018
…6466) * Add ssl support to consul_kv lookup (#42456) * implemented lookup_consul_kv * added missing version_added for consul_url ini section * added default value for ANSIBLE_CONSUL_CLIENT_CERT and added some more documentation * removed trailing whitespaces * fixed indention * Fixes in Documentation * removed trailing whitespace * removed trailing whitespace (cherry picked from commit bacbd4e) * * added changelog fragment
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
SUMMARY
Added the params scheme, cert and verify to the consul_kv lookup module
Fixes #40877
ISSUE TYPE
COMPONENT NAME
lookup/consul_kv
ANSIBLE VERSION