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

Investigate SIGSEGV in CI - do not merge (dqlite 1.16.5) #14468

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

markylaing
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the Documentation Documentation needs updating label Nov 15, 2024
@markylaing markylaing force-pushed the used-by-bug-dqlite-1.16.5 branch 2 times, most recently from f8bf55f to f5f973b Compare November 15, 2024 17:40
Adds instance and storage volume snapshots and backups to the OpenFGA
model. These entitlements cannot be assigned to identities, service
accounts, or group members. Instead they are inherited from the parent
instance or volume.

Signed-off-by: Mark Laing <[email protected]>
The auth.ValidateEntitlement function validates all entitlements that
can be granted via the API. Since the new entitlements on snapshots and
backups cannot be granted via the API, this check fails.

The OpenFGA server will return an error if an invalid query is performed
based on it's own understanding of the authorization model.

Signed-off-by: Mark Laing <[email protected]>
Previously the only entities that had inherited relations were project and
server. Now that we are linking instances and storage volumes to their
snapshots and backups, the OpenFGADatastore implementation needs to handle
these relations.

On Read, we can connect a snapshot or backup to its parent instance or
storage volume using the information stored in its URL. For example, the
storage volume backup URL:

/1.0/storage-pools/default/volumes/custom/vol1/backups/backup1?project=project1

is related to its parent:

/1.0/storage-pools/default/volumes/custom/vol1?project=project1

via the `storage_volume relation`.

Signed-off-by: Mark Laing <[email protected]>
…tartingWithUser.

Previously the only entities that had inherited relations were project and
server. Now that we are linking instances and storage volumes to their
snapshots and backups, the OpenFGADatastore implementation needs to handle
these relations.

On ReadStartingWithUser, the function needs to return all backups or snapshots that
are related to a parent instance or storage volume. This is used in the `ListObjects`
call to the OpenFGA server, which is used by `(auth.Authorizer).GetPermissionChecker`.

To do this, I have naively queried for all snapshots or backups in the project, and
filtered out those that don't have the correct parent. This keeps the implementation
simple and makes use of `GetEntityURLs`, which performs as few queries as possible.
Further optimisation may be needed.

Signed-off-by: Mark Laing <[email protected]>
We can now use the `can_view`, `can_edit`, and `can_delete` entitlements
with instance backups and snapshots. We should do this so that our checks
more accurately reflect the authorization model.

Signed-off-by: Mark Laing <[email protected]>
The access handler was performing some logic to determine
the location of the storage volume for use in the access check.
This was based on whether the storage pool is remote, and if not,
the cluster member where the volume is located.

This commit removes that logic and adds a "location" field to
`storageVolumeDetails` so that it can be used in the handlers.
The logic for determining the location is modified to suit the call
site. It is only set when the pool is not remote.

Signed-off-by: Mark Laing <[email protected]>
The storage volume snapshot and backup access handlers need to share
almost identical logic to the storage volume access handler. Including
getting the storage pool, understanding if the storage volume is located
on another cluster member, and so forth.

This commit parameterises the function so that it can be used by the
snapshot and backup entity types as well; creating and checking against
the correct URL when called.

Signed-off-by: Mark Laing <[email protected]>
We can now check `can_view`, `can_edit`, and `can_delete` against
the backup/snapshot itself. We should do so to more accurately reflect
the authorization model.

Signed-off-by: Mark Laing <[email protected]>
This reverts commit e85099d.

Signed-off-by: Mark Laing <[email protected]>
This reverts commit dd30aac.

Signed-off-by: Mark Laing <[email protected]>
@markylaing markylaing force-pushed the used-by-bug-dqlite-1.16.5 branch 2 times, most recently from 70fe40d to 6e1ddbc Compare November 18, 2024 11:00
test/main.sh Fixed Show fixed Hide fixed
test/main.sh Fixed Show fixed Hide fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation needs updating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant