(puppetlabsGH-1162) Expand configured paths relative to Boltdir

There are a number of config options for Bolt that accept filepaths,
both on the CLI and in the config file. 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 not already 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

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`

lib/bolt/bolt_option_parser.rb

@@ -372,7 +372,7 @@ def initialize(options)
           @options[:password] = password
         end
       end
-      define('--private-key KEY', 'Private ssh key to authenticate with') do |key|
+      define('--private-key KEY', 'Path to the private SSH key to authenticate with') do |key|
         @options[:'private-key'] = key
       end
       define('--[no-]host-key-check', 'Check host keys with SSH') do |host_key_check|
@@ -412,10 +412,7 @@ def initialize(options)
       define('-m', '--modulepath MODULES',
              "List of directories containing modules, separated by '#{File::PATH_SEPARATOR}'",
              'Directories are case-sensitive') do |modulepath|
-        # When specified from the CLI, modulepath entries are relative to pwd
-        @options[:modulepath] = modulepath.split(File::PATH_SEPARATOR).map do |moduledir|
-          File.expand_path(moduledir)
-        end
+        @options[:modulepath] = modulepath
       end
       define('--boltdir FILEPATH',
              'Specify what Boltdir to load config from (default: autodiscovered from current working dir)') do |path|
@@ -430,7 +427,7 @@ def initialize(options)
         if ENV.include?(Bolt::Inventory::ENVIRONMENT_VAR)
           raise Bolt::CLIError, "Cannot pass inventory file when #{Bolt::Inventory::ENVIRONMENT_VAR} is set"
         end
-        @options[:inventoryfile] = File.expand_path(path)
+        @options[:inventoryfile] = path
       end
       define('--[no-]save-rerun', 'Whether to update the rerun file after this command.') do |save|
         @options[:'save-rerun'] = save

lib/bolt/config.rb

@@ -173,6 +173,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[tmpdir 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']
@@ -183,8 +190,24 @@ def update_from_file(data)
     private :update_from_file
 
     def apply_overrides(options)
-      %i[concurrency transport format trace modulepath inventoryfile color].each do |key|
-        send("#{key}=", options[key]) if options.key?(key)
+      # Can we use this future instead of $future?
+      if @future
+        # Expand modulepaths and inventoryfile path relative to Boltdir
+        if options[:modulepath]
+          @modulepath = options[:modulepath].split(File::PATH_SEPARATOR).map do |moduledir|
+            File.expand_path(moduledir, @boltdir.path)
+          end
+        end
+
+        @inventoryfile = File.expand_path(options[:inventoryfile], @boltdir.path) if options[:inventoryfile]
+
+        %i[concurrency transport format trace color].each do |key|
+          send("#{key}=", options[key]) if options.key?(key)
+        end
+      else
+        %i[concurrency transport format trace modulepath inventoryfile color].each do |key|
+          send("#{key}=", options[key]) if options.key?(key)
+        end
       end
 
       @save_rerun = options[:'save-rerun'] if options.key?(:'save-rerun')
@@ -198,9 +221,12 @@ def apply_overrides(options)
       TRANSPORTS.each_key do |transport|
         transport = @transports[transport]
         TRANSPORT_OPTIONS.each do |key|
-          if options[key]
-            transport[key.to_s] = Bolt::Util.walk_keys(options[key], &:to_s)
-          end
+          next unless options[key]
+          transport[key.to_s] = if %i[tmpdir private-key cacert token-file].include?(key) && @future
+                                  File.expand_path(options[key], @boltdir.path)
+                                else
+                                  Bolt::Util.walk_keys(options[key], &:to_s)
+                                end
         end
       end
 

spec/bolt/cli_spec.rb

@@ -496,18 +496,18 @@ def stub_config(file_content = {})
     end
 
     describe "modulepath" do
-      it "treats relative modulepath as relative to pwd" do
+      it "returns modulepath without expanding" do
         site = File.expand_path('site')
         modulepath = [site, 'modules'].join(File::PATH_SEPARATOR)
         cli = Bolt::CLI.new(%W[command run uptime --modulepath #{modulepath} --nodes foo])
-        expect(cli.parse).to include(modulepath: [site, File.expand_path('modules')])
+        expect(cli.parse).to include(modulepath: modulepath)
       end
 
       it "accepts shorthand -m" do
         site = File.expand_path('site')
         modulepath = [site, 'modules'].join(File::PATH_SEPARATOR)
         cli = Bolt::CLI.new(%W[command run uptime -m #{modulepath} --nodes foo])
-        expect(cli.parse).to include(modulepath: [site, File.expand_path('modules')])
+        expect(cli.parse).to include(modulepath: modulepath)
       end
 
       it "generates an error message if no value is given" do
@@ -2118,4 +2118,29 @@ def stub_config(file_content = {})
       end
     end
   end
+
+  describe 'with future set' do
+    let(:configdir) { File.join(__dir__, '..', 'fixtures', 'configs') }
+
+    it 'expands CLI flag paths relative to the Boltdir' do
+      cli = Bolt::CLI.new(%W[command run uptime
+                             --configfile #{File.join(configdir, 'future.yml')}
+                             --nodes foo
+                             --private-key .ssh/private
+                             --inventoryfile ../inventory/empty.yml
+                             --tmpdir tmp/erature
+                             --modulepath mod:ule/pat/h])
+      cli.parse
+      expect(cli.config.transports[:ssh]['private-key'])
+        .to eq(File.expand_path('.ssh/private', cli.config.boltdir.path))
+      expect(cli.config.transports[:ssh]['tmpdir'])
+        .to eq(File.expand_path('tmp/erature', cli.config.boltdir.path))
+      expect(cli.config.inventoryfile)
+        .to eq(File.expand_path('../inventory/empty.yml', cli.config.boltdir.path))
+      mod_path = File.expand_path('mod', cli.config.boltdir.path)
+      ule_path = File.expand_path('ule/pat/h', cli.config.boltdir.path)
+      expect(cli.config.modulepath)
+        .to eq([mod_path, ule_path])
+    end
+  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 local tmpdir relative to boltdir" do
+      data = {
+        'local' => { 'tmpdir' => 'tmp/orary' },
+        'future' => true
+      }
+
+      config = Bolt::Config.new(boltdir, data)
+      expect(config.transports[:local]['tmpdir'])
+        .to eq(File.expand_path('tmp/orary', 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

@@ -687,7 +687,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/fixtures/configs/future.yml

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