From 782b593d982e7d4dd4150f9e37b3a686b5b9efaf Mon Sep 17 00:00:00 2001 From: Thomas Date: Mon, 11 Jul 2022 22:39:17 -0700 Subject: [PATCH 1/5] Add "generate" & "scan" to the local commands (#841) * Add "generate" & "scan" to the local commands * Update changelog, move scan and generate to local commands, deepcopy the cli for the cli_docs * fix a bug that caused the CLI to break if a subcommand was invoked * fix the auto-generating CLI docs --- CHANGELOG.md | 6 ++---- docs/fides/docs/cli.md | 2 +- src/fidesctl/cli/__init__.py | 23 ++++++++++------------- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8604040521..7bf3458a23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,9 +35,11 @@ The types of changes are: ### Changed * Updated the `datamap` endpoint to return human-readable column names as the first response item [#779](https://github.com/ethyca/fides/pull/779) +* Moved `scan` and `generate` to the list of commands that can be run in local mode [#841](https://github.com/ethyca/fides/pull/841) * Webserver dependencies now come as a standard part of the package [#881](https://github.com/ethyca/fides/pull/881) * Initial configuration wizard UI view * Refactored step & form results management to use Redux Toolkit slice. +* Remove the `obscure` requirement from the `generate` endpoint [#819](https://github.com/ethyca/fides/pull/819) ### Docs @@ -49,10 +51,6 @@ The types of changes are: * Datasets without the `third_country_transfer` will not cause the editing dataset form to not render. * Fixed a build issue causing an `unknown` version of `fidesctl` to be installed in published Docker images [#836](https://github.com/ethyca/fides/pull/836) -### Changed - -* Remove the `obscure` requirement from the `generate` endpoint [#819](https://github.com/ethyca/fides/pull/819) - ## [1.7.0](https://github.com/ethyca/fides/compare/1.6.1...1.7.0) - 2022-06-23 ### Added diff --git a/docs/fides/docs/cli.md b/docs/fides/docs/cli.md index ed82acefa3..813c1e6349 100644 --- a/docs/fides/docs/cli.md +++ b/docs/fides/docs/cli.md @@ -6,5 +6,5 @@ These docs reflect the latest PyPI release. ::: mkdocs-click :module: fidesctl.cli - :command: cli_docs + :command: cli :depth: 1 diff --git a/src/fidesctl/cli/__init__.py b/src/fidesctl/cli/__init__.py index 0fe4b8eed0..6343c3504e 100644 --- a/src/fidesctl/cli/__init__.py +++ b/src/fidesctl/cli/__init__.py @@ -21,20 +21,22 @@ from .commands.view import view CONTEXT_SETTINGS = dict(help_option_names=["-h", "--help"]) -LOCAL_COMMANDS = [evaluate, init, parse, view, webserver] +LOCAL_COMMANDS = [evaluate, generate, init, scan, parse, view, webserver] +LOCAL_COMMAND_DICT = { + command.name or str(command): command for command in LOCAL_COMMANDS +} API_COMMANDS = [ annotate, apply, database, delete, export, - generate, get, ls, - scan, status, sync, ] +API_COMMAND_DICT = {command.name or str(command): command for command in API_COMMANDS} ALL_COMMANDS = API_COMMANDS + LOCAL_COMMANDS SERVER_CHECK_COMMAND_NAMES = [ command.name for command in API_COMMANDS if command.name not in ["status"] @@ -70,14 +72,12 @@ def cli(ctx: click.Context, config_path: str, local: bool) -> None: ctx.ensure_object(dict) config = get_config(config_path) - for command in LOCAL_COMMANDS: - cli.add_command(command) + # Dyanmically add commands to the CLI + cli.commands = LOCAL_COMMAND_DICT - # If local_mode is enabled, don't add unsupported commands if not (local or config.cli.local_mode): config.cli.local_mode = False - for command in API_COMMANDS: - cli.add_command(command) + cli.commands = {**cli.commands, **API_COMMAND_DICT} else: config.cli.local_mode = True @@ -107,9 +107,6 @@ def cli(ctx: click.Context, config_path: str, local: bool) -> None: ) -# This is a special section used for auto-generating the CLI docs -# This has to be done due to the dynamic way in which commands are added for the real CLI group -cli_docs = cli - +# Add all commands here before dynamically checking them in the CLI for cli_command in ALL_COMMANDS: - cli_docs.add_command(cli_command) + cli.add_command(cli_command) From 7f3fcad6ead71dbd1706246ffa84246e4b8ba241 Mon Sep 17 00:00:00 2001 From: Allison King Date: Tue, 12 Jul 2022 10:06:01 -0400 Subject: [PATCH 2/5] Integrate generating a dataset from a database connection (#889) * Update types for new db generate functionality * Integrate adding a dataset via a database * Clean up * Update changelog --- CHANGELOG.md | 2 +- .../admin-ui/src/features/common/helpers.ts | 29 ++---- .../features/dataset/DatabaseConnectForm.tsx | 92 ++++++++++++++++--- .../src/features/dataset/DatasetYamlForm.tsx | 8 +- .../src/features/dataset/dataset.slice.ts | 9 ++ clients/admin-ui/src/types/api/index.ts | 1 + .../src/types/api/models/DatabaseConfig.ts | 10 ++ .../admin-ui/src/types/api/models/Generate.ts | 3 +- .../src/types/api/models/ValidTargets.ts | 1 + 9 files changed, 116 insertions(+), 39 deletions(-) create mode 100644 clients/admin-ui/src/types/api/models/DatabaseConfig.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bf3458a23..f2b5e6dd06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ The types of changes are: ### Added * Add datasets via YAML in the UI [#813](https://github.com/ethyca/fides/pull/813) -* Add datasets via database connection (UI only) [#834](https://github.com/ethyca/fides/pull/834) +* Add datasets via database connection [#834](https://github.com/ethyca/fides/pull/834) [#889](https://github.com/ethyca/fides/pull/889) * Add delete confirmation when deleting a field or collection from a dataset [#809](https://github.com/ethyca/fides/pull/809) * Add ability to delete datasets from the UI * Initial configuration wizard UI view diff --git a/clients/admin-ui/src/features/common/helpers.ts b/clients/admin-ui/src/features/common/helpers.ts index 1ac641ae38..3d31d6672c 100644 --- a/clients/admin-ui/src/features/common/helpers.ts +++ b/clients/admin-ui/src/features/common/helpers.ts @@ -78,25 +78,14 @@ export function isYamlException(error: unknown): error is YAMLException { ); } -type RTKReturnType = - | { - data: any; - } - | { - error: FetchBaseQueryError | SerializedError; - }; -export function getErrorFromResult(result: RTKReturnType) { - if ("error" in result) { - const { error } = result; - let errorMsg = "An unexpected error occurred. Please try again."; - if (isErrorWithDetail(error)) { - errorMsg = error.data.detail; - } else if (isErrorWithDetailArray(error)) { - errorMsg = `${error.data.detail[0].msg}: ${error.data.detail[0].loc}`; - } else if (isConflictError(error)) { - errorMsg = `${error.data.detail.error} (${error.data.detail.fides_key})`; - } - return errorMsg; +export function getErrorMessage(error: FetchBaseQueryError | SerializedError) { + let errorMsg = "An unexpected error occurred. Please try again."; + if (isErrorWithDetail(error)) { + errorMsg = error.data.detail; + } else if (isErrorWithDetailArray(error)) { + errorMsg = `${error.data.detail[0].msg}: ${error.data.detail[0].loc}`; + } else if (isConflictError(error)) { + errorMsg = `${error.data.detail.error} (${error.data.detail.fides_key})`; } - return null; + return errorMsg; } diff --git a/clients/admin-ui/src/features/dataset/DatabaseConnectForm.tsx b/clients/admin-ui/src/features/dataset/DatabaseConnectForm.tsx index 1de83ce7ab..2f6c629b3e 100644 --- a/clients/admin-ui/src/features/dataset/DatabaseConnectForm.tsx +++ b/clients/admin-ui/src/features/dataset/DatabaseConnectForm.tsx @@ -1,25 +1,87 @@ -import { Box, Button, Text } from "@fidesui/react"; -import { Form, Formik } from "formik"; +import { Box, Button, Spinner, Text, useToast } from "@fidesui/react"; +import { SerializedError } from "@reduxjs/toolkit"; +import { FetchBaseQueryError } from "@reduxjs/toolkit/dist/query"; +import { Form, Formik, FormikHelpers } from "formik"; +import { useRouter } from "next/router"; import * as Yup from "yup"; +import { GenerateResponse, GenerateTypes, ValidTargets } from "~/types/api"; + import { CustomTextInput } from "../common/form/inputs"; +import { getErrorMessage } from "../common/helpers"; +import { successToastParams } from "../common/toast"; +import { + setActiveDataset, + useCreateDatasetMutation, + useGenerateDatasetMutation, +} from "./dataset.slice"; +import { Dataset } from "./types"; const initialValues = { url: "" }; +type FormValues = typeof initialValues; + const ValidationSchema = Yup.object().shape({ url: Yup.string().required().label("Database URL"), }); const DatabaseConnectForm = () => { - const handleCreate = () => { - // TODO when backend supports this (829) + // TODO: where should this come from? + const organizationKey = "default_organization"; + const [generate, { isLoading: isGenerating }] = useGenerateDatasetMutation(); + const [createDataset, { isLoading: isCreating }] = useCreateDatasetMutation(); + const toast = useToast(); + const router = useRouter(); + + const handleSubmit = async ( + values: FormValues, + formikHelpers: FormikHelpers + ) => { + const { setErrors } = formikHelpers; + + const handleError = (error: FetchBaseQueryError | SerializedError) => { + const errorMessage = getErrorMessage(error); + setErrors({ url: errorMessage }); + }; + + const handleGenerateResults = async ( + results: GenerateResponse["generate_results"] + ) => { + if (results && results.length) { + const newDataset = results[0] as Dataset; + const createResult = await createDataset(newDataset); + if ("error" in createResult) { + handleError(createResult.error); + } else { + toast(successToastParams("Successfully loaded new dataset")); + setActiveDataset(createResult.data); + router.push(`/dataset/${createResult.data.fides_key}`); + } + } + }; + + const response = await generate({ + organization_key: organizationKey, + generate: { + config: { connection_string: values.url }, + target: ValidTargets.DB, + type: GenerateTypes.DATASETS, + }, + }); + + if ("error" in response) { + const errorMessage = getErrorMessage(response.error); + setErrors({ url: errorMessage }); + } else { + handleGenerateResults(response.data.generate_results); + } }; return ( @@ -33,14 +95,18 @@ const DatabaseConnectForm = () => { - + + + {isGenerating || isCreating ? : null} + )} diff --git a/clients/admin-ui/src/features/dataset/DatasetYamlForm.tsx b/clients/admin-ui/src/features/dataset/DatasetYamlForm.tsx index b0161910dd..a133f8f2d3 100644 --- a/clients/admin-ui/src/features/dataset/DatasetYamlForm.tsx +++ b/clients/admin-ui/src/features/dataset/DatasetYamlForm.tsx @@ -4,7 +4,7 @@ import yaml from "js-yaml"; import { useRouter } from "next/router"; import { CustomTextArea } from "../common/form/inputs"; -import { getErrorFromResult, isYamlException } from "../common/helpers"; +import { getErrorMessage, isYamlException } from "../common/helpers"; import { successToastParams } from "../common/toast"; import { setActiveDataset, useCreateDatasetMutation } from "./dataset.slice"; import { Dataset } from "./types"; @@ -44,9 +44,9 @@ const DatasetYamlForm = () => { } const result = await createDataset(dataset); - const error = getErrorFromResult(result); - if (error) { - setErrors({ datasetYaml: error }); + if ("error" in result) { + const errorMessage = getErrorMessage(result.error); + setErrors({ datasetYaml: errorMessage }); } else if ("data" in result) { toast(successToastParams("Successfully loaded new dataset YAML")); setActiveDataset(result.data); diff --git a/clients/admin-ui/src/features/dataset/dataset.slice.ts b/clients/admin-ui/src/features/dataset/dataset.slice.ts index 72d2dff0f0..00c6350d55 100644 --- a/clients/admin-ui/src/features/dataset/dataset.slice.ts +++ b/clients/admin-ui/src/features/dataset/dataset.slice.ts @@ -3,6 +3,7 @@ import { createApi, fetchBaseQuery } from "@reduxjs/toolkit/query/react"; import { HYDRATE } from "next-redux-wrapper"; import type { AppState } from "~/app/store"; +import { GenerateRequestPayload, GenerateResponse } from "~/types/api"; import { FidesKey } from "../common/fides-types"; import { Dataset } from "./types"; @@ -72,6 +73,13 @@ export const datasetApi = createApi({ }), invalidatesTags: ["Datasets"], }), + generateDataset: build.mutation({ + query: (payload) => ({ + url: `generate/`, + method: "POST", + body: payload, + }), + }), }), }); @@ -81,6 +89,7 @@ export const { useUpdateDatasetMutation, useCreateDatasetMutation, useDeleteDatasetMutation, + useGenerateDatasetMutation, } = datasetApi; export const datasetSlice = createSlice({ diff --git a/clients/admin-ui/src/types/api/index.ts b/clients/admin-ui/src/types/api/index.ts index 3a0efa802f..ccfc18b4bc 100644 --- a/clients/admin-ui/src/types/api/index.ts +++ b/clients/admin-ui/src/types/api/index.ts @@ -4,6 +4,7 @@ export type { AWSConfig } from './models/AWSConfig'; export type { ContactDetails } from './models/ContactDetails'; +export type { DatabaseConfig } from './models/DatabaseConfig'; export type { DataCategory } from './models/DataCategory'; export type { DataProtectionImpactAssessment } from './models/DataProtectionImpactAssessment'; export type { DataQualifier } from './models/DataQualifier'; diff --git a/clients/admin-ui/src/types/api/models/DatabaseConfig.ts b/clients/admin-ui/src/types/api/models/DatabaseConfig.ts new file mode 100644 index 0000000000..d332f5be66 --- /dev/null +++ b/clients/admin-ui/src/types/api/models/DatabaseConfig.ts @@ -0,0 +1,10 @@ +/* istanbul ignore file */ +/* tslint:disable */ +/* eslint-disable */ + +/** + * The model for the connection config for databases + */ +export type DatabaseConfig = { + connection_string: string; +}; diff --git a/clients/admin-ui/src/types/api/models/Generate.ts b/clients/admin-ui/src/types/api/models/Generate.ts index 246fc1c837..121d66c1cb 100644 --- a/clients/admin-ui/src/types/api/models/Generate.ts +++ b/clients/admin-ui/src/types/api/models/Generate.ts @@ -3,6 +3,7 @@ /* eslint-disable */ import type { AWSConfig } from './AWSConfig'; +import type { DatabaseConfig } from './DatabaseConfig'; import type { GenerateTypes } from './GenerateTypes'; import type { OktaConfig } from './OktaConfig'; import type { ValidTargets } from './ValidTargets'; @@ -11,7 +12,7 @@ import type { ValidTargets } from './ValidTargets'; * Defines attributes for generating resources included in a request. */ export type Generate = { - config: (AWSConfig | OktaConfig); + config: (AWSConfig | OktaConfig | DatabaseConfig); target: ValidTargets; type: GenerateTypes; }; diff --git a/clients/admin-ui/src/types/api/models/ValidTargets.ts b/clients/admin-ui/src/types/api/models/ValidTargets.ts index 54138cdeac..0f97d47144 100644 --- a/clients/admin-ui/src/types/api/models/ValidTargets.ts +++ b/clients/admin-ui/src/types/api/models/ValidTargets.ts @@ -7,5 +7,6 @@ */ export enum ValidTargets { AWS = 'aws', + DB = 'db', OKTA = 'okta', } \ No newline at end of file From 4065494323d37b8dedf72ef1ec38fc19d2d28db7 Mon Sep 17 00:00:00 2001 From: Allison King Date: Tue, 12 Jul 2022 10:09:23 -0400 Subject: [PATCH 3/5] Make endpoints work with or without a trailing slash (#886) * Use custom APIRouter * Add tests * Refactor main.py to separate out Health and Admin endpoints * Fix patch Co-authored-by: Thomas --- CHANGELOG.md | 1 + src/fidesapi/database/database.py | 10 ++++ src/fidesapi/main.py | 97 ++++--------------------------- src/fidesapi/routes/admin.py | 29 +++++++++ src/fidesapi/routes/crud.py | 3 +- src/fidesapi/routes/datamap.py | 3 +- src/fidesapi/routes/generate.py | 3 +- src/fidesapi/routes/health.py | 61 +++++++++++++++++++ src/fidesapi/routes/util.py | 3 +- src/fidesapi/routes/validate.py | 3 +- src/fidesapi/routes/visualize.py | 3 +- src/fidesapi/utils/api_router.py | 39 +++++++++++++ src/fidesapi/view.py | 2 +- tests/core/test_api.py | 15 ++++- 14 files changed, 177 insertions(+), 95 deletions(-) create mode 100644 src/fidesapi/routes/admin.py create mode 100644 src/fidesapi/routes/health.py create mode 100644 src/fidesapi/utils/api_router.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f2b5e6dd06..c399251365 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ The types of changes are: * CustomSelect input tooltips appear next to selector instead of wrapping to a new row. * Datasets without the `third_country_transfer` will not cause the editing dataset form to not render. * Fixed a build issue causing an `unknown` version of `fidesctl` to be installed in published Docker images [#836](https://github.com/ethyca/fides/pull/836) +* Endpoints now work with or without a trailing slash. [#886](https://github.com/ethyca/fides/pull/886) ## [1.7.0](https://github.com/ethyca/fides/compare/1.6.1...1.7.0) - 2022-06-23 diff --git a/src/fidesapi/database/database.py b/src/fidesapi/database/database.py index bfe3525612..d7e9da1123 100644 --- a/src/fidesapi/database/database.py +++ b/src/fidesapi/database/database.py @@ -133,3 +133,13 @@ def get_db_health(database_url: str) -> str: error_type = get_full_exception_name(error) log.error(f"Unable to reach the database: {error_type}: {error}") return "unhealthy" + + +async def configure_db(database_url: str) -> None: + "Set up the db to be used by the app." + try: + create_db_if_not_exists(database_url) + await init_db(database_url) + except Exception as error: # pylint: disable=broad-except + error_type = get_full_exception_name(error) + log.error(f"Unable to configure database: {error_type}: {error}") diff --git a/src/fidesapi/main.py b/src/fidesapi/main.py index 27a7467b63..446a70a5e6 100644 --- a/src/fidesapi/main.py +++ b/src/fidesapi/main.py @@ -2,10 +2,9 @@ Contains the code that sets up the API. """ from datetime import datetime -from enum import Enum from logging import WARNING from pathlib import Path -from typing import Callable, Dict +from typing import Callable from fastapi import FastAPI, HTTPException, Request, Response, status from fastapi.responses import FileResponse @@ -13,13 +12,10 @@ from loguru import logger as log from uvicorn import Config, Server -import fidesctl from fidesapi import view -from fidesapi.database import database -from fidesapi.database.database import get_db_health -from fidesapi.routes import crud, datamap, generate, validate, visualize +from fidesapi.database.database import configure_db +from fidesapi.routes import admin, crud, datamap, generate, health, validate, visualize from fidesapi.routes.util import API_PREFIX, WEBAPP_DIRECTORY, WEBAPP_INDEX -from fidesapi.utils.errors import get_full_exception_name from fidesapi.utils.logger import setup as setup_logging from fidesctl.core.config import FidesctlConfig, get_config @@ -28,7 +24,14 @@ ROUTERS = ( crud.routers + visualize.routers - + [view.router, generate.router, datamap.router, validate.router] + + [ + admin.router, + datamap.router, + generate.router, + health.router, + validate.router, + view.router, + ] ) @@ -42,16 +45,6 @@ def configure_routes() -> None: configure_routes() -async def configure_db(database_url: str) -> None: - "Set up the db to be used by the app." - try: - database.create_db_if_not_exists(database_url) - await database.init_db(database_url) - except Exception as error: # pylint: disable=broad-except - error_type = get_full_exception_name(error) - log.error(f"Unable to configure database: {error_type}: {error}") - - @app.on_event("startup") async def create_webapp_dir_if_not_exists() -> None: """Creates the webapp directory if it doesn't exist.""" @@ -92,74 +85,6 @@ async def log_request(request: Request, call_next: Callable) -> Response: return response -@app.get( - f"{API_PREFIX}/health", - response_model=Dict[str, str], - responses={ - status.HTTP_200_OK: { - "content": { - "application/json": { - "example": { - "status": "healthy", - "version": "1.0.0", - "database": "healthy", - } - } - } - }, - status.HTTP_503_SERVICE_UNAVAILABLE: { - "content": { - "application/json": { - "example": { - "detail": { - "status": "healthy", - "version": "1.0.0", - "database": "unhealthy", - } - } - } - } - }, - }, - tags=["Health"], -) -async def health() -> Dict: - "Confirm that the API is running and healthy." - database_health = get_db_health(CONFIG.api.sync_database_url) - response = { - "status": "healthy", - "version": str(fidesctl.__version__), - "database": database_health, - } - - for key in response: - if response[key] == "unhealthy": - raise HTTPException( - status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail=response - ) - - return response - - -class DBActions(str, Enum): - "The available path parameters for the `/admin/db/{action}` endpoint." - init = "init" - reset = "reset" - - -@app.post(API_PREFIX + "/admin/db/{action}", tags=["Admin"]) -async def db_action(action: DBActions) -> Dict: - """ - Initiate one of the enumerated DBActions. - """ - action_text = "initialized" - if action == DBActions.reset: - database.reset_db(CONFIG.api.sync_database_url) - action_text = DBActions.reset - await configure_db(CONFIG.api.sync_database_url) - return {"data": {"message": f"Fidesctl database {action_text}"}} - - # Configure the static file paths last since otherwise it will take over all paths @app.get("/", tags=["Default"]) def read_index() -> Response: diff --git a/src/fidesapi/routes/admin.py b/src/fidesapi/routes/admin.py new file mode 100644 index 0000000000..3d9d8884b3 --- /dev/null +++ b/src/fidesapi/routes/admin.py @@ -0,0 +1,29 @@ +from enum import Enum +from typing import Dict + +from fidesapi.database import database +from fidesapi.routes.util import API_PREFIX +from fidesapi.utils.api_router import APIRouter +from fidesctl.core.config import FidesctlConfig, get_config + +CONFIG: FidesctlConfig = get_config() +router = APIRouter(prefix=API_PREFIX, tags=["Admin"]) + + +class DBActions(str, Enum): + "The available path parameters for the `/admin/db/{action}` endpoint." + init = "init" + reset = "reset" + + +@router.post("/admin/db/{action}", tags=["Admin"]) +async def db_action(action: DBActions) -> Dict: + """ + Initiate one of the enumerated DBActions. + """ + action_text = "initialized" + if action == DBActions.reset: + database.reset_db(CONFIG.api.sync_database_url) + action_text = DBActions.reset + await database.configure_db(CONFIG.api.sync_database_url) + return {"data": {"message": f"Fidesctl database {action_text}"}} diff --git a/src/fidesapi/routes/crud.py b/src/fidesapi/routes/crud.py index 5dfa499417..8fd6bf84ae 100644 --- a/src/fidesapi/routes/crud.py +++ b/src/fidesapi/routes/crud.py @@ -8,7 +8,7 @@ from typing import Dict, List -from fastapi import APIRouter, Response, status +from fastapi import Response, status from fideslang import model_map from fidesapi.database.crud import ( @@ -21,6 +21,7 @@ ) from fidesapi.routes.util import API_PREFIX, get_resource_type from fidesapi.sql_models import sql_model_map +from fidesapi.utils.api_router import APIRouter # CRUD Endpoints routers = [] diff --git a/src/fidesapi/routes/datamap.py b/src/fidesapi/routes/datamap.py index 8a6d3ce26b..acc00ffd71 100644 --- a/src/fidesapi/routes/datamap.py +++ b/src/fidesapi/routes/datamap.py @@ -3,7 +3,7 @@ """ from typing import Dict, List -from fastapi import APIRouter, Response, status +from fastapi import Response, status from fideslang.parse import parse_dict from loguru import logger as log from pandas import DataFrame @@ -11,6 +11,7 @@ from fidesapi.routes.crud import get_resource, list_resource from fidesapi.routes.util import API_PREFIX from fidesapi.sql_models import sql_model_map +from fidesapi.utils.api_router import APIRouter from fidesapi.utils.errors import DatabaseUnavailableError, NotFoundError from fidesctl.core.export import build_joined_dataframe from fidesctl.core.export_helpers import DATAMAP_COLUMNS diff --git a/src/fidesapi/routes/generate.py b/src/fidesapi/routes/generate.py index 36ac6aa423..b495498782 100644 --- a/src/fidesapi/routes/generate.py +++ b/src/fidesapi/routes/generate.py @@ -4,7 +4,7 @@ from enum import Enum from typing import Dict, List, Optional, Union -from fastapi import APIRouter, HTTPException, Response, status +from fastapi import HTTPException, Response, status from fideslang.models import Dataset, Organization, System from loguru import logger as log from pydantic import BaseModel, root_validator @@ -16,6 +16,7 @@ route_requires_okta_connector, ) from fidesapi.sql_models import sql_model_map +from fidesapi.utils.api_router import APIRouter from fidesctl.connectors.models import ( AWSConfig, ConnectorAuthFailureException, diff --git a/src/fidesapi/routes/health.py b/src/fidesapi/routes/health.py new file mode 100644 index 0000000000..589e450c13 --- /dev/null +++ b/src/fidesapi/routes/health.py @@ -0,0 +1,61 @@ +from typing import Dict + +from fastapi import HTTPException, status + +import fidesctl +from fidesapi.database.database import get_db_health +from fidesapi.routes.util import API_PREFIX +from fidesapi.utils.api_router import APIRouter +from fidesctl.core.config import FidesctlConfig, get_config + +CONFIG: FidesctlConfig = get_config() + +router = APIRouter(prefix=API_PREFIX, tags=["Health"]) + + +@router.get( + "/health", + response_model=Dict[str, str], + responses={ + status.HTTP_200_OK: { + "content": { + "application/json": { + "example": { + "status": "healthy", + "version": "1.0.0", + "database": "healthy", + } + } + } + }, + status.HTTP_503_SERVICE_UNAVAILABLE: { + "content": { + "application/json": { + "example": { + "detail": { + "status": "healthy", + "version": "1.0.0", + "database": "unhealthy", + } + } + } + } + }, + }, +) +async def health() -> Dict: + "Confirm that the API is running and healthy." + database_health = get_db_health(CONFIG.api.sync_database_url) + response = { + "status": "healthy", + "version": str(fidesctl.__version__), + "database": database_health, + } + + for key in response: + if response[key] == "unhealthy": + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail=response + ) + + return response diff --git a/src/fidesapi/routes/util.py b/src/fidesapi/routes/util.py index 93056bb720..167b973ac6 100644 --- a/src/fidesapi/routes/util.py +++ b/src/fidesapi/routes/util.py @@ -2,8 +2,9 @@ from pathlib import Path from typing import Any, Callable -from fastapi import APIRouter, HTTPException, status +from fastapi import HTTPException, status +from fidesapi.utils.api_router import APIRouter from fidesctl.core.utils import API_PREFIX as _API_PREFIX API_PREFIX = _API_PREFIX diff --git a/src/fidesapi/routes/validate.py b/src/fidesapi/routes/validate.py index a9e1d665ef..6f2169388f 100644 --- a/src/fidesapi/routes/validate.py +++ b/src/fidesapi/routes/validate.py @@ -4,7 +4,7 @@ from enum import Enum from typing import Callable, Dict, Union -from fastapi import APIRouter, Response, status +from fastapi import Response, status from pydantic import BaseModel from fidesapi.routes.util import ( @@ -12,6 +12,7 @@ route_requires_aws_connector, route_requires_okta_connector, ) +from fidesapi.utils.api_router import APIRouter from fidesctl.connectors.models import ( AWSConfig, ConnectorAuthFailureException, diff --git a/src/fidesapi/routes/visualize.py b/src/fidesapi/routes/visualize.py index e17e67fc12..fa0b67f5fc 100644 --- a/src/fidesapi/routes/visualize.py +++ b/src/fidesapi/routes/visualize.py @@ -4,13 +4,14 @@ from enum import Enum from typing import Union -from fastapi import APIRouter, HTTPException +from fastapi import HTTPException from fastapi.responses import HTMLResponse from fideslang import model_map from fidesapi.routes.crud import list_resource from fidesapi.routes.util import API_PREFIX, get_resource_type from fidesapi.sql_models import sql_model_map +from fidesapi.utils.api_router import APIRouter from fidesctl.core import visualize # pylint: disable=redefined-outer-name,cell-var-from-loop diff --git a/src/fidesapi/utils/api_router.py b/src/fidesapi/utils/api_router.py new file mode 100644 index 0000000000..c4b23bbe40 --- /dev/null +++ b/src/fidesapi/utils/api_router.py @@ -0,0 +1,39 @@ +"""Copy pasta'd from +https://github.com/ethyca/fidesops/blob/a072f2a987e0f02e96c3b63a500b7bd6d9c4c8db/src/fidesops/util/api_router.py +""" + +from typing import Any, Callable + +from fastapi import APIRouter as FastAPIRouter +from fastapi.types import DecoratedCallable + + +class APIRouter(FastAPIRouter): + """ + Taken from: https://github.com/tiangolo/fastapi/issues/2060#issuecomment-834868906 + """ + + def api_route( # type: ignore + self, path: str, *, include_in_schema: bool = True, **kwargs: Any + ) -> Callable[[DecoratedCallable], DecoratedCallable]: + """ + Updated api_route function that automatically configures routes to have 2 versions. + One without and trailing slash and another with it. + """ + if path.endswith("/"): + path = path[:-1] + + add_path = super().api_route( + path, include_in_schema=include_in_schema, **kwargs + ) + + alternate_path = path + "/" + add_alternate_path = super().api_route( + alternate_path, include_in_schema=False, **kwargs + ) + + def decorator(func: DecoratedCallable) -> DecoratedCallable: # type: ignore + add_alternate_path(func) + return add_path(func) + + return decorator diff --git a/src/fidesapi/view.py b/src/fidesapi/view.py index e31248ce06..ce975c3ffb 100644 --- a/src/fidesapi/view.py +++ b/src/fidesapi/view.py @@ -1,11 +1,11 @@ """ Contains api endpoints for fides web pages """ -from fastapi import APIRouter from fastapi.responses import HTMLResponse from fidesapi.routes.crud import list_resource from fidesapi.sql_models import Evaluation +from fidesapi.utils.api_router import APIRouter router = APIRouter( tags=["View"], diff --git a/tests/core/test_api.py b/tests/core/test_api.py index 3f32b5d9cf..72ac265061 100644 --- a/tests/core/test_api.py +++ b/tests/core/test_api.py @@ -9,7 +9,7 @@ from pytest import MonkeyPatch from starlette.testclient import TestClient -from fidesapi import main +from fidesapi.routes import health from fidesctl.core import api as _api from fidesctl.core.config import FidesctlConfig from fidesctl.core.utils import API_PREFIX @@ -204,6 +204,17 @@ def test_api_ping( def mock_get_db_health(url: str) -> str: return database_health - monkeypatch.setattr(main, "get_db_health", mock_get_db_health) + monkeypatch.setattr(health, "get_db_health", mock_get_db_health) response = test_client.get(test_config.cli.server_url + API_PREFIX + "/health") assert response.status_code == expected_status_code + + +@pytest.mark.integration +@pytest.mark.parametrize("endpoint_name", ["organization", "health"]) +def test_trailing_slash(test_config: FidesctlConfig, endpoint_name: str) -> None: + """URLs both with and without a trailing slash should resolve and not 404""" + url = f"{test_config.cli.server_url}{API_PREFIX}/{endpoint_name}" + response = requests.get(url) + assert response.status_code == 200 + response = requests.get(f"{url}/") + assert response.status_code == 200 From 870501ee644f6de4a9d6b0c5c050b8355be8d120 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 12 Jul 2022 09:27:18 -0700 Subject: [PATCH 4/5] Fix SQLAlchemy M1 `greenlet` missing bug (#891) * Update requirements.txt * update the changelog --- CHANGELOG.md | 1 + requirements.txt | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c399251365..9f39a6bf1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ The types of changes are: * CustomSelect input tooltips appear next to selector instead of wrapping to a new row. * Datasets without the `third_country_transfer` will not cause the editing dataset form to not render. * Fixed a build issue causing an `unknown` version of `fidesctl` to be installed in published Docker images [#836](https://github.com/ethyca/fides/pull/836) +* Fixed an M1-related SQLAlchemy bug [#816](https://github.com/ethyca/fides/pull/891) * Endpoints now work with or without a trailing slash. [#886](https://github.com/ethyca/fides/pull/886) ## [1.7.0](https://github.com/ethyca/fides/compare/1.6.1...1.7.0) - 2022-06-23 diff --git a/requirements.txt b/requirements.txt index 20ebde74da..31c247d75c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,7 +17,7 @@ pydantic>=1.8.1,<2.0.0 # Required by fastapi PyJWT==2.4.0 pyyaml>=5,<6 requests>=2,<3 -sqlalchemy==1.4.36 +sqlalchemy[asyncio]==1.4.36 SQLAlchemy-Utils==0.38.2 toml>=0.10.1 uvicorn==0.17.6 From 7b6414dce530c1267ee9d2717384d5349c7968d8 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 12 Jul 2022 10:06:31 -0700 Subject: [PATCH 5/5] Check for uncommitted files before running `sync` (#869) * Update core.py * move gitpython from a dev requirement to a full requirement * re-add GitPython as a dev-requirement since it is required by nox for builds * remove accidental change * only throw an error for unstaged commits, not all uncommitted * add a more helpful error message * fix the dirty phrase to detect properly * inject the analytics_id env var into the container, don't write the analytics_id to the toml file if it is set as an env var * clarify naming for opt_out vars * add tests for the new git_is_dirty function * update the git_is_dirty to also catch new (untracked) files * update the changelog, fix the cli sync test * update sync docstring --- CHANGELOG.md | 2 ++ docker-compose.yml | 1 + requirements.txt | 1 + src/fidesctl/cli/commands/core.py | 10 +++++++++- src/fidesctl/cli/utils.py | 11 +++++++++-- src/fidesctl/core/utils.py | 15 +++++++++++++++ tests/cli/test_cli.py | 16 +++++++++++----- tests/core/test_utils.py | 23 +++++++++++++++++++++++ 8 files changed, 71 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f39a6bf1b..31ed9e17ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ The types of changes are: * System scanning results: AWS systems are stored and can be selected for review * Added Cypress for testing [713](https://github.com/ethyca/fides/pull/833) * CustomInput type "password" with show/hide icon. +* Sync CLI command now checks for untracked/unstaged files in the manifests dir [#869](https://github.com/ethyca/fides/pull/869) * Add Okta support to the `/generate` endpoint [#842](https://github.com/ethyca/fides/pull/842) * Add db support to `/generate` endpoint [849](https://github.com/ethyca/fides/pull/849) * Added OpenAPI TypeScript client generation for the UI app. See the [README](/clients/admin-ui/src/types/api/README.md) for more details. @@ -40,6 +41,7 @@ The types of changes are: * Initial configuration wizard UI view * Refactored step & form results management to use Redux Toolkit slice. * Remove the `obscure` requirement from the `generate` endpoint [#819](https://github.com/ethyca/fides/pull/819) +* Remove the `obscure` requirement from the `generate` endpoint [#819](https://github.com/ethyca/fides/pull/819) ### Docs diff --git a/docker-compose.yml b/docker-compose.yml index 3741e3f89d..dbb7a16173 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -29,6 +29,7 @@ services: FIDESCTL_TEST_MODE: "True" FIDESCTL__CLI__SERVER_HOST: "fidesctl" FIDESCTL__CLI__SERVER_PORT: "8080" + FIDESCTL__CLI__ANALYTICS_ID: ${FIDESCTL__CLI__ANALYTICS_ID} FIDESCTL__API__DATABASE_HOST: "fidesctl-db" fidesctl-ui: diff --git a/requirements.txt b/requirements.txt index 31c247d75c..b9eecc6cc5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,6 +8,7 @@ fastapi==0.77.1 fideslang==0.9.0 fideslib==2.2.2 fideslog==1.1.5 +GitPython==3.1 loguru>=0.5,<0.6 openpyxl==3.0.9 pandas==1.4 diff --git a/src/fidesctl/cli/commands/core.py b/src/fidesctl/cli/commands/core.py index 4911b05f0f..1ba68988dd 100644 --- a/src/fidesctl/cli/commands/core.py +++ b/src/fidesctl/cli/commands/core.py @@ -7,12 +7,13 @@ manifests_dir_argument, verbose_flag, ) -from fidesctl.cli.utils import pretty_echo, print_divider, with_analytics +from fidesctl.cli.utils import echo_red, pretty_echo, print_divider, with_analytics from fidesctl.core import apply as _apply from fidesctl.core import audit as _audit from fidesctl.core import evaluate as _evaluate from fidesctl.core import parse as _parse from fidesctl.core import sync as _sync +from fidesctl.core.utils import git_is_dirty @click.command() @@ -141,11 +142,18 @@ def parse(ctx: click.Context, manifests_dir: str, verbose: bool = False) -> None def sync(ctx: click.Context, manifests_dir: str) -> None: """ Update local resource files by their fides_key to match their server versions. + + Aborts the sync if there are unstaged or untracked files in the manifests dir. """ config = ctx.obj["CONFIG"] # Do this to validate the manifests since they won't get parsed during the sync process _parse.parse(manifests_dir) + if git_is_dirty(manifests_dir): + echo_red( + f"There are unstaged changes in your manifest directory: '{manifests_dir}' \nAborting sync!" + ) + raise SystemExit(1) _sync.sync( url=config.cli.server_url, manifests_dir=manifests_dir, diff --git a/src/fidesctl/cli/utils.py b/src/fidesctl/cli/utils.py index ca5332e0a5..bbd3bb6614 100644 --- a/src/fidesctl/cli/utils.py +++ b/src/fidesctl/cli/utils.py @@ -101,11 +101,18 @@ def check_and_update_analytics_config(ctx: click.Context, config_path: str) -> N user={"analytics_opt_out": ctx.obj["CONFIG"].user.analytics_opt_out} ) - if ctx.obj["CONFIG"].user.analytics_opt_out is False and get_config_from_file( + is_analytics_opt_out = ctx.obj["CONFIG"].user.analytics_opt_out + is_analytics_opt_out_config_empty = get_config_from_file( config_path, "cli", "analytics_id", - ) in ("", None): + ) in ("", None) + is_analytics_opt_out_env_var_set = getenv("FIDESCTL__CLI__ANALYTICS_ID") + if ( + not is_analytics_opt_out + and is_analytics_opt_out_config_empty + and not is_analytics_opt_out_env_var_set + ): config_updates.update(cli={"analytics_id": ctx.obj["CONFIG"].cli.analytics_id}) if len(config_updates) > 0: diff --git a/src/fidesctl/core/utils.py b/src/fidesctl/core/utils.py index b018b701b0..fced99c31d 100644 --- a/src/fidesctl/core/utils.py +++ b/src/fidesctl/core/utils.py @@ -12,6 +12,7 @@ import sqlalchemy from fideslang.models import DatasetField, FidesModel from fideslang.validation import FidesValidationError +from git.repo import Repo from sqlalchemy.engine import Engine logger = logging.getLogger("server_api") @@ -117,3 +118,17 @@ def sanitize_fides_key(proposed_fides_key: str) -> str: """ sanitized_fides_key = re.sub(r"[^a-zA-Z0-9_.-]", "_", proposed_fides_key) return sanitized_fides_key + + +def git_is_dirty(dir_to_check: str = ".") -> bool: + """ + Checks to see if the local repo has unstaged changes. + Can also specify a directory to check. + """ + repo = Repo() + git_session = repo.git() + + dirty_phrases = ["Changes not staged for commit:", "Untracked files:"] + git_status = git_session.status(dir_to_check).split("\n") + is_dirty = any(phrase in git_status for phrase in dirty_phrases) + return is_dirty diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 75ef7e8b00..312955e363 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -1,11 +1,11 @@ # pylint: disable=missing-docstring, redefined-outer-name import os from pathlib import PosixPath -from shutil import copytree from typing import Generator import pytest from click.testing import CliRunner +from git.repo import Repo from py._path.local import LocalPath from fidesctl.cli import cli @@ -102,13 +102,19 @@ def test_dry_diff_apply(test_config_path: str, test_cli_runner: CliRunner) -> No def test_sync( test_config_path: str, test_cli_runner: CliRunner, tmp_path: PosixPath ) -> None: - copytree("demo_resources", tmp_path, dirs_exist_ok=True) - result = test_cli_runner.invoke( - cli, ["-f", test_config_path, "sync", str(tmp_path)] - ) + """ + Due to the fact that this command checks the real git status, a pytest + tmp_dir can't be used. Consequently a real directory must be tested against + and then reset. + """ + test_dir = "demo_resources/" + result = test_cli_runner.invoke(cli, ["-f", test_config_path, "sync", test_dir]) print(result.output) assert result.exit_code == 0 + git_session = Repo().git() + git_session.checkout("HEAD", test_dir) + @pytest.mark.integration def test_audit(test_config_path: str, test_cli_runner: CliRunner) -> None: diff --git a/tests/core/test_utils.py b/tests/core/test_utils.py index 3934897519..877c7f4a48 100644 --- a/tests/core/test_utils.py +++ b/tests/core/test_utils.py @@ -1,4 +1,5 @@ # pylint: disable=missing-docstring, redefined-outer-name +import os from pathlib import PosixPath from typing import Generator @@ -90,3 +91,25 @@ def test_sanitize_fides_key(fides_key: str, sanitized_fides_key: str) -> None: ) def test_check_fides_key(fides_key: str, sanitized_fides_key: str) -> None: assert sanitized_fides_key == utils.check_fides_key(fides_key) + + +@pytest.mark.unit +class TestGitIsDirty: + """ + These tests can't use the standard pytest tmpdir + because the files need to be within the git repo + to be properly tested. + + They will therefore also break if the real dir + used for testing is deleted. + """ + + def test_not_dirty(self) -> None: + assert not utils.git_is_dirty("tests/data/example_sql/") + + def test_new_file_is_dirty(self) -> None: + test_file = "tests/data/example_sql/new_file.txt" + with open(test_file, "w") as file: + file.write("test file") + assert utils.git_is_dirty() + os.remove(test_file)