Make configurationparser use ready #386

Miauwkeru · 2023-09-05T12:36:10Z

(DIS-2156)

codecov · 2023-09-05T12:39:36Z

Codecov Report

Merging #386 (f09f244) into main (d3c195e) will increase coverage by 0.15%.
The diff coverage is 88.25%.

@@            Coverage Diff             @@
##             main     #386      +/-   ##
==========================================
+ Coverage   73.79%   73.95%   +0.15%     
==========================================
  Files         252      255       +3     
  Lines       20189    20289     +100     
==========================================
+ Hits        14899    15005     +106     
+ Misses       5290     5284       -6

Flag	Coverage Δ
unittests	`73.95% <88.25%> (+0.15%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
dissect/target/filesystem.py	`83.71% <100.00%> (+0.15%)`	⬆️
dissect/target/plugins/os/unix/etc.py	`83.33% <83.33%> (ø)`
dissect/target/plugins/general/config.py	`94.28% <94.28%> (ø)`
dissect/target/tools/shell.py	`48.06% <25.00%> (-0.07%)`	⬇️
dissect/target/filesystems/config.py	`87.62% <87.62%> (ø)`
dissect/target/helpers/configparser.py	`89.13% <89.13%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

dissect/target/plugins/os/unix/config.py

Schamper · 2023-09-13T07:51:40Z

dissect/target/plugins/os/unix/config.py

+
+
+class ConfigurationTreePlugin(Plugin):
+    __namespace__ = "config_tree"


Might be nice UX to call this etc? So calling would be t.etc("/blah").

Then I'd assume you would only be able to use it with the /etc directory. Now it can be used within any directory. You could theoretically even use it on windows machines (for their ini files and stuff)

Fair enough, though you could still make an internal shortcut available for etc right? And use this name if you want to parse other paths.

Schamper · 2023-09-13T09:43:10Z

dissect/target/plugins/os/unix/config.py

+class ConfigurationTreePlugin(Plugin):
+    __namespace__ = "config_tree"
+
+    def __call__(


Can we LRU cache this?

I'll look into it

Schamper · 2023-09-13T09:48:40Z

tests/test_plugins_unix_config_filesystem.py

@@ -65,7 +70,7 @@ def __exit__(self, _, __, ___):


 def test_parse_functions(target_unix: Target, etc_directory: VirtualFilesystem):
-    config_fs = ConfigurationFs(target_unix)
+    config_fs = ConfigurationFilesystem(target_unix)
    entry: ConfigurationEntry = config_fs.get("/new/path/config", collapse=True)

    assert entry.parser_items["help"] == "you"


Could we make a nicer way to get configuration values? Perhaps just make the entry itself behave like a dictionary with __item__ and .get()?

The get would be hard to change, as it follows the api for FilesystemEntries. I can however use the __getitem__ to access the dictionary directly

You could, but instead of raising a NotADirectoryError you'd just be returning the actual configuration value.

The NotADirectoryError is there when there is no value though...?

Sorry, I probably meant that instead of letting end values be entries as well, that those just return the actual values, instead of having to use .open(). Or instead of the if not path: return self, return the value. I'm not sure what my train of thought was anymore.

Maybe I'm still just unsure if the filesystem approach is a good UX/API experience. I feel like a "magic dictionary" filled with configuration options is a much nicer programming experience.

tests/test_plugins_unix_config_default_parser.py

tests/test_plugins_unix_config_file_parser.py

tests/test_plugins_unix_config_filesystem.py

dissect/target/plugins/os/unix/config.py

dissect/target/tools/shell.py

dissect/target/plugins/os/unix/config.py

Schamper · 2023-09-19T20:50:37Z

tests/test_plugins_unix_config_filesystem.py

@@ -65,7 +70,7 @@ def __exit__(self, _, __, ___):


 def test_parse_functions(target_unix: Target, etc_directory: VirtualFilesystem):
-    config_fs = ConfigurationFs(target_unix)
+    config_fs = ConfigurationFilesystem(target_unix)
    entry: ConfigurationEntry = config_fs.get("/new/path/config", collapse=True)

    assert entry.parser_items["help"] == "you"


You could, but instead of raising a NotADirectoryError you'd just be returning the actual configuration value.

dissect/target/plugins/os/unix/config.py

dissect/target/plugins/os/unix/etc.py

tests/test_plugins_os_unix_config_default_parser.py

dissect/target/helpers/configparser.py

dissect/target/filesystems/config.py

dissect/target/helpers/configparser.py

dissect/target/filesystems/config.py

tests/test_plugins_generic_config.py

dissect/target/helpers/configparser.py

dissect/target/plugins/general/config.py

dissect/target/filesystems/config.py

dissect/target/helpers/configparser.py

dissect/target/filesystems/config.py

dissect/target/plugins/general/config.py

tests/data/config_tree/hosts

dissect/target/filesystems/config.py

Also converts it to bytesio at the `open` of the filesystem entry

This allows an analyst to make it into a dict on command. otherwise, just ask `as_dict()` on a ConfigurationEntry

Has been implemented and discussed

(DIS-2156) Co-authored-by: pyrco <[email protected]> Co-authored-by: cecinestpasunepipe <[email protected]> Co-authored-by: Erik Schamper <[email protected]>

Miauwkeru marked this pull request as ready for review September 5, 2023 12:47

Schamper requested changes Sep 5, 2023

View reviewed changes

dissect/target/plugins/os/unix/config.py Outdated Show resolved Hide resolved

dissect/target/plugins/os/unix/config.py Outdated Show resolved Hide resolved

dissect/target/plugins/os/unix/config.py Outdated Show resolved Hide resolved

Miauwkeru requested review from Schamper, pyrco and cecinestpasunepipe September 12, 2023 16:11

Schamper requested changes Sep 13, 2023

View reviewed changes

tests/test_plugins_unix_config_default_parser.py Outdated Show resolved Hide resolved

tests/test_plugins_unix_config_file_parser.py Outdated Show resolved Hide resolved

tests/test_plugins_unix_config_filesystem.py Outdated Show resolved Hide resolved

cecinestpasunepipe suggested changes Sep 13, 2023

View reviewed changes

dissect/target/plugins/os/unix/config.py Outdated Show resolved Hide resolved

dissect/target/plugins/os/unix/config.py Outdated Show resolved Hide resolved

dissect/target/tools/shell.py Outdated Show resolved Hide resolved

pyrco requested changes Sep 18, 2023

View reviewed changes

Miauwkeru requested review from Schamper, cecinestpasunepipe and pyrco September 19, 2023 14:19

Schamper reviewed Sep 19, 2023

View reviewed changes

pyrco requested changes Sep 21, 2023

View reviewed changes

cecinestpasunepipe suggested changes Sep 22, 2023

View reviewed changes

tests/test_plugins_os_unix_config_default_parser.py Outdated Show resolved Hide resolved

cecinestpasunepipe previously approved these changes Sep 22, 2023

View reviewed changes

Miauwkeru dismissed cecinestpasunepipe’s stale review via 1c8f9b4 September 25, 2023 08:22

Miauwkeru force-pushed the DIS-2156_make-configurationparser-useful branch from 8a15ef5 to 11e5728 Compare September 25, 2023 09:29

Miauwkeru requested review from Schamper and pyrco September 25, 2023 10:41

pyrco requested changes Sep 25, 2023

View reviewed changes

pyrco requested changes Sep 28, 2023

View reviewed changes

Miauwkeru requested a review from pyrco September 28, 2023 11:30

Miauwkeru force-pushed the DIS-2156_make-configurationparser-useful branch 3 times, most recently from 9da1b82 to e71a21f Compare September 28, 2023 13:19

Schamper previously requested changes Sep 29, 2023

View reviewed changes

dissect/target/filesystems/config.py Outdated Show resolved Hide resolved

dissect/target/plugins/general/config.py Outdated Show resolved Hide resolved

tests/data/config_tree/hosts Outdated Show resolved Hide resolved

pyrco requested changes Sep 29, 2023

View reviewed changes

dissect/target/filesystems/config.py Show resolved Hide resolved

Miauwkeru requested a review from Schamper October 3, 2023 12:33

Miauwkeru added 23 commits October 9, 2023 16:00

Rename test files

e2a9d02

Resolve review comments

7c823f1

Add docstrings

3aae4dd

Create a shortcut with target.etc

b379117

Fix linting and import order

e69b961

Move code to specific directories

a776efe

Resolve comments

8e6480a

Update error description

6160572

Fix tests

a377860

Change _write_value_mapping to a string buffer.

5c96df2

Also converts it to bytesio at the `open` of the filesystem entry

Change the indentation behavior

fb5cdb5

Add suggestions

74ee882

Fix UnixConfigTreeCli

a764d5f

Fix fs tests to use /etc

2800140

Update documentation

7f93926

Add as_dict to config_tree and etc functionality

99054d8

This allows an analyst to make it into a dict on command. otherwise, just ask `as_dict()` on a ConfigurationEntry

Resolve comments

e619952

Fix fs according to feedback

776be20

Move test files to helpers

327d5f7

Use lru_cache differently

1612fb2

Remove redundant pass

56ab6f0

add docstring to _get_till_file

a11b1f3

Add comment at ConfigurationEntry creation

f09f244

Miauwkeru force-pushed the DIS-2156_make-configurationparser-useful branch from 14a69ca to f09f244 Compare October 9, 2023 14:01

Miauwkeru merged commit 795cd7e into main Oct 9, 2023

Miauwkeru deleted the DIS-2156_make-configurationparser-useful branch October 9, 2023 14:43

Schamper added a commit that referenced this pull request Oct 27, 2023

Make configurationparser use ready (#386)

3d7d28f

(DIS-2156) Co-authored-by: pyrco <[email protected]> Co-authored-by: cecinestpasunepipe <[email protected]> Co-authored-by: Erik Schamper <[email protected]>

Poeloe pushed a commit that referenced this pull request Feb 29, 2024

Make configurationparser use ready (#386)

bff178f

(DIS-2156) Co-authored-by: pyrco <[email protected]> Co-authored-by: cecinestpasunepipe <[email protected]> Co-authored-by: Erik Schamper <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make configurationparser use ready #386

Make configurationparser use ready #386

Miauwkeru commented Sep 5, 2023

codecov bot commented Sep 5, 2023 •

edited

Loading

Schamper Sep 13, 2023

Miauwkeru Sep 19, 2023

Schamper Sep 19, 2023

Schamper Sep 13, 2023

Miauwkeru Sep 19, 2023

Schamper Sep 13, 2023

Miauwkeru Sep 19, 2023

Schamper Sep 19, 2023

Miauwkeru Sep 25, 2023

Schamper Sep 26, 2023 •

edited

Loading

Schamper Sep 19, 2023



		class ConfigurationTreePlugin(Plugin):
		__namespace__ = "config_tree"

Make configurationparser use ready #386

Make configurationparser use ready #386

Conversation

Miauwkeru commented Sep 5, 2023

codecov bot commented Sep 5, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Schamper Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 5, 2023 •

edited

Loading

Schamper Sep 26, 2023 •

edited

Loading