Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The mapfile banner should not be duplicated #103

Closed
jcbollinger opened this issue Mar 19, 2018 · 0 comments · Fixed by #119
Closed

The mapfile banner should not be duplicated #103

jcbollinger opened this issue Mar 19, 2018 · 0 comments · Fixed by #119
Labels
enhancement New feature or request

Comments

@jcbollinger
Copy link
Contributor

jcbollinger commented Mar 19, 2018

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: 5.4
  • Ruby: 2.1
  • Distribution: RedHat 7.4
  • Module version: 4.3.0

How to reproduce (e.g Puppet code you use)

The issue revolves around defining the contents of a single map file via multiple autofs::map resources. This is a useful thing to do, because it allows the mapfile definition to be distributed among different classes, and it already works. But the current module implementation emits a warning banner into the target map file for each contributing autofs::map resource, instead of just one banner at the top of the file.

You don't even need to weigh the merits of distributed map file definitions, because the issue can be reproduced by applying a catalog built from as little as this:

include '::autofs'

autofs::mount { 'test':
  ensure  => 'present',
  mount   => '/mnt/test',
  mapfile => '/etc/test.auto'
}

autofs::map { 'test1':
  mapfile     => '/etc/test.auto',
  mapcontents => ['test1 -rw test.example.com:/export/test1']
}

(There is a workaround for that particular case, however.)

What are you seeing

In the example manifest above, the autofs::mount declares an autofs::map with the (default; empty) mapcontents that is declared directly on the autofs::mount, so there are two autofs::map resources contributing to /etc/test.auto. Result:

###############################################################
#                                                             #
# THIS FILE IS MANAGED BY PUPPET. ANY CHANGES MADE TO THIS    #
#   FILE WILL BE REVERTED BACK ON THE NEXT PUPPET RUN.        #
#                                                             #
###############################################################

###############################################################
#                                                             #
# THIS FILE IS MANAGED BY PUPPET. ANY CHANGES MADE TO THIS    #
#   FILE WILL BE REVERTED BACK ON THE NEXT PUPPET RUN.        #
#                                                             #
###############################################################

test1 -rw test.example.com:/export/test1

If there were more contributing autofs::map resources, there would be an additional banner for each.

The extra banners do not invalidate the map file, of course, but they do make it needlessly hard for a human to read. It gets worse when there are more autofs::map resources, so that the actual mappings are sandwiched between banners.

What behaviour did you expect instead

The banner ought to be presented only once, at the top of the map file, no matter how many autofs::map resources contribute to it:

###############################################################
#                                                             #
# THIS FILE IS MANAGED BY PUPPET. ANY CHANGES MADE TO THIS    #
#   FILE WILL BE REVERTED BACK ON THE NEXT PUPPET RUN.        #
#                                                             #
###############################################################

test1 -rw test.example.com:/export/test1

Output log

(irrelevant)

Any additional information you'd like to impart

It looks like this should be easy to fix. The concat resource type has a warn parameter that serves exactly the purpose of presenting a warning banner, and it can be used to present a customized banner. This (autofs) module does not use that, but it probably should. Instead, it puts the banner -- a selection between two alternatives, in fact -- in the per-autofs::map templates. Even if it were for some reason undesirable to use $autofs::warn for this purpose, the problem could also be solved by putting the banner in its own concat::fragment, and ensuring that that fragment comes first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants