Skip to content

Commit

Permalink
Add full configuration validation. (sonic-net#3316)
Browse files Browse the repository at this point in the history
#### What I did

Before apply the json patch, we will precheck and simulate-patch the payload in entire box level.

#### How I did it

1. Add Duplication check
2. JSON patch structure validating
3. Simulating patch to full configuration
4. Verifying simulating result match YANG validation.

#### How to verify it

1. Single ASIC

```
admin@str2-msn2700-spy-2:~/gcu$ cat empty.json 
[]
admin@str2-msn2700-spy-2:~/gcu$ sudo config apply-patch empty.json 
Patch Applier: localhost: Patch application starting.
Patch Applier: localhost: Patch: []
Patch Applier: localhost getting current config db.
Patch Applier: localhost: simulating the target full config after applying the patch.
Patch Applier: localhost: validating all JsonPatch operations are permitted on the specified fields
Patch Applier: localhost: validating target config does not have empty tables,
                               since they do not show up in ConfigDb.
Patch Applier: localhost: sorting patch updates.
Patch Applier: The localhost patch was converted into 0 changes.
Patch Applier: localhost: applying 0 changes in order.
Patch Applier: localhost: verifying patch updates are reflected on ConfigDB.
Patch Applier: localhost patch application completed.
Patch applied successfully.
```

2. Multi ASIC

```
stli@str2-7250-2-lc01:~/gcu$ cat empty.json 
[]
stli@str2-7250-2-lc01:~/gcu$ sudo config apply-patch empty.json 
sonic_yang(6):Note: Below table(s) have no YANG models: DHCP_SERVER, KUBERNETES_MASTER
sonic_yang(6):Note: Below table(s) have no YANG models: KUBERNETES_MASTER
sonic_yang(6):Note: Below table(s) have no YANG models: KUBERNETES_MASTER
Patch Applier: localhost: Patch application starting.
Patch Applier: localhost: Patch: []
Patch Applier: localhost getting current config db.
Patch Applier: localhost: simulating the target full config after applying the patch.
Patch Applier: localhost: validating all JsonPatch operations are permitted on the specified fields
Patch Applier: localhost: validating target config does not have empty tables,
                               since they do not show up in ConfigDb.
Patch Applier: localhost: sorting patch updates.
Patch Applier: The localhost patch was converted into 0 changes.
Patch Applier: localhost: applying 0 changes in order.
Patch Applier: localhost: verifying patch updates are reflected on ConfigDB.
Patch Applier: localhost patch application completed.
Patch Applier: asic0: Patch application starting.
Patch Applier: asic0: Patch: []
Patch Applier: asic0 getting current config db.
Patch Applier: asic0: simulating the target full config after applying the patch.
Patch Applier: asic0: validating all JsonPatch operations are permitted on the specified fields
Patch Applier: asic0: validating target config does not have empty tables,
                               since they do not show up in ConfigDb.
Patch Applier: asic0: sorting patch updates.
Patch Applier: The asic0 patch was converted into 0 changes.
Patch Applier: asic0: applying 0 changes in order.
Patch Applier: asic0: verifying patch updates are reflected on ConfigDB.
Patch Applier: asic0 patch application completed.
Patch Applier: asic1: Patch application starting.
Patch Applier: asic1: Patch: []
Patch Applier: asic1 getting current config db.
Patch Applier: asic1: simulating the target full config after applying the patch.
Patch Applier: asic1: validating all JsonPatch operations are permitted on the specified fields
Patch Applier: asic1: validating target config does not have empty tables,
                               since they do not show up in ConfigDb.
Patch Applier: asic1: sorting patch updates.
Patch Applier: The asic1 patch was converted into 0 changes.
Patch Applier: asic1: applying 0 changes in order.
Patch Applier: asic1: verifying patch updates are reflected on ConfigDB.
Patch Applier: asic1 patch application completed.
Patch applied successfully.
```
  • Loading branch information
xincunli-sonic authored May 17, 2024
1 parent 7d027bb commit 7b97ac5
Show file tree
Hide file tree
Showing 4 changed files with 206 additions and 28 deletions.
62 changes: 51 additions & 11 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@
from jsonpointer import JsonPointerException
from collections import OrderedDict
from generic_config_updater.generic_updater import GenericUpdater, ConfigFormat, extract_scope
from generic_config_updater.gu_common import HOST_NAMESPACE, GenericConfigUpdaterError
from minigraph import parse_device_desc_xml, minigraph_encoder
from natsort import natsorted
from portconfig import get_child_ports
from socket import AF_INET, AF_INET6
from sonic_py_common import device_info, multi_asic
from sonic_py_common.general import getstatusoutput_noshell
from sonic_py_common.interface import get_interface_table_name, get_port_table_name, get_intf_longname
from sonic_yang_cfg_generator import SonicYangCfgDbGenerator
from utilities_common import util_base
from swsscommon import swsscommon
from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector
Expand Down Expand Up @@ -1155,25 +1157,59 @@ def validate_gre_type(ctx, _, value):
return gre_type_value
except ValueError:
raise click.UsageError("{} is not a valid GRE type".format(value))

# Function to apply patch for a single ASIC.
def apply_patch_for_scope(scope_changes, results, config_format, verbose, dry_run, ignore_non_yang_tables, ignore_path):
scope, changes = scope_changes
# Replace localhost to DEFAULT_NAMESPACE which is db definition of Host
if scope.lower() == "localhost" or scope == "":
if scope.lower() == HOST_NAMESPACE or scope == "":
scope = multi_asic.DEFAULT_NAMESPACE
scope_for_log = scope if scope else "localhost"

scope_for_log = scope if scope else HOST_NAMESPACE
try:
# Call apply_patch with the ASIC-specific changes and predefined parameters
GenericUpdater(namespace=scope).apply_patch(jsonpatch.JsonPatch(changes), config_format, verbose, dry_run, ignore_non_yang_tables, ignore_path)
GenericUpdater(namespace=scope).apply_patch(jsonpatch.JsonPatch(changes),
config_format,
verbose,
dry_run,
ignore_non_yang_tables,
ignore_path)
results[scope_for_log] = {"success": True, "message": "Success"}
log.log_notice(f"'apply-patch' executed successfully for {scope_for_log} by {changes}")
except Exception as e:
results[scope_for_log] = {"success": False, "message": str(e)}
log.log_error(f"'apply-patch' executed failed for {scope_for_log} by {changes} due to {str(e)}")


def validate_patch(patch):
try:
command = ["show", "runningconfiguration", "all"]
proc = subprocess.Popen(command, text=True, stdout=subprocess.PIPE)
all_running_config, returncode = proc.communicate()
if returncode:
log.log_notice(f"Fetch all runningconfiguration failed as output:{all_running_config}")
return False

# Structure validation and simulate apply patch.
all_target_config = patch.apply(json.loads(all_running_config))

# Verify target config by YANG models
target_config = all_target_config.pop(HOST_NAMESPACE) if multi_asic.is_multi_asic() else all_target_config
target_config.pop("bgpraw", None)
if not SonicYangCfgDbGenerator().validate_config_db_json(target_config):
return False

if multi_asic.is_multi_asic():
for asic in multi_asic.get_namespace_list():
target_config = all_target_config.pop(asic)
target_config.pop("bgpraw", None)
if not SonicYangCfgDbGenerator().validate_config_db_json(target_config):
return False

return True
except Exception as e:
raise GenericConfigUpdaterError(f"Validate json patch: {patch} failed due to:{e}")

# This is our main entrypoint - the main 'config' command
@click.group(cls=clicommon.AbbreviationGroup, context_settings=CONTEXT_SETTINGS)
@click.pass_context
Expand Down Expand Up @@ -1381,6 +1417,9 @@ def apply_patch(ctx, patch_file_path, format, dry_run, ignore_non_yang_tables, i
patch_as_json = json.loads(text)
patch = jsonpatch.JsonPatch(patch_as_json)

if not validate_patch(patch):
raise GenericConfigUpdaterError(f"Failed validating patch:{patch}")

results = {}
config_format = ConfigFormat[format.upper()]
# Initialize a dictionary to hold changes categorized by scope
Expand All @@ -1403,7 +1442,8 @@ def apply_patch(ctx, patch_file_path, format, dry_run, ignore_non_yang_tables, i
# Empty case to force validate YANG model.
if not changes_by_scope:
asic_list = [multi_asic.DEFAULT_NAMESPACE]
asic_list.extend(multi_asic.get_namespace_list())
if multi_asic.is_multi_asic():
asic_list.extend(multi_asic.get_namespace_list())
for asic in asic_list:
changes_by_scope[asic] = []

Expand All @@ -1416,7 +1456,7 @@ def apply_patch(ctx, patch_file_path, format, dry_run, ignore_non_yang_tables, i

if failures:
failure_messages = '\n'.join([f"- {failed_scope}: {results[failed_scope]['message']}" for failed_scope in failures])
raise Exception(f"Failed to apply patch on the following scopes:\n{failure_messages}")
raise GenericConfigUpdaterError(f"Failed to apply patch on the following scopes:\n{failure_messages}")

log.log_notice(f"Patch applied successfully for {patch}.")
click.secho("Patch applied successfully.", fg="cyan", underline=True)
Expand Down Expand Up @@ -1620,9 +1660,9 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form
file_input = read_json_file(file)

platform = file_input.get("DEVICE_METADATA", {}).\
get("localhost", {}).get("platform")
get(HOST_NAMESPACE, {}).get("platform")
mac = file_input.get("DEVICE_METADATA", {}).\
get("localhost", {}).get("mac")
get(HOST_NAMESPACE, {}).get("mac")

if not platform or not mac:
log.log_warning("Input file does't have platform or mac. platform: {}, mac: {}"
Expand Down Expand Up @@ -1995,8 +2035,8 @@ def override_config_table(db, input_config_db, dry_run):
if multi_asic.is_multi_asic() and len(config_input):
# Golden Config will use "localhost" to represent host name
if ns == DEFAULT_NAMESPACE:
if "localhost" in config_input.keys():
ns_config_input = config_input["localhost"]
if HOST_NAMESPACE in config_input.keys():
ns_config_input = config_input[HOST_NAMESPACE]
else:
click.secho("Wrong config format! 'localhost' not found in host config! cannot override.. abort")
sys.exit(1)
Expand Down
29 changes: 12 additions & 17 deletions generic_config_updater/generic_updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import jsonpointer
import os
from enum import Enum
from .gu_common import GenericConfigUpdaterError, EmptyTableError, ConfigWrapper, \
from .gu_common import HOST_NAMESPACE, GenericConfigUpdaterError, EmptyTableError, ConfigWrapper, \
DryRunConfigWrapper, PatchWrapper, genericUpdaterLogging
from .patch_sorter import StrictPatchSorter, NonStrictPatchSorter, ConfigSplitter, \
TablesWithoutYangConfigSplitter, IgnorePathsFromYangConfigSplitter
Expand All @@ -16,28 +16,26 @@ def extract_scope(path):
if not path:
raise Exception("Wrong patch with empty path.")

try:
pointer = jsonpointer.JsonPointer(path)
parts = pointer.parts
except Exception as e:
raise Exception(f"Error resolving path: '{path}' due to {e}")
pointer = jsonpointer.JsonPointer(path)
parts = pointer.parts

if not parts:
raise Exception("Wrong patch with empty path.")
raise GenericConfigUpdaterError("Wrong patch with empty path.")
if parts[0].startswith("asic"):
if not parts[0][len("asic"):].isnumeric():
raise Exception(f"Error resolving path: '{path}' due to incorrect ASIC number.")
raise GenericConfigUpdaterError(f"Error resolving path: '{path}' due to incorrect ASIC number.")
scope = parts[0]
remainder = "/" + "/".join(parts[1:])
elif parts[0] == "localhost":
scope = "localhost"
elif parts[0] == HOST_NAMESPACE:
scope = HOST_NAMESPACE
remainder = "/" + "/".join(parts[1:])
else:
scope = ""
remainder = path

return scope, remainder


class ConfigLock:
def acquire_lock(self):
# TODO: Implement ConfigLock
Expand Down Expand Up @@ -67,7 +65,7 @@ def __init__(self,
self.changeapplier = changeapplier if changeapplier is not None else ChangeApplier(namespace=self.namespace)

def apply(self, patch, sort=True):
scope = self.namespace if self.namespace else 'localhost'
scope = self.namespace if self.namespace else HOST_NAMESPACE
self.logger.log_notice(f"{scope}: Patch application starting.")
self.logger.log_notice(f"{scope}: Patch: {patch}")

Expand All @@ -84,10 +82,10 @@ def apply(self, patch, sort=True):
self.config_wrapper.validate_field_operation(old_config, target_config)

# Validate target config does not have empty tables since they do not show up in ConfigDb
self.logger.log_notice(f"{scope}: alidating target config does not have empty tables, " \
"since they do not show up in ConfigDb.")
self.logger.log_notice(f"""{scope}: validating target config does not have empty tables,
since they do not show up in ConfigDb.""")
empty_tables = self.config_wrapper.get_empty_tables(target_config)
if empty_tables: # if there are empty tables
if empty_tables: # if there are empty tables
empty_tables_txt = ", ".join(empty_tables)
raise EmptyTableError(f"{scope}: given patch is not valid because it will result in empty tables " \
"which is not allowed in ConfigDb. " \
Expand All @@ -105,9 +103,6 @@ def apply(self, patch, sort=True):
self.logger.log_notice(f"The {scope} patch was converted into {changes_len} " \
f"change{'s' if changes_len != 1 else ''}{':' if changes_len > 0 else '.'}")

for change in changes:
self.logger.log_notice(f" * {change}")

# Apply changes in order
self.logger.log_notice(f"{scope}: applying {changes_len} change{'s' if changes_len != 1 else ''} " \
f"in order{':' if changes_len > 0 else '.'}")
Expand Down
1 change: 1 addition & 0 deletions generic_config_updater/gu_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
SYSLOG_IDENTIFIER = "GenericConfigUpdater"
SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__))
GCU_FIELD_OP_CONF_FILE = f"{SCRIPT_DIR}/gcu_field_operation_validators.conf.json"
HOST_NAMESPACE = "localhost"

class GenericConfigUpdaterError(Exception):
pass
Expand Down
Loading

0 comments on commit 7b97ac5

Please sign in to comment.