[MWTELE-48]: Do not create report if cert/key not present and report cannot be sent. #41
Conversation
There was a problem hiding this comment.
One comment. Otherwise LGTM.
We might also consider improvement in InsightsMultiClient.sendInsightsReport() (https://github.com/RedHatInsights/insights-java-client/blob/main/api/src/main/java/com/redhat/insights/http/InsightsMultiClient.java#L38) to filter clients which "are not ready" to sent.
| @Override | ||
| public boolean isReadyToSend() { | ||
| return new File(config.getCertFilePath()).exists() | ||
| && new File(config.getKeyFilePath()).exists(); |
There was a problem hiding this comment.
I think that for file upload the location of cert/key is given and we should use defaults:
InsightsConfiguration.DEFAULT_RHEL_CERT_FILE_PATH and InsightsConfiguration.DEFAULT_RHEL_KEY_FILE_PATH
pointing to key and cert in /etc/pki/consumer directory. Retrieving it from configuration might be overriden by env/system properties.
We should investigate also https://github.com/RedHatInsights/insights-ingress-java/issues/126 to improve this logic.
| @Override | ||
| public boolean isReadyToSend() { | ||
| return new File(configuration.getCertFilePath()).exists() | ||
| && new File(configuration.getKeyFilePath()).exists(); |
There was a problem hiding this comment.
How will this code work if we're using token-based auth & there are no certs?
…cannot be sent. * InsightsMultiClient checks that at leat one of the client is ready. * InsightsFileWritingClient checks that the Insights certificates are present. * InsightsJdkHttpClient checks that the Insights certificates are present or that a token is being used. Jira: https://issues.redhat.com/browse/MWTELE-48 Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
Jira: https://issues.redhat.com/browse/MWTELE-48