per-module grains/states/modules/returners #2385

Closed
madduck opened this Issue Oct 29, 2012 · 9 comments

Comments

Projects
None yet
3 participants
@madduck
Contributor

madduck commented Oct 29, 2012

In the interest of sharing and code reuse, I am trying to write self-contained
state modules (i.e. mystate/init.sls and e.g. mystate/templatej2).

Some of my more complex modules need custom grains, states, modules or even
returners.

Currently, these need to be put into /_grains etc., which is outside the
state module. I would appreciate if custom grains etc. would also be copied
from _grains directories found in subdirectories.

@madduck

This comment has been minimized.

Show comment
Hide comment
@madduck

madduck Jan 30, 2013

Contributor

Would you consider a patch that copied e.g. /sudo/_states/permission.py to the minions as extmod/states/sudo/permission.py?

I don't know if this will then work in terms of namespacing, but it'd be the cleanest way to handle it.

But would this work, i.e. having two dots in the state declaration?

permit-apt-update-to-staff:
  sudo.permission.installed:
    […]

Also, this would mean that the fileserver would have to recursively scan the
file_repos for ''-directories and show them under subdirectories of the
current '
'-directories.

And the minions would have to make sure they obtain these recursively.

Contributor

madduck commented Jan 30, 2013

Would you consider a patch that copied e.g. /sudo/_states/permission.py to the minions as extmod/states/sudo/permission.py?

I don't know if this will then work in terms of namespacing, but it'd be the cleanest way to handle it.

But would this work, i.e. having two dots in the state declaration?

permit-apt-update-to-staff:
  sudo.permission.installed:
    […]

Also, this would mean that the fileserver would have to recursively scan the
file_repos for ''-directories and show them under subdirectories of the
current '
'-directories.

And the minions would have to make sure they obtain these recursively.

@madduck

This comment has been minimized.

Show comment
Hide comment
@madduck

madduck Jan 30, 2013

Contributor

I started hacking on salt.modules.saltutil and specifically I managed to
modify the _sync function such that sudo/_states/permissions.py gets
copies to extmods/states/sudo/permissions.py.

Unfortunately, sudo.permissions.installed is an invalid state name/function.
Salt seems to insist that what's before the first dot is the state, and the
second component is the function. It discards the third component:

# salt-call -l warning state.single sudo.permissions.installed tester
local:
    Data failed to compile:
    ----------
        Specified state sudo.permissions is unavailable.

This might be a trivial fix, and it might also be impossible to fix. I have to
pack up now, but if you have a pointer for where to look, I'd return to this
tomorrow.

Contributor

madduck commented Jan 30, 2013

I started hacking on salt.modules.saltutil and specifically I managed to
modify the _sync function such that sudo/_states/permissions.py gets
copies to extmods/states/sudo/permissions.py.

Unfortunately, sudo.permissions.installed is an invalid state name/function.
Salt seems to insist that what's before the first dot is the state, and the
second component is the function. It discards the third component:

# salt-call -l warning state.single sudo.permissions.installed tester
local:
    Data failed to compile:
    ----------
        Specified state sudo.permissions is unavailable.

This might be a trivial fix, and it might also be impossible to fix. I have to
pack up now, but if you have a pointer for where to look, I'd return to this
tomorrow.

@ghost ghost assigned thatch45 Jan 31, 2013

@thatch45

This comment has been minimized.

Show comment
Hide comment
@thatch45

thatch45 Feb 1, 2013

Member

Yes, there is a lot around the single dot, and in the compiler you would see that the dot is changed before execution anyway. If these states just got loaded into the root in extmods it would work just fine, and you would take off the sudo and just need to call it sudo_perms or something like that.
As for recursively looking for these, I think that we should make it a flag, or a config option, since someone with a huge tree does not need to have it parsing everywhere, and we could make it only load the ones matched in the top file.

Member

thatch45 commented Feb 1, 2013

Yes, there is a lot around the single dot, and in the compiler you would see that the dot is changed before execution anyway. If these states just got loaded into the root in extmods it would work just fine, and you would take off the sudo and just need to call it sudo_perms or something like that.
As for recursively looking for these, I think that we should make it a flag, or a config option, since someone with a huge tree does not need to have it parsing everywhere, and we could make it only load the ones matched in the top file.

@madduck

This comment has been minimized.

Show comment
Hide comment
@madduck

madduck Feb 1, 2013

Contributor

The point is that I want /states/sudo to be self-contained (a Git repository), so having something in /states/_states wouldn't work.

And yes, I could make /states/sudo/_states/permissions.py available as sudo_permissions (i.e. using '_' instead of '.'), or sudo::permissions, but I would prefer to stay as close as possible to the underlying Python, I guess.

Contributor

madduck commented Feb 1, 2013

The point is that I want /states/sudo to be self-contained (a Git repository), so having something in /states/_states wouldn't work.

And yes, I could make /states/sudo/_states/permissions.py available as sudo_permissions (i.e. using '_' instead of '.'), or sudo::permissions, but I would prefer to stay as close as possible to the underlying Python, I guess.

@thatch45

This comment has been minimized.

Show comment
Hide comment
@thatch45

thatch45 Feb 4, 2013

Member

I see where you are coming from, the question is how the loader system works, and the loader system does not support deep package loading. So it we allow for states to be in lower level directories they would still be loaded into a global namespace. even if they were in subdirs we need to load them into top level dirs when they sync. Otherwise we would need to modify the loader to accept subdirs, which would break a few other very legitimate features wrt a module being able to send a subdir over and not have to worry about it being exposed.

Member

thatch45 commented Feb 4, 2013

I see where you are coming from, the question is how the loader system works, and the loader system does not support deep package loading. So it we allow for states to be in lower level directories they would still be loaded into a global namespace. even if they were in subdirs we need to load them into top level dirs when they sync. Otherwise we would need to modify the loader to accept subdirs, which would break a few other very legitimate features wrt a module being able to send a subdir over and not have to worry about it being exposed.

@stale stale bot added the stale label May 13, 2017

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot May 13, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

stale bot commented May 13, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@UtahDave

This comment has been minimized.

Show comment
Hide comment
@UtahDave

UtahDave May 16, 2017

Member

This seems to still not work with how Salt's loader functions. We can reopen this issue if someone wants to do more work on this feature.

Thanks!

Member

UtahDave commented May 16, 2017

This seems to still not work with how Salt's loader functions. We can reopen this issue if someone wants to do more work on this feature.

Thanks!

@UtahDave UtahDave closed this May 16, 2017

@stale stale bot removed stale labels May 16, 2017

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot May 16, 2017

Thank you for updating this issue. It is no longer marked as stale.

stale bot commented May 16, 2017

Thank you for updating this issue. It is no longer marked as stale.

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot May 16, 2017

Thank you for updating this issue. It is no longer marked as stale.

stale bot commented May 16, 2017

Thank you for updating this issue. It is no longer marked as stale.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment