saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.1k stars 5.47k forks source link

[BUG] Consul agent_service_maintenance using incorrect HTTP method #60192

Open xmsk opened 3 years ago

xmsk commented 3 years ago

Description The agent_service_maintenance function int he consul module uses an HTTP GET call rather than an HTTP PUT call as defined in the API documentation https://www.consul.io/api-docs/agent/service#enable-maintenance-mode.

Setup Set up a salt minion with the consul agent installed locally and a service running that is registered in consul.

Steps to Reproduce the behavior Run the consul module using the command

salt '*' consul.agent_service_maintenance consul_url='http://localhost:8500' serviceid='redis' enable='True' reason='Down for upgrade'

Expected behavior The consul service is set into maintenance mode

Additional context The relevant context in the source code is here https://github.com/saltstack/salt/blob/f1da86977d0d690c621c33e1c2ca8eecc261c03c/salt/modules/consul.py#L1164 where the method parameter is omitted which makes it default to HTTP GET.

xmsk commented 3 years ago

I tried fixing it by simply adding method="PUT" to the _query call but it turns out that the Consul API returns an empty body without a Content-Type header

[xmsk@pc] $ curl -v -X PUT http://localhost:8500/v1/agent/service/maintenance/<service>?enable=False
*   Trying ::1:8500...
* connect to ::1 port 8500 failed: Connection refused
*   Trying 127.0.0.1:8500...
* Connected to localhost (127.0.0.1) port 8500 (#0)
> PUT /v1/agent/service/maintenance/capp?enable=False HTTP/1.1
> Host: localhost:8500
> User-Agent: curl/7.76.1
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Vary: Accept-Encoding
< X-Consul-Default-Acl-Policy: allow
< Date: Sat, 15 May 2021 20:32:33 GMT
< Content-Length: 0
< 
* Connection #0 to host localhost left intact

It seems that salt.utils.http.query defaults to decoding it as application/json and salt.utils.json.loads fails when decoding an empty (byte-)string.

Unfortunately I am not familiar enough with the code base to judge what the best way to fix this issue is - I suppose there might be more cases where an empty body is returned and json decoding of an empty string fails?

Does anyone have a good idea on how to fix this?

xmsk commented 3 years ago

I realized that I can just pass in the decode_type parameter to the http.query function to explicitly request plain text decoding. I tested it locally and it seems to work fine now.

sagetherage commented 3 years ago

@xmsk thank you for the issue report and the PR! Looks like the PR needs a test case. We hold Test Clinics via Twitch Tuesdays from 1-2 pm Mountain and Thursdays from 8-10 am Mountain with @waynew on Twitch and the schedule is listed there, too.