voxpupuli / puppet-augeasproviders_puppet

Augeas-based Puppet configuration types and providers for Puppet
Apache License 2.0
2 stars 10 forks source link

Diff backwards and Changes not applied #3

Closed acidprime closed 9 years ago

acidprime commented 9 years ago

Given the following auth.conf

path ~ ^/catalog/([^/]+)$
method find
auth yes
allow $1

path ~ ^/node/([^/]+)$
method find
auth yes
allow $1

path  /certificate_revocation_list/ca
method find
auth yes
allow *

path ~ ^/report/([^/]+)$
method save
auth yes
allow $1

path  /file
auth yes
allow *

path  /certificate/ca
method find
auth any
allow *

path  /certificate/
method find
auth any
allow *

path  /certificate_request
method find, save
auth any
allow *

path  /v2.0/environments
method find
auth yes
allow *

path  /facts
method find, search
auth any
allow pe-internal-dashboard

path  /resource_type
method find, search
auth yes
allow pe-internal-dashboard, pe-internal-classifier

path  /
auth any

With the following puppet code

  puppet_auth { 'allow the diff server to retrieve any catalog':
    ensure        => present,
    path          => '^/catalog/([^/]+)$',
    path_regex    => true,
    authenticated => 'yes',
    methods       => 'find',
    allow         => ['$1', $diff_master],
  }

  puppet_auth { 'allow the diff server to query facts':
    ensure        => present,
    path          => '/facts',
    authenticated => 'any',
    path_regex    => false,
    methods       => [ 'find', 'search'],
    allow         => ['$1', $diff_master, 'pe-internal-dashboard'],
  }

Causes the no changes and results in a rather strange diff

Notice: /Stage[main]/Catalog_diff/Puppet_auth[allow the diff server to retrieve any catalog]/allow: allow changed ['$1'] to '$1 compile-master-1.vm'
Notice: /Stage[main]/Catalog_diff/Puppet_auth[allow the diff server to query facts]/allow: allow changed ['pe-internal-dashboard'] to '$1 compile-master-1.vm pe-internal-dashboard'
Notice: /Stage[main]/Puppet_enterprise::Profile::Master::Auth_conf/File[/etc/puppetlabs/puppet/auth.conf]/content:
--- /etc/puppetlabs/puppet/auth.conf    2015-08-31 19:11:55.472823920 +0000
+++ /tmp/puppet-file20150831-22170-12rzjfw  2015-08-31 19:11:55.482828920 +0000
@@ -1,7 +1,7 @@
 path ~ ^/catalog/([^/]+)$
 method find
 auth yes
-allow $1, compile-master-1.vm
+allow $1

 path ~ ^/node/([^/]+)$
 method find
@@ -45,7 +45,7 @@
 path  /facts
 method find, search
 auth any
-allow $1, compile-master-1.vm, pe-internal-dashboard
+allow pe-internal-dashboard

 path  /resource_type
 method find, search
acidprime commented 9 years ago
/opt/puppet/bin/puppet --version
3.8.2 (Puppet Enterprise 3.8.2)
/opt/puppet/bin/augtool --version
augtool 1.3.0 <http://augeas.net/>
Copyright (C) 2007-2011 David Lutterkort
License LGPLv2+: GNU LGPL version 2.1 or later
                 <http://www.gnu.org/licenses/lgpl-2.1.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by David Lutterkort
uname -a
Linux ca.vm 2.6.32-504.8.1.el6.x86_64 #1 SMP Wed Jan 28 21:11:36 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
acidprime commented 9 years ago

/opt/puppet/share/augeas/lenses/dist/puppet_auth.aug

(*
Module: Puppet_Auth
  Parses /etc/puppet/auth.conf

Author: Raphael Pinson <raphael.pinson@camptocamp.com>

About: Reference
  This lens tries to keep as close as possible to `http://docs.puppetlabs.com/guides/rest_auth_conf.html` where possible.

About: License
   This file is licenced under the LGPL v2+, like the rest of Augeas.

About: Lens Usage
   To be documented

About: Configuration files
   This lens applies to /etc/puppet/auth.conf. See <filter>.

About: Examples
   The <Test_Puppet_Auth> file contains various examples and tests.
*)

module Puppet_Auth =

autoload xfm

(* View: list
   A list of values *)
let list (kw:string) (val:regexp) =
     let item = [ seq kw . store val ]
  in let comma = del /[ \t]*,[ \t]*/ ", "
  in [ Util.indent . key kw . Sep.space
     . Build.opt_list item comma . Util.comment_or_eol ]

(* View: auth
   An authentication stanza *)
let auth =
  [ Util.indent . Build.xchg /auth(enticated)?/ "auth" "auth"
  . Sep.space . store /yes|no|on|off|any/ . Util.comment_or_eol ]

(* View: reset_counters *)
let reset_counters =
    counter "environment" . counter "method"
  . counter "allow" . counter "allow_ip"

(* View: setting *)
let setting = list "environment" Rx.word
            | list "method" /find|search|save|destroy/
            | list "allow" /[^# \t\n,][^#\n,]*[^# \t\n,]|[^# \t\n,]/
            | list "allow_ip" /[A-Za-z0-9.:\/]+/
            | auth

(* View: record *)
let record =
     let operator = [ label "operator" . store "~" ]
  in [ Util.indent . key "path"
      . (Sep.space . operator)?
      . Sep.space . store /[^~# \t\n][^#\n]*[^# \t\n]|[^~# \t\n]/ . Util.eol
      . reset_counters
      . (Util.empty | Util.comment | setting)*
      . setting ]

(* View: lns *)
let lns = (Util.empty | Util.comment | record)*

(* Variable: filter *)
let filter = (incl "/etc/puppet/auth.conf"
             .incl "/usr/local/etc/puppet/auth.conf"
             .incl "/etc/puppetlabs/puppet/auth.conf")

let xfm = transform lns filter
acidprime commented 9 years ago

Removing the respective entries from the file, results in their creation

path ~ ^/node/([^/]+)$
method find
auth yes
allow $1

path  /certificate_revocation_list/ca
method find
auth yes
allow *

path ~ ^/report/([^/]+)$
method save
auth yes
allow $1

path  /file
auth yes
allow *

path  /certificate/ca
method find
auth any
allow *

path  /certificate/
method find
auth any
allow *

path  /certificate_request
method find, save
auth any
allow *

path  /v2.0/environments
method find
auth yes
allow *

path  /resource_type
method find, search
auth yes
allow pe-internal-dashboard, pe-internal-classifier

path  /
auth any
#allow * <- here so I could do the puppet run after removal
path ~ ^/catalog/([^/]+)$
method find
allow $1, compile-master-1.vm
auth yes
path /facts
method find, search
allow $1, compile-master-1.vm, pe-internal-dashboard
auth any
raphink commented 9 years ago

So if the entries don't exist, they get created, and if they already exist, they get removed?

acidprime commented 9 years ago

They don't actually get removed, they don't get changed at all. The diff shows them as being removed but thats basically showing the opposite of what should be happening. As if we were managing the rules in reverse.

raphink commented 9 years ago

So if they exist, the behavior is OK, but the logs are wrong?

acidprime commented 9 years ago

If they exist, they are NOT edited. And the log above is displayed, indicating a diff which on its face appears to show it removing the things it should be adding. The resultant file is basically not touched.

If they don't exist they are added:

Removing the respective entries from the file, results in their creation

path ~ ^/node/([^/]+)$ method find auth yes allow $1

path /certificate_revocation_list/ca method find auth yes allow *

path ~ ^/report/([^/]+)$ method save auth yes allow $1

path /file auth yes allow *

path /certificate/ca method find auth any allow *

path /certificate/ method find auth any allow *

path /certificate_request method find, save auth any allow *

path /v2.0/environments method find auth yes allow *

path /resource_type method find, search auth yes allow pe-internal-dashboard, pe-internal-classifier

path / auth any

allow * <- here so I could do the puppet run after removal

path ~ ^/catalog/([^/]+)$ method find allow $1, compile-master-1.vm auth yes path /facts method find, search allow $1, compile-master-1.vm, pe-internal-dashboard auth any

raphink commented 9 years ago

It'd be really great if you could provide a failing unit test for this.

raphink commented 9 years ago

@acidprime just reading your issue again:

Notice: /Stage[main]/Catalog_diff/Puppet_auth[allow the diff server to retrieve any catalog]/allow: allow changed ['$1'] to '$1 compile-master-1.vm'
Notice: /Stage[main]/Catalog_diff/Puppet_auth[allow the diff server to query facts]/allow: allow changed ['pe-internal-dashboard'] to '$1 compile-master-1.vm pe-internal-dashboard'
Notice: /Stage[main]/Puppet_enterprise::Profile::Master::Auth_conf/File[/etc/puppetlabs/puppet/auth.conf]/content:

Your Puppet_enterprise::Profile::Master::Auth_conf class manages the file in its entirety with a content parameter, which is why the diff is "reversed" when you apply. It's just a resource collision.

acidprime commented 9 years ago

@raphink heh, good catch