voxpupuli / puppet-caddy

Puppet Caddy module
MIT License
6 stars 12 forks source link

Change the default values #54

Closed dhoppe closed 4 years ago

dhoppe commented 4 years ago

The default values were only taken from the following file:

I think we should use the following default values:

class { 'caddy':
  $install_path  => '/opt/caddy',
  $caddy_home    => '/var/www',
  $caddy_ssl_dir => '/etc/ssl/caddy',
}
dhoppe commented 4 years ago

@voxpupuli/collaborators I need your feedback, before I do any code changes.

runejuhl commented 4 years ago

The default values were only taken from the following file:

* https://github.com/caddyserver/caddy/tree/v1.0.4/dist/init/linux-systemd

Any idea why the service unit file looks to be gone from the caddy repo after v1.0.4? Has anything major changed since?

On the whole I'm in favor of trying to avoid deviating from upstream, so I'd be fine with this change.

dhoppe commented 4 years ago

@runejuhl Caddy v2 will be released next week. As you can see, the default values have changed again:

They also added an API which allows the configuration of Caddy. This might make #58 obsolete.

ekohl commented 4 years ago

As a general rule, I always prefer the systemd unit file provided as part of the package and use overrides if needed.

dhoppe commented 4 years ago

As a general rule, I always prefer the systemd unit file provided as part of the package and use overrides if needed.

I would agree in most cases, but it does not make sense to extract a .tar.gz to /usr/local/bin.

root@debian8-64-1:/# ls -l /usr/local/bin/
total 21936
-r--r--r-- 1 1001 1001    25603 Feb 27 18:41 CHANGES.txt
-r--r--r-- 1 1001 1001    24728 Feb 27 18:41 LICENSES.txt
-r--r--r-- 1 1001 1001      853 Feb 27 18:41 README.txt
-rwxr-xr-x 1 1001 1001 22395422 Apr 20 20:24 caddy
dr-xr-xr-x 7 1001 1001     4096 Feb 27 18:41 init

The file data/family/Debian.yaml is also messing arround with the home directory of the user www-data. In case the user already exists, it would be changed from /var/www to /opt/caddy.

ekohl commented 4 years ago

Right, I forgot this was a tarball and not a proper RPM/deb that handles it.

dhoppe commented 4 years ago

That would be the next problem, because Caddy v2 does not provide an init script for CentOS 6. So it would really make sense to drop the support for it. On the other hand, they started to build packages for .deb and .rpm.

ekohl commented 4 years ago

I mentioned this on IRC, but dropping CentOS 6 is fine IMHO. We're a community run project with limited resources. It'll be EOL in November and focusing on the future is a good thing for us.