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

Support scheme definition for GitLab remotes #2392

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bauglir
Copy link
Contributor

@bauglir bauglir commented Jan 6, 2024

GitLab is fairly easy to self-host. Although best practice, this also means that the scheme over which GitLab is served is not guaranteed to be https. For this reason, the provided host should be allowed to define what scheme the GitLab instance is being hosted under and respect that in generated links.

The same applies to ports and potentially paths, but those are already supported given the current mechanism (provided no trailing slash is included in the host).

One benefit of this approach is that it also allows a full "base" URL to be provided as the host, e.g. the CI_SERVER_URL predefined variable within GitLab CI environments1.

According to RFC39862, URI schemes should be matched case-insensitively, which this approach does. It also prescribes that only lowercase schemes should be produced, which this chooses to not do to keep the implementation simpler. If this is something that should be changed, I do have the implementation ready to go locally and can easily amend this PR.

Footnotes

  1. https://docs.gitlab.com/ee/ci/variables/predefined_variables.html

  2. https://datatracker.ietf.org/doc/html/rfc3986#section-3.1

GitLab is fairly easy to self-host. Although best practice, this also
means that the scheme over which GitLab is served is not guaranteed to
be `https`. For this reason, the provided `host` should be allowed to
define what scheme the GitLab instance is being hosted under and respect
that in generated links.

The same applies to ports and potentially paths, but those are already
supported given the current mechanism (provided no trailing slash is
included in the `host`).

One benefit of this approach is that it also allows a full "base" URL to
be provided as the host, e.g. the `CI_SERVER_URL` predefined variable
within GitLab CI environments[^1].

According to RFC3986[^2], URI schemes should be matched case
insensitively, which this approach does. It also prescribes that only
lowercase schemes should be produced, which this chooses to not do to
keep the implementation simpler.

[^1]: https://docs.gitlab.com/ee/ci/variables/predefined_variables.html
[^2]: https://datatracker.ietf.org/doc/html/rfc3986#section-3.1
@mortenpi
Copy link
Member

What do you think about doing this via an optional kwarg to GitLab (e.g. GitLab(host, user, repo; scheme="https")) instead? Feels a bit simpler than trying to parse the URL?

Although, I am also wondering, under what circumstances will you not have your self-hosted GitLab instance behind TLS, while still hosting docs?

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.

2 participants