Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Fix atscfg meta to omit config files for nonexistent delivery services#4256

Merged
rawlinp merged 2 commits into
apache:masterfrom
rob05c:atscfg-omit-meta-wrong-dses
Jan 9, 2020
Merged

Fix atscfg meta to omit config files for nonexistent delivery services#4256
rawlinp merged 2 commits into
apache:masterfrom
rob05c:atscfg-omit-meta-wrong-dses

Conversation

@rob05c

@rob05c rob05c commented Jan 4, 2020

Copy link
Copy Markdown
Member

This fixes atscfg to omit files with location parameters for config
files, hdr_rw_, regex_remap_, url_sig_, and uri_signing_ for DSes
which don't exist (typically after deleting a DS but forgetting to
delete the corresponding location Parameters).

This is important to make atstccfg work because without it, ORT will
request url_sig_ files for DSes which don't exist, and the API
endpoint to get the keys will fail. Because atstccfg uses the API,
there's no reasonable way to make it work, except to fix the meta
config to omit those nonexistent files.

In addition to nonexistent files, this also omits files for DSes
not assigned to the server. There won't even be remap lines for
those DSes anyway, so it's better and safer to omit the ancillary
files altogether.

Tagged major, because it causes bad data to break ORT, where it previously didn't and doesn't need to.

Includes tests.
Includes changelog.
No documentation, bug fix.

  • This PR is not related to any other Issue

Which Traffic Control components are affected by this PR?

  • Traffic Ops
  • Traffic Ops ORT

What is the best way to verify this PR?

Run tests. Request meta config /api/1.4/servers/{id}/configfiles/ats on a TO with location parameters for a nonexistent DS (the odds are good your Production environment has some), and verify those files are no longer in the returned JSON.

If this is a bug fix, what versions of Traffic Control are affected?

  • 4.0.x RC0

The following criteria are ALL met by this PR

  • This PR includes tests OR I have explained why tests are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • This PR includes an update to CHANGELOG.md OR such an update is not necessary
  • This PR includes any and all required license headers
  • This PR ensures that database migration sequence is correct OR this PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Additional Information

@rob05c rob05c added bug something isn't working as intended high impact impacts the basic function, deployment, or operation of a CDN cache-config Cache config generation labels Jan 4, 2020
This fixes atscfg to omit files with location parameters for config
files, hdr_rw_, regex_remap_, url_sig_, and uri_signing_ for DSes
which don't exist (typically after deleting a DS but forgetting to
delete the corresponding location Parameters).

This is important to make atstccfg work because without it, ORT will
request url_sig_ files for DSes which don't exist, and the API
endpoint to get the keys will fail. Because atstccfg uses the API,
there's no reasonable way to make it work, except to fix the meta
config to omit those nonexistent files.

In addition to nonexistent files, this also omits files for DSes
not assigned to the server. There won't even be remap lines for
those DSes anyway, so it's better and safer to omit the ancillary
files altogether.
@rob05c
rob05c force-pushed the atscfg-omit-meta-wrong-dses branch from d93b6cf to 37760cd Compare January 4, 2020 00:50

@rawlinp rawlinp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests pass, but getting this TO API test failure in this PR but not the commit before it:

=== RUN   TestATSConfigMeta
--- FAIL: TestATSConfigMeta (2.13s)
    atsconfig_meta_test.go:67: Getting server 'atlanta-edge-01' config list: expected: hdr_rw_mid_anymap-ds.config actual: not found

@rob05c

rob05c commented Jan 8, 2020

Copy link
Copy Markdown
Member Author

Ah, looks like the test isn't assigning the DS it's testing to that server. Thanks for finding that! I'll fix.

@rob05c

rob05c commented Jan 8, 2020

Copy link
Copy Markdown
Member Author

Ok, I fixed the API test. API tests all pass for me (after fixing Perl number-string issues), unit tests pass.

@rob05c
rob05c requested a review from rawlinp January 8, 2020 21:14

@rawlinp rawlinp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UTs and TO API tests pass, verified that a hdr_rw location param for a DS that didn't exist was omitted from the config meta API result as expected, observed that the new warnings were logged for those params.

@rawlinp
rawlinp merged commit ea315f0 into apache:master Jan 9, 2020
rob05c added a commit to rob05c/trafficcontrol that referenced this pull request Jan 9, 2020
apache#4256)

* Fix atscfg meta to omit nonexistent ds files

This fixes atscfg to omit files with location parameters for config
files, hdr_rw_, regex_remap_, url_sig_, and uri_signing_ for DSes
which don't exist (typically after deleting a DS but forgetting to
delete the corresponding location Parameters).

This is important to make atstccfg work because without it, ORT will
request url_sig_ files for DSes which don't exist, and the API
endpoint to get the keys will fail. Because atstccfg uses the API,
there's no reasonable way to make it work, except to fix the meta
config to omit those nonexistent files.

In addition to nonexistent files, this also omits files for DSes
not assigned to the server. There won't even be remap lines for
those DSes anyway, so it's better and safer to omit the ancillary
files altogether.

* Fix TO API test for meta cfg to set DSS, real DS

(cherry picked from commit ea315f0)
rawlinp pushed a commit that referenced this pull request Jan 9, 2020
#4256) (#4278)

* Fix atscfg meta to omit nonexistent ds files

This fixes atscfg to omit files with location parameters for config
files, hdr_rw_, regex_remap_, url_sig_, and uri_signing_ for DSes
which don't exist (typically after deleting a DS but forgetting to
delete the corresponding location Parameters).

This is important to make atstccfg work because without it, ORT will
request url_sig_ files for DSes which don't exist, and the API
endpoint to get the keys will fail. Because atstccfg uses the API,
there's no reasonable way to make it work, except to fix the meta
config to omit those nonexistent files.

In addition to nonexistent files, this also omits files for DSes
not assigned to the server. There won't even be remap lines for
those DSes anyway, so it's better and safer to omit the ancillary
files altogether.

* Fix TO API test for meta cfg to set DSS, real DS

(cherry picked from commit ea315f0)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug something isn't working as intended cache-config Cache config generation high impact impacts the basic function, deployment, or operation of a CDN

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants