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

Support ssl (--[no-]ssl option) #87

Merged
merged 2 commits into from
Aug 22, 2014
Merged

Support ssl (--[no-]ssl option) #87

merged 2 commits into from
Aug 22, 2014

Conversation

sawanoboly
Copy link
Contributor

update for us and #86 .

@jaymzh
Copy link
Collaborator

jaymzh commented Aug 22, 2014

Well, that was easier than I thought it'd be. Nice!!! :)

@odcinek check this awesomeness out. :)

👍

I technically have merge capability here but lets see the tests pass as well as make sure no one at ChefInc has any objections.

@odcinek
Copy link

odcinek commented Aug 22, 2014

woot 👍

@sawanoboly
Copy link
Contributor Author

@jaymzh @odcinek
Thanks for review 😄

Regards,

@jkeiser
Copy link
Contributor

jkeiser commented Aug 22, 2014

That is pretty easy! Nice!

jkeiser added a commit that referenced this pull request Aug 22, 2014
Support ssl (--[no-]ssl option)
@jkeiser jkeiser merged commit 6319b1e into chef:master Aug 22, 2014
@jkeiser
Copy link
Contributor

jkeiser commented Aug 22, 2014

I'll drop this in the 3.0 release (coming soon now that enterprise support is pedant clean).

@sawanoboly
Copy link
Contributor Author

@jkeiser
Thanks 😃

A supplementary explanation, I have merged and tested this function in oc-chef-pedant branch(become 3.0?) locally. It works.

@jaymzh
Copy link
Collaborator

jaymzh commented Nov 25, 2014

This doesn't seem to work ... at least not with the omnibus build... knife (10, 11, and 12) get a connection refused on a knife cookbook upload -a after the first several requests:

...
DEBUG: Initiating PUT to http://HOSTNAME_HERE:5460/file_store/checksums/f626ed24a129a4f89103b0b2f43f5542
DEBUG: ---- HTTP Request Header Data: ----
DEBUG: Chef::HTTP calling Chef::HTTP::ValidateContentLength#handle_request
...
DEBUG: Content-Length: 666
DEBUG: Content-Length: 666
/opt/chef/embedded/lib/ruby/2.1.0/net/protocol.rb:211:in `write': Connection reset by peer (Errno::ECONNRESET)

This is with chef-12 rc1, but we get the same problem with chef-10 and chef-11.

chef-zero doesn't seem to notice or care... from that side, with debug on, I see:

--- RESPONSE (201) ---
{
  "uri": "http://HOSTNAME_HERE:5460/sandboxes/1",
  "checksums": {
     ...
  },
  "sandbox_id": "1"
}
--- END RESPONSE ---

and nothing else. CHef zero is still happy and will still accept new connections, but knife will continue to fail after the initial sandbox report.

@jaymzh
Copy link
Collaborator

jaymzh commented Nov 25, 2014

oh, and:

$ /opt/chef/embedded/bin/chef-zero --version
3.2

again, from chef 12 rc1

@sawanoboly sawanoboly deleted the support_ssl branch November 25, 2014 03:53
@sawanoboly
Copy link
Contributor Author

@jaymzh
hmm .. I seem that chef-zero(3.2) has bug on filesystem backend. I think it has caused some problems.

@jaymzh
Copy link
Collaborator

jaymzh commented Nov 25, 2014

You talking about #99? The symptom is pretty different, we get no has_key? errors. We had similar problems with chef-zero 2x... but I'll investigate more.

@sawanoboly
Copy link
Contributor Author

@jaymzh
I understand. I have reproduced this error.

 % knife cookbook upload --all 
ffi-yajl/json_gem is deprecated, these monkeypatches will be dropped shortly
Uploading apache2      [1.0.0]
Uploading php          [1.0.0]
ERROR: Errno::EPIPE: Broken pipe

Maybe, the URI scheme of response must be https when using ssl.
I'll try to fix it.

@sawanoboly
Copy link
Contributor Author

@jaymzh

I have fixed it 😃 Please try #104.

@jaymzh
Copy link
Collaborator

jaymzh commented Nov 25, 2014

You da man, thanks!

sersut pushed a commit that referenced this pull request Nov 26, 2014
fix: should set https to rack.url_scheme #87
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants