You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by oc...@apache.org on 2021/01/13 23:29:31 UTC
[trafficcontrol] branch 5.0.x updated: Adding check to make sure
that you cannot add two servers with identical content (#5415)
This is an automated email from the ASF dual-hosted git repository.
ocket8888 pushed a commit to branch 5.0.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/5.0.x by this push:
new 6c32df7 Adding check to make sure that you cannot add two servers with identical content (#5415)
6c32df7 is described below
commit 6c32df71093b580ccc2f97f4c9a0f667afaaae94
Author: Srijeet Chatterjee <30...@users.noreply.github.com>
AuthorDate: Wed Jan 13 13:18:28 2021 -0700
Adding check to make sure that you cannot add two servers with identical content (#5415)
* Adding check to make sure that you cannot add two servers with identical content
* wip
* move the modifications from the code to migrations
* changelog entry
* Code review fixes
* code review fixes
(cherry picked from commit 468065cdb3f2060d2216a9474dc82362cc1631c5)
---
CHANGELOG.md | 3 +-
...0900000000_server_ip_profile_trigger_update.sql | 124 +++++++++++++++++++++
traffic_ops/testing/api/v1/servers_test.go | 57 ++++++++++
traffic_ops/traffic_ops_golang/api/api.go | 14 +++
traffic_ops/traffic_ops_golang/server/servers.go | 5 +-
5 files changed, 199 insertions(+), 4 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f978541..91d2376 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,7 +28,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
on page refresh.
- [#5295](https://github.com/apache/trafficcontrol/issues/5295) - TP types/servers table now clears all filters instead
of just column filters
-- #2881 Some API endpoints have incorrect Content-Types
+- [#5407](https://github.com/apache/trafficcontrol/issues/5407) - Make sure that you cannot add two servers with identical content
+- [#2881](https://github.com/apache/trafficcontrol/issues/2881) - Some API endpoints have incorrect Content-Types
- [#5311](https://github.com/apache/trafficcontrol/issues/5311) - Better TO log messages when failures calling TM CacheStats
>>>>>>> 60e7f208d (TP: adds the ability to clone a topology (#5400))
diff --git a/traffic_ops/app/db/migrations/2021010900000000_server_ip_profile_trigger_update.sql b/traffic_ops/app/db/migrations/2021010900000000_server_ip_profile_trigger_update.sql
new file mode 100644
index 0000000..35cc584
--- /dev/null
+++ b/traffic_ops/app/db/migrations/2021010900000000_server_ip_profile_trigger_update.sql
@@ -0,0 +1,124 @@
+/*
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+-- +goose Up
+DROP TRIGGER IF EXISTS before_update_ip_address_trigger ON ip_address;
+DROP TRIGGER IF EXISTS before_create_ip_address_trigger ON ip_address;
+-- +goose StatementBegin
+CREATE OR REPLACE FUNCTION before_ip_address_table()
+ RETURNS TRIGGER
+AS
+$$
+DECLARE
+ server_count BIGINT;
+ server_id BIGINT;
+ server_profile BIGINT;
+BEGIN
+ WITH server_ips AS (
+ SELECT s.id as sid, ip.interface, i.name, ip.address, s.profile, ip.server
+ FROM server s
+ JOIN interface i
+ on i.server = s.ID
+ JOIN ip_address ip
+ on ip.Server = s.ID and ip.interface = i.name
+ WHERE ip.service_address = true
+ )
+ SELECT count(distinct(sip.sid)), sip.sid, sip.profile
+ INTO server_count, server_id, server_profile
+ FROM server_ips sip
+ WHERE (sip.server <> NEW.server AND (SELECT host(sip.address)) = (SELECT host(NEW.address)) AND sip.profile = (SELECT profile from server s WHERE s.id = NEW.server))
+ GROUP BY sip.sid, sip.profile;
+
+ IF server_count > 0 THEN
+ RAISE EXCEPTION 'ip_address is not unique across the server [id:%] profile [id:%], [%] conflicts',
+ server_id,
+ server_profile,
+ server_count;
+ END IF;
+ RETURN NEW;
+END;
+$$ LANGUAGE PLPGSQL;
+-- +goose StatementEnd
+
+CREATE TRIGGER before_create_ip_address_trigger
+ BEFORE INSERT
+ ON ip_address
+ FOR EACH ROW
+EXECUTE PROCEDURE before_ip_address_table();
+
+CREATE TRIGGER before_update_ip_address_trigger
+ BEFORE UPDATE
+ ON ip_address
+ FOR EACH ROW
+ WHEN (NEW.address <> OLD.address)
+EXECUTE PROCEDURE before_ip_address_table();
+
+-- +goose Down
+-- SQL section 'Down' is executed when this migration is rolled back
+DROP TRIGGER IF EXISTS before_update_ip_address_trigger ON ip_address;
+DROP TRIGGER IF EXISTS before_create_ip_address_trigger ON ip_address;
+
+DROP FUNCTION IF EXISTS before_ip_address_table();
+
+-- +goose StatementBegin
+CREATE OR REPLACE FUNCTION before_ip_address_table()
+ RETURNS TRIGGER
+AS
+$$
+DECLARE
+ server_count BIGINT;
+ server_id BIGINT;
+ server_profile BIGINT;
+BEGIN
+ WITH server_ips AS (
+ SELECT s.id as sid, ip.interface, i.name, ip.address, s.profile, ip.server
+ FROM server s
+ JOIN interface i
+ on i.server = s.ID
+ JOIN ip_address ip
+ on ip.Server = s.ID and ip.interface = i.name
+ WHERE i.monitor = true
+ )
+ SELECT count(sip.sid), sip.sid, sip.profile
+ INTO server_count, server_id, server_profile
+ FROM server_ips sip
+ JOIN server_ips sip2 on sip.sid <> sip2.sid
+ WHERE (sip.server = NEW.server AND sip.address = NEW.address AND sip.interface = NEW.interface)
+ AND sip2.address = sip.address
+ AND sip2.profile = sip.profile
+ GROUP BY sip.sid, sip.profile;
+
+ IF server_count > 0 THEN
+ RAISE EXCEPTION 'ip_address is not unique across the server [id:%] profile [id:%], [%] conflicts',
+ server_id,
+ server_profile,
+ server_count;
+ END IF;
+ RETURN NEW;
+END;
+$$ LANGUAGE PLPGSQL;
+-- +goose StatementEnd
+
+CREATE TRIGGER before_create_ip_address_trigger
+ BEFORE INSERT
+ ON ip_address
+ FOR EACH ROW
+EXECUTE PROCEDURE before_ip_address_table();
+
+CREATE TRIGGER before_update_ip_address_trigger
+ BEFORE UPDATE
+ ON ip_address
+ FOR EACH ROW
+ WHEN (NEW.address <> OLD.address)
+EXECUTE PROCEDURE before_ip_address_table();
diff --git a/traffic_ops/testing/api/v1/servers_test.go b/traffic_ops/testing/api/v1/servers_test.go
index 65c0f95..88293f3 100644
--- a/traffic_ops/testing/api/v1/servers_test.go
+++ b/traffic_ops/testing/api/v1/servers_test.go
@@ -23,6 +23,7 @@ import (
func TestServers(t *testing.T) {
WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, DeliveryServices, Servers}, func() {
+ CreateTestServerWithSameServiceIPAddressAndProfile(t)
UpdateTestServers(t)
GetTestServers(t)
})
@@ -122,6 +123,62 @@ func UpdateTestServers(t *testing.T) {
}
+func CreateTestServerWithSameServiceIPAddressAndProfile(t *testing.T) {
+ if len(testData.Servers) == 0 {
+ t.Fatal("no test data servers, quitting")
+ }
+ firstServer := testData.Servers[0]
+ hostName := firstServer.HostName
+ resp, _, err := TOSession.GetServerByHostName(hostName)
+ if err != nil {
+ t.Errorf("cannot GET Server by hostname: %v - %v", firstServer.HostName, err)
+ }
+ if len(resp) == 0 {
+ t.Fatal("no response servers, quitting")
+ }
+ newServer := tc.ServerV1{
+ TCPPort: resp[0].TCPPort,
+ HostName: "testServerCreate",
+ DomainName: resp[0].DomainName,
+ HTTPSPort: resp[0].HTTPSPort,
+ InterfaceName: "test-interface",
+ IPAddress: "100.100.100.100",
+ ProfileID: resp[0].ProfileID,
+ CDNID: resp[0].CDNID,
+ TypeID: resp[0].TypeID,
+ StatusID: resp[0].StatusID,
+ PhysLocationID: resp[0].PhysLocationID,
+ CachegroupID: resp[0].CachegroupID,
+ InterfaceMtu: resp[0].InterfaceMtu,
+ }
+ _, _, err = TOSession.CreateServer(newServer)
+ if err != nil {
+ t.Fatalf("error while CREATEing a new server: %v", err.Error())
+ }
+ s, _, err := TOSession.GetServerByHostName("testServerCreate")
+ if err != nil {
+ t.Fatalf("error GETting the server with hostname %s, that was just created: %v", newServer.HostName, err.Error())
+ }
+ if len(s) == 0 {
+ t.Fatalf("no servers returned")
+ }
+ // Try creating a server with the same service IP address
+ _, _, err = TOSession.CreateServer(newServer)
+ if err == nil {
+ t.Error("expected error about an existing server with the same profile and IP address, but got nothing")
+ }
+ // Now try creating a server with the same service IP address but different netmask
+ newServer.IPNetmask = "255.255.100.100"
+ _, _, err = TOSession.CreateServer(newServer)
+ if err == nil {
+ t.Error("expected error about an existing server with the same profile and IP address, but got nothing")
+ }
+ _, _, err = TOSession.DeleteServerByID(s[0].ID)
+ if err != nil {
+ t.Errorf("error DELETEing the server just created: %v", err.Error())
+ }
+}
+
func DeleteTestServers(t *testing.T) {
for _, server := range testData.Servers {
diff --git a/traffic_ops/traffic_ops_golang/api/api.go b/traffic_ops/traffic_ops_golang/api/api.go
index 737b878..0d06e97 100644
--- a/traffic_ops/traffic_ops_golang/api/api.go
+++ b/traffic_ops/traffic_ops_golang/api/api.go
@@ -745,6 +745,16 @@ func toCamelCase(str string) string {
return strings.Replace(string(mutable[:]), "_", "", -1)
}
+// parses pq errors for any trigger based conflicts
+func parseTriggerConflicts(err *pq.Error) (error, error, int) {
+ pattern := regexp.MustCompile(`^(.*?)conflicts`)
+ match := pattern.FindStringSubmatch(err.Message)
+ if match == nil {
+ return nil, nil, http.StatusOK
+ }
+ return fmt.Errorf("%s", toCamelCase(match[0])), nil, http.StatusBadRequest
+}
+
// parses pq errors for not null constraint
func parseNotNullConstraint(err *pq.Error) (error, error, int) {
pattern := regexp.MustCompile(`null value in column "(.+)" violates not-null constraint`)
@@ -861,6 +871,10 @@ func ParseDBError(ierr error) (error, error, int) {
return usrErr, sysErr, errCode
}
+ if usrErr, sysErr, errCode := parseTriggerConflicts(err); errCode != http.StatusOK {
+ return usrErr, sysErr, errCode
+ }
+
return nil, err, http.StatusInternalServerError
}
diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go
index 0690250..0baf08e 100644
--- a/traffic_ops/traffic_ops_golang/server/servers.go
+++ b/traffic_ops/traffic_ops_golang/server/servers.go
@@ -605,7 +605,7 @@ SELECT s.ID, ip.address FROM server s
JOIN profile p on p.Id = s.Profile
JOIN interface i on i.server = s.ID
JOIN ip_address ip on ip.Server = s.ID and ip.interface = i.name
-WHERE i.monitor = true
+WHERE ip.service_address = true
and p.id = $1
`
var rows *sql.Rows
@@ -1170,7 +1170,6 @@ func createInterfaces(id int, interfaces []tc.ServerInterfaceInfo, tx *sql.Tx) (
if err != nil {
return api.ParseDBError(err)
}
-
return nil, nil, http.StatusOK
}
@@ -1447,7 +1446,7 @@ func createV1(inf *api.APIInfo, w http.ResponseWriter, r *http.Request) {
return
}
- if userErr, sysErr, errCode := createInterfaces(*server.ID, ifaces, tx); err != nil {
+ if userErr, sysErr, errCode := createInterfaces(*server.ID, ifaces, tx); userErr != nil || sysErr != nil || errCode != http.StatusOK {
api.HandleErr(w, r, tx, errCode, userErr, sysErr)
return
}