diff --git a/lib/spanner/config.ex b/lib/spanner/config.ex index 93d6461..884738a 100644 --- a/lib/spanner/config.ex +++ b/lib/spanner/config.ex @@ -1,14 +1,13 @@ defmodule Spanner.Config do alias Spanner.Config.SyntaxValidator alias Spanner.Config.SemanticValidator - alias Spanner.Config.Upgrader # Currently we support version n and n-1. Adding new optional fields # does not require a version bump, but adding new mandatory fields, # or changing the overall structure of the configuration file does # require a bump - @current_config_version 3 + @current_config_version 4 @old_config_version @current_config_version - 1 @config_extensions [".yaml", ".yml", ".json"] @config_file "config" @@ -61,7 +60,8 @@ defmodule Spanner.Config do """ @spec validate(Map.t) :: {:ok, Map.t} | {:error, List.t, List.t} | {:warning, Map.t, List.t} - def validate(%{"cog_bundle_version" => @current_config_version}=config) do + def validate(%{"cog_bundle_version" => version}=config) + when version >= @old_config_version do case SyntaxValidator.validate(config) do :ok -> config = fixup_rules(config) @@ -75,24 +75,6 @@ defmodule Spanner.Config do {:error, errors, []} end end - def validate(%{"cog_bundle_version" => @old_config_version}=config) do - # Upgrader will return an upgraded config and a list of warnings - # or an error - case Upgrader.upgrade(config) do - {:ok, upgraded_config, warnings} -> - # We still need to validate the upgraded config - case validate(upgraded_config) do - {:ok, validated_config} -> - # If everything goes well, we return the validated config - # and a list of warnings. - {:warning, validated_config, warnings} - {:error, errors, _} -> - {:error, errors, warnings} - end - {:error, errors, warnings} -> - {:error, errors, warnings} - end - end def validate(%{"cog_bundle_version" => version}) do {:error, [{""" diff --git a/lib/spanner/config/syntax_validator.ex b/lib/spanner/config/syntax_validator.ex index 431019a..5366ac3 100644 --- a/lib/spanner/config/syntax_validator.ex +++ b/lib/spanner/config/syntax_validator.ex @@ -19,9 +19,10 @@ defmodule Spanner.Config.SyntaxValidator do @doc """ Accepts a config map and validates syntax. """ - @spec validate(Map.t, integer()) :: :ok | {:error, [{String.t, String.t}]} - def validate(config, version \\ @current_config_version) do - with {:ok, schema} <- load_schema(version), + @spec validate(Map.t) :: :ok | {:error, [{String.t, String.t}]} + def validate(config) do + with version <- Map.fetch!(config, "cog_bundle_version"), + {:ok, schema} <- load_schema(version), {:ok, resolved_schema} <- resolve_schema(schema), :ok <- ExJsonSchema.Validator.validate(resolved_schema, config), do: :ok diff --git a/lib/spanner/config/upgrader.ex b/lib/spanner/config/upgrader.ex deleted file mode 100644 index 2572920..0000000 --- a/lib/spanner/config/upgrader.ex +++ /dev/null @@ -1,119 +0,0 @@ -defmodule Spanner.Config.Upgrader do - alias Spanner.Config.SyntaxValidator - - @moduledoc """ - Attempts to upgrade bundle config to current version. - """ - - @current_version Spanner.Config.current_config_version - @upgradable_version @current_version - 1 - - @doc """ - When possible upgrades config to the current version. Returns the upgraded - config and a list of warnings for deprecated config options or an error if - the upgrade fails. - """ - @spec upgrade(Map.t) :: {:ok, Map.t, List.t} | {:error, List.t} - def upgrade(%{"cog_bundle_version" => @upgradable_version}=config) do - deprecation_msg = - {""" - Bundle config version #{@upgradable_version} has been deprecated. \ - Please update to version #{@current_version}.\ - """, - "#/cog_bundle_version"} - # We run the validator for the old version here. So if the user passes an - # old version that is also invalid, we don't crash trying to access fields - # that don't exist. - case SyntaxValidator.validate(config, @upgradable_version) do - :ok -> - do_upgrade(config) - |> insert_deprecation_msg(deprecation_msg) - {:error, errors} -> - {:error, errors, [deprecation_msg]} - end - end - - defp do_upgrade(config) do - case execution_once?(config["commands"]) do - {true, {warnings, errors}} -> - {:error, errors, warnings} - {false, warnings} -> - {enforce_warnings, updated_commands} = update_enforcing(config["commands"]) - updated_config = %{config | "commands" => updated_commands} - |> Map.put("cog_bundle_version", @current_version) - {:ok, updated_config, warnings ++ enforce_warnings} - end - end - - # Checks to see if the execution field exists in the bundle config. If it - # does but contains "multiple" we just return a warning. If it contains - # "once" we return an error. - defp execution_once?(commands) do - {warnings, errors} = Enum.reduce(commands, {[],[]}, - fn - ({cmd_name, cmd}, {warnings, errors}) -> - case Map.get(cmd, "execution", nil) do - "once" -> - msg = "Execution 'once' commands are no longer supported" - location = "#/commands/#{cmd_name}/execution" - updated_errors = [{msg, location} | errors] - {warnings, updated_errors} - "multiple" -> - msg = """ - Execution type has been deprecated. \ - Please update your bundle config to version #{@current_version}.\ - """ - location = "#/commands/#{cmd_name}/execution" - updated_warnings = [{msg, location} | warnings] - {updated_warnings, errors} - nil -> - {warnings, errors} - end - end) - - if length(errors) > 0 do - {true, {warnings, errors}} - else - {false, warnings} - end - - end - - # Updates for non enforcing commands. If 'enforcing: false' is specified we - # add the "allow" rule, delete the enforcing field and return a warning. If - # 'enforcing: true' is specified we return a warning and delete the field. - defp update_enforcing(commands) do - Enum.reduce(commands, {[], commands}, - fn - ({cmd_name, %{"enforcing" => enforcing}=cmd}, {warnings, commands}) when not(enforcing) -> - msg = """ - Non-enforcing commands have been deprecated. \ - Please update your bundle config to version #{@current_version}.\ - """ - updated_warnings = [{msg, "#/commands/#{cmd_name}/enforcing"} | warnings] - - updated_cmd = Map.put(cmd, "rules", ["allow"]) - |> Map.delete("enforcing") - updated_commands = Map.put(commands, cmd_name, updated_cmd) - - {updated_warnings, updated_commands} - ({cmd_name, %{"enforcing" => _}=cmd}, {warnings, commands}) -> - msg = """ - The 'enforcing' field has been deprecated. \ - Please update your bundle config to version #{@current_version}.\ - """ - updated_warnings = [{msg, "#/commands/#{cmd_name}/enforcing"} | warnings] - - updated_cmd = Map.delete(cmd, "enforcing") - updated_commands = Map.put(commands, cmd_name, updated_cmd) - - {updated_warnings, updated_commands} - (_, acc) -> - acc - end) - end - - defp insert_deprecation_msg({status, errors, warnings}, msg) do - {status, errors, [msg | warnings]} - end -end diff --git a/priv/schemas/bundle_config_schema_v2.yaml b/priv/schemas/bundle_config_schema_v4.yaml similarity index 70% rename from priv/schemas/bundle_config_schema_v2.yaml rename to priv/schemas/bundle_config_schema_v4.yaml index 04fda7b..0d6959d 100644 --- a/priv/schemas/bundle_config_schema_v2.yaml +++ b/priv/schemas/bundle_config_schema_v4.yaml @@ -1,21 +1,29 @@ --- "$schema": http://json-schema.org/draft-04/schema# -title: Bundle Config v2 +title: Bundle Config v3 description: A config schema for bundles type: object required: - cog_bundle_version - name + - description - version - commands -additionalProperties: false properties: cog_bundle_version: type: number enum: - - 2 + - 4 name: type: string + description: + type: string + long_descirption: + type: string + author: + type: string + homepage: + type: string version: type: string pattern: ^\d+\.\d+($|\.\d+$) @@ -28,15 +36,23 @@ properties: required: - image - tag + optional: + - binds properties: image: type: string tag: type: string + binds: + type: array + items: + type: string templates: type: object - additionalProperties: - "$ref": "#/definitions/template" + patternProperties: + "^[A-Za-z0-9_]+$": + "$ref": "#/definitions/template" + additionalProperties: false commands: type: object additionalProperties: @@ -46,27 +62,32 @@ properties: definitions: template: type: object - additionalProperties: false properties: - slack: - type: string - hipchat: + body: type: string + required: + - body + additionalProperties: false command: type: object required: - executable + - rules properties: executable: type: string - documentation: + description: type: string - execution: - enum: - - once - - multiple - enforcing: - type: boolean + long_description: + type: string + examples: + type: string + arguments: + type: string + subcommands: + type: object + additionalProperties: + type: string rules: type: array items: @@ -86,6 +107,7 @@ definitions: type: object required: - type + additionalProperties: false properties: type: type: string @@ -102,4 +124,3 @@ definitions: type: boolean short_flag: type: string - diff --git a/test/spanner/config/validator_test.exs b/test/spanner/config/v3_validator_test.exs similarity index 80% rename from test/spanner/config/validator_test.exs rename to test/spanner/config/v3_validator_test.exs index 507e637..fb50e32 100644 --- a/test/spanner/config/validator_test.exs +++ b/test/spanner/config/v3_validator_test.exs @@ -1,4 +1,4 @@ -defmodule Spanner.Config.Validator.Test do +defmodule Spanner.Config.V3Validator do use ExUnit.Case, async: true alias Spanner.Config @@ -26,19 +26,6 @@ defmodule Spanner.Config.Validator.Test do } end - defp old_config do - %{"cog_bundle_version" => 2, - "name" => "foo", - "version" => "0.0.1", - "commands" => %{ - "date" => %{ - "executable" => "/bin/date", - "enforcing" => false - } - } - } - end - defp incomplete_rules_config do %{"cog_bundle_version" => 3, "name" => "foo", @@ -67,12 +54,12 @@ defmodule Spanner.Config.Validator.Test do test "wrong cog_bundle_version" do result = update_in(minimal_config, ["cog_bundle_version"], fn(_) -> 1 end) |> validate - assert result == {:error, [{"cog_bundle_version 1 is not supported. Please update your bundle config to version 3.", "#/cog_bundle_version"}], []} + assert result == {:error, [{"cog_bundle_version 1 is not supported. Please update your bundle config to version 4.", "#/cog_bundle_version"}], []} end test "missing cog_bundle_version" do result = Map.delete(minimal_config, "cog_bundle_version") |> validate - assert result == {:error, [{"cog_bundle_version not specified. You must specify a valid bundle version. The current version is 3.", "#/cog_bundle_version"}], []} + assert result == {:error, [{"cog_bundle_version not specified. You must specify a valid bundle version. The current version is 4.", "#/cog_bundle_version"}], []} end test "incomplete rules" do @@ -132,22 +119,6 @@ defmodule Spanner.Config.Validator.Test do assert response == {:error, [{"Schema does not allow additional properties.", "#/templates/foo/meh"}], []} end - test "upgrading bundle versions" do - response = validate(old_config) - {:warning, config, warnings} = response - - assert %{"cog_bundle_version" => 3, - "name" => "foo", - "version" => "0.0.1", - "commands" => %{ - "date" => %{ - "executable" => "/bin/date", - "rules" => ["when command is foo:date allow"]}}, - } = config - assert [{"Bundle config version 2 has been deprecated. Please update to version 3.", "#/cog_bundle_version"}, - {"Non-enforcing commands have been deprecated. Please update your bundle config to version 3.", "#/commands/date/enforcing"}] = warnings - end - # env_vars can be strings, booleans and numbers Enum.each(["string", true, 4], fn(type) -> test "env_vars can be a #{type}" do diff --git a/test/spanner/config/v4_validator_test.exs b/test/spanner/config/v4_validator_test.exs new file mode 100644 index 0000000..be18241 --- /dev/null +++ b/test/spanner/config/v4_validator_test.exs @@ -0,0 +1,147 @@ +defmodule Spanner.Config.V4ValidatorTest do + # This is effectively the same as the v3 validator test, but with + # altered template tests, since that's the only difference between + # the two versions. + + use ExUnit.Case, async: true + alias Spanner.Config + + defp validate(config) do + case Config.validate(config) do + {:ok, _} -> + :ok + error -> + error + end + end + + defp minimal_config do + %{"cog_bundle_version" => 4, + "name" => "foo", + "version" => "0.0.1", + "description" => "Does some foo and a bar", + "commands" => %{ + "date" => %{ + "executable" => "/bin/date", + "rules" => [ + "allow" + ] + } + } + } + end + + defp incomplete_rules_config do + %{"cog_bundle_version" => 4, + "name" => "foo", + "version" => "0.1", + "description" => "Does some foo and a bar", + "permissions" => ["foo:view"], + "commands" => %{ + "bar" => %{"executable" => "/bin/bar", + "rules" => ["must have foo:view"]}, + "baz" => %{"executable" => "/bin/baz", + "rules" => ["allow"]}}} + end + + test "minimal config" do + assert validate(minimal_config) == :ok + end + + test "bad bundle versions" do + updated = put_in(minimal_config, ["version"], "1") + assert validate(updated) == {:error, [{"String \"1\" does not match pattern \"^\\\\d+\\\\.\\\\d+($|\\\\.\\\\d+$)\".", "#/version"}], []} + updated = put_in(minimal_config, ["version"], "0.0.1-beta") + assert validate(updated) == {:error, [{"String \"0.0.1-beta\" does not match pattern \"^\\\\d+\\\\.\\\\d+($|\\\\.\\\\d+$)\".", "#/version"}], []} + end + + test "wrong cog_bundle_version" do + result = update_in(minimal_config, ["cog_bundle_version"], fn(_) -> 1 end) + |> validate + assert result == {:error, [{"cog_bundle_version 1 is not supported. Please update your bundle config to version 4.", "#/cog_bundle_version"}], []} + end + + test "missing cog_bundle_version" do + result = Map.delete(minimal_config, "cog_bundle_version") |> validate + assert result == {:error, [{"cog_bundle_version not specified. You must specify a valid bundle version. The current version is 4.", "#/cog_bundle_version"}], []} + end + + test "incomplete rules" do + assert validate(incomplete_rules_config) == :ok + end + + test "rules are fixed up" do + {:ok, config} = Spanner.Config.validate(incomplete_rules_config) + [rule] = get_in(config, ["commands", "bar", "rules"]) + assert String.starts_with?(rule, "when command is foo:bar") + end + + test "errors when permissions don't match rules" do + rules = ["when command is foo:date must have foo:view"] + config = put_in(minimal_config, ["commands", "date", "rules"], rules) + response = validate(config) + + assert response == {:error, [{"The permission 'foo:view' is not in the list of permissions.", "#/commands/date/rules/0"}], []} + end + + test "errors on bad rule" do + rules = ["when command is foo:bar must have permission == foo:baz"] + config = put_in(minimal_config, ["commands", "date", "rules"], rules) + + response = validate(config) + + assert response == {:error, [{"(Line: 1, Col: 34) References to permissions must be the literal \"allow\" or start with a command bundle name or \"site\".", "#/commands/date/rules/0"}], []} + end + + test "rules are required" do + config = update_in(minimal_config, ["commands", "date"], &Map.delete(&1, "rules")) + |> validate + assert config == {:error, [{"Required property rules was not present.", "#/commands/date"}], []} + end + + test "errors on bad command option type" do + options = %{"option_1" => %{"type" => "integer", "required" => false}} + config = put_in(minimal_config, ["commands", "date", "options"], options) + response = validate(config) + + assert response == {:error, [{"Value \"integer\" is not allowed in enum.", "#/commands/date/options/option_1/type"}], []} + end + + # env_vars can be strings, booleans and numbers + Enum.each(["string", true, 4], fn(type) -> + test "env_vars can be a #{type}" do + env_var = %{"env1" => unquote(type)} + config = put_in(minimal_config, ["commands", "date", "env_vars"], env_var) + + response = validate(config) + assert response == :ok + end + end) + + ######################################################################## + # Templates + + test "templates are validated" do + templates = %{"blah" => %{"body" => "blahblah"}, + "foo" => %{"body" => "foofoo"}} + config = put_in(minimal_config, ["templates"], templates) + assert :ok == validate(config) + end + + test "errors on bad template format" do + templates = %{"foo" => "Some template"} + config = put_in(minimal_config, ["templates"], templates) + response = validate(config) + assert {:error, [{"Type mismatch. Expected Object but got String.", "#/templates/foo"}], []} == response + end + + test "templates must have proper names" do + templates = %{"1?" => %{"body" => "Some template"}} + config = put_in(minimal_config, ["templates"], templates) + response = validate(config) + + # Not the greatest error message for this, sadly :( + assert response == {:error, [{"Schema does not allow additional properties.", "#/templates/1?"}], []} + end + +end