sous-chefs / users

Development repository for the users cookbook

https://supermarket.chef.io/cookbooks/users

Apache License 2.0

138 stars 217 forks source link

Make resource independent of data bags #445

Closed frank-m closed 3 years ago

frank-m commented 3 years ago

Description

This pull request removes the hard depedency on data bags from the manage resource. As such the user can choose if they want to use attributes/data bags or a different solution of their preference.

Check List

[x] All tests pass. See TESTING.md for details.
[x] New functionality includes testing.
[x] New functionality has been documented in the README if applicable.

ramereth commented 3 years ago

@frank-m are you intending on completely removing support of databags or only making it optional?

frank-m commented 3 years ago

Make it optional. You can still pass the content of a data bag as the users property, but in the recipe instead of the the resource. I will provide an upgrading.md with details and make sure there is tests for that.

frank-m commented 3 years ago

So, I updated the unit and integration tests and made sure that both data bag and non data bag usage is tested. I also updated the readme and added upgrading.md.

Can someone do a first review? After that I will remove the WIP: and squash the commits.

frank-m commented 3 years ago

@ramereth @bmhughes I just ran into something while implementing the users cookbook that I want to add as a feature.

It allows you to specify if a user needs to have a group with its own name or not.

Do you think I can add it to this pull request so we can release it into 6.0.0, or do you want me to open a separate one later?

bmhughes commented 3 years ago

No it can go into this as it's a major release anyway

frank-m commented 3 years ago

@bmhughes did you also see the new functionality I added?

bmhughes commented 3 years ago

The username_group stuff? Yes I did, the branch could do with a good squash/rebase/reword when you're finished as it's a little hard to follow what's been done in what commit.

Also, you're probably going to hate me for this, but looking at this as a whole from the outside (I don't personally use this cookbook) I'd consider refactorting it to operate on a single user per resource and leave the multiple-user stuff to the wrapping cookbook. If you did this all the stuff around variables types and checks go away as you aren't dependant on an arbitrary hash being passed in that you hope has the correct types and values in it. As it stands this is a recipe implemented as a resource, I might be missing the point by not being a user of the cookbook though.

frank-m commented 3 years ago

I thought about that, but that would be a massive breaking change I reckon.

No problem to do that, but not sure if the users of the users cookbook would still want to use it then :)

ramereth commented 3 years ago

I thought about that, but that would be a massive breaking change I reckon.

No problem to do that, but not sure if the users of the users cookbook would still want to use it then :)

Yeah kind of the point of this cookbook is being able to manage a group of users easily. I don't think it'd be a good idea to make that change.

frank-m commented 3 years ago

I think I have addressed everything. Can you approve if you agree?

kitchen-porter commented 3 years ago

Released as: 6.0.0