Conductor REST API
by Mark McLoughlin
Hi,
Since Rails makes it appear trivial, I hoped we could make some quick
wins wrt to providing a more RESTful API for conductor in the coming
release.
It turns out that we have a fair bit of work to do here, so I've just
summarized my thoughts below.
Does a redmine issue tracking this currently exist? I'd be happy to link
to these notes and perhaps split it up into more discreet redmine tasks.
Cheers,
Mark.
= The API =
An XML/JSON over HTTP interface is not automatically a RESTful
interface. There's a fairly academic definition of REST, but what I look
for is:
- Resource based - i.e. collections of resources with
GET/PUT/POST/DELETE methods
- Resource identifiers - i.e. all resources have an ID and can be
referenced by a URL
- Sane document format representing the resources - i.e. reasonable
looking XML/JSON documents
- GET is a safe method and PUT is idempotent - i.e. GET doesn't
result in state changes
- A nod to HATEOAS - i.e. documents contain links to other resources,
no major reliance on query parameters
- An entry point - i.e. GET on the base URL lists the available
resources
- HTTP authentication supported - i.e. no need for cookies
Another indicator of whether your RESTful API is sane is how well it
works existing tools. For example, with a simple tool like resty, you
should be able to do:
$> curl -L http://github.com/micha/resty/raw/master/resty > resty
$> . ./resty
$> resty http://localhost:3000/conductor -u admin:password -H 'Accept: application/json' -H 'Content-type: application/json'
$> GET /
[ {rel => "pools", href => "/conductor/pools"}, {rel => "deployments", href => "/conductor/deployments"}, ... ]
$> GET /pools
[ {"name" => "default_pool", "id" => 1, "url" => "/conductor/pools/1", ...}, ...}]
$> POST /deployments --data-binary '{"name" => "test", "deployable_url" => "...", "realm" => {"id" => "1"}}'
{"name" => "test", id => "2", href => "/conductor/deployments/2", "deployable_url" => "...", "deployable_xml" => "...", "realm" => {"id" => "1", "href" => "/conductor/realms/1"}, ...}
Good examples of API styles which we should try to follow include the
Deltacloud and RHEV-M APIs. See also:
http://fedoraproject.org/wiki/Cloud_APIs_REST_Style_Guide.
= Current State =
The providers, provider_types and provider_accounts collections have
some XML representation support which aeolus-image uses.
We also have minimal JSON support in the pools, instances and
deployments collections.
You can create various resources using POST and query parameters even
where JSON or XML isn't supported e.g.
$> POST /pools -d pool[name]=foo -d pool[pool_family_id]=1
Finally, we already have HTTP authentication support allowing you do
just do:
$> resty http://localhost:3000/conductor -u admin:password
= Default Media Type =
If no media type is specified via an Accept header or otherwise, we
should have a predictable default media type.
Preferably, that would be XML or JSON, since browsers always specify a
media type.
Rails 2 defaults to the first format in the respond_to block, which is
often either JS or HTML in our code.
It looks like Rails 3 always default to HTML[1], so either we change
that to XML/JSON or stick with HTML by default.
[1] - See ActionDispatch::Http::MimeNegotiation::formats()
= XML Document Format =
Our basic XML document format should be e.g.
<bars>
<bar id="..." href="/conductor/...">
<name>...</name>
<property>value</property>
...
<bar>
</bars>
More specific modelling would help in cases where there are multiple
related properties e.g.
<user id="..." href="/users/...">
<name>foo</name>
<logins>
<count>2</count>
<failed_count>0</failed_count>
<current_login>
<ip>...</ip>
<date>...</date>
</current_login>
<last_login>
<ip>...</ip>
<date>...</date>
</last_login>
</logins>
</user>
Also, if you consider the provider_type_id property in providers, do we
want to automate it's serialization as:
<provider id="..." href="/providers/...">
<name>foo</name>
<provider_type id="1" href="/providers_types/1"/>
...
</provider>
Or prefer simply:
<provider id="..." href="/providers/...">
<name>foo</name>
<type>ec2</type>
...
</provider>
Automated serialization of the ActiveRecord models only gets us so far,
we need to do XML-specific modelling too.
= JSON Document Format =
Our JSON document format should be a very simple mapping from the XML
format. That should be easier to implement and document. It also doesn't
make sense to have a different serialization.
For example:
[ {"id" => "...", "href" => "/conductor/...", "name" => "...",
"property" => "value", ... }, ... ]
Note that we currently do:
ActiveRecord::Base.include_root_in_json = true
which gives:
[ {"bar" => {"id" => "...", "href" => "/conductor/...", "name" =>
"...", "property" => "value", ... } }, {"bar" => {...} }, ... ]
This seems fairly unnatural and redundant, so we should probably turn it
off. However, at least the code in backbone.rails.js which relies on
this format would need to be updated.
JSON serialization happens using the to_json and as_json methods. The
latter returns a string and the former returns something which is
automatically serializable.
For example, to omit some internal properties from the JSON, you can do:
class Provider < ActiveRecord::Base
...
def as_json(options = nil)
options[:except] ||= []
options[:except] += [:created_at, :updated_at]
options[:except] += [:lock_version]
super(options)
end
...
end
In Rails 3, there is a serializable_hash method which is also used for
automatic XML serialization. This is probably what we want to override.
= Links and Entry Point =
The HATEAOS constraint basically means that all parts of the API should
be accessible by following links from a top-level hypertext document.
One implication is that we should provide links describing all
relationships and possible application state changes.
Another is that a GET on the base URL should return a document with
links to our various collections e.g.
$> GET /
<api>
<link rel="pools" href="/conductor/pools"/>
<link rel="deployments" href="/conductor/deployments"/>
...
</api>
= Authentication =
We currently support HTTP authentication just fine, but we should figure
out a way to avoid allocating cookies for such clients.
i.e. this seems strange:
> GET /conductor/pools HTTP/1.1
> Authorization: Basic YWRtaW46cGFzc3dvcmQ=
...
>
< HTTP/1.1 200 OK
...
< Set-Cookie: _rails23-app_session=8b0177c606d7de23437e5b13f5c8791f; path=/; HttpOnly
= Create/Update Media Types =
For Create and Update, we should support XML and JSON. It should be
possible to take a representation of an existing resource, modify it and
use that to update or create a resource.
Currently, all our Create/Update operations accept x-www-form-urlencoded
as method bodies. We could consider supporting this as part of the API
but, if we do, we need to consider parameters like "commit=Save" and how
to support things like "provider[type]=ec2". It's perhaps easiest to
just say that this media type is an unsupported interface only intended
to be used by Conductors HTML and Javascript.
= Resource Creation =
Our current behaviour is:
> POST /conductor/pools HTTP/1.1
> Authorization: Basic YWRtaW46cGFzc3dvcmQ=
...
> Content-Type: application/x-www-form-urlencoded
>
} [data not shown]
< HTTP/1.1 302 Found
< Location: http://localhost:3000/conductor/pools/3
...
i.e. we get redirected to the newly created pool.
This screws up curl which tries to follow the redirection and re-post.
We should return "HTTP 201 Created" and include a representation of the
created resource.
12 years, 10 months
#718968 - Provider field blank in deployments
by Matt Wagner
Honestly, I'm not too proud of this patch, but here it is.
In defense of this patch, the old code was flat-out wrong. A deployment doesn't necessarily have a realm, so getting provider through the realm was incorrect. This patch corrects it.
On the other hand, we now access this information through a really circuitious association, albeit the correct one. Worse, I can't seem to coerce Rails to do this with a join, so this replaces a single SQL query with three. Incidentally, if you rewrite this as a find and pass ":include => {:provider_account => :provider}" as an argument, Rails still won't actually do it as a join. Comments on the Rails docs for 2.x suggest that you need to a find_by_sql to force the join... I'd rather wait for 3.0 and see if this becomes less of a mess, personally, but I'll rewrite this as a multi-line SQL statement if asked.
-- Matt
12 years, 10 months
#717541 - Better validation error when stopping instances
by Matt Wagner
This is a quick little fix for 717541 -- if you clicked "Stop" on the instances view without selecting any instances, we'd end up doing something like Instances.find(nil) which caused an exception to be raised. With this patch, we guard against this and display an on-topic error message in this case.
-- Matt
12 years, 10 months
[PATCH] final change for Bug 714790
by Scott Seago
Most of Bug 714790 has been pushed, but the remaining issue was that non-admin users, when clicking the 'administer' tab would result in an 'insufficient privileges' error.
As it turns out, the default action for 'administer' was the 'users' list, which happened to be the only 'administer' action that doesn't allow non-admin users from viewing the page at all. In other cases, normal users should get viewing (but not editing) rights -- HWPs, Realms, and Suggested deployables. In some, (providers, etc) the action is valid but the results list is filtered by permissions.
This is only a temporary fix, since UX design for 'administer' hasn't been done yet. I'm changing the default action to view 'Suggested Deployables' instead of 'Users', as everyone can see that. In addition, the nav pulldown filters 'Users' section is now conditionally included based on the same permissions check that complains if non-admin users attempt to navigate there. I'm not enforcing permissions in nav anywhere else, since the pages themselves don't enforce them.
Signed-off-by: Scott Seago <sseago(a)redhat.com>
---
src/app/views/layouts/_admin_header.haml | 3 ++-
src/app/views/layouts/application.haml | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/app/views/layouts/_admin_header.haml b/src/app/views/layouts/_admin_header.haml
index ebae58c..37f6a90 100644
--- a/src/app/views/layouts/_admin_header.haml
+++ b/src/app/views/layouts/_admin_header.haml
@@ -9,7 +9,8 @@
%label.label.small{:for => 'admin-section-select'} Control Panels
%select#admin-section-select
%option{:value => users_path} Choose section...
- %option{:value => users_path} Users
+ - if check_privilege(Privilege::VIEW, User)
+ %option{:value => users_path} Users
%option{:value => roles_path} Roles
%option{:value => providers_path} Providers
%option{:value => provider_accounts_path} Provider Accounts
diff --git a/src/app/views/layouts/application.haml b/src/app/views/layouts/application.haml
index 443af7f..edeba61 100644
--- a/src/app/views/layouts/application.haml
+++ b/src/app/views/layouts/application.haml
@@ -37,7 +37,7 @@
%li{:class => "#{'active' unless [:design, :administer].include? controller.top_section}" }
= link_to 'Monitor', pools_path
%li{:class => "#{'active' if controller.top_section == :administer}" }
- = link_to 'Administer', users_path
+ = link_to 'Administer', suggested_deployables_path
#content
-# works with any 960 container (.container_16 or .container_24)
--
1.7.4.4
12 years, 10 months
[PATCH aeolus] Added template data in aeolus-image list output
by Martyn Taylor
From: Martyn Taylor <mtaylor(a)redhat.com>
---
.../image_factory/aeolus-image/lib/list_command.rb | 20 ++++++++++++++++++--
1 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/services/image_factory/aeolus-image/lib/list_command.rb b/services/image_factory/aeolus-image/lib/list_command.rb
index 5e5d62a..b16a2a6 100644
--- a/services/image_factory/aeolus-image/lib/list_command.rb
+++ b/services/image_factory/aeolus-image/lib/list_command.rb
@@ -6,8 +6,24 @@ module Aeolus
end
def images
- doc = Nokogiri::XML iwhd['/images'].get
- doc.xpath("/objects/object/key").collect { |node| "uuid: " + node.text }
+ images = [["UUID", "NAME", "OS", "OS VERSION", "ARCH", "DESCRIPTION"]]
+ doc = Nokogiri::XML iwhd['/target_images'].get
+ doc.xpath("/objects/object/key").each do |targetimage|
+ begin
+ build = iwhd["/target_images/" + targetimage + "/build"].get
+ image = iwhd["/builds/" + build + "/image"].get
+ template_xml = Nokogiri::XML iwhd["/templates/" + iwhd["/target_images/" + targetimage + "/template"].get].get
+ images << [image,
+ template_xml.xpath("/template/name").text,
+ template_xml.xpath("/template/os/name").text,
+ template_xml.xpath("/template/os/version").text,
+ template_xml.xpath("/template/os/arch").text,
+ template_xml.xpath("/template/description").text]
+ rescue RestClient::ResourceNotFound, RestClient::InternalServerError
+ end
+ end
+ format_print(images)
+ quit(0)
end
def builds
--
1.7.4
12 years, 10 months
[PATCH] Admin shall not be able to delete itself
by Jirka Tomasek
From: Jiri Tomasek <jtomasek(a)redhat.com>
This patch fixes BZ717533
---
src/app/controllers/users_controller.rb | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/app/controllers/users_controller.rb b/src/app/controllers/users_controller.rb
index 16c514d..efebe39 100644
--- a/src/app/controllers/users_controller.rb
+++ b/src/app/controllers/users_controller.rb
@@ -113,7 +113,12 @@ class UsersController < ApplicationController
def destroy
require_privilege(Privilege::MODIFY, User)
- User.destroy(params[:id])
+ user = User.find(params[:id])
+ if user == current_user
+ flash[:warning] = "Cannot delete #{user.login}: you are logged in as this user"
+ else
+ user.destroy
+ end
respond_to do |format|
format.html { redirect_to users_path }
--
1.7.4.2
12 years, 10 months
[PATCH aeolus] Updated Help Message for aeolus-image
by Martyn Taylor
From: Martyn Taylor <mtaylor(a)redhat.com>
---
.../aeolus-image/lib/config_parser.rb | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/services/image_factory/aeolus-image/lib/config_parser.rb b/services/image_factory/aeolus-image/lib/config_parser.rb
index cf4fa74..c62b599 100644
--- a/services/image_factory/aeolus-image/lib/config_parser.rb
+++ b/services/image_factory/aeolus-image/lib/config_parser.rb
@@ -125,11 +125,11 @@ module Aeolus
opts.separator ""
opts.separator "List Examples:"
opts.separator "aeolus-image list --images # list available images"
- opts.separator "aeolus-image list --builds $image_id # (NOT IMPLEMENTED) list the builds of an image"
- opts.separator "aeolus-image list --targetimages $build_id # (NOT IMPLEMENTED) list the target images from a build"
- opts.separator "aeolus-image list --targets # (NOT IMPLEMENTED) list the values available for the --target parameter"
- opts.separator "aeolus-image list --providers # (NOT IMPLEMENTED) list the values available for the --provider parameter"
- opts.separator "aeolus-image list --accounts # (NOT IMPLEMENTED) list the values available for the --account parameter"
+ opts.separator "aeolus-image list --builds $image_id # list the builds of an image"
+ opts.separator "aeolus-image list --targetimages $build_id # list the target images from a build"
+ opts.separator "aeolus-image list --targets # list the values available for the --target parameter"
+ opts.separator "aeolus-image list --providers # list the values available for the --provider parameter"
+ opts.separator "aeolus-image list --accounts # list the values available for the --account parameter"
opts.separator ""
opts.separator "Build examples:"
--
1.7.4
12 years, 10 months
[PATCH conductor 0/2]: Minor cleanups
by Chris Lalancette
Just a couple of minor cleanup patches to the conductor. These should have
no functional change, but make our packaging easier.
12 years, 10 months
