-
Notifications
You must be signed in to change notification settings - Fork 1
[DPE-8473] Add 'databases' to the invalid database names list #18
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
Conversation
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (46.72%) is below the target coverage (70.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## 16/edge #18 +/- ##
===========================================
+ Coverage 46.32% 46.72% +0.39%
===========================================
Files 4 4
Lines 803 809 +6
Branches 94 94
===========================================
+ Hits 372 378 +6
Misses 414 414
Partials 17 17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
…se-name Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
…tion to limit the check only for VM Signed-off-by: Marcelo Henrique Neppel <[email protected]>
…se-name Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tnx @marceloneppel for the quick protection here!
} | ||
|
||
INVALID_DATABASE_NAME_BLOCKING_MESSAGE = "invalid database name" | ||
INVALID_DATABASE_NAMES = ["databases", "postgres", "template0", "template1"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@7annaba3l JFYI, side-effect from charmed_databases_owner
spec.
CC: @delgod
Issue
With the new predefined roles implemented for PG 16, if a client app requests a database called databases while the PG charm tries to set up the roles for it, a failure will happen because one of the roles will conflict with the already existing charmed_databases_owner role (responsible for owning all the databases).
Solution
Add 'databases' to the invalid database names list.
This is a fix for what was discussed on canonical/postgresql-k8s-operator#1107 (comment).
Follow-up PRs on PG VM and K8s repos:
databases
) postgresql-operator#1190databases
) postgresql-k8s-operator#1108Also, constants were added for the different substrates (VM and K8s). This allows the lib to know when it's needed to fix permissions (like for the
temp
storage in VM, when it'stmpfs
).Checklist