Skip to content

Update pillar.example to good-citizen defaults#63

Closed
jinnatar wants to merge 2 commits intosaltstack-formulas:masterfrom
jinnatar:patch-1
Closed

Update pillar.example to good-citizen defaults#63
jinnatar wants to merge 2 commits intosaltstack-formulas:masterfrom
jinnatar:patch-1

Conversation

@jinnatar
Copy link

@jinnatar jinnatar commented Feb 3, 2019

Specifying renew-by-default in the config, regardless of True / False enables the flag. This causes certbot to force-renew on every timer run and means you will hit the rate-limit fairly quickly. A safer default is keep-until-expiring where the renew will only be done when it's actually advisable.

expand allows an expanded domain list to renew correctly without forking a new domainset. This is something that most users will want.

@jinnatar
Copy link
Author

jinnatar commented Feb 3, 2019

This will fix #58 and mitigate #57

@javierbertoli
Copy link
Member

@Artanicus , changing these values in pillar.example won't mitigate the issues you mention, as pillar.example is just that, an example.

Adding these values to defaults.yaml will help (and I'd say adding them there would be a good thing), but #57 won't be fixed, that will require an extra PR.

Mind you add these values in defaults.yaml too, keeping also the old values in pillar.example?

Copy link

@aanriot aanriot left a comment

Choose a reason for hiding this comment

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

I agree with @javierbertoli . Thanks for you work @Artanicus .

Copy link

@aboe76 aboe76 left a comment

Choose a reason for hiding this comment

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

I agree with @javierbertoli

Copy link
Member

@alxwr alxwr left a comment

Choose a reason for hiding this comment

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

I agree with @javierbertoli . Thanks for you work @Artanicus .

@pull-assistant
Copy link

pull-assistant bot commented May 26, 2020

Score: 1.00

Best reviewed: commit by commit


Optimal code review plan

     docs(pillar): update pillar.example to good-citizen defaults

     feat(config): add sane defaults

Powered by Pull Assistant. Last update 6354f7c ... e822af5. Read the comment docs.

jinnatar and others added 2 commits May 26, 2020 09:52
Specifying `renew-by-default` in the config, regardless of True / False enables the flag. This causes certbot to force-renew on every timer run and means you will hit the rate-limit fairly quickly. A safer default is `keep-until-expiring` where the renew will only be done when it's actually advisable.

`expand` allows an expanded domain list to renew correctly without forking a new domainset. This is something that most users will want.
Also specify `config` as a hash to make it easier to provide/compose the config from
multiple pillar files.  Maintain backward compatibility (supplying `config` as
a string) so that the formula continues to work as expected if a `config`
string is already available.

BREAKING CHANGE: `config` can now be provided as a hash or a string; defaults
are modified and, while sane and desirable, do change the behavior of the
formula.
@javierbertoli javierbertoli linked an issue May 26, 2020 that may be closed by this pull request
@javierbertoli javierbertoli mentioned this pull request Jul 23, 2020
19 tasks
@javierbertoli
Copy link
Member

As this PR got stale, I updated and merged it on #73.

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.

Dangerous force-renew in example pillar

5 participants