Skip to content

Add dependencies as a parameter #52

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

Closed
c33s opened this issue Sep 7, 2016 · 3 comments
Closed

Add dependencies as a parameter #52

c33s opened this issue Sep 7, 2016 · 3 comments

Comments

@c33s
Copy link
Member

c33s commented Sep 7, 2016

kind of related to: #41

hardcoded parameters are not that good, having $package_name and $dependency as a parameter for letsencrypt::install would be awesome.

https://github.com/danzilio/puppet-letsencrypt/blob/v1.0.0/manifests/install.pp#L56
https://github.com/danzilio/puppet-letsencrypt/blob/v1.0.0/manifests/install.pp#L62
https://github.com/danzilio/puppet-letsencrypt/blob/v1.0.0/manifests/install.pp#L44

class letsencrypt::install (
  $package_name        = $letsencrypt::package_name, # 'letsencrypt' or 'certbot'
  $dependencies        = $letsencrypt::dependencies, # ['python', 'git']
  $manage_install      = $letsencrypt::manage_install,
  $manage_dependencies = $letsencrypt::manage_dependencies,
  $configure_epel      = $letsencrypt::configure_epel,
  $install_method      = $letsencrypt::install_method,
  $package_ensure      = $letsencrypt::package_ensure,
  $path                = $letsencrypt::path,
  $repo                = $letsencrypt::repo,
  $version             = $letsencrypt::version,
)
  if $install_method == 'vcs' {
    if $manage_dependencies {
      ensure_packages($dependencies)
      Package[$dependencies] -> Vcsrepo[$path]
}
    package { $package_name:
      ensure => $package_ensure,
}

because for example ensure_packages($dependencies) causes an error, if a manifest already includes package { 'git': } or package { 'python': } and not ensure_packages(['git']) which sadly is quite common.

@danzilio
Copy link
Member

danzilio commented Sep 8, 2016

$package_name is already a parameter on letsencrypt::install (though, a version with this param has not been pushed yet) and we have a feature flag to disable managing the dependencies in this module if there's a conflict via the $manage_dependencies flag. But, I see your point and I would accept a PR that added a $dependencies param to this class.

@c33s
Copy link
Member Author

c33s commented Sep 8, 2016

i am a little bit confused, i have seen, that there is a parameter for $package_name but it isn't used but it has been used before commit aa9f9ab

i will happily provide a PR for $dependencies, should i wait for your push of the revert to $package_name or should i also include that in my PR?

@ekohl ekohl changed the title Add Package Name & dependencies as parameter Add dependencies as a parameter Dec 4, 2018
@ekohl
Copy link
Member

ekohl commented Dec 4, 2018

We have a parameter for the package name. The workaround for dependencies is to pass in manage_dependencies => false and manage them yourself so I'm going to close this issue. A patch to add the dependencies would be accepted though.

@ekohl ekohl closed this as completed Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants