From b2286388f085135cfdd4a91f4a6d0c2676875050 Mon Sep 17 00:00:00 2001 From: Gilles Cornu Date: Wed, 20 Apr 2016 22:27:55 +0200 Subject: [PATCH] provisioners/ansible: add basic config validators With this change, the `raw_arguments` and `raw_ssh_args` options are: - STILL automatically converted as an Array when they are set a String (no behaviour change) - rejected if they are not of Array data type otherwise Additional Notes: - the 'as_array' tiny helper has been removed since it was no longer used. - there is for now no deeper validation (i.e. verifying that the Array elements are only *String* objects) --- plugins/provisioners/ansible/config/base.rb | 10 ++++- plugins/provisioners/ansible/config/host.rb | 11 +++++ plugins/provisioners/ansible/helpers.rb | 4 -- .../provisioners/ansible/provisioner/host.rb | 2 +- templates/locales/en.yml | 4 ++ .../provisioners/ansible/config_test.rb | 40 +++++++++++++++++++ 6 files changed, 65 insertions(+), 6 deletions(-) diff --git a/plugins/provisioners/ansible/config/base.rb b/plugins/provisioners/ansible/config/base.rb index d6881c70c..00b3f5072 100644 --- a/plugins/provisioners/ansible/config/base.rb +++ b/plugins/provisioners/ansible/config/base.rb @@ -113,8 +113,16 @@ module VagrantPlugins end if raw_arguments - @raw_arguments = Helpers::as_array(raw_arguments) + if raw_arguments.kind_of?(String) + @raw_arguments = [raw_arguments] + elsif !raw_arguments.kind_of?(Array) + @errors << I18n.t( + "vagrant.provisioners.ansible.errors.raw_arguments_invalid", + type: raw_arguments.class.to_s, + value: raw_arguments.to_s) + end end + end end end diff --git a/plugins/provisioners/ansible/config/host.rb b/plugins/provisioners/ansible/config/host.rb index 8f9433c63..04d2fac10 100644 --- a/plugins/provisioners/ansible/config/host.rb +++ b/plugins/provisioners/ansible/config/host.rb @@ -34,6 +34,17 @@ module VagrantPlugins def validate(machine) super + if raw_ssh_args + if raw_ssh_args.kind_of?(String) + @raw_ssh_args = [raw_ssh_args] + elsif !raw_ssh_args.kind_of?(Array) + @errors << I18n.t( + "vagrant.provisioners.ansible.errors.raw_ssh_args_invalid", + type: raw_ssh_args.class.to_s, + value: raw_ssh_args.to_s) + end + end + { "ansible remote provisioner" => @errors } end diff --git a/plugins/provisioners/ansible/helpers.rb b/plugins/provisioners/ansible/helpers.rb index 24534c260..ce189b6d2 100644 --- a/plugins/provisioners/ansible/helpers.rb +++ b/plugins/provisioners/ansible/helpers.rb @@ -12,10 +12,6 @@ module VagrantPlugins def self.as_list_argument(v) v.kind_of?(Array) ? v.join(',') : v end - - def self.as_array(v) - v.kind_of?(Array) ? v : [v] - end end end end \ No newline at end of file diff --git a/plugins/provisioners/ansible/provisioner/host.rb b/plugins/provisioners/ansible/provisioner/host.rb index 2ff957bc8..1eca11d45 100644 --- a/plugins/provisioners/ansible/provisioner/host.rb +++ b/plugins/provisioners/ansible/provisioner/host.rb @@ -243,7 +243,7 @@ module VagrantPlugins ssh_options << "-o ForwardAgent=yes" if @ssh_info[:forward_agent] # Unchecked SSH Parameters - ssh_options.concat(Helpers::as_array(config.raw_ssh_args)) if config.raw_ssh_args + ssh_options.concat(config.raw_ssh_args) if config.raw_ssh_args # Re-enable ControlPersist Ansible defaults, # which are lost when ANSIBLE_SSH_ARGS is defined. diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 3ca78008d..5fd667f48 100755 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2121,6 +2121,10 @@ en: for more information. extra_vars_invalid: |- `extra_vars` must be a hash or a path to an existing file. Received: %{value} (as %{type}) + raw_arguments_invalid: |- + `raw_arguments` must be an array of strings. Received: %{value} (as %{type}) + raw_ssh_args_invalid: |- + `raw_ssh_args` must be an array of strings. Received: %{value} (as %{type}) galaxy_role_file_invalid: |- `galaxy_role_file` does not exist on the %{system}: %{path} inventory_path_invalid: |- diff --git a/test/unit/plugins/provisioners/ansible/config_test.rb b/test/unit/plugins/provisioners/ansible/config_test.rb index f836382a3..5ba9d638d 100644 --- a/test/unit/plugins/provisioners/ansible/config_test.rb +++ b/test/unit/plugins/provisioners/ansible/config_test.rb @@ -200,6 +200,46 @@ describe VagrantPlugins::Ansible::Config::Host do ]) end + it "returns an error if the raw_arguments is of the wrong data type" do + subject.raw_arguments = { arg1: 1, arg2: "foo" } + subject.finalize! + + result = subject.validate(machine) + expect(result["ansible remote provisioner"]).to eql([ + I18n.t("vagrant.provisioners.ansible.errors.raw_arguments_invalid", + type: subject.raw_arguments.class.to_s, + value: subject.raw_arguments.to_s) + ]) + end + + it "converts a raw_arguments option defined as a String into an Array" do + subject.raw_arguments = "--foo=bar" + subject.finalize! + + result = subject.validate(machine) + expect(subject.raw_arguments).to eql(%w(--foo=bar)) + end + + it "returns an error if the raw_ssh_args is of the wrong data type" do + subject.raw_ssh_args = { arg1: 1, arg2: "foo" } + subject.finalize! + + result = subject.validate(machine) + expect(result["ansible remote provisioner"]).to eql([ + I18n.t("vagrant.provisioners.ansible.errors.raw_ssh_args_invalid", + type: subject.raw_ssh_args.class.to_s, + value: subject.raw_ssh_args.to_s) + ]) + end + + it "converts a raw_ssh_args option defined as a String into an Array" do + subject.raw_arguments = "-o ControlMaster=no" + subject.finalize! + + result = subject.validate(machine) + expect(subject.raw_arguments).to eql(["-o ControlMaster=no"]) + end + it "it collects and returns all detected errors" do subject.playbook = non_existing_file subject.inventory_path = non_existing_file