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

EDNS0 Client Subnet for Delivery Services#4198

Merged
rawlinp merged 6 commits into
apache:masterfrom
mattjackson220:ecs_ds
Jan 6, 2020
Merged

EDNS0 Client Subnet for Delivery Services#4198
rawlinp merged 6 commits into
apache:masterfrom
mattjackson220:ecs_ds

Conversation

@mattjackson220

@mattjackson220 mattjackson220 commented Dec 11, 2019

Copy link
Copy Markdown
Contributor

What does this PR (Pull Request) do?

This PR adds the ability to enable EDNS0 client subnet on the delivery service level. Currently, this can be enabled on the CDN level, and this PR still supports that functionality but allows for enabling it per delivery service as well.

Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Ops
  • Traffic Portal
  • Traffic Router

What is the best way to verify this PR?

  • Verify that the Delivery Services page in Traffic Portal has been updated in the Routing section to include a new boolean value for EcsEnabled.

  • Verify that updating this value gets updated in the TO database.

  • Verify that the API delivery services endpoints work as expected.

  • In a public facing environment, update a DNS Delivery Service to be Ecs Enabled. Perform a snapshot.

  • Run the following commands replacing yourDeliverySevice with your delivery service edge location (ex: edge.example.com), yourSubnet and yourSubnet2 with a valid subnets from 2 far apart locations (can be found from /opt/traffic_router/db/czmap.json):
    dig +trace @8.8.8.8 yourDeliverySevice +dnssec ALL
    dig +trace @8.8.8.8 yourDeliverySevice +dnssec +subnet=yourSubnet ALL
    dig +trace @8.8.8.8 yourDeliverySevice +dnssec +subnet=yourSubnet2 ALL

  • In Traffic Router (could be in any of the active Traffic Routers) view the access log and find the entry for your requests.

  • Verify that the first request has "chi=anIPAddress rhi=-" and that ans= list of IPs of the edge cache/s closest to YOU.

  • Verify that the second request has "chi=anIPAddressInYourSubnet rhi=IPAddressFromFirstRequest" and that ans = list of IPs of the edge cache/s closest to the subnet location.

  • Verify that the third request has "chi=anIPAddressInYourSubnet2 rhi=IPAddressFromFirstRequest" and that ans = list of IPs of the edge cache/s closest to the subnet2 location.

  • Follow steps again for a different DNS Delivery Service but leave Ecs Disabled and verify that all requests get "chi=anIPAddress rhi=-" and that ans= list of IPs of the edge cache/s closest to YOU.

The following criteria are ALL met by this PR

  • This PR includes tests
  • This PR includes documentation
  • This PR includes an update to CHANGELOG.md
  • 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

@asf-ci

asf-ci commented Dec 11, 2019

Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@rawlinp

rawlinp commented Dec 11, 2019

Copy link
Copy Markdown
Contributor

add to whitelist

@rawlinp rawlinp added documentation related to documentation new feature A new feature, capability or behavior Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1 Traffic Router related to Traffic Router labels Dec 11, 2019
@asf-ci

asf-ci commented Dec 11, 2019

Copy link
Copy Markdown
Contributor

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4911/

@asf-ci

asf-ci commented Dec 18, 2019

Copy link
Copy Markdown
Contributor

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4958/

@asf-ci

asf-ci commented Dec 18, 2019

Copy link
Copy Markdown
Contributor

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4959/

@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.

Code looks good, but I think we have to add a new minor version for the DS API now that 4.0.0-RCO is out. Hopefully the steps I commented are easy to follow.

Comment thread docs/source/api/deliveryservices.rst Outdated
Comment thread lib/go-tc/deliveryservices.go Outdated
Comment thread traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go Outdated

@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.

looks good! just a minor changelog issue

Comment thread CHANGELOG.md Outdated
@rawlinp

rawlinp commented Jan 3, 2020

Copy link
Copy Markdown
Contributor

Also, I'm not sure if you want to do this as a follow-up PR or in this one, but in order for TP to work with the new DS field, it needs to be pointed to 1.5 now -- which I'm pretty sure is this:

traffic_portal/app/src/scripts/config.js:.constant('ENV', { api: { root:'/api/1.4/' } })

With TP making 1.5 requests, I think TO-Perl also needs updated to have a 1.5 section so that any remaining Perl routes will be found when requesting 1.5, like there are for 1.4:

TrafficOpsRoutes.pm: # 1.4 Routes
TrafficOpsRoutes.pm:    $version = "1.4";

@mattjackson220

mattjackson220 commented Jan 3, 2020

Copy link
Copy Markdown
Contributor Author

Also, I'm not sure if you want to do this as a follow-up PR or in this one, but in order for TP to work with the new DS field, it needs to be pointed to 1.5 now -- which I'm pretty sure is this:

traffic_portal/app/src/scripts/config.js:.constant('ENV', { api: { root:'/api/1.4/' } })

With TP making 1.5 requests, I think TO-Perl also needs updated to have a 1.5 section so that any remaining Perl routes will be found when requesting 1.5, like there are for 1.4:

TrafficOpsRoutes.pm: # 1.4 Routes
TrafficOpsRoutes.pm:    $version = "1.4";

I'll open a new PR for this so it isn't tied up with this one. Here's the new one: PR #4254

@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.

A little bit of unit test failure fallout from updating the minor version:

# github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/deliveryservice/request [github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/deliveryservice/request.test]
traffic_ops/traffic_ops_golang/deliveryservice/request/requests_test.go:65:4: cannot use promoted field DeliveryServiceNullableV14.DeliveryServiceNullableV13 in struct literal of type tc.DeliveryServiceNullable
--- FAIL: TestMakeDSes (0.00s)
    deliveryservice_test.go:258: makeDSes expected: nil error, actual: scanning deliveryservice: sql: expected 28 destination arguments in Scan, not 29
FAIL
FAIL	github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/crconfig	0.043s
FAIL	github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/deliveryservice/request [build failed]

But the TO API tests pass so 🎉

@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.

Manually tested the snapshot + TR functionality, as well as the DS 1.4 vs 1.5 functionality, and everything worked as expected. 🎉

@rawlinp
rawlinp merged commit 9df084d into apache:master Jan 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation related to documentation new feature A new feature, capability or behavior Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1 Traffic Router related to Traffic Router

Projects

None yet

Development

Successfully merging this pull request may close these issues.

eDNS0 client subnet extension support by DS

4 participants