You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by dg4prez <gi...@git.apache.org> on 2017/05/23 20:24:37 UTC
[GitHub] incubator-trafficcontrol pull request #605: Add CDN match checks when assign...
GitHub user dg4prez opened a pull request:
https://github.com/apache/incubator-trafficcontrol/pull/605
Add CDN match checks when assigning or editing profiles
Adds CDN mismatch/null checks to changing server profile in the UI and API. Also checks for a mismatch against the servers with a particular profile assigned when changing that profile's CDN.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dg4prez/incubator-trafficcontrol safeguards
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafficcontrol/pull/605.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #605
----
commit 5edf1b1e4825adcb268cef15783add1db1aadcad
Author: Derek Gelinas <de...@cable.comcast.com>
Date: 2017-05-23T20:22:30Z
Add CDN match checks when assigning or editing profiles
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #605: [TC-355] Add CDN match checks when assi...
Posted by dg4prez <gi...@git.apache.org>.
Github user dg4prez commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/605
added and tested with sphinx make.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #605: [TC-355] Add CDN match checks wh...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafficcontrol/pull/605
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #605: Add CDN match checks when assigning or ...
Posted by dg4prez <gi...@git.apache.org>.
Github user dg4prez commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/605
You're right Jeremy, I didn't add a check for that. I'll put that in there as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #605: [TC-355] Add CDN match checks wh...
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/605#discussion_r119394809
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -911,6 +911,21 @@ sub is_server_valid {
return ( 0, "Invalid server type" );
}
+ my $cdn_mismatch;
+ if ($id) {
+ my $profile = $self->db->resultset('Profile')->search( { 'me.id' => $params->{profileId}}, { prefetch => ['cdn'] } )->single();
+ if ( !defined($profile->cdn) ) {
+ $cdn_mismatch = 1;
+ }
+ elsif ( $params->{cdnId} != $profile->cdn->id ) {
+ $cdn_mismatch = 1;
+ }
+ }
+
+ if ($cdn_mismatch) {
+ return ( 0, "CDN of profile does not match Server CDN" );
+ }
--- End diff --
you could simplify all this code to simply:
if ($id) {
my $profile = $self->db->resultset('Profile')->search( { 'me.id' => $params->{profileId}}, { prefetch => ['cdn'] } )->single();
if ( !defined($profile->cdn) || $params->{cdnId} != $profile->cdn->id ) {
return ( 0, "CDN of profile does not match Server CDN" );
}
}
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #605: Add CDN match checks when assigning or ...
Posted by dg4prez <gi...@git.apache.org>.
Github user dg4prez commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/605
found an error, standby on this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #605: Add CDN match checks when assigning or ...
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/605
In the API, I see where you are checking this:
- when updating/creating a server, check that server.profile.cdn == server.cdn
but i don't see where you are checking this in the API:
- when updating a profile, check that profile.cdn == the cdn of the servers currently assigned to the profile
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #605: [TC-355] Add CDN match checks when assi...
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/605
Can you update the docs? https://github.com/apache/incubator-trafficcontrol/blob/master/docs/source/development/traffic_ops_api/v12/server.rst
specifically under POST /api/1.2/servers and PUT /api/1.2/servers/{:id}
just add a note next to the "profile" property description. something along the lines of "profile cdn must equal server cdn".
trying to improves these api docs a bit...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---