This is an automated email from the ASF dual-hosted git repository.
ocket8888 pushed a commit to branch 5.1.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/5.1.x by this push:
new 82026c4 Disallow updating the Type of a Cache Group if it is in use
by a (#5483) (#5517)
82026c4 is described below
commit 82026c494b68a32b256882fcdfc3a72a8e64c53a
Author: ocket8888 <[email protected]>
AuthorDate: Mon Feb 15 15:31:50 2021 -0500
Disallow updating the Type of a Cache Group if it is in use by a (#5483)
(#5517)
Topology
(cherry picked from commit 830f32e2532303ccf4f3da66d0065b54eac38277)
Co-authored-by: Zach Hoffman <[email protected]>
---
CHANGELOG.md | 1 +
.../traffic_ops_golang/cachegroup/cachegroups.go | 74 ++++++++++++++++++++++
.../cachegroup/cachegroups_test.go | 8 ++-
3 files changed, 81 insertions(+), 2 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2826601..7ea5383 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -42,6 +42,7 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
- [#5390](https://github.com/apache/trafficcontrol/issues/5390) - Improve the
way TO deals with delivery service server assignments
- [#5339](https://github.com/apache/trafficcontrol/issues/5339) - Ensure
Changelog entries for SSL key changes
- [#5461](https://github.com/apache/trafficcontrol/issues/5461) - Fixed
steering endpoint to be ordered consistently
+- [#5395](https://github.com/apache/trafficcontrol/issues/5395) - Added
validation to prevent changing the Type any Cache Group that is in use by a
Topology
### Changed
- Refactored the Traffic Ops Go client internals so that all public methods
have a consistent behavior/implementation
diff --git a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go
b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go
index bf80ba3..70ba7c5 100644
--- a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go
+++ b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go
@@ -158,6 +158,79 @@ func IsValidParentCachegroupID(id *int) bool {
return false
}
+// ValidateTypeInTopology validates cachegroup updates to ensure the type of
the cachegroup does not change
+// if it is assigned to a topology.
+func (cg *TOCacheGroup) ValidateTypeInTopology() error {
+ userErr := fmt.Errorf("unable to check whether cachegroup %s is used in
any topologies", *cg.Name)
+
+ // language=SQL
+ const previousTypeQuery = `
+ SELECT t.id
+ FROM cachegroup c
+ JOIN "type" t ON c."type" = t.id
+ WHERE c.id = $1
+ `
+ var previousTypeID int
+ // We only run this validation on PUT, not POST
+ if cg.ID == nil {
+ return nil
+ }
+ err := cg.ReqInfo.Tx.QueryRow(previousTypeQuery,
*cg.ID).Scan(&previousTypeID)
+ // Cachegroup does not exist in the database yet
+ if err == sql.ErrNoRows {
+ return nil
+ }
+ if err != nil {
+ log.Errorf("%s: getting the previous type of cachegroup %s:
%s", userErr.Error(), *cg.Name, err.Error())
+ return userErr
+ }
+ if cg.TypeID == nil || *cg.TypeID == previousTypeID {
+ return nil
+ }
+
+ // language=SQL
+ const usedInTopologyQuery = `
+ SELECT EXISTS (SELECT
+ FROM topology_cachegroup tc
+ WHERE tc.cachegroup = $1)
+ `
+ var usedInTopology bool
+ err = cg.ReqInfo.Tx.QueryRow(usedInTopologyQuery,
*cg.Name).Scan(&usedInTopology)
+ if err != nil {
+ log.Errorf("%s: querying topology_cachegroup by cachegroup
name: %s", userErr.Error(), err.Error())
+ return userErr
+ }
+ if !usedInTopology {
+ return nil
+ }
+
+ // language=SQL
+ const readableTypesQuery = `
+ SELECT t.id, t."name"
+ FROM "type" t
+ WHERE id = $1 OR id = $2
+ `
+ rows, err := cg.ReqInfo.Tx.Query(readableTypesQuery, previousTypeID,
*cg.TypeID)
+ if err != nil {
+ log.Errorf("%s: querying type names: %s", userErr.Error(),
err.Error())
+ return userErr
+ }
+ typeNameByID := map[int]string{}
+ for rows.Next() {
+ var typeID int
+ var typeName string
+ err = rows.Scan(&typeID, &typeName)
+ if err != nil {
+ log.Errorf("%s: scanning type names: %s",
userErr.Error(), err.Error())
+ return userErr
+ }
+ typeNameByID[typeID] = typeName
+ }
+ log.Close(rows, "error closing rows for type names")
+
+ return fmt.Errorf("cannot change type of cachegroup %s from %s to %s
because it is in use by a topology", *cg.Name, typeNameByID[previousTypeID],
typeNameByID[*cg.TypeID])
+}
+
// Validate fulfills the api.Validator interface
func (cg TOCacheGroup) Validate() error {
if _, err := tc.ValidateTypeID(cg.ReqInfo.Tx.Tx, cg.TypeID,
"cachegroup"); err != nil {
@@ -196,6 +269,7 @@ func (cg TOCacheGroup) Validate() error {
"parentCacheGroupID":
validation.Validate(cg.ParentCachegroupID, validation.Min(1)),
"secondaryParentCachegroupID":
validation.Validate(cg.SecondaryParentCachegroupID, validation.Min(1)),
"localizationMethods":
validation.Validate(cg.LocalizationMethods,
validation.By(tovalidate.IsPtrToSliceOfUniqueStringersICase("CZ", "DEEP_CZ",
"GEO"))),
+ "type": cg.ValidateTypeInTopology(),
}
return util.JoinErrs(tovalidate.ToErrors(errs))
}
diff --git a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups_test.go
b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups_test.go
index 24ab463..0139958 100644
--- a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups_test.go
+++ b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups_test.go
@@ -203,7 +203,7 @@ func TestValidate(t *testing.T) {
rows.AddRow("EDGE_LOC", "cachegroup")
mock.ExpectBegin()
- mock.ExpectQuery("SELECT").WillReturnRows(rows)
+ mock.ExpectQuery("SELECT\\s+name,\\s+use_in_table").WillReturnRows(rows)
tx := db.MustBegin()
reqInfo := api.APIInfo{Tx: tx}
@@ -239,6 +239,7 @@ func TestValidate(t *testing.T) {
errors.New(`'longitude' Must be a floating point number within
the range +-180`),
errors.New(`'name' invalid characters found - Use alphanumeric
. or - or _ .`),
errors.New(`'shortName' invalid characters found - Use
alphanumeric . or - or _ .`),
+ errors.New(`'type' unable to check whether cachegroup
not!a!valid!cachegroup is used in any topologies`),
})
if !reflect.DeepEqual(expectedErrs, errs) {
@@ -246,7 +247,10 @@ func TestValidate(t *testing.T) {
}
rows.AddRow("EDGE_LOC", "cachegroup")
- mock.ExpectQuery("SELECT").WillReturnRows(rows)
+ mock.ExpectQuery("SELECT\\s+name,\\s+use_in_table").WillReturnRows(rows)
+
+ rows = sqlmock.NewRows([]string{"id", "name"})
+
mock.ExpectQuery("SELECT\\s+t\\.id\\s+FROM\\s+cachegroup").WillReturnRows(rows)
// valid name, shortName latitude, longitude
nm = "This.is.2.a-Valid---Cachegroup."
