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.

Sign up for GitHub

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

Jump to bottom

Do not destroy SNIs when updating unrelated certificate fields #386

Merged
merged 1 commit into from
May 18, 2021
Merged

Do not destroy SNIs when updating unrelated certificate fields #386

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions diff/cert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,26 @@ func (sc *Syncer) createUpdateCertificate(

// found, check if update needed
if !currentCertificate.EqualWithOpts(certificateCopy, false, true) {
// Certificate and SNI objects have a special relationship. A PUT request
// (which we use for updates) with a certificate that contains no SNI
// children will in fact delete any existing SNI objects associated with
// that certificate, rather than leaving them as-is.

// To work around this issues, we set SNIs on certificates here using the
// current certificate's SNI list. If there are changes to the SNIs,
// subsequent actions on the SNI objects will handle those.
currentSNIs, err := sc.currentState.SNIs.GetAllByCertID(*currentCertificate.ID)
Copy link
Member

Choose a reason for hiding this comment

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

This is a fairly special case. Do you mind copying the git commit body in here as a code comment to help future maintainers?

if err != nil {
return nil, errors.Wrapf(err, "error looking up current certificate SNIs %v",
certificate.Identifier())
rainest marked this conversation as resolved.
Show resolved Hide resolved
}
sniNames := make([]*string, 0)
rainest marked this conversation as resolved.
Show resolved Hide resolved
for _, s := range currentSNIs {
sniNames = append(sniNames, s.Name)
}

certificateCopy.SNIs = sniNames
currentCertificate.SNIs = sniNames
return &Event{
Op: crud.Update,
Kind: "certificate",
Expand Down