search

chef / cheffish

Resources and tools for testing and interacting with Chef and Chef Server.
Apache License 2.0
38 stars 28 forks source link

Fixes ability to add user group to certain organization on chef 12 server #40

Closed poliva83 closed 9 years ago

poliva83 commented 9 years ago

Fixes Issue #38 so the chef_group lwrp can be used to add users to certain group of an organization. User must be part of organization already so had to patch chef_organization lwrp to add all users in members attribute to organization when create action is called. Changes needed for cheffish gem to work with lastest chef 12 chef-server-core package.

chef-supermarket commented 9 years ago

Hi. I am an automated pull request bot named Curry. There are commits in this pull request whose authors are not yet authorized to contribute to Chef Software, Inc. projects or are using a non-GitHub verified email address. To become authorized to contribute, you will need to sign the Contributor License Agreement (CLA) as an individual or on behalf of your company. You can read more on Chef's blog.

Non-GitHub Verified Committers

There are 1 commit author(s) whose commits are authored by a non-GitHub verified email address. Chef will have to manually verify that they are authorized to contribute.

Please sign the CLA here.

poliva83 commented 9 years ago

My github account is linked to a chef account that is under CCLA

andrewelizondo commented 9 years ago

:+1:

poliva83 commented 9 years ago

@tyler-ball This PR isn't waiting on me correct? I should be covered under CCLA.

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

tyler-ball commented 9 years ago

@poliva83 correct, its not waiting on you - we just need to review it

tyler-ball commented 9 years ago

I'm :+1: on the code, but I don't have a good grasp of the scope of these changes. I would really like to get some time from @jkeiser to review, but if he isn't available I don't want to block on merging this.

tyler-ball commented 9 years ago

Hey @poliva83 can you check the spec for this? https://github.com/chef/cheffish/blob/master/spec/integration/chef_organization_spec.rb#L58-L65

A change in behavior should require a change in tests. Can you reach out to @randomcamel and see if y'all can figure out if this test was failing before or why it isn't failing now? Could chef-zero be missing support for these new API endpoints?

If we can get the tests to pass and we think they are actually testing this functionality, I'll feel good about merging it.

tyler-ball commented 9 years ago

Hey @poliva83 I'm going to close this because it looks like we merged a duplicate change in https://github.com/chef/cheffish/pull/50. Thats my fault that I didn't realize these PRs were the same.

I know that you had to manually remove the association_request but the server should be taking care of that for us. Would you mind using the master branch of Cheffish and seeing if it behaves the way you expect? If not we can re-open this with the intention of managing the association_request ourselves (or we can start ignoring the empty JSON output as @jkeiser suggests).

tyler-ball commented 9 years ago

I thought there was a pedant test in https://github.com/chef/chef-server/blob/master/oc-chef-pedant/spec/api/account/account_association_spec.rb which correctly verified that, upon adding a user to an org, we delete the association request. At least, we had to update chef-zero to have that behavior in order to pass the current pedant tests. @poliva83 Please let me know if you try this and the server doesn't correctly delete the association request! It should.

poliva83 commented 9 years ago

@tyler-ball I'll be switching back to chef-server cookbook development again soon. Will verify on latest cheffish gem.