Multiple policy bundle dirs #862

dspalmer99 · 2021-01-26T01:32:24Z

This PR updates localconfig to allow multiple folly-qualified dirs containing policy bundles to be passed in during bootstrap.

dspalmer99 · 2021-01-26T20:01:42Z

anchore_engine/configuration/localconfig.py

+def register_policy_bundle_source_dir(source_dir):
+    global POLICY_BUNDLE_SOURCE_DIRS
+    POLICY_BUNDLE_SOURCE_DIRS.append(source_dir)
+    load_policy_bundle_paths()


I'd like to refactor this to not have this extra call here, but as the config/bootstrapping currently exists this is necessary because in

anchore-engine/anchore_engine/twisted.py

Line 217 in e4d21c6

self._init_config(options)

when the service starts up, the path that leads to load_policy_bundle_paths() below is called before the service is initialized, so if the service adds policy bundles to the config object via a handler it will be to late, even if with a pre_config handler.

This is maybe slightly hacky, but multiple calls to load_policy_bundle_paths() will have no effect on existing bundles in the case where subsequent calls are just adding more bundles.

alfredodeza · 2021-01-26T20:11:24Z

anchore_engine/configuration/localconfig.py

+        if (
+            src_dirs is not None
+            and len(src_dirs) > 0
+            and policy_bundles_dir is not None
+        ):


in Python a None or an empty data structure like a list, will evaluate to False so this could be:

Suggested change

if (

src_dirs is not None

and len(src_dirs) > 0

and policy_bundles_dir is not None

):

if policy_bundles_dir is not None and src_dirs:

In that case, couldn't it also further be simplified to if policy_bundles_dir and src_dirs: ?

I wasn't sure what policy_bundles_dir were (was not clear if that was a list as well). Sometimes the logic may want to really ensure it isn't a None so that is valid. If you know that it doesn't need to check for a None just if it is empty, then your suggestion works!

alfredodeza · 2021-01-26T20:11:42Z

anchore_engine/configuration/localconfig.py

+                            }
+                        )
+                        copy_config_file(file, file_name, src_dir)
+                    except Exception as e:


what exception are we catching here?

Looking through the commit history, it looks like it was originally intended to catch an exception in the line you called out above.

Right. I mean, it is catching any exception that may show up. If this is existing code, feel free to ignore. For reference, this issue captures the problems with broad exceptions: #867

Yeah I had looked at 867 this morning. Totally agree that making that more targeted is a good thing to do, but this is existing code.

alfredodeza · 2021-01-26T20:11:57Z

anchore_engine/configuration/localconfig.py

+            for src_dir in src_dirs:
+                for file_name in os.listdir(src_dir):
+                    try:
+                        file = os.path.join(policy_bundles_dir_full_path, file_name)


this line will probably not bubble up an exception

From the docs it doesn't look like it does. I'll simplify.

alfredodeza · 2021-01-26T20:12:29Z

anchore_engine/configuration/localconfig.py

+                        )
+                        copy_config_file(file, file_name, src_dir)
+                    except Exception as e:
+                        logger.warn(


using logger.warn eats the traceback. Are we trying to fully obscure the traceback?

alfredodeza · 2021-01-26T20:13:18Z

tests/unit/anchore_engine/configuration/test_localconfig.py

-    assert len(config["policy_bundles"]) == len(config_filenames)
-    for config_filename in config_filenames:
+    assert len(config["policy_bundles"]) == len(config_filenames_flat)
+    for config_filename in config_filenames_flat:


I know you aren't adding this change in the PR, but loops in tests are not good because they don't allow to see the failures in each item of the loop

I agree that could be refactored but I'm not sure I want to pick that up as part of this PR. Also, when this failed on me (occasionally) during dev it tended to be all or nothing. Not saying that would always be the case though.

Signed-off-by: Daniel Palmer <dan.palmer@anchore.com>

dspalmer99 · 2021-01-26T22:48:13Z

Thanks for the feedback @alfredodeza . I've made the changes you asked for, and added a little extra logging and another test as well.

Signed-off-by: Daniel Palmer <dan.palmer@anchore.com>

zhill

lgtm

zhill · 2021-01-28T10:11:14Z

I'm going to squash-merge this since it's into the release branch and has 10 commits many of which are linting updates.

dspalmer99 · 2021-01-28T13:56:08Z

Ack, thanks @zhill

* Allow for localconfig to read policy bundles from multiple dirs. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Expect fully-qualifed policy bundle dirs. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Reload policy bundle from file whenever a new bundle dir is added. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting, again. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting commas Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Fix test. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Code review comments, add some extra logging and another test. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> Signed-off-by: Robert Prince <robert.prince@anchore.com>

* Allow for localconfig to read policy bundles from multiple dirs. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Expect fully-qualifed policy bundle dirs. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Reload policy bundle from file whenever a new bundle dir is added. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting, again. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting commas Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Fix test. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Code review comments, add some extra logging and another test. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> Signed-off-by: Samuel Dacanay <sam.dacanay@anchore.com>

* Improve the message and description for vulnerability_data_unavailable and stale_feed_data triggers in the vulnerabilities gate. Fixes #879 Signed-off-by: Zach Hill <zach@anchore.com> * Bump version numbers for 0.9.1 Signed-off-by: Robert Prince <robert.prince@anchore.com> * Multiple policy bundle dirs (#862) * Allow for localconfig to read policy bundles from multiple dirs. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Expect fully-qualifed policy bundle dirs. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Reload policy bundle from file whenever a new bundle dir is added. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting, again. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting commas Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Fix test. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Code review comments, add some extra logging and another test. Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Linting Signed-off-by: Daniel Palmer <dan.palmer@anchore.com> * Fix method name to match parent class Signed-off-by: Samuel Dacanay <sam.dacanay@anchore.com> * Removed ui from swagger url Signed-off-by: Zane Burstein <zane.burstein@anchore.com> * Add ability to support multiple grant types for the oauth client Signed-off-by: Zach Hill <zach@anchore.com> * Update Dockerfile to use UBI 8.3. Fixes #888 Signed-off-by: Zach Hill <zach@anchore.com> * Update CHANGELOG.md for 0.9.1 Signed-off-by: Zach Hill <zach@anchore.com> * Fix confusing typo in changelog Signed-off-by: Zach Hill <zach@anchore.com> * Update syft to v0.12.5 Signed-off-by: Dan Luhring <dan.luhring@anchore.com> * add bundles/ dir to anchore_service_dir Signed-off-by: Brady Todhunter <bradyt@anchore.com> * Updates to vulnerability listing dedup logic - prioritize vulnerabilities from other namespaces over nvd out vulnerabilities - filter duplicates Fixes #893 Signed-off-by: Swathi Gangisetty <swathi@anchore.com> * Set the python package location according to the package key, which is the absolute path (#895) Signed-off-by: Samuel Dacanay <sam.dacanay@anchore.com> * Update the scanner config method in policy engine for providing overr… (#896) * Update the scanner config method in policy engine for providing overridable functions for vuln and cpe results. Adds use of that in the vulnerability policy gate Signed-off-by: Zach Hill <zach@anchore.com> * first draft at a dedup pass Signed-off-by: Swathi Gangisetty <swathi@anchore.com> * Try to load Policy Engine ImageCpes from syft generated cpes, with fallback to fuzzy matching Signed-off-by: Samuel Dacanay <sam.dacanay@anchore.com> * Add unit test for loader paths Signed-off-by: Samuel Dacanay <sam.dacanay@anchore.com> * include update and meta into cpe comparison Signed-off-by: Samuel Dacanay <sam.dacanay@anchore.com> * fix return type Signed-off-by: Samuel Dacanay <sam.dacanay@anchore.com> * Unit tests for cpe comparisons used for vulnerability dedup Signed-off-by: Swathi Gangisetty <swathi@anchore.com> * Downgrade empty content log message to debug level Signed-off-by: Swathi Gangisetty <swathi@anchore.com> * tests: change previous invalid schema for integers/floats According to draft-6 which the new jsonschema supports 1.0 is considered an integer. Relevant doc from the draft: In draft-04, "integer" is listed as a primitive type and defined as “a JSON number without a fraction or exponent part”; in draft-06, "integer" is not considered a primitive type and is only defined in the section for keyword "type" as “any number with a zero fractional part”; 1.0 is thus not a valid "integer" type in draft-04 and earlier, but is a valid "integer" type in draft-06 and later; note that both drafts say that integers SHOULD be encoded in JSON without fractional parts Link https://json-schema.org/draft-06/json-schema-release-notes.html Signed-off-by: Alfredo Deza <adeza@anchore.com> (cherry picked from commit 2f859a1) * requirements: bump jsonschema to avoid legacy validator import issues Signed-off-by: Alfredo Deza <adeza@anchore.com> (cherry picked from commit 99dcb10) * Update syft to 0.12.7 to fix analysis failure due to syft parsing issue. Fixes #910 Signed-off-by: Zach Hill <zach@anchore.com> * Update cryptography lib to 3.3.2 from 3.3.1. Fixes #909 Signed-off-by: Zach Hill <zach@anchore.com> * add package filtering by relationships Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * Fix the client metadata merge process during oauth init. Fixes #931 Signed-off-by: Zach Hill <zach@anchore.com> * Bump version Signed-off-by: Robert Prince <robert.prince@anchore.com> * Add default admin pw to e2e test values file Signed-off-by: Robert Prince <robert.prince@anchore.com> * Make sure to return content correctly for manifest and dockerfile content types Signed-off-by: Samuel Dacanay <sam.dacanay@anchore.com> * [docs] 0.9.2 release notes and changelog updates. includes missing release notes for 0.9.1 (#939) Updates CHANGELOG for 0.9.2 and adds 0.9.1 and 0.9.2 release notes Also fixes ordering problem in release notes page Signed-off-by: Zach Hill <zach@anchore.com> * Update Quickstart Docker-Compose image tag to v0.9.2 Signed-off-by: Samuel Dacanay <sam.dacanay@anchore.com> * Iterate API patch version 0.1.16->0.1.17 Signed-off-by: Samuel Dacanay <sam.dacanay@anchore.com> * Add distro mapping from "redhat" to "rhel" for vuln matching Signed-off-by: Zach Hill <zach@anchore.com> * Adds distro mapper in import path to ensure rhel instead of redhat distro name Signed-off-by: Zach Hill <zach@anchore.com> * Fix integration tests that used redhat as a negative test example Signed-off-by: Zach Hill <zach@anchore.com> Co-authored-by: Zach Hill <zach@anchore.com> Co-authored-by: Dan Palmer <dan.palmer@anchore.com> Co-authored-by: Samuel Dacanay <sam.dacanay@anchore.com> Co-authored-by: Zane Burstein <zane.burstein@anchore.com> Co-authored-by: Dan Luhring <dan.luhring@anchore.com> Co-authored-by: Brady Todhunter <bradyt@anchore.com> Co-authored-by: Swathi Gangisetty <swathi@anchore.com> Co-authored-by: Alfredo Deza <adeza@anchore.com> Co-authored-by: Alex Goodman <alex.goodman@anchore.com> Co-authored-by: Alex Goodman <wagoodman@users.noreply.github.com>

dspalmer99 force-pushed the multiple_policy_bundle_dirs branch from e9d4a41 to b81e3d6 Compare January 26, 2021 19:17

dspalmer99 changed the title ~~WIP - Multiple policy bundle dirs~~ Multiple policy bundle dirs Jan 26, 2021

dspalmer99 requested a review from zhill January 26, 2021 19:53

dspalmer99 commented Jan 26, 2021

View reviewed changes

alfredodeza reviewed Jan 26, 2021

View reviewed changes

dspalmer99 added 8 commits January 26, 2021 17:29

Allow for localconfig to read policy bundles from multiple dirs.

7b7a5c6

Signed-off-by: Daniel Palmer <dan.palmer@anchore.com>

Expect fully-qualifed policy bundle dirs.

ef5ce07

Signed-off-by: Daniel Palmer <dan.palmer@anchore.com>

Reload policy bundle from file whenever a new bundle dir is added.

22589ce

Signed-off-by: Daniel Palmer <dan.palmer@anchore.com>

Linting

bfdde43

Signed-off-by: Daniel Palmer <dan.palmer@anchore.com>

Linting, again.

c848f3b

Signed-off-by: Daniel Palmer <dan.palmer@anchore.com>

Linting commas

b29b5cc

Signed-off-by: Daniel Palmer <dan.palmer@anchore.com>

Fix test.

a67bd19

Signed-off-by: Daniel Palmer <dan.palmer@anchore.com>

Code review comments, add some extra logging and another test.

b81ed55

Signed-off-by: Daniel Palmer <dan.palmer@anchore.com>

dspalmer99 force-pushed the multiple_policy_bundle_dirs branch from afc1617 to b81ed55 Compare January 26, 2021 22:44

dspalmer99 added 2 commits January 26, 2021 17:49

Linting

d8d041b

Signed-off-by: Daniel Palmer <dan.palmer@anchore.com>

Merge branch 'master' into multiple_policy_bundle_dirs

e05b7bf

dspalmer99 changed the base branch from master to v0.9.1-dev January 27, 2021 20:43

zhill approved these changes Jan 28, 2021

View reviewed changes

zhill merged commit 5e2a032 into anchore:v0.9.1-dev Jan 28, 2021

dspalmer99 deleted the multiple_policy_bundle_dirs branch January 28, 2021 13:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiple policy bundle dirs #862

Multiple policy bundle dirs #862

dspalmer99 commented Jan 26, 2021

dspalmer99 Jan 26, 2021

alfredodeza Jan 26, 2021

dspalmer99 Jan 26, 2021

alfredodeza Jan 27, 2021

alfredodeza Jan 26, 2021

dspalmer99 Jan 26, 2021

alfredodeza Jan 27, 2021

dspalmer99 Jan 27, 2021

alfredodeza Jan 26, 2021

dspalmer99 Jan 26, 2021

alfredodeza Jan 26, 2021

alfredodeza Jan 26, 2021

dspalmer99 Jan 26, 2021

dspalmer99 commented Jan 26, 2021

zhill left a comment

zhill commented Jan 28, 2021

dspalmer99 commented Jan 28, 2021

Multiple policy bundle dirs #862

Multiple policy bundle dirs #862

Conversation

dspalmer99 commented Jan 26, 2021

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

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

dspalmer99 commented Jan 26, 2021

zhill left a comment

Choose a reason for hiding this comment

zhill commented Jan 28, 2021

dspalmer99 commented Jan 28, 2021