Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

certificates: Adding Certificate Manager #67

Merged
merged 1 commit into from
Jan 25, 2020

Conversation

draychev
Copy link
Contributor

This PR proposes a certificate.Manager, which will be additionally discussed in the Design Document here #51

This implementation of a Certificate Manager relies on hard-coded PEMs on disk.

At a later stage the implementation will change.

}

func newCertificate(cn CommonName) (*Certificate, error) {
glog.V(7).Infof("[certificate] Creating a certificate for CN=%s", cn)
Copy link
Member

Choose a reason for hiding this comment

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

nit: replace 7 with a constant, the constants for the log levels can be in a separate file/pkg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shashankram That's a good idea - I'll make a github issue for that for us to follow up in a separate PR - just so we keep the scope here narrow to the cert manager. (#69)

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good.

return c.privateKey
}

func newCertificate(cn CommonName) (*Certificate, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't 'n' of "newCertificate" capitalized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question... because there was no need to use newCertificate outside of the package - it is not exported.
But if we find a need to use it outside somewhere - we could. The newCertificate is called by the CertificateManager (factory) -- so once you do NewCertificateManager then you call IssueCertifiacate on it... which ends up calling newCertificate

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see. I am a bit spoiled by other languages where all functions names are styled similarly.

return c.privateKey
}

func newCertificate(cn CommonName) (*Certificate, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Ah I see. I am a bit spoiled by other languages where all functions names are styled similarly.

}

func newCertificate(cn CommonName) (*Certificate, error) {
glog.V(7).Infof("[certificate] Creating a certificate for CN=%s", cn)
Copy link
Member

Choose a reason for hiding this comment

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

Sounds good.

@draychev draychev merged commit 3683c79 into master Jan 25, 2020
@draychev draychev deleted the draychev/certificate-manager branch January 25, 2020 00:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants