Re: [jclouds] Add keystone user add and delete methods. (#290)
Closed #290. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290
Re: [jclouds] Add keystone user add and delete methods. (#290)
Thanks a lot for the update and the PR @trainman419! I'll close this as a duplicate and we'll move forward with #303. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-37375486
Re: [jclouds] Add keystone user add and delete methods. (#290)
@pnavarro has a point. I'm not seeing POST /users and DELETE /users/{userId}
methods in the [Identity Service admin API
v2.0](http://api.openstack.org/api-ref-identity.html#identity-admin-v2)
However, I do see POST /users and DELETE /users/{userId} methods in the
[Identity Service API
v3](http://api.openstack.org/api-ref-identity.html#User_Calls)
@trainman419 Are these calls intended to be for the Keystone v3 API?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/290#issuecomment-37212671
Re: [jclouds] Add keystone user add and delete methods. (#290)
I'm not OK with this PR. The POST and DELETE operations are included in OS-KASDM extension: https://github.com/openstack/keystone/blob/master/keystone/contrib/admin_crud/core.py#L84. This is way it's been addressed in the OS-KSADM PR: https://github.com/jclouds/jclouds/pull/303 --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-35986674
Re: [jclouds] Add keystone user add and delete methods. (#290)
I'm not OK with this PR. The POST and DELETE operations are included in OS-KASDM extension: @pnavarro: is the problem simply the duplication of functionality, or do you think the implementation is incorrect? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-36009584
Re: [jclouds] Add keystone user add and delete methods. (#290)
@demobox I think the implementation is incorrect, since these actions are just exposed by Keystone API trough an extension, so depending on the deployments It may be disabled. That's why It should not be implemented in the UserApi, that matches the default actions in the default API, but in a API class in an extension package annotated with the corresponding Extension. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-36010284
Re: [jclouds] Add keystone user add and delete methods. (#290)
since these actions are just exposed by Keystone API through an extension Ah, like that. Thanks for explaining. Thoughts on that, @everett-toews? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-36014198
Re: [jclouds] Add keystone user add and delete methods. (#290)
All you have to do is close and reopen. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-35414823
Re: [jclouds] Add keystone user add and delete methods. (#290)
Closed #290. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290
Re: [jclouds] Add keystone user add and delete methods. (#290)
[jclouds-pull-requests #608](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/608/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-35421995
Re: [jclouds] Add keystone user add and delete methods. (#290)
@trainman419 The live test is failing for me. It's running against Keystone v2
API in Havana. I'm running it with the following command in the
`apis/openstack-keystone` dir.
mvn clean test -Plive
-Dtest.openstack-keystone.endpoint=http://162.242.219.27:5000/v2.0/
-Dtest.openstack-keystone.identity=admin:admin
-Dtest.openstack-keystone.credential=devstack
-Dtest=UserApiLiveTest#testAddDeleteUser
I get the error
Failed tests:
testAddDeleteUser(org.jclouds.openstack.keystone.v2_0.features.UserApiLiveTest):
command: POST http://162.242.219.27:35357/v2.0/users HTTP/1.1 failed with
response:
HTTP/1.1 400 Bad Request;
content: [{error: {message: create_user() takes exactly 3 arguments (2
given), code: 400, title: Bad Request}}]
You can that mvn command to test from your side too.
You can also check your logs in
`apis/openstack-keystone/target/test-data/jclouds-wire.log` It appears to me
that the add method isn't send any body content.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/290#issuecomment-35099131
Re: [jclouds] Add keystone user add and delete methods. (#290)
Thanks! I'll give that a try from here and see if I can get the live tests working. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-35120286
Re: [jclouds] Add keystone user add and delete methods. (#290)
[jclouds » jclouds #845](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/845/) UNSTABLE Looks like there's a problem with this pull request [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-35134154
Re: [jclouds] Add keystone user add and delete methods. (#290)
jclouds » jclouds #845 UNSTABLE Real [test failures](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/org.apache.jclouds.api$openstack-keystone/845/testReport/junit/org.jclouds.openstack.keystone.v2_0.features/UserApiExpectTest/testCreateUser/) here. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-35137246
Re: [jclouds] Add keystone user add and delete methods. (#290)
@@ -123,4 +137,29 @@ @Fallback(EmptySetOnNotFoundOr404.class) ListenableFuture? extends Set? extends Role listRolesOfUserOnTenant(@PathParam(userId) String userId, @PathParam(tenantId) String tenantId); + + /** +* @see UserApi#add(String, String, Boolean, String) +*/ + @Named(user:add) + @POST + @SelectJson(user) + @Consumes(MediaType.APPLICATION_JSON) + @Path(/users) + @RequestFilters(AuthenticateRequest.class) + @Fallback(NullOnNotFoundOr404.class) Is the create request supposed to return a 404? All parameters seem to be related to the new user being created, so is this fallback actually needed? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290/files#r9700462
Re: [jclouds] Add keystone user add and delete methods. (#290)
@@ -81,4 +81,21 @@ */ Set? extends Role listRolesOfUserOnTenant(String userId, String tenantId); + /** +* Create a new user in keystone. Creates (per Javadoc style guide - descriptive over imperative) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290/files#r9715276
Re: [jclouds] Add keystone user add and delete methods. (#290)
+ @Path(/users)
+ @RequestFilters(AuthenticateRequest.class)
+ @Fallback(NullOnNotFoundOr404.class)
+ ListenableFuture? extends User add(@PayloadParam(username) String
userName,
+@PayloadParam(email) String
userEmail,
+@PayloadParam(enabled) boolean
enabled,
+@PayloadParam(OS-KSADM:password)
String password);
+
+ /**
+* @see UserApi#delete(String)
+*/
+ @Named(user:delete)
+ @DELETE
+ @Path(/users/{userId})
+ @RequestFilters(AuthenticateRequest.class)
+ @Fallback(TrueOnNotFoundOr404.class)
Is this desirable behaviour? Are all user applications necessarily not going to
care whether the user we try to delete actually existed or not?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/290/files#r9715343
Re: [jclouds] Add keystone user add and delete methods. (#290)
+
+ User newUser = userApi.add(randUser, email, true, password);
+
+ // validate that our new user exists
+ Set? extends User newUsers = userApi.list().concat().toSet();
+ assertTrue(newUsers.contains(newUser));
+
+ // validate that we can delete our user
+ assertTrue(userApi.delete(newUser.getId()));
+
+ // validate that the user no longer exists
+ Set? extends User finalUsers = userApi.list().concat().toSet();
+ assertEquals(initialUsers, finalUsers);
+ }
+
+ public void testDeleteInvalidUser() {
See comment above...is this silent failure what we think users will want in
all cases?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/290/files#r9720539
Re: [jclouds] Add keystone user add and delete methods. (#290)
Fixed javadoc style comments. I'm not particularly tied to the fallback behavior on user deletion; I'm open to other options. Once we figure out what behavior we want, I'll update the javadoc so that it better describes the behavior. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-35018490
Re: [jclouds] Add keystone user add and delete methods. (#290)
@@ -86,4 +88,37 @@ public void testUsersByName() {
}
}
+
+ public void testAddDeleteUser() {
I wrote this as one test because the test for the delete operation requires
that the add operation succeed, and that we know the ID of the user that was
created.
We should be able to figure out which operation failed by looking at which
assert failed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/290/files#r9724443
Re: [jclouds] Add keystone user add and delete methods. (#290)
[jclouds-pull-requests #601](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/601/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-35023554
Re: [jclouds] Add keystone user add and delete methods. (#290)
@@ -123,4 +127,25 @@ @Fallback(EmptySetOnNotFoundOr404.class) ListenableFuture? extends Set? extends Role listRolesOfUserOnTenant(@PathParam(userId) String userId, @PathParam(tenantId) String tenantId); + + /** @see UserApi#add(String, String, Boolean, String) */ Needs proper Javadoc commenting. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290/files#r9694208
Re: [jclouds] Add keystone user add and delete methods. (#290)
@@ -123,4 +127,25 @@ @Fallback(EmptySetOnNotFoundOr404.class) ListenableFuture? extends Set? extends Role listRolesOfUserOnTenant(@PathParam(userId) String userId, @PathParam(tenantId) String tenantId); + + /** @see UserApi#add(String, String, Boolean, String) */ + @Named(user:add) + @POST + @SelectJson(user) + @Consumes(MediaType.APPLICATION_JSON) + @Path(/users) + @RequestFilters(AuthenticateRequest.class) + @Fallback(NullOnNotFoundOr404.class) + ListenableFuture? extends User add(@PayloadParam(username) String userName, +@PayloadParam(email) String userEmail, +@PayloadParam(enabled) boolean enabled, +@PayloadParam(OS-KSADM:password) String password); + + /** @see UserApi#delete(String) */ Needs proper Javadoc commenting. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290/files#r9694209
Re: [jclouds] Add keystone user add and delete methods. (#290)
The code looks good but we really do need at least a couple of live tests before we can accept this. When the live tests are ready I'll test them against [DevStack with Havana](http://blog.phymata.com/2013/12/18/devstack-havana-on-the-rackspace-cloud/). --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-34938954
Re: [jclouds] Add keystone user add and delete methods. (#290)
Addressed javadoc issue. I reformatted the existing comments as well to keep things consistent. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-34939055
Re: [jclouds] Add keystone user add and delete methods. (#290)
[jclouds » jclouds #839](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/839/) SUCCESS This pull request looks good [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-34940066
Re: [jclouds] Add keystone user add and delete methods. (#290)
Added live tests. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-34941215
Re: [jclouds] Add keystone user add and delete methods. (#290)
[jclouds-pull-requests #598](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/598/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-34941409
Re: [jclouds] Add keystone user add and delete methods. (#290)
[jclouds-pull-requests #599](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/599/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-34942022
Re: [jclouds] Add keystone user add and delete methods. (#290)
[jclouds » jclouds #840](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/840/) SUCCESS This pull request looks good [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/290#issuecomment-34942390
