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

Update Playbook docs for Butler registry permissions #195

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

kfindeisen
Copy link
Member

This PR changes the expected registry permissions to be more minimalist; we only need delete permission for a handful of tables, and usage permission for a single sequence. Requesting only the permissions that we truly need should minimize friction with the DBAs as we move into Simonyi commissioning.

Copy link
Collaborator

@hsinfang hsinfang left a comment

Choose a reason for hiding this comment

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

Thanks for tracking this down and documenting it!

Some tables also need DELETE (d), but we are still confirming which ones.

We need SELECT (r) and USAGE (U) permissions for the sequence ``collection_seq_collection_id``, but *not* for ``dataset_calibs_*_seq_id``, ``dataset_type_seq_id``, or ``dimension_graph_key_seq_id``.
We expect that most future sequences will only be touched by repository maintenance and not by pipeline runs or data transfers.

If any tables are missing permissions, run:
Copy link
Collaborator

Choose a reason for hiding this comment

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

On Slack @ktlim mentioned that these were manually applied
GRANT SELECT,INSERT,UPDATE ON ALL TABLES IN SCHEMA embargo, embargo_old TO latiss_prompt, lsstcomcamsim_prompt;

Should we just advise to grant on all tables, not selected tables?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather keep some human judgment in the mix -- I've seen too many people interpret "run this"-type instructions as something to obey without question. That's why I said to run \dp and see what's there.

Our previous permissions required delete permission on all tables
(which we generally don't need), and permissions for all sequences
(most of which we don't use).
SQL is whitespace-agnostic, so there's no reason to make it unreadable.
@kfindeisen kfindeisen merged commit d4ed261 into main Sep 11, 2024
2 checks passed
@kfindeisen kfindeisen deleted the u/kfindeisen/permissions-fix-2 branch September 11, 2024 21:55
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.

2 participants