Koperator can handle intermediate and leaf certificates in generated kafkaUser's TLS certificate #843
Conversation
controllers/kafkauser_controller.go
Outdated
| @@ -238,7 +238,13 @@ func (r *KafkaUserReconciler) Reconcile(ctx context.Context, request reconcile.R | |||
| return requeueWithError(reqLogger, "failed to reconcile user secret", err) | |||
| } | |||
| } | |||
| kafkaUser = user.DN() | |||
| kafkaUser, err = user.DN() | |||
There was a problem hiding this comment.
Note: we need to rename that function for public interface clarity purposes....
| return cert.Subject.String() | ||
| cert, err := certutil.DecodeCertificate(u.Certificate) | ||
| if err != nil { | ||
| return "", err |
There was a problem hiding this comment.
Question: wouldn't it be better to wrap the error with some content information (we are doing this in KafkaUser DistinguishedName retrieval), IMO it would help with debug clarification.
(These cases I always have the os.Open() errors in mind where the non-existing path object error is no such file or directory, but the path itself is not included - although that would be a detail, not a wrapping, but I mean the contextual information in general).
There was a problem hiding this comment.
I'm not sure if any useful information is available here that could be added to the error context. The caller can add more details to the error like the KafkaUser custom resources name (https://github.com/banzaicloud/koperator/pull/843/files#diff-2b2cc6c6efb0a3a4b49078c08a64ff936f6d02a73543d0c8f98a7b9b0c4f2a2eR243)
Maybe a more descriptive error message could help here errors.WrapIfWithDetails(err, "couldn't decode KafkaUser certificate", "cert", string(u.Certificate))
There was a problem hiding this comment.
Yeah, I meant an error message to narrow the context from generic certificate error to KafkaUser DistringuishedName determination certificate error
There was a problem hiding this comment.
The error log message already contains the kafkauser name, namespace.
There was a problem hiding this comment.
Reconciler dumps the stacktrace.
What's in this PR?
It fixes bug in the kafkaUser reconcile when trying to get Distinguish Name from the generated TLS certificate.
Why?
When the generated TLS certificate contains intermediate certificates, not only the leaf certificate, the Koperator gets panic because of nil pointer dereference.
Checklist