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

Use correct user-association endpoint for Chef 12 #50

Merged
merged 3 commits into from
Aug 20, 2015

Conversation

stevendanna
Copy link
Contributor

The correct endpoint for user-association is:

POST /organizations/ORGNAME/users

The API currently being made will be rejected with a 405 Method Not
Allowed error.

@stevendanna
Copy link
Contributor Author

Tests are failing as expected. I think to completely fix them we likely need some changes in chef-zero as well. Getting chef-zero in line with Chef Server 12 was started here:

chef/chef-zero#117

@tyler-ball
Copy link
Contributor

Can you update https://github.com/chef/cheffish/blob/master/spec/integration/chef_organization_spec.rb to add testing for this? If chef-zero is updated it will no longer throw a 405 but I would expect the specs that test this functionality to fail.

This is also a breaking change for old versions of chef-server, correct? So this would either need to be released as a major version or it would need to be updated to support both CS11 and CS12.

@tyler-ball
Copy link
Contributor

It looks like the integration tests only test against Chef Server 12. Do we even care about supporting Chef Server 11?

@stevendanna
Copy link
Contributor Author

Do we even care about supporting Chef Server 11?

Chef Server 11 is supported until Chef Server 13 comes out. In my opinion all API clients we maintain should support both.

jtimberman pushed a commit to chef-cookbooks/chef-server that referenced this pull request Jun 11, 2015
As part of a larger effort to make use of our own primitives, the
test::post-install recipe should use Cheffish chef_user and
chef_organization resources to create the example user and organization.

It may be many more lines of code, but at the end of the day it is
declarative, and these resources are inherently both convergent and
idempotent. Running the chef-server-ctl commands requires a guard, which
parses output that can change more easily in a user-facing tool. The
Cheffish resources use the REST API of the Chef Server, and changes to
the API itself is made with far more consideration.

That said, there's a bug in the Cheffish chef_organization resource,
addressed by the following pull request:

chef/cheffish#50

As such, we install the Cheffish gem built from the branch in that PR.
When that is merged and a new Cheffish gem is released, then we can
remove the git and rake use in the recipe, and just use chef_gem.
@tyler-ball
Copy link
Contributor

I'm going to mark this as blocked because it depends on chef/chef-zero#117

@tyler-ball
Copy link
Contributor

@stevendanna I just force pushed this. There is still 1 test failing so I'm going to troubleshoot that next.

@tyler-ball
Copy link
Contributor

  1) Chef::Resource::ChefOrganization When the Chef server is in multi-org mode and chef_server_url is pointed at /organizations/foo and already has an organization named x invites and membership tests chef_organization "x" upgrades invites to members when asked
     Failure/Error: expect(get('/organizations/x/association_requests').map { |u| u['username'] }).to eq(%w(invited2))

       expected: ["invited2"]
            got: ["invited", "invited2"]

       (compared using ==)
     # ./spec/integration/chef_organization_spec.rb:152:in `block (6 levels) in <top (required)>'

@tyler-ball tyler-ball self-assigned this Aug 18, 2015
@jkeiser
Copy link
Contributor

jkeiser commented Aug 19, 2015

@tyler-ball this change looks good assuming the endpoint is the way it looks. Is the above test failure actually caused by this change?

@tyler-ball
Copy link
Contributor

@jkeiser yes - the failures were different before this change.

The correct endpoint for user-association is:

POST /organizations/ORGNAME/users

The API currently being made will be rejected with a 405 Method Not
Allowed error.
@jkeiser
Copy link
Contributor

jkeiser commented Aug 19, 2015

Yeah, it looks like that is likely a chef-zero issue: if you do the POST thing to add a user to an org, it should remove any association requests as well.

@tyler-ball tyler-ball force-pushed the ssd/organization-force-associate branch from e2bd4db to 3e5c0db Compare August 20, 2015 13:29
tyler-ball added a commit that referenced this pull request Aug 20, 2015
Use correct user-association endpoint for Chef 12
@tyler-ball tyler-ball merged commit a5df7c0 into master Aug 20, 2015
@tyler-ball tyler-ball deleted the ssd/organization-force-associate branch August 20, 2015 13:50
@thommay thommay added Type: Bug Does not work as expected. and removed Bug labels Jan 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Does not work as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants