Add additional tags for x509 Input Plugin #6686
Conversation
| @@ -129,10 +139,15 @@ func getFields(cert *x509.Certificate, now time.Time) map[string]interface{} { | |||
| return fields | |||
| } | |||
|
|
|||
| func getTags(subject pkix.Name, location string) map[string]string { | |||
| func (c *X509Cert) getTags(cert *x509.Certificate, location string) map[string]string { | |||
| subject := cert.Subject | |||
There was a problem hiding this comment.
Nitpick, just get cert.Subject.CommonName on line 147 without creating a local variable.
There was a problem hiding this comment.
subject is used in more than one place, but I've removed the local variable.
| "common_name": subject.CommonName, | ||
| "source": location, | ||
| "common_name": subject.CommonName, | ||
| "serial_number": cert.SerialNumber.Text(16), |
There was a problem hiding this comment.
How large is the serial_number? Can you add unit tests with examples of all the data used by these changes?
Also, how does this compare to the cert.Subject.SerialNumber?
There was a problem hiding this comment.
The certificate serial number is at most 20 bytes and this will be at most 40 Hex characters. This is supposed to be unique per CA.
On the other hand, based on what I know, cert.Subject.SerialNumber is an optional serial number that the public key owner can include. It's probably not as useful to disambiguate certificates than the certificate serial.
There was a problem hiding this comment.
I have added a new test.
etc/telegraf.conf
Outdated
| # # include_san = false | ||
| # ## Separator between each SAN in tag | ||
| # # san_separator = "," | ||
| # |
There was a problem hiding this comment.
Can you undo this file, we will take care of it afterwards. We usually just update this once before release to avoid conflicts.
| ## Include Certificate SAN in tag | ||
| # include_san = false | ||
| ## Separator between each SAN in tag | ||
| # san_separator = "," |
There was a problem hiding this comment.
Let's remove all of these options, the plugin user can use tagexclude/taginclude instead. On the SAN separator we can just always use ,, I think there is little reason to modify it but I suppose it could be done with a processor, and comma is consistent with our other uses.
There was a problem hiding this comment.
OK. I have removed them.
danielnelson
added
the
feat
Required for all PRs:
Fixes #6685