From 50b8f238b6f9adbc9ff0b20e18b78b2948c2f440 Mon Sep 17 00:00:00 2001 From: Abdul Hameed Date: Thu, 19 Sep 2024 14:04:56 -0400 Subject: [PATCH] fix: Logger settings for feature servers and updated logger for permission flow (#4531) * Logger setting for feature servers Signed-off-by: Abdul Hameed * added/updated logger for permission flow Signed-off-by: Abdul Hameed * set defualt logger level for feature servers as WARNING Signed-off-by: Abdul Hameed --------- Signed-off-by: Abdul Hameed --- .../feast-feature-server/templates/deployment.yaml | 8 ++++++++ infra/charts/feast-feature-server/values.yaml | 2 ++ .../feast/permissions/auth/kubernetes_token_parser.py | 6 ++++-- sdk/python/feast/permissions/auth/oidc_token_parser.py | 4 ++-- .../client/intra_comm_authentication_client_manager.py | 1 + sdk/python/feast/permissions/enforcer.py | 4 ++++ sdk/python/feast/permissions/server/arrow.py | 9 +++++---- sdk/python/feast/permissions/server/grpc.py | 7 ++++--- sdk/python/feast/permissions/server/utils.py | 1 - 9 files changed, 30 insertions(+), 12 deletions(-) diff --git a/infra/charts/feast-feature-server/templates/deployment.yaml b/infra/charts/feast-feature-server/templates/deployment.yaml index dc62be8b95..1f673280fe 100644 --- a/infra/charts/feast-feature-server/templates/deployment.yaml +++ b/infra/charts/feast-feature-server/templates/deployment.yaml @@ -42,20 +42,28 @@ spec: command: {{- if eq .Values.feast_mode "offline" }} - "feast" + - "--log-level" + - "{{ .Values.logLevel }}" - "serve_offline" - "-h" - "0.0.0.0" {{- else if eq .Values.feast_mode "ui" }} - "feast" + - "--log-level" + - "{{ .Values.logLevel }}" - "ui" - "-h" - "0.0.0.0" {{- else if eq .Values.feast_mode "registry" }} - "feast" + - "--log-level" + - "{{ .Values.logLevel }}" - "serve_registry" {{- else }} {{- if .Values.metrics.enlabled }} - "feast" + - "--log-level" + - "{{ .Values.logLevel }}" - "serve" - "--metrics" - "-h" diff --git a/infra/charts/feast-feature-server/values.yaml b/infra/charts/feast-feature-server/values.yaml index 22bbdeace0..f0bc55a646 100644 --- a/infra/charts/feast-feature-server/values.yaml +++ b/infra/charts/feast-feature-server/values.yaml @@ -11,6 +11,8 @@ image: # image.tag -- The Docker image tag (can be overwritten if custom feature server deps are needed for on demand transforms) tag: 0.40.0 +logLevel: "WARNING" # Set log level DEBUG, INFO, WARNING, ERROR, and CRITICAL (case-insensitive) + imagePullSecrets: [] nameOverride: "" fullnameOverride: "" diff --git a/sdk/python/feast/permissions/auth/kubernetes_token_parser.py b/sdk/python/feast/permissions/auth/kubernetes_token_parser.py index 7724163e5f..c34ebf386d 100644 --- a/sdk/python/feast/permissions/auth/kubernetes_token_parser.py +++ b/sdk/python/feast/permissions/auth/kubernetes_token_parser.py @@ -40,14 +40,16 @@ async def user_details_from_access_token(self, access_token: str) -> User: """ sa_namespace, sa_name = _decode_token(access_token) current_user = f"{sa_namespace}:{sa_name}" - logging.info(f"Received request from {sa_name} in {sa_namespace}") + logger.info( + f"Request received from ServiceAccount: {sa_name} in namespace: {sa_namespace}" + ) intra_communication_base64 = os.getenv("INTRA_COMMUNICATION_BASE64") if sa_name is not None and sa_name == intra_communication_base64: return User(username=sa_name, roles=[]) else: roles = self.get_roles(sa_namespace, sa_name) - logging.info(f"SA roles are: {roles}") + logger.info(f"Roles for ServiceAccount {sa_name}: {roles}") return User(username=current_user, roles=roles) diff --git a/sdk/python/feast/permissions/auth/oidc_token_parser.py b/sdk/python/feast/permissions/auth/oidc_token_parser.py index 28273e8c10..ffff7e7ad3 100644 --- a/sdk/python/feast/permissions/auth/oidc_token_parser.py +++ b/sdk/python/feast/permissions/auth/oidc_token_parser.py @@ -17,7 +17,6 @@ from feast.permissions.user import User logger = logging.getLogger(__name__) -logger.setLevel(logging.INFO) class OidcTokenParser(TokenParser): @@ -69,8 +68,9 @@ async def user_details_from_access_token(self, access_token: str) -> User: try: await self._validate_token(access_token) - logger.info("Validated token") + logger.debug("Token successfully validated.") except Exception as e: + logger.error(f"Token validation failed: {e}") raise AuthenticationError(f"Invalid token: {e}") optional_custom_headers = {"User-agent": "custom-user-agent"} diff --git a/sdk/python/feast/permissions/client/intra_comm_authentication_client_manager.py b/sdk/python/feast/permissions/client/intra_comm_authentication_client_manager.py index 678e1f39e5..30476316c1 100644 --- a/sdk/python/feast/permissions/client/intra_comm_authentication_client_manager.py +++ b/sdk/python/feast/permissions/client/intra_comm_authentication_client_manager.py @@ -13,6 +13,7 @@ class IntraCommAuthClientManager(AuthenticationClientManager): def __init__(self, auth_config: AuthConfig, intra_communication_base64: str): self.auth_config = auth_config self.intra_communication_base64 = intra_communication_base64 + logger.debug(f"AuthConfig type set to {self.auth_config.type}") def get_token(self): if self.auth_config.type == AuthType.OIDC.value: diff --git a/sdk/python/feast/permissions/enforcer.py b/sdk/python/feast/permissions/enforcer.py index d94a81ba04..d9855fef8c 100644 --- a/sdk/python/feast/permissions/enforcer.py +++ b/sdk/python/feast/permissions/enforcer.py @@ -67,8 +67,12 @@ def enforce_policy( if evaluator.is_decided(): grant, explanations = evaluator.grant() if not grant and not filter_only: + logger.error(f"Permission denied: {','.join(explanations)}") raise FeastPermissionError(",".join(explanations)) if grant: + logger.debug( + f"Permission granted for {type(resource).__name__}:{resource.name}" + ) _permitted_resources.append(resource) break else: diff --git a/sdk/python/feast/permissions/server/arrow.py b/sdk/python/feast/permissions/server/arrow.py index 4f0afc3ee5..bf517d94ac 100644 --- a/sdk/python/feast/permissions/server/arrow.py +++ b/sdk/python/feast/permissions/server/arrow.py @@ -17,7 +17,6 @@ from feast.permissions.user import User logger = logging.getLogger(__name__) -logger.setLevel(logging.INFO) class AuthorizationMiddlewareFactory(fl.ServerMiddlewareFactory): @@ -49,7 +48,9 @@ def __init__(self, access_token: str, *args, **kwargs): def call_completed(self, exception): if exception: - print(f"{AuthorizationMiddleware.__name__} received {exception}") + logger.exception( + f"{AuthorizationMiddleware.__name__} encountered an exception: {exception}" + ) async def extract_user(self) -> User: """ @@ -69,14 +70,14 @@ def inject_user_details(context: ServerCallContext): context: The endpoint context. """ if context.get_middleware("auth") is None: - logger.info("No `auth` middleware.") + logger.warning("No `auth` middleware.") return sm = get_security_manager() if sm is not None: auth_middleware = cast(AuthorizationMiddleware, context.get_middleware("auth")) current_user = asyncio.run(auth_middleware.extract_user()) - print(f"extracted user: {current_user}") + logger.debug(f"User extracted: {current_user}") sm.set_current_user(current_user) diff --git a/sdk/python/feast/permissions/server/grpc.py b/sdk/python/feast/permissions/server/grpc.py index 96f2690b88..9feea47a6c 100644 --- a/sdk/python/feast/permissions/server/grpc.py +++ b/sdk/python/feast/permissions/server/grpc.py @@ -9,7 +9,6 @@ from feast.permissions.security_manager import get_security_manager logger = logging.getLogger(__name__) -logger.setLevel(logging.INFO) class AuthInterceptor(grpc.ServerInterceptor): @@ -22,11 +21,13 @@ def intercept_service(self, continuation, handler_call_details): metadata=dict(handler_call_details.invocation_metadata) ) - print(f"Fetching user for token: {len(access_token)}") + logger.debug( + f"Fetching user details for token of length: {len(access_token)}" + ) current_user = asyncio.run( auth_manager.token_parser.user_details_from_access_token(access_token) ) - print(f"User is: {current_user}") + logger.debug(f"User is: {current_user}") sm.set_current_user(current_user) return continuation(handler_call_details) diff --git a/sdk/python/feast/permissions/server/utils.py b/sdk/python/feast/permissions/server/utils.py index 9a8b319dbc..cd72ae5820 100644 --- a/sdk/python/feast/permissions/server/utils.py +++ b/sdk/python/feast/permissions/server/utils.py @@ -30,7 +30,6 @@ from feast.permissions.server.rest_token_extractor import RestTokenExtractor logger = logging.getLogger(__name__) -logger.setLevel(logging.INFO) class ServerType(enum.Enum):