Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Initial POC of using custom_resources [WIP] #357

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

teknofire
Copy link
Contributor

@teknofire teknofire commented Feb 13, 2019

WORK IN PROGRESS DO NOT MERGE

Description

The idea here is to replace the attribute driven nature of this cookbook with custom resources.

One primary driver for this is to make it easier to handle the transition between the profile list being an Array of Hashes vs. a Hash of Hashes (https://github.com/chef-cookbooks/audit#configure-node) and the errors and confusion that happens when users try to use hash syntax to add a profile to the default array format.

One difficulty is the report handler pulls the list of profiles from the node attributes. Which it appears cannot be modified inside a custom resource. To get around this, I've changed it to use node.run_state, which is transient and doesn't get saved to the chef-server. If someone needs those to show up there, then they can still use the old method to do that.

Issues Resolved

[List any existing issues this PR resolves]

Check List

The idea here is to replace the attribute driven nature of this cookbook
with node attributes. One difficult issue with this though is the report
handler pulls the list of profiles from the node attributes.  Which it
appears cannot modified inside a custom resource.

Signed-off-by: Will Fisher <[email protected]>
@teknofire teknofire requested a review from a team February 13, 2019 00:22
Will Fisher added 3 commits February 12, 2019 16:25
Basically right now it checks to see if there is already one with the
same name and will skip.

Signed-off-by: Will Fisher <[email protected]>
@btm
Copy link
Contributor

btm commented Feb 21, 2019

@teknofire what was the issue you had with modifying the node attribute from inside the resource?

Was it something like this?

Chef::Exceptions::ImmutableAttributeModification (Node attributes are read-only when you do not specify which precedence level to set. To set an attribute use code like `node.default["key"] = "value"')

@lamont-granquist
Copy link
Contributor

I was wondering if that was more of a compile-converge issue due to custom resource execution order (it will always be in the converge phase of the outer run context), but that would affect the node.run_state as well.

@teknofire
Copy link
Contributor Author

teknofire commented Mar 27, 2019

@btm @lamont-granquist There was no error, I didn't see the updated attributes in the audit report handler. Although at this point I think I've figured out how to fix that now using the override precedence although I'm having to make a few more adjustments as it's a VividMash at that point and can't push additional profiles as an array like I was attempting.

@teknofire
Copy link
Contributor Author

Although then it gets into the issue of who should win when setting the attributes and making sure something doesn't come in later modifying the format I specified which is why I was leaning more towards using the run_state with its own set of key names to prevent any confusion on what should win based on attribute precedence.

@zenspider zenspider added the Type: Enhancement Adds new functionality. label Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Adds new functionality.
Development

Successfully merging this pull request may close these issues.

4 participants