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

Explicit and document ACL rules format #21303

Closed
Lothiraldan opened this issue Mar 4, 2015 · 5 comments
Closed

Explicit and document ACL rules format #21303

Lothiraldan opened this issue Mar 4, 2015 · 5 comments
Labels
Bug broken, incorrect, or confusing behavior Documentation Relates to Salt documentation P1 Priority 1 severity-low 4th level, cosemtic problems, work around exists
Milestone

Comments

@Lothiraldan
Copy link
Contributor

ACL rules format are not documented, by digging into the code, we saw that rules are either "glob" rule or compound rule but with only one matcher.

Here (https://github.com/saltstack/salt/blob/develop/salt/utils/minions.py#L550-L551) the matcher os forced to to "glob" if the rule doesn't start with ".@".

For compound rules (https://github.com/saltstack/salt/blob/develop/salt/utils/minions.py#L545-L548), parsing is quite rudimentary and so will fail with compound matcher with more than one matcher.
For exemple with this rule 'S@10.0.2.0/24 or web*', the parsed v_expr will be '10.0.2.0/24 or web*' and so will be invalid.

What format do we want to support in ACL rules ?

@jfindlay jfindlay added Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists labels Mar 9, 2015
@jfindlay jfindlay added this to the Approved milestone Mar 9, 2015
@jfindlay
Copy link
Contributor

jfindlay commented Mar 9, 2015

@Lothiraldan, thanks for the report.

@ssgward ssgward added Documentation Relates to Salt documentation P1 Priority 1 labels Apr 16, 2015
@Lothiraldan
Copy link
Contributor Author

According to #31598, external auth support for all target types has been added.

@Lothiraldan
Copy link
Contributor Author

salt/salt/utils/minions.py

Lines 637 to 655 in 0a9e062

def _expand_matching(self, auth_entry):
ref = {'G': 'grain',
'P': 'grain_pcre',
'I': 'pillar',
'J': 'pillar_pcre',
'L': 'list',
'S': 'ipcidr',
'E': 'pcre',
'N': 'node',
None: 'glob'}
target_info = parse_target(auth_entry)
if not target_info:
log.error('Failed to parse valid target "{0}"'.format(auth_entry))
v_matcher = ref.get(target_info['engine'])
v_expr = target_info['pattern']
return set(self.check_minions(v_expr, v_matcher))
and
def parse_target(target_expression):
'''Parse `target_expressing` splitting it into `engine`, `delimiter`,
`pattern` - returns a dict'''
match = TARGET_REX.match(target_expression)
if not match:
log.warning('Unable to parse target "{0}"'.format(target_expression))
ret = {
'engine': None,
'delimiter': None,
'pattern': target_expression,
}
else:
ret = match.groupdict()
return ret
still fails to recognize compound matcher:

from salt.utils.minions import parse_target
parse_target('S@10.0.2.0/24 or web*')
>>> {'delimiter': None, 'engine': 'S', 'pattern': '10.0.2.0/24 or web*'}

I propose to make a PR which says explicitly that all matcher except compound are supported in eauth configuration.

@jfindlay
Copy link
Contributor

@Lothiraldan, should we close this in favor of #32737?

@Lothiraldan
Copy link
Contributor Author

@jfindlay Yes I forget but since I've create a PR for documentation and an issue for discussing the non-support of compound matcher, we can now close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Documentation Relates to Salt documentation P1 Priority 1 severity-low 4th level, cosemtic problems, work around exists
Projects
None yet
Development

No branches or pull requests

3 participants