Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some named objects in DB lacking UNIQUE name index #2124

Open
3 tasks
smklein opened this issue Jan 10, 2023 · 4 comments
Open
3 tasks

Some named objects in DB lacking UNIQUE name index #2124

smklein opened this issue Jan 10, 2023 · 4 comments
Assignees
Milestone

Comments

@smklein
Copy link
Collaborator

smklein commented Jan 10, 2023

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:

  • omicron.public.saml_identity_provider: Has an index on id + silo_id, but no name-based index. This might be fine, if the assumption exists that "only one can ever exist within the scope of a silo anyway".
  • omicron.public.external_ip: Has no name-based index, but has the constraint that names are NOT NULL for floating ips. I believe this means that multiple floating IPs could exist with the same name.
  • omicron.public.saga: I'm not sure if name uniqueness matters; these names are more like labels than unique identifiers.
@davepacheco
Copy link
Collaborator

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
Copy link
Collaborator

See also #1534

@davepacheco
Copy link
Collaborator

#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
Copy link
Contributor

luqmana commented Jul 8, 2023

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 (#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.

@morlandi7 morlandi7 modified the milestones: FCS, 1.0.2 Aug 15, 2023
@askfongjojo askfongjojo modified the milestones: 1.0.2, MVP Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants