aau-network-security / haaukins

A Highly Accessible and Automated Virtualization Platform for Security Education
https://general.haaukins.com
Apache License 2.0
187 stars 39 forks source link

Feature/integrate grpc gateway #718 #720

Closed mrtrkmn closed 2 years ago

mrtrkmn commented 2 years ago

Closes #718

In addition to the issue #718, some extra modifications required to be taken:

mrtrkmn commented 2 years ago

Even though this is a draft PR, it would be nice to have feedback during development of it.

Mikkelhost commented 2 years ago

Im still not quite the fan of gRPC for user - server relationships. It feels like alot of overhead to bring in proxies just to translate instead of just implementing these enpoints as a REST API. Not sure if time is spent well here since we may consider changing these enpoints to normal HTTP endpoints when we are going for making Haaukins more scalable (distributed agents).

mrtrkmn commented 2 years ago

Im still not quite the fan of gRPC for user - server relationships. It feels like alot of overhead to bring in proxies just to translate instead of just implementing these enpoints as a REST API. Not sure if time is spent well here since we may consider changing these enpoints to normal HTTP endpoints when we are going for making Haaukins more scalable (distributed agents).

Hmm, but there might be some misunderstanding here, applying gRPC-gateway is much more time efficient then completely turning all endpoints to REST API. Since the part that I integrated the gRPC-gateway is haaukins and webclient communication to remove envoy *(since it has not been updated and updates to envoy proxy container is not fun), while turning webclient into completely REST based haaukins can still serve REST and gRPC responses. Once webclient is in all REST, then we can take of Haaukins side. I just consider availability of the platform, so Haaukins will still run and serve both while webclient is turning the endpoint into REST based.

Don't you think it is logical ? @Mikkelhost

Mikkelhost commented 2 years ago

I did not consider that, makes total sense!

mrtrkmn commented 2 years ago

Now it is possible to call all existing gRPC endpoints as REST endpoints. We can start to work on webclient to remove gRPC part and integrate rest :)

some example calls:


 $ curl -X POST -k http://localhost:8090/admin/login -d '{"username":"mrturkmen", "password":"123456"}'

{"token":"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdSI6dHJ1ZSwidW4iOiJtcnR1cmttZW4iLCJ2dSI6MTY1NzU1MjA0MH0.dLDktZ-vWooocWPWQ_ToLthpD5Ocknt5ljYZdMb0E2o", "error":""}

 $ curl --header "token: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdSI6dHJ1ZSwidW4iOiJtcnR1cmttZW4iLCJ2dSI6MTY1NzU1MjA0MH0.dLDktZ-vWooocWPWQ_ToLthpD5Ocknt5ljYZdMb0E2o" -X GET -k http://localhost:8090/admin/user/list 

{"users":[{"username":"mrturkmen", "name":"A", "surname":"mrturkmen", "email":"a@a.com", "createdAt":"2022-06-10T16:43:42+02:00", "isSuperUser":true, "isNPUser":false}], "error":""}

$  curl --header "token: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdSI6dHJ1ZSwidW4iOiJtcnR1cmttZW4iLCJ2dSI6MTY1NzU1MjA0MH0.dLDktZ-vWooocWPWQ_ToLthpD5Ocknt5ljYZdMb0E2o" -X GET -k http://localhost:8090/admin/user/list 
{"users":[{"username":"mrturkmen", "name":"A", "surname":"mrturkmen", "email":"a@a.com", "createdAt":"2022-06-10T16:43:42+02:00", "isSuperUser":true, "isNPUser":false}], "error":""}

$ curl -H "token: 
 eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdSI6dHJ1ZSwidW4iOiJtcnR1cmttZW4iLCJ2dSI6MTY1NzU1MjA0MH0.dLDktZ-vWooocWPWQ_ToLthpD5Ocknt5ljYZdMb0E2o" -X GET -k http://localhost:8090/admin/frontends/list
{"frontends":[{"image":"tiny", "size":"6714368", "memoryMB":"256", "cpu":1}]}
mrtrkmn commented 2 years ago

The config file might need to be re-updated once this PR merged to develop but it is not a big deal.

mrtrkmn commented 2 years ago

Could you review when you are available @Mikkelhost & @Lanestolen ?

mrtrkmn commented 2 years ago

CLI needs to be updated if it is planned to be supported. We can discuss on it @Mikkelhost

mrtrkmn commented 2 years ago

Tests are failing due to CLI incompatibility, since streaming is removed ( reason: not used earlier as well )

Mikkelhost commented 2 years ago

Everything looks good!

So this means as of now we cannot throw away the envoy proxy from the webclient before we have found a fix for the streaming issue?

mrtrkmn commented 2 years ago

Everything looks good!

So this means as of now we cannot throw away the envoy proxy from the webclient before we have found a fix for the streaming issue?

Yes there is a solution for streaming an endpoint for REST clients, and it can be achieved with gRPC web socket proxy, here it is, https://github.com/tmc/grpc-websocket-proxy I am planning to use it and complete this feature along with webclient side.

mrtrkmn commented 2 years ago

Latest changes are synced with webclient requirements on the PR of the webclient which is: https://github.com/aau-network-security/haaukins-webclient/pull/232

mrtrkmn commented 2 years ago

It is ready to ship now, you already reviewed the changes until latest commits, now it would be great to do general review and ship it to servers !! :) @Mikkelhost

mrtrkmn commented 2 years ago

Everything looks good! So this means as of now we cannot throw away the envoy proxy from the webclient before we have found a fix for the streaming issue?

Yes there is a solution for streaming an endpoint for REST clients, and it can be achieved with gRPC web socket proxy, here it is, https://github.com/tmc/grpc-websocket-proxy I am planning to use it and complete this feature along with webclient side.

This did not work out, hence I created new endpoint on Haaukins side which is websocket enpoint, it serves all information instead of gRPC.

Mikkelhost commented 2 years ago

Nice! It is looking great, how often is it sending a message to the client? I can see that it is just running and endless loop until the connection is severed, but shouldn't there be some kind of delay in between each message? Mostly concerned about any performance impact it may have to just keep spamming the client with cpu and memory data :)

mrtrkmn commented 2 years ago

Nice! It is looking great, how often is it sending a message to the client? I can see that it is just running and endless loop until the connection is severed, but shouldn't there be some kind of delay in between each message? Mostly concerned about any performance impact it may have to just keep spamming the client with cpu and memory data :)

It can be arranged no problem, do you have an interval in your mind ? @Mikkelhost

mrtrkmn commented 2 years ago

Arranged with 2 sec but it can be easily changed afterwards :)

Mikkelhost commented 2 years ago

Looks good !