Re: Review Request 69398: Added validation for `FrameworkID`s.
--- 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 5768ac8fe802f28855fbd7be135c75532771 > src/tests/master_validation_tests.cpp > aa7c8f70c09459be32c6c415497e95fcdc218efd > > > Diff: https://reviews.apache.org/r/69398/diff/1/ > > > Testing > --- > > `make check` > > > Thanks, > > Benjamin Bannier > >
Re: Review Request 69398: Added validation for `FrameworkID`s.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69398/#review210771 --- Ship it! Ship It! - Meng Zhu On Nov. 19, 2018, 7:52 a.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69398/ > --- > > (Updated Nov. 19, 2018, 7:52 a.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 5768ac8fe802f28855fbd7be135c75532771 > src/tests/master_validation_tests.cpp > aa7c8f70c09459be32c6c415497e95fcdc218efd > > > Diff: https://reviews.apache.org/r/69398/diff/1/ > > > Testing > --- > > `make check` > > > Thanks, > > Benjamin Bannier > >
Re: Review Request 69398: Added validation for `FrameworkID`s.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69398/#review210666 --- PASS: Mesos patch 69398 was successfully built and tested. Reviews applied: `['69397', '69398']` All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2622/mesos-review-69398 - Mesos Reviewbot Windows 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 5768ac8fe802f28855fbd7be135c75532771 > src/tests/master_validation_tests.cpp > aa7c8f70c09459be32c6c415497e95fcdc218efd > > > Diff: https://reviews.apache.org/r/69398/diff/1/ > > > Testing > --- > > `make check` > > > Thanks, > > Benjamin Bannier > >
Review Request 69398: Added validation for `FrameworkID`s.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69398/ --- Review request for mesos 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 5768ac8fe802f28855fbd7be135c75532771 src/tests/master_validation_tests.cpp aa7c8f70c09459be32c6c415497e95fcdc218efd Diff: https://reviews.apache.org/r/69398/diff/1/ Testing --- `make check` Thanks, Benjamin Bannier
