Skip to content
This repository has been archived by the owner on Jun 4, 2021. It is now read-only.

Commit

Permalink
Better handling of empty boolean arguments
Browse files Browse the repository at this point in the history
Given this example: `razor create-policy ... --enabled --hostname
'host${id}.com' ...`

Razor is thinking `--hostname` is the value for `--enabled`, and
`'host${id}.com'` is then treated as a positional argument.

This makes two changes which should fix this issue:
- Boolean types must now be `"true"`, `"false"`, or `nil`/empty. The
  latter-most evaluates to `"true"`.
- Any value for a boolean type starting with two dashes should be
  counted as the next argument.

Fixes https://tickets.puppetlabs.com/browse/RAZOR-1114
  • Loading branch information
smcclellan authored and branan committed Aug 28, 2019
1 parent 544b506 commit 81c2a69
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
6 changes: 4 additions & 2 deletions lib/razor/cli/command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def extract_command
# `--arg=value`/`--arg value`
# `-a=value`/`-a value`
arg_name, value = [$1, $3]
value = @segments.shift if value.nil? && @segments[0] !~ /^-[a-z]/
value = @segments.shift if value.nil? && @segments[0] !~ /^-[-]?[a-z]/
arg_name = self.class.resolve_alias(arg_name, @cmd_schema)
body[arg_name] = self.class.convert_arg(arg_name, value, body[arg_name], @cmd_schema)
elsif argument =~ /\A-([a-z][a-z_-]+)(=(.+))?\Z/ and
Expand Down Expand Up @@ -127,7 +127,9 @@ def self.convert_arg(arg_name, value, existing_value, cmd_schema)
{argument_name: arg_name, error: error.message}
end
when "boolean"
["true", nil].include?(value)
raise ArgumentError, _("Invalid boolean for argument '%{argument_name}': %{value}") %
{argument_name: arg_name, value: value} unless ["true", "false", nil].include?(value)
value != "false"
when "number"
begin
Integer(value)
Expand Down
13 changes: 13 additions & 0 deletions spec/cli/command_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ def require_relative(path)
to raise_error(ArgumentError, /Expected nothing for argument 'tags', but was: 'abc'/)
end
end
context "boolean conversion" do
it "errors when anything besides 'true', 'false' or nil are provided" do
cmd_schema = {"enabled"=>{"type"=>"boolean"}}
expect{Command.convert_arg('enabled', 'abc', existing_value, cmd_schema)}.
to raise_error(ArgumentError, /Invalid boolean for argument 'enabled': abc/)
expect(Command.convert_arg('enabled', 'true', existing_value, cmd_schema)).to eq(true)
expect(Command.convert_arg('enabled', nil, existing_value, cmd_schema)).to eq(true)
expect(Command.convert_arg('enabled', 'false', existing_value, cmd_schema)).to eq(false)
end
end
end

context "resolve_alias" do
Expand Down Expand Up @@ -104,6 +114,9 @@ def extract(schema, run_array)
it "succeeds with a double dash for long flags" do
extract({'schema' => {'name' => {'type' => 'array'}}},
['--name', 'abc'])['name'].should == ['abc']
extract({'schema' => {'true' => {'type' => 'boolean'},
'other' => {'type' => 'number'}}},
['--true', '--other', '123'])['true'].should be_true
end
it "succeeds with a single dash for short flags" do
c = Razor::CLI::Command.new(nil, nil, {'schema' => {'n' => {'type' => 'array'}}},
Expand Down

0 comments on commit 81c2a69

Please sign in to comment.