You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ma...@apache.org on 2020/09/28 22:35:08 UTC

[trafficcontrol] branch master updated: Creating an ASN with the same number and same cache group should not be allowed (#5043)

This is an automated email from the ASF dual-hosted git repository.

mattjackson 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 62fb2d7  Creating an ASN with the same number and same cache group should not be allowed (#5043)
62fb2d7 is described below

commit 62fb2d79ee0527d17cdf1c7dc02b445377175d73
Author: Srijeet Chatterjee <30...@users.noreply.github.com>
AuthorDate: Mon Sep 28 16:34:55 2020 -0600

    Creating an ASN with the same number and same cache group should not be allowed (#5043)
    
    * Dont allow creationg of more than one asns with same asn number and cachegroup ID
    
    * Fixing 500 errors to 400s for user errors
    
    * go fmt
    
    * changelog entry
    
    * code review changes
    
    * Code review
    
    * Code review fix
---
 CHANGELOG.md                                    |  1 +
 docs/source/api/v3/asns.rst                     |  2 ++
 traffic_ops/traffic_ops_golang/asn/asns.go      | 30 ++++++++++++++++++---
 traffic_ops/traffic_ops_golang/asn/asns_test.go | 36 +++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b0abbcc..9979000 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -78,6 +78,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Added Delivery Service Raw Remap `__RANGE_DIRECTIVE__` directive to allow inserting the Range Directive after the Raw Remap text. This allows Raw Remaps which manipulate the Range.
 - Added an option for `coordinateRange` in the RGB configuration file, so that in case a client doesn't have a postal code, we can still determine if it should be allowed or not, based on whether or not the latitude/ longitude of the client falls within the supplied ranges. [Related github issue](https://github.com/apache/trafficcontrol/issues/4372)
 - Fixed #3548 - Prevents DS regexes with non-consecutive order from generating invalid CRconfig/snapshot.
+- Fixed #5020, #5021 - Creating an ASN with the same number and same cache group should not be allowed.
 - Fixed #4680 - Change Content-Type to application/json for TR auth calls
 
 ### Changed
diff --git a/docs/source/api/v3/asns.rst b/docs/source/api/v3/asns.rst
index 652ebd4..15f7541 100644
--- a/docs/source/api/v3/asns.rst
+++ b/docs/source/api/v3/asns.rst
@@ -107,6 +107,8 @@ Response Structure
 ========
 Creates a new :abbr:`ASN (Autonomous System Number)`.
 
+.. note:: There cannot be two different ASN object with the same ``asn``.
+
 :Auth. Required: Yes
 :Roles Required: "admin" or "operations"
 :Response Type: Object
diff --git a/traffic_ops/traffic_ops_golang/asn/asns.go b/traffic_ops/traffic_ops_golang/asn/asns.go
index 0b3dbf8..32ad36c 100644
--- a/traffic_ops/traffic_ops_golang/asn/asns.go
+++ b/traffic_ops/traffic_ops_golang/asn/asns.go
@@ -20,6 +20,7 @@ package asn
  */
 
 import (
+	"errors"
 	"net/http"
 	"strconv"
 	"time"
@@ -48,8 +49,8 @@ func (v *TOASNV11) NewReadObj() interface{}       { return &tc.ASNNullable{} }
 func (v *TOASNV11) SelectQuery() string           { return selectQuery() }
 func (v *TOASNV11) ParamColumns() map[string]dbhelpers.WhereColumnInfo {
 	return map[string]dbhelpers.WhereColumnInfo{
-		"asn":            dbhelpers.WhereColumnInfo{"a.asn", nil},
-		"cachegroup":     dbhelpers.WhereColumnInfo{"c.id", nil},
+		"asn":            dbhelpers.WhereColumnInfo{"a.asn", api.IsInt},
+		"cachegroup":     dbhelpers.WhereColumnInfo{"c.id", api.IsInt},
 		"id":             dbhelpers.WhereColumnInfo{"a.id", api.IsInt},
 		"cachegroupName": dbhelpers.WhereColumnInfo{"c.name", nil},
 	}
@@ -90,12 +91,35 @@ func (asn TOASNV11) GetType() string {
 	return "asn"
 }
 
+func (asn TOASNV11) ASNExists() error {
+	if asn.APIInfo() == nil || asn.APIInfo().Tx == nil {
+		return errors.New("couldn't perform check to see if asn number exists already")
+	}
+	if asn.ASN == nil || asn.CachegroupID == nil {
+		return errors.New("no asn or cachegroup ID specified")
+	}
+	query := `SELECT id from asn where asn=$1`
+	rows, err := asn.APIInfo().Tx.Query(query, *asn.ASN)
+	if err != nil {
+		return errors.New("selecting asns: " + err.Error())
+	}
+	defer rows.Close()
+	if rows.Next() {
+		return errors.New("an asn with the specified number already exists")
+	}
+	return nil
+}
+
 func (asn TOASNV11) Validate() error {
 	errs := validation.Errors{
 		"asn":          validation.Validate(asn.ASN, validation.NotNil, validation.Min(0)),
 		"cachegroupId": validation.Validate(asn.CachegroupID, validation.NotNil, validation.Min(0)),
 	}
-	return util.JoinErrs(tovalidate.ToErrors(errs))
+	if errs["asn"] != nil || errs["cachegroupId"] != nil {
+		return util.JoinErrs(tovalidate.ToErrors(errs))
+	}
+	err := asn.ASNExists()
+	return err
 }
 
 func (as *TOASNV11) Create() (error, error, int) { return api.GenericCreate(as) }
diff --git a/traffic_ops/traffic_ops_golang/asn/asns_test.go b/traffic_ops/traffic_ops_golang/asn/asns_test.go
index 304ca45..655c51e 100644
--- a/traffic_ops/traffic_ops_golang/asn/asns_test.go
+++ b/traffic_ops/traffic_ops_golang/asn/asns_test.go
@@ -134,3 +134,39 @@ func TestValidate(t *testing.T) {
 		t.Errorf(`expected %v,  got %v`, expected, errs)
 	}
 }
+
+func TestValidateASNExists(t *testing.T) {
+	expected := `an asn with the specified number already exists`
+	mockDB, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("an error '%s' was not expected when opening a stub database connection", err)
+	}
+	defer mockDB.Close()
+
+	db := sqlx.NewDb(mockDB, "sqlmock")
+	defer db.Close()
+
+	cols := []string{"id"}
+	rows := sqlmock.NewRows(cols)
+	rows = rows.AddRow(
+		1,
+	)
+	mock.ExpectBegin()
+	mock.ExpectQuery("SELECT").WillReturnRows(rows)
+	mock.ExpectCommit()
+
+	reqInfo := api.APIInfo{Tx: db.MustBegin()}
+	asnNum := 2
+	cachegroupID := 10
+	asn := TOASNV11{
+		api.APIInfoImpl{&reqInfo},
+		tc.ASNNullable{ASN: &asnNum, CachegroupID: &cachegroupID},
+	}
+	err = asn.ASNExists()
+	if err == nil {
+		t.Fatalf("expected error but got none")
+	}
+	if err.Error() != expected {
+		t.Errorf("Expected error detail to be %v, got %v", expected, err.Error())
+	}
+}