Dont attempt to read certificate unless ssl is enabled

[rethab]

trafficstars

Bug report

The tlsConfig in WithCACerts relies on ssl being enabled, because it is initialized in WithSSL. If it is not, transporter blows up with an incomprehensible exception.

System info:

• 0.3
• Linux
• 3.2

What did you expect to happened?

If ssl is set to false, don't even try to read the certificates. I guess the if could just be extended with the ssl flag: https://github.com/compose/transporter/blob/master/adaptor/mongodb/client.go#L137

[jipperinbham]

@rethab

could you provide the pipeline.js with any sensitive information redacted? I feel like we have tests to cover the case you're describing so I want to make sure I'm following it properly.

[rethab]

@jipperinbham please apologize, I should have been clearer. So here's my scenario: I actually have a cacert configured, but at the same time disabled ssl (ie. ssl = false). However the certificate is still being read and eventually, transporter test fails, because the certificate I have configured is not correct for that host.

So I have either of these configs:

• ssl: true and valid entry in cacerts
• ssl: false and invalid entry in cacerts

The latter case is important for me, because if my sink is localhost, I don't have ssl on that end and I still want to flexibly use multiple sinks. This is my config:

var sink = mongodb({
  "uri": "mongodb://${DEST_USR_PWD}${DEST_HOST}/${DEST_DB}"
  "ssl": ${DEST_SSL},
  "cacerts": ["${DEST_CERT_FILE}"]
})

Since transporter attempts to read the cacert even when ssl is disabled, I would have to somehow make sure the cacerts array is empty if I don't want it to verify the certificate, which would make for a rather clumsy shell-wrapper.. I think it would be more straightforward, if transporter just ignored the cacerts if ssl is false. Maybe print a warning..

What do you think?

[jipperinbham]

ok, I completely understand what you're seeing, why, and the expectations.

My original thought behind ignoring the ssl value when processing the cacerts value was a rather naive assumption that people wouldn't set cacerts to something and ssl to false.

I want to say my reasoning was that people would want to just set cacerts and it thus be implied that ssl is true regardless of whether it was included in the configuration for the adaptor.

For the sake of "less magic", I agree the more straightforward approach is to also validate ssl is true before even attempting to process whatever may be provided in cacerts.