fix: propagate role removal to postgres cluster when databases are removed from postgres database spec#1816
Conversation
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request |
mploski
force-pushed
the
fix/postgres-controller-roles-removal
branch
from
5618891 to
e725c53
Compare
| // +kubebuilder:default=true | ||
| // +optional | ||
| Exists bool `json:"exists,omitempty"` | ||
| Exists bool `json:"exists"` |
There was a problem hiding this comment.
if it's optional it should have omitempty and should be a pointer type
There was a problem hiding this comment.
btw. is "exists" tag our custom or added by k8s api?
There was a problem hiding this comment.
It is not optional anymore, need to remove optional flag - optionality causing problems with SSA if not set properly. @M4KIF this is our attribute so we know if we should add or remove provided role
| updatedCluster := &enterprisev4.PostgresCluster{} | ||
| Expect(k8sClient.Get(ctx, types.NamespacedName{Name: clusterName, Namespace: namespace}, updatedCluster)).To(Succeed()) | ||
|
|
||
| expectManagedRoleExists(updatedCluster, "keepdb_admin", true) |
There was a problem hiding this comment.
Here, and at line 530-533 the values could get packed into constants at the top of the file, so they are easy to change and read. Ie. those permisions are quite verbose, but by doing them with constants they can carry more meaning and less manual string pasting.
There was a problem hiding this comment.
good point, lets do this
4 participants
Description
Fixes a bug where removing a database from postgres database CR spec.databases (or deleting the CR with DeletionPolicy: Delete) did not cause the corresponding PostgreSQL roles to be dropped in postgres cluster. postgres cluster only drops a role from cnpg when it receives Exists: false in the cluster spec — previously, removed roles were never marked absent but silently wiped out
Key Changes
extracted expectManagedRoleExists helper used across both role lifecycle tests
Testing and Verification
Added unit and integration tests. All green
Related Issues
Jira tickets, GitHub issues, Support tickets...
PR Checklist