claranet / puppet-consul_template

A Puppet module to manage the config and jobs of Consul Template from Hashicorp
Apache License 2.0
30 stars 89 forks source link

config file format error when pretty_config is false #115

Closed C0reFast closed 6 years ago

C0reFast commented 6 years ago

Affected Puppet, Ruby, OS and module versions/distributions

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

class { '::consul_template':
        service_enable => false,
        install_method => 'package', 
        package_name   => 'consul-template',
        package_ensure => '0.19.4-1',
        bin_dir        => '/usr/local/bin',
        config_hash    => {
            wait      => '5s:30s',
            max_stale => '1s'
        }
    }
    consul_template::watch { 'nginx':
    template      => 'data/nginx.ctmpl.erb',
    config_hash   => {
      destination => '/etc/nginx/conf/conf.d/consul.conf',
      command     => 'service openresty reload',
    },
}

What are you seeing

the config.json will be:

{"consul":"localhost:8500","max_stale":"1s","wait":"5s:30s"},
    "template": [
    {"command":"service openresty reload","destination":"/tmp/consul1.conf", "source":"/etc/consul-template/nginx.ctmpl"},
    ],
}

the first line is closed by '}', the config is not a valid json

What behaviour did you expect instead

when add "pretty_config => true" to class { '::consul_template':,the config is valid。

Output log

Any additional information you'd like to impart

I think this line:https://github.com/claranet/puppet-consul_template/blob/master/manifests/config.pp#L13 case the problem, when pretty_config is set to false, the main config json is not end with "\n}\n$", so the code 'regsubst($content_full, "\n}\n$", '')' didn't strip the '}', this make config format error

craigwatson commented 6 years ago

Admittedly the JSON generated by the module isn't strictly valid, but - in my tests, at least - HCL doesn't necessarily care.

See the note here: https://github.com/claranet/puppet-consul_template/blob/9583592d1860017cb043810f1c858bd473b56a0c/manifests/watch.pp#L54-L55

Does consul-template itself have any issue with the config generated by the module?

C0reFast commented 6 years ago

in my case, the consul-template logs a warning: "2017/12/18 10:20:25 [WARN] consul now accepts a stanza instead of a string. Update your configuration files and change consul = "" to consul { } instead.", and didn't generate the nginx config file. and the consul-template version is "consul-template v0.19.4 (68b1da2)"

craigwatson commented 6 years ago

Thanks for reporting this - there was indeed a bug in the module.

I've backported the fix into version 1.0.1 of the module (the last to support Puppet 3) - I'll close this issue as fixed for now, but please feel free to re-open if the problem still exists using the latest Forge release.