Merge pull request #33 from chef/SHACK-107/attributes

[SHACK-107] Specify resource attributes on command line

components/chef-workstation/i18n/en.yml

@@ -45,7 +45,7 @@ commands:
             Converge the specified <TARGET> with the single <RESOURCE>. [ATTRIBUTES]
             should be specified as key=value. EG:
 
-            chef target converge myec2node directory /tmp/test mode="0777"
+            chef target converge myec2node directory /tmp/test mode="0777" action=create
 
         ARGS:
           <TARGET>          The host or IP address to converge. Can also be an SSH or WinRM URL
@@ -89,7 +89,7 @@ actions:
 errors:
   # Installer action errors
   CHEFINS001: |
-    %1 is not a supported target operating system at this time.
+    '%1' is not a supported target operating system at this time.
 
     We plan to support a wide range of target operating systems,
     but during this targeted pre-release we are constraining our efforts
@@ -122,6 +122,16 @@ errors:
 
       Available commands are: %2
 
+  # CLI Validation errors are prefixed with CHEFVAL
+  CHEFVAL001: |
+    No identity file at %1
+
+  CHEFVAL002: |
+    You must supply <TARGET>, <RESOURCE> and <RESOURCE_NAME>
+
+  CHEFVAL003: |
+    Attribute '%1' did not match the 'key=value' syntax required
+
   # General errors/unknown errors are handled with CHEFINT
   CHEFINT001: |
     An unexpected error has occurred:
@@ -149,5 +159,3 @@ errors:
     neither:  |
       If you are not able to resolve this issue, please contact Chef support
       at support@chef.io.
-
-

components/chef-workstation/lib/chef-workstation/action/converge_target.rb

@@ -5,19 +5,33 @@ module ChefWorkstation::Action
   class ConvergeTarget < Base
     T = ChefWorkstation::Text.actions.converge_target
 
-    attr_reader :resource_type, :resource_name
+    attr_reader :resource_type, :resource_name, :attributes
     def initialize(config)
       super(config)
       @resource_type = @config.delete :resource_type
       @resource_name = @config.delete :resource_name
+      @attributes = @config.delete(:attributes) || []
     end
 
     def perform_action
-      apply_args = "\"#{@resource_type} '#{@resource_name}'\""
+      apply_args = "\"#{@resource_type} '#{@resource_name}'"
+
+      # lets format the attributes into the correct syntax Chef expects
+      unless attributes.empty?
+        apply_args += " do; "
+        attributes.each do |k, v|
+          v = "\\\"#{v}\\\"" if v.is_a? String
+          apply_args += "#{k} #{v}; "
+        end
+        apply_args += "end\""
+      end
+
+      full_rs_name = "#{resource_type}[#{resource_name}]"
+      ChefWorkstation::Log.debug("Converging #{full_rs_name} with attributes #{attributes}")
+
       c = connection.run_command("#{chef_apply} --no-color -e #{apply_args}")
       if c.exit_status == 0
         ChefWorkstation::Log.debug(c.stdout)
-        full_rs_name = "#{resource_type}[#{resource_name}]"
         reporter.success(T.success(full_rs_name))
       else
         reporter.error(T.error(ChefWorkstation::Log.location))
@@ -34,5 +48,6 @@ def perform_action
         end
       end
     end
+
   end
 end

components/chef-workstation/lib/chef-workstation/command/base.rb

@@ -19,6 +19,7 @@
 require "chef-workstation/config"
 require "chef-workstation/text"
 require "chef-workstation/log"
+require "chef-workstation/error"
 
 module ChefWorkstation
   module Command
@@ -128,6 +129,10 @@ def subcommands
         @command_spec.subcommands
       end
 
+      class OptionValidationError < ChefWorkstation::Error
+        def initialize(id, *args); super(id, *args); end
+      end
+
     end
   end
 end

components/chef-workstation/lib/chef-workstation/command/target/converge.rb

