Doesn't really make sense to have fedora or redhat in the directory name.

I think we want to load policies from /etc/greenwave/policies/*.yaml and maybe we ship a /etc/greenwave/policies/fedora.yaml in the package, and then for the Red Hat deployment we can have extra YAML files dropped into /etc/greenwave/policies/ via some other means (config or a custom container or Ansible or whatever it is).

Also loading the policies afresh on every HTTP request might be too costly.

We could either treat them like config (load once at startup, you have to restart the app if you change the policies) or we could do some caching based on modtime of the directory and its files or similar.

I think for now it would be reasonable to treat the policies like configuraiton, and only load them once at start. Particularly if we are storing them under /etc/greenwave.

If we are OpenShift most likely it will require a re-deployment if we want to change them anyway. So the whole caching by modtime thing is not needed I think.

+1 with Dan's suggestion on putting this on /etc/greenwave.

Could you have rules be a dictionary instead of a list?

Then you could do something like:

rules:
    !PassingTestCaseRule:
    - test_case_name: dist.rpmdiff.analysis.abi_symbols
    - test_case_name: dist.rpmdiff.analysis.binary_stripping
    - etc...

This seems cleaner but I may be missing something here.

No, each rule needs to be an instance of PassingTestCaseRule.

My idea of using fedora/redhat directory is we may have to use multiply yaml files in the future if we have a huge list of policies. It would make hard to maintain. I might worry about too much though, :-)

Right so that is why I was suggesting it should load /etc/greenwave/policies/*.yaml. Then we can have redhat.yaml or we can split it up into rhel.yaml and blah.yaml or however else we want.

Rebased to address code review comments with the changes:

• load policies from /etc/greenwave/policies/*.yaml
• load policies from functional-test/policies/*.yaml when running functional tests
• add conf/policies/fedora.yaml so we can use it to ship a /etc/greenwave/policies/fedora.yaml
  in the package.

Maybe we should just use conf/policies for this one too? So that there is only two possible places to put the instead of three.

But this is okay too.

This is more like policy_filenames not policies_path, since it's a list.

This is a list of lists of policies, not a list of policies...

Might be clearer to use a for loop:

app.config['policies'] = []
for policy_filename in policy_filenames:
    app.config['policies'].extend(yaml.load_all(open(policy_filename, 'r')))

Gotta remember to put this into the .spec also

Yeap.

Yeah, policy_pathnames might be more accurately.

https://docs.python.org/2/library/glob.html

Rebased to address Dan's comments.

Rebased again to use conf/policies for storing the policies used for the tests rather than functional-test/policies.

Since #21 is merged we have a spec file now, which means this commit will need to add PyYAML to the spec file.

PyYAML is already in the spec file in this patch.

Sorry. I scrolled right past that and didn't see.

:+1: