Disroot-Ansible/nginx
11
1
Fork 0
Code Pull requests Projects Releases 1 Activity

1.0 - Improved vhost creation and headers #68

Merged
muppeth merged 11 commits from staging into main 2024-06-03 23:21:55 +02:00
Conversation 0 Commits 11 Files changed 7 +163 -119
muppeth commented 2024-05-28 00:40:08 +02:00
Owner
Copy link

Ton of changes. Mainly:

  • Improved vhost creation - vhosts are not created for https when no certificate present
  • Updated nextcloud template
  • Updated akkoma template
  • Improved header section for core and proxy templates
Ton of changes. Mainly: - Improved vhost creation - vhosts are not created for https when no certificate present - Updated nextcloud template - Updated akkoma template - Improved header section for core and proxy templates
meaz was assigned by muppeth 2024-05-28 00:40:08 +02:00
muppeth added 8 commits 2024-05-28 00:40:09 +02:00 
Changed the way vhosts are created. This is to prevent situation where https vhosts are created without corresponding certificate which is  causing error. Solution to that is to check if ssl cert exists for vhost before creating them.

Suggested approach is to create vhost called '01.letsencrypt' or `01.domain.ltd` using `letsencrypt` template. This will allow new certificates to be created for upcoming vhosts and once certs are  created, nginx will be able to create vhosts and not error out.  (so first run letsencrypt and then nginx).

Currently vhost creation and enabling is done separate for HTTP and HTTPS vhosts. Not the best solution, but works for now.

Reviewed-on: #63
Reviewed-by: meaz <meaz@no-reply@disroot.org>
Co-authored-by: muppeth <muppeth@disroot.org>
Co-committed-by: muppeth <muppeth@disroot.org>
Reviewed-on: #62
Reviewed-by: muppeth <muppeth@no-reply@disroot.org>
Co-authored-by: meaz <meaz@disroot.org>
Co-committed-by: meaz <meaz@disroot.org>
Reviewed-on: #66
changed the cgheck for certificate existance to item.ssl_name instead of item.name. This is cover situation where name of the certificate is different then then name of the vhost (eg. wildcard cert). Additionally added `item.name` to name of the tasks for things like vhost create to make sure we see which vhost is created (better visability)

Reviewed-on: #67
Reviewed-by: meaz <meaz@no-reply@disroot.org>
Co-authored-by: muppeth <muppeth@disroot.org>
Co-committed-by: muppeth <muppeth@disroot.org>
floss4good reviewed 2024-05-28 09:51:13 +02:00
tasks/vhost.yml Outdated
@ -44,0 +87,4 @@
when:
- item.state is defined
- item.state == 'disable'
- item.state == 'delete'
floss4good commented 2024-05-28 09:51:12 +02:00
First-time contributor
Copy link

Isn't this equal to ... and item.state == 'disable' and item.state == 'delete' (that can never be true)?

Isn't this equal to `... and item.state == 'disable' and item.state == 'delete'` (that can never be true)?
meaz commented 2024-05-28 20:18:04 +02:00
Owner
Copy link

yes, but I guess that this is for readability :)

yes, but I guess that this is for readability :)
muppeth commented 2024-05-28 21:16:03 +02:00
Author
Owner
Copy link

Yes. At some point when working on stuff I thought of splitting it per line to avoid having long ass lines. That said I will change the 'version' to below 1.0. I think we could "release" 1.0 when we have a proper documentation in place and we review the entire role and steamline/cleanup as much as we can as things got a bit complex in here.

Yes. At some point when working on stuff I thought of splitting it per line to avoid having long ass lines. That said I will change the 'version' to below 1.0. I think we could "release" 1.0 when we have a proper documentation in place and we review the entire role and steamline/cleanup as much as we can as things got a bit complex in here.
floss4good commented 2024-05-29 10:20:34 +02:00
First-time contributor
Copy link

For the other cases - where multiple conditions linked by logical and were rearranged as a list - it was clear that it was a style change, for readability.

However, my question was strictly for this case, in which the initial chain of conditions had a logical or operator, and by switching from a single-line set of conditions to a list (one per line) you are actually changing the logic (since now all the conditions must be true).
I did considered a possible deliberate change (i.e. the original when statement was wrong and you fixed it) but it does not make sense, since item.state cannot be 'disable' and 'delete' simultaneously.

For the other cases - where multiple conditions linked by logical **and** were rearranged as a list - it was clear that it was a _style_ change, for readability. However, my question was strictly for this case, in which the initial chain of conditions had a logical **or** operator, and by switching from a single-line set of conditions to a list (one per line) you are actually changing the logic (since now all the conditions must be true). I did considered a possible deliberate change (i.e. the original `when` statement was wrong and you fixed it) but it does not make sense, since `item.state` cannot be `'disable'` **and** `'delete'` simultaneously.
muppeth commented 2024-05-29 22:20:46 +02:00
Author
Owner
Copy link

You are totally right. I did not notice you pointed to specific task at first and I missed the fact that indeed there was an OR condition there. Thanks again for find this stuff correcting our sloppy work.

You are totally right. I did not notice you pointed to specific task at first and I missed the fact that indeed there was an **OR** condition there. Thanks again for find this stuff correcting our sloppy work.
muppeth marked this conversation as resolved
meaz approved these changes 2024-05-28 20:17:13 +02:00
muppeth added 1 commit 2024-05-29 22:18:31 +02:00
muppeth added 1 commit 2024-05-29 22:46:41 +02:00
muppeth added 1 commit 2024-06-03 23:20:15 +02:00
muppeth merged commit 0dfa370f56 into main 2024-06-03 23:21:55 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
meaz
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
meaz
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Disroot-Ansible/nginx#68
No description provided.
Delete branch "staging"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?