Support custom ACME provider #18340

LecrisUT · 2022-01-20T09:07:58Z

Closes #18306
Tested with self-hosted Smallstep CA.

A few points:

I think the options should be renamed to not be limited to Let'sEncrypt
The function could be simplified to not pass domain, directory, etc. since other options are derived from settings inside anyway

Let me know of any naming conventions that I should adhere to here.

Signed-off-by: Cristian Le git@lecris.me

wxiaoguang

The documents and app.example.ini need to be updated.

modules/setting/setting.go

cmd/web_letsencrypt.go

silverwind · 2022-01-20T09:44:22Z

We should refactor all references to LetsEncrypt to ACME, e.g. rename web_letsencrypt.go to web_acme.go.

LecrisUT · 2022-01-21T05:43:06Z

Please advise if 6344684 is ok. The documentation for LetsEncrypt settings was not present in app.example.ini, I've added it to the current names, and deprecated the old configuration names.

~~For web doc documentations, where can I find that?~~ Nvm, found them and updated the documentation.

Closes go-gitea#18306 Signed-off-by: Cristian Le <git@lecris.me>

…d documented app.example.ini Signed-off-by: Cristian Le <git@lecris.me>

Signed-off-by: Cristian Le <git@lecris.me>

… not implemented yet. Signed-off-by: Cristian Le <git@lecris.me>

codecov-commenter · 2022-01-21T08:14:10Z

Codecov Report

Merging #18340 (d21a5f3) into main (9712f7d) will increase coverage by 0.32%.
The diff coverage is 21.76%.

@@            Coverage Diff             @@
##             main   #18340      +/-   ##
==========================================
+ Coverage   46.31%   46.63%   +0.32%     
==========================================
  Files         846      846              
  Lines      121244   121313      +69     
==========================================
+ Hits        56155    56577     +422     
+ Misses      58271    57862     -409     
- Partials     6818     6874      +56

Impacted Files	Coverage Δ
cmd/web.go	`0.00% <0.00%> (ø)`
cmd/web_acme.go	`0.00% <0.00%> (ø)`
models/migrate.go	`40.00% <ø> (+0.73%)`	⬆️
models/unittest/testdb.go	`15.87% <0.00%> (-0.26%)`	⬇️
modules/context/context.go	`64.48% <ø> (-0.39%)`	⬇️
routers/install/install.go	`0.67% <0.00%> (+0.01%)`	⬆️
routers/web/repo/blame.go	`0.00% <0.00%> (ø)`
routers/web/repo/issue_content_history.go	`0.00% <0.00%> (ø)`
services/forms/repo_form.go	`42.10% <ø> (ø)`
modules/setting/setting.go	`46.11% <2.04%> (-2.44%)`	⬇️
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a60e8be...d21a5f3. Read the comment docs.

cmd/web_acme.go

…ed the previous change Signed-off-by: Cristian Le <git@lecris.me>

custom/conf/app.example.ini

Gusted

Just some code styling.

cmd/web_acme.go

custom/conf/app.example.ini

modules/setting/setting.go

Co-authored-by: Gusted <williamzijl7@hotmail.com>

cmd/web_acme.go

Co-authored-by: Gusted <williamzijl7@hotmail.com>

LecrisUT · 2022-01-24T16:55:04Z

Did you guys just pushed commits from github interface? Got woken up by a barage of notifications. Anyway there's still documentation to be fixed around the ACME_TOS and implement the required code upstream.

wxiaoguang · 2022-01-25T02:21:38Z

Did you guys just pushed commits from github interface? Got woken up by a barage of notifications. Anyway there's still documentation to be fixed around the ACME_TOS and implement the required code upstream.

Sorry to wake you up .... I just committed some small suggested changes from GitHub web. I would leave it next time.

LecrisUT · 2022-01-25T03:25:17Z

Sorry to wake you up .... I just committed some small suggested changes from GitHub web. I would leave it next time.

No sweat, just condense them into a single commit next time.

Signed-off-by: Cristian Le <git@lecris.me>

LecrisUT · 2022-01-31T03:16:03Z

I have a stashed changes waiting on caddyserver/certmagic#165. The difference is that, the user does not need to explicitly add any settings of ACME_ACCEPTTOS, instead the user will be prompted at the account creation with the relevant TOS and accept/not accept. Then gitea will use the cached acceptance over the ACME_ACCEPTTOS settings. (We can also cache the TOS link so that when it is changed, the user should confirm once again)

Signed-off-by: Cristian Le <git@lecris.me>

custom/conf/app.example.ini

Signed-off-by: Cristian Le <git@lecris.me>

wxiaoguang · 2022-02-08T03:07:43Z

No more changes? Prepare to merge ~~~

LecrisUT · 2022-02-08T03:23:56Z

Sure, when the changes to certmagic are merged, I will create a new PR.

LecrisUT · 2022-02-08T04:34:33Z

Uhm, checking back on the drone build, it is stuck in some infinite loop. @wxiaoguang how about triggering it again?

wxiaoguang · 2022-02-08T05:07:32Z

No worry, maintainers can handle it

* giteaofficial/main: (28 commits) Added auto-save whitespace behavior if it changed manually (go-gitea#15566) Support custom ACME provider (go-gitea#18340) Refactor i18n, use Locale to provide i18n/translation related functions (go-gitea#18648) Only request write when necessary (go-gitea#18657) [skip ci] Updated translations via Crowdin Add separate SSH_USER config option (go-gitea#17584) Be more lenient with label colors (go-gitea#17752) remove redundant call to UpdateRepoStats during migration (go-gitea#18591) more repo dump/restore tests, including pull requests (go-gitea#18621) No longer show the db-downgrade SQL in production (go-gitea#18653) Fix the missing i18n key for update checker (go-gitea#18646) Update gitea-vet (go-gitea#18640) Future proof for 1.18 (go-gitea#18644) Add `contrib/upgrade.sh` (go-gitea#18286) If rendering has failed due to a net.OpError stop rendering (go-gitea#18642) Delete old git.NewCommand() and use it as git.NewCommandContext() (go-gitea#18552) Update JS dependencies (go-gitea#18636) fix commits_list_small.tmpl (go-gitea#18641) Fix `make fmt` and `make fmt-check` (go-gitea#18633) Frontport of changelog for v1.16.1 (go-gitea#18615) ...

* Added ACMECAURL option to support custom ACME provider. Closes go-gitea#18306 * Refactor setting.go https settings, renamed options and variables, and documented app.example.ini * Refactored runLetsEncrypt to runACME * Improved documentation

wxiaoguang requested changes Jan 20, 2022

View reviewed changes

modules/setting/setting.go Outdated Show resolved Hide resolved

cmd/web_letsencrypt.go Outdated Show resolved Hide resolved

cmd/web_letsencrypt.go Outdated Show resolved Hide resolved

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 20, 2022

LecrisUT added 3 commits January 21, 2022 15:16

Added ACMECAURL option to support custom ACME provider

eb2dc85

Closes go-gitea#18306 Signed-off-by: Cristian Le <git@lecris.me>

Refactor setting.go https settings, renamed options and variables, an…

11f0ef8

…d documented app.example.ini Signed-off-by: Cristian Le <git@lecris.me>

Refactored runLetsEncrypt to runACME

4350b76

Signed-off-by: Cristian Le <git@lecris.me>

LecrisUT force-pushed the acme-provider branch from 6344684 to 4350b76 Compare January 21, 2022 06:17

Updated the documentation and switched off TLS ALPN challenge as it's…

21a57c7

… not implemented yet. Signed-off-by: Cristian Le <git@lecris.me>

LecrisUT mentioned this pull request Jan 21, 2022

Custom ACME Provider Support #18306

Closed

wxiaoguang reviewed Jan 21, 2022

View reviewed changes

cmd/web_acme.go Outdated Show resolved Hide resolved

wxiaoguang reviewed Jan 21, 2022

View reviewed changes

cmd/web_acme.go Show resolved Hide resolved

silverwind added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Jan 21, 2022

TLS ALPN challenge is actually used, but it is not documented. Revert…

cc43ff6

…ed the previous change Signed-off-by: Cristian Le <git@lecris.me>

wxiaoguang approved these changes Jan 21, 2022

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 21, 2022

Merge branch 'main' into acme-provider

133db1d

lunny added this to the 1.17.0 milestone Jan 22, 2022

silverwind reviewed Jan 22, 2022

View reviewed changes

custom/conf/app.example.ini Outdated Show resolved Hide resolved

Gusted reviewed Jan 23, 2022

View reviewed changes

LecrisUT mentioned this pull request Jan 23, 2022

Check TOS caddyserver/certmagic#164

Open

silverwind added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Jan 24, 2022

wxiaoguang and others added 6 commits January 24, 2022 23:24

Update modules/setting/setting.go

8045ff5

Co-authored-by: Gusted <williamzijl7@hotmail.com>

Update modules/setting/setting.go

816fe98

Co-authored-by: Gusted <williamzijl7@hotmail.com>

Update cmd/web_acme.go

ba01a22

Co-authored-by: Gusted <williamzijl7@hotmail.com>

Update cmd/web_acme.go

c2f0a1d

Co-authored-by: Gusted <williamzijl7@hotmail.com>

Update custom/conf/app.example.ini

321fa92

Co-authored-by: Gusted <williamzijl7@hotmail.com>

Update custom/conf/app.example.ini

31ed266

Co-authored-by: Gusted <williamzijl7@hotmail.com>

Gusted reviewed Jan 24, 2022

View reviewed changes

cmd/web_acme.go Outdated Show resolved Hide resolved

Update cmd/web_acme.go

572c88a

Co-authored-by: Gusted <williamzijl7@hotmail.com>

Changed setting to ACME_ACCEPTTOS and improved CA root reading

f96f206

Signed-off-by: Cristian Le <git@lecris.me>

LecrisUT force-pushed the acme-provider branch from 1015b9c to f96f206 Compare January 31, 2022 03:06

Improved documentation

cd58378

Signed-off-by: Cristian Le <git@lecris.me>

silverwind reviewed Feb 1, 2022

View reviewed changes

custom/conf/app.example.ini Outdated Show resolved Hide resolved

Removed TODO note

7ea21e9

Signed-off-by: Cristian Le <git@lecris.me>

silverwind approved these changes Feb 2, 2022

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 2, 2022

zeripath and others added 2 commits February 3, 2022 20:43

Merge branch 'main' into acme-provider

1ec10b8

Merge branch 'main' into acme-provider

d21a5f3

wxiaoguang merged commit 60f2033 into go-gitea:main Feb 8, 2022

go-gitea locked and limited conversation to collaborators Apr 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support custom ACME provider #18340

Support custom ACME provider #18340

LecrisUT commented Jan 20, 2022

wxiaoguang left a comment

silverwind commented Jan 20, 2022

LecrisUT commented Jan 21, 2022 •

edited

Loading

codecov-commenter commented Jan 21, 2022 •

edited

Loading

Gusted left a comment

LecrisUT commented Jan 24, 2022

wxiaoguang commented Jan 25, 2022

LecrisUT commented Jan 25, 2022

LecrisUT commented Jan 31, 2022

wxiaoguang commented Feb 8, 2022

LecrisUT commented Feb 8, 2022

LecrisUT commented Feb 8, 2022

wxiaoguang commented Feb 8, 2022

Support custom ACME provider #18340

Support custom ACME provider #18340

Conversation

LecrisUT commented Jan 20, 2022

wxiaoguang left a comment

Choose a reason for hiding this comment

silverwind commented Jan 20, 2022

LecrisUT commented Jan 21, 2022 • edited Loading

codecov-commenter commented Jan 21, 2022 • edited Loading

Codecov Report

Gusted left a comment

Choose a reason for hiding this comment

LecrisUT commented Jan 24, 2022

wxiaoguang commented Jan 25, 2022

LecrisUT commented Jan 25, 2022

LecrisUT commented Jan 31, 2022

wxiaoguang commented Feb 8, 2022

LecrisUT commented Feb 8, 2022

LecrisUT commented Feb 8, 2022

wxiaoguang commented Feb 8, 2022

LecrisUT commented Jan 21, 2022 •

edited

Loading

codecov-commenter commented Jan 21, 2022 •

edited

Loading