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

sensu-handler.rb sends 'http://servername:port' to the API, along with stash request #160

Closed
raffraffraff opened this issue Oct 13, 2016 · 4 comments · Fixed by #162
Closed
Assignees
Milestone

Comments

@raffraffraff
Copy link

raffraffraff commented Oct 13, 2016

I'm using the latest version of sensu (0.26 git clone) and sensu-plugin 1.4.3. My stashes are not honored. I've done some debugging, and I've found that 'sensu-handler.rb' composes a 'correct' uri for each stash check. I've added a 'put' so it logs it to the sensu-server.log. I took the uri it logged and tested it with curl

curl -X -u user:pass http://127.0.0.1:4567/stashes/silence/server/check

This works. The sensu-api.log tells me that the sensu-handler.rb gets a 404 for the same uri. However, I also noticed that the handler appears to be sending the 'http://127.0.0.1:4567' to the http://127.0.0.1:4567.

sensu-api.log - curl

{
    "timestamp": "2016-10-13T17:21:31.135508+0000",
    "level": "info",
    "message": "api response",
    "request": {
        "remote_address": "127.0.0.1",
        "user_agent": "curl/7.50.3",
        "method": "GET",
        "uri": "/stashes/silence/host/check",
        "query_string": null,
        "body": ""
    },
    "status": 200,
    "content_length": 74
}

sensu-api.log - sensu-handler

{
    "timestamp": "2016-10-13T17:21:34.346704+0000",
    "level": "info",
    "message": "api response",
    "request": {
        "remote_address": "127.0.0.1",
        "user_agent": "Ruby",
        "method": "GET",
        "uri": "http://localhost:4567/stashes/silence/host/check",
        "query_string": null,
        "body": ""
    },
    "status": 404,
    "content_length": 0
}

The only way I can replicate this behavior using curl, is to do this:
curl -u user:pass -X GET http://127./0.0.1:4567?http://127.0.0.1:4567/stashes/silence/host/check

My api.json is pretty basic:

{
  "api": {
    "port": 4567,
    "bind": "0.0.0.0",
    "user": "user",
    "password": "pass"
  }
}
@cwjohnston cwjohnston added the Bug label Oct 13, 2016
cwjohnston added a commit to sensu/sensu-build that referenced this issue Oct 13, 2016
sensu-plugin 1.4.3 contains a regression in api_request which we don't want to
ship (see sensu-plugins/sensu-plugin#160).

This reverts commit ced730c.
@cwjohnston
Copy link
Contributor

@raffraffraff thanks for bringing this to our attention; this appears to be a regression.

@cwjohnston cwjohnston added this to the v1.4.4 milestone Oct 13, 2016
@raffraffraff
Copy link
Author

So a gem install sensu-plugin -v 1.4.2 gets me out of the soup, for now. Thanks for the very quick response.

@betorvs
Copy link

betorvs commented Oct 13, 2016

I do this (sensu-plugin-1.4.3/lib/sensu-handler.rb):

136c136
<       req = net_http_req_class(method).new(uri.to_s)
---
>       req = net_http_req_class(method).new(uri)

And my logs changed from this:

{"timestamp":"2016-10-13T16:22:00.926057-0300","level":"info","message":"api response","request":{"remote_address":"127.0.0.1","user_agent":"Ruby","method":"GET","uri":"http://localhost:4567/events/HOSTNAME/fs_writable_tmptmp","query_string":null,"body":""},"status":404,"content_length":0}

To this:

{"timestamp":"2016-10-13T16:24:01.437757-0300","level":"info","message":"api response","request":{"remote_address":"127.0.0.1","user_agent":"Ruby","method":"GET","uri":"/events/HOSTNAME/fs_writable_tmptmp","query_string":null,"body":""},"status":200,"content_length":2127}

@cwjohnston
Copy link
Contributor

As noted in #131 we don't have unit tests for most any aspect of api requests, including stashes. I've started adding a test here https://github.com/sensu-plugins/sensu-plugin/compare/tests/api-request-stash-exists but it is passing on 1.9 where this bug report leads me to believe it should be failing.

Perhaps webmock is hiding the bug? Any thoughts?

cwjohnston added a commit that referenced this issue Dec 5, 2016
Comparing the arguments for `Net::HTTPGenericRequest` on
[1.9.1](https://ruby-doc.org/stdlib-1.9.1/libdoc/net/http/rdoc/Net/HTTPGenericRequest.html)
vs
[2.3.0](https://ruby-doc.org/stdlib-2.3.0/libdoc/net/http/rdoc/Net/HTTPGenericRequest.html),
I see that 1.9.1 expects argument `path` where as 2.3.0 takes `uri_or_path`
argument. If the `uri_or_path` is not a `URI` object, it is treated as the path.

I believe we can unconditionally pass the value of  `uri.path` to
`net_http_req_class(method).new` on both ruby 1.9 and 2.x, as the hostname, port
and scheme are explicitly set when calling `Net::HTTP.start`.

Closes sensu-plugins/sensu-plugins-sensu#18
Closes #160
Closes #161
cwjohnston added a commit that referenced this issue Dec 5, 2016
Comparing the arguments for `Net::HTTPGenericRequest` on
[1.9.1](https://ruby-doc.org/stdlib-1.9.1/libdoc/net/http/rdoc/Net/HTTPGenericRequest.html)
vs
[2.3.0](https://ruby-doc.org/stdlib-2.3.0/libdoc/net/http/rdoc/Net/HTTPGenericRequest.html),
I see that 1.9.1 expects argument `path` where as 2.3.0 takes `uri_or_path`
argument. If the `uri_or_path` is not a `URI` object, it is treated as the path.

I believe we can unconditionally pass the value of  `uri.path` to
`net_http_req_class(method).new` on both ruby 1.9 and 2.x, as the hostname, port
and scheme are explicitly set when calling `Net::HTTP.start`.

Closes sensu-plugins/sensu-plugins-sensu#18
Closes #160
Closes #161
@cwjohnston cwjohnston self-assigned this Dec 5, 2016
@mattyjones mattyjones removed the Bug label Dec 5, 2016
cwjohnston added a commit to sensu/sensu-omnibus that referenced this issue Dec 9, 2016
sensu-plugin 1.4.3 [contains a regression][1] which is now fixed in 1.4.4.

[1]: sensu-plugins/sensu-plugin#160
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants