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

IBX-3465: Added Nginx vhost template #1704

Merged
merged 5 commits into from
Feb 23, 2023
Merged

IBX-3465: Added Nginx vhost template #1704

merged 5 commits into from
Feb 23, 2023

Conversation

glye
Copy link
Contributor

@glye glye commented Jul 29, 2022

Question Answer
JIRA Ticket https://issues.ibexa.co/browse/IBX-3465
Versions All supported

Like #1466 but for Nginx.
Sample vhost copied from https://github.com/ibexa/docker/blob/main/templates/nginx/vhost.template and Ibexified.

NB: It refers to files in ez_params.d, meaning in https://github.com/ibexa/docker/tree/main/templates/nginx/ez_params.d
Should we copy them over, too?

Alternatively, should we just link directly to the vhost file in the docker repo and not copy anything over?

@glye glye requested a review from DominikaK July 29, 2022 13:33
@DominikaK DominikaK self-assigned this Aug 1, 2022
Copy link
Contributor

@DominikaK DominikaK left a comment

Choose a reason for hiding this comment

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

The rationale for copying the files here was that the docker repo is unsupported, so I guess copying both files makes most sense.

docs/getting_started/install_ez_platform.md Outdated Show resolved Hide resolved
@glye
Copy link
Contributor Author

glye commented Aug 1, 2022

In that case, should we copy over the 6 extra params files, as well?

@DominikaK
Copy link
Contributor

Copying over is imperfect, ideally we should have a separate repo with this (I guess there could be other "resources" like this for the whole system), and keeping it in doc risks that the files will not be updated.

@glye
Copy link
Contributor Author

glye commented Aug 2, 2022

What say ye, @mnocon and @micszo? It does sound nice to me to maintain default vhosts and include them in testing, so they will be updated.

@micszo
Copy link
Contributor

micszo commented Aug 2, 2022

What about ibexa/post-install? Why not there?

@glye
Copy link
Contributor Author

glye commented Aug 2, 2022

So adding "apache" and "nginx" dirs with vhost examples under "resources" in ibexa/post-install? Sounds good to me. Ref. https://github.com/ibexa/post-install/tree/main/resources

What do the package maintainers there say about that? @Nattfarinn @webhdx @adamwojs

@glye
Copy link
Contributor Author

glye commented Aug 11, 2022

Post-release ping :)

What do the package maintainers there say about that? @Nattfarinn @webhdx @adamwojs

@webhdx
Copy link
Contributor

webhdx commented Aug 12, 2022

It makes sense to move them to ibexa/post-install. We should also make a command for generating vhosts. I remember we had a bash script back in a day: https://github.com/ezsystems/ezplatform/blob/master/bin/vhost.sh

@glye
Copy link
Contributor Author

glye commented Aug 12, 2022

Thanks. I copied the templates in ibexa/post-install#50
If that passes, I'll upgrade this PR to make use of them.

@DominikaK DominikaK force-pushed the master branch 3 times, most recently from 33f6b7e to 2b50cbd Compare February 2, 2023 14:26
@glye
Copy link
Contributor Author

glye commented Feb 22, 2023

It's necromancy time! 🦹‍♂️💀
@DominikaK, since ibexa/post-install#50 is merged, post-install now contains both apache and nginx vhosts in https://github.com/ibexa/post-install/tree/main/resources/templates and they are supposed to be maintained there.

So it makes sense to

  1. remove my nginx-vhost.template from this PR
  2. maybe also remove the existing apache vhost.template (or not, optional)
  3. update my changes to install_ibexa_dxp.md such that both the apache and nginx sections refer to the files in post-install

Agree?

@DominikaK
Copy link
Contributor

@glye: Yes, Almost yes and Yes :)

I'm all for removing the template file from this repo, but it's also referred to in https://github.com/ezsystems/developer-documentation/blob/master/docs/infrastructure_and_maintenance/environments.md and https://github.com/ezsystems/developer-documentation/blob/master/docs/infrastructure_and_maintenance/security/security_checklist.md. Can you update links in those two as well?

@glye
Copy link
Contributor Author

glye commented Feb 22, 2023

Bunch of changes:

  • Split install doc into Apache and nginx sections.
  • Standardised on the "nginx" all lower case spelling which is used above in the doc, and seems to be official.
  • Removed both Apache and nginx samples from doc, and refer both to those in post-install.
  • Clarify instructions, you don't have to hardcode doc root and app env.
  • Also pointed Environments and Security Checklist pages to post-install vhost.
  • Replaced ref to line number with a text string, so it doesn't break on changes.

@DominikaK
Copy link
Contributor

I've added one commit moving the two sections, Apache and nginx, into tabs for better viewing.

@glye
Copy link
Contributor Author

glye commented Feb 23, 2023

@DominikaK Right, that sounds good. 👍

@DominikaK DominikaK merged commit 538c938 into master Feb 23, 2023
@DominikaK DominikaK deleted the nginx-vhost-template branch February 23, 2023 08:42
DominikaK pushed a commit that referenced this pull request Feb 23, 2023
DominikaK pushed a commit that referenced this pull request Feb 23, 2023
DominikaK pushed a commit that referenced this pull request Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants