Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Openstack identity url #2655

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

mwhagedorn commented Feb 7, 2014

when using the autho url from a keystone or horizon, its necessary to use "/tokens" on the end of the url to get a authenticate working

Owner

geemus commented Feb 7, 2014

@krames could you review? thanks!

Owner

krames commented Feb 7, 2014

@mwhagedorn I have noticed that myself. I am wondering if we should take this even further and include v2.0/tokens as I would think that v3.0 could potentially have a different path.

http://docs.openstack.org/api/openstack-identity-service/2.0/content/Token_Operations.html

I am also wondering if we need to check to ensure that the provided url ends with a slash.

Thoughts?

Contributor

mwhagedorn commented Feb 7, 2014

how deep down this hole do we want to go :)?

first thought: code as it stands checks for a suffix of /2.0/ to determine whether or not to call the authenticate_v2 method. We could keep this pattern which would imply at some point there is a /3.0/ check and a authenticate_v3 check. But that admittedly could get kinda lame in a hurry.

second thought: where does the OS world sit as far as the introduction of a V3? How far away is that? That might inform whether or not changing the pattern as it stands is worth it. FWIW the HPCloud fog implementation does something very similar here, but that doesnt mean that its the best way :)

Coverage Status

Coverage increased (+0.01%) when pulling 3f29c9c on mwhagedorn:openstack_identity_url into 044ca68 on fog:master.

Contributor

mwhagedorn commented Feb 7, 2014

wondering how you get more details on the travis failure since Shindo isnt super helpful here, and local tests pass...

Owner

krames commented Feb 7, 2014

@mwhagedorn That error seems to crop up from time to time. (I have been meaning to try to squish that bug, but I seem to always be working on something else). If you would like to tackle it, that would be great, but I don't think we need to worry about it for this pr.

Getting back to your initial question. I think it's a safe assumption that most people are aware of their host and port, but not the associated path for OpenStack. Based upon that reasoning, I can see allowing users to specify the host and port like so ``:openstack_auth_uri => http://host:port` and then have fog tack on /v2.0/tokens to the url (or whatever the most popular auth version is at the time). If they specify more than the host and port we should use that unaltered. Do you think that's reasonable or do you think that just contributes more confusion?

Contributor

mwhagedorn commented Feb 7, 2014

@krames. that sounds like an interesting approach. The reason I stumbled on this bug was that the horizon console (devstack) gave a url that didnt work. So it was super confusing. It was only my previous fog and IOS bindings experience that made me recall that authentication requests end with "/tokens". this sounds crazy but what about a hybrid approach. 1) do what you said or 2) also check for a url that doesnt end with "tokens"

Owner

krames commented Feb 10, 2014

@mwhagedorn Sounds good to me

Contributor

mwhagedorn commented Feb 10, 2014

@krames any thoughts on getting this PR moved into master? the ruby 2.1 error cant be repeated locally. Its all good 193, 20, 21. So mr jenkins is holding this up for what appears to be a phantom reason

Coverage Status

Coverage remained the same when pulling 2ddcec3 on mwhagedorn:openstack_identity_url into 044ca68 on fog:master.

Member

rupakg commented Feb 10, 2014

My 2 cents :) I would implement all of the above suggestions. Excerpt from the HP provider.

  hp_auth_uri = options[:hp_auth_uri] || "https://region-a.geo-1.identity.hpcloudsvc.com:35357/v2.0/tokens"
  # append /tokens if missing from auth uri
  @hp_auth_uri = hp_auth_uri.include?('tokens')? hp_auth_uri : hp_auth_uri + "tokens"
  endpoint = URI.parse(@hp_auth_uri)
  @scheme = endpoint.scheme || "https"
  @host = endpoint.host || "region-a.geo-1.identity.hpcloudsvc.com"
  @port = endpoint.port.to_s || "35357"
  if (endpoint.path)
    @auth_path = endpoint.path.slice(1, endpoint.path.length)  # remove the leading slash
  else
    @auth_path = "v2.0/tokens"
  end
  service_url = "#{@scheme}://#{@host}:#{@port}"
Contributor

mwhagedorn commented Feb 10, 2014

@rupak many thanks for the input from the jedi master :)

Member

rupakg commented Feb 10, 2014

Contributor

mwhagedorn commented Feb 10, 2014

@krames, @rupakg... since the state goal here is future compatibility, and I since I opened up the can of worms.. when I do a GET against the base url for the auth url (i.e. no path on the correct host and port), on a recent version of devstack, I get a 300 response (i.e multiple entries) and a json representation of two different possible authorization urls, a V2 one and a V3 one. First question: Do we want to bother with correct RESTful conventions, i.e. discovery of the base URL and pick the url for them given the information returned with HTML return code of 300? This would be slow, but the correct way to traverse a RESTful service offering. (of course this logic would only be triggered if they dont be explicit about the version they want). Second question... how stable is Keystone 3.0, ive not used it. Keystone v3 and v2 are listed as stable. Which version of the url would we pick for the user (assuming again they entered only the base host and port for the user). Third question: the v1 auth endpoint is not even listed when you do a GET against the identity host:port address. Can we kill that entire branch? Thoughts? (and I am using the json representations here, understand there is xml as well)

Owner

krames commented Feb 11, 2014

@mwhagedorn I think we are starting to open up a can of worms here, but I do think there is something to this.

What do you think if we try to provide them with some suggested urls if the one they provided is not valid.

Member

rupakg commented Feb 11, 2014

@krames @mwhagedorn Here is my take. I like simple, but that is just me :) I would suggest that we pick a good workable default, and use it when no url is specified. If the user specifies scheme, host, port just use it as-is, and throw exceptions if they use bad values. If they got everything right, but did not add the /tokens at the end of the auth url, please recognize that and add it. That is all the intelligence we need, I think. Just my thoughts.

Contributor

mwhagedorn commented Feb 11, 2014

@krames @rupakg... ok simple wins :). Of course the RESTful discovery crowd will be displeased.....

Contributor

mwhagedorn commented Feb 11, 2014

so taking rupak's and kyles suggestions... append "/tokens" if not there, if they happen to supply just the scheme, host, port then harvest the information from the HTTP 300 return code, which will contain the available valid URLS from Keystone, and include that in the Exception message.

Owner

krames commented Feb 11, 2014

Sounds like a good approach to me.

Owner

krames commented Mar 17, 2014

We have recently gotten some interested in issue #1210 which is related to this PR.

@mwhagedorn Are you still working on this? Should be close this pr?

Owner

krames commented Mar 17, 2014

I don't believe this is a show stopper, so lets wait to fix this our openstack provider re-write. See #1210 (comment) for more information.

@krames krames closed this Mar 17, 2014

Owner

geemus commented Mar 17, 2014

Thanks!

On Mon, Mar 17, 2014 at 10:25 AM, Kyle Rames notifications@github.comwrote:

Closed #2655 #2655.

Reply to this email directly or view it on GitHubhttps://github.com/fog/fog/pull/2655
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment