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

This issue tracker is now in read-only archive mode and automatic ticket export has been disabled. Redmine users will need to create a new JIRA account to file tickets using https://tickets.puppetlabs.com. See the following page for information on filing tickets with JIRA:

Feature #16856

puppet should support data in modules

Added by R.I. Pienaar over 3 years ago. Updated over 2 years ago.

Status:Re-openedStart date:10/08/2012
Priority:NormalDue date:
Assignee:eric sorenson% Done:

0%

Category:-
Target version:-
Affected Puppet version: Branch:https://github.com/puppetlabs/puppet/pull/1217
Keywords:

We've Moved!

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

This ticket is now tracked at: https://tickets.puppetlabs.com/browse/PUP-1157


Description

At present there is a way to store data in modules using the puppet backend for hiera but it is optional and kind of broken. The site hierarchy impacts how the puppet backend behaves which makes it impossible for module authors to supply data in their modules they can rely on

I propose a new hiera backend that loads a hierarchy of data from the ‘data’ directory in the module, this module must always be present in a puppet install. This ability is key to the ability to create configurable forge modules that do not have hard coded values thanks to the puppet 3 hiera integration

reference the users list thread https://groups.google.com/d/topic/puppet-users/pvqzeyHkrY4/discussion


Related issues

Related to Puppet - Bug #19006: per-environment templatedir doesn't actually work, Needs More Information

History

#1 Updated by R.I. Pienaar over 3 years ago

  • Branch set to https://github.com/puppetlabs/puppet/pull/1217

The PR is an initial pull request, no doubt not perfect but would appreciate an early review

#2 Updated by eric sorenson over 3 years ago

  • Status changed from Unreviewed to Investigating

Added a few people as Watchers who expressed interest in this. I followed up on the ML thread asking for feedback from people who’ve tried it. Seems neat, but for something that’s a proposed module standard and a baked-in enhancement to puppet it’d be great to get some user testing in.

#3 Updated by R.I. Pienaar over 3 years ago

how do we get the ux team to do this testing for us? not been involved in that process before.

#4 Updated by Ryan Coleman over 3 years ago

FWIW, I was playing with this last weekend and will again next weekend. I’ve got a two-part blog post on experimenting with it and once I’m satisfied with my playing around, I’ll offer some feedback in this ticket.

#5 Updated by R.I. Pienaar over 3 years ago

Awesome Ryan thanks, I dont think this PR is quite ready for prime time but as Eric says this is a big decision and quite a behaviour change to a very important part of Puppet so I put this out there for feedback with a view to a refactor post feedback. I think though the problem it tackles is very important so hope we can get it into 3.1.x

#6 Updated by Stefan Goethals over 3 years ago

Hello, I have tested the branch https://github.com/ripienaar/puppet/tree/feature/master/16856. The basic functionality of the data in the module seems to work as desired. Just a few things

  • If only using the param auto-lookup not having a hiera.yaml file is ok
  • if there is NO hiera.yaml config file, calling hiera() causes an error “Error 400 on SERVER: undefined method `include?‘ for nil:NilClass”
  • Having an empty hiera.yaml is ok when using a hiera(call)
  • The lookup in the moduledata does not happen if only using the auto-lookup of the class params, unless you manually specify “module_data” as hiera backend.
  • When using the hiera() call, specifying “module_data” as hiera backend is not needed.

I hope this helps.

Regards,

Stefan Goethals.

#7 Updated by Stefan Goethals over 3 years ago

Hello,

Maybe i should be clearer and specify that the hiera.yaml i mention in the previous comment is /etc/puppet/hiera.yaml and not the modulename/data/hiera.yaml version. That last one indeed only requires a hierarchy setting for things to work.

Stefan.

#8 Updated by R.I. Pienaar over 3 years ago

  • Status changed from Investigating to Needs More Information

I’ve updated my pull request incorporating these items and the stuff from the list about the default hierarchy.

the problem with the backend only being there by default when using hiera() is because the indirector and the parser functions were not DRY – the parser functions used the HieraPuppet class while the indirector duplicated the functionality of this class so I refactored that up to be DRY leading to more consistency.

@zipkid – would be awesome if you could test again

#9 Updated by Stefan Goethals over 3 years ago

I forgot to put the comments here too, besides the mailinglist.

For completeness i add them now even though the comment about the default file naming has already been corrected by Volcane.

Stefan.

Dec 13: The good news:

Without /etc/puppet/hiera.yaml & Without /data/hiera.yaml = OK! Without /etc/puppet/hiera.yaml but with empty /data/hiera.yaml = OK! Without /etc/puppet/hiera.yaml but with /data/hiera.yaml with a hierarchy = OK!

With empty /etc/puppet/hiera.yaml & Without /data/hiera.yaml = OK! With /etc/puppet/hiera.yaml with a hierarch & Without /data/hiera.yaml = OK! With /etc/puppet/hiera.yaml with a hierarch & with empty /data/hiera.yaml = OK! With /etc/puppet/hiera.yaml with a hierarch & with /data/hiera.yaml with a hierarchy = OK!

Lookups via hiera() in a define also seem to work well!

Comment:

The default file looked up in the in-module data backend is ‘default.yaml’, not ‘common.yaml’ and that is different from the yaml backend so it is somewhat confusing. It is also not what was ‘agreed’ in this thread.

All this is based on a simple test with a module with one class and one define so it is not very in-depth but the results are clear and as expected….

Thank you Volcane ! We’re getting there. :–)

If anyone is interested, here is the puppet env i used to test this.

https://github.com/zipkid/puppet3-hiera_data_in_module

#10 Updated by R.I. Pienaar over 3 years ago

@eric – so various people have tested this and the feedback incorporated etc, do you think we can get this targeted for 3.1?

#11 Updated by eric sorenson over 3 years ago

  • Status changed from Needs More Information to In Topic Branch Pending Review
  • Assignee set to Anonymous

Put in review for this week

#12 Updated by Anonymous over 3 years ago

  • Status changed from In Topic Branch Pending Review to Needs More Information
  • Assignee changed from Anonymous to R.I. Pienaar

I’m sorry, but I don’t really understand the problems this is trying to solve. How is the current system making it ‘impossible for module authors to supply data in their modules they can rely on’? What specific use cases are impossible right now that this new functionality would provide?

I think what I’m looking for is specific examples of module setups that cannot be done now (or are done in a less than ideal manner and an explanation of why that isn’t ideal) and how specifically this would help.

#13 Updated by Stefan Goethals over 3 years ago

Hello Andrew,

Without this code, it is NOT possible to extend a module to support variables for F.I. different distributions without either:

  • modifying the module itself

  • Writing a wrapper module with multiple selectors to handle this.

The first is highly undesirable as it defeats the whole forge/git purpose to be able u use and update modules. The second introduces a high level of complexity that is also very undesirable, certainly to people starting out with puppet who see the forge and the are extremely amazed that they have to start editing the installed modules.

With it the module developer can expose all the class params to the module user and provide sensible defaults for all the os'es he knows and can test without limiting the module to those.

To people using Hiera to separate code from variables in a consistent way this is the best thing that has happened to Puppet in a long time (for me in 4,5 years).

Regards,

Stefan – Zipkid – Goethals.

#14 Updated by Ryan Coleman over 3 years ago

I gave this a try by setting out to replace my netatalk module’s params.pp pattern with the module_json hiera backend. If you’re interested in seeing exactly what I did with my code, this comparison on GitHub is useful. I was unable to use the pull request version of this code and resorted to the Ruby gem.

The problem with the params.pp pattern is that it’s an attempt to supply default data to your classes and falls over whenever someone wants to change data or provide their own for their site. They’re forced to either run with a modified module or commit changes upstream, which may contain private data. The module_json hiera backend does serve as a reasonable replacement for the params.pp pattern. I suspect it’ll be better at supplying site-specific data too. I’ll continue to play with it and figure out what the options are.

  • Error handling was abysmal when I mal-formed my JSON file. This example was caused when I had a trailing comma.
[root@centos6 netatalk]# puppet apply -e 'include netatalk' --noop --debug
Info: Loading facts in /etc/puppet/modules/concat/lib/facter/concat_basedir.rb
Debug: importing '/etc/puppet/modules/netatalk/manifests/init.pp' in environment production
Debug: Automatically imported netatalk from netatalk into production
Debug: hiera(): Hiera Module JSON backend starting
Debug: hiera(): Looking up netatalk::package_name in Module JSON backend
Debug: hiera(): Reading config from /etc/puppet/modules/netatalk/data/hiera.json file
Debug: hiera(): Looking for data in source /etc/puppet/modules/netatalk/data/hiera.json
Debug: hiera(): Looking for data in source /etc/puppet/modules/netatalk/data/RedHat.json
Error: malformed format string - %S at line 1 on node centos6.puppetlabs.vm
Wrapped exception:
malformed format string - %S
Error: malformed format string - %S at line 1 on node centos6.puppetlabs.vm
  • I no longer have the following construct that lets me control a friendly error message when Puppet encounters an OS that doesn’t have data supplied.
default: {
  fail("The ${module_name} module only supports EL variants, Ubuntu and Debian. Check the modules documentation for details")
 }

Andy, this solves a problem for my value stream — allowing authors to supply data with their module and allowing the consumer to add-to or modify this data easily. There will always be room for improvement, but this is useful to me as-is and I’d like to see it merged in.

In the mean-time, I’ll continue to play with this and think of what it’s doing and how it can be improved.

#15 Updated by R.I. Pienaar over 3 years ago

Just to clarify – the gem Ryan was playing with was a early attempt at this which had cleanups since and became the code in the Pull request, so some behaviours have changed but the over all goal and approach has not.

There’s some discussion in the PR about error handling which I suspect will cover these once we address it

#16 Updated by R.I. Pienaar over 3 years ago

I’d add here some points I mentioned to Ryan earlier. This is specifically what I found why placing this kind of data in data files beats putting it in a params.pp full of case statements.

Consider:

case $::operatingsystem {
   "debian": { $pkg = "apache2" }
   "redhat": { $pkg = "httpd" }
}

versus just having a RedHat.json and a Debian.json.

When the module author wants to add another Operating System or update an existing OS he’d only change the data files, this is significant because:

  • Editing the data is less error prone than the case statements simply because there is a lot less syntax and stuff to consider
  • The risk of adding a new OS when it is only data is completely contained – suse.yaml will only be consulted on suse machines. There’s no chance that redhat/debian/etc machines will be affected by the change to add a new OS support into the module. So even if a error is made in a new yaml file the error is contained to that specific OS only – you could not break the entire module without making mistakes in every data file for every OS.
  • I found while deploying modules configurable by data and not code in large environments with complex change control requirements that changes to configuration data required less change control than changes to case statements. In some cases we even got agreement to not require any code review on data changes. In any event the change review process is easier if all you have to consider is data changes and no logic changes.
  • Following on from the last point if a module is hosted somewhere like GitHub it’s easier to contribute new OS support and easier to review the addition

So while on a superficial basis you can get far with the params.pp pattern exploring and supporting ways to fully separate data and code has significant advances to module authors wrt how their modules are used and the life cycles of those modules.

Doing this in a way that is consistant with the design of the already present hiera enhances the overall solution and get us nearer to the point of having a single way to handle data thats adopted by a majority of the community and I believe ultimately to a better forge eco system.

#17 Updated by Ryan Coleman over 3 years ago

R.I. Pienaar wrote:

Just to clarify – the gem Ryan was playing with was a early attempt at this which had cleanups since and became the code in the Pull request, so some behaviours have changed but the over all goal and approach has not.

There’s some discussion in the PR about error handling which I suspect will cover these once we address it

Yeah, sorry about that. I just haven’t had as much time to spend on this as I’d like. Context switching blows.

#18 Updated by Anonymous over 3 years ago

R.I. Pienaar wrote:

When the module author wants to add another Operating System or update an existing OS he’d only change the data files, this is significant because:

[snip] * I found while deploying modules configurable by data and not code in large environments with complex change control requirements that changes to configuration data required less change control than changes to case statements. In some cases we even got agreement to not require any code review on data changes. In any event the change review process is easier if all you have to consider is data changes and no logic changes.

Aha! I think you’ve identified the new use case that all of the present ways of doing this cannot handle. In the case where an organization makes a distinction between data in the code and the code itself having this kind of separation can be an immense help.

Given this, then I agree with the basic idea of the feature. I’m still not too happy with the way it is implemented right now in the pull request, but as you pointed out in one of the comments there, a lot of that is related to just the current structure of hiera.

[snip] Doing this in a way that is consistant with the design of the already present hiera enhances the overall solution and get us nearer to the point of having a single way to handle data thats adopted by a majority of the community and I believe ultimately to a better forge eco system.

I agree that this would enhance the integration with hiera that was started with the inclusion of data bindings for parameterized classes. Would still like to get an idea of where exactly we are trying to take all of this so that we aren’t just blowing from one feature to the next without a larger plan, but that concern is beyond this one ticket.

#19 Updated by R.I. Pienaar over 3 years ago

Andrew Parker wrote:

Aha! I think you’ve identified the new use case that all of the present ways of doing this cannot handle. In the case where an organization makes a distinction between data in the code and the code itself having this kind of separation can be an immense help.

To be clear, there is nothing new about this. Hiera exist and is used widely for exactly the listed reasons and many more. When considering the role of for example inheritance as a means of creating data the common conclusion is that its inadequate as a means of code management and data management at a macro scale for an entire site comprising many roles/locations/environments etc. Hiera and in general code/data separation by a configurable hierarchial backend solves these problems well at a macro scale and have done so for 4 or 5 years now – extlookup being a early capable version of the same.

So these benefits are well known and have become basic practice in the community to the point of being the default answer whenever data is discussed. It’s how the community think by default now.

What was absent was a way for the module author to reap these same benefits on a micro scale of a single module.

Even with the old hiera-puppet backend the module author could not in a way that had an impedance match with hiera – which he was already using at site level:

  • bundle default data with his module on the forge or internal code share in the hiera way
  • ensure this data gets consulted when he shared his module

So we forced them to use inheritance and the params.pp pattern in his module. This seems to be against the generally accepted wisdom that inheritance was a bad fit in Puppet on a macro scale and the module author already understood well the value of code and data separation but could not reap those benefits on a micro scale.

So what this PR does is bring the many existing and widely realized benefits of hiera into the module space and create a suitable way to access the those even for modules he wish to share outside of his own team onto the forge etc. This enhances the overall consistency of code and allow us narrow down the way we think modules should be built to be in line with how people currently manage the other data for their site.

#20 Updated by R.I. Pienaar over 3 years ago

Andrew Parker wrote:

I agree that this would enhance the integration with hiera that was started with the inclusion of data bindings for parameterized classes. Would still like to get an idea of where exactly we are trying to take all of this so that we aren’t just blowing from one feature to the next without a larger plan, but that concern is beyond this one ticket.

I strongly disagree that this is ‘blowing from one feature to the next’.

Data separation has been a topic for years and years, I’ve personally probably discussed this problem with 1000+ people. Many in Puppet Labs have attempted to solve this problem, I think Nigel spent well over a year on and off on it even to the point of involving language designers at google. Many of us have implemented various takes on this and many in the community did the same. The current feature set and overall design of hiera is the result of literally years of small directed iterations driven strongly by discussing the needs of the user base and validating those ideas.

In the end I donated the current Hiera code for inclusion and it got included because it was clear that the model it uses is correct and solves the problem effectively for a very large % of users. It isn’t the solution for 100% of users but it was never supposed to be it aims to solve it for the high % of common users while leaving the integration points like ENCs etc for those with really special needs.

The fact that this is very widely adopted in the community, used effectively and often cited as one of the best recent additions to Puppet underpins this as the right model. Today we are very much at a place where there is one default way in which people manage data in Puppet. This is a huge win something we have all identified as being a massive short coming in Puppet.

As per the PR we can certainly improve the code structure and I’ve been wanting to do so. It though seems that the team just is a) not aware of the design and background of how this problem got solved b) not paying this code base much attention c) starting to bike shed on redesigning this yet again.

So rather than this being a matter of ‘blowing from feature to feature’ I consider this a natural progression and comfortable fit to what we have today which is the result of years of refinement and research. If anything is blowing from feature to feature it’s those who are again embarking on a process of re-solving this problem and looking to interrupt the community who has now rallied around a single data solution while it seems no-one was paying attention – to do so is a great risk, we are just about reaching this point since the inclusion of Hiera in Puppet 3 and now it seems we’re prepared to squander that opportunity.

Instead the focus should be improve the underlying hiera code, keeping its behavior and design and bolstering it for our future needs by iterative improvements which is exactly what this PR does.

On the subject of this specific feature. Nothing here is new by a long stretch, go back to the first Puppet Conf and mailing list threads around that time and you’ll find implementations of exactly this from Ohad for example and there has been several since. We’ve known for literally years modules need data and code separation and that it would improve matters. What we didnt have was the correct always-present data feature in Puppet one that meets the various needs of users so none of those got merged as they all had slightly different – but relevant – approaches. Fast forward to 3.0.0 release and we now have that, what you’re seeing is closing the loops on age old needs that has been presented, discussed, validated by POC etc for a very long time. This stuff is based on user research long before Puppetlabs thought user research mattered.

#21 Updated by Stefan Goethals over 3 years ago

Hi,

I created patch files on a clone of git://github.com/ripienaar/puppet.git

git format-patch b22eb3c443d1161653f58c270dee1b5f88676c5f

-rw-r—r— 1 root root 12255 Dec 27 13:03 0001-16856-Add-a-module-data-directory-and-hiera-backend-.patch -rw-r—r— 1 root root 5876 Dec 27 13:03 0002-16856-Add-a-module-data-directory-and-hiera-backend-.patch -rw-r—r— 1 root root 1063 Dec 27 13:03 0003-16856-Add-a-module-data-directory-and-hiera-backend-.patch

Applying them on the latest git clone of puppet with git am -3 /tmp/patches/*

then building the rpms.

The Hiera-in module and standard hiera lookups work as expected. But the ‘inherits’ method now seems broken.

class test (
  $testvar = 'blah',
  $ttt = $test::data::ttt,
) inherits ::test::data
{
  notify { "Test module -testvar- .... :${testvar}:": }
  notify { "Test module -  ttt  - .... :${ttt}:": }
}
class test::data {
  $ttt = '::test::data:ttt is a var with this as the val'
}

This gives the following error

Debug: hiera(): Hiera YAML backend starting
Debug: hiera(): Looking up test::testvar in YAML backend
Debug: hiera(): Looking for data source common
Debug: hiera(): Hiera Module Data backend starting
Debug: hiera(): Looking up test::testvar in Module Data backend
Debug: hiera(): Reading config from /etc/puppet/modules/test/data/hiera.yaml file
Debug: hiera(): Looking for data in source /etc/puppet/modules/test/data/hiera.yaml
Debug: hiera(): Looking for data in source /etc/puppet/modules/test/data/osfamily/RedHat.yaml
Debug: hiera(): Looking up test::ttt in YAML backend
Debug: hiera(): Looking for data source common
Debug: hiera(): Looking up test::ttt in Module Data backend
Debug: hiera(): Reading config from /etc/puppet/modules/test/data/hiera.yaml file
Debug: hiera(): Looking for data in source /etc/puppet/modules/test/data/hiera.yaml
Debug: hiera(): Looking for data in source /etc/puppet/modules/test/data/osfamily/RedHat.yaml
Debug: hiera(): Looking for data in source /etc/puppet/modules/test/data/common.yaml
Error: Could not find data item test::ttt in any Hiera data file and no default supplied at /etc/puppet/manifests/site.pp:2 on node pm3-h.lan.super-visions.com
Error: Could not find data item test::ttt in any Hiera data file and no default supplied at /etc/puppet/manifests/site.pp:2 on node pm3-h.lan.super-visions.com
Error: Could not find data item test::ttt in any Hiera data file and no default supplied at /etc/puppet/manifests/site.pp:2 on node pm3-h.lan.super-visions.com
Debug: Finishing transaction 69977590831240

#22 Updated by R.I. Pienaar over 3 years ago

Is it only the one for $ttt that’s broken, does $testvar work? Ie. trying to figure out if its all defaults or just the one that is based on inheriting.

#23 Updated by Stefan Goethals over 3 years ago

Sorry i did not specify that.

Only the inheritance fails –> $ttt

The other –> $testvar works ok.

#24 Updated by R.I. Pienaar over 3 years ago

I’ve pushed to my branch a commit that seems to fix it for me, thanks for reporting

#25 Updated by R.I. Pienaar over 3 years ago

  • Assignee changed from R.I. Pienaar to eric sorenson

What information is still needed here?

#26 Updated by Jan Ivar Beddari over 3 years ago

I was hoping this would be available before I started refactoring modules for 3.x

It impacts what I consider best practice module design in a big way and makes data/code separation for new Puppet users a lot easier to grasp.

#27 Updated by John Moser about 3 years ago

Was looking forward to this for 3.1 but it didn’t make it, hoping for 3.2. Will be good to:

  • dump 95% of my logic in favor of letting Hiera do the work of deciding which packages/files/etc I need
  • get rid of all the coding style decisions on whether to have a params-type class that decides this, or subclasses for debian/ubuntu/redhat/fedora, or to have small bits of logic inlined with decisions on facter checks

There’s exactly one way to do all that stuff now: You call the resources you want with hiera() or related functions, and you stick them in YAML or JSON files. No structure beyond that—and that’s not much structure. Convergence to having exactly one way TO do something is good, since there’s fewer ways to do it terribly.

#28 Updated by eric sorenson about 3 years ago

Thanks for all of the commentary and support on this request. I’ve written up what I believe is a complete summary of the use cases here :

https://gist.github.com/ahpook/4739246

The two blockers that need to be addressed are:

  • the heira.yaml is separate from the rest of the module metadata (in modulefile and metadata.json) rather than providing a consistent integration
  • automatic data-binding lookups, which would now extend into modules, do not have a way to specify override vs merge/append behaviour

#29 Updated by R.I. Pienaar about 3 years ago

+1 to moving the data into the module metadata

+1 to making data bindings non-pluggable and providing one well known and well understood plugin interface for those via hiera – allowing hiera extensions and data binding extensions just complicate matters for users and create a pile of complex cases where sometimes it works and othertimes not etc, less choice is more.

#30 Updated by Ryan Coleman about 3 years ago

eric sorenson wrote:

Thanks for all of the commentary and support on this request. I’ve written up what I believe is a complete summary of the use cases here :

https://gist.github.com/ahpook/4739246

The two blockers that need to be addressed are:

  • the heira.yaml is separate from the rest of the module metadata (in modulefile and metadata.json) rather than providing a consistent integration

Please consider both of these metadata files owned by the Forge team and keep us engaged on what you’d like to do with these. Totally open to their use, we’ll just need to coordinate.

  • automatic data-binding lookups, which would now extend into modules, do not have a way to specify override vs merge/append behaviour

#31 Updated by eric sorenson about 3 years ago

Ryan Coleman wrote:

Please consider both of these metadata files owned by the Forge team and keep us engaged on what you’d like to do with these. Totally open to their use, we’ll just need to coordinate.

ABSO-SMURFLY

#32 Updated by Jan Ivar Beddari about 3 years ago

Any updates? holding breath

#33 Updated by Stefan Goethals about 3 years ago

… helping holding breath …

#34 Updated by Stefan Goethals about 3 years ago

Hello,

What is keeping this from being added? What can be done to get it moving?

Regards,

Stefan – Zipkid – Goethals.

#35 Updated by Klavs Klavsen about 3 years ago

I would sooo much like this feature as well.

I’ve been doing this manually, using hiera-puppet, before 2.7.. and the drawbacks it had, is resolved with this as well..

And doing modules, used on Debian-based, RedHat based, Solaris and Windows hosts.. I really need a well structured way do handle OS abstraction and IMHO this is it.

What is missing before this can be applied?

#36 Updated by eric sorenson about 3 years ago

Thanks for the pings on the ticket, all. But the issues with this branch are enumerated in note 28 and 29 from me and RI. The doc which describes user stories and functional requirements is also in note 28, and is in an ARM now, here: https://github.com/puppetlabs/armatures/blob/master/arm-8.data_in_modules/index.md

The way forward if you want to advance the state of this feature is to read and comment/send a pull request on that ARM-8 doc with a proposal that will satisfy the design constraints.

#37 Updated by R.I. Pienaar about 3 years ago

Few comments then as it relates to the ARM – we can’t comment on the ARM itself which makes it quite pointless.

  • We should put this in the modulefile for sure, what’s the next step here? Ryan had been quite vocal that the forge team owns it, if this ticket is a priority how do we get a proposal in from them?
  • +1 to removing the plugability of the data bindings. The hiera backends should be good enough, so what’s the next step?
  • The lookup policies are a legit concern, but this does not really relate to this ticket its a larger improvement to puppet and the binding system. Merging this ticket as is even without that point addressed would add a great amount of ability and later once addressed this feature will simply become better in line with the rest of the data binding system.

So I do not see much here that’s blockers or requiring a pull request to the ARM to address. The user stories seem well catered for by the proposed solution.

The actions should be:

  • Get team forge to state how they wish to integrate this into the modulefile – a PR to the arm would do that though this ticket would be a more central source of truth as its obvious this is where everyones eyes are.
  • Adjust the PR to work with the module file and merge it
  • Independently in an additional ticket remove the plugin ability from the bindings
  • Independently in an additional ticket rewrite hiera if the backends arent considered to be adequate replacements for the binding backends
  • Independently improve the ability to specify merge policies for keys

Only steps 1 and 2 are really blockers here, the rest of the points would be incremental improvements that slowly improve this feature but wouldn’t change how it functions – they would only add abilities not remove any, I do not think there are many custom data binding backends out there that we’d block this for those given just how many people want this feature and how many significant problems it solves.

#38 Updated by Ryan Coleman about 3 years ago

R.I. Pienaar wrote:

Few comments then as it relates to the ARM – we can’t comment on the ARM itself which makes it quite pointless.

  • We should put this in the modulefile for sure, what’s the next step here? Ryan had been quite vocal that the forge team owns it, if this ticket is a priority how do we get a proposal in from them?
  • +1 to removing the plugability of the data bindings. The hiera backends should be good enough, so what’s the next step?
  • The lookup policies are a legit concern, but this does not really relate to this ticket its a larger improvement to puppet and the binding system. Merging this ticket as is even without that point addressed would add a great amount of ability and later once addressed this feature will simply become better in line with the rest of the data binding system.

So I do not see much here that’s blockers or requiring a pull request to the ARM to address. The user stories seem well catered for by the proposed solution.

The actions should be:

  • Get team forge to state how they wish to integrate this into the modulefile – a PR to the arm would do that though this ticket would be a more central source of truth as its obvious this is where everyones eyes are.

TL;DR, I don’t wish to integrate in-module hiera data into the modulefile.

You have stated in the past that you (I’m paraphrasing) expect in-module data to allow module consumers to more easily contribute patches upstream that support additional operating systems. Cool.

The Modulefile/metadata.json files are the authors expression of module metadata that allow them to publish to the Forge. It’s only meant for their manipulation and it’s critical to the operation of Forge and the module tool. If we store hiera data in this file, we’re introducing a use-case for the consumer to manipulate this file. If they decide to carry a patch locally or their submitted patch isn’t accepted upstream, now you cannot upgrade your module without resolving the conflict in the modules metadata. This doesn’t seem optimal. If all we’re buying is reducing the number of files that make up a module, I don’t think it’s worth it. Instead, I suggest that you go with another file or folder/file structure for this purpose.

  • Adjust the PR to work with the module file and merge it
  • Independently in an additional ticket remove the plugin ability from the bindings
  • Independently in an additional ticket rewrite hiera if the backends arent considered to be adequate replacements for the binding backends
  • Independently improve the ability to specify merge policies for keys

Only steps 1 and 2 are really blockers here, the rest of the points would be incremental improvements that slowly improve this feature but wouldn’t change how it functions – they would only add abilities not remove any, I do not think there are many custom data binding backends out there that we’d block this for those given just how many people want this feature and how many significant problems it solves.

#39 Updated by R.I. Pienaar about 3 years ago

TL;DR, I don’t wish to integrate in-module hiera data into the modulefile.

You have stated in the past that you (I’m paraphrasing) expect in-module data to allow module consumers to more easily contribute patches upstream that support additional operating systems. Cool.

The Modulefile/metadata.json files are the authors expression of module metadata that allow them to publish to the Forge. It’s only meant for their manipulation and it’s critical to the operation of Forge and the module tool. If we store hiera data in this file, we’re introducing a use-case for the consumer to manipulate this file. If they decide to carry a patch locally or their submitted patch isn’t accepted upstream, now you cannot upgrade your module without resolving the conflict in the modules metadata. This doesn’t seem optimal. If all we’re buying is reducing the number of files that make up a module, I don’t think it’s worth it. Instead, I suggest that you go with another file or folder/file structure for this purpose.

We weren’t suggesting storing data in this file, only the hierarchy description. In the most common case people contributing to the module would not edit the modulefile they’d just add data.

I agree though – overloading the modulefile for different purposes is not desirable though I am not bothered personally where this stuff goes.

#40 Updated by Ryan Coleman about 3 years ago

R.I. Pienaar wrote:

We weren’t suggesting storing data in this file, only the hierarchy description. In the most common case people contributing to the module would not edit the modulefile they’d just add data.

Oh! I apologize Ari. That makes loads more sense.

I agree though – overloading the modulefile for different purposes is not desirable though I am not bothered personally where this stuff goes.

Yeah, I’m still holding to the conflict of purposes being sub-optimal but I don’t feel strongly about the hierarchy being in there or not. As you say, it’s likely that only the author will care to modify the hierarchy.

#41 Updated by Richard Clamp about 3 years ago

Even if the authors hierarchy definition doesn’t get moved into the Modulefile, I think it’s a neccesity to to move it out of the data/ directory simply to reduce the potential for confusion in having a file that could end up referring to itself and so serving double duty.

#42 Updated by Stefan Goethals about 3 years ago

Reading the latest comments it seems to me the easiest and most logical solution is putting the hiera.yaml at the root of the module, next to the Modulefile. That way the hiera.yaml is out of the /data directory and the Modulefile must never be touched by module consumers. As the hiera.yaml is optional it is not even necessary to distribute it for simple modules.

I believe changing this should be very trivial and then maybe this can finally get added to Puppet…!

Regards,

Stefan – Zipkid – Goethals

#43 Updated by Jeff Goldschrafe almost 3 years ago

My thoughts on the “conflict of purposes” in the Modulefile as pertains to ARM-8:

The separate hiera.yaml configuration file lacks integration with the existing module metadata. The config should instead live inside the ‘modulefile’. This would provide a better user experience for both publishers and module users.

I think that this comes down to a really simple design decision, ultimately. Should users be able to rely on the in-module hiera.yaml having the exact same semantics as the site hiera.yaml, including being able to specify multiple backends for the data? If it should behave exactly the same way, hiera.yaml should be used. If not, the Modulefile (or some other not-hiera.yaml mechanism) should be used. Creating an identically-named file that looks really similar but has totally different supported functionality depending on where it lives seems like an obvious UX no-no to me.

#44 Updated by Chad Huneycutt over 2 years ago

So is this going to happen? I know it would come in really handy for the new environment I am building out now…

#45 Updated by Ryan Coleman over 2 years ago

The next iteration of this idea is available for experimentation. You can find it in this PR and read more about it in ARM-9.

I’m just playing the messenger, having just tried out Henrik’s branch. In order to use this, you’ll need to configure puppet.conf to set parser = future and binder = true. I believe that you’ll also need to install the rgen gem.

#46 Updated by Anonymous over 2 years ago

  • Status changed from Needs More Information to Merged - Pending Release
  • Target version set to 3.3.0

#47 Updated by Henrik Lindberg over 2 years ago

See ARM-9 https://github.com/puppetlabs/armatures/blob/master/arm-9.data_in_modules/index.md for the specification.

A very simple proof-of-concept example from Pro-Puppet can be found here: https://github.com/pro-puppet/puppet-module-startrek

#48 Updated by Anonymous over 2 years ago

  • Status changed from Merged - Pending Release to Closed

Released in 3.3.0

#49 Updated by Anonymous over 2 years ago

Released in 3.3.0

#50 Updated by eric sorenson over 2 years ago

  • Status changed from Closed to Re-opened
  • Target version deleted (3.3.0)

Re-opening for migration since the 3.3.0 Binder did not pass user testing and is slated for removal.

#51 Updated by eric sorenson over 2 years ago

Redmine Issue #16856 has been migrated to JIRA:

https://tickets.puppetlabs.com/browse/PUP-1157

Also available in: Atom PDF