[chef-dev] Re: Re: Re: CHEF-2988 allowed_recipes, restricted_recipes, and override_recipes

[Chris Roberts < >]

Hi Bryan,

Just wanted to make a few responses to your points:

On Mon, Mar 26, 2012 at 3:58 PM, Bryan McLellan < "> > wrote:

I don't like that in this patch we deal with allowed recipes and
restricted recipes in different parts of the code. That's not
necessarily the patches fault, but I think requires that if we went
down this road we did some refactoring.

The restricted/allowed recipes are dealt with in the same area of the code (run_context and language_include_recipe). It's the run list override that is dealt with in a separate area of the code (due to it's nature). 
 

A more easily correctable issue is that the logs regarding allowed and
restricted recipes both call them restricted recipes, making the
interaction between these features unclear.

When the allowed recipes option is used, the logs will output a notice with the list of allowed recipes. From that point, any recipe provided in the run list that is not within the allowed recipes is implicitly a restricted recipe at the run list level, which is then reported on attempted loads. Changing the output to specify that a recipe is restricted due to the fact that it is not within the allowed recipes list would be trivial to add and may provide more useful context.
 

How would this functionality work with a larger policy? Would this
extend well into setting allowed and restricted cookbooks in a role?

With regards to the allowed/restricted recipes, it would be very easy to allow this to be set via role attributes. After the attribute expansion within the #load on RunContext, the node could be checked for allowed/restricted recipes and set them. However, I would see any values already set having precedence as the command line call should have the final say.
 

> - A "straw man" fix to non-compatible recipe inclusion in the case of
> multi platform cookbook metadata dependency, e.g. apt & yum

It seems that the fix would be not including those cookbooks, and
rather including them based on node['platform'].

I would agree with this and personally would lean towards either a layout much like the templates/files use or separate platform specific recipes that can be dynamically loaded via a "base" recipe dependent on the currently detected platform. However, the latter falls back to cookbook design rather then providing platform specific structuring via the cookbook layout.

This patch removes a run list item from the nodes run list, but what
if we want to restrict a dependent recipe that isn't in the run list?
Given a run list of "recipe[check_sl]" where the check_sl cookbook
depends on the install_sl cookbook, if we run "sudo chef-client -r
recipe[install_sl]" we still run the install_sl::default recipe
because it was not in the run list but rather was in the expanded run
list. This is pretty confusing because it doesn't do what you just
asked it to do, which was prevent that cookbook from running.
Shouldn't this fail from the start?

The dependency selector has already run on the server and given us a
set of cookbook versions that match the provided dependencies, but
what are the consequences of restricting a cookbook now? I think we're
likely to have recipes fail and it is going to be unclear why, a
situation that is prevented by the chef-server preventing a set of
cookbook versions that don't resolve the dependencies. It seems that
we would want to pass the run list by the server for a thumbs up
before going too far down a road that could lead to peril.

When a recipe is specified within the restricted-recipes option it is simply not allowed to run, either explicitly on the run list or implicitly via dependency. If recipe[check_sl] is dependent on recipe[install_sl] and install_sl is a restricted recipe, neither recipe will be loaded.

Thanks,
- Chris