elektra-web HTTP API #912

omnidan · 2016-09-01T17:24:06Z

Purpose

This is a proposal to allow remote configuration of (multiple) elektra instances.

elektrad: http://docs.elektrad.apiary.io/
clusterd: http://docs.clusterd.apiary.io/

Checklist

split into 2 blueprints
move to doc/apiblueprints/
use {+path} instead of {path} - https://apiblueprint.org/documentation/specification.html#variable-with-reserved-characters-values
return ls/value/meta when accessing kdb paths via GET
http error/response codes
~~output kdb errors via api~~ does this really make sense since we're not dealing with plugins?
~~change links to point to the official repo~~

@markus2330 please review my pull request

markus2330 · 2016-09-01T17:33:50Z

@Namoshek Can you take a look?

Namoshek · 2016-09-05T08:32:46Z

doc/webui/README.md

+ * **GET /instances** - get a list of all instances
+ * **POST /instances** - create a new instance
+ * **GET /instances/:id** - get information about a single instance
+ * **POST /instances/:id** - edit a single instance


For updating/patching you should use PUT or PATCH.
You will also need a method for deleting instances, I guess. Use DELETE for this task.

Both PUT or PATCH make sense, but we cannot use json merge patch because in Elektra null and absent is something different.

With put we might not need DELETE, if everything missing in PUT is deleted.

I changed it to PUT and added DELETE. I'll still keep POST for the /kdb/:path commands, because the same method will be used for creating and editing keys (could additionally add PUT which doesn't allow setting keys that don't exist, not sure if there's a use case for that, though)

markus2330 · 2016-09-05T08:58:31Z

I am missing how to update parts of the configuration tree and how to select different configuration formats for transfer.

Namoshek · 2016-09-05T09:01:35Z

So far the API descriptions seems clear and well thought, except for the HTTP methods here and there. When you change them, make sure you only use either PUT or PATCH, but not both. I think they are interchangable, although some might say that PATCH is used for modifications and PUT for replacements (although this differentiation is quite hard in most scenarios). For removing resources obviously use DELETE. Concerning the rest of your proposal and the structure, I think I would have chosen a similar approach.

For further definition of the REST API, I have some thoughs on my mind that might be helpful for you:

How are you handling meta data?
How are you handling plugin operations / other common tasks (do you plan to support them at all)?

(I think these two points are quite difficult to answer, because there are basically only two options: Use unique API endpoints for each operation, which will bloat the whole API, or encapsulate the operations within one API endpoint, which doesn't really fit the RESTful approach...)
Your HTTP response codes for the elektrad daemon are quite clear, because the daemon can select them on his very own for a single operation. But how are you going to select the response codes for the clusterd daemon, if the response codes of the underlying instances are not all the same (e.g. one instance failed the operation)? (There is a multi-status response code (207) which could be used for the outer response and then you could send the individual response codes as part of the response message for each instance. This way you could probably also avoid having to implement transaction logic (not sure if this would be possible at all, and quite sure this would be out of scope for your work).)
What response codes will you be using within your system at all? (Probably 200, 404, maybe 403, ...)
When you setup an instance, you'll have to setup the elektrad daemon individually via SSH or something. Instead of having to put the daemon into the cluster configuration by hand after that operation, the daemon could register himself (based on a configuration file e.g.). This would also make things easier if the IP or domain of one of your machines (instances) changes for example. Of course, if your cluster management server would change his address, you would have to change the configuration for each machine by hand.
What is your plan for authorization / security?
Will hierarchical clusters be a thing (e.g. cluster A only encapsulates clusters B and C, which then hold real instances)?

I think that is enough to think about for the moment. 😄

Namoshek · 2016-09-05T09:08:18Z

I am missing how to update parts of the configuration tree

Which configuration tree are you talking about? Shouldn't this be easily possible with his

POST /kdb/:path - edit path configuration (similar to kdb set path)

operation? (For this operation I think POST is an appropriate HTTP method by the way.)

markus2330 · 2016-09-05T09:31:49Z

@Namoshek Thank you for the detailed feedback!

meta data

There are configuration formats that handle both config+metadata within the same syntax, so if we have support for arbitrary syntax, we would also handle metadata without more API. It won't work with JSON though.

plugin operations / other common tasks

Which one do you refer to? mounting and such? All of them are basically only key-value operations, but you are right that non-trivial algorithms are involved you want to avoid reimplementing in every language. @BernhardDenner is looking for a good solution here, too.

Which configuration tree are you talking about?

The hierarchy built up within Elektra separated with /. If the API only supports get/set I am missing something like import/export. In any case it is not clear what "similar to get" means.

Namespace support is also unclear.

Namoshek · 2016-09-05T09:47:24Z

There are configuration formats that handle both config+metadata within the same syntax, so if we have support for arbitrary syntax, we would also handle metadata without more API...

... which also depends on the storage plugin you use. The old story, you know. 😄
But because of your comment on this, the question how he is planning to manipulate individual endpoints arises immediately. From the current description it is not clear to me how much a single operation actually can do (e.g. only write a string as value for a key or place whole snippets under a key, like I do it for my application). Answering this would also require to think about blueprints for messages already, I guess.

Which one do you refer to? mounting and such? All of them are basically only key-value operations, but you are right that non-trivial algorithms are involved you want to avoid reimplementing in every language.

Yes, that is what i meant. Implementing these functions within his REST API a second time doesn't sound good, because it would potentially lead to unintended and unexpected behavior in later versions (if something of the plugin system changes). Of course you could leave this task open to the end-user, so he could simply use the normal interface, but I'm not sure if this is userfriendly at all.

The hierarchy built up within Elektra separated with /. If the API only supports get/set I am missing something like import/export. In any case it is not clear what "similar to get" means.

Ah yes, that is what I'm talking about in the first paragraph. Abstracting the import/export function could be done via an additional HTTP parameter repesenting the way how to handle the supplied value (e.g. no parameter = simple value, parameter set = use storage plugin for parameter or something similar).

markus2330 · 2016-09-05T11:39:48Z

place whole snippets under a key

This should only be a workaround for non-supported formats (actually even in this case the line-plugin would be preferable). It would not be possible to transform snippets to other formats nor to validate them if the whole snippet is within a key.

Namoshek · 2016-09-05T11:48:52Z

This should only be a workaround for non-supported formats (actually even in this case the line-plugin would be preferable). It would not be possible to transform snippets to other formats nor to validate them if the whole snippet is within a key.

What I actually meant was that you can place the configuration encapsulated by a snippet in the hierarchy below a certain key, not the snippet as standalone value. But it should still be possible to set single values as well, in my opinion. So the interface would actually offer two operations.

Import of snippets

key1 = value1
key2 = value2

as ini-script imported for path user/some/example/path would result in two keys being stored in the database

kdb set user/some/example/path/key1 "value1"
kdb set user/some/example/path/key2 "value2"

In other words, the provided path would be the root key for the snippet. Namespaces could be done via an optional parameter as well.

Setting of basic values

value1

as single value set for path user/some/example/path would result in one key being stored in the database

kdb set user/some/example/path "value1"

Of course there must be some way to tell the API how the provided value has to be treated.

omnidan · 2016-09-05T11:58:39Z

Thanks for clarifying the set mechanism, @Namoshek !

It would definitely be possible to save snippets under a sub-tree (specified via :path). How do we tell the API that we're saving a snippet and not raw data, though? (maybe a HTTP parameter, e.g. ?data=snippet)

All additional features could be added as HTTP parameters, e.g. ?action=import.

omnidan · 2016-09-05T12:07:19Z

I have updated the document, please re-review.

But how are you going to select the response codes for the clusterd daemon, if the response codes of the underlying instances are not all the same (e.g. one instance failed the operation)?

I would combine the responses from the elektrad daemons (would also include the HTTP error code). If everything is a success, the status code of the combined document will be 200. Otherwise it will show an error (400).

What response codes will you be using within your system at all? (Probably 200, 404, maybe 403, ...)

200, 404 for non-existing keys, 403 for authorization failures

When you setup an instance, you'll have to setup the elektrad daemon individually via SSH or something. Instead of having to put the daemon into the cluster configuration by hand after that operation, the daemon could register himself (based on a configuration file e.g.).

Good idea!

What is your plan for authorization / security?

clusterd could provide an API key (maybe a separate one for each instance) that has to be entered when setting up an elektrad instance (in addition to the clusterd IP)

Will hierarchical clusters be a thing (e.g. cluster A only encapsulates clusters B and C, which then hold real instances)?

I haven't considered that and I'm not sure if there's a use case for it, but it's an interesting idea.

markus2330 · 2016-09-05T13:59:09Z

I would prefer to directly review the blueprint version where the tools say the syntax is ok and which already has examples. Please make sure to respond to all concerns in the new document. If unsure you can add TODO markers.

markus2330 · 2016-09-05T18:19:02Z

I haven't considered that and I'm not sure if there's a use case for it, but it's an interesting idea.

There is definitely a use-case to not have a clusterd (to connect a browser to elektrad), so a proper recursive variants solves both issues:

The browser can directly connect to either clusterd or elektrad
Clusterd can connect to either elektrad and clusterd

To make recursive nesting easily possible, the API for elektrad and clusterd must be identical. Then you need something like GET topology which is trivial for elektrad (return hostname+version), but works recursively for clusterd (return a struct containing the whole topology).

Namoshek · 2016-09-05T18:42:54Z

For me, hierarchical clusters does not imply running several clusterd daemons, although it could make sense in some scenarios (but then the same interface would be required for the clusterd daemon and the elektrad daemon). I was simply talking about hierarchical/recursive clusters within the one cluster service. This would only require to store a meta value for each instance, telling whether it is a real instance or a cluster.

But with the current design even better abstractions are already possible. For example you could create clusters for each software-bundle you use on your servers, e.g. a tomcat7 cluster (which encapsulates instances 1, 3 and 4) and a mariadb10 cluster (which holds instances 1, 2, 4 and 7). If hierarchical clusters were a thing, they could then be clustered together into a production cluster for example.

markus2330 · 2016-09-05T19:35:46Z

running several clusterd

The question is if clusterd is needed at all (either elektrad can manage clusters or the browser can also connect to several elektrad). But that is a different architectural decisions which should not be a part of this document (as already stated).

but then the same interface would be required

It would be a better design to only use different APIs if it really abstracts something inherently different. It does not seem like that the abstraction differs if the API only merges some results together.

hierarchical clusters were a thing

Up to now I do not see a reason why we should forbid them.

Btw. @omnidan Please do not squash commits (as you have in the TODO above).

Namoshek · 2016-09-05T20:10:12Z

The question is if clusterd is needed at all (either elektrad can manage clusters or the browser can also connect to several elektrad). But that is a different architectural decisions which should not be a part of this document (as already stated).

Although you could connect with the browser to multiple machines, you'd still need to store the configuration for your clusters somewhere. The browsers local cache doesn't seem appropriate to me, but with a configuration export & import feature, it could be interesting again. Maybe make another issue for such a discussion and brainstorming, @omnidan?

It would be a better design to only use different APIs if it really abstracts something inherently different. It does not seem like that the abstraction differs if the API only merges some results together.

Be careful with that, I already wrote something about multi-status for a reason. Inventing a multi-status for single resources is not a good design and the other way round you would invent lots of possible bugs and inconveniences when using only a single status code for a whole batch of operations. So there is actually a good reason to use two interfaces.

omnidan · 2016-09-25T17:28:47Z

@markus2330 @Namoshek I have updated the blueprints, please let me know if everything is fine now.

About the kdb errors - is there anything besides plugins that trigger them? I'm not sure if it'll make sense to deal with them specifically in my API (I would just output the error in my frontend), I would just put the stderr result into the message field in case an edge case does happen.

markus2330 · 2016-09-26T15:50:10Z

doc/apiblueprints/clusterd.apib


-_an API and web user interface to remotely manage multiple elektra instances_
+the cluster management server (`clusterd`) provides a REST HTTP API to control
+multiple `elektrad` instances


elektrad instances and also clusterd instances

see #983 (comment)

markus2330 · 2016-09-26T15:51:52Z

doc/apiblueprints/clusterd.apib

- * (multiple) servers running an elektra daemon (`elektrad`)
- * a single cluster management server to communicate with the elektra daemons (`clusterd`)
- * a client (web browser) that accesses the Web UI on the cluster management server
+returns the current version of the API


please use the same text as @Namoshek in #921 (when we all agree)

markus2330 · 2016-09-26T16:11:25Z

doc/apiblueprints/elektrad.apib

+
+### get configuration [GET]
+
+this is the same as calling `kdb get {path}`


Is this only the split-up or did you already update these parts? You should describe the behaviour with text or by means of C-API, not by referring to other implementations.

And we should not repeat problems already known: e.g. #401

it is already updated, but I forgot to remove these descriptions (which aren't accurate anymore now)

omnidan · 2016-10-08T19:22:19Z

@markus2330 please re-review

markus2330 · 2016-10-22T09:12:10Z

doc/apiblueprints/clusterd.apib

+
+### delete configuration [DELETE]
+
+this is the same as calling `kdb rm {path}` on the instance


Please specify behavior

oops, I missed that - fixed now!

markus2330 · 2016-12-08T12:21:14Z

@Namoshek Can you check if everything is fine now and consistent with your API?

Namoshek · 2016-12-08T16:18:35Z

doc/apiblueprints/elektrad.apib

+    + Attributes (Error)
+        + i18n
+            A comprehensive list of possible errors:
+            - `APIKEY_MISSING` api key not specified


Does drafter accept this / it is rendered correctly? I thought you need an empty line before the "extended description" (i.e. add empty line between 46 and 47).

I also think you should write the type to i18n even if string is the default type, it makes the whole thing more clear.

I just checked and yes, it is rendered correctly. Thanks for the notice, I added the string type now!

Namoshek · 2016-12-08T16:19:11Z

doc/apiblueprints/elektrad.apib

+
+ Request (application/json)
+
+        "hello world"


Is this valid json?

It would be great to have an import/export interface where someone can directly upload/download snippets without packaging it into JSON.

The mime type should be fixed then, though.

You mean like I've done it where you send the plain text snippet in one field and its format in another field (of a json object)?

No, I mean the format encoded in the URL, and the content is the configuration file as-is.

@Namoshek yes, it is valid json (simply a json string)

@markus2330 more mime types could be implemented the same way as json is implemented right now (distinguish via request header) - which formats were you thinking of? which ones should be specified in the blueprint?

I think he means something like

POST [...]/set/path/to/var?format=augeas%20lens=Hosts.lns Content-Type: text/plain Body: 127.0.0.1 localhost loopback-adapter 192.168.0.1 gateway

Or in other words: Give format as GET parameter + whole payload is the snippet in plain text.
But normally one would avoid GET parameters in a POST request I think...

Namoshek · 2016-12-08T16:21:39Z

doc/apiblueprints/clusterd.apib

+
+ Request update host of instance (application/json)
+    + Attributes (object)
+        + host: 192.168.0.6


Make type string explicit and add a description for the field.

Nice catch - I did that now!

Namoshek

Some small things I've noticed, but all in all it looks good!

omnidan · 2016-12-26T11:16:59Z

@Namoshek thanks for reviewing! let me know if everything looks fine now 😁

Namoshek · 2016-12-26T11:22:19Z

To me it looks fine now. 👍

markus2330 · 2016-12-27T11:09:39Z

Thanks! Added #1234 for issues to be addressed later (after first version of implementation is merged).

omnidan added 3 commits August 31, 2016 15:01

create initial version of the elektra web proposal

1791d2a

temporarily link to images from fork repo

4a86b16

update webui API

a85a0be

omnidan added the proposal label Sep 1, 2016

point links to the official repo

72e8f4c

Namoshek reviewed Sep 5, 2016
View reviewed changes

markus2330 mentioned this pull request Sep 5, 2016

HTTP API documentation #917

Closed

omnidan added 2 commits September 5, 2016 13:47

use PUT, standardize /instances and /clusters API

1e4e3ca

add DELETE API

5fc13ee

similar to -> same as

4767fdf

add info about response codes

d4dce2a

omnidan added 3 commits September 25, 2016 18:32

return ls/value/meta when accessing kdb paths via GET

a6c183a

document all HTTP status codes/responses

ba4ee87

elektrad.apib: add APIKEY_MISSING responses

5f5acaa

omnidan added 2 commits September 25, 2016 22:48

use http status code 201 for POST responses

7bc8a82

fix i18n value

d4b4b87

markus2330 suggested changes Sep 26, 2016

View reviewed changes

omnidan added 3 commits October 3, 2016 17:21

docuemnt de/assigning instances to clusters

b053755

remove obsolete descriptions

c72956c

standardize /version API

0203fee

omnidan removed the work in progress label Oct 6, 2016

return id of deleted objects

e1319be

markus2330 reviewed Oct 22, 2016

View reviewed changes

markus2330 added the work in progress label Oct 22, 2016

specify behaviour of key get/set/delete

b12eebb

omnidan removed the work in progress label Nov 16, 2016

markus2330 removed the proposal label Nov 23, 2016

markus2330 approved these changes Nov 23, 2016

View reviewed changes

Namoshek reviewed Dec 8, 2016

View reviewed changes

omnidan added 2 commits December 26, 2016 12:10

add string type to i18n fields

1dfdd8e

describe host attribute

91b01aa

markus2330 merged commit ce076b3 into ElektraInitiative:master Dec 27, 2016


		### get configuration [GET]

		this is the same as calling `kdb get {path}`


		### delete configuration [DELETE]

		this is the same as calling `kdb rm {path}` on the instance

elektra-web HTTP API #912

elektra-web HTTP API #912

Conversation

omnidan commented Sep 1, 2016 • edited Loading

Purpose

Checklist

markus2330 commented Sep 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus2330 commented Sep 5, 2016

Namoshek commented Sep 5, 2016

Namoshek commented Sep 5, 2016

markus2330 commented Sep 5, 2016

Namoshek commented Sep 5, 2016

markus2330 commented Sep 5, 2016

Namoshek commented Sep 5, 2016

Import of snippets

Setting of basic values

omnidan commented Sep 5, 2016

omnidan commented Sep 5, 2016 • edited Loading

markus2330 commented Sep 5, 2016

markus2330 commented Sep 5, 2016

Namoshek commented Sep 5, 2016 • edited Loading

markus2330 commented Sep 5, 2016

Namoshek commented Sep 5, 2016

omnidan commented Sep 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus2330 Sep 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omnidan commented Oct 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus2330 commented Dec 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Namoshek Dec 8, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Namoshek left a comment

Choose a reason for hiding this comment

omnidan commented Dec 26, 2016

Namoshek commented Dec 26, 2016

markus2330 commented Dec 27, 2016

omnidan commented Sep 1, 2016 •

edited

Loading

omnidan commented Sep 5, 2016 •

edited

Loading

Namoshek commented Sep 5, 2016 •

edited

Loading

markus2330 Sep 26, 2016 •

edited

Loading

Namoshek Dec 8, 2016 •

edited

Loading