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

Add support for TLS servers #1210

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Add support for TLS servers #1210

wants to merge 30 commits into from

Conversation

ok-john
Copy link

@ok-john ok-john commented Feb 5, 2023

This PR adds support for TLS servers by adding additional command line arguments for domain, cert and key which represent the url, path to certificate and key files respectively.

This PR also adds a build option make static which generates all the static assets required to compile the project. This needs to be run before first build and the only location this info exists is in your documentation under contributing.

I tested this on my own servers and it works.

Copy link
Owner

@sundowndev sundowndev left a comment

Choose a reason for hiding this comment

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

Thanks for your PR 👍🏻

I left few comments about the default paths and CLI options. Also could you add a few lines in the docs just to let the users know we support TLS ? See this file.

Comment on lines +48 to +52
func fmtLetsEncrypt(sitename string) (string, string) {
return fmt.Sprintf("/etc/letsencrypt/live/%s/fullchain.pem", sitename),
fmt.Sprintf("/etc/letsencrypt/live/%s/privkey.pem", sitename)
}

Copy link
Owner

@sundowndev sundowndev Feb 5, 2023

Choose a reason for hiding this comment

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

This function is not windows-compatible. Anyway I think it's not worth to guess the cert and key paths, if the user doesn't specify it, just don't use TLS.

Comment on lines +85 to +93
if len(opts.Domain) != 0 {
if len(opts.CertfilePath) == 0 || len(opts.KeyfilePath) == 0 {
opts.CertfilePath, opts.KeyfilePath = fmtLetsEncrypt(opts.Domain)
}
if err := srv.ListenAndServeTLS(opts.Domain+":443", opts.CertfilePath, opts.KeyfilePath); err != nil && err != http.ErrServerClosed {
log.Fatalf("listen: %s\n", err)
}
}

Copy link
Owner

@sundowndev sundowndev Feb 5, 2023

Choose a reason for hiding this comment

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

I just noticed we don't have a addr flag to listen to a different address. I think it's worth adding a new --addr flag and use it for both TLS and non-TLS.

  • --addr should be empty by default and we should use TLS when --cert or --key is not empty
  • We can use Port option to listen to port 443, instead of hard-coding it

In the Gin's server implementation, it's the same option for both methods.

@ok-john
Copy link
Author

ok-john commented Feb 5, 2023

Hey @sundowndev,

Thanks for the comments - at the moment my availability is a bit limited.

Next time I'm free I'll look at getting to those comments, if you or someone else would like to address those changes and push to this PR that's cool too.

Thanks.

richwrightnyc and others added 26 commits May 6, 2023 18:53
…ptional arguments, a path to manual download if the automatic checks fail, and fallbacks to curl if wget is not installed.
…f =~ to match as a regex rather than literally.
1. SC2155 (warning): Declare and assign separately to avoid masking return values.
2. SC2061 (warning): Quote the parameter to -name so the shell won't interpret it.
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 3.2.0 to 3.5.0.
- [Release notes](https://github.com/actions/setup-go/releases)
- [Commits](actions/setup-go@v3.2.0...v3.5.0)

---
updated-dependencies:
- dependency-name: actions/setup-go
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [goreleaser/goreleaser-action](https://github.com/goreleaser/goreleaser-action) from 4.1.1 to 4.2.0.
- [Release notes](https://github.com/goreleaser/goreleaser-action/releases)
- [Commits](goreleaser/goreleaser-action@v4.1.1...v4.2.0)

---
updated-dependencies:
- dependency-name: goreleaser/goreleaser-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [docker/login-action](https://github.com/docker/login-action) from 1.14.1 to 2.1.0.
- [Release notes](https://github.com/docker/login-action/releases)
- [Commits](docker/login-action@v1.14.1...v2.1.0)

---
updated-dependencies:
- dependency-name: docker/login-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4.3.1 to 4.5.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@v4.3.1...v4.5.0)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.0.0 to 3.3.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3.0.0...v3.3.0)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Move golangci-lint install command into install-tools task
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.3.0 to 3.4.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3.3.0...v3.4.0)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.4.0 to 3.5.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3.4.0...v3.5.0)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 3.5.0 to 4.0.0.
- [Release notes](https://github.com/actions/setup-go/releases)
- [Commits](actions/setup-go@v3.5.0...v4.0.0)

---
updated-dependencies:
- dependency-name: actions/setup-go
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.5.0 to 3.5.2.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3.5.0...v3.5.2)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
dependabot bot and others added 3 commits May 6, 2023 18:53
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4.5.0 to 4.6.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@v4.5.0...v4.6.0)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
@sundowndev
Copy link
Owner

It's been a year, any news on this @ok-john ?

@ok-john
Copy link
Author

ok-john commented May 22, 2024

It's been a year, any news on this @ok-john ?

I could work on this now i have time - is this still a requested feature?

@sundowndev
Copy link
Owner

@ok-john At this time it's not requested. If you'd like to request it, please create an issue first then create a PR. If you don't need this feature, then I think we can close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants