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

Mine minion acl #54100

Closed
wants to merge 16 commits into from
Closed

Conversation

github-abcde
Copy link
Contributor

@github-abcde github-abcde commented Aug 2, 2019

What does this PR do?

It adds a new format for defining mine_functions that is equal to the format used for module.run.
It adds deprecation warnings for the old style format until version Aluminium.
It adds minion-side ACL when sending items to the mine, or updating the mine, which uses allow_tgt and allow_tgt_type to determine the list of minions that is allowed to retrieve this function from the salt mine. Minions that do not have access get an empty result, just as if the requested function was not present.
Also a slight refactor has been performed, cleaning up code in salt.modules.mine significantly.

What issues does this PR fix or reference?

I have mentioned the minion-side ACL as a possible solution for #45882. Whether or not this will resolve that specific issue is up for debate though :)
#6437

Previous Behavior

All mine functions (and their data) are retrievable by all minions.

New Behavior

Mine functions (and their data) can be made available to targeted minions only.

Tests written?

Yes, plenty. There were none for the salt mine in salt.modules.mine to begin with.

Commits signed with GPG?

Yes

@github-abcde github-abcde requested a review from a team as a code owner August 2, 2019 14:25
@ghost ghost requested a review from DmitryKuzmenko August 2, 2019 14:25
@mchugh19
Copy link
Contributor

mchugh19 commented Aug 2, 2019

Could use a release note entry about the data format update and new mine targeting functionality.

@github-abcde
Copy link
Contributor Author

@mchugh19 Since this is based on the develop branch...should I create a new sodium.rst for that (since there is none yet) in doc/topics/releases?

@mchugh19
Copy link
Contributor

mchugh19 commented Aug 2, 2019

Yep! I think that's correct.

Using kwargs is misleading as only the keyword func_args would be extracted from kwargs.
Updated function docstring.
Altered call_function to accept *args and **kwargs instead of just kwargs with a magic keyword that contained the args.
Removed unused continue. Removed unused variables na_type, kw_type. Renamed variables for more readability.
…ests to use self.assert* instead of test.fail.
@dwoz
Copy link
Contributor

dwoz commented Jan 2, 2020

Closing this as #55760 adds the functionality to the master branch.

@dwoz dwoz closed this Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants