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

ACL fixes - idempotentcy and port bug. #214

Merged
merged 2 commits into from
Jan 8, 2016

Conversation

sigerber
Copy link
Contributor

@sigerber sigerber commented Jan 6, 2016

Implemented prefetch so that multiple puppet runs will not attempt to recreate the ACLs. Also fixed ACLs not working due to a typo introduced by the addition of the port parameter.

Note: I've only been playing with puppet for a couple of weeks so this isn't necessarily the best implementation. I'm also a bit uncomfortable about the reliance on 'port' and 'acl_api_token' needing to be correct across all tokens or bad things might happen.

Implemented prefetch so that multiple puppet runs will not attempt to
recreate the ACLs. Also fixed ACLs not working due to a typo
introduced by the addition of the port parameter.
@solarkennedy
Copy link
Contributor

@sigerber does this actually make puppet resource consul_acl spit stuff out?

@michaeltchapman or @mdelagrange can you code review this?

@solarkennedy solarkennedy mentioned this pull request Jan 7, 2016
@sigerber
Copy link
Contributor Author

sigerber commented Jan 7, 2016

Sorry, no. You need to implement 'self.instances' for that - but self.instances doesn't take any parameters, so there isn't a way AFAIK to feed in the port and ACL token. This just fixes the port and id bug and stops puppet trying to create the resource every time there is a puppet run.

@mdelagrange
Copy link
Contributor

seems reasonable to me

@solarkennedy
Copy link
Contributor

@sigerber can you merge master then I'll merge it in?

Conflicts:
	lib/puppet/provider/consul_acl/default.rb
@sigerber
Copy link
Contributor Author

sigerber commented Jan 8, 2016

@solarkennedy Done - and hopefully did it right.

@solarkennedy
Copy link
Contributor

Thanks! I (personally) am not a user of this particular feature of this module, and we don't have a lot of test coverage on these, so we're going to depend on you and the other users on this thread to test this irl.

solarkennedy added a commit that referenced this pull request Jan 8, 2016
ACL fixes - idempotentcy and port bug.
@solarkennedy solarkennedy merged commit 7ba8c24 into voxpupuli:master Jan 8, 2016
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.

3 participants