(puppetlabsGH-1162) Expand configured paths relative to Boltdir

There are a number of config options for Bolt that accept filepaths.
Previously we expanded the path relative to where Bolt was run in some
cases, and relative to the Boltdir in others. This standardizes all
configurable paths to be expanded relative to the Boltdir if they are
defined in a config file, and relative to the current working directory
if they are specified on the CLI. This change doesn't affect absolute
paths.  This provides consistent and predictable behavior for users. New
path expansions are gated on the `future` option being set to `true`.

Closes puppetlabs#1162

CHANGELOG.md

@@ -22,6 +22,12 @@
 
   When a CLI option that can be overridden in inventory is provided and inventory is loaded from a file or from the BOLT_INVENTORY environment variable an issue is warned about which options could be overridden.
 
+### Bug fixes
+
+* **Standardize configured paths to be relative to Boltdir** ([#1162](https://github.com/puppetlabs/bolt/issues/1162))
+
+  Previously we expanded some configured paths relative to the directory Bolt was run from, and other paths were expanded relative to the Boltdir. This standardizes all configured paths, including the modulepath, to be relative to the Boltdir. This only applies to file-based config, not command line flags, which are expanded relative to CWD. This fix is gated on the `future` config option, and will be available by default in Bolt 2.0
+
 ## 1.35.0
 
 ### Deprecation

Puppetfile

@@ -30,9 +30,9 @@ mod 'puppetlabs-ruby_task_helper', '0.4.0'
 
 # Plugin modules
 mod 'puppetlabs-azure_inventory', '0.2.0'
-mod 'puppetlabs-terraform', '0.1.0'
+mod 'puppetlabs-terraform', '0.2.0'
 mod 'puppetlabs-vault', '0.2.1'
-mod 'puppetlabs-aws_inventory', '0.1.0'
+mod 'puppetlabs-aws_inventory', '0.2.0'
 
 # If we don't list these modules explicitly, r10k will purge them
 mod 'canary', local: true

documentation/bolt_configuration_options.md

@@ -45,7 +45,8 @@ ssh:
 -   `host-key-check`: Whether to perform host key validation when connecting over SSH. Default is `true`.
 -   `password`: Login password.
 -   `port`: Connection port. Default is `22`.
--   `private-key`: The path to the private key file to use for SSH authentication.
+-   `private-key`: Either the path to the private key file to use for SSH authentication, or a hash
+    with key `key-data` and the contents of the private key.
 -   `proxyjump`: A jump host to proxy SSH connections through, and an optional user to connect with, for example: jump.example.com or user1@jump.example.com.
 -   `run-as`: A different user to run commands as after login.
 -   `run-as-command`: The command to elevate permissions. Bolt appends the user and command strings to the configured run as a command before running it on the target. This command must not require an interactive password prompt, and the `sudo-password` option is ignored when `run-as-command` is specified. The run-as command must be specified as an array.
@@ -54,6 +55,22 @@ ssh:
 -   `tty`: Request a pseudo tty for the SSH session. This option is generally only used in conjunction with the `run_as` option when the sudoers policy requires a `tty`. Default is `false`.
 -   `user`: Login user. Default is `root`.
 
+For example:
+
+```yaml
+targets:
+  - name: host1.example.net
+    config:
+      transport: ssh
+      ssh:
+        host-key-check: true
+        port: 22
+        run-as-command: ['sudo', '-k', '-n']
+        private-key:
+          key-data: |
+            MY PRIVATE KEY CONTENT
+```
+
 
 ## OpenSSH configuration options
 
@@ -83,14 +100,14 @@ To illustrate, consider this example:
 `inventory.yaml`
 
 ```yaml
-nodes:
+targets:
   - name: host1.example.net
     config:
       transport: ssh
       ssh:
         host-key-check: true
         port: 22
-        private-key: /.ssh/id_rsa-example
+        private-key: ~/.ssh/id_rsa-example
 ```
 
 `ssh.config`

documentation/configuring_bolt.md

@@ -2,7 +2,14 @@
 
 Create a configuration file to store and automate the command-line flags you use every time you run Bolt.
 
-Configuration for Bolt is loaded from the [Bolt project directory](bolt_project_directories.md#). To set up a configuration file for Bolt to use outside of a project directory, create a `~/.puppetlabs/bolt/bolt.yaml` file with global options at the top level of the file. Configure transport-specific options for each transport. If a config option is set in the config file and passed with the corresponding command-line flag, the flag takes precedence.
+Configuration for Bolt is loaded from the [Bolt project directory](bolt_project_directories.md#). The default project directory is `~/.puppetlabs/bolt/`. Configure global options (i.e. the modulepath) at the top level of `<boltdir>/bolt.yaml`, and configure transport-specific options for each transport.
+
+Bolt config uses the following precedence, from highest precedence (cannot be overridden) to lowest:
+- Target URI (i.e. ssh://user:password@hostname:port)
+- [Inventory file](inventory_file.md) options
+- Command line flags
+- Config file options
+- SSH config file options (`~/.ssh/config`, if using SSH)
 
 -   **[Project directories](bolt_project_directories.md#)**  
  Bolt runs in the context of a project directory or a `Boltdir`. This directory contains all of the configuration, code, and data loaded by Bolt.

documentation/using_plugins.md

@@ -271,8 +271,8 @@ plugins:
 ```
 
 -   `keysize`: They size of the key to generate with `bolt secret createkeys`: default: `2048`
--   `private-key`: The path to the private key file. default: `keys/private_key.pkcs7.pem`
--   `public-key`: The path to the public key file. default: `keys/public_key.pkcs7.pem`
+-   `private-key`: The path to the private key file. default: `<boltdir>/keys/private_key.pkcs7.pem`
+-   `public-key`: The path to the public key file. default: `<boltdir>/keys/public_key.pkcs7.pem`
 
 ## Vault plugin
 

lib/bolt/cli.rb

@@ -247,7 +247,7 @@ def handle_parser_errors
 
     def puppetdb_client
       return @puppetdb_client if @puppetdb_client
-      puppetdb_config = Bolt::PuppetDB::Config.load_config(nil, config.puppetdb)
+      puppetdb_config = Bolt::PuppetDB::Config.load_config(nil, config.puppetdb, config.boltdir.path)
       @puppetdb_client = Bolt::PuppetDB::Client.new(puppetdb_config)
     end
 

lib/bolt/config.rb

@@ -110,7 +110,11 @@ def normalize_interpreters(interpreters)
     def normalize_log(target)
       return target if target == 'console'
       target = target[5..-1] if target.start_with?('file:')
-      'file:' + File.expand_path(target)
+      if @future
+        'file:' + File.expand_path(target, @boltdir.path)
+      else
+        'file:' + File.expand_path(target)
+      end
     end
 
     def update_logs(logs)
@@ -173,6 +177,13 @@ def update_from_file(data)
       TRANSPORTS.each do |key, impl|
         if data[key.to_s]
           selected = impl.filter_options(data[key.to_s])
+          if @future
+            to_expand = %w[private-key cacert token-file] & selected.keys
+            to_expand.each do |opt|
+              selected[opt] = File.expand_path(selected[opt], @boltdir.path) if opt.is_a?(String)
+            end
+          end
+
           @transports[key] = Bolt::Util.deep_merge(@transports[key], selected)
         end
         if @transports[key]['interpreters']

lib/bolt/puppetdb/config.rb

@@ -19,7 +19,7 @@ class Config
 
       end
 
-      def self.load_config(filename, options)
+      def self.load_config(filename, options, boltdir_path = nil)
         config = {}
         global_path = Bolt::Util.windows? ? DEFAULT_CONFIG[:win_global] : DEFAULT_CONFIG[:global]
         if filename
@@ -43,11 +43,12 @@ def self.load_config(filename, options)
         end
 
         config = config.fetch('puppetdb', {})
-        new(config.merge(options))
+        new(config.merge(options), boltdir_path)
       end
 
-      def initialize(settings)
+      def initialize(settings, boltdir_path = nil)
         @settings = settings
+        @boltdir_path = boltdir_path
         expand_paths
       end
 
@@ -66,7 +67,14 @@ def token
 
       def expand_paths
         %w[cacert cert key token].each do |file|
-          @settings[file] = File.expand_path(@settings[file]) if @settings[file]
+          next unless @settings[file]
+          # rubocop:disable Style/GlobalVars
+          @settings[file] = if $future && @boltdir_path
+                              File.expand_path(@settings[file], @boltdir_path)
+                            else
+                              File.expand_path(@settings[file])
+                            end
+          # rubocop:enable Style/GlobalVars
         end
       end
 

spec/bolt/config_spec.rb

@@ -169,6 +169,15 @@
       expect { Bolt::Config.new(boltdir, config) }.not_to raise_error
     end
 
+    it "expands the private-key hash with 'future' set" do
+      data = {
+        'ssh' => { 'private-key' => 'my-private-key' },
+        'future' => true
+      }
+      config = Bolt::Config.new(boltdir, data)
+      expect(config.transports[:ssh]['private-key']).to eq(File.expand_path('my-private-key', boltdir.path))
+    end
+
     it "does not accept a private-key hash without data" do
       config = {
         'ssh' => { 'private-key' => { 'not-data' => "foo" } }
@@ -232,4 +241,55 @@
       expect { Bolt::Config.new(boltdir, config) }.not_to raise_error
     end
   end
+
+  describe 'expanding paths' do
+    it "expands cacert relative to boltdir" do
+      expect(Bolt::Util)
+        .to receive(:validate_file)
+        .with('cacert', File.expand_path('ssl/ca.pem', boltdir.path))
+        .and_return(true)
+
+      data = {
+        'winrm' => { 'ssl' => true, 'cacert' => 'ssl/ca.pem' },
+        'future' => true
+      }
+
+      config = Bolt::Config.new(boltdir, data)
+      expect(config.transports[:winrm]['cacert'])
+        .to eq(File.expand_path('ssl/ca.pem', boltdir.path))
+    end
+
+    it "expands token-file relative to boltdir" do
+      data = {
+        'pcp' => { 'token-file' => 'token' },
+        'future' => true
+      }
+
+      config = Bolt::Config.new(boltdir, data)
+      expect(config.transports[:pcp]['token-file'])
+        .to eq(File.expand_path('token', boltdir.path))
+    end
+
+    it "expands private-key relative to boltdir" do
+      data = {
+        'ssh' => { 'private-key' => 'secret/key' },
+        'future' => true
+      }
+
+      config = Bolt::Config.new(boltdir, data)
+      expect(config.transports[:ssh]['private-key'])
+        .to eq(File.expand_path('secret/key', boltdir.path))
+    end
+
+    it "expands inventoryfile relative to boltdir" do
+      data = {
+        'inventoryfile' => 'targets.yml',
+        'future' => true
+      }
+
+      config = Bolt::Config.new(boltdir, data)
+      expect(config.inventoryfile)
+        .to eq(File.expand_path('targets.yml', boltdir.path))
+    end
+  end
 end

spec/bolt/inventory_spec.rb

@@ -701,7 +701,7 @@ def common_data(transport)
   end
 
   describe 'add_facts' do
-    context 'whith and without $future flag' do
+    context 'with and without $future flag' do
       let(:inventory) { Bolt::Inventory.new({}) }
       let(:target) { get_target(inventory, 'foo') }
       let(:facts) { { 'foo' => 'bar' } }

spec/bolt/puppetdb/config_spec.rb

@@ -5,6 +5,27 @@
 require 'bolt/util'
 
 describe Bolt::PuppetDB::Config do
+  context "with boltdir available" do
+    let(:cacert) { File.expand_path('relative/to/cacert') }
+    let(:token) { File.expand_path('relative/to/token') }
+    let(:boltdir) { '~/dirbolt' }
+    let(:options) do
+      {
+        'server_urls' => ['https://puppetdb:8081'],
+        'cacert' => cacert,
+        'token' => token
+      }
+    end
+
+    let(:config) { Bolt::PuppetDB::Config.new(options, boltdir) }
+
+    it 'expands the cacert relative to the boltdir if boltdir is available' do
+      allow(config).to receive(:validate_file_exists).with('cacert').and_return true
+
+      expect(config.cacert).to eq(File.expand_path(cacert, boltdir))
+    end
+  end
+
   context "when validating that options" do
     let(:cacert) { File.expand_path('/path/to/cacert') }
     let(:token) { File.expand_path('/path/to/token') }

spec/fixtures/configs/future.yml

@@ -0,0 +1 @@
+future: true