The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Feature #14137

Usability improvements to resource_type REST API

Added by Chris Price over 2 years ago. Updated about 2 years ago.

Status:ClosedStart date:04/23/2012
Priority:NormalDue date:
Assignee:Andrew Parker% Done:

0%

Category:service
Target version:3.0.0
Affected Puppet version: Branch:https://github.com/puppetlabs/puppet/pull/703
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


Description

In order to improve the ability for an external tool to support parameterized classes and other functionality, we need to make a few improvements to the REST API for the resource_type service.

The main shortcomings with the current version are as follows:

  1. The “search” mode does not support filtering by type; every query returns instances of all three types (classes, defined types, and nodes).
  2. The field names on the output objects are confusing. They use terminology / names like “hostclass” which are, for practical purposes, internal / implementation-specific terminology. The output should use names that are more consistent with our documentation / DSL / external exposure of these constructs (e.g. “class” instead of “hostclass”).

Proposed changes:

To address concern # 1 above, the proposal is to add support for a query parameter to allow filtering by type. So, e.g., you could issue a query like this:

    curl -k -H "Accept: pson" "https://localhost:8140/production/resource_types/*?type=class"

The new bit is the part in bold: ?type=class. This would also support the values “defined_type” and “node”. The behavior would be to simply filter the result set to the type that is specified by this query parameter.

To address concern # 2 above, we will modify the code that transforms the objects to PSON before serializing them. This will involve renaming fields/values that are not named clearly (e.g. “hostclass”=>“class” as mentioned above), and may also include eliminating some unnecessary fields from the output. I will add more comments to this ticket as I determine what changes seem appropriate.


Related issues

Related to Puppet - Feature #11608: Lowrider: Data/Model separation by including Hiera-like f... Closed 12/28/2011 01/09/2012
Duplicated by Puppet - Feature #7073: resource_type search should allow searching on type Duplicate 04/12/2011

History

#1 Updated by Chris Price over 2 years ago

  • Description updated (diff)

#2 Updated by Chris Price over 2 years ago

  • Description updated (diff)

#3 Updated by Chris Price over 2 years ago

  • Description updated (diff)

#4 Updated by Chris Price over 2 years ago

  • Description updated (diff)

#5 Updated by Chris Price over 2 years ago

  • Description updated (diff)

#6 Updated by Anonymous over 2 years ago

curl -k -H “Accept: pson” “https://localhost:8140/production/resource_types/*?type=class”

Is the star intentional? It reads like, “Get me all resource types, then filter by type,” rather than, “Get me all resources or class type.” That may be out of scope for what you’re asking.

Otherwise this looks sound to me.

#7 Updated by Luke Kanies over 2 years ago

What about returning native types? I think ‘class’, ‘node’, ‘definition’, and ‘native’ would be a good set, but there are definitely four classes, not three (and I thought resource_type showed them all now). It also seems like ‘definition’ is better than ‘defined_type’ – another implementation detail being exposed.

#8 Updated by Chris Price over 2 years ago

I’m glad you commented, Randall.

The word “type” is extremely overloaded here, and it bugs me… but I’m not sure what to do about it.

As I understand it, the “resource_type” service is intended to return, basically, definitions of custom “types” that users have made available in their environment. So, “https://blah/resource_types/*” basically means “give me back the definitions of all custom types”.

In practice there are currently three different categories that these definitions can fall into: “classes”, “defined types”, and “nodes”. These aren’t exactly “parent types” but that’s about as close as I’ve come to inventing terminology to disambiguate the use of the word “type” here from the use of the phrase “resource type”.

To give a practical example:

The puppet apache module includes a defined type called “apache::vhost”. In the current version of this API, this is considered an instance of “resource_type”, which has a property called “type” on it, whose value is (roughly) “defined type”.

There might also be a parameterized class defined by the module; it might be called “apache::foo”, which would be another instance of “resource_type”. It’s “type” property would have the value “class”.

A query with “?type=class” would return “apache::foo” (and all other classes), but not “apache::vhost”. A query with “?type=defined_type” would return “apache::vhost” and all other defined types, but not “apache::foo”.

To keep the scope of this problem manageable for the Telly timeframe, I don’t think I can change the use of the phrase “resource_type” in the URL. I can, however, very easily change the name of the parameter that we use in the query string (“?type=blah”). I would welcome suggestions on that.

#9 Updated by Chris Price over 2 years ago

Luke Kanies wrote:

What about returning native types? I think ‘class’, ‘node’, ‘definition’, and ‘native’ would be a good set, but there are definitely four classes, not three (and I thought resource_type showed them all now).

I don’t think it returns native types at the moment; at least, I haven’t seen it do so in my limited experimentation with it so far, and haven’t seen any code that would cause it to do so.

It also seems like ‘definition’ is better than ‘defined_type’ – another implementation detail being exposed.

My impression was that “definition” is our internal lingo, and that “defined type” is what we usually say in our documentation. If I have that wrong I’m more than happy to switch it.

#10 Updated by Luke Kanies over 2 years ago

Ah – I thought I’d added native type support, but I guess not. Weird, because I can swear I can remember writing the to_pson method in Puppet::Type. :/

I’m fine with any terms that the docs guys like. :)

#11 Updated by Chris Price over 2 years ago

Added Nick F. in case he has an opinion.

#12 Updated by Nick Fagerlund over 2 years ago

Hello!

  • “Defined type” is better than “definition,” and is more or less our official term for them. I decided this by fiat without doing any focus group or anything, but I still think it was definitely the right call, and can make my case for it if someone claims that listening to it won’t bore them to actual tears. (Short version: we can define three kinds of thing, so why is only one of them called a “definition?” Also, they act just like resource types, so the name should reflect that.)
  • You might want to consider *?kind=class instead of type=class.

#13 Updated by Chris Price over 2 years ago

ooh, I like “kind”. I’ll change it to that for now. Very easy to change again if anyone else has any strong feelings in another direction. Thanks Nick!

#14 Updated by Chris Price over 2 years ago

One thought, though… the output from a REST call currently looks something like this:

{
  "line": 1,
  "file": "/home/cprice/work/puppet/test/master/conf/modules/resource_type_foo/manifests/my_parameterized_class.pp",
  "arguments": {
"param1": null,
"param2": "\"default2\""
  },
  "name": "resource_type_foo::my_parameterized_class",
  "type": "hostclass"
},
{
  "line": 1,
  "file": "/home/cprice/work/puppet/test/master/conf/modules/resource_type_foo/manifests/my_defined_type.pp",
  "arguments": {
"param1": null,
"param2": "\"default2\""
  },
  "name": "resource_type_foo::my_defined_type",
  "type": "definition"
},    

If I change “type” on the query string to “kind”, it seems like I should change it to “kind” on the output as well. That’s also an easy change, just wanted to mention it to make sure no one has any objections.

#15 Updated by Luke Kanies over 2 years ago

I love the ‘kind=class’.

#16 Updated by Chris Price over 2 years ago

Anyone have any opinions on “arguments” vs. “parameters” in the output? It seems like in our docs we use the term “parameters”, so I think I’d be inclined to expose them that way… although that will be a little confusing in the code since we already have another meaning for that term. But, all things being equal, I’d rather have the code be confusing and the external API clear than the other way around.

#17 Updated by Nick Fagerlund over 2 years ago

We don’t use “arguments” anywhere to describe those things — they’re always either parameters or attributes.

Docs team has been trying to use “attributes” when we’re talking about the interface in the DSL for declaring a resource, and “parameters” when talking about the implementation (for a native type) or definition (for a class or defined type). That is: when you define a type you give it a set of parameters; these parameters become the attributes you have to specify when you declare a resource of that type.

But I don’t think it’s possible to completely keep the terms from slopping into each other. In this context I’d be fine with either one, and would prefer either of them to “arguments.”

#18 Updated by Chris Price over 2 years ago

This context is all about exposing the “definition” of these resource types to external users. Therefore, “parameters” seems most consistent with your comments above. It also seems most consistent with the term “parameterized classes”, and making those easier to work with is one of the main goals of these changes. So… I’ll got with “parameters” unless anyone suggests otherwise.

#19 Updated by Chris Price over 2 years ago

About to submit a pull request for this. Nothing that can’t be changed easily, but we do have a code freeze for Telly on Friday. The changes basically boiled down to:

  • add support a “?kind=blah” query parameter for filtering
  • add support for a “pp” query parameter that will pretty-print the pson… useful for testing/debugging
  • s/hostclass/class/g in serialization and deserialization
  • s/definition/defined_type/g in serialization and deserialization
  • s/arguments/parameters/g in serialization and deserialization

Happy to revisit if anyone has any other input over the next few days.

#20 Updated by Luke Kanies over 2 years ago

When you make the change, make sure you mention that it breaks backward compability. I don’t know if anyone is using the API, and I don’t think there’s much out there, but there’s likely something.

#21 Updated by Chris Price over 2 years ago

Yep, updating the “breaking changes” section in the Telly pre-docs now.

#22 Updated by Chris Price over 2 years ago

  • Status changed from Accepted to In Topic Branch Pending Review
  • Branch set to https://github.com/puppetlabs/puppet/pull/703

#23 Updated by Chris Price over 2 years ago

pre-docs commit is here:

https://github.com/puppetlabs/pre-docs/commit/a4128a5bbd5c8a8856dec924ccc45ec652e2a5b3

#24 Updated by Anonymous over 2 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release

#25 Updated by Anonymous over 2 years ago

  • Target version changed from 3.x to 3.0.0

#26 Updated by Nigel Kersten over 2 years ago

We’ve added functionality to the REST API here that isn’t available in the Face now.

There doesn’t appear to be any way to use the face to filter by class/node/type.

puppet resource_type find foo

I tried seeing if --extra let me do it, but it doesn’t seem to work.

#27 Updated by Chris Price over 2 years ago

  • Assignee changed from Chris Price to Andrew Parker

#28 Updated by Chris Price over 2 years ago

Andy, I assigned this to you so that you can prioritize with the rest of the stuff on your schedule. I’m more than happy to help with this if needed, just let me know!

#29 Updated by Nigel Kersten over 2 years ago

  • Priority changed from High to Normal

#30 Updated by Matthaus Owens about 2 years ago

  • Status changed from Merged - Pending Release to Closed

This is still merged pending release. What is left to do? (merged at https://github.com/puppetlabs/puppet/commit/0a958a7be2b15c1b3917a616e17dc605cdb1e4e2). Released in Puppet 3.0.0rc1. I’m closing it, but feel free to reopen, or open a related ticket.

Also available in: Atom PDF