Support for basic OpenStack API operations and Keystone authentication #543

Merged
merged 1 commit into from May 31, 2013

Projects

None yet

3 participants

@kellymclaughlin
Contributor

This PR addresses issues #373 and #374. It includes the following:

  • Add rewrite rules for basic bucket and object operations from
    OpenStack Object Storage API
  • Add authentication module for Keystone authentication service
  • Make URL rewrite module configurable
  • Refactoring to further decouple API requests and responses from core
    object store functionality.
  • Add helper module for JSON processing
  • Refactor XML handling code
  • Isolate configuration functions into riak_cs_config module

This result of this PR does not indicate that Riak CS is completely ready to be
dropped-in to an OpenStack deployment, but it is a big step in that
direction. Only the basic object storage API operations are
implemented and error responses may not be correct since they are not
well documented in the OpenStack API documentation. There is also not yet
support for checking for revoked PKI tokens or token caching of any
kind.

There are a number of more general refactoring steps included in these changes and I am interested in any feedback on them, especially if it is something that could be improved upon. Some of the refactoring effort is incomplete and this is largely intentional so that the focus of this effort did not become entirely lost.

I have added a number of entries to the repo wiki that should help
with understanding and testing these changes. See the API and authentication
and the OpenStack integration sections specfically.

Additionally here are a few other links that may be of use:

@andrewjstone andrewjstone commented on the diff May 1, 2013
src/riak_cs_api.erl
+ buckets=[Bucket || Bucket <- Buckets,
+ Bucket?RCS_BUCKET.last_action /= deleted]}.
+
+-type options() :: [{atom(), 'undefined' | binary()}].
+-spec list_objects([string()], binary(), non_neg_integer(), options(), pid()) ->
+ {ok, ?LORESP{}} | {error, term()}.
+list_objects([], _, _, _, _) ->
+ {error, no_such_bucket};
+list_objects(_UserBuckets, Bucket, MaxKeys, Options, RiakPid) ->
+ ListKeysRequest = riak_cs_list_objects:new_request(Bucket,
+ MaxKeys,
+ Options),
+ BinPid = riak_cs_utils:pid_to_binary(self()),
+ CacheKey = << BinPid/binary, <<":">>/binary, Bucket/binary >>,
+ UseCache = riak_cs_list_objects_ets_cache:cache_enabled(),
+ case riak_cs_list_objects_fsm:start_link(RiakPid,
@andrewjstone
andrewjstone May 1, 2013 Contributor

Since we just pass along the error below, I think this should just match on {ok, ListFSMPid} and fail-fast

@andrewjstone andrewjstone commented on the diff May 1, 2013
src/riak_cs_oos_response.erl
+ RD),
+ ResponseBody = [binary_to_list(KeyContent?LOKC.key) ++ "\n" ||
+ KeyContent <- Response?LORESP.contents,
+ KeyContent /= undefined],
+ {ResponseBody, UpdRD, Ctx}.
+
+api_error(Error, RD, Ctx) when is_atom(Error); is_tuple(Error) ->
+ error_response(status_code(Error), error_code(Error), error_message(Error),
+ RD, Ctx).
+
+error_response(StatusCode, _Code, _Message, RD, Ctx) ->
+ {{halt, StatusCode}, RD, Ctx}.
+
+%% @doc Convert an error code string into its corresponding atom
+-spec error_code_to_atom(string()) -> atom().
+error_code_to_atom(ErrorCode) ->
@andrewjstone
andrewjstone May 1, 2013 Contributor

what if this code was replaced with something like this?

error_code_to_atom(ErrorCode) ->
[H | AtomString] = lists:reverse(lists:foldl(fun(Char, Acc) ->
                               case string:to_lower(Char) of
                                   Char ->
                                       [Char | Acc];
                                   Lower ->
                                       [Lower, $_ | Acc]
                               end
                           end, [], ErrorCode)),
list_to_atom(AtomString). 
@andrewjstone
Contributor

The new abstractions are tight. The code looks much cleaner.

  • Eunit and eqc tests pass
  • Client and int tests pass
  • Dialyzer output clean
  • riak_test tests pass
  • openstack api with keystone auth
  • s3 api with keystone auth
@andrewjstone
Contributor

I'm seeing these dialyzer warnings:

riak_cs_passthru_auth.erl:23: Callback info about the riak_cs_auth behaviour is not available
riak_cs_storage_d.erl:384: Function fetch_user_list/1 will never be called
@andrewjstone
Contributor

list_objects_test riak_test is failing with the following error:

15:14:59.102 [warning] list_objects_test failed: {{assertException_failed,[{module,list_objects_test},{line,117},{expression,"erlcloud_s3 : list_objects ( ? TEST_BUCKET , Options7 , UserConfig )"},{pattern,"{ error , { aws_erro
r , { http_error , 400 , _ , _ } } , [...] }"},{unexpected_exception,{error,{aws_error,{socket_error,socket_closed_remotely}},[{erlcloud_s3,s3_request,9,[{file,"src/erlcloud_s3.erl"},{line,867}]},{erlcloud_s3,s3_xml_request,8,[
{file,"src/erlcloud_s3.erl"},{line,811}]},{erlcloud_s3,list_objects,3,[{file,"src/erlcloud_s3.erl"},{line,311}]},{list_objects_test,'-confirm/0-fun-10-',1,[{file,"riak_test/tests/list_objects_test.erl"},{line,118}]},{list_objec
ts_test,confirm,0,[{file,"riak_test/tests/list_objects_test.erl"},{line,117}]}]}}]},[{list_objects_test,'-confirm/0-fun-10-',1,[{file,"riak_test/tests/list_objects_test.erl"},{line,117}]},{list_objects_test,confirm,0,[{file,"ri
ak_test/tests/list_objects_test.erl"},{line,117}]}]}
15:14:59.103 [error]
================ list_objects_test failure stack trace =====================
{{assertException_failed,
     [{module,list_objects_test},
      {line,117},
      {expression,
          "erlcloud_s3 : list_objects ( ? TEST_BUCKET , Options7 , UserConfig )"},
      {pattern,
          "{ error , { aws_error , { http_error , 400 , _ , _ } } , [...] }"},
      {unexpected_exception,
          {error,
              {aws_error,{socket_error,socket_closed_remotely}},
              [{erlcloud_s3,s3_request,9,
                   [{file,"src/erlcloud_s3.erl"},{line,867}]},
               {erlcloud_s3,s3_xml_request,8,
                   [{file,"src/erlcloud_s3.erl"},{line,811}]},
               {erlcloud_s3,list_objects,3,
                   [{file,"src/erlcloud_s3.erl"},{line,311}]},
               {list_objects_test,'-confirm/0-fun-10-',1,
                   [{file,"riak_test/tests/list_objects_test.erl"},
                    {line,118}]},
               {list_objects_test,confirm,0,
                   [{file,"riak_test/tests/list_objects_test.erl"},
                    {line,117}]}]}}]},
 [{list_objects_test,'-confirm/0-fun-10-',1,
      [{file,"riak_test/tests/list_objects_test.erl"},{line,117}]},
  {list_objects_test,confirm,0,
      [{file,"riak_test/tests/list_objects_test.erl"},{line,117}]}]}
============================================================================

15:14:59.103 [notice] list_objects_test Test Run Complete

Test Results:
list_objects_test-riak_cs_kv_multi_backend,riak_kv_bitcask_backend: fail
---------------------------------------------
1 Tests Failed

I see the following in the ~/rt/riak_cs/current/dev/dev1/log/console.log also:

2013-05-02 15:14:59.099 [error] <0.2185.0> gen_fsm <0.2185.0> in state waiting_list_keys terminated with reason: bad arithmetic expression in riak_cs_list_objects_fsm:enough_results/3 line 646
2013-05-02 15:14:59.103 [error] <0.2185.0> CRASH REPORT Process <0.2185.0> with 1 neighbours exited with reason: bad arithmetic expression in riak_cs_list_objects_fsm:enough_results/3 line 646 in gen_fsm:terminate/7 line 611
@andrewjstone
Contributor

Once these dialyzer errors are fixed I'm a 👍 on this PR

riak_cs_oos_response.erl:59: The call riak_cs_oos_response:api_error(Error::{'error',},RD::any(),Ctx::any()) will never return since it differs in the 1st argument from the success typing arguments: (atom() | {'riak_connect_failed',},any(),any())
riak_cs_passthru_auth.erl:23: Callback info about the riak_cs_auth behaviour is not available
riak_cs_s3_response.erl:64: The pattern {'riak_connect_failed', Reason} can never match the type atom()
riak_cs_s3_response.erl:97: The pattern {'riak_connect_failed', _} can never match the type atom()
riak_cs_storage_d.erl:384: Function fetch_user_list/1 will never be called

@andrewjstone
Contributor
  • Eunit and eqc tests pass
  • Client and int tests pass
  • Dialyzer output clean
  • riak_test tests pass
  • openstack api with keystone auth
  • s3 api with keystone auth

👍

@kellymclaughlin
Contributor

Pushed a commit to address an issue with keystone user lookups. The issue is that riak_cs_wm_common:maybe_create_user makes a call (here and here) to create a user, but the return is not checked. This is intentional since the subsequent call to get_user will fail if the user creation was not successful. Whereas the first user lookup uses the openstack user's id instead of properly using the tenant id, this second call to get_user does correctly use the tenant id and the lookup is successful. This allowed this problem to go unnoticed during all previous testing. The negative effect of this problem is that every request results in a user creation request to stanchion which introduces unnecessary latency in processing the request.

@kellymclaughlin
Contributor

Part of this PR inadvertently lays the foundation for #565 by adding a riak_cs_utils:create_user/4 function that allows the user's key_id and key_secret to be externally specified. It does not take any action to expose the functionality via HTTP.

@reiddraper
Contributor

+1 on the additions of c3e31a0 and 6469a09.

@kellymclaughlin kellymclaughlin Support for basic OpenStack API operations and Keystone authentication
Fixes #373 #374

* Add rewrite rules for basic bucket and object operations from
  OpenStack Object Storage API
* Add authentication module for Keystone authentication service
* Make URL rewrite module configurable
* Refactoring to further decouple API requests and responses from core
  object store functionality.
* Add helper module for JSON processing
* Refactor XML handling code
* Isolate configuration functions into riak_cs_config module
83f6ce7
@kellymclaughlin kellymclaughlin merged commit 83f6ce7 into develop May 31, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment