-
Notifications
You must be signed in to change notification settings - Fork 44
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
GUACAMOLE-872: Guacamole Rest API Documentation #123
base: main
Are you sure you want to change the base?
Conversation
@samirchahine: I'll try to review the documentation here before too long - it may take a little while (longer) for me to get to it. |
Great! Just changed the title and the commit 👍 Let me know how the review goes! Thank you 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before going through the rest of this, I think it's worth considering whether there is a better approach. Documenting the full REST API within the manual doesn't feel maintainable.
Could this perhaps be accomplished in a similar way to the JSDoc, JavaDoc, etc. API documentation? In the cases of Guacamole's JavaScript, Java, and C APIs, the various functions and structures are documented with comments and annotations which allow API documentation to be generated. Perhaps there are similar tools available for REST services, in particular those that leverage JAX-RS?
<para>Method</para> | ||
<emphasis> | ||
Protocol your Guacamole instance is using e.g. http, https | ||
</emphasis> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "method" is a specific HTTP concept (GET, POST, etc.), not the protocol. You probably mean "protocol" here.
<listitem> | ||
<para>Data Source</para> | ||
<emphasis> | ||
Database your Guacamole is running on e.g. mysql, postgresql |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not necessarily be a database. Any extension can choose to provide data, and the identifier of the authentication provider that serves as the root object for reaching that data will be the value used for data source.
<para>You will recieve the following authentication object: </para> | ||
|
||
<informalexample> | ||
<programlisting>{ | ||
authToken: <replaceable>{token}</replaceable>, | ||
username: <replaceable>'adminuser'</replaceable>, | ||
dataSource: <replaceable>'mysql'</replaceable>, | ||
availableDataSources: <replaceable>[ 'mysql', 'mysql-shared' ]</replaceable> | ||
}</programlisting> | ||
</informalexample> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"will" may misleading here when the example is specific to the response that would be seen if the MySQL backend is being used. If the example is specific to a particular case, that should probably be noted.
<para>Example Payload (protocol dependent)</para> | ||
<informalexample> | ||
<programlisting> | ||
// RDP Connection Payload |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments are not legal within JSON.
// VNC Connection Payload | ||
{ | ||
"parentIdentifier": "ROOT", | ||
"name": "", | ||
"protocol": "vnc", | ||
"parameters": { | ||
"port": "5900", | ||
"read-only": "", | ||
"swap-red-blue": "", | ||
"cursor": "", | ||
"color-depth": "", | ||
"clipboard-encoding": "", | ||
"dest-port": "", | ||
"create-recording-path": "", | ||
"enable-sftp": "", | ||
"sftp-port": "", | ||
"sftp-server-alive-interval": "", | ||
"enable-audio": "", | ||
"hostname": "", | ||
"password": "" | ||
}, | ||
"attributes": { | ||
"max-connections": "", | ||
"max-connections-per-user": "", | ||
"weight": "", | ||
"failover-only": "", | ||
"guacd-port": "", | ||
"guacd-encryption": "", | ||
"guacd-hostname": "" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including multiple JSON bodies within the same listing might be misleading.
Hey, i found something that is not on your file, It's about Killing active Sessions, How can i contribute to this file ? |
I created a repo for guacamole rest api documentation. You can look from here: https://github.com/ridvanaltun/guacamole-rest-api-documentation |
I really think it would be best to look toward some means of autogenerating these docs from existing comments within the code. If that means those comments need to be updated or rephrased, then so be it, but having an entirely separate set of API docs on top of the in-code documentation feels unmaintainable. There must be existing tooling that can be leveraged here. |
I agree, the best approach is generating documents from code itself. |
Yes, but in the meantime accept this pull request and then remove it when you've got the auto-generated solutions. I need a way of importing users from LDAP, which for some reason Guacamole doesn't do, so I need to use the rest interface to do that. Unfortunately it's not documented anywhere else so this needs to be in the manual. |
@EthicsGradient, this pull request (and any pull request in general) will be accepted and merged when the feedback on the pull request is addressed and when time permits. There are comments here regarding the proposed content itself, as well as the approach. If the best approach is to auto-generate the documentation (as seems to be the case), then that really is what we should do, and this pull request should not be merged as a stopgap measure unless that approach is much more difficult. On the contrary, it sounds like that approach would be easier to implement, easier to review, and more correct. You'll notice that, regardless of the above, this pull request is not closed. It is still open and under review. If the comments regarding the content of the pull request are resolved and the pull request as it stands is correct, it will indeed be merged until a better solution renders this one obsolete. ... That said, I really think any effort editing this pull request to satisfy comments thus far would be better spent in identifying an appropriate documentation generation solution which supports JAX-RS, applying it within the guacamole It's also worth pointing out that you can use this proposed documentation as it stands even if we haven't merged it. We necessarily will not merge anything into the project until content/approach questions are resolved, but if you are convinced this documentation is exactly what you need, you can use it right now by building the manual from the downstream branch that is proposed for merge.
Guacamole doesn't do this because it doesn't need to do this. There are some cases where automatic user creation would be necessary (hence GUACAMOLE-708), but LDAP is not one of these. If you are having problems with the webapp that lead you to believe that you need to import users manually, I suggest posting to the [email protected] list and asking for assistance. The community there should be able to help you get things working.
You definitely do not need to use the REST API to manually import LDAP users. LDAP users do not need to be imported. If you have questions on this, we should be able to address them on the list. |
Does Swagger meet the license requirements for Guacamole? https://swagger.io/license/ |
@knacktim: Yes, it seems to be compatible from a license perspective. I think someone would just need to take a stab at actually setting it up, and then see what code changes would be required (if any - I think maybe Swagger uses Java annotations?) to generate the documentation. |
I am willing to work on a Swagger Spec or RAML Spec with someone. I haven't written Java REST code in a long time, though, or C. Can someone point me in the direction of a methodical way to go about finding all the endpoints in the source? Are there annotations like JAXRS or something? -- Thanks! |
Yep, there are indeed. All the servers are annotated with JAX-RS annotations. For example: There are only a few classes that serve as roughly root-level resources and have their own |
I believe there are a couple Maven plugins available that will auto-generate Swagger from JAX-RS annotations, probably with some massaging required. |
Ok. There is a tool from Mulesoft-Labs that auto-generates a RAML from jaxrs annotations. I am going to try it. Perhaps generation of the raml can be made part of the ci/cd chain for a commit or pull request?
Sent from a mobile, please excuse excess typing errors
… On Mar 25, 2021, at 16:46, Mike Jumper ***@***.***> wrote:
Yep, there are indeed. All the servers are annotated with JAX-RS annotations. For example:
https://github.com/apache/guacamole-client/blob/754e9649f1fa0ba225ee42b56ded64bc283d17df/guacamole/src/main/java/org/apache/guacamole/rest/session/SessionRESTService.java#L32-L39
https://github.com/apache/guacamole-client/blob/master/guacamole/src/main/java/org/apache/guacamole/rest/session/SessionResource.java#L77-L93
There are only a few classes that serve as roughly root-level resources and have their own @path annotations. Everything else is accessed through those via sub-resource locators.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
@mthurmaier: I haven't looked at that tool in any detail; however, I would think the one thing we'd need to validate is the terms of licensing for that tool, both for actually using it and any restrictions on the output from the tool. I'll try to peek at it later today and see what it looks like, but there's a chance that we'll either need approval from Apache's Legal team, or that we won't be able to use it due to the restrictions. |
@mthurmaier: Assuming this is the one you're talking about: https://github.com/mulesoft-labs/raml-java-tools We should be good - it is Apache 2.0 licensed. |
Or perhaps this one: https://github.com/mulesoft-labs/raml-for-jax-rs Still Apache 2.0 licensed, so I suspect it'll be fine to use it. |
This one.
Sent from a mobile, please excuse excess typing errors
… On Apr 3, 2021, at 13:29, Virtually Nick ***@***.***> wrote:
Or perhaps this one:
https://github.com/mulesoft-labs/raml-for-jax-rs
Still Apache 2.0 licensed, so I suspect it'll be fine to use it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Hi. I am wondering if there has been any progress on using the jac to raml tool in builds? If not, is there some way I can help this along?
Thanks,
Matthew
Sent from a mobile, please excuse excess typing errors
… On Apr 3, 2021, at 21:45, Matthew Thurmaier ***@***.***> wrote:
This one.
Sent from a mobile, please excuse excess typing errors
>> On Apr 3, 2021, at 13:29, Virtually Nick ***@***.***> wrote:
>>
>
> Or perhaps this one:
>
> https://github.com/mulesoft-labs/raml-for-jax-rs
>
> Still Apache 2.0 licensed, so I suspect it'll be fine to use it.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.
|
@mthurmaier: I started to take a look at it and got part of the way down running down all of the licenses to satisfy the RAT part of the build. So, nothing concrete, yet, still sorting through licensing. |
Thanks Nick! If there is anything I can do to help, please let me know.
Cheers,
Matthew
From: Virtually Nick ***@***.***>
Reply-To: apache/guacamole-manual ***@***.***>
Date: Thursday, May 13, 2021 at 6:55 AM
To: apache/guacamole-manual ***@***.***>
Cc: ***@***.***" ***@***.***>, Mention ***@***.***>
Subject: Re: [apache/guacamole-manual] GUACAMOLE-872: Guacamole Rest API Documentation (#123)
@mthurmaier<https://github.com/mthurmaier>: I started to take a look at it and got part of the way down running down all of the licenses to satisfy the RAT part of the build. So, nothing concrete, yet, still sorting through licensing.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#123 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIANNXCZ3ZAYGT5PGM443B3TNOV23ANCNFSM4IT2D6VQ>.
|
wanting to follow-up on the overall status of this documentation, as it would be extremely useful. As a separate yet related item our organization also currently manages and maintains an API wrapper for the rest API. Referencing @ridvanaltun work for initial understanding, and then guacamole-client itself we manage a full wrapper for all exposed rest options. This one is written in python, and we have been developing and maintaining for the past almost 2 years. src: https://gitlab.com/gacybercenter/open/guacamole-api-wrapper |
I also attempted a typescript api wrapper, but would prefer if there were updated docs / something else to use. |
I have created a new page for information regarding the access of the Guacamole API through a Rest Framework.
In my search for a well documented list of api calls I was never able to find one, rather I broke apart the pre-built API to discover the endpoints necessary to make these functionalities possible.
Please let me know if anything needs to be edited before this can be merged! 😊