You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ra...@apache.org on 2021/03/09 21:46:02 UTC
[trafficcontrol] branch master updated: Adding check for safer
conversion between types (#5600)
This is an automated email from the ASF dual-hosted git repository.
rawlin 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 e2fecc1 Adding check for safer conversion between types (#5600)
e2fecc1 is described below
commit e2fecc1997cb5539590b94d534027ef530de2777
Author: Srijeet Chatterjee <30...@users.noreply.github.com>
AuthorDate: Tue Mar 9 14:45:49 2021 -0700
Adding check for safer conversion between types (#5600)
* Adding check for safer conversion between types
* code review
* code review
* code review
---
CHANGELOG.md | 1 +
.../traffic_ops_golang/cachesstats/cachesstats.go | 29 +++-
.../cachesstats/cachesstats_test.go | 182 +++++++++++++++++++++
3 files changed, 206 insertions(+), 6 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5e9bc0b..703e8dd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -30,6 +30,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Added an endpoint for statuses on asynchronous jobs and applied it to the ACME renewal endpoint.
### Fixed
+- [#5565](https://github.com/apache/trafficcontrol/issues/5565) - TO GET /caches/stats panic converting string to uint64
- [#5558](https://github.com/apache/trafficcontrol/issues/5558) - Fixed `TM UI` and `/api/cache-statuses` to report aggregate `bandwidth_kbps` correctly.
- [#5288](https://github.com/apache/trafficcontrol/issues/5288) - Fixed the ability to create and update a server with MTU value >= 1280.
- [#5445](https://github.com/apache/trafficcontrol/issues/5445) - When updating a registered user, ignore updates on registration_sent field.
diff --git a/traffic_ops/traffic_ops_golang/cachesstats/cachesstats.go b/traffic_ops/traffic_ops_golang/cachesstats/cachesstats.go
index 540755d..cf3e175 100644
--- a/traffic_ops/traffic_ops_golang/cachesstats/cachesstats.go
+++ b/traffic_ops/traffic_ops_golang/cachesstats/cachesstats.go
@@ -79,19 +79,21 @@ func getCachesStats(tx *sql.Tx) ([]CacheData, error) {
}
var cacheStats tc.Stats
+ var url string
stats := []string{ATSCurrentConnectionsStat, tc.StatNameBandwidth}
- cacheStats, _, err = monitorhlp.GetCacheStats(monitorFQDN, client, stats)
+ cacheStats, url, err = monitorhlp.GetCacheStats(monitorFQDN, client, stats)
if err != nil {
- legacyCacheStats, _, err := monitorhlp.GetLegacyCacheStats(monitorFQDN, client, stats)
+ legacyCacheStats, legacyUrl, err := monitorhlp.GetLegacyCacheStats(monitorFQDN, client, stats)
if err != nil {
errs = append(errs, errors.New("getting CacheStats for CDN '"+string(cdn)+"' monitor '"+monitorFQDN+"': "+err.Error()))
continue
}
+ url = legacyUrl
cacheStats = monitorhlp.UpgradeLegacyStats(legacyCacheStats)
}
cacheData = addHealth(cacheData, crStates)
- cacheData = addStats(cacheData, cacheStats)
+ cacheData = addStats(cacheData, cacheStats, url)
success = true
break
}
@@ -135,7 +137,8 @@ func addTotals(data []CacheData) []CacheData {
return data
}
-func addStats(cacheData []CacheData, stats tc.Stats) []CacheData {
+func addStats(cacheData []CacheData, stats tc.Stats, url string) []CacheData {
+ var err error
if stats.Caches == nil {
return cacheData // TODO warn?
}
@@ -146,11 +149,25 @@ func addStats(cacheData []CacheData, stats tc.Stats) []CacheData {
}
bandwidth, ok := stat.Stats[tc.StatNameBandwidth]
if ok && len(bandwidth) > 0 {
- cache.KBPS = bandwidth[0].Val.(uint64)
+ if kbps, ok := bandwidth[0].Val.(string); !ok {
+ log.Warnf("bandwidth %v of cache %s from url %s couldn't be converted into string", bandwidth[0].Val, string(cache.HostName), url)
+ } else {
+ cache.KBPS, err = strconv.ParseUint(kbps, 10, 64)
+ if err != nil {
+ log.Warnf("'bandwidth' stat %v of cache %s from url %s couldn't be converted into uint64", kbps, string(cache.HostName), url)
+ }
+ }
}
connections, ok := stat.Stats[ATSCurrentConnectionsStat]
if ok && len(connections) > 0 {
- cache.Connections = connections[0].Val.(uint64)
+ if conn, ok := connections[0].Val.(string); !ok {
+ log.Warnf("'connections' stat %v of cache %s from url %s couldn't be converted into string", connections[0].Val, string(cache.HostName), url)
+ } else {
+ cache.Connections, err = strconv.ParseUint(conn, 10, 64)
+ if err != nil {
+ log.Warnf("'connections' stat %v of cache %s from url %s couldn't be converted into uint64", conn, string(cache.HostName), url)
+ }
+ }
}
cacheData[i] = cache
}
diff --git a/traffic_ops/traffic_ops_golang/cachesstats/cachesstats_test.go b/traffic_ops/traffic_ops_golang/cachesstats/cachesstats_test.go
new file mode 100644
index 0000000..4233949
--- /dev/null
+++ b/traffic_ops/traffic_ops_golang/cachesstats/cachesstats_test.go
@@ -0,0 +1,182 @@
+package cachesstats
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+import (
+ "encoding/json"
+ "testing"
+ "time"
+
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/apache/trafficcontrol/lib/go-util"
+)
+
+func TestAddStatsInvalidString(t *testing.T) {
+ cacheData := make([]CacheData, 0)
+ cacheServerStats := make(map[string]tc.ServerStats, 0)
+ stats := make(map[string][]tc.ResultStatVal, 0)
+ resultStatVals := make([]tc.ResultStatVal, 0)
+ resultStatVal := tc.ResultStatVal{
+ Span: 0,
+ Time: time.Now(),
+ Val: "invalid bandwidth",
+ }
+ resultStatVals = append(resultStatVals, resultStatVal)
+ stats[tc.StatNameBandwidth] = resultStatVals
+
+ resultStatVal = tc.ResultStatVal{
+ Span: 0,
+ Time: time.Now(),
+ Val: "invalid ats current connection stat",
+ }
+ resultStatVals = make([]tc.ResultStatVal, 0)
+ resultStatVals = append(resultStatVals, resultStatVal)
+ stats[ATSCurrentConnectionsStat] = resultStatVals
+
+ cacheServerStats["hostName"] = tc.ServerStats{
+ Interfaces: nil,
+ Stats: stats,
+ }
+ cacheStats := tc.Stats{
+ CommonAPIData: tc.CommonAPIData{},
+ Caches: cacheServerStats,
+ }
+ data := CacheData{
+ HostName: "hostName",
+ CacheGroup: "cacheGroup",
+ Status: "ONLINE",
+ Profile: "profile",
+ IP: util.StrPtr("127.30.30.30"),
+ Healthy: true,
+ KBPS: 0,
+ Connections: 0,
+ }
+ cacheData = append(cacheData, data)
+ result := addStats(cacheData, cacheStats, "url")
+ if len(result) != 1 {
+ t.Fatalf("expected a cache stat in the response, but got nothing")
+ }
+ if result[0].KBPS != 0 || result[0].Connections != 0 {
+ t.Errorf("expected 0 KBPS and 0 connections, but got %v and %v respectively", result[0].KBPS, result[0].Connections)
+ }
+}
+
+func TestAddStatsValidString(t *testing.T) {
+ cacheData := make([]CacheData, 0)
+ cacheServerStats := make(map[string]tc.ServerStats, 0)
+ stats := make(map[string][]tc.ResultStatVal, 0)
+ resultStatVals := make([]tc.ResultStatVal, 0)
+ resultStatVal := tc.ResultStatVal{
+ Span: 0,
+ Time: time.Now(),
+ Val: "200",
+ }
+ resultStatVals = append(resultStatVals, resultStatVal)
+ stats[tc.StatNameBandwidth] = resultStatVals
+
+ resultStatVal = tc.ResultStatVal{
+ Span: 0,
+ Time: time.Now(),
+ Val: "100",
+ }
+ resultStatVals = make([]tc.ResultStatVal, 0)
+ resultStatVals = append(resultStatVals, resultStatVal)
+ stats[ATSCurrentConnectionsStat] = resultStatVals
+
+ cacheServerStats["hostName"] = tc.ServerStats{
+ Interfaces: nil,
+ Stats: stats,
+ }
+ cacheStats := tc.Stats{
+ CommonAPIData: tc.CommonAPIData{},
+ Caches: cacheServerStats,
+ }
+ data := CacheData{
+ HostName: "hostName",
+ CacheGroup: "cacheGroup",
+ Status: "ONLINE",
+ Profile: "profile",
+ IP: util.StrPtr("127.30.30.30"),
+ Healthy: true,
+ KBPS: 0,
+ Connections: 0,
+ }
+ cacheData = append(cacheData, data)
+ result := addStats(cacheData, cacheStats, "url")
+ if len(result) != 1 {
+ t.Fatalf("expected a cache stat in the response, but got nothing")
+ }
+ if result[0].KBPS != 200 || result[0].Connections != 100 {
+ t.Errorf("expected 200 KBPS and 100 connections, but got %v and %v respectively", result[0].KBPS, result[0].Connections)
+ }
+}
+
+func TestAddStatsEmptyJsonObject(t *testing.T) {
+ cacheData := make([]CacheData, 0)
+ cacheServerStats := make(map[string]tc.ServerStats, 0)
+ stats := make(map[string][]tc.ResultStatVal, 0)
+ resultStatVals := make([]tc.ResultStatVal, 0)
+ type testJson struct{}
+ var req testJson
+ _ = json.Unmarshal([]byte(""), &req)
+ resultStatVal := tc.ResultStatVal{
+ Span: 0,
+ Time: time.Now(),
+ Val: req,
+ }
+ resultStatVals = append(resultStatVals, resultStatVal)
+ stats[tc.StatNameBandwidth] = resultStatVals
+
+ resultStatVal = tc.ResultStatVal{
+ Span: 0,
+ Time: time.Now(),
+ Val: req,
+ }
+ resultStatVals = make([]tc.ResultStatVal, 0)
+ resultStatVals = append(resultStatVals, resultStatVal)
+ stats[ATSCurrentConnectionsStat] = resultStatVals
+
+ cacheServerStats["hostName"] = tc.ServerStats{
+ Interfaces: nil,
+ Stats: stats,
+ }
+ cacheStats := tc.Stats{
+ CommonAPIData: tc.CommonAPIData{},
+ Caches: cacheServerStats,
+ }
+ data := CacheData{
+ HostName: "hostName",
+ CacheGroup: "cacheGroup",
+ Status: "ONLINE",
+ Profile: "profile",
+ IP: util.StrPtr("127.30.30.30"),
+ Healthy: true,
+ KBPS: 0,
+ Connections: 0,
+ }
+ cacheData = append(cacheData, data)
+ result := addStats(cacheData, cacheStats, "url")
+ if len(result) != 1 {
+ t.Fatalf("expected a cache stat in the response, but got nothing")
+ }
+ if result[0].KBPS != 0 || result[0].Connections != 0 {
+ t.Errorf("expected 0 KBPS and 0 connections, but got %v and %v respectively", result[0].KBPS, result[0].Connections)
+ }
+}