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

Add database table and methods for profile selectors #3731

Merged
merged 2 commits into from
Jun 27, 2024

Conversation

jhrozek
Copy link
Contributor

@jhrozek jhrozek commented Jun 27, 2024

Summary

  • Add the selectors table
  • Add DB methods for selector management

So far just adds the table and the methods and their tests. These will be used in later PRs.

Fixes #3720

Change Type

  • Bug fix (resolves an issue without affecting existing features)
  • Feature (adds new functionality without breaking changes)
  • Breaking change (may impact existing functionalities or require documentation updates)
  • Documentation (updates or additions to documentation)
  • Refactoring or test improvements (no bug fixes or new functionality)

Testing

with the unit tests

Review Checklist:

  • Reviewed my own code for quality and clarity.
  • Added comments to complex or tricky code sections.
  • Updated any affected documentation.
  • Included tests that validate the fix or feature.
  • Checked that related changes are merged.

Adds the table for the profile selectors.

Related: mindersec#3720
@jhrozek jhrozek requested a review from a team as a code owner June 27, 2024 10:09
Adds the needed SQL methods to manage profile selectors

Fixes: mindersec#3720
FROM profile_selectors
WHERE profile_id = $1;

-- name: UpdateSelector :one
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really wanted to add an upsert, but since the only column to conflict on would be the ID, the upsert would be able to add a zero UUID. I tried working around that with a little more SQL code, like:

INSERT INTO profile_selectors (id, profile_id, entity, selector, comment)
VALUES (
   CASE WHEN $1::uuid = '00000000-0000-0000-0000-000000000000' THEN gen_random_uuid() ELSE $1::uuid END,
   $2::uuid, $3, $4, $5
)
ON CONFLICT (id) DO UPDATE
    SET profile_id = $2::uuid,
        entity = $3,
        selector = $4,
        comment = $5
RETURNING id, profile_id, entity, selector, comment;

but that breaks sqlc - it wasn't able to infer the type of $1 and generated parameters with a column named Column_1 and type interface{}. Trying to use sqlc.arg(id) instead of $1 didn't work either, because sqlc apparently requires that all parameters be used in the query.

So I went with just an update now and will handle the upsert case in the code instead.

Copy link
Member

Choose a reason for hiding this comment

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

If profile_id is not unique, I'm not sure what the upsert would look like unless you always have the UUID already created (e.g. client-side generation). profile_id is not marked as unique at the moment, so I'm not sure that upsert is particularly useful -- you'll know whether you have an existing UUID or not, and can switch between the insert and update pretty easily.

@coveralls
Copy link

Coverage Status

coverage: 52.478% (-0.009%) from 52.487%
when pulling aa40ff6 on jhrozek:selectors
into cdb5ae7 on stacklok:main.

@jhrozek jhrozek merged commit c3c9453 into mindersec:main Jun 27, 2024
22 checks passed
Copy link
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

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

LGTM.

FROM profile_selectors
WHERE profile_id = $1;

-- name: UpdateSelector :one
Copy link
Member

Choose a reason for hiding this comment

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

If profile_id is not unique, I'm not sure what the upsert would look like unless you always have the UUID already created (e.g. client-side generation). profile_id is not marked as unique at the moment, so I'm not sure that upsert is particularly useful -- you'll know whether you have an existing UUID or not, and can switch between the insert and update pretty easily.

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

Successfully merging this pull request may close these issues.

Add new database tables and the associated operations for profile selectors
4 participants