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

CP-50193: Update new fingerprint fields on DB upgrade #5786

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

snwoods
Copy link
Contributor

@snwoods snwoods commented Jul 5, 2024

The new fingerprint_sha256 and fingerprint_sha1 fields will be empty when upgrading from a version without the fields. This commit checks for this and fills them in, stopping the certificate from being needlessly reinstalled.

@snwoods snwoods force-pushed the private/stevenwo/CP-50193 branch from 274ab83 to b103a8c Compare July 8, 2024 22:11
ocaml/xapi/certificates.ml Outdated Show resolved Hide resolved
ocaml/xapi/certificates_sync.ml Outdated Show resolved Hide resolved
ocaml/xapi/certificates_sync.ml Show resolved Hide resolved
ocaml/xapi/certificates_sync.ml Outdated Show resolved Hide resolved
ocaml/xapi/certificates_sync.ml Outdated Show resolved Hide resolved
Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

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

Agree with comments from @psafont

@snwoods snwoods force-pushed the private/stevenwo/CP-50193 branch from b103a8c to 8df3ef6 Compare July 9, 2024 15:04
The new fingerprint_sha256 and fingerprint_sha1 fields will be empty
when upgrading from a version without the fields. This commit checks for
this and fills them in, stopping the certificate from being needlessly
reinstalled.

Signed-off-by: Steven Woods <steven.woods@citrix.com>
@snwoods snwoods force-pushed the private/stevenwo/CP-50193 branch from 8df3ef6 to 826e6d0 Compare July 9, 2024 15:12
Copy link
Member

@psafont psafont left a comment

Choose a reason for hiding this comment

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

This looks good, have you tested this on a host?

@snwoods
Copy link
Contributor Author

snwoods commented Jul 10, 2024

This looks good, have you tested this on a host?

Yep, tested on both an rt-next pool and a pool which had the sha1 changes. In both occasions, the logs showed that the certificate was not reinstalled and in the former case it showed the new fingerprint fields were populated. It correctly sent both sha256 and sha1 when requested on HOST_IS_SLAVE error.

@lindig lindig merged commit d8fa301 into xapi-project:master Jul 10, 2024
15 checks passed
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.

4 participants