----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69398/#review211004 -----------------------------------------------------------
Seems a bit clearer to have a separate ticket for validating framework IDs and say on the crash ticket that we fix it via the validation ticket? Probably we need to talk about the fact that mesos is the one that generates the frameowrk IDs (and document the format we currently use in the protobuf), and mention that the behavior of framework generating them is not supported (but this has been an abuse used by a few frameworks at certain points in time). In the case of this ticket, do you know how the invalid one showed up? Also, note that we write framework Id as a part of file system paths, so need to deal with same validation issues that came up for windows recently with executor IDs. - Benjamin Mahler On Nov. 19, 2018, 3:52 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69398/ > ----------------------------------------------------------- > > (Updated Nov. 19, 2018, 3:52 p.m.) > > > Review request for mesos, Benjamin Mahler and Meng Zhu. > > > Bugs: MESOS-8470 > https://issues.apache.org/jira/browse/MESOS-8470 > > > Repository: mesos > > > Description > ------- > > We never explicitly supported `FrameworkID`s containing literal `/`. > Moreover, since the introduction of hierarchical roles such > `FrameworkID`s would cause fatal errors. > > This patch adds validation for `FrameworkID`s so such IDs are > rejected. > > > Diffs > ----- > > src/master/validation.hpp 9af903970795a5c8c479d1984a580e41d91f6c91 > src/master/validation.cpp 5768ac8fe802f28855fbd7be135c711115532771 > src/tests/master_validation_tests.cpp > aa7c8f70c09459be32c6c415497e95fcdc218efd > > > Diff: https://reviews.apache.org/r/69398/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
