Add self signed Issuer type #637
Conversation
jetstack-bot
added
the
release-note
jetstack-bot
added
size/L
jetstack-bot
added
size/L
|
/assign @wallrj |
| @@ -9,7 +9,7 @@ Welcome to cert-manager's documentation! | |||
|
|
|||
| cert-manager is a native Kubernetes_ certificate management controller. | |||
| It can help with issuing certificates from a variety of sources, such as | |||
| `Let's Encrypt`_, `HashiCorp Vault`_ or a simple signing keypair. | |||
| `Let's Encrypt`_, `HashiCorp Vault`_, a simple signing keypair, or self signed. | |||
There was a problem hiding this comment.
❓ Maybe link to the new self signed issuer page.
There was a problem hiding this comment.
As the other services do not link to internal pages (but to the website of the respective service/app), I decided not to here in favour of having links below.
docs/reference/issuers.rst
Outdated
| +-----------------------------------------------+----------------------------------------------------------------------+ | ||
| | :doc:`Vault <issuers/vault/index>` | Supports issuing certificates using HashiCorp Vault. | | ||
| +-----------------------------------------------+----------------------------------------------------------------------+ | ||
| | :doc:`Self signed <issuers/selfsigned/index>` | Supports issuing self signed Certificates | |
| The presence of the ``selfSigned: {}`` line is enough to indicate that this Issuer | ||
| is of type 'self signed'. | ||
|
|
||
| Once created, you should be able to Issue certificates like normal by |
| CA *CAIssuer `json:"ca,omitempty"` | ||
| Vault *VaultIssuer `json:"vault,omitempty"` | ||
| SelfSigned *SelfSignedIssuer `json:"selfSigned,omitempty"` | ||
| } |
There was a problem hiding this comment.
❓ Should there be validation to ensure that only one of these attributes is used?
There was a problem hiding this comment.
Yes - #478 introduces validation with a validating webhook, however in order to make setting this up seamless, we need a selfsigned issuer type too, causing a bit of a dependency cycle on our PRs 😄
| s := messageErrorGetCertKeyPair + err.Error() | ||
| crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorGetCertKeyPair, s, false) | ||
| return nil, nil, err | ||
| } |
There was a problem hiding this comment.
❓ This error handling doesn't seem quite right.
- If
k8sErrors.IsNotFound(err) == true, which seems to be an expected condition, we still return the error. - If
errors.IsInvalidData(err) == true, we generate a new private key, but why would the returned secret have invalid data? Maybe add an explanatory comment above this code, if it is correct.
There was a problem hiding this comment.
If k8sErrors.IsNotFound(err) == true, which seems to be an expected condition, we still return the error.
We won't return an error in this case, so long as generating the private key is successful (as GenerateRSAPrivateKey will return an error, or nil, which will be captured into err)
There was a problem hiding this comment.
If errors.IsInvalidData(err) == true, we generate a new private key, but why would the returned secret have invalid data? Maybe add an explanatory comment above this code, if it is correct.
There may be invalid data if a user has manually modified the Secret/private key (or something has happened causing it to not be parse-able)
| crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertRenewed, messageCertRenewed, true) | ||
|
|
||
| return pki.EncodePKCS1PrivateKey(signeeKey), certPem, nil | ||
| } |
munnerz
added
approved
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
This adds a basic self signed Issuer type, for issuing self signed Certificates.
This will be used for automatic setup of resource validation in #478
Which issue this PR fixes: fixes #84
Release note: