getmeli / meli

Platform for deploying static sites and frontend applications easily. Automatic SSL, deploy previews, reverse proxy, and more.
Other
2.41k stars 97 forks source link

Meli does not clip long branch names for subdomain #234

Open Runeii opened 3 years ago

Runeii commented 3 years ago

I've a project named client-outlet-catalog and deployed a branch named dsr-5936-correct-broken-links-and-overlapping-elements. It's not a pretty name, but it's to match internal processes where branches should follow ticket titles.

This branch is 54 characters long, therefore it doesn't hit the 64 character limit allowed for subdomains (as specified in RFC 5280).

However, it looks like Lets Encrypt can have issues with subdomains over 53 characters long: https://community.letsencrypt.org/t/ssl-for-a-63-character-max-number-of-characters-domain-name-s/36387/17

I can't see an obvious fix in that Lets Encrypt thread, so I would suggest maybe clipping branch names at 53 characters long to avoid this issue?

Obviously, in the interim, it's no real challenge to just not use super long branch names. But it would be good to automatically handle this issue (fixing it requires deleting the branch/PR then pushing a new one to remote).

pimartin commented 3 years ago

We should actually cap the entire domain name size. Which would mean adding constraints on both the project name and the branch name, based on the URL Meli would generate for them. An automatic cropping of the branch name could be added for the API (used by the CLI). Although it might be best to throw an error and make the user crop the string himself, to avoid surprises.

In your case, I believe $(cut -d "-" -f1-2 <<<"dsr-5936-correct-broken-links-and-overlapping-elements") would generate good branch names for Meli (it would output dsr-5936, the ticket reference). You can specify that as the --branch parameter of the upload command of the CLI.

Runeii commented 3 years ago

Thanks, that's useful to know. In the end we adjusted the branch name to be under the character limit, the ticket number alone would be a little vague to the actual dev when scanning the output ofgit branch in a week or two!

I would agree this should be something Meli handles as part of its API. While clipping the branch name may be a "surprise" in the sense that it results in a different subdomain name than expected, the current behaviour feels like much more like a nasty surprise for developers. While they are able to visit the subdomain in this example – because it is short enough to be a valid URL – it being too long for Lets Encrypt means they get an SSL error screen with no useful feedback. Without being aware there is a maximum character length for LE, this would be a very hard bug to debug, while the fix (log in to Meli, delete existing branch, change branch, repush) is a bit convoluted.

But also not going to demand a feature from an OS project! For the time being happy to just keep an eye on branch length.