Druid: Druid allows adding empty string dataSources (it shouldn't)

[vogievetsky]

It is currently possible to create a data source with the name set to "" by submitting a ingestion spec with "dataSource": "".
This task will work fine and create a data source.

The chances that this was intentional are 0% but this can be created by submitting some misconfigured template.

This will put Druid in a benignly bad state as this data source will not be deletable. Most of the RESTful APIs will not work for it because they have /<dataSourceName>/ in the URL which, being set to //, does not. Native queries would work against it but Druid SQL will not (Calcite parse error). In fact it is not possible to get rid of this dataSource in a 'normal' way. You have to go into the metadata sore an haxor it out.

Please add a check to Druid to return an error if an "" dataSource is submitted.

Also shoutout if you have a "" in your cluster due to a template error and you have grown to live with it.

[debdutto]

Hi @vogievetsky @gianm

I'm pretty new to druid. Would love to take a swing at this.

Let me see if I can whip up a PR by tonight, if it's okay with you?

[drcrallen]

@vogievetsky your bug reports are always the best

[clintropolis]

For indexing tasks at least, I am unable to replicate this issue in latest master, I believe due to this patch https://github.com/druid-io/druid/pull/5785 which fixed this very issue, causing an indexing task with a missing datasource to return an error message like this

{"error":"Instantiation of [simple type, class io.druid.segment.indexing.DataSchema] value failed: dataSource cannot be null or empty. Please provide a dataSource. ...

However, I think it highlights a larger underlying issue that there are very likely a lot of places where we are unintentionally using Preconditions.checkNotNull for json sourced strings instead of Preconditions.checkArgument in combination with Strings.isNullOrEmpty, in places where an empty string would be just as invalid as null, which might be worth investigating (along with json input validation in general). A quick search of tasks scored a handful of other task types which take a datasource that is not part of a DataSchema, that are only using Preconditions.checkNotNull check in AbstractTask, which should probably be modified to use the same Strings.isNullOrEmpty check. I am unsure offhand if any of these tasks can side-effect an emtpy string named datasource into existence however.

@Debdutto I was looking into this issue after @vogievetsky encountered it, and to offer some pointers to help you get started, but since I believe it is already fixed maybe you would instead be interested in looking into as much or as little as you prefer of the deeper issue I speculate exists?

[debdutto]

@clintropolis I was just in the process of setting everything up last night. I'm willing to take it up, if you think there's something larger going on. But I will need some direction / guidance.

What I'll do is, I'll take a look at @vogievetsky's issue and the underlying Preconditions.checkNotNull issue that you suggested. I'll come back with observations and a probable solution . Then you can let me know if I'm going off the rails somewhere.

[vogievetsky]

Oh indeed it appears the #5785 already fixed it, please feel free to close this issue for tidiness sake.
@Debdutto I hope you do look into the suggestions for stricter checks outlined by @clintropolis even if this issue is closed - who knows what other weirdness there is where "" is allowed.

[gianm]

Fixed by #5785.