added bash completion for Ubuntu and Debian - others untested

[jardleex]

Hi,

this aims to solve #407.

I'm open to any improvements you'd like to have for this.
Could test it only on Ubuntu and Debian, thus set $bash_completion only there on true.
If the user uses a package installation (like we do) he has to adjust $bin_dir, depending on what package he uses. The package from HashiCorp (which we use) does not include bash completion.

Best regards

Jard

[bastelfreak]

I'm wondering if this directory is correct? shouldn't it go into /etc/profile.d/? That works on Ubuntu/Debian/Arch/RedHat (I haven't tested others).

[jardleex]

Hi @bastelfreak

thanks for the input.

The package bash-completion provides /etc/profile.d/bash_completion.sh which calls /usr/share/bash-completion/bash_completion. The later one then checks /etc/bash_completion.d/ for any completion scripts.
The drawback I found is that /etc/bash_completion.d/ is not created by the package bash-completion (at least version 2.8-6 which we run). Other packages do so like git, unp or what so ever.

If the completion should get realized via /etc/profile.d/ another file content has to be used.
How did you deploy such completions in your landcape?

[bastelfreak]

ah you're right. /etc/profile.d doesn't work here because complete is a bash builtin. the code contains stuff like compat_dir=${BASH_COMPLETION_COMPAT_DIR:-/etc/bash_completion.d} so I Ithink /etc/bash_completion.d/ is indeed correct and my initial comment was just stupid. I think /etc/bash_completion.d should be configureable in case people have set BASH_COMPLETION_COMPAT_DIR. Those people will want to set the same path within this puppet module.

[bastelfreak]

and maybe we should have:

file { '/etc/bash_completion.d/`:
  ensure => 'directory',
}

because it doesn't exist by default on all platforms

[jardleex]

$bash_completion_compat_dir has been added together with the ensure block.

If this works on other os variants we may spread $bash_completion = true. Unfortunately I have no other distribution available to test this.

[solarkennedy]

Are you sure this would be better to just put in the OS package directly?

[solarkennedy]

Are you sure this would be better to just put in the OS package directly?

For example, this debian package appears to have the bash_completion file included:
https://packages.debian.org/sid/amd64/consul/filelist

[jardleex]

Having it inside the package would be great, yes.

We are using the official packages from HashiCorp themselves in order to have the latest version available. In there no completion files are included. So I came up with the idea to let Puppet do this.

I'm not sure how successful a change request in Consul itself would be to integrate a completion. It seems they kept the official package pretty tight to make it run or start at least on the majority of systems.

This request allows to control the setup of the completion files.
Would you prefer disable the completion file per default via $bash_completion = false?

[bastelfreak]

please don't add the value here, instead directly in the init.pp. that enabled puppet-strings to pickup the default value.

[jardleex]

Thanks for the feedback. I hope I got you right with this?

diff --git a/REFERENCE.md b/REFERENCE.md
index ffaf8db..e93022a 100644
--- a/REFERENCE.md
+++ b/REFERENCE.md
@@ -222,7 +222,7 @@ Data type: `String[1]`
 
 Directory to place bash-completion file for Consul into.
 
-Default value: `$consul::params::bash_completion_compat_dir`
+Default value: `'/etc/bash_completion.d/'`
 
 ##### <a name="-consul--bin_dir"></a>`bin_dir`
 
diff --git a/manifests/init.pp b/manifests/init.pp
index 20586b2..95fb6ec 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -98,7 +98,7 @@ class consul (
   String[1]                             $arch                        = $consul::params::arch,
   Optional[Stdlib::Absolutepath]        $archive_path                = undef,
   Boolean                               $bash_completion             = $consul::params::bash_completion,
-  String[1]                             $bash_completion_compat_dir  = $consul::params::bash_completion_compat_dir,
+  String[1]                             $bash_completion_compat_dir  = '/etc/bash_completion.d/',
   Stdlib::Absolutepath                  $bin_dir                     = $consul::params::bin_dir,
   Optional[String[1]]                   $binary_group                = $consul::params::binary_group,
   String[1]                             $binary_mode                 = $consul::params::binary_mode,
diff --git a/manifests/params.pp b/manifests/params.pp
index 3d3f6ed..bcabcb2 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -47,7 +47,6 @@ class consul::params {
     default: {
       # 0 instead of root because OS X uses "wheel".
       $data_dir_mode = '0755'
-      $bash_completion_compat_dir = '/etc/bash_completion.d/'
       $binary_group = '0'
       $binary_mode = '0555'
       $binary_name = 'consul'