You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by SolidWallOfCode <gi...@git.apache.org> on 2016/09/06 20:35:29 UTC
[GitHub] trafficserver pull request #971: TS-1882: Check for unrecognized configurati...
GitHub user SolidWallOfCode opened a pull request:
https://github.com/apache/trafficserver/pull/971
TS-1882: Check for unrecognized configuration values.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/SolidWallOfCode/trafficserver ts-1882
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/971.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #971
----
commit 3e15966e7fd99242d2bdec0a20fb3966b81d959f
Author: Alan M. Carroll <so...@yahoo-inc.com>
Date: 2016-09-06T20:34:21Z
TS-1882: Check for unrecognized configuration values.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the issue:
https://github.com/apache/trafficserver/pull/971
I will try to work on some documentation today, don't merge this yet.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #971: TS-1882: Check for unrecognized configurati...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/971#discussion_r78089078
--- Diff: lib/records/RecCore.cc ---
@@ -514,6 +514,18 @@ RecLookupMatchingRecords(unsigned rec_type, const char *match, void (*callback)(
return REC_ERR_OKAY;
}
+void
+RecLookupIterateRecords(std::function<void (RecRecord const*)> f)
--- End diff --
Isn't this the same as ``RecDumpRecords()``? Do we need to have both?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/971
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/746/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/971
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/714/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the issue:
https://github.com/apache/trafficserver/pull/971
I move the check logic later and based on testing it should be after both global plugins and remap plugins are initialized. I also added the check to happen after a configuration reload.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/971
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/766/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #971: TS-1882: Check for unrecognized configurati...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/971#discussion_r78089641
--- Diff: lib/records/I_RecCore.h ---
@@ -168,11 +169,15 @@ int RecGetRecordBool(const char *name, RecBool *rec_byte, bool lock = true);
//------------------------------------------------------------------------
// Record Attributes Reading
//------------------------------------------------------------------------
+// Values with names that have this prefix are exempted from being considered "unrecognized".
+const char REC_PLUGIN_CONFIG_NAME_PREFIX[] = "proxy.config.plugin.";
+static const size_t REC_PLUGIN_CONFIG_NAME_PREFIX_LEN = sizeof(REC_PLUGIN_CONFIG_NAME_PREFIX)-1;
--- End diff --
I don't think this is necessary now that you are checking whether the record is registered?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/971
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/642/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #971: TS-1882: Check for unrecognized configurati...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/971#discussion_r78096037
--- Diff: lib/records/RecCore.cc ---
@@ -514,6 +514,18 @@ RecLookupMatchingRecords(unsigned rec_type, const char *match, void (*callback)(
return REC_ERR_OKAY;
}
+void
+RecLookupIterateRecords(std::function<void (RecRecord const*)> f)
--- End diff --
Yes, I saw the `RecLookup...` functions and modeled the new call on those. I didn't see `RecDumpRecords`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #971: TS-1882: Check for unrecognized configurati...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/971#discussion_r78096159
--- Diff: lib/records/I_RecCore.h ---
@@ -168,11 +169,15 @@ int RecGetRecordBool(const char *name, RecBool *rec_byte, bool lock = true);
//------------------------------------------------------------------------
// Record Attributes Reading
//------------------------------------------------------------------------
+// Values with names that have this prefix are exempted from being considered "unrecognized".
+const char REC_PLUGIN_CONFIG_NAME_PREFIX[] = "proxy.config.plugin.";
+static const size_t REC_PLUGIN_CONFIG_NAME_PREFIX_LEN = sizeof(REC_PLUGIN_CONFIG_NAME_PREFIX)-1;
--- End diff --
I think it's a nice convenience to avoid additional API calls or work.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/971
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/641/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #971: TS-1882: Check for unrecognized configurati...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafficserver/pull/971
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #971: TS-1882: Check for unrecognized configurati...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/971#discussion_r77854033
--- Diff: proxy/Main.cc ---
@@ -476,6 +476,18 @@ check_config_directories(void)
}
}
+namespace {
+ // Values with names that have this prefix are exempted from being considered "unrecognized".
+ const char PLUGIN_CONFIG_RECORD_PREFIX[] = "proxy.config.plugin.";
--- End diff --
I think the right check is to test whether there are any unregistered local or config records. This can be done periodically since new records can be added at config reload time. A records that is present in ``records.config`` will be not be marked as registered intul a plugin calls ``TSMgmtStringCreate()`` or the equivalent.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/971
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/757/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/971
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/745/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #971: TS-1882: Check for unrecognized configurati...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/971#discussion_r77874593
--- Diff: proxy/Main.cc ---
@@ -476,6 +476,18 @@ check_config_directories(void)
}
}
+namespace {
+ // Values with names that have this prefix are exempted from being considered "unrecognized".
+ const char PLUGIN_CONFIG_RECORD_PREFIX[] = "proxy.config.plugin.";
--- End diff --
This is the root of the problem in that if arbitrary config names can be used by plugins, you can't do validity checks. The consensus at the bug scrub was it was reasonable to restrict plugin configuration values to that subtree.
I could move the check after plugin initialization so that if a plugin does call {{TSMgmtStringCreate}} it doesn't generate a warning.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/971
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/662/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #971: TS-1882: Check for unrecognized configurati...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/971#discussion_r77714260
--- Diff: proxy/Main.cc ---
@@ -476,6 +476,18 @@ check_config_directories(void)
}
}
+namespace {
+ // Values with names that have this prefix are exempted from being considered "unrecognized".
+ const char PLUGIN_CONFIG_RECORD_PREFIX[] = "proxy.config.plugin.";
--- End diff --
This is a big assumption. I have internal plugins that use configuration values that are not in this namespace.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/971
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/653/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #971: TS-1882: Check for unrecognized configurati...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/971#discussion_r78096463
--- Diff: lib/records/I_RecCore.h ---
@@ -168,11 +169,15 @@ int RecGetRecordBool(const char *name, RecBool *rec_byte, bool lock = true);
//------------------------------------------------------------------------
// Record Attributes Reading
//------------------------------------------------------------------------
+// Values with names that have this prefix are exempted from being considered "unrecognized".
+const char REC_PLUGIN_CONFIG_NAME_PREFIX[] = "proxy.config.plugin.";
+static const size_t REC_PLUGIN_CONFIG_NAME_PREFIX_LEN = sizeof(REC_PLUGIN_CONFIG_NAME_PREFIX)-1;
--- End diff --
You can't get the value of the record unless it is registered, so I don't think this saves you any work.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #971: TS-1882: Check for unrecognized configuration valu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/971
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/610/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---