Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Commit

Permalink
closes #36- masks params provided in % str formatter in logs, unless …
Browse files Browse the repository at this point in the history
…set otherwise (#68)

* closes #36- masks params provided in % str formatter in logs, unless set otherwise

* remove extra invalid param

* cr changes

* tests

Co-authored-by: catherinesmith <[email protected]>
  • Loading branch information
eastandwestwind and catherinesmith authored Nov 18, 2021
1 parent 2cd7182 commit 3acd69e
Show file tree
Hide file tree
Showing 23 changed files with 107 additions and 62 deletions.
1 change: 1 addition & 0 deletions docs/fidesops/docs/development/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ commands to give you different functionality.
- `ipython` - open a Python shell
- `make pytest` - runs all unit tests except those that talk to integration databases
- `make pytest-integration-access` - runs access integration tests
- `make pytest-integration-erasure` - runs erasure integration tests
- `make reset-db` - tears down the Fideops postgres db, then recreates and re-runs migrations.
- `make quickstart` - runs a quick, five second quickstart that talks to the Fidesops API to execute privacy requests
- `make check-migrations` - verifies there are no un-run migrations
Expand Down
1 change: 0 additions & 1 deletion docs/fidesops/docs/tutorial/outline.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import yaml
from datetime import datetime

logger = logging.getLogger(__name__)
logging.basicConfig(level=logging.INFO)

# For tutorial simplicity. In prod, this should go in an ENV file or similar.
FIDESOPS_URL = "http://localhost:8000"
Expand Down
5 changes: 4 additions & 1 deletion src/fidesops/api/v1/endpoints/connection_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
CONNECTION_DELETE,
CONNECTION_CREATE_OR_UPDATE,
)
from fidesops.util.logger import NotPii
from fidesops.util.oauth_util import verify_oauth_client
from fidesops.api import deps
from fidesops.models.connectionconfig import ConnectionConfig
Expand Down Expand Up @@ -191,7 +192,9 @@ def connection_status(
try:
connector.test_connection()
except ConnectionException as exc:
logger.warning(f"Connection test failed on {connection_config.key}: {str(exc)}")
logger.warning(
"Connection test failed on %s: %s", NotPii(connection_config.key), str(exc)
)
connection_config.update_test_status(succeeded=False, db=db)
return TestStatusMessage(
msg=msg,
Expand Down
4 changes: 2 additions & 2 deletions src/fidesops/api/v1/endpoints/policy_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,15 @@ def create_or_update_policies(
},
)
except KeyOrNameAlreadyExists as exc:
logger.warning(f"Create/update failed for policy: {exc}")
logger.warning("Create/update failed for policy: %s", exc)
failure = {
"message": exc.args[0],
"data": policy_data,
}
failed.append(BulkUpdateFailed(**failure))
continue
except PolicyValidationError as exc:
logger.warning(f"Create/update failed for policy: {exc}")
logger.warning("Create/update failed for policy: %s", exc)
failure = {
"message": "This record could not be added because the data provided was invalid.",
"data": policy_data,
Expand Down
4 changes: 2 additions & 2 deletions src/fidesops/api/v1/endpoints/privacy_request_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,14 @@ def create_privacy_request(
privacy_request=privacy_request,
).run()
except common_exceptions.RedisConnectionError as exc:
logger.error(exc)
logger.error("RedisConnectionError: %s", exc)
# Thrown when cache.ping() fails on cache connection retrieval
raise HTTPException(
status_code=HTTP_424_FAILED_DEPENDENCY,
detail=exc.args[0],
)
except Exception as exc:
logger.error(exc)
logger.error("Exception: %s", exc)
failure = {
"message": "This record could not be added",
"data": kwargs,
Expand Down
9 changes: 5 additions & 4 deletions src/fidesops/core/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from pydantic.env_settings import SettingsSourceCallable

from fidesops.common_exceptions import MissingConfig
from fidesops.util.logger import NotPii

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -201,9 +202,9 @@ def load_file(file_name: str) -> str:
for dir_str in directories:
possible_location = os.path.join(dir_str, file_name)
if possible_location and os.path.isfile(possible_location):
logger.info("Loading file %s from %s", file_name, dir_str)
logger.info("Loading file %s from %s", NotPii(file_name), NotPii(dir_str))
return possible_location
logger.debug("%s not found at %s", file_name, dir_str)
logger.debug("%s not found at %s", NotPii(file_name), NotPii(dir_str))
raise FileNotFoundError


Expand All @@ -229,14 +230,14 @@ def get_config() -> FidesopsConfig:
try:
return FidesopsConfig.parse_obj(load_toml("fidesops.toml"))
except (FileNotFoundError, ValidationError) as e:
logger.warning("fidesops.toml could not be loaded: %s", e)
logger.warning("fidesops.toml could not be loaded: %s", NotPii(e))
# If no path is specified Pydantic will attempt to read settings from
# the environment. Default values will still be used if the matching
# environment variable is not set.
try:
return FidesopsConfig()
except ValidationError as exc:
logger.error(exc)
logger.error("ValidationError: %s", exc)
# If FidesopsConfig is missing any required values Pydantic will throw
# an ImportError. This means the config has not been correctly specified
# so we can throw the missing config error.
Expand Down
2 changes: 1 addition & 1 deletion src/fidesops/db/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def commit(self) -> None:
try:
return super().commit()
except Exception as exc:
logger.error(exc)
logger.error("Exception: %s", exc)
# Rollback the current transaction after each failed commit
self.rollback()
raise
1 change: 0 additions & 1 deletion src/fidesops/graph/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
Field,
)

logging.basicConfig(level=logging.INFO)
logger = logging.getLogger(__name__)


Expand Down
4 changes: 2 additions & 2 deletions src/fidesops/graph/traversal.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
ROOT_COLLECTION_ADDRESS,
)
from fidesops.graph.graph import Node, Edge, DatasetGraph
from fidesops.util.logger import NotPii
from fidesops.util.queue import Queue
from fidesops.util.collection_util import append

logging.basicConfig(level=logging.INFO)
logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -174,7 +174,7 @@ def __verify_traversal(self) -> None:
and raises an error on any traversal failure conditions."""
self.traverse(
{self.root_node.address: [self.seed_data]},
lambda n, m: logger.info("Traverse %s", n.address),
lambda n, m: logger.info("Traverse %s", NotPii(n.address)),
)

def traversal_map(
Expand Down
2 changes: 2 additions & 0 deletions src/fidesops/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
from fidesops.db.database import init_db
from fidesops.core.config import config
from fidesops.tasks.scheduled.tasks import initiate_scheduled_request_intake
from fidesops.util.logger import get_fides_log_record_factory

logging.basicConfig(level=logging.INFO)
logging.setLogRecordFactory(get_fides_log_record_factory())
logger = logging.getLogger(__name__)

app = FastAPI(title="fidesops", openapi_url=f"{V1_URL_PREFIX}/openapi.json")
Expand Down
11 changes: 6 additions & 5 deletions src/fidesops/models/datasetconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
)
from fidesops.models.connectionconfig import ConnectionConfig
from fidesops.schemas.dataset import FidesopsDataset, FidesopsDatasetField
from fidesops.util.logger import NotPii

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -153,9 +154,9 @@ def convert_dataset_to_graph(dataset: FidesopsDataset, connection_key: str) -> D
]
logger.debug(
"Parsing dataset %s: parsed collection %s with %s fields",
dataset_name,
collection.name,
len(graph_fields),
NotPii(dataset_name),
NotPii(collection.name),
NotPii(len(graph_fields)),
)
collection_after: Set[CollectionAddress] = set()
if collection.fidesops_meta and collection.fidesops_meta.after:
Expand All @@ -169,8 +170,8 @@ def convert_dataset_to_graph(dataset: FidesopsDataset, connection_key: str) -> D
graph_collections.append(graph_collection)
logger.debug(
"Finished parsing dataset %s with %s collections",
dataset_name,
len(graph_collections),
NotPii(dataset_name),
NotPii(len(graph_collections)),
)

return Dataset(
Expand Down
3 changes: 0 additions & 3 deletions src/fidesops/models/privacy_request.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# pylint: disable=R0401
import logging
from typing import Any, Dict

from enum import Enum as EnumType
Expand All @@ -26,8 +25,6 @@
FidesopsRedis,
)

logging.basicConfig(level=logging.INFO)


class PrivacyRequestStatus(EnumType):
"""Enum for privacy request statuses, reflecting where they are in the Privacy Request Lifecycle"""
Expand Down
2 changes: 1 addition & 1 deletion src/fidesops/models/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def set_secrets(
KeyError,
ValidationError,
) as exc:
logger.error(exc)
logger.error("Error: %s", exc)
# We don't want to handle these explicitly here, only in the API view
raise

Expand Down
9 changes: 1 addition & 8 deletions src/fidesops/service/connectors/base_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from fidesops.models.policy import Policy
from fidesops.service.connectors.query_config import QueryConfig

logging.basicConfig(level=logging.INFO)
logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -52,13 +51,7 @@ def retrieve_data(
each input key that may be queried on."""

@abstractmethod
def mask_data(
self,
node: TraversalNode,
policy: Policy,
rows: List[Row],
log_queries_with_data: bool = False,
) -> int:
def mask_data(self, node: TraversalNode, policy: Policy, rows: List[Row]) -> int:
"""Execute a masking request. Return the number of rows that have been updated"""

def dry_run_query(self, node: TraversalNode) -> str:
Expand Down
21 changes: 8 additions & 13 deletions src/fidesops/service/connectors/mongodb_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
BaseConnector,
)
from fidesops.service.connectors.query_config import QueryConfig, MongoQueryConfig
from fidesops.util.logger import NotPii

logging.basicConfig(level=logging.INFO)
logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -103,13 +103,7 @@ def retrieve_data(
logger.info(f"Found {len(rows)} on {node.address}")
return rows

def mask_data(
self,
node: TraversalNode,
policy: Policy,
rows: List[Row],
log_queries_with_data: bool = True,
) -> int:
def mask_data(self, node: TraversalNode, policy: Policy, rows: List[Row]) -> int:
# pylint: disable=too-many-locals
"""Execute a masking request"""
query_config = self.query_config(node)
Expand All @@ -124,10 +118,11 @@ def mask_data(
collection = db[collection_name]
update_result = collection.update_one(query, update, upsert=False)
update_ct += update_result.modified_count

if log_queries_with_data:
logger.info(
f"db.{collection_name}.update_one({query}, {update}, upsert=False)"
)
logger.info(
"db.%s.update_one(%s, %s, upsert=False)",
NotPii(collection_name),
query,
update,
)

return update_ct
1 change: 0 additions & 1 deletion src/fidesops/service/connectors/query_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from fidesops.service.masking.strategy.masking_strategy_factory import get_strategy
from fidesops.util.collection_util import append

logging.basicConfig(level=logging.INFO)
logger = logging.getLogger(__name__)
T = TypeVar("T")

Expand Down
9 changes: 1 addition & 8 deletions src/fidesops/service/connectors/sql_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
)
from fidesops.service.connectors.query_config import SQLQueryConfig

logging.basicConfig(level=logging.INFO)
logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -87,13 +86,7 @@ def retrieve_data(
results = connection.execute(stmt)
return SQLConnector.cursor_result_to_rows(results)

def mask_data(
self,
node: TraversalNode,
policy: Policy,
rows: List[Row],
log_queries_with_data: bool = True,
) -> int:
def mask_data(self, node: TraversalNode, policy: Policy, rows: List[Row]) -> int:
"""Execute a masking request. Returns the number of records masked"""
query_config = self.query_config(node)
update_ct = 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
)
from fidesops.util.cache import FidesopsRedis

logging.basicConfig(level=logging.INFO)


class PrivacyRequestRunner:
"""The class responsible for dispatching PrivacyRequests into the execution layer"""
Expand Down
10 changes: 5 additions & 5 deletions src/fidesops/task/graph_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
from fidesops.service.connectors import BaseConnector
from fidesops.task.task_resources import TaskResources
from fidesops.util.collection_util import partition, append
from fidesops.util.logger import NotPii

logging.basicConfig(level=logging.INFO)
logger = logging.getLogger(__name__)

dask.config.set(scheduler="processes")
Expand Down Expand Up @@ -142,9 +142,9 @@ def to_dask_input_data(self, *data: List[Row]) -> Dict[str, List[Any]]:
if not len(data) == len(self.input_keys):
logger.warning(
"%s expected %s input keys, received %s",
self,
len(self.input_keys),
len(data),
NotPii(self),
NotPii(len(self.input_keys)),
NotPii(len(data)),
)

output: Dict[str, List[Any]] = {}
Expand Down Expand Up @@ -249,7 +249,7 @@ def erasure_request(self, retrieved_data: List[Row]) -> int:
return 0

output = self.connector.mask_data(
self.traversal_node, self.resources.policy, retrieved_data, True
self.traversal_node, self.resources.policy, retrieved_data
)
self.log_end(ActionType.erasure)
return output
Expand Down
1 change: 0 additions & 1 deletion src/fidesops/task/task_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
)
from fidesops.util.cache import get_cache

logging.basicConfig(level=logging.INFO)
logger = logging.getLogger(__name__)


Expand Down
Loading

0 comments on commit 3acd69e

Please sign in to comment.