nikhiln64 commented on PR #18434: URL: https://github.com/apache/dolphinscheduler/pull/18434#issuecomment-5248470201
Good catch, thank you, you are right that reading resourceList.get(0) is unsafe. resourceList is multi select and holds auxiliary files like keytabs and xml, so a task with keytab and job.json would have read the keytab as the DataX json, and a task carrying only auxiliary resources passed validation with no job definition at all. I have changed the contract to identify the job definition as the single resource whose name ends with .json, in a new DataxParameters.getJobDefinitionResource, rather than the first entry in the list. checkParameters now rejects the cases where there is no json resource and where there is more than one json resource, so a keytab only task and an ambiguous two json task are both invalid, and auxiliary resources stay separate. The worker reads that designated resource and throws a clear error if it is absent rather than falling back to a wrong file. For coverage I added a DataxParametersTest case for a keytab before the job file, two json resources, and an auxiliary only list, and a DataxTaskTest that attaches keytab and job.json and asserts the generated job file carries the job.json content and not the keytab. The datax module tests pass on Java 8 and the branch is rebased onto the latest dev. Let me know if you would prefer a stricter contract than the single .json convention, for example an explicit designated field. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
