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/03/26 12:07:51 UTC
[GitHub] [trafficcontrol] zrhoffman opened a new pull request #5681: Use the Python Postinstall implementation by default
zrhoffman opened a new pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681
<!--
************ STOP!! ************
If this Pull Request is intended to fix a security vulnerability, DO NOT submit it! Instead, contact
the Apache Software Foundation Security Team at security@trafficcontrol.apache.org and follow the
guidelines at https://www.apache.org/security/ regarding vulnerability disclosure.
-->
## What does this PR (Pull Request) do?
<!-- Explain the changes you made here. If this fixes an Issue, identify it by
replacing the text in the checkbox item with the Issue number e.g.
- [x] This PR fixes #9001 OR is not related to any Issue
^ This will automatically close Issue number 9001 when the Pull Request is
merged (The '#' is important).
Be sure you check the box properly, see the "The following criteria are ALL
met by this PR" section for details.
-->
- Makes Postinstall use the Python backend instead of the old Perl one
- Deprecates the Perl version of the Postinstall script
- CDN in Box now configures Traffic Ops using the Python rewrite of the TO Postinstall script (fixes #5680).
- [x] This PR fixes #3437 by documenting JSON input to the Postinstall script. We do not need to remove `input.json` because if it ever were to become out-of-date, CDN in a Box would break.<!-- You can check for an issue here: https://github.com/apache/trafficcontrol/issues -->
## Which Traffic Control components are affected by this PR?
<!-- Please delete all components from this list that are NOT affected by this
Pull Request. Also, feel free to add the name of a tool or script that is
affected but not on the list.
Additionally, if this Pull Request does NOT affect documentation, please
explain why documentation is not required. -->
- CDN in a Box
- Documentation
- Traffic Ops Postinstall script
## What is the best way to verify this PR?
<!-- Please include here ALL the steps necessary to test your Pull Request. If
it includes tests (and most should), outline here the steps needed to run the
tests. If not, lay out the manual testing procedure and please explain why
tests are unnecessary for this Pull Request. -->
- Run `postinstall.test.sh`
- Verify that the CDN in a Box Delivery Service can be reached
## The following criteria are ALL met by this PR
<!-- Check the boxes to signify that the associated statement is true. To
"check a box", replace the space inside of the square brackets with an 'x'.
e.g.
- [ x] <- Wrong
- [x ] <- Wrong
- [] <- Wrong
- [*] <- Wrong
- [x] <- Correct!
-->
- [x] This PR includes tests
- [x] This PR includes documentation
- [x] This PR includes an update to CHANGELOG.md
- [x] This PR includes any and all required license headers
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
## Additional Information
<!-- If you would like to include any additional information on the PR for
potential reviewers please put it here.
Some examples of this would be:
- Before and after screenshots/gifs of the Traffic Portal if it is affected
- Links to other dependent Pull Requests
- References to relevant context (e.g. new/updates to dependent libraries,
mailing list records, blueprints)
Feel free to leave this section blank (or, preferably, delete it entirely).
-->
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602509659
##########
File path: traffic_ops/install/bin/input.json
##########
@@ -1,179 +1,151 @@
{
- "/opt/traffic_ops/app/conf/production/database.conf":[
+ "/opt/traffic_ops/app/conf/cdn.conf": [
{
- "Database type":"Pg",
- "config_var":"type"
+ "Generate a new secret?": "yes",
+ "config_var": "genSecret"
},
{
- "Database name":"traffic_ops_db",
- "config_var":"dbname"
+ "Number of secrets to keep?": "10",
+ "config_var": "keepSecrets"
},
{
- "Database server hostname IP or FQDN":"localhost",
- "config_var":"hostname"
+ "Number of workers?": "12",
+ "config_var": "workers"
},
{
- "Database port number":"5432",
- "config_var":"port"
- },
- {
- "Traffic Ops database user":"traffic_ops",
- "config_var":"user"
- },
- {
- "Traffic Ops database password":"default",
- "config_var":"password",
- "hidden":"1"
+ "Traffic Ops url?": "https://[::]",
+ "config_var": "base_url"
}
],
- "/opt/traffic_ops/app/db/dbconf.yml":[
+ "/opt/traffic_ops/app/conf/ldap.conf": [
{
- "Database server root (admin) user":"root",
- "config_var":"dbAdminUser"
+ "Do you want to set up LDAP?": "no",
+ "config_var": "setupLdap"
},
{
- "Database server admin password":"default",
- "config_var":"dbAdminPw",
- "hidden":"1"
+ "LDAP server hostname": "",
+ "config_var": "hostname"
},
{
- "Download Maxmind Database?":"yes",
- "config_var":"maxmind"
- }
- ],
- "/opt/traffic_ops/app/conf/cdn.conf":[
- {
- "Generate a new secret?":"yes",
- "config_var":"genSecret"
+ "LDAP Admin DN": "",
+ "config_var": "admin_dn"
},
{
- "Number of secrets to keep?":"10",
- "config_var":"keepSecrets"
+ "LDAP Admin Password": "",
+ "config_var": "password",
+ "hidden": "1"
},
{
- "Port to serve on?":"443",
- "config_var":"port"
+ "LDAP Search Base": "",
+ "config_var": "search_base"
}
],
- "/opt/traffic_ops/app/conf/ldap.conf":[
+ "/opt/traffic_ops/app/conf/production/database.conf": [
{
- "Do you want to set up LDAP?":"no",
- "config_var":"setupLdap"
+ "Database type": "Pg",
+ "config_var": "type"
},
{
- "LDAP server hostname":"",
- "config_var":"hostname"
+ "Database name": "traffic_ops_db",
+ "config_var": "dbname"
},
{
- "LDAP Admin DN":"",
- "config_var":"admin_dn"
+ "Database server hostname IP or FQDN": "localhost",
+ "config_var": "hostname"
},
{
- "LDAP Admin Password":"",
- "config_var":"password",
- "hidden":"1"
+ "Database port number": "5432",
+ "config_var": "port"
},
{
- "LDAP Search Base":"",
- "config_var":"search_base"
+ "Traffic Ops database user": "dbuser",
+ "config_var": "user"
+ },
+ {
+ "Traffic Ops database password": "dbpass",
+ "config_var": "password",
+ "hidden": "1"
Review comment:
Gotcha, deprecated in 71a84ca259.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602435734
##########
File path: traffic_ops/install/bin/_postinstall
##########
@@ -1,5 +1,5 @@
-#!/usr/bin/perl
-
+#!/usr/bin/env bash
Review comment:
I did use `git mv`,
> Did you change this Python code at all?
Not in 83d69c3778, no. Running
```shell
git mv _postinstall _postinstall.pl;
git mv postinstall.py _postinstall;
```
yields
```shell
Changes to be committed:
(use "git restore --staged <file>..." to unstage)
modified: _postinstall
new file: _postinstall.pl
deleted: postinstall.py
```
, so I can see not wanting to do both steps in the same commit.
Rebased to split that commit into 2 `git mv` commits in ab87a7815e and 3179f52e43 (though the overall diff will still be just as big).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602437113
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -100,108 +92,103 @@ cat <<-EOF >/opt/traffic_ops/app/conf/cdn.conf
"log_location_info": "$TO_LOG_INFO",
"log_location_debug": "$TO_LOG_DEBUG",
"log_location_event": "$TO_LOG_EVENT",
- "max_db_connections": 20,
"db_conn_max_lifetime_seconds": ${DEBUGGING_TIMEOUT:-60},
- "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20},
- "backend_max_connections": {
- "mojolicious": 4
- },
- "whitelisted_oauth_urls": [],
- "oauth_client_secret": "",
- "routing_blacklist": {
- "ignore_unknown_routes": false,
- "disabled_routes": []
- },
- "supported_ds_metrics": [ "kbps", "tps_total", "tps_2xx", "tps_3xx", "tps_4xx", "tps_5xx" ]
- },
- "cors" : {
- "access_control_allow_origin" : "*"
+ "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20}
},
"to" : {
- "base_url" : "https://$TO_FQDN",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "no_account_found_msg" : "A Traffic Ops user account is required for access. Please contact your Traffic Ops user administrator."
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
"portal" : {
"base_url" : "https://$TP_HOST.$INFRA_SUBDOMAIN.$TLD_DOMAIN/#!/",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "pass_reset_path" : "user",
- "user_register_path" : "user"
- },
- "secrets" : [
- "$TO_SECRET"
- ],
- "geniso" : {
- "iso_root_path" : "/opt/traffic_ops/app/public"
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
- "inactivity_timeout" : 60,
"smtp" : {
"enabled" : true,
- "user" : "",
- "password" : "",
"address" : "${SMTP_FQDN}:${SMTP_PORT}"
},
"InfluxEnabled": true,
"influxdb_conf_path": "/opt/traffic_ops/app/conf/production/influx.conf",
"lets_encrypt" : {
- "user_email" : "",
- "send_expiration_email": false,
- "convert_self_signed": false,
- "renew_days_before_expiration": 30,
"environment": "staging"
- },
- "acme_renewal": {
- "summary_email": "",
- "renew_days_before_expiration": 30
- },
- "acme_accounts": [
- {
- "acme_provider" : "",
- "user_email" : "",
- "acme_url" : "",
- "kid" : "",
- "hmac_encoded" : ""
- }
- ]
+ }
}
EOF
+))"
-cat <<-EOF >/opt/traffic_ops/app/conf/production/database.conf
+<<RIAK_CONF cat >/opt/traffic_ops/app/conf/production/riak.conf
{
- "description": "Local PostgreSQL database on port 5432",
- "dbname": "$DB_NAME",
- "hostname": "$DB_FQDN",
- "user": "$DB_USER",
- "password": "$DB_USER_PASS",
- "port": "$DB_PORT",
- "ssl": false,
- "type": "Pg"
-}
-EOF
-
-cat <<-EOF >/opt/traffic_ops/app/db/dbconf.yml
-version: "1.0"
-name: dbconf.yml
-
-production:
- driver: postgres
- open: host=$DB_FQDN port=$DB_PORT user=$DB_USER password=$DB_USER_PASS dbname=$DB_NAME sslmode=disable
-test:
- driver: postgres
- open: host=$DB_FQDN port=$DB_PORT user=$DB_USER password=$DB_USER_PASS dbname=to_test sslmode=disable
-EOF
-
-cat <<-EOF >/opt/traffic_ops/app/conf/production/riak.conf
-{ "user": "$TV_RIAK_USER",
+ "MaxTLSVersion": "1.1",
"password": "$TV_RIAK_PASSWORD",
- "MaxTLSVersion": "1.1"
+ "user": "$TV_RIAK_USER"
}
-EOF
+RIAK_CONF
-cat <<-EOF >/opt/traffic_ops/app/conf/production/influx.conf
+<<INFLUX_CONF cat >/opt/traffic_ops/app/conf/production/influx.conf
{
- "user": "$INFLUXDB_ADMIN_USER",
- "password": "$INFLUXDB_ADMIN_PASSWORD",
- "secure": false
+ "password": "$INFLUXDB_ADMIN_PASSWORD",
+ "secure": false,
+ "user": "$INFLUXDB_ADMIN_USER"
}
-EOF
+INFLUX_CONF
+
+install_bin=/opt/traffic_ops/install/bin
+input_json="${install_bin}/input.json"
+echo "$(jq "$(<<JQ_FILTER cat
Review comment:
Using envsubst in 9c3d3f9ecf
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602445578
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -100,108 +92,103 @@ cat <<-EOF >/opt/traffic_ops/app/conf/cdn.conf
"log_location_info": "$TO_LOG_INFO",
"log_location_debug": "$TO_LOG_DEBUG",
"log_location_event": "$TO_LOG_EVENT",
- "max_db_connections": 20,
"db_conn_max_lifetime_seconds": ${DEBUGGING_TIMEOUT:-60},
- "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20},
- "backend_max_connections": {
- "mojolicious": 4
- },
- "whitelisted_oauth_urls": [],
- "oauth_client_secret": "",
- "routing_blacklist": {
- "ignore_unknown_routes": false,
- "disabled_routes": []
- },
- "supported_ds_metrics": [ "kbps", "tps_total", "tps_2xx", "tps_3xx", "tps_4xx", "tps_5xx" ]
- },
- "cors" : {
- "access_control_allow_origin" : "*"
+ "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20}
},
"to" : {
- "base_url" : "https://$TO_FQDN",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "no_account_found_msg" : "A Traffic Ops user account is required for access. Please contact your Traffic Ops user administrator."
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
"portal" : {
"base_url" : "https://$TP_HOST.$INFRA_SUBDOMAIN.$TLD_DOMAIN/#!/",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "pass_reset_path" : "user",
- "user_register_path" : "user"
- },
- "secrets" : [
- "$TO_SECRET"
- ],
- "geniso" : {
- "iso_root_path" : "/opt/traffic_ops/app/public"
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
- "inactivity_timeout" : 60,
"smtp" : {
"enabled" : true,
- "user" : "",
- "password" : "",
"address" : "${SMTP_FQDN}:${SMTP_PORT}"
},
"InfluxEnabled": true,
"influxdb_conf_path": "/opt/traffic_ops/app/conf/production/influx.conf",
"lets_encrypt" : {
- "user_email" : "",
- "send_expiration_email": false,
- "convert_self_signed": false,
- "renew_days_before_expiration": 30,
"environment": "staging"
- },
- "acme_renewal": {
- "summary_email": "",
- "renew_days_before_expiration": 30
- },
- "acme_accounts": [
- {
- "acme_provider" : "",
- "user_email" : "",
- "acme_url" : "",
- "kid" : "",
- "hmac_encoded" : ""
- }
- ]
Review comment:
Removed it again. The default `cdn.conf` includes these exact LE/ACME settings.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602461039
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/Dockerfile
##########
@@ -99,6 +115,8 @@ COPY infrastructure/cdn-in-a-box/enroller/server_template.json \
infrastructure/cdn-in-a-box/traffic_ops/trafficops-init.sh \
infrastructure/cdn-in-a-box/variables.env \
/
+COPY infrastructure/cdn-in-a-box/traffic_ops_data /traffic_ops_data
+COPY traffic_router/core/src/test/resources/geo/GeoLite2-City.mmdb.gz /opt/traffic_ops/app/public/
Review comment:
`COPY` instructions are fast and IMO should go at the end of the Dockerfile, but I don't have strong feelings about these 2 steps. Reverted those lines in 4bae66184d.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602455779
##########
File path: traffic_ops/install/bin/input.json
##########
@@ -1,179 +1,151 @@
{
- "/opt/traffic_ops/app/conf/production/database.conf":[
+ "/opt/traffic_ops/app/conf/cdn.conf": [
{
- "Database type":"Pg",
- "config_var":"type"
+ "Generate a new secret?": "yes",
+ "config_var": "genSecret"
},
{
- "Database name":"traffic_ops_db",
- "config_var":"dbname"
+ "Number of secrets to keep?": "10",
+ "config_var": "keepSecrets"
},
{
- "Database server hostname IP or FQDN":"localhost",
- "config_var":"hostname"
+ "Number of workers?": "12",
+ "config_var": "workers"
},
{
- "Database port number":"5432",
- "config_var":"port"
- },
- {
- "Traffic Ops database user":"traffic_ops",
- "config_var":"user"
- },
- {
- "Traffic Ops database password":"default",
- "config_var":"password",
- "hidden":"1"
+ "Traffic Ops url?": "https://[::]",
+ "config_var": "base_url"
}
],
- "/opt/traffic_ops/app/db/dbconf.yml":[
+ "/opt/traffic_ops/app/conf/ldap.conf": [
{
- "Database server root (admin) user":"root",
- "config_var":"dbAdminUser"
+ "Do you want to set up LDAP?": "no",
+ "config_var": "setupLdap"
},
{
- "Database server admin password":"default",
- "config_var":"dbAdminPw",
- "hidden":"1"
+ "LDAP server hostname": "",
+ "config_var": "hostname"
},
{
- "Download Maxmind Database?":"yes",
- "config_var":"maxmind"
- }
- ],
- "/opt/traffic_ops/app/conf/cdn.conf":[
- {
- "Generate a new secret?":"yes",
- "config_var":"genSecret"
+ "LDAP Admin DN": "",
+ "config_var": "admin_dn"
},
{
- "Number of secrets to keep?":"10",
- "config_var":"keepSecrets"
+ "LDAP Admin Password": "",
+ "config_var": "password",
+ "hidden": "1"
},
{
- "Port to serve on?":"443",
- "config_var":"port"
+ "LDAP Search Base": "",
+ "config_var": "search_base"
}
],
- "/opt/traffic_ops/app/conf/ldap.conf":[
+ "/opt/traffic_ops/app/conf/production/database.conf": [
{
- "Do you want to set up LDAP?":"no",
- "config_var":"setupLdap"
+ "Database type": "Pg",
+ "config_var": "type"
},
{
- "LDAP server hostname":"",
- "config_var":"hostname"
+ "Database name": "traffic_ops_db",
+ "config_var": "dbname"
},
{
- "LDAP Admin DN":"",
- "config_var":"admin_dn"
+ "Database server hostname IP or FQDN": "localhost",
+ "config_var": "hostname"
},
{
- "LDAP Admin Password":"",
- "config_var":"password",
- "hidden":"1"
+ "Database port number": "5432",
+ "config_var": "port"
},
{
- "LDAP Search Base":"",
- "config_var":"search_base"
+ "Traffic Ops database user": "dbuser",
+ "config_var": "user"
+ },
+ {
+ "Traffic Ops database password": "dbpass",
+ "config_var": "password",
+ "hidden": "1"
Review comment:
I'd rather not make breaking changes in this PR. This is supposed to be a drop-in-replacement right now, not a better-thought-out version with breaking changes.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602435734
##########
File path: traffic_ops/install/bin/_postinstall
##########
@@ -1,5 +1,5 @@
-#!/usr/bin/perl
-
+#!/usr/bin/env bash
Review comment:
I did use `git mv`
> Did you change this Python code at all?
Not in 83d69c3778, no. Running
```shell
git mv _postinstall _postinstall.pl;
git mv postinstall.py _postinstall;
```
yields
```shell
Changes to be committed:
(use "git restore --staged <file>..." to unstage)
modified: _postinstall
new file: _postinstall.pl
deleted: postinstall.py
```
, so I can see not wanting to do both steps in the same commit.
Rebased to split that commit into 2 `git mv` commits in ab87a7815e and 56644eb1d4 (though the overall diff will still be just as big).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602453564
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -100,108 +92,103 @@ cat <<-EOF >/opt/traffic_ops/app/conf/cdn.conf
"log_location_info": "$TO_LOG_INFO",
"log_location_debug": "$TO_LOG_DEBUG",
"log_location_event": "$TO_LOG_EVENT",
- "max_db_connections": 20,
"db_conn_max_lifetime_seconds": ${DEBUGGING_TIMEOUT:-60},
- "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20},
- "backend_max_connections": {
- "mojolicious": 4
- },
- "whitelisted_oauth_urls": [],
- "oauth_client_secret": "",
- "routing_blacklist": {
- "ignore_unknown_routes": false,
- "disabled_routes": []
- },
- "supported_ds_metrics": [ "kbps", "tps_total", "tps_2xx", "tps_3xx", "tps_4xx", "tps_5xx" ]
- },
- "cors" : {
- "access_control_allow_origin" : "*"
+ "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20}
},
"to" : {
- "base_url" : "https://$TO_FQDN",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "no_account_found_msg" : "A Traffic Ops user account is required for access. Please contact your Traffic Ops user administrator."
Review comment:
I keep `to.email_from`, but `to.no_account_found_msg` is provided by `cdn.conf` and `to.base_url` is generated properly from the postinstall.
Here is what that part of the config looks like after `postinstall` has run in CiaB:
```json
"to": {
"base_url": "https://trafficops.infra.ciab.test:443",
"email_from": "no-reply@infra.ciab.test",
"no_account_found_msg": "A Traffic Ops user account is required for access. Please contact your Traffic Ops user administrator."
},
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602481112
##########
File path: traffic_ops/install/bin/_postinstall
##########
@@ -1,5 +1,5 @@
-#!/usr/bin/perl
-
+#!/usr/bin/env bash
Review comment:
The number of commits doesn't bother me. I just wish the diff was handled cleaner by git itself.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602479562
##########
File path: traffic_ops/install/bin/input.json
##########
@@ -1,179 +1,151 @@
{
- "/opt/traffic_ops/app/conf/production/database.conf":[
+ "/opt/traffic_ops/app/conf/cdn.conf": [
{
- "Database type":"Pg",
- "config_var":"type"
+ "Generate a new secret?": "yes",
+ "config_var": "genSecret"
},
{
- "Database name":"traffic_ops_db",
- "config_var":"dbname"
+ "Number of secrets to keep?": "10",
+ "config_var": "keepSecrets"
},
{
- "Database server hostname IP or FQDN":"localhost",
- "config_var":"hostname"
+ "Number of workers?": "12",
+ "config_var": "workers"
},
{
- "Database port number":"5432",
- "config_var":"port"
- },
- {
- "Traffic Ops database user":"traffic_ops",
- "config_var":"user"
- },
- {
- "Traffic Ops database password":"default",
- "config_var":"password",
- "hidden":"1"
+ "Traffic Ops url?": "https://[::]",
+ "config_var": "base_url"
}
],
- "/opt/traffic_ops/app/db/dbconf.yml":[
+ "/opt/traffic_ops/app/conf/ldap.conf": [
{
- "Database server root (admin) user":"root",
- "config_var":"dbAdminUser"
+ "Do you want to set up LDAP?": "no",
+ "config_var": "setupLdap"
},
{
- "Database server admin password":"default",
- "config_var":"dbAdminPw",
- "hidden":"1"
+ "LDAP server hostname": "",
+ "config_var": "hostname"
},
{
- "Download Maxmind Database?":"yes",
- "config_var":"maxmind"
- }
- ],
- "/opt/traffic_ops/app/conf/cdn.conf":[
- {
- "Generate a new secret?":"yes",
- "config_var":"genSecret"
+ "LDAP Admin DN": "",
+ "config_var": "admin_dn"
},
{
- "Number of secrets to keep?":"10",
- "config_var":"keepSecrets"
+ "LDAP Admin Password": "",
+ "config_var": "password",
+ "hidden": "1"
},
{
- "Port to serve on?":"443",
- "config_var":"port"
+ "LDAP Search Base": "",
+ "config_var": "search_base"
}
],
- "/opt/traffic_ops/app/conf/ldap.conf":[
+ "/opt/traffic_ops/app/conf/production/database.conf": [
{
- "Do you want to set up LDAP?":"no",
- "config_var":"setupLdap"
+ "Database type": "Pg",
+ "config_var": "type"
},
{
- "LDAP server hostname":"",
- "config_var":"hostname"
+ "Database name": "traffic_ops_db",
+ "config_var": "dbname"
},
{
- "LDAP Admin DN":"",
- "config_var":"admin_dn"
+ "Database server hostname IP or FQDN": "localhost",
+ "config_var": "hostname"
},
{
- "LDAP Admin Password":"",
- "config_var":"password",
- "hidden":"1"
+ "Database port number": "5432",
+ "config_var": "port"
},
{
- "LDAP Search Base":"",
- "config_var":"search_base"
+ "Traffic Ops database user": "dbuser",
+ "config_var": "user"
+ },
+ {
+ "Traffic Ops database password": "dbpass",
+ "config_var": "password",
+ "hidden": "1"
Review comment:
I wasn't suggesting a breaking change, just a deprecation.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#issuecomment-808507297
As an aside, we could also add a GHA to run the tests when postinstall is updated. Doesn't need to go in this PR, but it might be helpful.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602441087
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -100,108 +92,103 @@ cat <<-EOF >/opt/traffic_ops/app/conf/cdn.conf
"log_location_info": "$TO_LOG_INFO",
"log_location_debug": "$TO_LOG_DEBUG",
"log_location_event": "$TO_LOG_EVENT",
- "max_db_connections": 20,
"db_conn_max_lifetime_seconds": ${DEBUGGING_TIMEOUT:-60},
- "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20},
- "backend_max_connections": {
- "mojolicious": 4
- },
- "whitelisted_oauth_urls": [],
- "oauth_client_secret": "",
- "routing_blacklist": {
- "ignore_unknown_routes": false,
- "disabled_routes": []
- },
- "supported_ds_metrics": [ "kbps", "tps_total", "tps_2xx", "tps_3xx", "tps_4xx", "tps_5xx" ]
- },
- "cors" : {
- "access_control_allow_origin" : "*"
+ "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20}
},
"to" : {
- "base_url" : "https://$TO_FQDN",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "no_account_found_msg" : "A Traffic Ops user account is required for access. Please contact your Traffic Ops user administrator."
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
"portal" : {
"base_url" : "https://$TP_HOST.$INFRA_SUBDOMAIN.$TLD_DOMAIN/#!/",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "pass_reset_path" : "user",
- "user_register_path" : "user"
- },
- "secrets" : [
- "$TO_SECRET"
- ],
- "geniso" : {
- "iso_root_path" : "/opt/traffic_ops/app/public"
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
- "inactivity_timeout" : 60,
"smtp" : {
"enabled" : true,
- "user" : "",
- "password" : "",
"address" : "${SMTP_FQDN}:${SMTP_PORT}"
},
"InfluxEnabled": true,
"influxdb_conf_path": "/opt/traffic_ops/app/conf/production/influx.conf",
"lets_encrypt" : {
- "user_email" : "",
- "send_expiration_email": false,
- "convert_self_signed": false,
- "renew_days_before_expiration": 30,
"environment": "staging"
- },
- "acme_renewal": {
- "summary_email": "",
- "renew_days_before_expiration": 30
- },
- "acme_accounts": [
- {
- "acme_provider" : "",
- "user_email" : "",
- "acme_url" : "",
- "kid" : "",
- "hmac_encoded" : ""
- }
- ]
Review comment:
Re-added LE/ACME things in baf2134eda
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602435734
##########
File path: traffic_ops/install/bin/_postinstall
##########
@@ -1,5 +1,5 @@
-#!/usr/bin/perl
-
+#!/usr/bin/env bash
Review comment:
I did use `git mv`,
> Did you change this Python code at all?
Not in 83d69c3778, no. Running
```shell
git mv _postinstall _postinstall.pl;
git mv postinstall.py _postinstall;
```
yields
```shell
Changes to be committed:
(use "git restore --staged <file>..." to unstage)
modified: _postinstall
new file: _postinstall.pl
deleted: postinstall.py
```
, so I can see not wanting to do both steps in the same commit.
Rebased to split that commit into 2 `git mv` commits in ab87a7815e and 56644eb1d4 (though the overall diff will still be just as big).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602397999
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/Dockerfile
##########
@@ -35,56 +35,72 @@ RUN set -o nounset -o errexit && \
if [[ "${RHEL_VERSION%%.*}" -eq 7 ]]; then \
use_repo=''; \
enable_repo=''; \
+ llvm_version=5.0; \
# needed for llvm-toolset-7-clang, which is needed for postgresql13-devel-13.2-1PGDG, required by TO rpm
dnf -y install gcc centos-release-scl-rh; \
else \
use_repo='--repo=pgdg13'; \
enable_repo='--enablerepo=powertools'; \
+ llvm_version=''; \
fi; \
dnf -y install "https://download.postgresql.org/pub/repos/yum/reporpms/EL-${RHEL_VERSION%%.*}-x86_64/pgdg-redhat-repo-latest.noarch.rpm"; \
- # libicu required by postgresql13
- dnf -y install libicu; \
- dnf -y $use_repo -- install postgresql13; \
dnf -y install epel-release; \
- dnf -y $enable_repo install \
+ dnf -y install \
+ # libicu is required by postgresql13
+ libicu \
+ # libicu-devel, clang-devel, and llvm-devel are required by postgresql13-devel
Review comment:
but why do we need postgresql13-devel?
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/Dockerfile
##########
@@ -99,6 +115,8 @@ COPY infrastructure/cdn-in-a-box/enroller/server_template.json \
infrastructure/cdn-in-a-box/traffic_ops/trafficops-init.sh \
infrastructure/cdn-in-a-box/variables.env \
/
+COPY infrastructure/cdn-in-a-box/traffic_ops_data /traffic_ops_data
+COPY traffic_router/core/src/test/resources/geo/GeoLite2-City.mmdb.gz /opt/traffic_ops/app/public/
Review comment:
Why move these steps? I think these change much, much less often than the RPM you want to use for CiaB, so putting them before the RPM addition and installation is more effective for caching build layers.
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -100,108 +92,103 @@ cat <<-EOF >/opt/traffic_ops/app/conf/cdn.conf
"log_location_info": "$TO_LOG_INFO",
"log_location_debug": "$TO_LOG_DEBUG",
"log_location_event": "$TO_LOG_EVENT",
- "max_db_connections": 20,
"db_conn_max_lifetime_seconds": ${DEBUGGING_TIMEOUT:-60},
- "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20},
- "backend_max_connections": {
- "mojolicious": 4
- },
- "whitelisted_oauth_urls": [],
- "oauth_client_secret": "",
- "routing_blacklist": {
- "ignore_unknown_routes": false,
- "disabled_routes": []
- },
- "supported_ds_metrics": [ "kbps", "tps_total", "tps_2xx", "tps_3xx", "tps_4xx", "tps_5xx" ]
Review comment:
Why get rid of all this? Particularly, without `supported_ds_metrics`, the Traffic Stats endpoints in the TO API for Delivery Service-based stats won't be able to return any data. I don't think `postinstall` adds quite all of these configuration options.
##########
File path: traffic_ops/install/bin/_postinstall
##########
@@ -1,5 +1,5 @@
-#!/usr/bin/perl
-
+#!/usr/bin/env bash
Review comment:
Did you change this Python code at all? I can't tell, the diff is so massive. I think if you use `git mv` to rename the files it'll make the diff cleaner, but I'm not sure since there was already a file with this name...
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -100,108 +92,103 @@ cat <<-EOF >/opt/traffic_ops/app/conf/cdn.conf
"log_location_info": "$TO_LOG_INFO",
"log_location_debug": "$TO_LOG_DEBUG",
"log_location_event": "$TO_LOG_EVENT",
- "max_db_connections": 20,
"db_conn_max_lifetime_seconds": ${DEBUGGING_TIMEOUT:-60},
- "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20},
- "backend_max_connections": {
- "mojolicious": 4
- },
- "whitelisted_oauth_urls": [],
- "oauth_client_secret": "",
- "routing_blacklist": {
- "ignore_unknown_routes": false,
- "disabled_routes": []
- },
- "supported_ds_metrics": [ "kbps", "tps_total", "tps_2xx", "tps_3xx", "tps_4xx", "tps_5xx" ]
- },
- "cors" : {
- "access_control_allow_origin" : "*"
+ "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20}
},
"to" : {
- "base_url" : "https://$TO_FQDN",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "no_account_found_msg" : "A Traffic Ops user account is required for access. Please contact your Traffic Ops user administrator."
Review comment:
Why get rid of these two things? Particularly, omitting `base_url` will cause a segfault in the password reset endpoint's handler. I don't believe `postinstall` will add that.
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -100,108 +92,103 @@ cat <<-EOF >/opt/traffic_ops/app/conf/cdn.conf
"log_location_info": "$TO_LOG_INFO",
"log_location_debug": "$TO_LOG_DEBUG",
"log_location_event": "$TO_LOG_EVENT",
- "max_db_connections": 20,
"db_conn_max_lifetime_seconds": ${DEBUGGING_TIMEOUT:-60},
- "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20},
- "backend_max_connections": {
- "mojolicious": 4
- },
- "whitelisted_oauth_urls": [],
- "oauth_client_secret": "",
- "routing_blacklist": {
- "ignore_unknown_routes": false,
- "disabled_routes": []
- },
- "supported_ds_metrics": [ "kbps", "tps_total", "tps_2xx", "tps_3xx", "tps_4xx", "tps_5xx" ]
- },
- "cors" : {
- "access_control_allow_origin" : "*"
+ "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20}
},
"to" : {
- "base_url" : "https://$TO_FQDN",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "no_account_found_msg" : "A Traffic Ops user account is required for access. Please contact your Traffic Ops user administrator."
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
"portal" : {
"base_url" : "https://$TP_HOST.$INFRA_SUBDOMAIN.$TLD_DOMAIN/#!/",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "pass_reset_path" : "user",
- "user_register_path" : "user"
- },
- "secrets" : [
- "$TO_SECRET"
- ],
- "geniso" : {
- "iso_root_path" : "/opt/traffic_ops/app/public"
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
- "inactivity_timeout" : 60,
"smtp" : {
"enabled" : true,
- "user" : "",
- "password" : "",
"address" : "${SMTP_FQDN}:${SMTP_PORT}"
},
"InfluxEnabled": true,
"influxdb_conf_path": "/opt/traffic_ops/app/conf/production/influx.conf",
"lets_encrypt" : {
- "user_email" : "",
- "send_expiration_email": false,
- "convert_self_signed": false,
- "renew_days_before_expiration": 30,
"environment": "staging"
- },
- "acme_renewal": {
- "summary_email": "",
- "renew_days_before_expiration": 30
- },
- "acme_accounts": [
- {
- "acme_provider" : "",
- "user_email" : "",
- "acme_url" : "",
- "kid" : "",
- "hmac_encoded" : ""
- }
- ]
Review comment:
I don't think `postinstall` will add LE/ACME things.
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -100,108 +92,103 @@ cat <<-EOF >/opt/traffic_ops/app/conf/cdn.conf
"log_location_info": "$TO_LOG_INFO",
"log_location_debug": "$TO_LOG_DEBUG",
"log_location_event": "$TO_LOG_EVENT",
- "max_db_connections": 20,
"db_conn_max_lifetime_seconds": ${DEBUGGING_TIMEOUT:-60},
- "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20},
- "backend_max_connections": {
- "mojolicious": 4
- },
- "whitelisted_oauth_urls": [],
- "oauth_client_secret": "",
- "routing_blacklist": {
- "ignore_unknown_routes": false,
- "disabled_routes": []
- },
- "supported_ds_metrics": [ "kbps", "tps_total", "tps_2xx", "tps_3xx", "tps_4xx", "tps_5xx" ]
- },
- "cors" : {
- "access_control_allow_origin" : "*"
+ "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20}
},
"to" : {
- "base_url" : "https://$TO_FQDN",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "no_account_found_msg" : "A Traffic Ops user account is required for access. Please contact your Traffic Ops user administrator."
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
"portal" : {
"base_url" : "https://$TP_HOST.$INFRA_SUBDOMAIN.$TLD_DOMAIN/#!/",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "pass_reset_path" : "user",
- "user_register_path" : "user"
- },
- "secrets" : [
- "$TO_SECRET"
- ],
- "geniso" : {
- "iso_root_path" : "/opt/traffic_ops/app/public"
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
- "inactivity_timeout" : 60,
"smtp" : {
"enabled" : true,
- "user" : "",
- "password" : "",
"address" : "${SMTP_FQDN}:${SMTP_PORT}"
},
"InfluxEnabled": true,
"influxdb_conf_path": "/opt/traffic_ops/app/conf/production/influx.conf",
"lets_encrypt" : {
- "user_email" : "",
- "send_expiration_email": false,
- "convert_self_signed": false,
- "renew_days_before_expiration": 30,
"environment": "staging"
- },
- "acme_renewal": {
- "summary_email": "",
- "renew_days_before_expiration": 30
- },
- "acme_accounts": [
- {
- "acme_provider" : "",
- "user_email" : "",
- "acme_url" : "",
- "kid" : "",
- "hmac_encoded" : ""
- }
- ]
+ }
}
EOF
+))"
-cat <<-EOF >/opt/traffic_ops/app/conf/production/database.conf
+<<RIAK_CONF cat >/opt/traffic_ops/app/conf/production/riak.conf
{
- "description": "Local PostgreSQL database on port 5432",
- "dbname": "$DB_NAME",
- "hostname": "$DB_FQDN",
- "user": "$DB_USER",
- "password": "$DB_USER_PASS",
- "port": "$DB_PORT",
- "ssl": false,
- "type": "Pg"
-}
-EOF
-
-cat <<-EOF >/opt/traffic_ops/app/db/dbconf.yml
-version: "1.0"
-name: dbconf.yml
-
-production:
- driver: postgres
- open: host=$DB_FQDN port=$DB_PORT user=$DB_USER password=$DB_USER_PASS dbname=$DB_NAME sslmode=disable
-test:
- driver: postgres
- open: host=$DB_FQDN port=$DB_PORT user=$DB_USER password=$DB_USER_PASS dbname=to_test sslmode=disable
-EOF
-
-cat <<-EOF >/opt/traffic_ops/app/conf/production/riak.conf
-{ "user": "$TV_RIAK_USER",
+ "MaxTLSVersion": "1.1",
"password": "$TV_RIAK_PASSWORD",
- "MaxTLSVersion": "1.1"
+ "user": "$TV_RIAK_USER"
}
-EOF
+RIAK_CONF
-cat <<-EOF >/opt/traffic_ops/app/conf/production/influx.conf
+<<INFLUX_CONF cat >/opt/traffic_ops/app/conf/production/influx.conf
{
- "user": "$INFLUXDB_ADMIN_USER",
- "password": "$INFLUXDB_ADMIN_PASSWORD",
- "secure": false
+ "password": "$INFLUXDB_ADMIN_PASSWORD",
+ "secure": false,
+ "user": "$INFLUXDB_ADMIN_USER"
}
-EOF
+INFLUX_CONF
+
+install_bin=/opt/traffic_ops/install/bin
+input_json="${install_bin}/input.json"
+echo "$(jq "$(<<JQ_FILTER cat
Review comment:
I feel like `envsubst` would be a lot easier to follow; probably using a new JSON input file specifically for CiaB.
##########
File path: traffic_ops/install/bin/input.json
##########
@@ -1,179 +1,151 @@
{
- "/opt/traffic_ops/app/conf/production/database.conf":[
+ "/opt/traffic_ops/app/conf/cdn.conf": [
{
- "Database type":"Pg",
- "config_var":"type"
+ "Generate a new secret?": "yes",
+ "config_var": "genSecret"
},
{
- "Database name":"traffic_ops_db",
- "config_var":"dbname"
+ "Number of secrets to keep?": "10",
+ "config_var": "keepSecrets"
},
{
- "Database server hostname IP or FQDN":"localhost",
- "config_var":"hostname"
+ "Number of workers?": "12",
+ "config_var": "workers"
},
{
- "Database port number":"5432",
- "config_var":"port"
- },
- {
- "Traffic Ops database user":"traffic_ops",
- "config_var":"user"
- },
- {
- "Traffic Ops database password":"default",
- "config_var":"password",
- "hidden":"1"
+ "Traffic Ops url?": "https://[::]",
+ "config_var": "base_url"
}
],
- "/opt/traffic_ops/app/db/dbconf.yml":[
+ "/opt/traffic_ops/app/conf/ldap.conf": [
{
- "Database server root (admin) user":"root",
- "config_var":"dbAdminUser"
+ "Do you want to set up LDAP?": "no",
+ "config_var": "setupLdap"
},
{
- "Database server admin password":"default",
- "config_var":"dbAdminPw",
- "hidden":"1"
+ "LDAP server hostname": "",
+ "config_var": "hostname"
},
{
- "Download Maxmind Database?":"yes",
- "config_var":"maxmind"
- }
- ],
- "/opt/traffic_ops/app/conf/cdn.conf":[
- {
- "Generate a new secret?":"yes",
- "config_var":"genSecret"
+ "LDAP Admin DN": "",
+ "config_var": "admin_dn"
},
{
- "Number of secrets to keep?":"10",
- "config_var":"keepSecrets"
+ "LDAP Admin Password": "",
+ "config_var": "password",
+ "hidden": "1"
},
{
- "Port to serve on?":"443",
- "config_var":"port"
+ "LDAP Search Base": "",
+ "config_var": "search_base"
}
],
- "/opt/traffic_ops/app/conf/ldap.conf":[
+ "/opt/traffic_ops/app/conf/production/database.conf": [
{
- "Do you want to set up LDAP?":"no",
- "config_var":"setupLdap"
+ "Database type": "Pg",
+ "config_var": "type"
},
{
- "LDAP server hostname":"",
- "config_var":"hostname"
+ "Database name": "traffic_ops_db",
+ "config_var": "dbname"
},
{
- "LDAP Admin DN":"",
- "config_var":"admin_dn"
+ "Database server hostname IP or FQDN": "localhost",
+ "config_var": "hostname"
},
{
- "LDAP Admin Password":"",
- "config_var":"password",
- "hidden":"1"
+ "Database port number": "5432",
+ "config_var": "port"
},
{
- "LDAP Search Base":"",
- "config_var":"search_base"
+ "Traffic Ops database user": "dbuser",
+ "config_var": "user"
+ },
+ {
+ "Traffic Ops database password": "dbpass",
+ "config_var": "password",
+ "hidden": "1"
Review comment:
Can we add something to the deprecation? The Perl version of `_postinstall` only supports `hidden` properties as strings containing an integer encoding a boolean concept (I think?), but in Python it'll happily work with just booleans. And they really ought to be boolean. I've got other complaints about the format of these input JSON files, but that's the big one.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602435734
##########
File path: traffic_ops/install/bin/_postinstall
##########
@@ -1,5 +1,5 @@
-#!/usr/bin/perl
-
+#!/usr/bin/env bash
Review comment:
I did use `git mv`
> Did you change this Python code at all?
Not in 83d69c3778, no. Running
```shell
git mv _postinstall _postinstall.pl;
git mv postinstall.py _postinstall;
```
yields
```shell
Changes to be committed:
(use "git restore --staged <file>..." to unstage)
modified: _postinstall
new file: _postinstall.pl
deleted: postinstall.py
```
, so I can understanding not wanting both steps to be done in the same commit.
Rebased to split that commit into 2 `git mv` commits in ab87a7815e and 56644eb1d4 (though the overall diff will still be just as big).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602426030
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/Dockerfile
##########
@@ -35,56 +35,72 @@ RUN set -o nounset -o errexit && \
if [[ "${RHEL_VERSION%%.*}" -eq 7 ]]; then \
use_repo=''; \
enable_repo=''; \
+ llvm_version=5.0; \
# needed for llvm-toolset-7-clang, which is needed for postgresql13-devel-13.2-1PGDG, required by TO rpm
dnf -y install gcc centos-release-scl-rh; \
else \
use_repo='--repo=pgdg13'; \
enable_repo='--enablerepo=powertools'; \
+ llvm_version=''; \
fi; \
dnf -y install "https://download.postgresql.org/pub/repos/yum/reporpms/EL-${RHEL_VERSION%%.*}-x86_64/pgdg-redhat-repo-latest.noarch.rpm"; \
- # libicu required by postgresql13
- dnf -y install libicu; \
- dnf -y $use_repo -- install postgresql13; \
dnf -y install epel-release; \
- dnf -y $enable_repo install \
+ dnf -y install \
+ # libicu is required by postgresql13
+ libicu \
+ # libicu-devel, clang-devel, and llvm-devel are required by postgresql13-devel
Review comment:
The spec requires it:
https://github.com/apache/trafficcontrol/blob/95db4e8ef65c22f6af17aa6f63733ef80322a02f/traffic_ops/build/traffic_ops.spec#L39
Maybe we can remove that dependency in a future PR.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
ocket8888 merged pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#issuecomment-808570169
> The script itself seems to work well enough as a drop-in replacement for Perl, but the renaming seems to have broken the tests:
Fixed in 3c3c5b4e36
> As an aside, we could also add a GHA to run the tests when postinstall is updated. Doesn't need to go in this PR, but it might be helpful.
GHA added in 8b2740d54b
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602441087
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -100,108 +92,103 @@ cat <<-EOF >/opt/traffic_ops/app/conf/cdn.conf
"log_location_info": "$TO_LOG_INFO",
"log_location_debug": "$TO_LOG_DEBUG",
"log_location_event": "$TO_LOG_EVENT",
- "max_db_connections": 20,
"db_conn_max_lifetime_seconds": ${DEBUGGING_TIMEOUT:-60},
- "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20},
- "backend_max_connections": {
- "mojolicious": 4
- },
- "whitelisted_oauth_urls": [],
- "oauth_client_secret": "",
- "routing_blacklist": {
- "ignore_unknown_routes": false,
- "disabled_routes": []
- },
- "supported_ds_metrics": [ "kbps", "tps_total", "tps_2xx", "tps_3xx", "tps_4xx", "tps_5xx" ]
- },
- "cors" : {
- "access_control_allow_origin" : "*"
+ "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20}
},
"to" : {
- "base_url" : "https://$TO_FQDN",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "no_account_found_msg" : "A Traffic Ops user account is required for access. Please contact your Traffic Ops user administrator."
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
"portal" : {
"base_url" : "https://$TP_HOST.$INFRA_SUBDOMAIN.$TLD_DOMAIN/#!/",
- "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN",
- "pass_reset_path" : "user",
- "user_register_path" : "user"
- },
- "secrets" : [
- "$TO_SECRET"
- ],
- "geniso" : {
- "iso_root_path" : "/opt/traffic_ops/app/public"
+ "email_from" : "no-reply@$INFRA_SUBDOMAIN.$TLD_DOMAIN"
},
- "inactivity_timeout" : 60,
"smtp" : {
"enabled" : true,
- "user" : "",
- "password" : "",
"address" : "${SMTP_FQDN}:${SMTP_PORT}"
},
"InfluxEnabled": true,
"influxdb_conf_path": "/opt/traffic_ops/app/conf/production/influx.conf",
"lets_encrypt" : {
- "user_email" : "",
- "send_expiration_email": false,
- "convert_self_signed": false,
- "renew_days_before_expiration": 30,
"environment": "staging"
- },
- "acme_renewal": {
- "summary_email": "",
- "renew_days_before_expiration": 30
- },
- "acme_accounts": [
- {
- "acme_provider" : "",
- "user_email" : "",
- "acme_url" : "",
- "kid" : "",
- "hmac_encoded" : ""
- }
- ]
Review comment:
~Re-added LE/ACME things in baf2134eda~
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5681: Use the Python Postinstall implementation by default
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5681:
URL: https://github.com/apache/trafficcontrol/pull/5681#discussion_r602445052
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -100,108 +92,103 @@ cat <<-EOF >/opt/traffic_ops/app/conf/cdn.conf
"log_location_info": "$TO_LOG_INFO",
"log_location_debug": "$TO_LOG_DEBUG",
"log_location_event": "$TO_LOG_EVENT",
- "max_db_connections": 20,
"db_conn_max_lifetime_seconds": ${DEBUGGING_TIMEOUT:-60},
- "db_query_timeout_seconds": ${DEBUGGING_TIMEOUT:-20},
- "backend_max_connections": {
- "mojolicious": 4
- },
- "whitelisted_oauth_urls": [],
- "oauth_client_secret": "",
- "routing_blacklist": {
- "ignore_unknown_routes": false,
- "disabled_routes": []
- },
- "supported_ds_metrics": [ "kbps", "tps_total", "tps_2xx", "tps_3xx", "tps_4xx", "tps_5xx" ]
Review comment:
The default `cdn.conf` includes all of this already.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org