oxidecomputer / omicron

Omicron: Oxide control plane
Mozilla Public License 2.0
251 stars 39 forks source link

Some named objects in DB lacking UNIQUE name index #2124

Open smklein opened 1 year ago

smklein commented 1 year ago

Some objects in the DB have a name, and a corresponding index to ensure the name remains unique.

For example:

CREATE TABLE omicron.public.silo (
    /* Identity metadata */
    id UUID PRIMARY KEY,
    name STRING(63) NOT NULL,
    description STRING(512) NOT NULL,
    time_created TIMESTAMPTZ NOT NULL,
    time_modified TIMESTAMPTZ NOT NULL,
    time_deleted TIMESTAMPTZ,
    ...
);

CREATE UNIQUE INDEX ON omicron.public.silo (
    name
) WHERE
    time_deleted IS NULL;

The following objects within the database have a name field, but no UNIQUE index on the name:

davepacheco commented 1 year ago

Good call! For saml_identity_provider, I think the index needs to be there. We do expect multiple IdPs in a Silo and they should not have overlapping names. For saga, I think it's fine as is. As you said, the name is not a unique identifier for these.

davepacheco commented 1 year ago

See also #1534

davepacheco commented 1 year ago

3530 addresses the saml_identity_provider case and I think saga is fine as-is.

That leaves external_ip. I'm not sure in what scope names are supposed to be unique for these.

luqmana commented 1 year ago

I believe external_ip.name not being UNIQUE is intended. Only kind = floating should have them and RFD 21 says:

Floating IPs have their own DNS names and schemes. They do not show up in an instance’s external DNS, but rather in the Floating IP DNS scheme. Multiple Floating IPs can share the same DNS name. This creates a single DNS entry with multiple records.

Of course floating IPs still need to be fully fleshed out (https://github.com/oxidecomputer/omicron/issues/1334).

But we do create some floating external IP records today for external services like Nexus & External DNS which need inbound connectivity. But those don't really fit into the scheme described by the RFD as the external_ip table is a bit bifurcated with an is_service column.