You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2021/09/10 15:42:55 UTC
[GitHub] [trafficcontrol] ocket8888 opened a new pull request #6191: Traffic Portal Server Options
ocket8888 opened a new pull request #6191:
URL: https://github.com/apache/trafficcontrol/pull/6191
This PR adds the ability to run Traffic Portal using an arbitrary configuration file via the new `-c` flag to its `server.js` main script, an environment variable to control passing said flag to `grunt`, and enables logging to `stdout` when logging is not configured in the configuration file.
This also adds documentation for Traffic Portal's main configuration file.
<!-- **^ Add meaningful description above** --><hr>
## Which Traffic Control components are affected by this PR?
- Documentation
- Traffic Portal
## What is the best way to verify this PR?
Make sure existing tests pass, test the new behavior by making a copy of `configDev.js` (or `config.js`) somewhere, changing some configuration (e.g. `sslPort`) and running `grunt` with `TP_SERVER_CONFIG_FILE` set to the location of the modified copy. If the server that's run has the modified settings, then you know it used the right configuration file, which is how I tested it.
## PR submission checklist
- [x] This PR has documentation
- [x] This PR has a CHANGELOG.md entry
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**
--
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: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #6191: Traffic Portal Server Options
Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #6191:
URL: https://github.com/apache/trafficcontrol/pull/6191#discussion_r706317357
##########
File path: traffic_portal/server.js
##########
@@ -28,19 +28,29 @@ var constants = require('constants'),
var config;
-try {
- config = require('/etc/traffic_portal/conf/config');
-}
-catch(e) {
- let file = "./conf/config";
- if((process.env.NODE_ENV || "prod") === "dev")
- file = './conf/configDev';
- config = require(file);
+// forward-compatible in case this script becomes executable with a node
+// shebang.
+if (process.argv.length === 3 || process.argv.length === 4 && process.argv.slice(-2)[0] === "-c") {
+ config = require(process.argv.slice(-1)[0]);
+} else {
+ try {
+ config = require('/etc/traffic_portal/conf/config');
+ }
+ catch(e) {
+ let file = "./conf/config";
+ if((process.env.NODE_ENV || "prod") === "dev")
+ file = './conf/configDev';
+ config = require(file);
+ }
}
-var logStream = fs.createWriteStream(config.log.stream, { flags: 'a' }),
- useSSL = config.useSSL;
-
+const useSSL = config.useSSL;
+let logStream;
+if (!Object.prototype.hasOwnProperty.call(config, "log") || !config.log || !Object.prototype.hasOwnProperty.call(config.log, "stream") || !config.log.stream) {
+ logStream = null;
Review comment:
I think we would want this to halt execution in this state.
--
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: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #6191: Traffic Portal Server Options
Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #6191:
URL: https://github.com/apache/trafficcontrol/pull/6191#discussion_r710121328
##########
File path: traffic_portal/package-lock.json
##########
@@ -5,6 +5,7 @@
"requires": true,
"packages": {
"": {
+ "name": "traffic_portal",
Review comment:
I don't understand why this wasn't added a long time ago, the name field in `package.json` has been set for like 4 years.
--
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: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6191: Traffic Portal Server Options
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6191:
URL: https://github.com/apache/trafficcontrol/pull/6191#discussion_r706368504
##########
File path: traffic_portal/server.js
##########
@@ -28,19 +28,29 @@ var constants = require('constants'),
var config;
-try {
- config = require('/etc/traffic_portal/conf/config');
-}
-catch(e) {
- let file = "./conf/config";
- if((process.env.NODE_ENV || "prod") === "dev")
- file = './conf/configDev';
- config = require(file);
+// forward-compatible in case this script becomes executable with a node
+// shebang.
+if (process.argv.length === 3 || process.argv.length === 4 && process.argv.slice(-2)[0] === "-c") {
+ config = require(process.argv.slice(-1)[0]);
+} else {
+ try {
+ config = require('/etc/traffic_portal/conf/config');
+ }
+ catch(e) {
+ let file = "./conf/config";
+ if((process.env.NODE_ENV || "prod") === "dev")
+ file = './conf/configDev';
+ config = require(file);
+ }
}
-var logStream = fs.createWriteStream(config.log.stream, { flags: 'a' }),
- useSSL = config.useSSL;
-
+const useSSL = config.useSSL;
+let logStream;
+if (!Object.prototype.hasOwnProperty.call(config, "log") || !config.log || !Object.prototype.hasOwnProperty.call(config.log, "stream") || !config.log.stream) {
+ logStream = null;
Review comment:
No, that's intentional, and causes logging to go to stdout per
> ... enables logging to `stdout` when logging is not configured in the configuration file.
Previously it *would* halt execution because it would be trying to either access a property of a "falsey" value or trying to open a writestream to something that isn't a valid filename. That's no longer desired behavior, which is why I do the intentional checks here.
--
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: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus merged pull request #6191: Traffic Portal Server Options
Posted by GitBox <gi...@apache.org>.
shamrickus merged pull request #6191:
URL: https://github.com/apache/trafficcontrol/pull/6191
--
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: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org