search

AlfrescoArchive / activiti-cloud-charts

Helm Charts for Activiti cloud Apps
Apache License 2.0
29 stars 28 forks source link

Replace Activiti-Keycloak with Alfresco-Identity-Service #45

Closed cristina-sirbu closed 5 years ago

cristina-sirbu commented 5 years ago

Replace Activiti-Keycloak with Alfresco-Identity-Service

salaboy commented 5 years ago

@igdianov @almerico can you guys review as well?

ryandawsonuk commented 5 years ago

Would definitely like to go this way as per https://github.com/Activiti/Activiti/issues/2450 , these chart changes look good to me and would like to merge... but then we need to do the other things explained in that issue

cristina-sirbu commented 5 years ago

I added to condition for the requirement. I missed that. Good spot! 👍

ryandawsonuk commented 5 years ago

I'm thinking the easiest way to get this working with the new templating of URLs and common chart stuff is to wrap identity-service in an activiti-keycloak chart and let activiti-keycloak chart provide the ingress. Unfortunately to do that I would need to be able to disable the ingress of the identity-service and it currently doesn't support that.

ryandawsonuk commented 5 years ago

I've tried vendoring the chart and adding the option to turn off the ingress but then I hit the problem that it requires a persistent volume for identity service postgres:

persistentvolumeclaim "alfresco-volume-claim" not found

ryandawsonuk commented 5 years ago

I've created a new branch that builds on top of this and tries to disable the ingress and persistence in the identity service chart so that we can use the common template approach to generate ingress url. I had to override the identity service and include as source code to be able to remove its ingress.

In that branch I'm able to deploy everything and see it come up and get into the keycloak admin console and see the users and roles there. I'm able to make calls to the runtime bundle and query via Postman and see the modeling UI talk to the backend.

Not able to get the tests running though - I'm not sure whether we've made the clientId parameterisable in our acceptance tests so think we might need some refactoring in the tests. The realm correctly gets replaced but the request contains client_id=activiti&grant_type=password&username=hradmin i.e. not alfresco

sergiuv2020 commented 5 years ago

@ryandawsonuk Probably because of this -> https://github.com/Activiti/activiti-cloud-modeling/issues/78 hit my head to this today also.

go with ... activiti-cloud-full-example: activiti-cloud-modeling: backend: extraEnv: |

ryandawsonuk commented 5 years ago

I did hit that and had to change it, at least for the charts in this repo. The one you reference in the other repo is basically a duplicate that we use for the internal pipelines.

Now I'm able to at least get tokens in the tests by parameterising the client. Unfortunately I then get 403 Forbidden errors in the services when listing tasks. Listing process instances works fine but with tasks I get:

Caused by: javax.ws.rs.ForbiddenException: HTTP 403 Forbidden
    at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.handleErrorStatus(ClientInvocation.java:197)
    at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.extractResult(ClientInvocation.java:170)
    at org.jboss.resteasy.client.jaxrs.internal.proxy.extractors.BodyEntityExtractor.extractEntity(BodyEntityExtractor.java:60)
    at org.jboss.resteasy.client.jaxrs.internal.proxy.ClientInvoker.invoke(ClientInvoker.java:104)
    at org.jboss.resteasy.client.jaxrs.internal.proxy.ClientProxy.invoke(ClientProxy.java:76)
    at com.sun.proxy.$Proxy293.search(Unknown Source)
    at org.activiti.cloud.services.identity.keycloak.KeycloakUserGroupManager.loadRepresentation(KeycloakUserGroupManager.java:91)
    at org.activiti.cloud.services.identity.keycloak.KeycloakUserGroupManager.getUserGroups(KeycloakUserGroupManager.java:40)

Seems like the group lookup with the client/client user is failing.

It would be good if we could make the clientId configurable in identity service, in addition to providing the option to disable the ingress. Better would be if there were the option to add multiple clients.

ryandawsonuk commented 5 years ago

I gave the client/client user the same roles as it has in the activiti realm json file and now the acc tests pass from a deployment in the ryan-identity-service branch

If we merge from that branch then AIS would become the default in these charts. We'd also need to make the changes necessary in the defaults for the tests, apps and internal charts.

ryandawsonuk commented 5 years ago

Closing this so we can review https://github.com/Activiti/activiti-cloud-charts/pull/51 , which builds on top of the commits