Skip to content

Conversation

ImeevMA
Copy link
Collaborator

@ImeevMA ImeevMA commented Mar 21, 2024

Closes #9643

@TarantoolBot document
Title: config: changes in credentials section

Now the privileges that were not granted by the configuration, as well as privileges that were not granted solely by the configuration, are not revoked on reload. Privileges only granted by the config module will still be revoked if they are removed from the credentials section on reload.

@ImeevMA ImeevMA requested review from Totktonada and a team as code owners March 21, 2024 14:22
@ImeevMA ImeevMA requested a review from Lord-KA March 21, 2024 14:24
@ImeevMA ImeevMA force-pushed the imeevma/gh-9643-use-origin branch 2 times, most recently from 397dc48 to d84f629 Compare March 21, 2024 14:45
@coveralls
Copy link

coveralls commented Mar 21, 2024

Coverage Status

coverage: 86.989% (-0.009%) from 86.998%
when pulling 6634833 on ImeevMA:imeevma/gh-9643-use-origin
into 369c255
on tarantool:master
.

@ImeevMA ImeevMA force-pushed the imeevma/gh-9643-use-origin branch from d84f629 to 412a889 Compare March 21, 2024 15:13
@Totktonada
Copy link
Member

The patchset looks OK on a brief glance. I'll return here back a bit later to look into the credentials applier test changes deeper.

@ImeevMA ImeevMA force-pushed the imeevma/gh-9643-use-origin branch from 412a889 to 184089f Compare March 22, 2024 10:05
Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

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

Thank you for this effort!

I have a few nits (see above), but the patchset is OK for me.

I would like to hear @Lord-KA's feedback before push.

@ImeevMA ImeevMA force-pushed the imeevma/gh-9643-use-origin branch from 184089f to 7940cf7 Compare March 22, 2024 12:20
Copy link
Contributor

@Lord-KA Lord-KA 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 the patchset! The fist commit seems good, but I would like some more detailed explanation for the change, if you don't mind

@ImeevMA ImeevMA force-pushed the imeevma/gh-9643-use-origin branch from 7940cf7 to dbdc145 Compare March 22, 2024 13:39
@ImeevMA ImeevMA requested a review from Lord-KA March 22, 2024 13:44
@ImeevMA ImeevMA force-pushed the imeevma/gh-9643-use-origin branch from dbdc145 to 36f120f Compare March 22, 2024 15:05
Copy link
Contributor

@Lord-KA Lord-KA 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 this patch too, I really like the idea! Everything looks clean besides a few minor details below. Anyway, LGTM.

Currently, if we try to run Tarantool 3.0 with config using old
snapshot, we may get a SCHEMA_NEEDS_UPGRADE error because granting and
revoking privileges are DDL operations. This leads to a situation where
loading Tarantool to perform an upgrade becomes quite problematic. To
avoid the issue, this patch causes 'credentials.lua' to issue a warning
instead of an error in case of the SCHEMA_NEEDS_UPGRADE error during
granting and revoking privileges.

Note that it was still possible to startup and perform the upgrade by
removing the 'credentials' section from the config or without using
config.

This is only a part of the solution, the issue will be fixed in tarantool#9849.

Part of tarantool#9849
Needed for tarantool#9643

NO_DOC=will be added later
NO_CHANGELOG=will be added later
@ImeevMA ImeevMA force-pushed the imeevma/gh-9643-use-origin branch 2 times, most recently from 8875c82 to 8937227 Compare March 25, 2024 07:30
Closes tarantool#9643

@TarantoolBot document
Title: config: changes in `credentials` section

Now the privileges that were not granted by the configuration, as well
as privileges that were not granted solely by the configuration, are not
revoked on reload. Privileges that have been granted only by the config
module will still be revoked if they are removed from the `credentials`
section on reload.
@ImeevMA ImeevMA force-pushed the imeevma/gh-9643-use-origin branch from 8937227 to 6634833 Compare March 25, 2024 07:40
@ImeevMA ImeevMA added the full-ci Enables all tests for a pull request label Mar 25, 2024
@Totktonada Totktonada merged commit b982b46 into tarantool:master Mar 25, 2024
@Totktonada
Copy link
Member

Backported to release/3.0 (future 3.0.2) in PR #9861.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full-ci Enables all tests for a pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

config: allow to give priveleges from app roles
6 participants