@@ -27,39 +27,95 @@ module Command
     class Target
       class Converge < ChefWorkstation::Command::Base
         T = Text.commands.target.converge
+
         option :root,
           :long => "--[no-]root",
           :description => T.root_description,
           :boolean => true,
           :default => true
 
+        # TODO unique error code, make sure this works with SHACK-105
         option :identity_file,
           :long => "--identity-file PATH",
           :short => "-i PATH",
           :description => T.identity_file,
-          # TODO unique error code, make sure this works with SHACK-105
-          :proc => Proc.new { |path| raise "No identity file at #{path}" unless File.exist?(path) }
+          :proc => (Proc.new do |path|
+            unless File.exist?(path)
+              raise OptionValidationError.new("CHEFVAL001", path)
+            end
+            path
+          end)
 
         def run(params)
+          validate_params(cli_arguments)
           # TODO: option: --no-install
-          target = params.shift
-          resource = params.shift
-          resource_name = params.shift
-          full_rs_name = "#{resource}[#{resource_name}]"
+          target = cli_arguments.shift
+          resource = cli_arguments.shift
+          resource_name = cli_arguments.shift
+          attributes = format_attributes(cli_arguments)
 
           conn = connect(target, { sudo: config[:root], key_file: config[:identity_file] })
           UI::Terminal.spinner(T.status.verifying, prefix: "[#{conn.config[:host]}]") do |r|
             Action::InstallChef.instance_for_target(conn, reporter: r).run
           end
 
+          full_rs_name = "#{resource}[#{resource_name}]"
           UI::Terminal.spinner(T.status.converging(full_rs_name), prefix: "[#{conn.config[:host]}]") do |r|
             converger = Action::ConvergeTarget.new(reporter: r,
                                                    connection: conn,
                                                    resource_type: resource,
-                                                   resource_name: resource_name)
+                                                   resource_name: resource_name,
+                                                   attributes: attributes)
             converger.run
           end
         end
+
+        # TODO raise wrapped errors that get formatted and displayed appropriately IE SHACK-105
+        ATTRIBUTE_MATCHER = /^([a-zA-Z0-9]+)=(\w+)$/
+        def validate_params(params)
+          if params.size < 3
+            raise OptionValidationError.new("CHEFVAL002")
+          end
+          attributes = params[3..-1]
+          attributes.each do |attribute|
+            unless attribute =~ ATTRIBUTE_MATCHER
+              raise OptionValidationError.new("CHEFVAL003", attribute)
+            end
+          end
+        end
+
+        def format_attributes(string_attrs)
+          attributes = {}
+          string_attrs.each do |a|
+            key, value = ATTRIBUTE_MATCHER.match(a)[1..-1]
+            value = transform_attribute_value(value)
+            attributes[key] = value
+          end
+          attributes
+        end
+
+        # Incoming attributes are always read as a string from the command line.
+        # Depending on their type we should transform them so we do not try and pass
+        # a string to a resource attribute that expects an integer or boolean.
+        def transform_attribute_value(value)
+          case value
+          when /^0/
+            # when it is a zero leading value like "0777" don't turn
+            # it into a number (this is a mode flag)
+            value
+          when /\d+/
+            value.to_i
+          when /(^(\d+)(\.)?(\d+)?)|(^(\d+)?(\.)(\d+))/
+            value.to_f
+          when /true/i
+            true
+          when /false/i
+            false
+          else
+            value
+          end
+        end
+
       end
     end
   end

components/chef-workstation/spec/unit/action/converge_target_spec.rb

@@ -11,7 +11,8 @@
   end
   let(:r1) { "directory" }
   let(:r2) { "/tmp" }
-  let(:opts) { { reporter: reporter, connection: connection, resource_type: r1, resource_name: r2 } }
+  let(:attrs) { nil }
+  let(:opts) { { reporter: reporter, connection: connection, resource_type: r1, resource_name: r2, attributes: attrs } }
   subject(:action) { ChefWorkstation::Action::ConvergeTarget.new(opts) }
 
   describe "#initialize" do
