Improving the Resources Resource Type

[Henrik Lindberg]

Hi,
I am bringing up this topic because of a recent discussion and PR for
the ticket https://tickets.puppetlabs.com/browse/PUP-1486

Here is a recap.

The Resources resource is used to "manage unmanaged" resources of a
particular given type - e.g. to purge users, groups etc. that are not
otherwise managed.

The issues are that "managing the unmanaged" may require additional
attributes that are specific to the type that is referenced. The base
type only has the attributes "unless_system_user" and "unless_uid" (se
https://docs.puppetlabs.com/references/latest/type.html#resources for
the documentation).

At times it is suggested that the Resources type should have additional
attributes that are specific only to one type. This solution has the
obvious problem that over time, the Resource type would then have a very
large number of attributes for different types and the temptation will
be great to borrow attributes and apply similar semantics. Over time
this solution will become unmanageable.

Another solution was proposed in PR-3269
(https://github.com/puppetlabs/puppet/pull/3269) which we closed for
various reasons.

The approach in the PR was to add a "newresourcesparam" to Puppet::Type
allowing this to define additional parameters to the Resources type.
An obvious problem with this approach is the need to load all types in
order to check the validity of attributes of a Resources instance.

Instead, I would like to see something where the concerns are separated
(to avoid the problem of "the base class knows about all the things").
One good pattern is "the strategy pattern" / "adapter pattern" which is
used internally in the future parser. (You can read about this here:
http://puppet-on-the-edge.blogspot.se/2014/02/puppet-internals-adapter-pattern.html).
TL;DR - an adapter is responsible for "sticking a label" on someone else
without that someone having to be concerned about its existence,
protocol etc.

How can we achieve something similar in a simple way in Puppet?

One very simple approach is to add the basic ability to handle such
"adapters", or "extra data" if you like, in the Resources type. We
cannot use the actual adapter pattern since when it comes to resources,
they are serialized and processed in ways that would drop such adapters
(since those are really a runtime concept).

Proposal:

* Add the attribute options (with the type Array[Resource])
* Add the ability in a resource type to specify a list of type names
that it allows instances of as options (typically one)
* Add the ability in type to get the options.

Here is a simulation of the above using just user defined types - a type
"base" specifies that it takes an "option" that must be a reference to a
"Base_option":

define base_options($p1, $p2) {
}

define base(Resource[base_option] $option = undef) {
}

base { 'the_base':
extraref => Base_options[the_options]
}

base_options { the_options:
p1 => 'extra 1',
p2 => 'extra 2'
}

This way, everything ends up in the catalog. The same using Resources
would look like this:

resources { base:
options => [Base_options[the_options]]
}

How the rest of the protocol between Resources and the types that
support options should work is left to be designed, but basically; the
Resources implementation should present the options to the type either
for just asking if a particular resource should be purged or not (as it
does now but done as a delegation to the particular type).

I think this design is:

* type safe - it is possible to statically check the combination of the
managed type, the options it supports, and the type of the entries in
options.
* The optional type can be a user defined type - all that is needed are
the parameter values specified by the user.
* A user defined type can be name-spaced

Alternative - using the type system in Puppet 4.0

Another alternative is to simply use the new type system in Puppet 4.0.
A type that supports options to Resources returns a Struct type that
describes the options. (A Struct is a detailed type specification of a
hash). (Read more about the Struct type here:
http://puppet-on-the-edge.blogspot.se/2014/02/adding-struct-and-tuple-to-puppet-type.html)

In the catalog a Struct type is simply encoded as a String, and it is
easily converted back to an instance of the type which can be used for
type checking the options received from the Resources type.

Thoughts?
- henrik

--

Visit my Blog "Puppet on the Edge"
http://puppet-on-the-edge.blogspot.se/

[Raphaël Pinson]

Hi,

On Thursday, November 6, 2014 3:37:08 AM UTC+1, henrik lindberg wrote:

I am bringing up this topic because of a recent discussion and PR for
the ticket https://tickets.puppetlabs.com/browse/PUP-1486

Thanks for bringing it up.

 

Here is a recap.

The Resources resource is used to "manage unmanaged" resources of a
particular given type - e.g. to purge users, groups etc. that are not
otherwise managed.

The issues are that "managing the unmanaged" may require additional
attributes that are specific to the type that is referenced. The base
type only has the attributes "unless_system_user" and "unless_uid" (se
https://docs.puppetlabs.com/references/latest/type.html#resources for
the documentation).

At times it is suggested that the Resources type should have additional
attributes that are specific only to one type. This solution has the
obvious problem that over time, the Resource type would then have a very
large number of attributes for different types and the temptation will
be great to borrow attributes and apply similar semantics. Over time
this solution will become unmanageable.

Another solution was proposed in PR-3269
(https://github.com/puppetlabs/puppet/pull/3269) which we closed for
various reasons.

The approach in the PR was to add a "newresourcesparam" to Puppet::Type
allowing this to define additional parameters to the Resources type.
An obvious problem with this approach is the need to load all types in
order to check the validity of attributes of a Resources instance.

The main idea there was to avoid breaking compatibility, although I am fully aware that forcing the loading of all types is probably not the best option. 

I did consider the possibility of passing an `options` attribute, but didn't go this way for backwards-compatibility reasons.

If we go the options attribute way, I'd like to suggest a simple alternative solution: using providers for each type to be managed. There would be different providers for the resources type, managing the details of how to purge various resource types. The options attribute that would be passed could then be processed by the provider for the chosen type (e.g. the user provider when dealing with the user type).

[Luke Kanies]

I think this design is sufficiently complex that nearly no one would be able to use it.

 

Alternative - using the type system in Puppet 4.0

Another alternative is to simply use the new type system in Puppet 4.0.
A type that supports options to Resources returns a Struct type that describes the options. (A Struct is a detailed type specification of a hash). (Read more about the Struct type here: http://puppet-on-the-edge.blogspot.se/2014/02/adding-struct-and-tuple-to-puppet-type.html)

In the catalog a Struct type is simply encoded as a String, and it is easily converted back to an instance of the type which can be used for type checking the options received from the Resources type.

Of the two, I prefer this approach.

However, I think it's worth looking at a third approach:

Right now, the 'resources' type isn't actually special.  It uses hacks to do its work, rather than being part of the system and treated specially.

I think we should build it, or at least its functionality, more into the core, so that we don't have to use those hacks, and we don't have to ask the users to do things like the above.

For instance, you could modify the system to allow attributes to Resources that match the specified type.  This would break the current parameter checking, but it wouldn't be too hard to do this, and it could make a lot of sense.  With this, you'd want to be able to extend types to give affordances to the Resources type, so you could control this from there.

E.g., rather than teaching Resources about special kinds of users, you teach User about what a system_user is, and then you give Resources the ability to select against that status.

A fourth option would be to combine the best of Resources and exported resources:  Build a querying system akin to exported resources, but against the current system, rather than against the catalog.  Or really, take the exact same querying we already have, but provide the ability to control where it's searching against, and what to do with the results.

This is my preferred solution.  It makes querying more powerful, and separates the concept of querying from the source of the data.

I really do think this kind of functionality should be supported at the framework level, and more hacks like Resources aren't the best long term solution.

--

http://puppetlabs.com/ | http://about.me/lak | @puppetmasterd

[Felix Frank]

On 11/06/2014 12:04 PM, Luke Kanies wrote:
> A fourth option would be to combine the best of Resources and exported
> resources: Build a querying system akin to exported resources, but
> against the current system, rather than against the catalog. Or really,
> take the exact same querying we already have, but provide the ability to
> control where it's searching against, and what to do with the results.
>
> This is my preferred solution. It makes querying more powerful, and
> separates the concept of querying from the source of the data.

This is a very attractive idea.

Currently the purging through resources is likely to appear entirely too
mystical to most users.

To get a behavior that matches today's resources type, we'd need to
explicitly query unmanaged resources.

User<~| managed == false |~> { ensure => absent }

This has the advantage of being more explicit about the unmanaged part than

resources { 'user': purge => true }

With that said, if we end up staying on the resources type track, I'd
lean towards Raphael's suggestion from his reply: Add providers to for
the resources type. Take the magic from the type, add a slew of
providers. This would move this problem into the space of, say, things
like package installation options, for which there are patterns in place.

I think I would prefer that over making resources a core concept.

Cheers,
Felix

[Henrik Lindberg]

On 2014-06-11 12:04, Luke Kanies wrote:
> A fourth option would be to combine the best of Resources and exported
> resources: Build a querying system akin to exported resources, but
> against the current system, rather than against the catalog. Or really,
> take the exact same querying we already have, but provide the ability to
> control where it's searching against, and what to do with the results.
>
> This is my preferred solution. It makes querying more powerful, and
> separates the concept of querying from the source of the data.
>

I like this a lot. There are some design challenges though as this
pushes the querying to the agent, and that this is probably best
supported at the language level rather than being expressed as a
function. (This because variables etc. in a query needs to be
evaluated/resolved in scope when the catalog is produced, just like they
are today in virtual/exported collection).

> I really do think this kind of functionality should be supported at the
> framework level, and more hacks like Resources aren't the best long term
> solution.
>

Agree.

[Erik Dalén]

On Thu Nov 06 2014 at 6:33:55 PM Henrik Lindberg <henrik....@cloudsmith.com> wrote:

On 2014-06-11 12:04, Luke Kanies wrote:
> A fourth option would be to combine the best of Resources and exported
> resources:  Build a querying system akin to exported resources, but
> against the current system, rather than against the catalog.  Or really,
> take the exact same querying we already have, but provide the ability to
> control where it's searching against, and what to do with the results.
>
> This is my preferred solution.  It makes querying more powerful, and
> separates the concept of querying from the source of the data.
>

I like this a lot. There are some design challenges though as this
pushes the querying to the agent, and that this is probably best
supported at the language level rather than being expressed as a
function. (This because variables etc. in a query needs to be
evaluated/resolved in scope when the catalog is produced, just like they
are today in virtual/exported collection).

I'm in favour of a more generic query solution here instead.

 

> I really do think this kind of functionality should be supported at the
> framework level, and more hacks like Resources aren't the best long term
> solution.
>
Agree.

I guess this could be built into the resources type and be made backwards compatible, but then the query would either be a string with lots of variable interpolation, or it would be an array and look something like the puppetdb query API. Neither seem very appealing, so building it into the language itself would probably be best, like the other collectors.

[Joshua Hoblitt]

On 11/06/2014 04:04 AM, Luke Kanies wrote:
> A fourth option would be to combine the best of Resources and exported
> resources: Build a querying system akin to exported resources, but
> against the current system, rather than against the catalog. Or
> really, take the exact same querying we already have, but provide the
> ability to control where it's searching against, and what to do with
> the results.

How would make use of that information on the agent if expressions are
evaluated on the master? As an example, I'd like to be able to 'query'
the home dir of a role account (without having to install a fact) and
interpolate it into a string to form a path. I've always found this
scenario a bit frustrating as the type has the data I want; it's just
inaccessible while the catalog is built.

-Josh

--

[Luke Kanies]

As others have mentioned, the only way this is possible is if we find a way to send the query down to the client. We need different queries for different locations, but hopefully similar enough that it doesn’t like a completely different system, like it is today.

[Luke Kanies]

On Nov 6, 2014, at 10:38 PM, Felix Frank <felix...@alumni.tu-berlin.de> wrote:

> On 11/06/2014 12:04 PM, Luke Kanies wrote:
>> A fourth option would be to combine the best of Resources and exported
>> resources: Build a querying system akin to exported resources, but
>> against the current system, rather than against the catalog. Or really,
>> take the exact same querying we already have, but provide the ability to
>> control where it's searching against, and what to do with the results.
>>
>> This is my preferred solution. It makes querying more powerful, and
>> separates the concept of querying from the source of the data.
>
> This is a very attractive idea.
>
> Currently the purging through resources is likely to appear entirely too
> mystical to most users.
>
> To get a behavior that matches today's resources type, we'd need to
> explicitly query unmanaged resources.
>
> User<~| managed == false |~> { ensure => absent }
>
> This has the advantage of being more explicit about the unmanaged part than
>
> resources { 'user': purge => true }

Agreed.

We do need some way to make it clear that this query happens on the client, rather than on the server, and then the catalog needs some way to store that query.

The only other way to do it would be to have the server be able to do real-time queries against the client, but I think that would have too many crazy consequences.

It’s been a long time, but I seem to remember trying to treat the resource as a query; something like:

user { *:
uid >= 100,
purge => true
}

But I think this is not the right answer.

Maybe something like:

user { <| uid > 100 |>:
purge => true
}

Then the title is the query, and the parameters get applied to all resources that match that title.

Thoughts?

> With that said, if we end up staying on the resources type track, I'd
> lean towards Raphael's suggestion from his reply: Add providers to for
> the resources type. Take the magic from the type, add a slew of
> providers. This would move this problem into the space of, say, things
> like package installation options, for which there are patterns in place.
>
> I think I would prefer that over making resources a core concept.

[Raphaël Pinson]

On Thursday, November 6, 2014 12:39:22 PM UTC+1, Felix Frank wrote:

On 11/06/2014 12:04 PM, Luke Kanies wrote:
> A fourth option would be to combine the best of Resources and exported
> resources:  Build a querying system akin to exported resources, but
> against the current system, rather than against the catalog.  Or really,
> take the exact same querying we already have, but provide the ability to
> control where it's searching against, and what to do with the results.
>
> This is my preferred solution.  It makes querying more powerful, and
> separates the concept of querying from the source of the data.

This is a very attractive idea.

Currently the purging through resources is likely to appear entirely too
mystical to most users.

To get a behavior that matches today's resources type, we'd need to
explicitly query unmanaged resources.

User<~| managed == false |~> { ensure => absent }

One thing I really like about this option is the possibility to set these resources to something else than "absent". For example, you might want to ensure that unmanaged packages are purged or that unmanaged services are stopped.

[Felix Frank]

On 11/07/2014 05:59 AM, Luke Kanies wrote:
>> To get a behavior that matches today's resources type, we'd need to
>> > explicitly query unmanaged resources.
>> >
>> > User<~| managed == false |~> { ensure => absent }
>> >
>> > This has the advantage of being more explicit about the unmanaged part than
>> >
>> > resources { 'user': purge => true }
> Agreed.
>
> We do need some way to make it clear that this query happens on the client, rather than on the server, and then the catalog needs some way to store that query.
>
> The only other way to do it would be to have the server be able to do real-time queries against the client, but I think that would have too many crazy consequences.
>
> It’s been a long time, but I seem to remember trying to treat the resource as a query; something like:
>
> user { *:
> uid >= 100,
> purge => true
> }
>
> But I think this is not the right answer.
>
> Maybe something like:
>
> user { <| uid > 100 |>:
> purge => true
> }
>
> Then the title is the query, and the parameters get applied to all resources that match that title.
>
> Thoughts?
>

I think I would prefer a mix of both approaches:

user { 'arbitrary-purger-title':
match => <| uid > 100 && !managed |>,
ignore => <| title == sysmaint |>,
ensure => absent,
}

This would have the benefit that the user can

exec { 'tidy-all-home-dirs: require => User['arbitrary-purger-title'] }

at the expense of more metaparameters, putting restraints on type
authors wrt. attribute names that they can use for their own types.
Hell, we have at least one core type that uses "ignore" even today.

So perhaps a more dedicated syntax like

user { 'arbitrary-purger-title':
<| uid > 100 && !managed |>,
...
}

has more merit.

On 11/07/2014 05:39 AM, Luke Kanies wrote:> On Nov 7, 2014, at 2:02 PM,

There be dragons.

System entities are queried at the start of the agent transaction at the
moment. The transaction starts after the catalog has been retrieved. We
would have to intertwine these steps, and this would raise a plethora of
problems, I imagine.

For example - you cannot be sure what provider the catalog will end up
assigning to the resource that you are querying before everything is
properly resolved. It would be awkward if a query is performed using the
default provider, but the catalog ends up specifying that the user in
question be managed through LDAP, for example.

The compiler calling back to the agent during its catalog request would
be a major game changer. I'm thinking "whole new paradigm".

Cheers,
Felix

[John Bollinger]

On Wednesday, November 5, 2014 8:37:08 PM UTC-6, henrik lindberg wrote:

Hi,
I am bringing up this topic because of a recent discussion and PR for
the ticket https://tickets.puppetlabs.com/browse/PUP-1486

Here is a recap.

The Resources resource is used to "manage unmanaged" resources of a
particular given type - e.g. to purge users, groups etc. that are not
otherwise managed.

Purging unmanaged resources is what Resources can do now, but I've always thought the original idea was much grander.  The type's own documentation is couched partially in terms of generating resource instances, which  fits with "managing [otherwise] unmanaged resources", but it also described as "managing other resource types", which sounds like something different.  I suspect that some of the features that might otherwise have been implemented via the Resources type went other places instead (especially into collectors).
 

The issues are that "managing the unmanaged" may require additional
attributes that are specific to the type that is referenced. The base
type only has the attributes "unless_system_user" and "unless_uid" (se
https://docs.puppetlabs.com/references/latest/type.html#resources for
the documentation).

Yes, I think this essential problem stunted the development of the Resources type.

Instead, I would like to see something where the concerns are separated
(to avoid the problem of "the base class knows about all the things").
One good pattern is "the strategy pattern" / "adapter pattern" which is
used internally in the future parser.

Most everyone here should be deeply familiar with this pattern, though they may not know it by name.  Puppet has relied on it forever in the form of resource providers.

I'm not sure it quite fits the situation, though, because fundamental to the Strategy pattern is a common view of the inputs directing the work to be performed.  For strategies to be applicable to a very general problem, such as "managing unmanaged resources", the form of the inputs must be very general.  If you went down this path you would soon have to consider what you are gaining by using the Resources type as a front end.

 

One very simple approach is to add the basic ability to handle such
"adapters", or "extra data" if you like, in the Resources type. We
cannot use the actual adapter pattern since when it comes to resources,
they are serialized and processed in ways that would drop such adapters
(since those are really a runtime concept).

I'm not buying that.  The adapter pattern works fine for resources in general, including specifying non-default strategies (providers) for particular resources.  This is a problem that could be solved for Resources --  for instance, by using the existing provider system, as Felix suggested.

 

Proposal:

* Add the attribute options (with the type Array[Resource])
* Add the ability in a resource type to specify a list of type names
that it allows instances of as options (typically one)
* Add the ability in type to get the options.

So the idea here is what?  Resource instances for unmanaged physical resources of the specified type(s) are created, with the specified parameters, to manage those physical resources?

Or are the options supposed to effectively be resource-type-specific parameters to some sort of "things_Resources_can_do_with_this_type()" function that all resources will provide?  If the latter, then what are the implications for providers?

 

How the rest of the protocol between Resources and the types that
support options should work is left to be designed, but basically; the
Resources implementation should present the options to the type either
for just asking if a particular resource should be purged or not (as it
does now but done as a delegation to the particular type).

Are you supposing that this would be limited to purging unmanaged resources, or is that just an example?  I know there are issues with purging resources of types that don't prefetch or that aren't Ensurable, but if that's all you're going after then this sounds a bit out of proportion.
 
 

Alternative - using the type system in Puppet 4.0

Another alternative is to simply use the new type system in Puppet 4.0.
A type that supports options to Resources returns a Struct type that
describes the options. (A Struct is a detailed type specification of a
hash). (Read more about the Struct type here:
http://puppet-on-the-edge.blogspot.se/2014/02/adding-struct-and-tuple-to-puppet-type.html)

In the catalog a Struct type is simply encoded as a String, and it is
easily converted back to an instance of the type which can be used for
type checking the options received from the Resources type.

I know that the type system is a shiny new toy, and it has lots of promise, but it seems a lot lower level than the Resources metatype.  I am not enthusiastic about putting low-level hooks into the type system intended narrowly for the Resources type to manipulate.  I could, however, get behind something more generic, such as a mechanism in the type system by which information about the parameters supported by a resource type can be exposed.

Having said all that, I can completely get behind Luke's idea to provide for client-side queries.  I am confused, however, as to why the ideas for that seem to be running in directions such as overloading resource titles and introducing new collector-like forms, when the entire purpose of the Resources type is to serve this kind of function.  Why not just add a parameter to Resources to specify the a query predicate (the default being something like "!managed", and another to specify a hash of resource properties to set, e.g. { ensure => absent }.  Deprecate the existing 'purge' and 'unless_system_user' parameters.  This would look similar to some of the forms being proposed, but it would rely mostly on existing facilities for everything other than actually performing queries.  Example:

resources { 'remove-unmanaged-users':
  name  => 'user',

  match => "uid > 100 && !managed",

  set   => { ensure => 'absent' }
}

I admit that I'm a bit leery of the "!managed" bit of the predicate, as its presence suggests that maybe it can be omitted to allow whatever is implemented for this to modify the properties of managed resources, too.  This could be especially nasty if it were possible to stack these things so that more than one applies to the same resource.  I urge that that possibility be foreclosed, at least in the initial implementation.  It can be added later if there turns out to be sufficient justification, but it cannot easily be taken away once granted.


John

[Erik Dalén]

I think this is sensible, likely the parser for this query should be implemented in the munging method of the type though, so it can happen server side and turn it into some sort of AST like format (which is what I meant with something looking a bit like the PuppetDB query API).

Possibly it should allow users to submit that AST format directly, so then you could have other parser functions to implement different syntaxes for the query language.

as an example:

instead of match => "title='foo' and version='2.1.2'"

allow something like ["and", ["=", "title", "foo"], ["=", "version", "2.1.2"]]

And then you could have a function that could return something like that instead of having to turn it into a string that then gets parsed again.

I also agree about the managed attribute, this should really be for unmanaged resources.

[Henrik Lindberg]

On 2014-07-11 17:29, John Bollinger wrote:
>
> Having said all that, I can completely get behind Luke's idea to provide
> for client-side queries.

I am +1 on that too. Much better idea.
You can forget my earlier proposals.

[Henrik Lindberg]

On 2014-07-11 18:28, Erik Dalén wrote:
> On Fri Nov 07 2014 at 5:29:15 PM John Bollinger

> <john.bo...@stjude.org <mailto:john.bo...@stjude.org>> wrote:
>
> Having said all that, I can completely get behind Luke's idea to
> provide for client-side queries. I am confused, however, as to why
> the ideas for that seem to be running in directions such as
> overloading resource titles and introducing new collector-like
> forms, when the entire purpose of the Resources type is to serve
> this kind of function. Why not just add a parameter to Resources to
> specify the a query predicate (the default being something like
> "!managed", and another to specify a hash of resource properties to
> set, e.g. { ensure => absent }. Deprecate the existing 'purge' and
> 'unless_system_user' parameters. This would look similar to some of
> the forms being proposed, but it would rely mostly on existing
> facilities for everything other than actually performing queries.
> Example:
>
> resources {'remove-unmanaged-users':
> name =>'user',
> match =>"uid > 100 && !managed",
>

> set=>{ensure=>'absent'}

> }
>
> I admit that I'm a bit leery of the "!managed" bit of the predicate,
> as its presence suggests that maybe it can be omitted to allow
> whatever is implemented for this to modify the properties of

> /managed/ resources, too. This could be especially nasty if it were

> possible to stack these things so that more than one applies to the

> same resource. *I urge that that possibility be foreclosed*, at

> least in the initial implementation. It can be added later if there
> turns out to be sufficient justification, but it cannot easily be
> taken away once granted.
>
>
> I think this is sensible, likely the parser for this query should be
> implemented in the munging method of the type though, so it can happen
> server side and turn it into some sort of AST like format (which is what
> I meant with something looking a bit like the PuppetDB query API).
>
> Possibly it should allow users to submit that AST format directly, so
> then you could have other parser functions to implement different
> syntaxes for the query language.
>
> as an example:
> instead of match => "title='foo' and version='2.1.2'"
> allow something like ["and", ["=", "title", "foo"], ["=", "version",
> "2.1.2"]]
> And then you could have a function that could return something like that
> instead of having to turn it into a string that then gets parsed again.
>

That is what a query would produce as serialization if allowed as a
value expression. e.g.

match => <| title == foo and version = '2.1.2' |>

the big benefit is that the parser can validate the query, all querying
is expressed the same way, etc. Just as if this was done with a string,
any interpolation is done server side.

match => <| title == foo and version = $ver |>

i.e. $ver is evaluated and the result is stored in the resulting "baked"
query that is sent to the agent.

As part of getting rid of the old 3x AST, we just finished the rewrite
of the Collector. It is now much easier to do the collector expression
encoding transformation to "array form" or "ruby proc predicate" (used
internally for catalog collection and exported collection), and we can
now easily add an "array form" to "ruby proc predicate" transformation
that makes it really easy to use as a filter in a type.