search

TWCable / grabbit

Grabbit - Fast Content Sync tool for AEM/CQ
Apache License 2.0
125 stars 64 forks source link

Add https sync support (fix #149) #154

Closed jonekdahl closed 7 years ago

jonekdahl commented 7 years ago

Introduces support for https via a serverScheme configuration parameter, allowed values are http or https. The parameter defaults to http so it should be backwards compatible.

Nothing has been changed in grabbit.sh as I wasn't sure how to best implement a flag for disabling certificate checking.

review-ninja commented 7 years ago

ReviewNinja

jbornemann commented 7 years ago

Great work @jonekdahl! Thank you!!

We may be able to add https://curl.haxx.se/docs/manpage.html#-k to the grabbit.sh curl commands.

jbornemann commented 7 years ago

Pulling to try out the changes..

jbornemann commented 7 years ago

Looks good for http case! I don't have certificate stores, keys, etc set up properly for SSL. Did everything look fine when you tested in your setup?

A last note, is maybe we should add a line to

https://github.com/TWCable/grabbit/blob/master/docs/Running.adoc

describing serverScheme as an optional field.

Thanks! We will get this merged!

jonekdahl commented 7 years ago

Hi @jbornemann. Yes, I have verified the https case. I also pushed a minor documentation patch adding a description for serverScheme, I hope that covers it?

jbornemann commented 7 years ago

That should cover it @jonekdahl! Thank you for your contribution! 😄

I'll get this merged. +1

sagarsane commented 7 years ago

Awesome 👍 thanks for the contribution @jonekdahl!

jbornemann commented 7 years ago

Squashed and merged! Thanks again!