fix: Making description uneditable using config file (#497)

* Making description uneditable using config file

* Table Description Uneditable using regex

* Column Description uneditable

* fixed regex statement

* fixed lint error

* removed print statements

* Adding source to edit button

* Added edit description text

* Added list MatchRules object in configuration

* ignore linting for metadata_utils.py

* addint data type to match rules

* adding data type to match rules

* fixing mypy errors

* restructured logic in metadata_utils

* fixed mypy checks

* fix parse_editable_rule parameters

* Adding unit test cases

* Code cleanup and better comments

* Added documentation

* Modified unit test case

* fixed documentation

* documentation change

amundsen_application/api/utils/metadata_utils.py

@@ -6,13 +6,15 @@ from amundsen_common.models.dashboard import DashboardSummary, DashboardSummaryS
 from amundsen_common.models.popular_table import PopularTable, PopularTableSchema
 from amundsen_common.models.table import Table, TableSchema
 from amundsen_application.models.user import load_user, dump_user
+from amundsen_application.config import MatchRuleObject
 from flask import current_app as app
+import re
 
 
 def marshall_table_partial(table_dict: Dict) -> Dict:
     """
     Forms a short version of a table Dict, with selected fields and an added 'key'
-    :param table: Dict of partial table object
+    :param table_dict: Dict of partial table object
     :return: partial table Dict
 
     TODO - Unify data format returned by search and metadata.
@@ -23,17 +25,46 @@ def marshall_table_partial(table_dict: Dict) -> Dict:
     results = schema.dump(table).data
     # TODO: fix popular tables to provide these? remove if we're not using them?
     # TODO: Add the 'key' or 'id' to the base PopularTableSchema
-    results['key'] = f'{table.database}://{table.cluster}.{table.schema}/{ table.name}'
+    results['key'] = f'{table.database}://{table.cluster}.{table.schema}/{table.name}'
     results['last_updated_timestamp'] = None
     results['type'] = 'table'
 
     return results
 
 
+def _parse_editable_rule(rule: MatchRuleObject,
+                         schema: str,
+                         table: str) -> bool:
+    """
+    Matches table name and schema with corresponding regex in matching rule
+    :parm rule: MatchRuleObject defined in list UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES in config file
+    :parm schema: schema name from Table Dict received from metadata service
+    :parm table: table name from Table Dict received from metadata service
+    :return: boolean which determines if table desc is editable or not for given table as per input matching rule
+    """
+    if rule.schema_regex and rule.table_name_regex:
+        match_schema = re.match(rule.schema_regex, schema)
+        match_table = re.match(rule.table_name_regex, table)
+        if match_schema and match_table:
+            return False
+        return True
+    if rule.schema_regex:
+        match_schema = re.match(rule.schema_regex, schema)
+        if match_schema:
+            return False
+        return True
+    if rule.table_name_regex:
+        match_table = re.match(rule.table_name_regex, table)
+        if match_table:
+            return False
+        return True
+    return True
+
+
 def marshall_table_full(table_dict: Dict) -> Dict:
     """
     Forms the full version of a table Dict, with additional and sanitized fields
-    :param table: Table Dict from metadata service
+    :param table_dict: Table Dict from metadata service
     :return: Table Dict with sanitized fields
     """
 
@@ -42,7 +73,16 @@ def marshall_table_full(table_dict: Dict) -> Dict:
     table: Table = schema.load(table_dict).data
     results: Dict[str, Any] = schema.dump(table).data
 
-    is_editable = results['schema'] not in app.config['UNEDITABLE_SCHEMAS']
+    # Check if schema is uneditable
+    is_editable_schema = results['schema'] not in app.config['UNEDITABLE_SCHEMAS']
+
+    # Check if Table Description is uneditable
+    is_editable_table = True
+    uneditable_table_desc_match_rules = app.config['UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES']
+    for rule in uneditable_table_desc_match_rules:
+        is_editable_table = is_editable_table and _parse_editable_rule(rule, results['schema'], results['name'])
+
+    is_editable = is_editable_schema and is_editable_table
     results['is_editable'] = is_editable
 
     # TODO - Cleanup https://github.com/lyft/amundsen/issues/296
@@ -63,7 +103,7 @@ def marshall_table_full(table_dict: Dict) -> Dict:
             col['is_editable'] = is_editable
 
     # TODO: Add the 'key' or 'id' to the base TableSchema
-    results['key'] = f'{table.database}://{table.cluster}.{table.schema}/{ table.name}'
+    results['key'] = f'{table.database}://{table.cluster}.{table.schema}/{table.name}'
     # Temp code to make 'partition_key' and 'partition_value' part of the table
     results['partition'] = _get_partition_data(results['watermarks'])
 

amundsen_application/config.py

@@ -7,6 +7,15 @@ from flask import Flask  # noqa: F401
 from amundsen_application.tests.test_utils import get_test_user
 
 
+class MatchRuleObject:
+    def __init__(self,
+                 schema_regex=None,  # type: str
+                 table_name_regex=None,   # type: str
+                 ) -> None:
+        self.schema_regex = schema_regex
+        self.table_name_regex = table_name_regex
+
+
 class Config:
     LOG_FORMAT = '%(asctime)s.%(msecs)03d [%(levelname)s] %(module)s.%(funcName)s:%(lineno)d (%(process)d:' \
                  + '%(threadName)s) - %(message)s'
@@ -22,6 +31,8 @@ class Config:
 
     UNEDITABLE_SCHEMAS = set()  # type: Set[str]
 
+    UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES = []  # type: List[MatchRuleObject]
+
     # Number of popular tables to be displayed on the index/search page
     POPULAR_TABLE_COUNT = 4  # type: int
 

amundsen_application/static/js/components/TableDetail/ColumnListItem/index.tsx

@@ -167,7 +167,10 @@ export class ColumnListItem extends React.Component<
           {this.state.isExpanded && (
             <section className="expanded-content">
               <div className="stop-propagation" onClick={this.stopPropagation}>
-                <EditableSection title="Description">
+                <EditableSection
+                  title="Description"
+                  readOnly={!metadata.is_editable}
+                >
                   <ColumnDescEditableText
                     columnIndex={this.props.index}
                     editable={metadata.is_editable}

amundsen_application/static/js/components/TableDetail/constants.ts

 export const PROGRMMATIC_DESC_HEADER = 'Read-only information, auto-generated';
 export const ERROR_MESSAGE = 'Something went wrong...';
+export const EDIT_DESC_TEXT = 'Click to edit description in';

amundsen_application/static/js/components/TableDetail/index.tsx

@@ -48,7 +48,11 @@ import { getLoggingParams } from 'utils/logUtils';
 import RequestDescriptionText from './RequestDescriptionText';
 import RequestMetadataForm from './RequestMetadataForm';
 
-import { PROGRMMATIC_DESC_HEADER, ERROR_MESSAGE } from './constants';
+import {
+  PROGRMMATIC_DESC_HEADER,
+  ERROR_MESSAGE,
+  EDIT_DESC_TEXT,
+} from './constants';
 
 import './styles.scss';
 
@@ -175,6 +179,10 @@ export class TableDetail extends React.Component<
       innerContent = <ErrorMessage />;
     } else {
       const data = tableData;
+      const editText = data.source
+        ? `${EDIT_DESC_TEXT} ${data.source.source_type}`
+        : '';
+      const editUrl = data.source ? data.source.source : '';
 
       innerContent = (
         <div className="resource-detail-layout table-detail">
@@ -220,7 +228,12 @@ export class TableDetail extends React.Component<
           </header>
           <div className="column-layout-1">
             <aside className="left-panel">
-              <EditableSection title="Description">
+              <EditableSection
+                title="Description"
+                readOnly={!data.is_editable}
+                editText={editText}
+                editUrl={editUrl}
+              >
                 <TableDescEditableText
                   maxLength={AppConfig.editableText.tableDescLength}
                   value={data.description}

docs/flask_config.md

@@ -100,3 +100,50 @@ description sources not mentioned in the configuration will be alphabetically pl
 
 Here is a screenshot of what it would look like in the bottom left here:
 ![programmatic_description](img/programmatic_descriptions.png)
+
+## Uneditable Table Descriptions
+Amundsen supports configuring table and column description to be non-editable for selective tables. You may want to make table 
+descriptions non-editable due to various reasons such as table already has table description from source of truth.
+You can define matching rules in  [config.py](https://github.com/lyft/amundsenfrontendlibrary/blob/master/amundsen_application/config.py) for selecting tables. This configuration is useful as table selection criteria can 
+be company specific which will not directly integrated with Amundsen. 
+You can use different combinations of schema and table name for selecting tables.
+
+Here are some examples when this feature can be used: 
+1. You want to set all tables with a given schema or schema pattern as un-editable.
+2. You want to set all tables with a specific table name pattern in a given schema pattern as un-editable.
+3. You want to set all tables with a given table name pattern as un-editable.
+
+Amundsen has two variables in `config.py` file which can be used to define match rules:
+1. `UNEDITABLE_SCHEMAS` : Set of schemas where all tables should be un-editable. It takes exact schema name.
+2. `UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES` : List of MatchRuleObject, where each MatchRuleObject consists of regex for 
+schema name or regex for table name or both.
+
+Purpose of `UNEDITABLE_SCHEMAS` can be fulfilled by `UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES` but we are keeping both 
+variables for backward compatibility.
+If you want to restrict tables from a given schemas then you can use `UNEDITABLE_SCHEMAS` as follows:
+```python
+UNEDITABLE_SCHEMAS = set(['schema1', 'schema2'])
+```
+After above configuration, all tables in 'schema1' and 'schema2' will have non-editable table and column descriptions.
+
+If you have more complex matching rules you can use `UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES`. It provides you more flexibility 
+and control as you can create multiple match rules and use regex for matching schema nad table names.
+
+You can configure your match rules in `config.py` as follow:
+```python
+UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES = [
+        # match rule for all table in schema1
+        MatchRuleObject(schema_regex=r"^(schema1)"),
+        # macth rule for all tables in schema2 and schema3
+        MatchRuleObject(schema_regex=r"^(schema2|schema3)"),
+        # match rule for tables in schema4 with table name pattern 'noedit_*'
+        MatchRuleObject(schema_regex=r"^(schema4)", table_name_regex=r"^noedit_([a-zA-Z_0-9]+)"),
+        # match rule for tables in schema5, schema6 and schema7 with table name pattern 'noedit_*'
+        MatchRuleObject(schema_regex=r"^(schema5|schema6|schema7)", table_name_regex=r"^noedit_([a-zA-Z_0-9]+)"),
+        # match rule for all tables with table name pattern 'others_*'
+        MatchRuleObject(table_name_regex=r"^others_([a-zA-Z_0-9]+)")
+    ]
+```
+
+After configuring this, users will not be able to edit table and column descriptions of any table matching above match rules
+from UI.

tests/unit/utils/test_metadata_utils.py

@@ -2,7 +2,9 @@ import unittest
 
 from unittest.mock import patch, Mock
 
-from amundsen_application.api.utils.metadata_utils import _update_prog_descriptions, _sort_prog_descriptions
+from amundsen_application.api.utils.metadata_utils import _update_prog_descriptions, _sort_prog_descriptions, \
+    _parse_editable_rule
+from amundsen_application.config import MatchRuleObject
 from amundsen_application import create_app
 
 local_app = create_app('amundsen_application.config.TestConfig', 'tests/templates')
@@ -63,3 +65,34 @@ class ProgrammaticDescriptionsTest(unittest.TestCase):
             }
             not_in_config_value = {'source': 'test', 'text': 'I am a test'}
             self.assertEqual(_sort_prog_descriptions(mock_config, not_in_config_value), len(mock_config))
+
+
+class UneditableTableDescriptionTest(unittest.TestCase):
+    def setUp(self) -> None:
+        pass
+
+    def test_table_desc_match_rule_schema_only(self) -> None:
+        # Mock match rule, table name and schema
+        test_match_rule = MatchRuleObject(schema_regex=r"^(schema1)")
+
+        # assert result for given schema and match rule
+        self.assertEqual(_parse_editable_rule(test_match_rule, 'schema1', 'test_table'), False)
+        self.assertEqual(_parse_editable_rule(test_match_rule, 'schema2', 'test_table'), True)
+
+    def test_table_desc_match_rule_table_only(self) -> None:
+        # Mock match rule, table name and schema
+        test_match_rule = MatchRuleObject(table_name_regex=r"^noedit_([a-zA-Z_0-9]+)")
+
+        # assert result for given table name and match rule
+        self.assertEqual(_parse_editable_rule(test_match_rule, 'schema', 'noedit_test_table'), False)
+        self.assertEqual(_parse_editable_rule(test_match_rule, 'schema', 'editable_test_table'), True)
+
+    def test_table_desc_match_rule_schema_and_table(self) -> None:
+        # Mock match rule, table name and schema
+        test_match_rule = MatchRuleObject(schema_regex=r"^(schema1|schema2)",
+                                          table_name_regex=r"^other_([a-zA-Z_0-9]+)")
+        # assert result for given schema, table name and match rule
+        self.assertEqual(_parse_editable_rule(test_match_rule, 'schema1', 'other_test_table'), False)
+        self.assertEqual(_parse_editable_rule(test_match_rule, 'schema1', 'test_table'), True)
+        self.assertEqual(_parse_editable_rule(test_match_rule, 'schema3', 'other_test_table'), True)
+        self.assertEqual(_parse_editable_rule(test_match_rule, 'schema3', 'test_table'), True)