bneradt commented on code in PR #13807:
URL: https://github.com/apache/trafficserver/pull/13807#discussion_r4201392056
##########
plugins/experimental/jax_fingerprint/plugin.cc:
##########
@@ -384,69 +357,49 @@ TSPluginInit(int argc, char const **argv)
return;
}
- auto owned_config = std::make_unique<PluginConfig>();
- owned_config->plugin_type = PluginType::GLOBAL;
-
- if (!read_config_option(argc, argv, *owned_config)) {
- TSError("[%s] Failed to parse options.", PLUGIN_NAME);
+ if (argc != 2) {
+ TSError("[%s] Usage: %s <config.yaml>", PLUGIN_NAME, argv[0]);
return;
}
- if (!owned_config->log_filename.empty()) {
- if (!create_log_file(owned_config->log_filename,
owned_config->log_handle)) {
- TSError("[%s] Failed to create log.", PLUGIN_NAME);
- return;
- } else {
- Dbg(dbg_ctl, "Created log file.");
- }
+ PluginConfigs configs;
+ if (!load_config_file(argv[1], PluginType::GLOBAL, configs)) {
+ TSError("[%s] Failed to load configuration from %s.", PLUGIN_NAME,
argv[1]);
+ return;
}
- // Reserve the index before registering the log field, so that every failure
exit happens while the
- // configuration is still owned here and nothing has taken a reference to it
yet.
- if (reserve_user_arg(*owned_config) == TS_ERROR) {
- TSError("[%s] Failed to reserve user arg index.", PLUGIN_NAME);
+ if (!prepare_configs(configs)) {
return;
}
- // A global plugin's configuration lives for the life of the process: the
log field callback and the
- // continuation below both keep a reference to it, so release it from the
unique_ptr here.
- PluginConfig *config = owned_config.release();
-
- if (!config->log_symbol.empty()) {
- std::string name = "jax_fingerprint-";
- name += config->method.name;
- TSLogFieldRegister(
- name.c_str(), config->log_symbol, TS_LOG_TYPE_STRING,
- [config](TSHttpTxn txnp, char *buf) -> int {
- void *container;
- if (config->method.type == Method::Type::CONNECTION_BASED) {
- container = TSHttpSsnClientVConnGet(TSHttpTxnSsnGet(txnp));
- } else {
- container = txnp;
- }
- JAxContext *ctx = get_user_arg(container, *config);
- if (ctx) {
- return TSLogStringMarshal(buf, ctx->get_fingerprint());
- } else {
- return TSLogStringMarshal(buf, "-");
- }
- },
- TSLogIntUnmarshal);
- }
+ g_global_state.config_filename = argv[1];
- TSCont cont = TSContCreate(main_handler, nullptr);
- TSContDataSet(cont, config);
- if (config->method.on_client_hello) {
- TSHttpHookAdd(TS_SSL_CLIENT_HELLO_HOOK, cont);
- }
- if (config->standalone) {
- TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, cont);
- }
- if (config->method.type == Method::Type::CONNECTION_BASED) {
- TSHttpHookAdd(TS_VCONN_CLOSE_HOOK, cont);
- } else {
- TSHttpHookAdd(TS_HTTP_TXN_CLOSE_HOOK, cont);
+ // Global configurations live for the life of the process: the log field
callbacks and the
+ // continuations below keep references to them, so release them from their
unique_ptrs here.
+ for (auto &owned_config : configs) {
+ PluginConfig *config = owned_config.release();
+ g_global_state.configs.push_back(config);
Review Comment:
[P2] Keep reload state separate for each global plugin entry
A second `jax_fingerprint.so <config.yaml>` line overwrites
`g_global_state.config_filename` but appends its configurations to the same
`g_global_state.configs`. ATS permits these duplicate global loads (with a
warning), and each initialization registers another lifecycle callback. For
example, loading `a.yaml` and `b.yaml` with one fingerprint each succeeds, but
`traffic_ctl plugin msg jax_fingerprint.reload` loads only `b.yaml` and
compares its one entry against the combined two-entry list. Every callback then
rejects the reload with "the number of fingerprint entries changed from 2 to
1", so neither file can be reloaded. I reproduced this by executing the current
TSPluginInit and lifecycle callbacks with ATS API stubs; a single-entry control
reloaded successfully. Keep a filename/configuration list per initialization
and attach that state to its lifecycle continuation, and add a reload case with
two global entries. If multiple global entries are intentionally unsupported,
rejec
t them explicitly during initialization instead of accepting a deployment
whose reload can never work.
--
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]
