Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: parameter aggregation fix #1443

Merged
merged 15 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions tests/trestle/core/commands/author/profile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,7 @@ def test_profile_force_overwrite(tmp_trestle_dir: pathlib.Path, monkeypatch: Mon
header, tree = md_api.processor.process_markdown(md_path)

assert header
header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] = []
old_value = header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES]
header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] = 'New value'

Expand All @@ -948,6 +949,7 @@ def test_profile_force_overwrite(tmp_trestle_dir: pathlib.Path, monkeypatch: Mon
test_utils.execute_command_and_assert(prof_generate, 0, monkeypatch)

header, _ = md_api.processor.process_markdown(md_path)
header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] = []
assert header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] == old_value

# test that file is unchanged
Expand Down Expand Up @@ -1129,20 +1131,37 @@ def test_profile_generate_assemble_parameter_aggregation(
nist_cat, _ = ModelUtils.load_model_for_class(tmp_trestle_dir, 'nist_cat', cat.Catalog, FileContentType.JSON)

appended_prop = {'name': 'aggregates', 'value': 'at-02_odp.01'}
second_appended_prop = {'name': 'aggregates', 'value': 'at-02_odp.02'}
third_appended_prop = {'name': 'alt-identifier', 'value': 'this_is_an_identifier'}
ac_1 = nist_cat.groups[0].controls[0]
ac_1.params[2].props = []
ac_1.params[2].props.append(appended_prop)
ac_1.params[6].props = []
ac_1.params[6].props.append(appended_prop)
ac_1.params[6].props.append(second_appended_prop)
ac_1.params[6].props.append(third_appended_prop)
appended_extra_param = {
'id': 'at-02_odp.01',
'props': [{
'name': 'label', 'value': 'AT-02_ODP[01]', 'class': 'sp800-53a'
}],
'label': 'frequency',
'values': ['value-1', 'value-2'],
'guidelines': [{
'prose': 'blah'
}]
}
second_appended_extra_param = {
'id': 'at-02_odp.02',
'props': [{
'name': 'label', 'value': 'AT-02_ODP[02]', 'class': 'sp800-53a'
}],
'label': 'frequency',
'values': ['value-3', 'value-4'],
'guidelines': [{
'prose': 'blah'
}]
}
ac_1.params.append(appended_extra_param)
ac_1.params.append(second_appended_extra_param)

ModelUtils.save_top_level_model(nist_cat, tmp_trestle_dir, 'nist_cat', FileContentType.JSON)

Expand Down
1 change: 0 additions & 1 deletion tests/trestle/core/commands/author/ssp_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ def test_ssp_generate_header_edit(tmp_trestle_dir: pathlib.Path) -> None:
# confirm new items were added from yaml but not when the same key was alread present (values not updated)
header, tree = md_api.processor.process_markdown(ac_1)
assert 'control-origination' in header
assert header['x-trestle-set-params']['ac-1_prm_5']['values'] is None
assert header['x-trestle-set-params']['ac-1_prm_5']['label'] == 'meetings cancelled from cli yaml'
AleJo2995 marked this conversation as resolved.
Show resolved Hide resolved

# generate again with header and DO overwrite header values
Expand Down
4 changes: 4 additions & 0 deletions trestle/common/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,10 @@

CONTROL_IMPLEMENTATION = 'control-implementation'

AGGREGATES = 'aggregates'

ALT_IDENTIFIER = 'alt-identifier'

IMPLEMENTED_REQUIREMENT = 'implemented-requirement'
AleJo2995 marked this conversation as resolved.
Show resolved Hide resolved

# Following 5 are allowed control origination values for
Expand Down
6 changes: 6 additions & 0 deletions trestle/common/model_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,12 @@ def dict_to_parameter(param_dict: Dict[str, Any]) -> common.Parameter:
if const.DISPLAY_NAME in param_dict:
display_name = param_dict.pop(const.DISPLAY_NAME)
props.append(common.Property(name=const.DISPLAY_NAME, value=display_name, ns=const.TRESTLE_GENERIC_NS))
if const.AGGREGATES in param_dict:
# removing aggregates as this is prop just informative in markdown
param_dict.pop(const.AGGREGATES)
if const.ALT_IDENTIFIER in param_dict:
# removing alt-identifier as this is prop just informative in markdown
param_dict.pop(const.ALT_IDENTIFIER)

if 'ns' in param_dict:
param_dict.pop('ns')
Expand Down
9 changes: 8 additions & 1 deletion trestle/core/catalog/catalog_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,16 @@ def read_additional_content(
)
alters_map[sort_id] = control_alters
for param_id, param_dict in control_param_dict.items():
param_dict[const.VALUES] = param_dict[const.VALUES] if const.VALUES in param_dict else []
# if profile_values are present, overwrite values with them
if const.PROFILE_VALUES in param_dict:
param_dict[const.VALUES] = param_dict.pop(const.PROFILE_VALUES)
if param_dict[const.PROFILE_VALUES] != [] and param_dict[const.PROFILE_VALUES] is not None:
if not write_mode and '<REPLACE_ME>' in param_dict[const.PROFILE_VALUES]:
param_dict[const.PROFILE_VALUES].remove('<REPLACE_ME>')
if param_dict[const.PROFILE_VALUES] != [] and param_dict[const.PROFILE_VALUES] is not None:
param_dict[const.VALUES] = param_dict[const.PROFILE_VALUES]
if not write_mode:
param_dict.pop(const.PROFILE_VALUES)
final_param_dict[param_id] = param_dict
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the additional check for write mode? it was not there earlier.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is meant to not write replace me or pop profile values in read mode. Meaning that while writing into markdown replace me is not going to be removed or profile values are not as well. Logic before was implemented differently to pop the value out

param_sort_map[param_id] = sort_id
new_alters: List[prof.Alter] = []
Expand Down
48 changes: 32 additions & 16 deletions trestle/core/catalog/catalog_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import trestle.common.const as const
import trestle.oscal.catalog as cat
from trestle.common.list_utils import as_list, deep_get, delete_list_from_list, none_if_empty
from trestle.common.list_utils import as_list, deep_get, none_if_empty
from trestle.common.model_utils import ModelUtils
from trestle.core.catalog.catalog_interface import CatalogInterface
from trestle.core.catalog.catalog_merger import CatalogMerger
Expand Down Expand Up @@ -63,18 +63,6 @@ def write_catalog_as_profile_markdown(

# get all params and vals for this control from the resolved profile catalog with block adds in effect
control_param_dict = ControlInterface.get_control_param_dict(control, False)
to_delete = []
# removes aggregate parameters to be non-editable in markdowns
props_by_param_ids = {}
for param_id, values_dict in control_param_dict.items():
props_by_param_ids[param_id] = [
prop for prop in as_list(values_dict.props) if prop.name == 'aggregates'
]
to_delete = list({k: v for k, v in props_by_param_ids.items() if v != []}.keys())
unique_params_to_del = list(set(to_delete))
if unique_params_to_del:
delete_list_from_list(control_param_dict, unique_params_to_del)

set_param_dict = self._construct_set_parameters_dict(profile_set_param_dict, control_param_dict, context)
AleJo2995 marked this conversation as resolved.
Show resolved Hide resolved

if set_param_dict:
Expand Down Expand Up @@ -174,15 +162,43 @@ def _construct_set_parameters_dict(
# all the other elements are from the profile set_param
new_dict[const.VALUES] = orig_dict.get(const.VALUES, None)
new_dict[const.GUIDELINES] = orig_dict.get(const.GUIDELINES, None)
if new_dict[const.VALUES] is None:
new_dict.pop(const.VALUES)
if new_dict[const.GUIDELINES] is None:
new_dict.pop(const.GUIDELINES)
else:
# if the profile doesnt change this param at all, show it in the header with values
tmp_dict = ModelUtils.parameter_to_dict(param_dict, True)
values = tmp_dict.get('values', None)
new_dict = {'id': param_id, 'values': values}
# if values are None then don´t display them in the markdown
if values is not None:
new_dict = {'id': param_id, 'values': values, const.PROFILE_VALUES: ['<REPLACE_ME>']}
else:
new_dict = {'id': param_id, const.PROFILE_VALUES: ['<REPLACE_ME>']}
new_dict.pop('id', None)
AleJo2995 marked this conversation as resolved.
Show resolved Hide resolved
if display_name:
# validates if there are aggregated parameter values to the current parameter
aggregated_props = [prop for prop in as_list(param_dict.props) if prop.name == const.AGGREGATES]
if aggregated_props != []:
props_to_add = []
for prop in aggregated_props:
props_to_add.append(prop.value)
new_dict[const.AGGREGATES] = props_to_add
new_dict.pop(const.PROFILE_VALUES, None)
alt_identifier = [prop for prop in as_list(param_dict.props) if prop.name == const.ALT_IDENTIFIER]
if alt_identifier != []:
new_dict[const.ALT_IDENTIFIER] = alt_identifier[0].value
# adds display name, if no display name then do not add to dict
if display_name != '' and display_name is not None:
new_dict[const.DISPLAY_NAME] = display_name
key_order = (const.LABEL, const.GUIDELINES, const.PROFILE_VALUES, const.VALUES, const.DISPLAY_NAME)
key_order = (
const.LABEL,
const.GUIDELINES,
const.VALUES,
const.AGGREGATES,
const.ALT_IDENTIFIER,
const.DISPLAY_NAME,
const.PROFILE_VALUES
AleJo2995 marked this conversation as resolved.
Show resolved Hide resolved
)
ordered_dict = {k: new_dict[k] for k in key_order if k in new_dict.keys()}
set_param_dict[param_id] = ordered_dict

Expand Down
35 changes: 0 additions & 35 deletions trestle/core/commands/author/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,33 +285,6 @@ def _replace_modify_set_params(
profile.modify.set_parameters = none_if_empty(profile.modify.set_parameters)
return changed

@staticmethod
def _add_aggregated_parameter(
param: Any, param_dict: Dict[str, Any], control_id: str, controls: Any, param_map: Dict[str, str]
) -> None:
"""
Add aggregated parameter value to original parameter.

Notes:
None
"""
# verifies aggregated param is not on grabbed param dict
if param.id not in list(param_dict.keys()):
param.values = []
agg_props = [prop for prop in param.props if prop.name == 'aggregates']
for prop in as_list(agg_props):
if param_dict[prop.value].get('values'):
agg_param_values = param_dict[prop.value].get('values')
for value in as_list(agg_param_values):
param.values.append(value)
else:
agg_param_values = [p for p in controls[control_id] if p.id == prop.value][0]
param.values.append(agg_param_values.props[0].value)
dict_param = ModelUtils.parameter_to_dict(param, False)
param_dict[param.id] = dict_param
param_map[param.id] = control_id
return None

@staticmethod
def assemble_profile(
trestle_root: pathlib.Path,
Expand Down Expand Up @@ -381,14 +354,6 @@ def assemble_profile(
catalog_api = CatalogAPI(catalog=catalog, context=context)
found_alters, param_dict, param_map = catalog_api.read_additional_content_from_md(label_as_key=True)

controls = {}
for group in as_list(catalog.groups):
for control in as_list(group.controls):
controls[control.id] = control.params
for control_id, params in controls.items():
for param in as_list(params):
ProfileAssemble._add_aggregated_parameter(param, param_dict, control_id, controls, param_map)
# technically if allowed sections is [] it means no sections are allowed
if allowed_sections is not None:
for bad_part in [
part for alter in found_alters for add in as_list(alter.adds)
Expand Down
2 changes: 1 addition & 1 deletion trestle/core/remote/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ def _do_fetch(self) -> None:
auth = HTTPBasicAuth(self._username, self._password)

try:
response = requests.get(self._url, auth=auth, verify=verify, timeout=10)
response = requests.get(self._url, auth=auth, verify=verify, timeout=30)
except Exception as e:
raise TrestleError(f'Cache update failure to connect via HTTPS: {self._url} ({e})')

Expand Down
Loading