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

feat: Add SSL development option #8212

Closed
wants to merge 2 commits into from

Conversation

tofumatt
Copy link
Member

@tofumatt tofumatt commented Jul 26, 2018

Description

Adds an SSL option for the local environment; the site can be accessed via https://localhost:9999/ and an --ssl option can be passed to setup-local-env.sh to make the SSL site the default.

Fixes #8211.

Uses a self-signed certificate, because it's localhost, but otherwise works great. Should be handy for testing mixed-content issues, etc.

How has this been tested?

Ran locally; it worked. 😄

@tofumatt tofumatt requested a review from a team July 26, 2018 01:45
@tofumatt tofumatt changed the title feat: Add SSL development option (fix #8211) feat: Add SSL development option Jul 26, 2018

# Wait until the docker containers are setup properely
echo -en $(status_message "Attempting to connect to wordpress...")
until $(curl -L http://localhost:$HOST_PORT -so - 2>&1 | grep -q "WordPress"); do
until $(curl -L --insecure $PROTOCOL://localhost:$HOST_PORT -so - 2>&1 | grep -q "WordPress"); do
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future we should probably allow users to specify their hostname; I, for instance, find it handy to assign my Mac's local network name here instead of localhost for easier mobile/VM testing.

@tofumatt tofumatt force-pushed the try/8211-ssl-option-local-env branch 2 times, most recently from c280a3b to b9d745f Compare July 26, 2018 16:24
@tofumatt tofumatt force-pushed the try/8211-ssl-option-local-env branch from b9d745f to fbc9423 Compare August 7, 2018 23:07
@designsimply designsimply added the [Type] Build Tooling Issues or PRs related to build tooling label Jan 30, 2019
@designsimply
Copy link
Member

@ntwb I noticed your comment at #13441 (comment) saying you're interested in reviewing things about tooling and I was wondering if you'd like to help with this one?

@@ -6,6 +6,7 @@ services:
image: wordpress
ports:
- 8888:80
- 9999:443
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the change of port required here, why not keep using 8888 and use 8888:443?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's required, I just did it so you'd know which version of the site you were working against during development. It's not usual in my experience to expect the protocol to change without the port changing. No reason it can't change though.

@ntwb
Copy link
Member

ntwb commented Jan 31, 2019

@designsimply Sure, it's late here, left a quick comment, I'll test this tomorrow and add some more feedback and/or approve :)

@gziolo
Copy link
Member

gziolo commented Jan 31, 2019

@tofumatt - will you have some time to get it up to date or should someone take it over from here?

@tofumatt
Copy link
Member Author

I think it just needs a quick rebase but I don't really have the chance to get to it this week. It was originally related to #8164 which I noticed I was still assigned to but am not working on anymore, so I cleared assignment.

Sorry 'bout that!

@gziolo gziolo added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Mar 8, 2019
@gziolo
Copy link
Member

gziolo commented Apr 3, 2019

@WraithKenny - it looks like this PR won't get any further updates. Would you like to take over this issue and submit a patch based on your comment?

I'm closing this PR to let others work on #8211. Thanks, @tofumatt on your initial work 😃

@gziolo gziolo closed this Apr 3, 2019
@gziolo gziolo deleted the try/8211-ssl-option-local-env branch April 3, 2019 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants