livebook-dev/livebook
1
1
Fork 0
mirror of https://github.com/livebook-dev/livebook.git synced 2025-09-11 23:44:23 +08:00
Code Issues Projects Releases Packages Wiki Activity

Final touchups to secrets (#1464)

Browse source 
* Use font-mono on secret names
* Unify error handling with changesets
* Unify put_env/delete_env as set_env/unset_env
* Review text and descriptions
This commit is contained in:
José Valim 2022-10-06 21:44:04 +02:00 • committed by GitHub
parent 195bc502fd
commit 950982304d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 193 additions and 205 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
assets/css/components.css
View file

@ -124,7 +124,7 @@
} }
.switch-button { .switch-button {
@apply relative inline-block w-14 h-7 mr-2 select-none; @apply relative inline-block w-14 h-7 select-none;
} }
.switch-button--disabled { .switch-button--disabled {

6
lib/livebook/hubs.ex
View file

@ -53,12 +53,10 @@ defmodule Livebook.Hubs do
@spec save_hub(Provider.t()) :: Provider.t() @spec save_hub(Provider.t()) :: Provider.t()
def save_hub(struct) do def save_hub(struct) do
attributes = struct |> Map.from_struct() |> Map.to_list() attributes = struct |> Map.from_struct() |> Map.to_list()
:ok = Storage.insert(@namespace, struct.id, attributes)
with :ok <- Storage.insert(@namespace, struct.id, attributes), :ok = broadcast_hubs_change()
:ok <- broadcast_hubs_change() do
struct struct
end end
end
@doc false @doc false
def delete_hub(id) do def delete_hub(id) do

4
lib/livebook/hubs/fly_client.ex
View file

@ -62,7 +62,7 @@ defmodule Livebook.Hubs.FlyClient do
end end
end end
def put_secrets(%Fly{access_token: access_token, application_id: application_id}, secrets) do def set_secrets(%Fly{access_token: access_token, application_id: application_id}, secrets) do
mutation = """ mutation = """
mutation($input: SetSecretsInput!) { mutation($input: SetSecretsInput!) {
setSecrets(input: $input) { setSecrets(input: $input) {
@ -85,7 +85,7 @@ defmodule Livebook.Hubs.FlyClient do
end end
end end
def delete_secrets(%Fly{access_token: access_token, application_id: application_id}, keys) do def unset_secrets(%Fly{access_token: access_token, application_id: application_id}, keys) do
mutation = """ mutation = """
mutation($input: UnsetSecretsInput!) { mutation($input: UnsetSecretsInput!) {
unsetSecrets(input: $input) { unsetSecrets(input: $input) {

28
lib/livebook/secrets.ex
View file

@ -36,25 +36,23 @@ defmodule Livebook.Secrets do
end end
@doc """ @doc """
Sets the given secret. Validates a secret map and either returns a struct struct or changeset.
""" """
@spec set_secret(Secret.t() | %Secret{}, map()) :: @spec validate_secret(map()) :: {:ok, Secret.t()} | {:error, Ecto.Changeset.t()}
{:ok, Secret.t()} | {:error, Ecto.Changeset.t()} def validate_secret(attrs) do
def set_secret(%Secret{} = secret \\ %Secret{}, attrs) do changeset = Secret.changeset(%Secret{}, attrs)
changeset = Secret.changeset(secret, attrs) apply_action(changeset, :validate)
with {:ok, secret} <- apply_action(changeset, :insert) do
save_secret(secret)
end
end end
defp save_secret(secret) do @doc """
Stores the given secret as is, without validation.
"""
@spec set_secret(Secret.t()) :: Secret.t()
def set_secret(secret) do
attributes = secret |> Map.from_struct() |> Map.to_list() attributes = secret |> Map.from_struct() |> Map.to_list()
:ok = Storage.insert(:secrets, secret.name, attributes)
with :ok <- Storage.insert(:secrets, secret.name, attributes), :ok = broadcast_secrets_change({:set_secret, secret})
:ok <- broadcast_secrets_change({:set_secret, secret}) do secret
{:ok, secret}
end
end end
@doc """ @doc """

2
lib/livebook/secrets/secret.ex
View file

@ -18,7 +18,7 @@ defmodule Livebook.Secrets.Secret do
|> cast(attrs, [:name, :value]) |> cast(attrs, [:name, :value])
|> update_change(:name, &String.upcase/1) |> update_change(:name, &String.upcase/1)
|> validate_format(:name, ~r/^\w+$/, |> validate_format(:name, ~r/^\w+$/,
message: "should contain only alphanumeric and underscore" message: "should contain only alphanumeric characters and underscore"
) )
|> validate_required([:name, :value]) |> validate_required([:name, :value])
end end

24
lib/livebook/session.ex
View file

@ -505,17 +505,17 @@ defmodule Livebook.Session do
@doc """ @doc """
Sends a secret addition request to the server. Sends a secret addition request to the server.
""" """
@spec put_secret(pid(), map()) :: :ok @spec set_secret(pid(), map()) :: :ok
def put_secret(pid, secret) do def set_secret(pid, secret) do
GenServer.cast(pid, {:put_secret, self(), secret}) GenServer.cast(pid, {:set_secret, self(), secret})
end end
@doc """ @doc """
Sends a secret deletion request to the server. Sends a secret deletion request to the server.
""" """
@spec delete_secret(pid(), map()) :: :ok @spec unset_secret(pid(), map()) :: :ok
def delete_secret(pid, secret_name) do def unset_secret(pid, secret_name) do
GenServer.cast(pid, {:delete_secret, self(), secret_name}) GenServer.cast(pid, {:unset_secret, self(), secret_name})
end end
@doc """ @doc """
@ -980,15 +980,15 @@ defmodule Livebook.Session do
end end
end end
def handle_cast({:put_secret, client_pid, secret}, state) do def handle_cast({:set_secret, client_pid, secret}, state) do
client_id = client_id(state, client_pid) client_id = client_id(state, client_pid)
operation = {:put_secret, client_id, secret} operation = {:set_secret, client_id, secret}
{:noreply, handle_operation(state, operation)} {:noreply, handle_operation(state, operation)}
end end
def handle_cast({:delete_secret, client_pid, secret_name}, state) do def handle_cast({:unset_secret, client_pid, secret_name}, state) do
client_id = client_id(state, client_pid) client_id = client_id(state, client_pid)
operation = {:delete_secret, client_id, secret_name} operation = {:unset_secret, client_id, secret_name}
{:noreply, handle_operation(state, operation)} {:noreply, handle_operation(state, operation)}
end end
@ -1451,12 +1451,12 @@ defmodule Livebook.Session do
state state
end end
defp after_operation(state, _prev_state, {:put_secret, _client_id, secret}) do defp after_operation(state, _prev_state, {:set_secret, _client_id, secret}) do
if Runtime.connected?(state.data.runtime), do: set_runtime_secret(state, secret) if Runtime.connected?(state.data.runtime), do: set_runtime_secret(state, secret)
state state
end end
defp after_operation(state, _prev_state, {:delete_secret, _client_id, secret_name}) do defp after_operation(state, _prev_state, {:unset_secret, _client_id, secret_name}) do
if Runtime.connected?(state.data.runtime), do: delete_runtime_secrets(state, [secret_name]) if Runtime.connected?(state.data.runtime), do: delete_runtime_secrets(state, [secret_name])
state state
end end

16
lib/livebook/session/data.ex
View file

@ -193,8 +193,8 @@ defmodule Livebook.Session.Data do
| {:set_file, client_id(), FileSystem.File.t() | nil} | {:set_file, client_id(), FileSystem.File.t() | nil}
| {:set_autosave_interval, client_id(), non_neg_integer() | nil} | {:set_autosave_interval, client_id(), non_neg_integer() | nil}
| {:mark_as_not_dirty, client_id()} | {:mark_as_not_dirty, client_id()}
| {:put_secret, client_id(), secret()} | {:set_secret, client_id(), secret()}
| {:delete_secret, client_id(), String.t()} | {:unset_secret, client_id(), String.t()}
@type action :: @type action ::
:connect_runtime :connect_runtime
@ -764,17 +764,17 @@ defmodule Livebook.Session.Data do
|> wrap_ok() |> wrap_ok()
end end
def apply_operation(data, {:put_secret, _client_id, secret}) do def apply_operation(data, {:set_secret, _client_id, secret}) do
data data
|> with_actions() |> with_actions()
|> put_secret(secret) |> set_secret(secret)
|> wrap_ok() |> wrap_ok()
end end
def apply_operation(data, {:delete_secret, _client_id, secret_name}) do def apply_operation(data, {:unset_secret, _client_id, secret_name}) do
data data
|> with_actions() |> with_actions()
|> delete_secret(secret_name) |> unset_secret(secret_name)
|> wrap_ok() |> wrap_ok()
end end
@ -1515,12 +1515,12 @@ defmodule Livebook.Session.Data do
end end
end end
defp put_secret({data, _} = data_actions, secret) do defp set_secret({data, _} = data_actions, secret) do
secrets = Map.put(data.secrets, secret.name, secret.value) secrets = Map.put(data.secrets, secret.name, secret.value)
set!(data_actions, secrets: secrets) set!(data_actions, secrets: secrets)
end end
defp delete_secret({data, _} = data_actions, secret_name) do defp unset_secret({data, _} = data_actions, secret_name) do
secrets = Map.delete(data.secrets, secret_name) secrets = Map.delete(data.secrets, secret_name)
set!(data_actions, secrets: secrets) set!(data_actions, secrets: secrets)
end end

10
lib/livebook/settings.ex
View file

@ -159,17 +159,15 @@ defmodule Livebook.Settings do
changeset = EnvVar.changeset(env_var, attrs) changeset = EnvVar.changeset(env_var, attrs)
with {:ok, env_var} <- apply_action(changeset, :insert) do with {:ok, env_var} <- apply_action(changeset, :insert) do
save_env_var(env_var) {:ok, save_env_var(env_var)}
end end
end end
defp save_env_var(env_var) do defp save_env_var(env_var) do
attributes = env_var |> Map.from_struct() |> Map.to_list() attributes = env_var |> Map.from_struct() |> Map.to_list()
:ok = Storage.insert(:env_vars, env_var.name, attributes)
with :ok <- Storage.insert(:env_vars, env_var.name, attributes), :ok = broadcast_env_vars_change({:env_var_set, env_var})
:ok <- broadcast_env_vars_change({:env_var_set, env_var}) do env_var
{:ok, env_var}
end
end end
@doc """ @doc """

4
lib/livebook_web/live/hub/edit/fly_component.ex
View file

@ -161,7 +161,7 @@ defmodule LivebookWeb.Hub.Edit.FlyComponent do
env_operation = attrs["operation"] env_operation = attrs["operation"]
attrs = %{"key" => attrs["name"], "value" => attrs["value"]} attrs = %{"key" => attrs["name"], "value" => attrs["value"]}
case FlyClient.put_secrets(socket.assigns.hub, [attrs]) do case FlyClient.set_secrets(socket.assigns.hub, [attrs]) do
{:ok, _} -> {:ok, _} ->
message = message =
if env_operation == "new", if env_operation == "new",
@ -192,7 +192,7 @@ defmodule LivebookWeb.Hub.Edit.FlyComponent do
end end
def handle_event("delete_env_var", %{"env_var" => key}, socket) do def handle_event("delete_env_var", %{"env_var" => key}, socket) do
case FlyClient.delete_secrets(socket.assigns.hub, [key]) do case FlyClient.unset_secrets(socket.assigns.hub, [key]) do
{:ok, _} -> {:ok, _} ->
{:noreply, {:noreply,
socket socket

70
lib/livebook_web/live/session_live.ex
View file

@ -57,7 +57,9 @@ defmodule LivebookWeb.SessionLive do
data_view: data_to_view(data), data_view: data_to_view(data),
autofocus_cell_id: autofocus_cell_id(data.notebook), autofocus_cell_id: autofocus_cell_id(data.notebook),
page_title: get_page_title(data.notebook.name), page_title: get_page_title(data.notebook.name),
livebook_secrets: Secrets.fetch_secrets() |> Map.new(&{&1.name, &1.value}) livebook_secrets: Secrets.fetch_secrets() |> Map.new(&{&1.name, &1.value}),
select_secret_ref: nil,
select_secret_options: nil
) )
|> assign_private(data: data) |> assign_private(data: data)
|> prune_outputs() |> prune_outputs()
@ -557,29 +559,43 @@ defmodule LivebookWeb.SessionLive do
defp secrets_list(assigns) do defp secrets_list(assigns) do
~H""" ~H"""
<div class="flex flex-col grow"> <div class="flex flex-col grow">
<div>
<h3 class="uppercase text-sm font-semibold text-gray-500"> <h3 class="uppercase text-sm font-semibold text-gray-500">
Secrets Secrets
</h3> </h3>
<span class="mt-4 text-sm font-semibold text-gray-500">Available to this notebook</span> <span class="text-sm text-gray-500">Available only to this session</span>
<div class="flex flex-col mt-4 space-y-4"> </div>
<div class="flex flex-col">
<div class="flex flex-col space-y-4 mt-6">
<%= for {secret_name, _} <- session_only_secrets(@data_view.secrets, @livebook_secrets) do %> <%= for {secret_name, _} <- session_only_secrets(@data_view.secrets, @livebook_secrets) do %>
<div class="flex justify-between items-center text-gray-500"> <div class="flex justify-between items-center text-gray-500">
<span class="text-sm break-all"> <span class="text-sm font-mono break-all">
<%= secret_name %> <%= secret_name %>
</span> </span>
<span class="rounded-full bg-gray-200 px-2 text-xs text-gray-600">
Session
</span>
</div> </div>
<% end %> <% end %>
<div class="w-full border-t border-gray-300 py-1"></div>
<div class="flex justify-between mt-4">
<span class="text-sm font-semibold text-gray-500">Stored in your Livebook</span>
<span class="text-sm font-light text-gray-500">On session</span>
</div> </div>
<%= live_patch to: Routes.session_path(@socket, :secrets, @session.id),
class: "inline-flex items-center justify-center p-8 py-1 mt-6 space-x-2 text-sm font-medium text-gray-500 border border-gray-400 border-dashed rounded-xl hover:bg-gray-100",
aria_label: "add secret",
role: "button" do %>
<.remix_icon icon="add-line" class="text-lg align-center" />
<span>New secret</span>
<% end %>
<div class="mt-16">
<h3 class="uppercase text-sm font-semibold text-gray-500">
App secrets
</h3>
<span class="text-sm text-gray-500">Toggle to share with this session</span>
</div>
<div class="flex flex-col space-y-4 mt-6">
<%= for {secret_name, secret_value} = secret <- Enum.sort(@livebook_secrets) do %> <%= for {secret_name, secret_value} = secret <- Enum.sort(@livebook_secrets) do %>
<div class="flex justify-between items-center text-gray-500"> <div class="flex justify-between items-center text-gray-500">
<span class="text-sm break-all"> <span class="text-sm font-mono break-all">
<%= secret_name %> <%= secret_name %>
</span> </span>
<.switch_checkbox <.switch_checkbox
@ -592,13 +608,7 @@ defmodule LivebookWeb.SessionLive do
</div> </div>
<% end %> <% end %>
</div> </div>
<%= live_patch to: Routes.session_path(@socket, :secrets, @session.id), </div>
class: "inline-flex items-center justify-center p-8 py-1 mt-8 space-x-2 text-sm font-medium text-gray-500 border border-gray-400 border-dashed rounded-xl hover:bg-gray-100",
aria_label: "add secret",
role: "button" do %>
<.remix_icon icon="add-line" class="text-lg align-center" />
<span>New secret</span>
<% end %>
</div> </div>
""" """
end end
@ -783,13 +793,19 @@ defmodule LivebookWeb.SessionLive do
def handle_params(params, _url, socket) def handle_params(params, _url, socket)
when socket.assigns.live_action == :secrets do when socket.assigns.live_action == :secrets do
{:noreply, socket =
if params["preselect_name"] do
assign(socket, prefill_secret_name: params["preselect_name"])
else
# Erase any previously stored reference
assign(socket, assign(socket,
prefill_secret_name: params["secret_name"] || params["preselect_name"], prefill_secret_name: params["secret_name"],
select_secret_ref: if(params["preselect_name"], do: socket.assigns.select_secret_ref), select_secret_ref: nil,
select_secret_options: select_secret_options: nil
if(params["preselect_name"], do: socket.assigns.select_secret_options) )
)} end
{:noreply, socket}
end end
def handle_params(_params, _url, socket) do def handle_params(_params, _url, socket) do
@ -1179,12 +1195,12 @@ defmodule LivebookWeb.SessionLive do
socket socket
) do ) do
secret = %{name: secret_name, value: secret_value} secret = %{name: secret_name, value: secret_value}
Livebook.Session.put_secret(socket.assigns.session.pid, secret) Livebook.Session.set_secret(socket.assigns.session.pid, secret)
{:noreply, socket} {:noreply, socket}
end end
def handle_event("toggle_secret", %{"secret-name" => secret_name}, socket) do def handle_event("toggle_secret", %{"secret-name" => secret_name}, socket) do
Livebook.Session.delete_secret(socket.assigns.session.pid, secret_name) Livebook.Session.unset_secret(socket.assigns.session.pid, secret_name)
{:noreply, socket} {:noreply, socket}
end end

94
lib/livebook_web/live/session_live/secrets_component.ex
View file

@ -12,6 +12,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
else else
assign(socket, assign(socket,
data: %{"name" => prefill_form, "value" => "", "store" => "session"}, data: %{"name" => prefill_form, "value" => "", "store" => "session"},
errors: [{"value", {"can't be blank", []}}],
title: title(socket), title: title(socket),
grant_access: must_grant_access(socket), grant_access: must_grant_access(socket),
has_prefill: prefill_form != "" has_prefill: prefill_form != ""
@ -51,7 +52,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
<%= for {secret_name, _} <- livebook_only_secrets(@secrets, @livebook_secrets) do %> <%= for {secret_name, _} <- livebook_only_secrets(@secrets, @livebook_secrets) do %>
<.secret_with_badge <.secret_with_badge
secret_name={secret_name} secret_name={secret_name}
stored="livebook" stored="Livebook"
action="select_livebook_secret" action="select_livebook_secret"
active={false} active={false}
target={@myself} target={@myself}
@ -76,7 +77,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
phx-change="validate" phx-change="validate"
autocomplete="off" autocomplete="off"
phx-target={@myself} phx-target={@myself}
errors={data_errors(@data)} errors={@errors}
class="basis-1/2 grow" class="basis-1/2 grow"
> >
<div class="flex flex-col space-y-4"> <div class="flex flex-col space-y-4">
@ -106,13 +107,13 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
) %> ) %>
</.input_wrapper> </.input_wrapper>
<div> <div>
<span class="text-base font-medium text-gray-900">Store</span> <div class="input-label">Storage</div>
<div class="mt-2 space-y-1"> <div class="my-2 space-y-1 text-sm">
<%= label class: "flex items-center gap-2 text-gray-600" do %> <%= label class: "flex items-center gap-2 text-gray-600" do %>
<%= radio_button(f, :store, "session", checked: @data["store"] == "session") %> Session <%= radio_button(f, :store, "session", checked: @data["store"] == "session") %> only this session
<% end %> <% end %>
<%= label class: "flex items-center gap-2 text-gray-600" do %> <%= label class: "flex items-center gap-2 text-gray-600" do %>
<%= radio_button(f, :store, "livebook", checked: @data["store"] == "livebook") %> Notebook <%= radio_button(f, :store, "livebook", checked: @data["store"] == "livebook") %> in the Livebook app
<% end %> <% end %>
</div> </div>
</div> </div>
@ -134,25 +135,27 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
~H""" ~H"""
<div <div
role="button" role="button"
class={ class={[
"flex justify-between w-full font-mono text-sm p-2 border-b cursor-pointer",
if @active do if @active do
"flex justify-between w-full bg-blue-100 text-sm text-blue-700 p-2 border-b cursor-pointer" "bg-blue-100 text-blue-700"
else else
"flex justify-between w-full text-sm text-gray-700 p-2 border-b cursor-pointer hover:bg-gray-100" "text-gray-700 hover:bg-gray-100"
end end
} ]}
phx-value-secret_name={@secret_name} phx-value-secret_name={@secret_name}
phx-target={@target} phx-target={@target}
phx-click={@action} phx-click={@action}
> >
<%= @secret_name %> <%= @secret_name %>
<span class={ <span class={[
"inline-flex items-center font-sans rounded-full px-2.5 py-0.5 text-xs font-medium bg-gray-100",
if @active do if @active do
"inline-flex items-center rounded-full bg-indigo-100 px-2.5 py-0.5 text-xs font-medium text-blue-800" "bg-indigo-100 text-blue-800"
else else
"inline-flex items-center rounded-full bg-gray-100 px-2.5 py-0.5 text-xs font-medium text-gray-800" "bg-gray-100 text-gray-800"
end end
}> ]}>
<%= if @active do %> <%= if @active do %>
<svg class="-ml-0.5 mr-1.5 h-2 w-2 text-blue-400" fill="currentColor" viewBox="0 0 8 8"> <svg class="-ml-0.5 mr-1.5 h-2 w-2 text-blue-400" fill="currentColor" viewBox="0 0 8 8">
<circle cx="4" cy="4" r="3" /> <circle cx="4" cy="4" r="3" />
@ -168,7 +171,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
~H""" ~H"""
<div> <div>
<div class="mx-auto"> <div class="mx-auto">
<div class="rounded-lg bg-blue-600 p-2 shadow-sm"> <div class="rounded-lg bg-blue-600 py-1 px-4 shadow-sm">
<div class="flex flex-wrap items-center justify-between"> <div class="flex flex-wrap items-center justify-between">
<div class="flex w-0 flex-1 items-center"> <div class="flex w-0 flex-1 items-center">
<.remix_icon <.remix_icon
@ -176,8 +179,9 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
class="align-middle text-2xl flex text-gray-100 rounded-lg py-2" class="align-middle text-2xl flex text-gray-100 rounded-lg py-2"
/> />
<span class="ml-2 text-sm font-normal text-gray-100"> <span class="ml-2 text-sm font-normal text-gray-100">
The secret <span class="font-semibold text-white"><%= @grant_access %></span> There is a secret named
needs to be made available to the session <span class="font-semibold text-white"><%= @grant_access %></span>
in your Livebook app. Allow this session to access it?
</span> </span>
</div> </div>
<button <button
@ -197,23 +201,25 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
@impl true @impl true
def handle_event("save", %{"data" => data}, socket) do def handle_event("save", %{"data" => data}, socket) do
if data_errors(data) == [] do assigns = socket.assigns
secret_name = String.upcase(data["name"])
secret = %{name: secret_name, value: data["value"]} case Livebook.Secrets.validate_secret(data) do
{:ok, secret} ->
store = data["store"] store = data["store"]
put_secret(socket.assigns.session.pid, secret, store) set_secret(assigns.session.pid, secret, store)
if store == "livebook" && if store == "livebook" &&
(socket.assigns.select_secret_ref || (assigns.select_secret_ref ||
{secret.name, socket.assigns.livebook_secrets[secret.name]} in socket.assigns.secrets) do {secret.name, assigns.livebook_secrets[secret.name]} in assigns.secrets) do
put_secret(socket.assigns.session.pid, secret, "session") set_secret(assigns.session.pid, secret, "session")
end end
{:noreply, {:noreply,
socket |> push_patch(to: socket.assigns.return_to) |> push_secret_selected(secret_name)} socket |> push_patch(to: assigns.return_to) |> push_secret_selected(secret.name)}
else
{:noreply, assign(socket, data: data)} {:error, changeset} ->
{:noreply, assign(socket, errors: changeset.errors)}
end end
end end
@ -230,7 +236,12 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
end end
def handle_event("validate", %{"data" => data}, socket) do def handle_event("validate", %{"data" => data}, socket) do
{:noreply, assign(socket, data: data)} socket = assign(socket, data: data)
case Livebook.Secrets.validate_secret(data) do
{:ok, _} -> {:noreply, assign(socket, errors: [])}
{:error, changeset} -> {:noreply, assign(socket, errors: changeset.errors)}
end
end end
def handle_event("grant_access", %{"secret_name" => secret_name}, socket) do def handle_event("grant_access", %{"secret_name" => secret_name}, socket) do
@ -240,27 +251,6 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
socket |> push_patch(to: socket.assigns.return_to) |> push_secret_selected(secret_name)} socket |> push_patch(to: socket.assigns.return_to) |> push_secret_selected(secret_name)}
end end
defp data_errors(data) do
Enum.flat_map(data, fn {key, value} ->
if error = data_error(key, value) do
[{String.to_existing_atom(key), {error, []}}]
else
[]
end
end)
end
defp data_error("name", value) do
cond do
String.match?(value, ~r/^\w+$/) -> nil
value == "" -> "can't be blank"
true -> "is invalid"
end
end
defp data_error("value", ""), do: "can't be blank"
defp data_error(_key, _value), do: nil
defp push_secret_selected(%{assigns: %{select_secret_ref: nil}} = socket, _), do: socket defp push_secret_selected(%{assigns: %{select_secret_ref: nil}} = socket, _), do: socket
defp push_secret_selected(%{assigns: %{select_secret_ref: ref}} = socket, secret_name) do defp push_secret_selected(%{assigns: %{select_secret_ref: ref}} = socket, secret_name) do
@ -289,13 +279,13 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
defp title(%{assigns: %{select_secret_options: %{"title" => title}}}), do: title defp title(%{assigns: %{select_secret_options: %{"title" => title}}}), do: title
defp title(_), do: "Select secret" defp title(_), do: "Select secret"
defp put_secret(pid, secret, "session"), do: Livebook.Session.put_secret(pid, secret) defp set_secret(pid, secret, "session"), do: Livebook.Session.set_secret(pid, secret)
defp put_secret(_pid, secret, "livebook"), do: Livebook.Secrets.set_secret(secret) defp set_secret(_pid, secret, "livebook"), do: Livebook.Secrets.set_secret(secret)
defp grant_access(secret_name, socket) do defp grant_access(secret_name, socket) do
secret_value = socket.assigns.livebook_secrets[secret_name] secret_value = socket.assigns.livebook_secrets[secret_name]
secret = %{name: secret_name, value: secret_value} secret = %{name: secret_name, value: secret_value}
put_secret(socket.assigns.session.pid, secret, "session") set_secret(socket.assigns.session.pid, secret, "session")
end end
defp livebook_only_secrets(secrets, livebook_secrets) do defp livebook_only_secrets(secrets, livebook_secrets) do

14
test/livebook/hubs/fly_client_test.exs
View file

@ -115,7 +115,7 @@ defmodule Livebook.Hubs.FlyClientTest do
end end
end end
describe "put_secrets/2" do describe "set_secrets/2" do
test "puts a list of secrets inside application", %{bypass: bypass} do test "puts a list of secrets inside application", %{bypass: bypass} do
secrets = [ secrets = [
%{ %{
@ -135,7 +135,7 @@ defmodule Livebook.Hubs.FlyClientTest do
end) end)
hub = build(:fly) hub = build(:fly)
assert {:ok, ^secrets} = FlyClient.put_secrets(hub, [%{key: "FOO", value: "BAR"}]) assert {:ok, ^secrets} = FlyClient.set_secrets(hub, [%{key: "FOO", value: "BAR"}])
end end
test "returns error when input is invalid", %{bypass: bypass} do test "returns error when input is invalid", %{bypass: bypass} do
@ -172,7 +172,7 @@ defmodule Livebook.Hubs.FlyClientTest do
end) end)
hub = build(:fly) hub = build(:fly)
assert {:error, ^message} = FlyClient.put_secrets(hub, [%{key: "FOO", Value: "BAR"}]) assert {:error, ^message} = FlyClient.set_secrets(hub, [%{key: "FOO", Value: "BAR"}])
end end
test "returns unauthorized when token is invalid", %{bypass: bypass} do test "returns unauthorized when token is invalid", %{bypass: bypass} do
@ -188,11 +188,11 @@ defmodule Livebook.Hubs.FlyClientTest do
hub = build(:fly) hub = build(:fly)
assert {:error, "request failed with code: UNAUTHORIZED"} = assert {:error, "request failed with code: UNAUTHORIZED"} =
FlyClient.put_secrets(hub, [%{key: "FOO", value: "BAR"}]) FlyClient.set_secrets(hub, [%{key: "FOO", value: "BAR"}])
end end
end end
describe "delete_secrets/2" do describe "unset_secrets/2" do
test "deletes a list of secrets inside application", %{bypass: bypass} do test "deletes a list of secrets inside application", %{bypass: bypass} do
response = %{"data" => %{"unsetSecrets" => %{"app" => %{"secrets" => []}}}} response = %{"data" => %{"unsetSecrets" => %{"app" => %{"secrets" => []}}}}
@ -203,7 +203,7 @@ defmodule Livebook.Hubs.FlyClientTest do
end) end)
hub = build(:fly) hub = build(:fly)
assert {:ok, []} = FlyClient.delete_secrets(hub, ["FOO"]) assert {:ok, []} = FlyClient.unset_secrets(hub, ["FOO"])
end end
test "returns unauthorized when token is invalid", %{bypass: bypass} do test "returns unauthorized when token is invalid", %{bypass: bypass} do
@ -219,7 +219,7 @@ defmodule Livebook.Hubs.FlyClientTest do
hub = build(:fly) hub = build(:fly)
assert {:error, "request failed with code: UNAUTHORIZED"} = assert {:error, "request failed with code: UNAUTHORIZED"} =
FlyClient.delete_secrets(hub, ["FOO"]) FlyClient.unset_secrets(hub, ["FOO"])
end end
end end
end end

33
test/livebook/secrets_test.exs
View file

@ -6,7 +6,7 @@ defmodule Livebook.SecretsTest do
alias Livebook.Secrets.Secret alias Livebook.Secrets.Secret
test "fetch secrets" do test "fetch secrets" do
Secrets.set_secret(%{name: "FOO", value: "111"}) Secrets.set_secret(%Secret{name: "FOO", value: "111"})
assert %Secret{name: "FOO", value: "111"} in Secrets.fetch_secrets() assert %Secret{name: "FOO", value: "111"} in Secrets.fetch_secrets()
Secrets.unset_secret("FOO") Secrets.unset_secret("FOO")
@ -14,7 +14,7 @@ defmodule Livebook.SecretsTest do
end end
test "fetch an specific secret" do test "fetch an specific secret" do
secret = %{name: "FOO", value: "111"} secret = %Secret{name: "FOO", value: "111"}
Secrets.set_secret(secret) Secrets.set_secret(secret)
assert_raise Livebook.Storage.NotFoundError, assert_raise Livebook.Storage.NotFoundError,
@ -30,41 +30,28 @@ defmodule Livebook.SecretsTest do
test "secret_exists?/1" do test "secret_exists?/1" do
Secrets.unset_secret("FOO") Secrets.unset_secret("FOO")
refute Secrets.secret_exists?("FOO") refute Secrets.secret_exists?("FOO")
Secrets.set_secret(%{name: "FOO", value: "111"}) Secrets.set_secret(%Secret{name: "FOO", value: "111"})
assert Secrets.secret_exists?("FOO") assert Secrets.secret_exists?("FOO")
Secrets.unset_secret("FOO") Secrets.unset_secret("FOO")
end end
describe "set_secret/1" do describe "validate_secret/1" do
test "creates and stores a secret" do test "returns a valid secret" do
attrs = %{name: "FOO", value: "111"} attrs = %{name: "FOO", value: "111"}
assert {:ok, secret} = Secrets.set_secret(attrs) assert {:ok, secret} = Secrets.validate_secret(attrs)
assert attrs.name == secret.name assert attrs.name == secret.name
assert attrs.value == secret.value assert attrs.value == secret.value
Secrets.unset_secret(secret.name)
end
test "updates an stored secret" do
secret = %Secret{name: "FOO", value: "111"}
attrs = %{value: "222"}
assert {:ok, updated_secret} = Secrets.set_secret(secret, attrs)
assert secret.name == updated_secret.name
assert updated_secret.value == attrs.value
Secrets.unset_secret(secret.name)
end end
test "returns changeset error" do test "returns changeset error" do
attrs = %{value: "111"} attrs = %{value: "111"}
assert {:error, changeset} = Secrets.set_secret(attrs) assert {:error, changeset} = Secrets.validate_secret(attrs)
assert "can't be blank" in errors_on(changeset).name assert "can't be blank" in errors_on(changeset).name
attrs = %{name: "@inavalid", value: "111"} attrs = %{name: "@inavalid", value: "111"}
assert {:error, changeset} = Secrets.set_secret(attrs) assert {:error, changeset} = Secrets.validate_secret(attrs)
assert "should contain only alphanumeric and underscore" in errors_on(changeset).name
assert "should contain only alphanumeric characters and underscore" in errors_on(changeset).name
end end
end end
end end

8
test/livebook_web/live/hub/edit_live_test.exs
View file

@ -205,10 +205,10 @@ defmodule LivebookWeb.Hub.EditLiveTest do
response = response =
cond do cond do
body["query"] =~ "setSecrets" -> body["query"] =~ "setSecrets" ->
put_secrets_response() set_secrets_response()
body["query"] =~ "unsetSecrets" -> body["query"] =~ "unsetSecrets" ->
delete_secrets_response() unset_secrets_response()
true -> true ->
Agent.get(agent_pid, fn Agent.get(agent_pid, fn
@ -299,11 +299,11 @@ defmodule LivebookWeb.Hub.EditLiveTest do
] ]
end end
defp put_secrets_response do defp set_secrets_response do
%{"data" => %{"setSecrets" => %{"app" => %{"secrets" => secrets(:add)}}}} %{"data" => %{"setSecrets" => %{"app" => %{"secrets" => secrets(:add)}}}}
end end
defp delete_secrets_response do defp unset_secrets_response do
%{"data" => %{"unsetSecrets" => %{"app" => %{"secrets" => secrets(:mount)}}}} %{"data" => %{"unsetSecrets" => %{"app" => %{"secrets" => secrets(:mount)}}}}
end end
end end

25
test/livebook_web/live/session_live_test.exs
View file

@ -5,6 +5,7 @@ defmodule LivebookWeb.SessionLiveTest do
alias Livebook.{Sessions, Session, Settings, Runtime, Users, FileSystem} alias Livebook.{Sessions, Session, Settings, Runtime, Users, FileSystem}
alias Livebook.Notebook.Cell alias Livebook.Notebook.Cell
alias Livebook.Secrets.Secret
setup do setup do
{:ok, session} = Sessions.create_session(notebook: Livebook.Notebook.new()) {:ok, session} = Sessions.create_session(notebook: Livebook.Notebook.new())
@ -948,13 +949,13 @@ defmodule LivebookWeb.SessionLiveTest do
|> element(~s{form[phx-submit="save"]}) |> element(~s{form[phx-submit="save"]})
|> render_submit(%{data: %{name: "bar", value: "456", store: "livebook"}}) |> render_submit(%{data: %{name: "bar", value: "456", store: "livebook"}})
assert %Livebook.Secrets.Secret{name: "BAR", value: "456"} in Livebook.Secrets.fetch_secrets() assert %Secret{name: "BAR", value: "456"} in Livebook.Secrets.fetch_secrets()
end end
test "sync secrets when they're equal", %{conn: conn, session: session} do test "sync secrets when they're equal", %{conn: conn, session: session} do
Livebook.Secrets.set_secret(%{name: "FOO", value: "123"}) Livebook.Secrets.set_secret(%Secret{name: "FOO", value: "123"})
{:ok, view, _} = live(conn, "/sessions/#{session.id}/secrets") {:ok, view, _} = live(conn, "/sessions/#{session.id}/secrets")
Session.put_secret(session.pid, %{name: "FOO", value: "123"}) Session.set_secret(session.pid, %{name: "FOO", value: "123"})
view view
|> element(~s{form[phx-submit="save"]}) |> element(~s{form[phx-submit="save"]})
@ -962,13 +963,13 @@ defmodule LivebookWeb.SessionLiveTest do
assert %{secrets: %{"FOO" => "456"}} = Session.get_data(session.pid) assert %{secrets: %{"FOO" => "456"}} = Session.get_data(session.pid)
assert %Livebook.Secrets.Secret{name: "FOO", value: "456"} in Livebook.Secrets.fetch_secrets() assert %Secret{name: "FOO", value: "456"} in Livebook.Secrets.fetch_secrets()
end end
test "doesn't sync secrets when they are not the same", %{conn: conn, session: session} do test "doesn't sync secrets when they are not the same", %{conn: conn, session: session} do
Livebook.Secrets.set_secret(%{name: "FOO_BAR", value: "456"}) Livebook.Secrets.set_secret(%Secret{name: "FOO_BAR", value: "456"})
{:ok, view, _} = live(conn, "/sessions/#{session.id}/secrets") {:ok, view, _} = live(conn, "/sessions/#{session.id}/secrets")
Session.put_secret(session.pid, %{name: "FOO_BAR", value: "123"}) Session.set_secret(session.pid, %{name: "FOO_BAR", value: "123"})
view view
|> element(~s{form[phx-submit="save"]}) |> element(~s{form[phx-submit="save"]})
@ -976,15 +977,15 @@ defmodule LivebookWeb.SessionLiveTest do
assert %{secrets: %{"FOO_BAR" => "123"}} = Session.get_data(session.pid) assert %{secrets: %{"FOO_BAR" => "123"}} = Session.get_data(session.pid)
assert %Livebook.Secrets.Secret{name: "FOO_BAR", value: "999"} in Livebook.Secrets.fetch_secrets() assert %Secret{name: "FOO_BAR", value: "999"} in Livebook.Secrets.fetch_secrets()
refute %Livebook.Secrets.Secret{name: "FOO_BAR", value: "456"} in Livebook.Secrets.fetch_secrets() refute %Secret{name: "FOO_BAR", value: "456"} in Livebook.Secrets.fetch_secrets()
end end
test "never sync secrets when updating from session", %{conn: conn, session: session} do test "never sync secrets when updating from session", %{conn: conn, session: session} do
Livebook.Secrets.set_secret(%{name: "FOO", value: "123"}) Livebook.Secrets.set_secret(%Secret{name: "FOO", value: "123"})
{:ok, view, _} = live(conn, "/sessions/#{session.id}/secrets") {:ok, view, _} = live(conn, "/sessions/#{session.id}/secrets")
Session.put_secret(session.pid, %{name: "FOO", value: "123"}) Session.set_secret(session.pid, %{name: "FOO", value: "123"})
view view
|> element(~s{form[phx-submit="save"]}) |> element(~s{form[phx-submit="save"]})
@ -992,9 +993,9 @@ defmodule LivebookWeb.SessionLiveTest do
assert %{secrets: %{"FOO" => "456"}} = Session.get_data(session.pid) assert %{secrets: %{"FOO" => "456"}} = Session.get_data(session.pid)
refute %Livebook.Secrets.Secret{name: "FOO", value: "456"} in Livebook.Secrets.fetch_secrets() refute %Secret{name: "FOO", value: "456"} in Livebook.Secrets.fetch_secrets()
assert %Livebook.Secrets.Secret{name: "FOO", value: "123"} in Livebook.Secrets.fetch_secrets() assert %Secret{name: "FOO", value: "123"} in Livebook.Secrets.fetch_secrets()
end end
test "shows the 'Add secret' button for unavailable secrets", %{conn: conn, session: session} do test "shows the 'Add secret' button for unavailable secrets", %{conn: conn, session: session} do