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

Remove admin.secret from configuration #1279

Merged
merged 5 commits into from
Dec 17, 2015
Merged

Remove admin.secret from configuration #1279

merged 5 commits into from
Dec 17, 2015

Conversation

kuenishi
Copy link
Contributor

Instead it is retrieved from Riak directly in startup sequence, from
admin.key specified at configuration file. But in special case
admin.key is specified as "admin-key", it does not retrieve admin
secret from Riak, instead sets it as admin-secret. admin.secret will
be deprecated in near future, which is currently ignored at all.

catch T:E ->
_ = lager:error("Couldn't get admin user (~s) record: ~p",
[Key, {T, E}]),
{T, E}
Copy link
Contributor

Choose a reason for hiding this comment

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

{error, {T,E}}? To satisfy return type of check_admin_creds

@kuenishi
Copy link
Contributor Author

Updated ^^;

@shino
Copy link
Contributor

shino commented Dec 15, 2015

Diff looks nice 👍

First run of r_t with mb flavor:

Test Results:
user_test-cs_multi_backend                  : pass
upgrade_downgrade_test-cs_multi_backend     : pass
too_large_entity_test-cs_multi_backend      : pass
storage_stats_test_2-cs_multi_backend       : pass
storage_stats_test-cs_multi_backend         : pass
storage_stats_detailed_test-cs_multi_backend: pass
stats_test-cs_multi_backend                 : pass
stanchion_switch_test-cs_multi_backend      : fail
sibling_benchmark-cs_multi_backend          : pass
select_gc_bucket_test-cs_multi_backend      : pass
riak_cs_debug_test-cs_multi_backend         : pass
repl_v3_test-cs_multi_backend               : pass
regression_tests_2-cs_multi_backend         : pass
regression_tests-cs_multi_backend           : pass
put_copy_test-cs_multi_backend              : pass
object_get_test-cs_multi_backend            : pass
object_get_conditional_test-cs_multi_backend: pass
mp_upload_test-cs_multi_backend             : pass
migration_mixed_test-cs_multi_backend       : fail
migration_15_to_20_test-cs_multi_backend    : fail
mb_trans_test-cs_multi_backend              : fail
mb_trans2_test-cs_multi_backend             : fail
mb_pg_test-cs_multi_backend                 : pass
mb_disjoint_test-cs_multi_backend           : pass
list_objects_v2_test-cs_multi_backend       : pass
list_objects_test-cs_multi_backend          : pass
legacy_s3_rewrite_test-cs_multi_backend     : pass
gc_tests-cs_multi_backend                   : pass
external_client_tests-cs_multi_backend      : pass
cs743_regression_test-cs_multi_backend      : pass
buckets_test-cs_multi_backend               : pass
block_audit_test-cs_multi_backend           : pass
auth_bypass_test-cs_multi_backend           : pass
active_delete_test-cs_multi_backend         : pass
access_stats_test-cs_multi_backend          : pass
---------------------------------------------
5 Tests Failed
30 Tests Passed

@kuenishi
Copy link
Contributor Author

stanchion_switch_test passed for me in both mb and single SUPERcluster(tm).

@kuenishi
Copy link
Contributor Author

My results:

Test Results:
user_test-cs_multi_backend                  : pass (  38.6 sec)
upgrade_downgrade_test-cs_multi_backend     : pass ( 167.8 sec)
too_large_entity_test-cs_multi_backend      : pass (  35.4 sec)
storage_stats_test_2-cs_multi_backend       : pass (  43.9 sec)
storage_stats_test-cs_multi_backend         : pass (  43.7 sec)
storage_stats_detailed_test-cs_multi_backend: pass (  59.3 sec)
stats_test-cs_multi_backend                 : pass (  36.4 sec)
stanchion_switch_test-cs_multi_backend      : pass (  40.1 sec)
sibling_benchmark-cs_multi_backend          : pass (  70.4 sec)
select_gc_bucket_test-cs_multi_backend      : pass (  65.1 sec)
riak_cs_debug_test-cs_multi_backend         : fail (  20.1 sec)
repl_v3_test-cs_multi_backend               : pass ( 204.9 sec)
regression_tests_2-cs_multi_backend         : pass (  91.3 sec)
regression_tests-cs_multi_backend           : pass (  35.6 sec)
put_copy_test-cs_multi_backend              : pass (  36.7 sec)
object_get_test-cs_multi_backend            : pass (  39.6 sec)
object_get_conditional_test-cs_multi_backend: pass (  31.3 sec)
mp_upload_test-cs_multi_backend             : pass (  43.7 sec)
migration_mixed_test-cs_multi_backend       : pass ( 265.4 sec)
migration_15_to_20_test-cs_multi_backend    : pass ( 249.2 sec)
mb_trans_test-cs_multi_backend              : fail ( 236.4 sec)
mb_trans2_test-cs_multi_backend             : fail ( 279.3 sec)
mb_pg_test-cs_multi_backend                 : pass ( 227.2 sec)
mb_disjoint_test-cs_multi_backend           : pass (  59.6 sec)
list_objects_v2_test-cs_multi_backend       : pass ( 370.3 sec)
list_objects_test-cs_multi_backend          : pass (  48.4 sec)
legacy_s3_rewrite_test-cs_multi_backend     : pass (  51.4 sec)
gc_tests-cs_multi_backend                   : pass ( 106.0 sec)
external_client_tests-cs_multi_backend      : pass ( 171.2 sec)
cs743_regression_test-cs_multi_backend      : pass (  55.1 sec)
buckets_test-cs_multi_backend               : pass (  37.2 sec)
block_audit_test-cs_multi_backend           : pass (  67.9 sec)
auth_bypass_test-cs_multi_backend           : pass (  35.2 sec)
active_delete_test-cs_multi_backend         : pass (  70.3 sec)
access_stats_test-cs_multi_backend          : pass (  57.1 sec)
---------------------------------------------
3 Tests Failed
32 Tests Passed
That's 91.42857142857143% for those keeping score

mb_trans_test is failing because rcs-dev2 node is not able to fetch admin key and secret from Riak (dev2 node). Is riak_host in riak.conf supposed to be the master supercluster member?

@shino
Copy link
Contributor

shino commented Dec 16, 2015 via email

@kuenishi
Copy link
Contributor Author

Please review this. basho/riak_cs_multibag#33

@kuenishi
Copy link
Contributor Author

migration_mixed_test-cs_multi_backend       : fail ( 181.9 sec)
migration_15_to_20_test-cs_multi_backend    : fail ( 353.8 sec)
mb_trans_test-cs_multi_backend              : pass ( 106.6 sec)
mb_trans2_test-cs_multi_backend             : fail ( 278.1 sec)

Instead it is retrieved from Riak directly in startup sequence, from
admin.key specified at configuration file. But in special case
admin.key is specified as "admin-key", it does not retrieve admin
secret from Riak, instead sets it as admin-secret. admin.secret will
be deprecated in near future, which is currently ignored at all.
@kuenishi kuenishi force-pushed the ku/remove-admin-secret branch 2 times, most recently from 2078845 to 3ad6484 Compare December 17, 2015 02:17
@shino
Copy link
Contributor

shino commented Dec 17, 2015

xref basho/riak_cs_multibag#34

@shino
Copy link
Contributor

shino commented Dec 17, 2015

All riak_test cases passed for both basic and mb flavors by the combination with basho/riak_cs_multibag#33.

Great for better security / less operational burden ❗

borshop added a commit that referenced this pull request Dec 17, 2015
Remove admin.secret from configuration

Reviewed-by: shino
@shino
Copy link
Contributor

shino commented Dec 17, 2015

@borshop merge

@borshop borshop merged commit 7ba6eca into 2.1 Dec 17, 2015
@kuenishi kuenishi deleted the ku/remove-admin-secret branch December 17, 2015 07:16
@TJC
Copy link

TJC commented Nov 22, 2016

hey guys -- could you update http://docs.basho.com/riak/cs/2.1.1/cookbooks/configuration/riak-cs/ to reflect the current correct and best practices in regard to the admin secret? I'm confused.

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 this pull request may close these issues.

None yet

4 participants