feat(mysql): Allow to Grant permissions on specific columns in table#330
Conversation
ce36e61 to
b41ee96
Compare
|
@ymaniukevich would it make sense to update the examples to get e2e coverage here? We'll also need to rebase this PR after #328 is in because e2e will fail now |
b41ee96 to
21e7234
Compare
|
@ymaniukevich I guess this also fixes #176 ? |
6fdc292 to
9c8f974
Compare
@chlunde I've updated the examples to improve e2e coverage. |
9c8f974 to
95e20ec
Compare
mysql: add e2e tests to cover granting permission on table Signed-off-by: ymaniukevich <yauheni.maniukevich@gmail.com>
95e20ec to
69512e6
Compare
| - DROP | ||
| - INSERT | ||
| - SELECT | ||
| - UPDATE (status, updated_at) |
There was a problem hiding this comment.
will mysql store this as a string and not normalize it, like reorder or change whitespace (UPDATE(status, updated_at) or UPDATE (updated_at, status))?
There was a problem hiding this comment.
MySQL normalizes the statement: uppercases keywords (even if you write update, it will be stored as UPDATE), adds missing spaces/backticks, and sorts column names alphabetically.
There was a problem hiding this comment.
@ymaniukevich cool, thanks for researching. This will likely burn som users, if they write it in another order. We should at minimum document it, or normalize it outselves in our observe method?
There was a problem hiding this comment.
or disallow it via CEL expression, similar to this, but that's not very pretty 😮💨
x-kubernetes-validations:
- rule: >-
self.spec.forProvider.privileges.all(p,
!p.contains('(') ||
p.split('(')[1].split(')')[0].split(', ').isSorted()
)
message: "Column names in column-level privileges must be listed alphabetically"There was a problem hiding this comment.
I don’t think this is actually an issue if the user specifies the order differently. The provider will treat the resource as synced. I already have a managed resource with a different order, and everything works as expected.
Or did I misunderstand your concern?
There was a problem hiding this comment.
My concern is if it reconciles (does an update) every poll interval if the user specifies a different order
There was a problem hiding this comment.
@chlunde you are right — on each loop I see Successfully requested update of external resource for Grant.
I’ll come back with changes (need to fix parseGrant function so it doesn't do the split if we have string with permission on the column)
Description of your changes
Improved regexp expression for granting permission to specific columns in MySQL.
Fixes #214, #176
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested