You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by mi...@apache.org on 2020/08/13 21:51:13 UTC
[trafficcontrol] branch master updated: Add verification for TO
server ips across a profile (#4907)
This is an automated email from the ASF dual-hosted git repository.
mitchell852 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new e313410 Add verification for TO server ips across a profile (#4907)
e313410 is described below
commit e31341007223b9609637929b28b9cf4293ed7cd8
Author: Steve Hamrick <sh...@users.noreply.github.com>
AuthorDate: Thu Aug 13 15:50:58 2020 -0600
Add verification for TO server ips across a profile (#4907)
* Add verification for TO server ips across a profile
* Fix whitespace
* Add details to error message
* Add trigger check
* Fix tests
Co-authored-by: Steve Hamrick <st...@comcast.com>
---
...0200811082611_add_server_ip_profile_trigger.sql | 124 +++++++++++++++++++++
traffic_ops/testing/api/v3/servers_test.go | 74 +++++++++++-
traffic_ops/testing/api/v3/tc-fixtures.json | 4 +-
traffic_ops/traffic_ops_golang/server/servers.go | 44 +++++++-
.../traffic_ops_golang/server/servers_test.go | 5 +-
5 files changed, 243 insertions(+), 8 deletions(-)
diff --git a/traffic_ops/app/db/migrations/20200811082611_add_server_ip_profile_trigger.sql b/traffic_ops/app/db/migrations/20200811082611_add_server_ip_profile_trigger.sql
new file mode 100644
index 0000000..daee1fc
--- /dev/null
+++ b/traffic_ops/app/db/migrations/20200811082611_add_server_ip_profile_trigger.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
+-- SQL in section 'Up' is executed when this migration is applied
+-- +goose StatementBegin
+CREATE OR REPLACE FUNCTION before_server_table()
+ RETURNS TRIGGER AS
+$$
+DECLARE
+ server_count BIGINT;
+BEGIN
+ WITH server_ips AS (
+ SELECT s.id, i.name, ip.address, s.profile
+ 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(*)
+ INTO server_count
+ FROM server_ips sip
+ JOIN server_ips sip2 on sip.id <> sip2.id
+ WHERE sip.id = NEW.id
+ AND sip2.address = sip.address
+ AND sip2.profile = sip.profile;
+
+ IF server_count > 0 THEN
+ RAISE EXCEPTION 'Server [id:%] does not have a unique ip_address over the profile [id:%], [%] conflicts',
+ NEW.id,
+ NEW.profile,
+ server_count;
+ END IF;
+ RETURN NEW;
+END;
+$$ LANGUAGE plpgsql;
+-- +goose StatementEnd
+
+-- +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 accross the server [id:%] profile [id:%], [%] conflicts',
+ server_id,
+ server_profile,
+ server_count;
+ END IF;
+ RETURN NEW;
+END;
+$$ LANGUAGE PLPGSQL;
+-- +goose StatementEnd
+
+CREATE TRIGGER before_update_server_trigger
+ BEFORE UPDATE
+ ON server
+ FOR EACH ROW
+ WHEN (NEW.profile <> OLD.profile)
+EXECUTE PROCEDURE before_server_table();
+
+CREATE TRIGGER before_create_server_trigger
+ BEFORE INSERT
+ ON server
+ FOR EACH ROW
+EXECUTE PROCEDURE before_server_table();
+
+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 FUNCTION IF EXISTS before_server_table();
+DROP FUNCTION IF EXISTS before_ip_address_table();
+
+DROP TRIGGER IF EXISTS before_update_server_trigger ON server;
+DROP TRIGGER IF EXISTS before_create_server_trigger ON server;
+DROP TRIGGER IF EXISTS before_update_ip_address_trigger ON ip_address;
+DROP TRIGGER IF EXISTS before_create_ip_address_trigger ON ip_address;
+
diff --git a/traffic_ops/testing/api/v3/servers_test.go b/traffic_ops/testing/api/v3/servers_test.go
index b9857fc..7aae452 100644
--- a/traffic_ops/testing/api/v3/servers_test.go
+++ b/traffic_ops/testing/api/v3/servers_test.go
@@ -17,12 +17,15 @@ package v3
import (
"fmt"
- "github.com/apache/trafficcontrol/lib/go-rfc"
"net/http"
"net/url"
"strconv"
"testing"
"time"
+
+ "github.com/apache/trafficcontrol/lib/go-rfc"
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/apache/trafficcontrol/lib/go-util"
)
func TestServers(t *testing.T) {
@@ -38,6 +41,7 @@ func TestServers(t *testing.T) {
GetTestServers(t)
GetTestServersIMSAfterChange(t, header)
GetTestServersQueryParameters(t)
+ UniqueIPProfileTestServers(t)
})
}
@@ -233,6 +237,74 @@ func GetTestServersQueryParameters(t *testing.T) {
params.Del("parentCacheGroup")
}
+func UniqueIPProfileTestServers(t *testing.T) {
+ serversResp, _, err := TOSession.GetServers(nil, nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(serversResp.Response) < 1 {
+ t.Fatal("expected more than 0 servers")
+ }
+ xmppId := "unique"
+ server := serversResp.Response[0]
+ _, _, err = TOSession.CreateServer(tc.ServerNullable{
+ CommonServerProperties: tc.CommonServerProperties{
+ Cachegroup: server.Cachegroup,
+ CDNName: server.CDNName,
+ DomainName: util.StrPtr("mydomain"),
+ FQDN: util.StrPtr("myfqdn"),
+ FqdnTime: time.Time{},
+ HostName: util.StrPtr("myhostname"),
+ HTTPSPort: util.IntPtr(443),
+ LastUpdated: &tc.TimeNoMod{
+ Time: time.Time{},
+ Valid: false,
+ },
+ PhysLocation: server.PhysLocation,
+ Profile: server.Profile,
+ StatusID: server.StatusID,
+ Type: server.Type,
+ UpdPending: util.BoolPtr(false),
+ XMPPID: &xmppId,
+ },
+ Interfaces: server.Interfaces,
+ })
+
+ if err == nil {
+ t.Error("expected an error when updating a server with an ipaddress that already exists on another server with the same profile")
+ // Cleanup, don't want to break other tests
+ pathParams := url.Values{}
+ pathParams.Add("xmppid", xmppId)
+ server, _, err := TOSession.GetServers(&pathParams, nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ _, _, err = TOSession.DeleteServerByID(*server.Response[0].ID)
+ if err != nil {
+ t.Fatalf("unable to delete server: %v", err)
+ }
+ }
+
+ var changed bool
+ for i, interf := range server.Interfaces {
+ if interf.Monitor {
+ for j, ip := range interf.IPAddresses {
+ if ip.ServiceAddress {
+ server.Interfaces[i].IPAddresses[j].Address = "127.0.0.1/24"
+ changed = true
+ }
+ }
+ }
+ }
+ if !changed {
+ t.Fatal("did not find ip address to update")
+ }
+ _, _, err = TOSession.UpdateServerByID(*server.ID, server)
+ if err != nil {
+ t.Fatalf("expected update to pass: %s", err)
+ }
+}
+
func UpdateTestServers(t *testing.T) {
if len(testData.Servers) < 1 {
t.Fatal("Need at least one server to test updating")
diff --git a/traffic_ops/testing/api/v3/tc-fixtures.json b/traffic_ops/testing/api/v3/tc-fixtures.json
index b1f492d..83b7e0d 100644
--- a/traffic_ops/testing/api/v3/tc-fixtures.json
+++ b/traffic_ops/testing/api/v3/tc-fixtures.json
@@ -2556,7 +2556,7 @@
"serviceAddress": false
},
{
- "address": "127.0.0.13/30",
+ "address": "127.0.0.19/30",
"gateway": "127.0.0.1",
"serviceAddress": true
}
@@ -2590,7 +2590,7 @@
"serviceAddress": false
},
{
- "address": "127.0.0.13/30",
+ "address": "127.0.0.20/30",
"gateway": "127.0.0.1",
"serviceAddress": true
}
diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go
index 88c9a3d..a2b10f5 100644
--- a/traffic_ops/traffic_ops_golang/server/servers.go
+++ b/traffic_ops/traffic_ops_golang/server/servers.go
@@ -26,8 +26,6 @@ import (
"encoding/json"
"errors"
"fmt"
- "github.com/apache/trafficcontrol/lib/go-rfc"
- "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
"net"
"net/http"
"strconv"
@@ -35,17 +33,18 @@ import (
"time"
"github.com/apache/trafficcontrol/lib/go-log"
+ "github.com/apache/trafficcontrol/lib/go-rfc"
"github.com/apache/trafficcontrol/lib/go-tc"
"github.com/apache/trafficcontrol/lib/go-tc/tovalidate"
"github.com/apache/trafficcontrol/lib/go-util"
-
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/routing/middleware"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
- "github.com/go-ozzo/ozzo-validation"
+ validation "github.com/go-ozzo/ozzo-validation"
"github.com/go-ozzo/ozzo-validation/is"
"github.com/google/uuid"
"github.com/jmoiron/sqlx"
@@ -427,7 +426,9 @@ func validateV3(s *tc.ServerNullable, tx *sql.Tx) (string, error) {
}
var errs []error
var serviceAddrV4Found bool
+ var ipv4 string
var serviceAddrV6Found bool
+ var ipv6 string
var serviceInterface string
for _, iface := range s.Interfaces {
@@ -468,11 +469,13 @@ func validateV3(s *tc.ServerNullable, tx *sql.Tx) (string, error) {
errs = append(errs, fmt.Errorf("interfaces: address '%s' of interface '%s' is marked as a service address, but an IPv4 service address appears earlier in the list", addr.Address, iface.Name))
}
serviceAddrV4Found = true
+ ipv4 = addr.Address
} else {
if serviceAddrV6Found {
errs = append(errs, fmt.Errorf("interfaces: address '%s' of interface '%s' is marked as a service address, but an IPv6 service address appears earlier in the list", addr.Address, iface.Name))
}
serviceAddrV6Found = true
+ ipv6 = addr.Address
}
}
}
@@ -483,6 +486,39 @@ func validateV3(s *tc.ServerNullable, tx *sql.Tx) (string, error) {
}
errs = append(errs, validateCommon(&s.CommonServerProperties, tx)...)
+
+ query := `
+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
+and p.id = $1
+`
+ var rows *sql.Rows
+ var err error
+ //ProfileID already validated
+ if s.ID != nil {
+ rows, err = tx.Query(query+" and s.id != $2", *s.ProfileID, *s.ID)
+ } else {
+ rows, err = tx.Query(query, *s.ProfileID)
+ }
+ if err != nil {
+ errs = append(errs, errors.New("unable to determine service address uniqueness"))
+ } else if rows != nil {
+ defer rows.Close()
+ for rows.Next() {
+ var id int
+ var ipaddress string
+ err = rows.Scan(&id, &ipaddress)
+ if err != nil {
+ errs = append(errs, errors.New("unable to determine service address uniqueness"))
+ } else if (ipaddress == ipv4 || ipaddress == ipv6) && (s.ID == nil || *s.ID != id) {
+ errs = append(errs, errors.New(fmt.Sprintf("there exists a server with id %v on the same profile that has the same service address %s", id, ipaddress)))
+ }
+ }
+ }
+
return serviceInterface, util.JoinErrs(errs)
}
diff --git a/traffic_ops/traffic_ops_golang/server/servers_test.go b/traffic_ops/traffic_ops_golang/server/servers_test.go
index ae56053..aed94a7 100644
--- a/traffic_ops/traffic_ops_golang/server/servers_test.go
+++ b/traffic_ops/traffic_ops_golang/server/servers_test.go
@@ -537,12 +537,15 @@ func TestV3Validations(t *testing.T) {
typeCols := []string{"name", "use_in_table"}
cdnCols := []string{"cdn"}
+ ipCols := []string{"id", "address"}
typeRows := sqlmock.NewRows(typeCols).AddRow("EDGE", "server")
cdnRows := sqlmock.NewRows(cdnCols).AddRow(*testServer.CDNID)
+ ipRows := sqlmock.NewRows(ipCols)
mock.ExpectBegin()
mock.ExpectQuery("SELECT name, use_in_table").WillReturnRows(typeRows)
- mock.ExpectQuery("SELECT").WillReturnRows(cdnRows)
+ mock.ExpectQuery("SELECT cdn").WillReturnRows(cdnRows)
+ mock.ExpectQuery("SELECT s.ID").WillReturnRows(ipRows)
tx := db.MustBegin().Tx