The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com
The RAL and Parser resource and Type classes should be refactored and merged
|Assignee:||eric sorenson||% Done:|
|Affected Puppet version:||2.7.19||Branch:|
Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com
This ticket may be automatically exported to the PUP project on JIRA using the button below:
Right now we have two resource type classes (Puppet::Type and Puppet::Resource::Type) and three resource classes (Puppet::Type, Puppet::Resource, and Puppet::Parser::Resource). The parser resource class is a subclass of Puppet::Resource, but it’s still another class. In addition, we have two ways of managing collections of types — Puppet::Resource::TypeCollection in the parser, and Puppet::Type in the RAL.
These classes should be reduced to one Type class and one Resource class, along with one mechanism for managing collections of Types.
The benefits of this simplification are many:
Sheer maintainability. The current RAL and parser resource stuff is largely calcified. It’s barely been changed in years, and nearly everyone finds it impossible to understand or maintain.
Reduced duplication. There is a large amount of painful duplication or near duplication in the classes – they all define mechanisms for defining parameters, validating them, etc.
Cleaner code. The Puppet::Type class is a mess. It is a class management system (for managing subclasses of Puppet::Type), a class of classes (for defining behavior of subclasses), and a class itself (for defining behavior of resources). These three behaviors should be split into three classes, like is done in the parser. The classes of Puppet::Resource, Puppet::Resource::Type, and Puppet::Resource::TypeCollection are dramatically easier to understand and maintain than Puppet::Type.
I have taken multiple cuts at this (e.g., https://github.com/lak/puppet/tree/prototype/2.7.x/ral_refactor and https://github.com/lak/puppet/tree/prototype/master/ral_refactor_after_param_refactor), but it’s a bunch of work, and there are some fundamental problems in the tests that prevented me from getting it done.
My recommended route, as you’ll see in my code, is that we split Puppet::Type into the three component classes (I chose OldResource, OldType, and OldTypeCollection, I think, for the class names), then merge the respective classes.
Note that one of the major benefits of this refactor is that resource types move from being classes, which are global (and thus cannot be environment-specific), to being instances, which means they work fine in different environments. Providers would still be classes and thus cross-environment, but that could be fixed, too.