Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 43 additions & 9 deletions lib/cog/repository/bundles.ex
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ defmodule Cog.Repository.Bundles do

@permanent_site_bundle_version "0.0.0"

@install_types [:normal, :force]
@default_install_type :normal

########################################################################
# CRUD Operations

Expand All @@ -48,10 +51,11 @@ defmodule Cog.Repository.Bundles do
#{inspect @reserved_bundle_names}

"""
def install(%{"name" => reserved}) when reserved in @reserved_bundle_names,
def install(install_type \\ @default_install_type, params)
def install(_, %{"name" => reserved}) when reserved in @reserved_bundle_names,
do: {:error, {:reserved_bundle, reserved}}
def install(params),
do: __install(params)
def install(install_type, params) when install_type in @install_types,
do: __install(install_type, params)

def install_from_registry(bundle, version) do
case BundleRegistry.get_config(bundle, version) do
Expand All @@ -67,8 +71,12 @@ defmodule Cog.Repository.Bundles do

# TODO: clean this up so we only need the config file part;
# everything else is redundant
defp __install(%{"name" => _name, "version" => _version, "config_file" => _config}=params) do
case install_bundle(params) do
defp __install(install_type \\ @default_install_type, params)
defp __install(install_type, %{"name" => _name,
"version" => _version,
"config_file" => _config}=params)
when install_type in @install_types do
case install_bundle(install_type, params) do
{:ok, bundle_version} ->
{:ok, preload(bundle_version)}
{:error, _}=error ->
Expand Down Expand Up @@ -634,7 +642,7 @@ defmodule Cog.Repository.Bundles do
Repo.preload(bundle, [:permissions, :commands])
end

defp new_version!(bundle, params) do
defp new_version!(:normal, bundle, params) do
params = Map.merge(params, Map.take(params["config_file"], ["description", "long_description", "author", "homepage"]))

bundle
Expand All @@ -643,6 +651,32 @@ defmodule Cog.Repository.Bundles do
|> Repo.insert!
|> preload
end
defp new_version!(:force, bundle, params) do
# If we are doing a force install we check to see if the version
# already exists.
version_num = params["config_file"]["version"]
result = Repo.one(from bv in BundleVersion,
where: bv.bundle_id == ^bundle.id,
where: bv.version == ^version_num)

# If there is an existing version we remove it before adding the
# new version.
case result do
nil ->
new_version!(:normal, bundle, params)
old_version ->
# If the old version was enabled we enable the new version
if enabled?(old_version) do
Repo.delete!(old_version)
new_version = new_version!(:normal, bundle, params)
set_bundle_version_status(new_version, :enabled)
new_version
else
Repo.delete!(old_version)
new_version!(:normal, bundle, params)
end
end
end

# Consolidate what we need to preload for various things so we stay
# consistent
Expand All @@ -668,14 +702,14 @@ defmodule Cog.Repository.Bundles do
alias Cog.Models.CommandOption
alias Cog.Models.Permission

defp install_bundle(bundle_params) do
defp install_bundle(install_type, bundle_params) do
Repo.transaction(fn ->
try do
# Create a bundle record if it doesn't exist yet
bundle = find_or_create_bundle(bundle_params["name"])

# Create a new version record
version = new_version!(bundle, bundle_params)
# Create the new version
version = new_version!(install_type, bundle, bundle_params)

# Add permissions, after deduping
:ok = register_permissions_for_version(bundle, version)
Expand Down
112 changes: 112 additions & 0 deletions test/cog/repository/bundles_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,72 @@ defmodule Cog.Repository.BundlesTest do
assert {:error, {:db_errors, [version: {"has already been taken", []}]}} = Bundles.install(%{"name" => "testing", "version" => "1.0.0", "config_file" => %{}})
end

describe "forced bundle installation" do

setup [:forced_installation_configs]

test "installing the same version overwrites the original version", %{old_config: config} do
{:ok, _version} = Bundles.install(%{"name" => config["name"], "version" => config["version"], "config_file" => config})

assert {:ok, _overwritten_version} = Bundles.install(:force, %{"name" => config["name"], "version" => config["version"], "config_file" => config})
end

test "different configs persist properly", %{old_config: old_config, new_config: new_config} do

{:ok, orig_version} = Bundles.install(%{"name" => old_config["name"],
"version" => old_config["version"],
"config_file" => old_config})

# Make sure the bundle installs
assert {:ok, new_version} = Bundles.install(:force, %{"name" => new_config["name"],
"version" => new_config["version"],
"config_file" => new_config})

new_version = Repo.preload(new_version, :templates)

# Make sure the bundle name and version didn't change
assert orig_version.bundle.name == new_version.bundle.name
assert orig_version.version == new_version.version

# Check the config file
assert new_version.config_file == new_config

# Check commands
expected_command_names = Map.keys(new_config["commands"]) |> Enum.sort
actual_command_names = Enum.map(new_version.commands, &(&1.command.name)) |> Enum.sort
assert actual_command_names == expected_command_names

# Check permissions
expected_permissions = new_config["permissions"] |> Enum.sort
actual_permissions = Enum.map(new_version.permissions, &("test_bundle:#{&1.name}")) |> Enum.sort
assert actual_permissions == expected_permissions

# Check templates
expected_template_names = Map.keys(new_config["templates"]) |> Enum.sort
actual_template_names = Enum.map(new_version.templates, &(&1.name)) |> Enum.sort
assert actual_template_names == expected_template_names

expected_templates = Enum.map(new_config["templates"], fn({_, source}) -> source["body"] end) |> Enum.sort
actual_templates = Enum.map(new_version.templates, &(&1.source)) |> Enum.sort
assert actual_templates == expected_templates
end

test "maintains enabled status", %{old_config: old_config, new_config: new_config} do
{:ok, orig_version} = Bundles.install(%{"name" => old_config["name"],
"version" => old_config["version"],
"config_file" => old_config})

:ok = Bundles.set_bundle_version_status(orig_version, :enabled)

{:ok, new_version} = Bundles.install(:force, %{"name" => new_config["name"],
"version" => new_config["version"],
"config_file" => new_config})

assert Bundles.enabled?(new_version)
end

end

test "deleting the last version of a bundle deletes the bundle itself" do
{:ok, version} = Bundles.install(%{"name" => "testing", "version" => "1.0.0", "config_file" => %{}})

Expand Down Expand Up @@ -404,4 +470,50 @@ defmodule Cog.Repository.BundlesTest do
v
end

# Setup function for testing forced bundle installations
defp forced_installation_configs(context) do
old_config =
%{"cog_bundle_version" => 4,
"name" => "test_bundle",
"description" => "A test bundle",
"version" => "0.1.0",
"permissions" => ["test_bundle:date", "test_bundle:time"],
"docker" => %{"image" => "operable-bundle/test_bundle",
"tag" => "v0.1.0"},
"commands" => %{"date" => %{"executable" => "/usr/local/bin/date",
"options" => %{"option1" => %{"type" => "string",
"description" => "An option",
"required" => false,
"short_flag" => "o"}},
"rules" => ["when command is test_bundle:date must have test_bundle:date"]},
"time" => %{"executable" => "/usr/local/bin/time",
"rules" => ["when command is test_bundle:time must have test_bundle:time"]}},
"templates" => %{"time" => %{"body" => "~$results[0].time~"},
"date" => %{"body" => "~$results[0].date~"}}}

new_config =
%{"cog_bundle_version" => 4,
"name" => "test_bundle",
"description" => "An updated test bundle",
"version" => "0.1.0",
"permissions" => ["test_bundle:new_date", "test_bundle:new_time", "test_bundle:another_command"],
"docker" => %{"image" => "operable-bundle/test_bundle_update",
"tag" => "v0.1.1"},
"commands" => %{"new_date" => %{"executable" => "/usr/local/bin/date",
"options" => %{"option1" => %{"type" => "string",
"description" => "An option",
"required" => false,
"short_flag" => "o"}},
"rules" => ["when command is test_bundle:date must have test_bundle:date"]},
"new_time" => %{"executable" => "/usr/local/bin/time",
"rules" => ["when command is test_bundle:time must have test_bundle:time"]},
"another_command" => %{"executable" => "/usr/local/bin/time",
"rules" => ["when command is test_bundle:time must have test_bundle:time"]}},
"templates" => %{"new_time" => %{"body" => "~$results[0].new_time~"},
"new_date" => %{"body" => "~$results[0].new_date~"},
"another" => %{"body" => "~$results[0].another~"}}}

Map.merge(context, %{old_config: old_config, new_config: new_config})
end

end
8 changes: 8 additions & 0 deletions test/controllers/v1/bundles_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,14 @@ defmodule Cog.V1.BundlesControllerTest do
"version has already been taken"] = json_response(conn, 409)["errors"]
end

test "force enables installation of an already installed version", %{authed: requestor} do
config = config(:map)
{:ok, _orig_version3} = Bundles.install(%{"name" => config["name"], "version" => config["version"], "config_file" => config})

conn = api_request(requestor, :post, "/v1/bundles", body: %{"bundle" => %{"config" => config, "force" => true}})
assert response(conn, 201)
end

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned that there are a lot of edge cases in overall behavior that we're not addressing with this test. I'm thinking in particular of the rules and permissions associated with the old and new bundles.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree. I just did a quick smoke test to validate it was working. If you think my general approach is reasonable, I'll add some more tests to make sure everything is working as expected.

test "fails to install with semantically invalid config", %{authed: requestor} do
# The config includes rules that mention permissions; if we remove
# those permissions, installation should fail
Expand Down
8 changes: 7 additions & 1 deletion web/controllers/v1/bundles_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,10 @@ defmodule Cog.V1.BundlesController do

defp install_bundle(params) do
with {:ok, config} <- parse_config(params),
{:ok, install_type} <- install_type(params),
{:ok, valid_config, warnings} <- validate_config(config),
{:ok, params} <- merge_config(params, valid_config),
{:ok, bundle} <- Repository.Bundles.install(params) do
{:ok, bundle} <- Repository.Bundles.install(install_type, params) do
{:ok, bundle, warnings}
end
end
Expand All @@ -92,6 +93,11 @@ defmodule Cog.V1.BundlesController do
defp parse_config(_),
do: {:error, :no_config}

defp install_type(%{"force" => true}),
do: {:ok, :force}
defp install_type(_),
do: {:ok, :normal}

# If we have a file, check to see if the filename has the correct extension
defp validate_file_format(%Plug.Upload{filename: filename}) do
if Config.config_extension?(filename) do
Expand Down