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

enh: subdomain addition + domain addition in app install #579

Merged
merged 6 commits into from
Sep 16, 2024

Conversation

Axolotle
Copy link
Member

@Axolotle Axolotle commented Sep 9, 2024

The problem

  • sometimes you just want to add a subdomain, being able to just enter the subpath with your already added domain is less error prone.
  • in AppInstall view you are sometimes required to add a specific subdomain for an app, having to return to DomainAdd to setup said new domain then run the diagnosis or force install a Let's Encrypt certificate is tedious.

Solution

  • add subdomain option to DomainForm with ability to auto install the Let's encrypt certificate if a wildcard is detected in DNS records
  • add ability to add a domain/subdomain directly in the AppInstall view in the domain field, as a modal, with auto select of the newly added domain.

PR Status

could'nt test the letsencrypt install properly (can't install certs on my setup idk why)

Requires: YunoHost/yunohost#1936
Fix: YunoHost/issues#1634

Preview

image
image

@selfhoster1312
Copy link

selfhoster1312 commented Sep 9, 2024

I don't understand why/how this is done on client side. In my opinion, it should be part of the app install route that you can specify a subdomain that does not exist that would be instantiated with a LetsEncrypt certificate. Whether this should be enabled by an additional flag/checkbox or be automatic i have no idea.

But doing it on the client side does not for example allow to do it from the CLI. Something like: yunohost app install -d my_sub.domain.tld my_app or yunohost app install --subdomain my_sub.domain.tld my_app or yunohost app install --new-subdomain -d my_sub.domain.tld.

Client side logic should only be for form validation and UX, and not for server configuration logic, otherwise review/maintenance becomes 10x more complex.

@alexAubin
Copy link
Member

Having an entire set of argument for domain creation during the app install doesn't seem right ... CLI / API routes should be about somewhat ~atomic operations and there already are routes about adding a new domain ... The point of this PR is to

  • a) somewhat formalize in terms of UI the procedure of "adding a subdomain" which currently ain't really clear (because people legitimately don't necessarily realize / understand that "adding a domain" can be used to add a subdomain),
  • b) automatically add the Lets Encrypt cert to reduce the number of clicks (because the current workflow also require to visit the diagnostic etc)
  • c) be able to add a domain from a quick modal that you can open from the app install form

Base automatically changed from vue3 to bookworm September 12, 2024 14:08
@Axolotle Axolotle marked this pull request as ready for review September 13, 2024 08:21
@alexAubin alexAubin merged commit 7a567ea into bookworm Sep 16, 2024
1 check passed
@alexAubin alexAubin deleted the enh-domain-add-subdomain branch September 16, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants