Page MenuHomeFreeBSD

security/py-certbot: Improve periodic script
ClosedPublic

Authored by dbaio on Jun 21 2020, 5:59 PM.

Details

Summary
security/py-certbot: Improve periodic script

This change will keep the default behavior in the periodic script
and will add options to customize each parameter for those who want to:
 - weekly_certbot_pre_hook
 - weekly_certbot_post_hook
 - weekly_certbot_deploy_hook
 - weekly_certbot_custom_args

Approved by:    ???
PR:   245674, 245954
Test Plan

poudriere ok (11, 12, CURRENT; i386, amd64)

Periodic script was tested with a mix of these configs:

$ cat /etc/periodic.conf
weekly_certbot_enable="YES"
#weekly_certbot_service="nginx"
#weekly_certbot_pre_hook="service nginx onestatus"
#weekly_certbot_post_hook="service nginx onestatus"
#weekly_certbot_deploy_hook="/tmp/a.sh"
#weekly_certbot_custom_args="--force-renewal"

#Enable logging and disable temporarily other weekly scripts
anticongestion_sleeptime=1
weekly_show_badconfig="YES"
weekly_status_security_enable="NO"
weekly_locate_enable="NO"
weekly_whatis_enable="NO"
weekly_output=/var/log/weekly.log

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dbaio requested review of this revision.Jun 21 2020, 5:59 PM

fix the comments alignment

Thanks for this!

Missing explicit QA confirmations in TEST PLAN section and please add Approved by: ??? and MFH: <branch|No> <reason> to Summary commit log message

security/py-certbot/Makefile
38 ↗(On Diff #73441)

This is interesting in that its an opportunity to have 'concurrent' handle these

Might try adding this file to SUFFIX_FILES and seeing if that does the same as was done here

dbaio added inline comments.
security/py-certbot/Makefile
38 ↗(On Diff #73441)

with UNIQUE_SUFFIX_FILES, it will end having two scripts in the periodic folder,

500.certbot -> 500.certbot-3.7 (symlink)
500.certbot-3.7

We don't want that.

Thanks for this!

Missing explicit QA confirmations in TEST PLAN section and please add Approved by: ??? and MFH: <branch|No> <reason> to Summary commit log message

-> Approved by: ??? and MFH: <branch|No> <reason> to Summary commit log message (*i* would merge: missing startup script)

security/py-certbot/Makefile
38 ↗(On Diff #73441)

Roger that

koobs requested changes to this revision.Jun 24 2020, 4:24 AM
This revision now requires changes to proceed.Jun 24 2020, 4:24 AM
dbaio marked an inline comment as done.

Thanks for this!

Missing explicit QA confirmations in TEST PLAN section and please add Approved by: ??? and MFH: <branch|No> <reason> to Summary commit log message

-> Approved by: ??? and MFH: <branch|No> <reason> to Summary commit log message (*i* would merge: missing startup script)

It's not missing startup script, we are adding more config options to the periodic script.

IMHO we can let as it is on 2020Q2 and work to commit this before 2020Q3.

I plan to add man pages before 2020Q3 as well (PR 235701).

dbaio retitled this revision from security/py-certbot: Improve periodic script, Fix Concurrency to security/py-certbot: Improve periodic script.
dbaio edited the summary of this revision. (Show Details)

Hi @koobs , about the MFH, I don't mind in MFH it to 2020Q2 branch, but soon we will have 2020Q3.
Do you have other concerns here?

And thanks for taking a look.

koobs added a subscriber: kevans.

Hi @koobs , about the MFH, I don't mind in MFH it to 2020Q2 branch, but soon we will have 2020Q3.
Do you have other concerns here?

And thanks for taking a look.

A committer may MFH any non feature update to head at any time, so dont worry about branch cut points to target commit timing

@kevans Can you assist with RC semantics review?

This revision is now accepted and ready to land.Jul 3 2020, 10:57 AM
This revision was automatically updated to reflect the committed changes.