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

setup: Allow renewing certificates during the warning period #486

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

mz-pdm
Copy link
Member

@mz-pdm mz-pdm commented Jun 22, 2022

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

I think we should allow to renew certificates from the same date as we start to raise warnings about their expiration:

https://github.com/oVirt/ovirt-engine/blob/master/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CertificationValidityChecker.java#L120

So we should use the CertExpirationWarnPeriodInDays also in engine-setup. We have na existing infra to get vdc_options' vaue within engine_setup:

https://github.com/oVirt/ovirt-engine/blob/master/packaging/setup/ovirt_engine_setup/engine/vdcoption.py#L63
https://github.com/oVirt/ovirt-engine/blob/master/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/aaa.py#L193

So we should use this option also in https://github.com/oVirt/ovirt-engine/blob/master/packaging/setup/ovirt_engine_setup/engine_common/pki_utils.py#L65 for longer valid certificates instead of hardcoded value.

@mwperina mwperina requested a review from dangel101 June 23, 2022 07:39
The default value of CertExpirationWarnPeriodInDays, which applies to
CA, Engine and host certificates, is 365.  However engine-setup
offers (and allows) renewing the CA and Engine certificates only 60
days or less before their expiration.  This results in warnings about
certificate expiration presented to users, while the users cannot
actually renew the certificates for more than 300 days.

Let’s allow renewing CA and Engine certificate during the warning
period or, if it cannot be obtained, up to 365 days before their
expiration.

Bug-Url: https://bugzilla.redhat.com/2093954
Bug-Url: https://bugzilla.redhat.com/2096862
Bug-Url: https://bugzilla.redhat.com/2097725
@mz-pdm mz-pdm changed the title setup: Allow renewing certificates 365 days in advance setup: Allow renewing certificates during the warning period Jun 23, 2022
@mz-pdm
Copy link
Member Author

mz-pdm commented Jun 23, 2022

Thank you for the pointers to config values retrieval. CertExpirationWarnPeriodInDays is used now.

It seems to be working, but it's difficult to check all the different scenarios so please review carefully.

@michalskrivanek
Copy link
Member

lgtm

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

Looks good to me

@mz-pdm
Copy link
Member Author

mz-pdm commented Jun 28, 2022

ping

@michalskrivanek michalskrivanek merged commit 1cf709d into oVirt:master Jun 28, 2022
@Klaas-
Copy link

Klaas- commented Jun 28, 2022

should we also change the default to something like half a year instead of the 365 days? Otherwise it will start warning 33 days after creating the engine cert

@Klaas-
Copy link

Klaas- commented Jun 28, 2022

also thinking about this: maybe we should set the warn period to half a year , but renew at warn period * 1.5 so that it gets renewed before it starts warning if you regularly update ovirt

@mz-pdm
Copy link
Member Author

mz-pdm commented Jun 28, 2022

should we also change the default to something like half a year instead of the 365 days? Otherwise it will start warning 33 days after creating the engine cert

No, the Engine certificate (not the web one) has lifetime of 5 years now.

@Klaas-
Copy link

Klaas- commented Jun 28, 2022

ah okay, so the engine cert will go back to a longterm cert next time I run engine-setup after this patch has merged :) That's good -- thanks for the info

@didib
Copy link
Member

didib commented Jun 28, 2022

Sorry for commenting only now, after it's merged.

I think this will fail - meaning, fallback to 365 days and info - with dwh/grafana on a separate machine.

@michalskrivanek
Copy link
Member

I think it's fine. getting the config is basically a nice to have. - if it gracefully falls back.

@mz-pdm mz-pdm deleted the cert-renewal# branch August 9, 2022 08:07
@didib
Copy link
Member

didib commented Aug 23, 2022

Sorry for commenting only now, after it's merged.

I think this will fail - meaning, fallback to 365 days and info - with dwh/grafana on a separate machine.

It did :-( :

https://lists.ovirt.org/archives/list/users@ovirt.org/thread/NBJC4EVZDJUUWLLMLYFXM7TJFU4PZEYO/#NBJC4EVZDJUUWLLMLYFXM7TJFU4PZEYO

Fix is in oVirt/ovirt-dwh#48 , but does not work (didn't test, though) if grafana is on its own separate machine.

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.

None yet

5 participants