This is an automated email from the ASF dual-hosted git repository.
zrhoffman 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 14d88e5 Consistent profile name restrictions (#6018)
14d88e5 is described below
commit 14d88e5cd8d1b84746d0869043dd3c86f3d3f8c7
Author: ocket8888 <[email protected]>
AuthorDate: Wed Aug 11 12:58:32 2021 -0600
Consistent profile name restrictions (#6018)
* Added restriction to TO to disallow creating/updating Profiles with names
containing spaces
* Remove unnecessary event listener
* Add unit test coverage for new constraint
* Add client/API integration testing coverage for new contstraint
* Update CHANGELOG
* Update documentation
---
CHANGELOG.md | 1 +
docs/source/overview/profiles_and_parameters.rst | 5 ++
lib/go-tc/profiles.go | 17 +++++-
traffic_ops/testing/api/v4/profiles_test.go | 62 ++++++++++++++++++----
traffic_ops/traffic_ops_golang/profile/copy.go | 13 +++++
traffic_ops/traffic_ops_golang/profile/profiles.go | 26 ++++++++-
.../traffic_ops_golang/profile/profiles_test.go | 18 ++++++-
.../modules/form/profile/form.profile.tpl.html | 2 +-
8 files changed, 128 insertions(+), 16 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 551bdcb..9c07e6c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -96,6 +96,7 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
- [#5984](https://github.com/apache/trafficcontrol/issues/5894) -
`/servers/{{ID}}/deliveryservices` returns incorrect response for the requested
API version
- [#6027](https://github.com/apache/trafficcontrol/issues/6027) - Collapsed DB
migrations
- [#6066](https://github.com/apache/trafficcontrol/issues/6066) - Fixed
missing/incorrect indices on some tables
+- [#5576](https://github.com/apache/trafficcontrol/issues/5576) - Inconsistent
Profile Name restrictions
### Changed
- Updated the Traffic Ops Python client to 3.0
diff --git a/docs/source/overview/profiles_and_parameters.rst
b/docs/source/overview/profiles_and_parameters.rst
index 154cd9d..c92b2546 100644
--- a/docs/source/overview/profiles_and_parameters.rst
+++ b/docs/source/overview/profiles_and_parameters.rst
@@ -56,6 +56,11 @@ Name
""""
Ostensibly this is simply the Profile's name. However, the name of a Profile
has drastic consequences for how Traffic Control treats it. Particularly, the
name of a Profile is heavily conflated with its Type_. These relationships are
discussed further in the Type_ section, on a Type-by-Type basis.
+The Name of a Profile may not contain spaces.
+
+.. versionchanged:: ATCv6
+ In older versions of :abbr:`ATC (Apache Traffic Control)`, Profile
Names were allowed to contain spaces. The :ref:`to-api` will reject creation or
update of Profiles that have spaces in their Names as of :abbr:`ATC (Apache
Traffic Control)` version 6, so legacy Profiles will need to be updated to meet
this constraint before they can be modified.
+
.. _profile-routing-disabled:
Routing Disabled
diff --git a/lib/go-tc/profiles.go b/lib/go-tc/profiles.go
index 23b8362..0565147 100644
--- a/lib/go-tc/profiles.go
+++ b/lib/go-tc/profiles.go
@@ -24,6 +24,7 @@ import (
"errors"
"fmt"
"strconv"
+ "strings"
"github.com/apache/trafficcontrol/lib/go-log"
"github.com/apache/trafficcontrol/lib/go-tc/tovalidate"
@@ -145,7 +146,21 @@ func (profileImport *ProfileImportRequest) Validate(tx
*sql.Tx) error {
// Profile fields are valid
errs := tovalidate.ToErrors(validation.Errors{
- "name": validation.Validate(profile.Name,
validation.Required),
+ "name": validation.Validate(profile.Name, validation.By(
+ func(value interface{}) error {
+ name, ok := value.(*string)
+ if !ok {
+ return fmt.Errorf("wrong type, need:
string, got: %T", value)
+ }
+ if name == nil || *name == "" {
+ return errors.New("required and cannot
be blank")
+ }
+ if strings.Contains(*name, " ") {
+ return errors.New("cannot contain
spaces")
+ }
+ return nil
+ },
+ )),
"description": validation.Validate(profile.Description,
validation.Required),
"cdnName": validation.Validate(profile.CDNName,
validation.Required),
"type": validation.Validate(profile.Type,
validation.Required),
diff --git a/traffic_ops/testing/api/v4/profiles_test.go
b/traffic_ops/testing/api/v4/profiles_test.go
index 71da5e5..0d84916 100644
--- a/traffic_ops/testing/api/v4/profiles_test.go
+++ b/traffic_ops/testing/api/v4/profiles_test.go
@@ -32,25 +32,25 @@ import (
func TestProfiles(t *testing.T) {
WithObjs(t, []TCObj{CDNs, Types, Profiles, Parameters}, func() {
- CreateBadProfiles(t)
- UpdateTestProfiles(t)
+ t.Run("Attempt to create invalid Profiles", CreateBadProfiles)
+ t.Run("Basic update of properties", UpdateTestProfiles)
currentTime := time.Now().UTC().Add(-5 * time.Second)
time := currentTime.Format(time.RFC1123)
var header http.Header
header = make(map[string][]string)
header.Set(rfc.IfUnmodifiedSince, time)
- UpdateTestProfilesWithHeaders(t, header)
- GetTestProfilesIMS(t)
- GetTestProfiles(t)
- GetTestProfilesWithParameters(t)
- ImportProfile(t)
- CopyProfile(t)
+ t.Run("Try to update a Profile with If-Unmodified-Since set to
5 seconds ago - presumably before its creation", testPreconditionFailed(header))
+ t.Run("Support for If-Modified-Since in GET",
GetTestProfilesIMS)
+ t.Run("Basic GET request for /profiles and export endpoint",
GetTestProfiles)
+ t.Run("Check 'parameters' property returned in GET requsets",
GetTestProfilesWithParameters)
+ t.Run("Import endpoint basic operation", ImportProfile)
+ t.Run("Copy endpoint basic operation", CopyProfile)
header = make(map[string][]string)
etag := rfc.ETag(currentTime)
header.Set(rfc.IfMatch, etag)
- UpdateTestProfilesWithHeaders(t, header)
- GetTestPaginationSupportProfiles(t)
- CUDProfileWithLocks(t)
+ t.Run("Try to update a Profile with If-Match set",
testPreconditionFailed(header))
+ t.Run("Verify pagination query string parameters support",
GetTestPaginationSupportProfiles)
+ t.Run("Verify Profile endpoints are locked by CDN locks",
CUDProfileWithLocks)
})
}
@@ -178,6 +178,12 @@ func CUDProfileWithLocks(t *testing.T) {
}
}
+func testPreconditionFailed(h http.Header) func(*testing.T) {
+ return func(t *testing.T) {
+ UpdateTestProfilesWithHeaders(t, h)
+ }
+}
+
func UpdateTestProfilesWithHeaders(t *testing.T, header http.Header) {
if len(testData.Profiles) < 1 {
t.Fatal("Need at least one Profile to test updating a Profile
with HTTP headers")
@@ -254,6 +260,14 @@ func CopyProfile(t *testing.T) {
err string
}{
{
+ description: "new profile name contains spaces",
+ profile: tc.ProfileCopy{
+ ExistingName: "EDGE1",
+ Name: "Profile Copy",
+ },
+ err: "cannot contain spaces",
+ },
+ {
description: "copy profile",
profile: tc.ProfileCopy{
Name: "profile-2",
@@ -332,7 +346,11 @@ func CopyProfile(t *testing.T) {
func CreateTestProfiles(t *testing.T) {
opts := client.NewRequestOptions()
+ var cdnID int
+ var typ string
for _, pr := range testData.Profiles {
+ cdnID = pr.CDNID
+ typ = pr.Type
resp, _, err := TOSession.CreateProfile(pr,
client.RequestOptions{})
if err != nil {
t.Errorf("could not create Profile '%s': %v - alerts:
%+v", pr.Name, err, resp.Alerts)
@@ -388,6 +406,19 @@ func CreateTestProfiles(t *testing.T) {
}
}
+
+ p := tc.Profile{
+ CDNID: cdnID,
+ Description: "test Profile creation with a name that contains
spaces",
+ Name: "A Profile that has spaces in its name",
+ Type: typ,
+ }
+ resp, _, err := TOSession.CreateProfile(p, client.RequestOptions{})
+ if err == nil {
+ t.Error("Expected an error trying to create a Profile with a
Name that has spaces in it")
+ } else if !alertsHaveError(resp.Alerts, "cannot contain spaces") {
+ t.Errorf("Expected an error about spaces in the Profile name,
got: %v - alerts: %+v", err, resp.Alerts)
+ }
}
// Note this test will break if certain changes are made to the content and/or
@@ -567,6 +598,15 @@ func ImportProfile(t *testing.T) {
Name: *newParam.Name,
Value: *newParam.Value,
})
+
+ *profile.Name = "Test Profile Import"
+ importReq.Profile = profile
+ importResp, _, err = TOSession.ImportProfile(importReq,
client.RequestOptions{})
+ if err == nil {
+ t.Error("Expected an error importing a Profile with a space in
its name")
+ } else if !alertsHaveError(importResp.Alerts.Alerts, "cannot contain
spaces") {
+ t.Errorf("Expected an error about the Profile name containing
spaces, got: %v - alerts: %+v", err, importResp.Alerts)
+ }
}
func GetTestProfilesWithParameters(t *testing.T) {
diff --git a/traffic_ops/traffic_ops_golang/profile/copy.go
b/traffic_ops/traffic_ops_golang/profile/copy.go
index 40669cf..f7d6d58 100644
--- a/traffic_ops/traffic_ops_golang/profile/copy.go
+++ b/traffic_ops/traffic_ops_golang/profile/copy.go
@@ -24,6 +24,7 @@ import (
"errors"
"fmt"
"net/http"
+ "strings"
"github.com/apache/trafficcontrol/lib/go-log"
"github.com/apache/trafficcontrol/lib/go-tc"
@@ -71,6 +72,18 @@ func CopyProfileHandler(w http.ResponseWriter, r
*http.Request) {
}
func copyProfile(inf *api.APIInfo, p *tc.ProfileCopy) errorDetails {
+ if inf == nil || p == nil {
+ return errorDetails{
+ sysErr: errors.New("copyProfile received nil APIInfo
or ProfileCopy reference"),
+ errCode: http.StatusInternalServerError,
+ }
+ }
+ if strings.Contains(p.Name, " ") {
+ return errorDetails{
+ userErr: errors.New("new Profile name cannot contain
spaces"),
+ errCode: http.StatusBadRequest,
+ }
+ }
// check if the newProfile already exists
ok, err := tc.ProfileExistsByName(p.Name, inf.Tx.Tx)
if ok {
diff --git a/traffic_ops/traffic_ops_golang/profile/profiles.go
b/traffic_ops/traffic_ops_golang/profile/profiles.go
index a89016a..6e12387 100644
--- a/traffic_ops/traffic_ops_golang/profile/profiles.go
+++ b/traffic_ops/traffic_ops_golang/profile/profiles.go
@@ -1,3 +1,8 @@
+// Package profile includes logic and handlers for Profile-related API
+// endpoints, including /profiles, /profiles/name/{{name}}/parameters,
+// /profiles/{{ID}}/parameters,
+// /profiles/name/{{New Profile Name}}/copy/{{existing Profile Name}},
+// /profiles/import, and /profiles/{{ID}}/export.
package profile
/*
@@ -21,9 +26,10 @@ package profile
import (
"errors"
-
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
+ "fmt"
"net/http"
"strconv"
+ "strings"
"time"
"github.com/apache/trafficcontrol/lib/go-log"
@@ -34,11 +40,13 @@ import (
"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/parameter"
+
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
validation "github.com/go-ozzo/ozzo-validation"
"github.com/jmoiron/sqlx"
)
+// Supported (non-pagination) query string parameters for /profiles.
const (
CDNQueryParam = "cdn"
DescriptionQueryParam = "description"
@@ -96,7 +104,21 @@ func (prof *TOProfile) GetType() string {
func (prof *TOProfile) Validate() error {
errs := validation.Errors{
- NameQueryParam: validation.Validate(prof.Name,
validation.Required),
+ NameQueryParam: validation.Validate(prof.Name, validation.By(
+ func(value interface{}) error {
+ name, ok := value.(*string)
+ if !ok {
+ return fmt.Errorf("wrong type, need:
string, got: %T", value)
+ }
+ if name == nil || *name == "" {
+ return errors.New("required and cannot
be blank")
+ }
+ if strings.Contains(*name, " ") {
+ return errors.New("cannot contain
spaces")
+ }
+ return nil
+ },
+ )),
DescriptionQueryParam: validation.Validate(prof.Description,
validation.Required),
CDNQueryParam: validation.Validate(prof.CDNID,
validation.Required),
TypeQueryParam: validation.Validate(prof.Type,
validation.Required),
diff --git a/traffic_ops/traffic_ops_golang/profile/profiles_test.go
b/traffic_ops/traffic_ops_golang/profile/profiles_test.go
index 4a25ded..26a98dc 100644
--- a/traffic_ops/traffic_ops_golang/profile/profiles_test.go
+++ b/traffic_ops/traffic_ops_golang/profile/profiles_test.go
@@ -22,6 +22,7 @@ package profile
import (
"errors"
"reflect"
+ "strings"
"testing"
"time"
@@ -141,11 +142,26 @@ func TestValidate(t *testing.T) {
expected := util.JoinErrsStr(test.SortErrors([]error{
errors.New("'cdn' cannot be blank"),
errors.New("'description' cannot be blank"),
- errors.New("'name' cannot be blank"),
+ errors.New("'name' required and cannot be blank"),
errors.New("'type' cannot be blank"),
}))
if !reflect.DeepEqual(expected, errs) {
t.Errorf("expected %++v, got %++v", expected, errs)
}
+
+ p.CDNID = new(int)
+ *p.CDNID = 1
+ p.Description = new(string)
+ *p.Description = "description"
+ p.Name = new(string)
+ *p.Name = "A name with spaces"
+ p.Type = new(string)
+ *p.Type = "type"
+
+ err := p.Validate()
+ if !strings.Contains(err.Error(), "cannot contain spaces") {
+ t.Error("Expected an error about the Profile name containing
spaces")
+ }
+
}
diff --git
a/traffic_portal/app/src/common/modules/form/profile/form.profile.tpl.html
b/traffic_portal/app/src/common/modules/form/profile/form.profile.tpl.html
index 5538c0a..e281f48 100644
--- a/traffic_portal/app/src/common/modules/form/profile/form.profile.tpl.html
+++ b/traffic_portal/app/src/common/modules/form/profile/form.profile.tpl.html
@@ -51,7 +51,7 @@ under the License.
<div class="form-group" ng-class="{'has-error':
hasError(profileForm.name), 'has-feedback': hasError(profileForm.name)}">
<label class="control-label col-md-2 col-sm-2 col-xs-12">Name
*</label>
<div class="col-md-10 col-sm-10 col-xs-12">
- <input name="name" type="text" class="form-control"
ng-model="profile.name" ng-pattern="/^\S*$/" required autofocus>
+ <input name="name" type="text" class="form-control"
ng-model="profile.name" pattern="\S+" required autofocus>
<small class="input-error"
ng-show="hasPropertyError(profileForm.name, 'required')">Required</small>
<small class="input-error"
ng-show="hasPropertyError(profileForm.name, 'maxlength')">Too Long</small>
<small class="input-error"
ng-show="hasPropertyError(profileForm.name, 'pattern')">No spaces</small>
