Allow variable creation with different target environments

[lovincyrus]

closes #6150

https://www.notion.so/rilldata/Environment-Variables-management-UI-42304f145b07410d86c2a523496aa6bc

• allow 1 unique variable key + environment
• disallow updating existing variable in create dialog perhaps
• https://rilldata.slack.com/archives/C07NUPQ02EA/p1733167795245509
• A variable with the name test already exists for the target development

CleanShot.2024-11-26.at.11.28.13.mp4

[ericpgreen2]

It should probably be a server-side check, but it shouldn't be possible to have multiple test keys for a given environment. If test[dev,prod]=abc and test[prod]=def, what is the value of test in production? I see that Vercel prevents such a submission:

[lovincyrus]

It should probably be a server-side check, but it shouldn't be possible to have multiple test keys for a given environment. If test[dev,prod]=abc and test[prod]=def, what is the value of test in production? I see that Vercel prevents such a submission:

Agreed - This should ideally be a server-side check.

[begelundmuller]

It should probably be a server-side check, but it shouldn't be possible to have multiple test keys for a given environment

This already is not possible at the DB level, so you may have a UI state issue if that seems to be the case. If you see this, can you try refreshing the page?

Also note that the UpdateProjectVariables API is an update API, so it's not going to give you errors like the one Vercel returns – if test[dev,prod]=abc and you update test[prod]=def, then the new state is test[dev]=abc and test[prod]=def.

---

It sounds like it may be helpful with some context about how variables are stored in the database. This is the schema:

CREATE TABLE project_variables (
  id UUID NOT NULL DEFAULT uuid_generate_v4() PRIMARY KEY,
  project_id UUID NOT NULL REFERENCES projects(id) ON DELETE CASCADE,
  name TEXT NOT NULL,
  value TEXT NOT NULL,
  -- Encryption key ID if the value is encrypted
  value_encryption_key_id TEXT NOT NULL DEFAULT '',
  -- Environment it belongs to ("prod" or "dev").
  -- If empty, then the value is used as the fallback for all environments.
  environment TEXT NOT NULL DEFAULT '',
  -- The user who most recently edited the variable
  updated_by_user_id UUID REFERENCES users(id) ON DELETE SET NULL,
  created_on TIMESTAMPTZ DEFAULT now() NOT NULL,
  updated_on TIMESTAMPTZ DEFAULT now() NOT NULL
);

CREATE UNIQUE INDEX project_variables_project_id_environment_name_idx ON project_variables (project_id, environment, lower(name));

Note that there's a unique index on project_id, environment, lower(name). This unique index is used to determine if a variable should be added or updated in calls to UpdateProjectVariables in the following upsert statement:

INSERT INTO project_variables (project_id, environment, name, value, value_encryption_key_id, updated_by_user_id, updated_on)
VALUES ...
ON CONFLICT (project_id, environment, lower(name)) DO UPDATE SET
  value = EXCLUDED.value,
  value_encryption_key_id = EXCLUDED.value_encryption_key_id,
  updated_by_user_id = EXCLUDED.updated_by_user_id,
  updated_on = now()
RETURNING *

When resolving variables for the runtime, it first builds a list of all variables with environment = '' (the fallback values for all environments), and then overwrites it with all variables with environment = '<target env>' (the override values for the specific environment).

[lovincyrus]

Latest demo:

CleanShot.2024-12-03.at.13.38.49.mp4

[lovincyrus]

It should probably be a server-side check, but it shouldn't be possible to have multiple test keys for a given environment

This already is not possible at the DB level, so you may have a UI state issue if that seems to be the case. If you see this, can you try refreshing the page?

Also note that the UpdateProjectVariables API is an update API, so it's not going to give you errors like the one Vercel returns – if test[dev,prod]=abc and you update test[prod]=def, then the new state is test[dev]=abc and test[prod]=def.

It sounds like it may be helpful with some context about how variables are stored in the database. This is the schema:

CREATE TABLE project_variables (
  id UUID NOT NULL DEFAULT uuid_generate_v4() PRIMARY KEY,
  project_id UUID NOT NULL REFERENCES projects(id) ON DELETE CASCADE,
  name TEXT NOT NULL,
  value TEXT NOT NULL,
  -- Encryption key ID if the value is encrypted
  value_encryption_key_id TEXT NOT NULL DEFAULT '',
  -- Environment it belongs to ("prod" or "dev").
  -- If empty, then the value is used as the fallback for all environments.
  environment TEXT NOT NULL DEFAULT '',
  -- The user who most recently edited the variable
  updated_by_user_id UUID REFERENCES users(id) ON DELETE SET NULL,
  created_on TIMESTAMPTZ DEFAULT now() NOT NULL,
  updated_on TIMESTAMPTZ DEFAULT now() NOT NULL
);

CREATE UNIQUE INDEX project_variables_project_id_environment_name_idx ON project_variables (project_id, environment, lower(name));

Note that there's a unique index on project_id, environment, lower(name). This unique index is used to determine if a variable should be added or updated in calls to UpdateProjectVariables in the following upsert statement:

INSERT INTO project_variables (project_id, environment, name, value, value_encryption_key_id, updated_by_user_id, updated_on)
VALUES ...
ON CONFLICT (project_id, environment, lower(name)) DO UPDATE SET
  value = EXCLUDED.value,
  value_encryption_key_id = EXCLUDED.value_encryption_key_id,
  updated_by_user_id = EXCLUDED.updated_by_user_id,
  updated_on = now()
RETURNING *

When resolving variables for the runtime, it first builds a list of all variables with environment = '' (the fallback values for all environments), and then overwrites it with all variables with environment = '<target env>' (the override values for the specific environment).

Added more checks on the client side to check for duplicates before hitting the actual Update API operation call.

[ericpgreen2]

LGTM!