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

traffic_ops_ort: fix for syncds having too many header rewrite false …#4343

Merged
rob05c merged 2 commits intoapache:masterfrom
traeak:ort_comment_strip
Jan 30, 2020
Merged

traffic_ops_ort: fix for syncds having too many header rewrite false …#4343
rob05c merged 2 commits intoapache:masterfrom
traeak:ort_comment_strip

Conversation

@traeak
Copy link
Copy Markdown
Contributor

@traeak traeak commented Jan 27, 2020

…positives

What does this PR (Pull Request) do?
This PR fixes an issue when canned comments aren't consistently stripped for header_rewrites, logs_xml.config or *.cer files. Also promotes File changed message from DEBUG to ERROR for report mode.

  • This PR is not related to any Issue

Which Traffic Control components are affected by this PR?

  • Traffic Ops ORT perl

What is the best way to verify this PR?

Set up a test box with traffic_ops_ort. Badass an initial EDGE configuration with some DS's that contain EDGE header rewrite rules.

First test: Queue CDN. Run syncds, WARN mode. Old version the header_rewrite header file will trigger false positives. New version should not. Edit a DS's header rewrite rule, queue, run syncds. Only the edited header rewrite rule should update.

Next test: Edit header_rewrite for DS and queue. Run report mode with WARN mode. Ensure the changed accompanying header rewrite and only that one shows up as an ERROR message.

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

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

@rawlinp rawlinp added bug something isn't working as intended Traffic Ops ORT *DEPRECATED* related to the traffic_ops_ort.pl script labels Jan 27, 2020
@traeak traeak marked this pull request as ready for review January 28, 2020 19:53
@rob05c rob05c self-assigned this Jan 30, 2020
Copy link
Copy Markdown
Contributor

@alficles alficles left a comment

Choose a reason for hiding this comment

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

LGTM. Does what it says on the tin.

Copy link
Copy Markdown
Member

@rob05c rob05c left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

I tested running ORT, this fixes the false positives on header rewrites problem, syncds didn't update files that hadn't changed. I also made changes to header rewrite and other configs, and verified real changes still get placed properly.

The code also looks good to me (though I'm not a Perl Wizard).

@rob05c rob05c merged commit 7ee3ffa into apache:master Jan 30, 2020
rawlinp pushed a commit to rawlinp/trafficcontrol that referenced this pull request Jan 31, 2020
apache#4343)

* traffic_ops_ort: fix for syncds having too many header rewrite false positives

* traffic_ops_ort.pl: allow all files with changes to run through the diff_file_lines function

(cherry picked from commit 7ee3ffa)
rawlinp added a commit that referenced this pull request Jan 31, 2020
#4343) (#4362)

* traffic_ops_ort: fix for syncds having too many header rewrite false positives

* traffic_ops_ort.pl: allow all files with changes to run through the diff_file_lines function

(cherry picked from commit 7ee3ffa)

Co-authored-by: Brian Olsen <bnolsen@gmail.com>
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 Traffic Ops ORT *DEPRECATED* related to the traffic_ops_ort.pl script

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants