-
Notifications
You must be signed in to change notification settings - Fork 5.6k
[master] feat: allow configurable destination for netbox ext_pillar data #65531
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
base: master
Are you sure you want to change the base?
[master] feat: allow configurable destination for netbox ext_pillar data #65531
Conversation
| default_kwargs["connected_devices"] = True | ||
| default_kwargs["destination_pillar_key"] = "netbox:minion" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copied the full test so this could be cleaned up for sure. I can add a minimal response fixture that is dedicated for this test if wanted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scratch that, I have now copied the fixture data also. In theory we could deduplicate this a little now if you would like.
| A colon-delimited compound key string of nested pillar keys in which the | ||
| data should be populated. This lets put the data in the pillar at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's a cleaner way to say this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this from ChatGPT:
A colon-delimited string representing a nested path within the pillar where the data should be placed. This allows flexibility in organizing pillar data. For example, you can store the data under netbox:minion or an-arbitrary-location, which is useful if you already use the netbox pillar and need to avoid key conflicts within it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll update this PR tonight, thanks!
|
|
|
the logic doesn't work, I will update |
|
Looks like the current test failure may be unrelated? |
| A colon-delimited compound key string of nested pillar keys in which the | ||
| data should be populated. This lets put the data in the pillar at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this from ChatGPT:
A colon-delimited string representing a nested path within the pillar where the data should be placed. This allows flexibility in organizing pillar data. For example, you can store the data under netbox:minion or an-arbitrary-location, which is useful if you already use the netbox pillar and need to avoid key conflicts within it.
What does this PR do?
This adds support to allow the user to configure in what pillar key the netbox ext_pillar data gets populated in. For example I already use the
netboxpillar for server and client configuration and there are a few overlapping keys there. Personally I would prefer to leave the existing config as-is and then move the minion data in to the pillar locationnetbox:minion.What issues does this PR fix or reference?
I didn't open an issue for this since it seemed like a trivial addition for me to just add and ask for opinions on afterwards.
Previous Behavior
The netbox minion data would always be populated in the
netboxpillar.New Behavior
The netbox minion data will be populated in the
netboxpillar by default but the user can now specify the pillar location they would like it placed in to, optionally.Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.