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

[poliva83]

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.

[jkeiser]

Things in Cheffish don't generally follow this pattern (though it's not unreasonable). Generally, they expect chef_server_url to be https:///organizations/org . That's a general Chef pattern.

[poliva83]

This is the recipe snippet I was playing around with. So your saying I should be changing the chef_server_url before calling chef_group lwrp and append /organizations/myorgname?

chef_server_url = "https://localhost:443"

run_context.cheffish.current_chef_server = {
  :chef_server_url => chef_server_url,
  :options => {
    :client_name => "pivotal",
    :signing_key_filename => "/etc/opscode/pivotal.pem" }}

private_key "/etc/opscode/poliva.pem" do
  public_key_path "/etc/opscode/poliva.pub"
end

chef_user "poliva" do
  display_name "Phil Oliva"
  admin true
  email "[email protected]"
  password "passw0rd"
  source_key_path "/etc/opscode/poliva.pem"
end

chef_organization "myorgname" do
  full_name "MyOrgName"
  members "poliva"
end

chef_group "admins" do
  users "poliva"
  org "myorgname"
end

[jkeiser]

Yes, the convention is that :chef_server_url is always the thing just above /nodes, /clients, etc. The reason not to change it at the moment is that this variable corresponds to the Chef::Config.chef_server_url variable, which refers to that with such ubiquity that changing it would be rather issueful. There is a new config variable coming down the pike that refers to the root, but until that lands what I've been doing is assuming rest refers to "organizations/blah" and using rest.root_url instead of a relative PUT when I need to talk to the top level: https://github.com/chef/cheffish/blob/master/lib/chef/provider/chef_organization.rb#L18

That makes it so you can respect users who leave off /organizations/orgname, and support users who put it on, all with the same code.

[poliva83]

@jkeiser Thanks for the clarification on the Chef pattern. It makes sense and I will back off this PR after we figure the API for adding members in organization in one-shot.

[chef-supermarket]

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]

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

[andrewelizondo]

👍

[poliva83]

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

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

[tyler-ball]

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

[tyler-ball]

Can you add a # TODO comment here saying we would like to use the single API but it wasn't working?

[tyler-ball]

I'm 👍 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]

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]

Hey @poliva83 I'm going to close this because it looks like we merged a duplicate change in #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]

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]

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