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

BUG: allow server query with non-admin credentials #152

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions actions/server.list.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ parameters:
default: "search_all"
immutable: true
type: string
as_admin:
default: True
immutable: true
type: boolean
cloud_account:
description: "The clouds.yaml account to use whilst performing this action"
required: true
Expand Down
4 changes: 4 additions & 0 deletions actions/server.search.by.datetime.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ parameters:
default: "search_by_datetime"
immutable: true
type: string
as_admin:
default: True
immutable: true
type: boolean
cloud_account:
description: "The clouds.yaml account to use whilst performing this action"
required: true
Expand Down
4 changes: 4 additions & 0 deletions actions/server.search.by.property.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ parameters:
default: "search_by_property"
immutable: true
type: string
as_admin:
default: True
immutable: true
type: boolean
cloud_account:
description: The clouds.yaml account to use whilst performing this action
required: true
Expand Down
4 changes: 4 additions & 0 deletions actions/server.search.by.regex.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ parameters:
default: "search_by_regex"
immutable: true
type: string
as_admin:
default: True
immutable: true
type: boolean
cloud_account:
description: The clouds.yaml account to use whilst performing this action
required: true
Expand Down
4 changes: 2 additions & 2 deletions lib/openstack_query/mappings/server_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def get_server_side_handler() -> ServerSideHandler:
ServerProperties.SERVER_DESCRIPTION: lambda value: {
"description": value
},
ServerProperties.SERVER_STATUS: lambda value: {"vm_state": value},
ServerProperties.SERVER_STATUS: lambda value: {"status": value},
ServerProperties.SERVER_CREATION_DATE: lambda value: {
"created_at": value
},
Expand All @@ -85,7 +85,7 @@ def get_server_side_handler() -> ServerSideHandler:
{"description": value} for value in values
],
ServerProperties.SERVER_STATUS: lambda values: [
{"vm_state": value} for value in values
{"status": value} for value in values
],
ServerProperties.SERVER_CREATION_DATE: lambda values: [
{"created_at": value} for value in values
Expand Down
6 changes: 2 additions & 4 deletions lib/openstack_query/runners/runner_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def run(
) -> List[OpenstackResourceObj]:
"""
Public method that runs the query by querying openstacksdk and then applying a filter function.
:param cloud_account: An string for the account from the clouds configuration to use
:param cloud_account: A string for the account from the clouds configuration to use
:param client_side_filters: An Optional list of filter functions to run locally that we can use to limit the
results after querying openstacksdk
:param server_side_filters: An Optional list of filter kwargs to limit the results by when querying openstacksdk
Expand Down Expand Up @@ -114,9 +114,7 @@ def _run_with_openstacksdk(
if not server_filters:
server_filters = [None]

if kwargs:
kwargs = self._parse_meta_params(conn, **kwargs)

kwargs = self._parse_meta_params(conn, **kwargs)
meta_param_log_str = "\n\t\t".join(
[f"{key}: '{val}'" for key, val in kwargs.items()]
)
Expand Down
66 changes: 47 additions & 19 deletions lib/openstack_query/runners/server_runner.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Optional, List, Dict
import logging

import openstack.exceptions
from openstack.compute.v2.server import Server
from openstack.exceptions import ResourceNotFound

Expand Down Expand Up @@ -31,16 +32,48 @@ def _parse_meta_params(
self,
conn: OpenstackConnection,
from_projects: Optional[List[ProjectIdentifier]] = None,
all_projects: bool = False,
as_admin: bool = False,
) -> Dict:
"""
This method is a helper function that will parse a set of query meta params related to openstack server queries.
:param conn: An OpenstackConnection object - used to connect to openstack and parse meta params
:param from_projects: A list of projects to search in
:param all_projects: A boolean which, if true - will run query on all available projects to the user
:param as_admin: A boolean which, if true - will run query as an admin
"""
if not from_projects:
return {}

project_list = []
for proj in from_projects:
# raise error if ambiguous query
if from_projects and all_projects:
raise ParseQueryError(
"Failed to execute query: ambiguous run params - run with either "
"from_projects or all_projects and not both"
)

# default to providing servers only from the current scoped project
curr_project_id = conn.current_project_id
if not as_admin:
logger.warning(
"NOT RUNNING AS ADMIN - query will only work on the scoped project id: %s",
curr_project_id,
)
if from_projects or all_projects:
raise ParseQueryError(
"Failed to execute query: run_params given won't work if "
"you're not running as admin"
)
return {"projects": [curr_project_id]}

# don't provide any projects to scope the query so it runs on all projects
if all_projects:
return {"all_tenants": True}

# scope the query to user projects if from_projects not given
# we need to validate that user is actually admin by validating that it can find each user project
user_projects = list(conn.identity.user_projects(conn.current_user_id))
project_list = from_projects if from_projects else user_projects
res_list = []
for proj in project_list:
try:
logger.debug(
"running conn.identity.find_project(%s, ignore_missing=False)", proj
Expand All @@ -50,8 +83,13 @@ def _parse_meta_params(
raise ParseQueryError(
"Failed to execute query: Failed to parse meta params"
) from exp
project_list.append(project)
return {"projects": project_list}
except openstack.exceptions.ForbiddenException as exp:
raise ParseQueryError(
"Failed to execute query: Not authorized to access project(s) given "
"- please run with admin credentials"
) from exp
res_list.append(project)
return {"all_tenants": True, "projects": res_list}

def _run_query(
self,
Expand All @@ -72,22 +110,12 @@ def _run_query(
if not filter_kwargs:
filter_kwargs = {}

# so we can search in all projects instead of default set in clouds.yaml
filter_kwargs.update({"all_tenants": True})

if "projects" not in meta_params:
logger.debug("running query on all projects")
logger.debug(
"running openstacksdk command conn.compute.servers (%s)",
",".join(f"{key}={value}" for key, value in filter_kwargs.items()),
)
return self._run_paginated_query(conn.compute.servers, filter_kwargs)

if "project_id" in filter_kwargs.keys():
if "project_id" in filter_kwargs.keys() and "projects" in meta_params:
raise ParseQueryError(
"This query uses a preset that requires searching on project_ids "
"- but you've provided projects to search in using from_projects meta param"
"- please use one or the other, not both"
"- or you are not running as admin"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this applies in this case? Since the exception only applies to the query lib and how the user sets the flag regardless of their cred status

Copy link
Collaborator Author

@anish-mudaraddi anish-mudaraddi Oct 10, 2023

Choose a reason for hiding this comment

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

quirk of how it works basically
if a user tries to run a query where they are trying to filter by project id and they are not admin - run_query() will be run with both a meta-param auto-generated which contains one project (the one that is scoped to the project) and filter kwargs set by the user - then we will hit the error

)

query_res = []
Expand All @@ -100,7 +128,7 @@ def _run_query(
)
logger.debug(
"running openstacksdk command conn.compute.servers (%s)",
",".join(f"{key}={value}" for key, value in filter_kwargs.items()),
", ".join(f"{key}={value}" for key, value in filter_kwargs.items()),
)
query_res.extend(
self._run_paginated_query(conn.compute.servers, dict(filter_kwargs))
Expand Down
4 changes: 2 additions & 2 deletions tests/lib/openstack_query/mappings/test_server_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_server_side_handler_mappings_equal_to(server_side_test_mappings):
ServerProperties.USER_ID: "user_id",
ServerProperties.SERVER_ID: "uuid",
ServerProperties.SERVER_NAME: "hostname",
ServerProperties.SERVER_STATUS: "vm_state",
ServerProperties.SERVER_STATUS: "status",
ServerProperties.SERVER_DESCRIPTION: "description",
ServerProperties.SERVER_CREATION_DATE: "created_at",
ServerProperties.FLAVOR_ID: "flavor",
Expand Down Expand Up @@ -68,7 +68,7 @@ def test_server_side_handler_mappings_any_in(server_side_any_in_mappings):
ServerProperties.USER_ID: "user_id",
ServerProperties.SERVER_ID: "uuid",
ServerProperties.SERVER_NAME: "hostname",
ServerProperties.SERVER_STATUS: "vm_state",
ServerProperties.SERVER_STATUS: "status",
ServerProperties.SERVER_DESCRIPTION: "description",
ServerProperties.SERVER_CREATION_DATE: "created_at",
ServerProperties.FLAVOR_ID: "flavor",
Expand Down
10 changes: 2 additions & 8 deletions tests/lib/openstack_query/runners/test_runner_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,8 @@ def _run_with_openstacksdk_test_case(mock_server_filters, **mock_kwargs):
**mock_kwargs,
)

if mock_kwargs:
mock_parse_meta_params.assert_called_once_with(
mock_connection, **mock_kwargs
)
run_query_kwargs = mock_parse_meta_params.return_value
else:
mock_parse_meta_params.assert_not_called()
run_query_kwargs = {}
mock_parse_meta_params.assert_called_once_with(mock_connection, **mock_kwargs)
run_query_kwargs = mock_parse_meta_params.return_value

# check that run_query has been called correctly for each mock filter given
for mock_filter in mock_server_filters:
Expand Down
Loading
Loading