@@ -24,18 +25,44 @@
   describe "#perform_action" do
     let(:result) { double("command result", exit_status: 0, stdout: "") }
 
-    before do
-      expect(connection).to receive(:run_command).with(/chef-apply.+#{r1}/).and_return(result)
-    end
-
     it "runs the converge and reports back success" do
+      expect(connection).to receive(:run_command).with(/chef-apply.+#{r1}/).and_return(result)
       expect(reporter).to receive(:success).with(/#{r1}/)
       action.perform_action
     end
 
+    context "when attributes are provided" do
+      let(:attrs) do
+        {
+          "key1" => "value",
+          "key2" => 0.1,
+          "key3" => 100,
+          "key4" => true,
+          "key_with_underscore" => "value",
+        }
+      end
+      it "runs the converge and reports back success" do
+        expect(connection).to receive(:run_command).with(
+          "cmd /c C:/opscode/chef/bin/chef-apply --no-color -e \"directory '/tmp' do; " \
+          "key1 \\\"value\\\"; " \
+          "key2 0.1; " \
+          "key3 100; " \
+          "key4 true; " \
+          "key_with_underscore \\\"value\\\"; " \
+          "end\""
+        ).and_return(result)
+        expect(reporter).to receive(:success).with(/#{r1}/)
+        action.perform_action
+      end
+    end
+
     context "when command fails" do
+      before do
+        expect(connection).to receive(:run_command).with(/chef-apply.+#{r1}/).and_return(result)
+      end
       let(:result) { double("command result", exit_status: 1) }
       let(:stacktrace_result) { double("stacktrace scrape result", exit_status: 0, stdout: "") }
+
       it "scrapes the remote log" do
         expect(reporter).to receive(:error).with(/converge/)
         expect(connection).to receive(:run_command).with(/chef-stacktrace/).and_return(stacktrace_result)

components/chef-workstation/spec/unit/command/target/converge_spec.rb

@@ -0,0 +1,69 @@
+# Copyright:: Copyright (c) 2018 Chef Software Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require "spec_helper"
+require "chef-workstation/commands_map"
+require "chef-workstation/command/target/converge"
+
+RSpec.describe ChefWorkstation::Command::Target::Converge do
+  let(:cmd_spec) { instance_double(ChefWorkstation::CommandsMap::CommandSpec) }
+  subject(:cmd) do
+    ChefWorkstation::Command::Target::Converge.new(cmd_spec)
+  end
+  OptionValidationError = ChefWorkstation::Command::Target::Converge::OptionValidationError
+
+  describe "#validate_params" do
+    it "raises an error if not enough params are specified" do
+      params = [
+        [],
+        %w{one two}
+      ]
+      params.each do |p|
+        expect { cmd.validate_params(p) }.to raise_error(OptionValidationError) do |e|
+          e.id == "CHEFVAL002"
+        end
+      end
+    end
+
+    it "raises an error if attributes are not specified as key value pairs" do
+      params = [
+        %w{one two three four},
+        %w{one two three four=value five six=value},
+        %w{one two three non.word=value},
+      ]
+      params.each do |p|
+        expect { cmd.validate_params(p) }.to raise_error(OptionValidationError) do |e|
+          e.id == "CHEFVAL003"
+        end
+      end
+    end
+  end
+
+  describe "#format_attributes" do
+    it "parses attributes into a hash" do
+      provided = %w{key1=value key2=1 key3=true key4=FaLsE key5=0777}
+      expected = {
+        "key1" => "value",
+        "key2" => 1,
+        "key3" => true,
+        "key4" => false,
+        "key5" => "0777"
+      }
+      expect(cmd.format_attributes(provided)).to eq(expected)
+    end
+
+  end
+end