search

apache / trafficcontrol

Apache Traffic Control is an Open Source implementation of a Content Delivery Network
https://trafficcontrol.apache.org/
Apache License 2.0
1.09k stars 344 forks source link

Traffic Ops `db/admin` tool depends on passwordless DB connection as `root` user #7202

Open zrhoffman opened 2 years ago

zrhoffman commented 2 years ago

This Improvement request (usability, performance, tech debt, etc.) affects these Traffic Control components:

Current behavior:

The Traffic Ops db/admin tool relies on the postgres user requiring no password when connecting as root.

https://github.com/apache/trafficcontrol/blob/02b9f0444cdee24dd14c0be852b276415d5468c0/traffic_ops/app/db/admin.go#L307

7142, which made Traffic Ops run as a non-root user, set PGPASSWORD for the entire binary, which worked for Dev CDN in a Box because the password in dbconf.yml, which this strategy set PGPASSWORD to,

https://github.com/apache/trafficcontrol/blob/02b9f0444cdee24dd14c0be852b276415d5468c0/dev/traffic_ops/dbconf.yml#L21

happened to be "twelve12", the same password set for the postgres user of the Postgres server.

https://github.com/apache/trafficcontrol/blob/02b9f0444cdee24dd14c0be852b276415d5468c0/docker-compose.yml#L46

However, once #7142 was merged, the Cache Config integration tests started failing, because its postgres user password

https://github.com/apache/trafficcontrol/blob/02b9f0444cdee24dd14c0be852b276415d5468c0/cache-config/testing/docker/variables.env#L31

is different than its traffic_ops password (which ends up in dbconf.yml).

https://github.com/apache/trafficcontrol/blob/02b9f0444cdee24dd14c0be852b276415d5468c0/cache-config/testing/docker/variables.env#L35-L36

https://github.com/apache/trafficcontrol/blob/02b9f0444cdee24dd14c0be852b276415d5468c0/cache-config/testing/docker/traffic_ops/run.sh#L98

We reverted the change to db/admin from #7142 in #7198 to make the Cache Config integration tests pass again without knowing, at the time, why that change made them fail.

As a side note, finding the reason the Cache Config integration tests were failing was not straightforward because the errors go only to a file that is not printed to the to_server container's output anywhere.

https://github.com/apache/trafficcontrol/blob/02b9f0444cdee24dd14c0be852b276415d5468c0/cache-config/testing/docker/traffic_ops/run.sh#L144-L145

New behavior:

ocket8888 commented 2 years ago

Is

Traffic Ops db/admin tool depends on passwordless DB connection as root user

strictly true, or is it the case that db/admin depends on the calling user to be the root user?

zrhoffman commented 2 years ago

Not sure what the difference between strictly true and db/admin depends on the calling user to be the root user is, but they both seem true to me.

Notice how no password is set for the psql command (and PGPASSWORD is not set).

https://github.com/apache/trafficcontrol/blob/02b9f0444cdee24dd14c0be852b276415d5468c0/traffic_ops/app/db/admin.go#L307

ocket8888 commented 2 years ago

If the title is strictly true, then the tool doesn't depend on passwordless DB connections when not run as the root user

zrhoffman commented 2 years ago

It sounds like you know what I mean, feel free to change the title to whatever you feel is accurate

ocket8888 commented 2 years ago

I do not. If it's true that db/admin requires you to run it as the root user, that's also an issue IMO.

zrhoffman commented 2 years ago

If the title is strictly true, then the tool doesn't depend on passwordless DB connections when not run as the root user

The tool didn't depend on passwordless DB connections before the revert in #7198 was merged.

If it's true that db/admin requires you to run it as the root user, that's also an issue IMO.

That's not also an issue, it's the issue

ocket8888 commented 2 years ago

idk the way I see it there are two issues

zrhoffman commented 2 years ago

Got it. I'm not sure how to describe one of those 2 points without describing the other, since both cases are related to Postgres default installations, but if you think they should be separated into 2 2 issues, I'll create another one.

ocket8888 commented 2 years ago

The number of issues isn't important to me, just want to be sure both are fixed.