Here is an example of this problem. This could have been caught early: https://github.com/apache/trafficserver/blob/master/src/records/RecordsConfig.cc#L1337
thanks. On Thu, Aug 8, 2024 at 3:20 PM Damian Meden <dme...@apache.org> wrote: > Hi guys, wanted to get some feedback from you, so: > > > *The goal:* > > > Add records consistency check for default values on startup(core records > and plugin records(TSMgmt*Create) > > Details: > > Working around some record consistency check issues > <https://github.com/apache/trafficserver/issues/11649> this week I found > that we do not perform this check against the default values specified > during registration > <https://github.com/apache/trafficserver/blob/6774984a3f71f7473092d1af7ba99a004e8890d9/src/records/RecordsConfig.cc#L39> > (4th parameter) of a record(either core or throughout the TS API). > > What’s the consistency check? > > This is supposed to be used to validate the value you set on a record, so > if the check(regex) does not match the value then you get an error and the > value is not set. This is quite clear when you try to set a new record > value using traffic_ctl: > > > $ traffic_ctl config set proxy.config.accept_threads 99999999 > > Server Error found: > > [9] Error during execution > > - [2000] Validity check failed. [2004] > > > > So, I want to add a record consistency check on startup for default > records values set on (RecordsConfig.cc > <https://github.com/apache/trafficserver/blob/6774984a3f71f7473092d1af7ba99a004e8890d9/src/records/RecordsConfig.cc#L39> > and > the TSMgmt*Create API) to avoid possible bugs. > > As I said this is already done for values we read from the records.yaml > and values set through traffic_ctl but not for the default value specified > in the record registration code(RecordsConfig.cc and TS API) > > > Looking for feedback on: > > Is it OK to add this check for default values( ATS startup )? > > If so: > > - (A) Should we fatal(ATS will not start) > > - (B) Do not register the record and print an error message > > This may break some instances with plugins which registers new records > with wrong value/regex in it. > > Is important to note that currently if you register a new record and > configure the check to be performed but you do not specify a regex, then > ATS will fail > <https://github.com/apache/trafficserver/blob/6774984a3f71f7473092d1af7ba99a004e8890d9/src/records/RecordsConfigUtils.cc#L58-L60>(fatal). > I think something similar can be done. > > Looking forward to hearing from you. > > > Thanks > > Damian. > > Yahoo > >
