-
Notifications
You must be signed in to change notification settings - Fork 227
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
Add configuration options to the terminus to filter facts out #2634
base: 6.x
Are you sure you want to change the base?
Conversation
This changeset adds two configuration options used by the facts PuppetDB indirector: * fact_names_blacklist * fact_names_blacklist_regex They can be used to configure a list of fact names that will never be sent to PuppetDB, based on exact fact names or regular expressions.
Can one of the admins verify this patch? |
5 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Waiting for CLA signature by @nbarrientos @nbarrientos - We require a Contributor License Agreement (CLA) for people who contribute to Puppet, but we have an easy click-through license with instructions, which is available at https://cla.puppet.com/ Note: if your contribution is trivial and you think it may be exempt from the CLA, please post a short reply to this comment with details. http://docs.puppet.com/community/trivial_patch_exemption.html |
The CLA is already signed by my employer. |
@nbarrientos thanks for the PR! We may be interested in this change, but I'll need a bit more time to get back to you. The code you're changing here ends up getting packaged with a different project (puppetserver) so it would require changes from that team to validate any user supplied regex patterns in the blacklist option when they're starting up. Also, need to talk with the people I work with and see what their opinions are with regards to overall performance, etc. I'm on leave next week, but will try to give a more detailed response when I return. Thank you again for your interest! |
@nbarrientos sorry it's taken a little while to get back to you. I asked around and someone suggested using the facter blocklist option as a way to block facts before they're collected. This could be used to filter out troublesome facts before they even make it to the indirector. Would this satisfy your use case, or is having regex support for blacklisting facts in the indirector important to you? One reason we need to have the existing blacklist option in PDB is that the ingest api is public and in theory people may be submitting data to PDB outside of the indirector. The only way to make sure a blacklisted fact doesn't end up in PDB is to have the filtering done on the ingestion side. |
Hi, Thanks for getting back to me. In our case that approach does not suffice because we actually need those facts in the masters to make configuration decisions based on their values. What we don't want is to store them in PDB :) |
Friendly ping :) |
Hi @nbarrientos thanks for the ping and sorry for the delay getting back to you again. I spoke with the puppetserver team where the terminus code is run. They're ok with the proposed changes of adding a blacklist option in the terminus before sending facts along to pdb. If you would like to work on this and add testing and documentation like you suggested we'll gladly take a look. With the approaching holidays and an upcoming scheduled release we may be a little slow to review, but we'll get to it as soon as possible. If you're busy this may be something we'd want to add ourselves, but it would have to wait on a few other things we have in progress. One outstanding thing we'd need to figure out is the best way to validate any user supplied regex and how that validation would fit into puppetserver. I can help look into this once I clear a few other things off my todo list. We may also need to think about what we'd want to call the terminus blacklist vs. the one that's already in puppetdb to avoid confusion. Thanks again for the pr! |
Hi @Zak-Kent! Thanks for coming back to us. Ok, perfect. I will create a task on our side to improve the patch to add some extra tests and all the necessary docs. We expect to work on it early next year. As I mentioned, this is code that we have already running since a few months in production in our deployment. We can work out later together the naming of the configuration parameters etc. |
Thanks @nbarrientos! that sounds great |
|
This changeset adds two configuration options to be consumed by the facts PuppetDB indirector:
They can be used to configure a list of fact names that will never be sent to PuppetDB, based on exact fact names or regular expressions. I'm aware that PuppetDB supports black listing facts but over here we do prefer to filter them in the origin to avoid sending bits over the wire that will be discarded by the other end.
I'm happy to further develop the change request adding tests and documentation if you're interested.