-
Notifications
You must be signed in to change notification settings - Fork 113
Wire in SSL certificate support #387
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
Conversation
8da12ba to
5e25200
Compare
csordasmarton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small comments otherwise LGTM!
| std::string filename = it->path().filename().native(); | ||
|
|
||
| if (!fs::is_directory(filename)) | ||
| // Ignore plain files in the workspace directory - projects are always |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can log some warning that the workspace directory contains some irrelevant files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be done... the "workspaceservice" should not have authority over what the webserver loads from there, etc. We'd have to duplicate ignoring "authentication.json", "certificate.pem" and potential other files here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can log some warning that the workspace directory contains some irrelevant files.
The plugin registrar emits warnings in case the project directory does not contain the appropriate project_info.json - it was only workspace service which despite this, sent them name of the project out on the API.
5e25200 to
ed3c8b5
Compare
csordasmarton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
If a
certificate.pemis found under the workspace, start the server with SSL enabled. Turns out the Mongoose we currently use actually does support this.