Skip to content
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

Fix health.py read_health_status GET method #653

Merged
merged 1 commit into from Dec 14, 2020

Conversation

el-deano
Copy link
Contributor

@el-deano el-deano commented Dec 9, 2020

Fixed sys.read_health_status GET method passing params as POST body instead of query-string (params).

Testing against 3 node cluster before change, Response 429 returned for 2 out of 3 calls when standby_ok=True, and the URL has no query string.

>>> import logging, hvac
>>> logging.basicConfig()
>>> logging.getLogger().setLevel(logging.DEBUG)
>>> requests_log = logging.getLogger("requests.packages.urllib3")
>>> requests_log.setLevel(logging.DEBUG)
>>> requests_log.propagate = True
>>> client = hvac.Client(url='https://127.0.0.1:8200')
>>> client.sys.read_health_status(method='GET', standby_ok=True)                                                  
DEBUG:urllib3.connectionpool:https://127.0.0.1:8200 "GET /v1/sys/health HTTP/1.1" 200 None                                                                                                         
{'initialized': True, 'sealed': False, 'standby': False, 'performance_standby': False, 'replication_performance_mode': 'disabled', 'replication_dr_mode': 'disabled', 'server_time_utc': 1607527388, 'versi$
n': '1.1.5', 'cluster_name': 'vault-cluster-redacted', 'cluster_id': 'redacted'}
>>> client.sys.read_health_status(method='GET', standby_ok=True)                                                  
DEBUG:urllib3.connectionpool:https://127.0.0.1:8200 "GET /v1/sys/health HTTP/1.1" 429 293                                                                                                          
<Response [429]>                                                                                           
>>> client.sys.read_health_status(method='GET', standby_ok=True)
DEBUG:urllib3.connectionpool:https://127.0.0.1:8200 "GET /v1/sys/health HTTP/1.1" 429 293                
<Response [429]>  

After the fix (same setup as above) query string appears in the URL and a valid dictionary is returned each time as expected with standby_ok=True

>>> client.sys.read_health_status(method='GET', standby_ok=True)
DEBUG:urllib3.connectionpool:https://127.0.0.1:8200 "GET /v1/sys/health?standbyok=True HTTP/1.1" 200 None
{'initialized': True, 'sealed': False, 'standby': True, 'performance_standby': False, 'replication_performance_mode': 'disabled', 'replication_dr_mode': 'disabled', 'server_time_utc': 1607527527, 'versio$
': '1.1.5', 'cluster_name': 'vault-cluster-redacted', 'cluster_id': 'redacted'}

@el-deano el-deano requested a review from a team as a code owner December 9, 2020 15:45
Copy link
Member

@jeffwecan jeffwecan left a comment

Choose a reason for hiding this comment

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

Ah, great catch thanks!

@jeffwecan jeffwecan added bug system backend generally related to the Vault system backend routes labels Dec 14, 2020
@jeffwecan jeffwecan merged commit 969fac9 into hvac:develop Dec 14, 2020
@jeffwecan jeffwecan changed the title fix health.py read_health_status GET method Fix health.py read_health_status GET method Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug system backend generally related to the Vault system backend routes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants