policy-engine: add TOML configuration for policy plugins

[lucab]

This adds logic and basic tests to source policy plugins settings
from TOML configuration file.

Ref: https://jira.coreos.com/browse/CIN-3

[steveej]

Preliminary review pass with a few high-level comments.

[steveej]

This is pretty ugly. Do you see any chance we don't need this?
Why do we need this trait for BoxedPlugin?

[lucab]

This is used (so far exactly once) in tests: https://github.com/openshift/cincinnati/pull/129/files#diff-abed32e590a01cf6f55fd42829e13404R159.

This is indeed pretty ugly. However trait object safety rules disallow any use of generics (which PartialEq has), so we can't just require/use the implementation from underlying real type. Similar consideration for Serialize. Instead, Debug contains no generics, so it works here.

The only improvement I can think of here is to make this #[cfg(test)], as I think we are not going to use it outside of tests.

[steveej]

The only improvement I can think of here is to make this #[cfg(test)], as I think we are not going to use it outside of tests.

That seems like a reasonable workaround. You could even pull the impl into the tests module.

[lucab]

Bad news: the consumer of this is in a separate crate, so neither of the two approaches above work (as the cfg(test) affects the top-level crate, not the dependencies).

[steveej]

That's unfortunate. So we can only place a sentence in the docstring that it's only used for testing but we can't statically enforce that right now.

[lucab]

Amended, PTAL.

[steveej]

Do you mind explaining why you would like to have a separate trait for this? It would also work to have a blanket implementation of try_merge in the MergeOptions<T> trait with just Ok(self.merge(T)).

[lucab]

Discussed out of band: let's unify and just keep a single trait (the fallible one).

[lucab]

@steveej rebased to latest master and addressed all comments, PTAL.

[steveej]

This
/lgtm
But putting it on
/hold
to give you another opportunity to dig for a solution to the PartiqlEq workaround. Feel free to cancel the hold on your own accord.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lucab, steveeJ

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [lucab,steveeJ]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[lucab]

I looked a bit more into cargo test environment variables but didn't find anything I can directly use for conditional behavior. I'm merging as is.

/hold cancel