zrhoffman commented on code in PR #7099:
URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1025419058
##########
lib/go-tc/copy.go:
##########
@@ -0,0 +1,112 @@
+package tc
Review Comment:
`copy.go` belongs in the `go-utils` package
##########
traffic_ops/testing/api/utils/utils.go:
##########
@@ -129,7 +140,7 @@ type V3TestDataT[B any] struct {
// V4TestData represents the data needed for testing the v4 api endpoints.
type V4TestData struct {
- EndpointId func() int
+ EndpointID func() int
Review Comment:
Renaming `V4TestData.EndpointId` to `V4TestData.EndpointID` is unrelated to
reworking the Delivery Service `Active` flag and creates a large diff. Please
revert it (happy to review if you put it in a separate PR).
##########
traffic_ops/testing/api/utils/utils.go:
##########
@@ -86,29 +94,32 @@ func Compare(t *testing.T, expected []string, alertsStrs
[]string) {
}
// CreateV3Session creates a session for client v4 using the passed in
username and password.
-func CreateV3Session(t *testing.T, TrafficOpsURL string, username string,
password string, toReqTimeout int) *v3client.Session {
- userSession, _, err := v3client.LoginWithAgent(TrafficOpsURL, username,
password, true, "to-api-v3-client-tests", false,
time.Second*time.Duration(toReqTimeout))
+func CreateV3Session(t *testing.T, trafficOpsURL string, username string,
password string, toReqTimeout int) *v3client.Session {
+ t.Helper()
+ userSession, _, err := v3client.LoginWithAgent(trafficOpsURL, username,
password, true, "to-api-v3-client-tests", false,
time.Second*time.Duration(toReqTimeout))
assert.RequireNoError(t, err, "Could not login with user %v: %v",
username, err)
return userSession
}
// CreateV4Session creates a session for client v4 using the passed in
username and password.
-func CreateV4Session(t *testing.T, TrafficOpsURL string, username string,
password string, toReqTimeout int) *v4client.Session {
- userSession, _, err := v4client.LoginWithAgent(TrafficOpsURL, username,
password, true, "to-api-v4-client-tests", false,
time.Second*time.Duration(toReqTimeout))
+func CreateV4Session(t *testing.T, trafficOpsURL string, username string,
password string, toReqTimeout int) *v4client.Session {
+ t.Helper()
+ userSession, _, err := v4client.LoginWithAgent(trafficOpsURL, username,
password, true, "to-api-v4-client-tests", false,
time.Second*time.Duration(toReqTimeout))
assert.RequireNoError(t, err, "Could not login with user %v: %v",
username, err)
return userSession
}
// CreateV5Session creates a session for client v5 using the passed in
username and password.
-func CreateV5Session(t *testing.T, TrafficOpsURL, username, password string,
toReqTimeout int) *v5client.Session {
- userSession, _, err := v5client.LoginWithAgent(TrafficOpsURL, username,
password, true, "to-api-v5-client-tests", false,
time.Second*time.Duration(toReqTimeout))
+func CreateV5Session(t *testing.T, trafficOpsURL, username, password string,
toReqTimeout int) *v5client.Session {
+ t.Helper()
+ userSession, _, err := v5client.LoginWithAgent(trafficOpsURL, username,
password, true, "to-api-v5-client-tests", false,
time.Second*time.Duration(toReqTimeout))
assert.RequireNoError(t, err, "Could not login with user %v: %v",
username, err)
return userSession
}
// V3TestData represents the data needed for testing the v3 api endpoints.
type V3TestData struct {
- EndpointId func() int
+ EndpointID func() int
Review Comment:
Renaming `V3TestData.EndpointId` to `V3TestData.EndpointID` is unrelated to
reworking the Delivery Service `Active` flag and creates a large diff. Please
revert it (happy to review if you put it in a separate PR).
##########
traffic_ops/testing/api/utils/utils.go:
##########
@@ -119,7 +130,7 @@ type V3TestData struct {
// V3TestDataT represents the data needed for testing the v3 api endpoints.
type V3TestDataT[B any] struct {
- EndpointId func() int
+ EndpointID func() int
Review Comment:
Renaming `V3TestDataT.EndpointId` to `V3TestDataT.EndpointID` is unrelated
to reworking the Delivery Service `Active` flag and creates a large diff.
Please revert it (happy to review if you put it in a separate PR).
##########
traffic_ops/testing/api/utils/utils.go:
##########
@@ -139,7 +150,7 @@ type V4TestData struct {
// V5TestData represents the data needed for testing the v5 api endpoints.
type V5TestData struct {
- EndpointId func() int
+ EndpointID func() int
Review Comment:
Renaming `V5TestData.EndpointId` to `V5TestData.EndpointID` is unrelated to
reworking the Delivery Service `Active` flag and creates a large diff. Please
revert it (happy to review if you put it in a separate PR).
##########
traffic_ops/testing/api/utils/utils.go:
##########
@@ -157,7 +168,7 @@ type requestOpts interface {
// TestData represents the data needed for testing the api endpoints.
type TestData[C clientSession, R requestOpts, B any] struct {
- EndpointId func() int
+ EndpointID func() int
Review Comment:
Renaming `TestData.EndpointId` to `TestData.EndpointID` is unrelated to
reworking the Delivery Service `Active` flag and creates a large diff. Please
revert it (happy to review if you put it in a separate PR).
##########
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go:
##########
@@ -264,7 +265,7 @@ func TestReadGetDeliveryServices(t *testing.T) {
key string
value driver.Value
}{
- {"active", true},
+ {"active", "ACTIVE"},
Review Comment:
`"ACTIVE"` should be `tc.DSActiveStateActive`
##########
traffic_ops/traffic_ops_golang/crconfig/servers.go:
##########
@@ -274,7 +274,7 @@ inner join deliveryservice_regex as dsr on dsr.regex = r.id
inner join deliveryservice as ds on ds.id = dsr.deliveryservice
inner join type as dt on dt.id = ds.type
where ds.cdn_id = (select id from cdn where name = $1)
-and ds.active = true` +
+and ds.active = 'ACTIVE'` +
Review Comment:
`'ACTIVE'` should be `tc.DSActiveStateActive`, either as a concatenation or
a parameter
##########
traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go:
##########
@@ -500,7 +500,7 @@ inner join deliveryservice as d on d.id = dr.deliveryservice
inner join type as t on t.id = r.type
inner join type as dt on dt.id = d.type
where d.cdn_id = (select id from cdn where name = $1)
-and d.active = true
+and d.active = 'ACTIVE'
Review Comment:
`'ACTIVE'` should be `tc.DSActiveStateActive`, either as a concatenation or
a parameter
##########
traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go:
##########
@@ -465,7 +465,7 @@ from staticdnsentry as e
inner join deliveryservice as d on d.id = e.deliveryservice
inner join type as t on t.id = e.type
where d.cdn_id = (select id from cdn where name = $1)
-and d.active = true
+and d.active = 'ACTIVE'
Review Comment:
`'ACTIVE'` should be `tc.DSActiveStateActive`, either as a concatenation or
a parameter
##########
traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go:
##########
@@ -617,7 +617,7 @@ INNER JOIN type AS dt ON dt.id = ds.type
INNER JOIN profile AS p ON p.id = s.profile
INNER JOIN status AS st ON st.id = s.status
WHERE ds.cdn_id = (SELECT id FROM cdn WHERE name = $1)
-AND ds.active = true
+AND ds.active = 'ACTIVE'
Review Comment:
`'ACTIVE'` should be `tc.DSActiveStateActive`, either as a concatenation or
a parameter
##########
lib/go-tc/copy_test.go:
##########
@@ -0,0 +1,166 @@
+package tc
Review Comment:
`copy_test.go` belongs in the `go-utils` package
##########
traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go:
##########
@@ -145,7 +145,7 @@ FROM deliveryservice AS d
INNER JOIN type AS t ON t.id = d.type
LEFT OUTER JOIN profile AS p ON p.id = d.profile
WHERE d.cdn_id = (select id FROM cdn WHERE name = $1)
-AND d.active = true
+AND d.active = 'ACTIVE'
Review Comment:
`'ACTIVE'` should be `tc.DSActiveStateActive`, either as a concatenation or
a parameter
##########
traffic_ops/traffic_ops_golang/server/servers.go:
##########
@@ -2094,7 +2094,7 @@ FROM deliveryservice_server dss
JOIN server s ON dss.server = s.id
JOIN type t ON s.type = t.id
JOIN deliveryservice ds ON dss.deliveryservice = ds.id
-WHERE t.name LIKE $1 AND ds.active
+WHERE t.name LIKE $1 AND ds.active = 'ACTIVE'
Review Comment:
`'ACTIVE'` should be `tc.DSActiveStateActive`, either as a concatenation or
a parameter
##########
traffic_ops/traffic_ops_golang/server/servers_assignment.go:
##########
@@ -74,7 +74,7 @@ WHERE d.id IN (
FROM deliveryservice_server dss
INNER JOIN deliveryservice d ON d.id = dss.deliveryservice
WHERE dss.server=$1
- AND d.active
+ AND d.active = 'ACTIVE'
Review Comment:
`'ACTIVE'` should be `tc.DSActiveStateActive`, either as a concatenation or
a parameter
##########
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##########
@@ -1090,135 +1236,170 @@ func updateV41(w http.ResponseWriter, r
*http.Request, inf *api.APIInfo, dsV4 *t
return nil, http.StatusInternalServerError, nil,
fmt.Errorf("updating TLS versions for DS #%d: %w", *ds.ID, err)
}
- newDSType, err := getTypeFromID(*ds.TypeID, tx)
+ newDSType, err := getTypeFromID(ds.TypeID, tx)
if err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("getting delivery service type after update: " + err.Error())
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("getting delivery service type after update: %w", err)
}
- ds.Type = &newDSType
+ ds.Type = (*string)(&newDSType)
cdnDomain, err := getCDNDomain(*ds.ID, tx) // need to get the domain
again, in case it changed.
if err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("getting CDN domain: (" + cdnDomain + ") after update: " +
err.Error())
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("getting CDN domain: (%s) after update: %w", cdnDomain, err)
}
- matchLists, err := GetDeliveryServicesMatchLists([]string{*ds.XMLID},
tx)
+ matchLists, err := GetDeliveryServicesMatchLists([]string{ds.XMLID}, tx)
if err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("getting matchlists after update: " + err.Error())
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("getting matchlists after update: %w", err)
}
- if ml, ok := matchLists[*ds.XMLID]; !ok {
+ ml, ok := matchLists[ds.XMLID]
+ if !ok {
return nil, http.StatusInternalServerError, nil, errors.New("no
matchlists after update")
- } else {
- ds.MatchList = &ml
}
+ ds.MatchList = ml
- if err := EnsureParams(tx, *ds.ID, *ds.XMLID, ds.EdgeHeaderRewrite,
ds.MidHeaderRewrite, ds.RegexRemap, ds.SigningAlgorithm, newDSType,
ds.MaxOriginConnections); err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("ensuring ds parameters:: " + err.Error())
+ if err := EnsureParams(tx, *ds.ID, ds.XMLID, ds.EdgeHeaderRewrite,
ds.MidHeaderRewrite, ds.RegexRemap, ds.SigningAlgorithm, newDSType,
ds.MaxOriginConnections); err != nil {
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("ensuring ds parameters: %w", err)
}
- if oldDetails.OldOrgServerFqdn != nil && ds.OrgServerFQDN != nil &&
*oldDetails.OldOrgServerFqdn != *ds.OrgServerFQDN {
- if err := updatePrimaryOrigin(tx, user, ds); err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("updating delivery service: " + err.Error())
+ if oldDetails.OldOrgServerFQDN != nil && ds.OrgServerFQDN != nil &&
*oldDetails.OldOrgServerFQDN != *ds.OrgServerFQDN {
+ if err := updatePrimaryOrigin(tx, user, *ds); err != nil {
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("updating delivery service: %w", err)
}
}
- ds.LastUpdated = &lastUpdated
+ ds.LastUpdated = lastUpdated
// the update may change or delete the query params -- delete existing
and re-add if any provided
q := `DELETE FROM deliveryservice_consistent_hash_query_param WHERE
deliveryservice_id = $1`
if res, err := tx.Exec(q, *ds.ID); err != nil {
- return nil, http.StatusInternalServerError, nil,
fmt.Errorf("deleting consistent hash query params for ds %s: %w", *ds.XMLID,
err)
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("deleting consistent hash query params for ds %s: %w", ds.XMLID, err)
} else if c, _ := res.RowsAffected(); c > 0 {
- api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*ds.XMLID+", ID:
"+strconv.Itoa(*ds.ID)+", ACTION: Deleted "+strconv.FormatInt(c, 10)+"
consistent hash query params", user, tx)
+ api.CreateChangeLogRawTx(api.ApiChange, "DS: "+ds.XMLID+", ID:
"+strconv.Itoa(*ds.ID)+", ACTION: Deleted "+strconv.FormatInt(c, 10)+"
consistent hash query params", user, tx)
}
if _, err = createConsistentHashQueryParams(tx, *ds.ID,
ds.ConsistentHashQueryParams); err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
return nil, code, usrErr, sysErr
}
- if err := api.CreateChangeLogRawErr(api.ApiChange, "Updated ds:
"+*ds.XMLID+" id: "+strconv.Itoa(*ds.ID), user, tx); err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("writing change log entry: " + err.Error())
+ if err := api.CreateChangeLogRawErr(api.ApiChange, "Updated ds:
"+ds.XMLID+" id: "+strconv.Itoa(*ds.ID), user, tx); err != nil {
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("writing change log entry: %w", err)
}
- dsV4 = &ds
-
if inf.Config.TrafficVaultEnabled && ds.Protocol != nil &&
(*ds.Protocol == tc.DSProtocolHTTPS || *ds.Protocol ==
tc.DSProtocolHTTPAndHTTPS || *ds.Protocol == tc.DSProtocolHTTPToHTTPS) {
- err, errCode := GeneratePlaceholderSelfSignedCert(*dsV4, inf,
r.Context())
+ err, errCode := GeneratePlaceholderSelfSignedCert(*ds, inf,
r.Context())
if err != nil || errCode != http.StatusOK {
- return nil, errCode, nil, fmt.Errorf("creating self
signed default cert: %v", err)
+ return nil, errCode, nil, fmt.Errorf("creating self
signed default cert: %w", err)
}
}
- return dsV4, http.StatusOK, nil, nil
+ return ds, http.StatusOK, nil, nil
}
-// Delete is the DeliveryService implementation of the Deleter interface.
+// Delete implements the
+// github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api.Deleter
+// interface (given that the
+//
github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api.APIInfoer
+// and
+//
github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api.Identifier
+// interfaces are already implemented).
func (ds *TODeliveryService) Delete() (error, error, int) {
if ds.ID == nil {
return errors.New("missing id"), nil, http.StatusBadRequest
}
xmlID, ok, err := GetXMLID(ds.ReqInfo.Tx.Tx, *ds.ID)
if err != nil {
- return nil, errors.New("ds delete: getting xmlid: " +
err.Error()), http.StatusInternalServerError
+ return nil, fmt.Errorf("ds delete: getting xmlid: %w", err),
http.StatusInternalServerError
} else if !ok {
return errors.New("delivery service not found"), nil,
http.StatusNotFound
}
- ds.XMLID = &xmlID
+ ds.XMLID = xmlID
- if ds.CDNID != nil {
- userErr, sysErr, errCode :=
dbhelpers.CheckIfCurrentUserCanModifyCDNWithID(ds.APIInfo().Tx.Tx,
int64(*ds.CDNID), ds.APIInfo().User.UserName)
- if userErr != nil || sysErr != nil {
- return userErr, sysErr, errCode
- }
+ var userErr error
+ var sysErr error
+ var errCode int
+ if ds.CDNID != 0 {
+ userErr, sysErr, errCode =
dbhelpers.CheckIfCurrentUserCanModifyCDNWithID(ds.APIInfo().Tx.Tx,
int64(ds.CDNID), ds.APIInfo().User.UserName)
} else if ds.CDNName != nil {
- userErr, sysErr, errCode :=
dbhelpers.CheckIfCurrentUserCanModifyCDN(ds.APIInfo().Tx.Tx, *ds.CDNName,
ds.APIInfo().User.UserName)
- if userErr != nil || sysErr != nil {
- return userErr, sysErr, errCode
- }
+ userErr, sysErr, errCode =
dbhelpers.CheckIfCurrentUserCanModifyCDN(ds.APIInfo().Tx.Tx, *ds.CDNName,
ds.APIInfo().User.UserName)
} else {
_, cdnName, _, err :=
dbhelpers.GetDSNameAndCDNFromID(ds.ReqInfo.Tx.Tx, *ds.ID)
if err != nil {
- return nil, fmt.Errorf("couldn't get cdn name for DS:
%v", err), http.StatusBadRequest
- }
- userErr, sysErr, errCode :=
dbhelpers.CheckIfCurrentUserCanModifyCDN(ds.APIInfo().Tx.Tx, string(cdnName),
ds.APIInfo().User.UserName)
- if userErr != nil || sysErr != nil {
- return userErr, sysErr, errCode
+ return nil, fmt.Errorf("couldn't get cdn name for DS:
%w", err), http.StatusBadRequest
}
+ userErr, sysErr, errCode =
dbhelpers.CheckIfCurrentUserCanModifyCDN(ds.APIInfo().Tx.Tx, string(cdnName),
ds.APIInfo().User.UserName)
}
+ if userErr != nil || sysErr != nil {
+ return userErr, sysErr, errCode
+ }
+
// Note ds regexes MUST be deleted before the ds, because there's a ON
DELETE CASCADE on deliveryservice_regex (but not on regex).
// Likewise, it MUST happen in a transaction with the later DS delete,
so they aren't deleted if the DS delete fails.
if _, err := ds.ReqInfo.Tx.Tx.Exec(`DELETE FROM regex WHERE id IN
(SELECT regex FROM deliveryservice_regex WHERE deliveryservice=$1)`, *ds.ID);
err != nil {
- return nil, errors.New("TODeliveryService.Delete deleting
regexes for delivery service: " + err.Error()), http.StatusInternalServerError
+ return nil, fmt.Errorf("TODeliveryService.Delete deleting
regexes for delivery service: %w", err), http.StatusInternalServerError
}
if _, err := ds.ReqInfo.Tx.Tx.Exec(`DELETE FROM deliveryservice_regex
WHERE deliveryservice=$1`, *ds.ID); err != nil {
- return nil, errors.New("TODeliveryService.Delete deleting
delivery service regexes: " + err.Error()), http.StatusInternalServerError
+ return nil, fmt.Errorf("TODeliveryService.Delete deleting
delivery service regexes: %w", err), http.StatusInternalServerError
}
- userErr, sysErr, errCode := api.GenericDelete(ds)
+ userErr, sysErr, errCode = api.GenericDelete(ds)
if userErr != nil || sysErr != nil {
return userErr, sysErr, errCode
}
paramConfigFilePrefixes := []string{"hdr_rw_", "hdr_rw_mid_",
"regex_remap_", "cacheurl_"}
configFiles := []string{}
for _, prefix := range paramConfigFilePrefixes {
- configFiles = append(configFiles, prefix+*ds.XMLID+".config")
+ configFiles = append(configFiles, prefix+ds.XMLID+".config")
}
if _, err := ds.ReqInfo.Tx.Tx.Exec(`DELETE FROM parameter WHERE name =
'location' AND config_file = ANY($1)`, pq.Array(configFiles)); err != nil {
- return nil, errors.New("TODeliveryService.Delete deleting
delivery service parameteres: " + err.Error()), http.StatusInternalServerError
+ return nil, fmt.Errorf("TODeliveryService.Delete deleting
delivery service parameters: %w", err), http.StatusInternalServerError
}
return nil, nil, http.StatusOK
}
-func (v *TODeliveryService) DeleteQuery() string {
+// DeleteQuery implements part of the
+//
github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api.GenericDeleter
+// interface.
+func (*TODeliveryService) DeleteQuery() string {
return `DELETE FROM deliveryservice WHERE id = :id`
}
-func readGetDeliveryServices(h http.Header, params map[string]string, tx
*sqlx.Tx, user *auth.CurrentUser, useIMS bool) ([]tc.DeliveryServiceV4, error,
error, int, *time.Time) {
+// addActive adds the query parameter for a DS's "active" state for the
+// appropriate given API version to the provided 'WHERE' clause, and updates
+// queryValues with the client-provided value from params as necessary.
+func addActive(where string, params map[string]string, queryValues
map[string]interface{}, version api.Version) (string, error) {
+ active, ok := params["active"]
+ if !ok {
+ return where, nil
+ }
+ if version.Major < 5 {
+ b, err := strconv.ParseBool(active)
+ if err != nil {
+ return where, fmt.Errorf("active cannot parse to
boolean: %w", err)
+ }
+ if b {
+ return dbhelpers.AppendWhere(where, "ds.active =
'ACTIVE'"), nil
+ }
+ return dbhelpers.AppendWhere(where, "ds.active <> 'ACTIVE'"),
nil
Review Comment:
`ACTIVE` should be `tc.DSActiveStateActive`
##########
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##########
@@ -1090,135 +1236,170 @@ func updateV41(w http.ResponseWriter, r
*http.Request, inf *api.APIInfo, dsV4 *t
return nil, http.StatusInternalServerError, nil,
fmt.Errorf("updating TLS versions for DS #%d: %w", *ds.ID, err)
}
- newDSType, err := getTypeFromID(*ds.TypeID, tx)
+ newDSType, err := getTypeFromID(ds.TypeID, tx)
if err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("getting delivery service type after update: " + err.Error())
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("getting delivery service type after update: %w", err)
}
- ds.Type = &newDSType
+ ds.Type = (*string)(&newDSType)
cdnDomain, err := getCDNDomain(*ds.ID, tx) // need to get the domain
again, in case it changed.
if err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("getting CDN domain: (" + cdnDomain + ") after update: " +
err.Error())
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("getting CDN domain: (%s) after update: %w", cdnDomain, err)
}
- matchLists, err := GetDeliveryServicesMatchLists([]string{*ds.XMLID},
tx)
+ matchLists, err := GetDeliveryServicesMatchLists([]string{ds.XMLID}, tx)
if err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("getting matchlists after update: " + err.Error())
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("getting matchlists after update: %w", err)
}
- if ml, ok := matchLists[*ds.XMLID]; !ok {
+ ml, ok := matchLists[ds.XMLID]
+ if !ok {
return nil, http.StatusInternalServerError, nil, errors.New("no
matchlists after update")
- } else {
- ds.MatchList = &ml
}
+ ds.MatchList = ml
- if err := EnsureParams(tx, *ds.ID, *ds.XMLID, ds.EdgeHeaderRewrite,
ds.MidHeaderRewrite, ds.RegexRemap, ds.SigningAlgorithm, newDSType,
ds.MaxOriginConnections); err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("ensuring ds parameters:: " + err.Error())
+ if err := EnsureParams(tx, *ds.ID, ds.XMLID, ds.EdgeHeaderRewrite,
ds.MidHeaderRewrite, ds.RegexRemap, ds.SigningAlgorithm, newDSType,
ds.MaxOriginConnections); err != nil {
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("ensuring ds parameters: %w", err)
}
- if oldDetails.OldOrgServerFqdn != nil && ds.OrgServerFQDN != nil &&
*oldDetails.OldOrgServerFqdn != *ds.OrgServerFQDN {
- if err := updatePrimaryOrigin(tx, user, ds); err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("updating delivery service: " + err.Error())
+ if oldDetails.OldOrgServerFQDN != nil && ds.OrgServerFQDN != nil &&
*oldDetails.OldOrgServerFQDN != *ds.OrgServerFQDN {
+ if err := updatePrimaryOrigin(tx, user, *ds); err != nil {
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("updating delivery service: %w", err)
}
}
- ds.LastUpdated = &lastUpdated
+ ds.LastUpdated = lastUpdated
// the update may change or delete the query params -- delete existing
and re-add if any provided
q := `DELETE FROM deliveryservice_consistent_hash_query_param WHERE
deliveryservice_id = $1`
if res, err := tx.Exec(q, *ds.ID); err != nil {
- return nil, http.StatusInternalServerError, nil,
fmt.Errorf("deleting consistent hash query params for ds %s: %w", *ds.XMLID,
err)
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("deleting consistent hash query params for ds %s: %w", ds.XMLID, err)
} else if c, _ := res.RowsAffected(); c > 0 {
- api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*ds.XMLID+", ID:
"+strconv.Itoa(*ds.ID)+", ACTION: Deleted "+strconv.FormatInt(c, 10)+"
consistent hash query params", user, tx)
+ api.CreateChangeLogRawTx(api.ApiChange, "DS: "+ds.XMLID+", ID:
"+strconv.Itoa(*ds.ID)+", ACTION: Deleted "+strconv.FormatInt(c, 10)+"
consistent hash query params", user, tx)
}
if _, err = createConsistentHashQueryParams(tx, *ds.ID,
ds.ConsistentHashQueryParams); err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
return nil, code, usrErr, sysErr
}
- if err := api.CreateChangeLogRawErr(api.ApiChange, "Updated ds:
"+*ds.XMLID+" id: "+strconv.Itoa(*ds.ID), user, tx); err != nil {
- return nil, http.StatusInternalServerError, nil,
errors.New("writing change log entry: " + err.Error())
+ if err := api.CreateChangeLogRawErr(api.ApiChange, "Updated ds:
"+ds.XMLID+" id: "+strconv.Itoa(*ds.ID), user, tx); err != nil {
+ return nil, http.StatusInternalServerError, nil,
fmt.Errorf("writing change log entry: %w", err)
}
- dsV4 = &ds
-
if inf.Config.TrafficVaultEnabled && ds.Protocol != nil &&
(*ds.Protocol == tc.DSProtocolHTTPS || *ds.Protocol ==
tc.DSProtocolHTTPAndHTTPS || *ds.Protocol == tc.DSProtocolHTTPToHTTPS) {
- err, errCode := GeneratePlaceholderSelfSignedCert(*dsV4, inf,
r.Context())
+ err, errCode := GeneratePlaceholderSelfSignedCert(*ds, inf,
r.Context())
if err != nil || errCode != http.StatusOK {
- return nil, errCode, nil, fmt.Errorf("creating self
signed default cert: %v", err)
+ return nil, errCode, nil, fmt.Errorf("creating self
signed default cert: %w", err)
}
}
- return dsV4, http.StatusOK, nil, nil
+ return ds, http.StatusOK, nil, nil
}
-// Delete is the DeliveryService implementation of the Deleter interface.
+// Delete implements the
+// github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api.Deleter
+// interface (given that the
+//
github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api.APIInfoer
+// and
+//
github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api.Identifier
+// interfaces are already implemented).
func (ds *TODeliveryService) Delete() (error, error, int) {
if ds.ID == nil {
return errors.New("missing id"), nil, http.StatusBadRequest
}
xmlID, ok, err := GetXMLID(ds.ReqInfo.Tx.Tx, *ds.ID)
if err != nil {
- return nil, errors.New("ds delete: getting xmlid: " +
err.Error()), http.StatusInternalServerError
+ return nil, fmt.Errorf("ds delete: getting xmlid: %w", err),
http.StatusInternalServerError
} else if !ok {
return errors.New("delivery service not found"), nil,
http.StatusNotFound
}
- ds.XMLID = &xmlID
+ ds.XMLID = xmlID
- if ds.CDNID != nil {
- userErr, sysErr, errCode :=
dbhelpers.CheckIfCurrentUserCanModifyCDNWithID(ds.APIInfo().Tx.Tx,
int64(*ds.CDNID), ds.APIInfo().User.UserName)
- if userErr != nil || sysErr != nil {
- return userErr, sysErr, errCode
- }
+ var userErr error
+ var sysErr error
+ var errCode int
+ if ds.CDNID != 0 {
+ userErr, sysErr, errCode =
dbhelpers.CheckIfCurrentUserCanModifyCDNWithID(ds.APIInfo().Tx.Tx,
int64(ds.CDNID), ds.APIInfo().User.UserName)
} else if ds.CDNName != nil {
- userErr, sysErr, errCode :=
dbhelpers.CheckIfCurrentUserCanModifyCDN(ds.APIInfo().Tx.Tx, *ds.CDNName,
ds.APIInfo().User.UserName)
- if userErr != nil || sysErr != nil {
- return userErr, sysErr, errCode
- }
+ userErr, sysErr, errCode =
dbhelpers.CheckIfCurrentUserCanModifyCDN(ds.APIInfo().Tx.Tx, *ds.CDNName,
ds.APIInfo().User.UserName)
} else {
_, cdnName, _, err :=
dbhelpers.GetDSNameAndCDNFromID(ds.ReqInfo.Tx.Tx, *ds.ID)
if err != nil {
- return nil, fmt.Errorf("couldn't get cdn name for DS:
%v", err), http.StatusBadRequest
- }
- userErr, sysErr, errCode :=
dbhelpers.CheckIfCurrentUserCanModifyCDN(ds.APIInfo().Tx.Tx, string(cdnName),
ds.APIInfo().User.UserName)
- if userErr != nil || sysErr != nil {
- return userErr, sysErr, errCode
+ return nil, fmt.Errorf("couldn't get cdn name for DS:
%w", err), http.StatusBadRequest
}
+ userErr, sysErr, errCode =
dbhelpers.CheckIfCurrentUserCanModifyCDN(ds.APIInfo().Tx.Tx, string(cdnName),
ds.APIInfo().User.UserName)
}
+ if userErr != nil || sysErr != nil {
+ return userErr, sysErr, errCode
+ }
+
// Note ds regexes MUST be deleted before the ds, because there's a ON
DELETE CASCADE on deliveryservice_regex (but not on regex).
// Likewise, it MUST happen in a transaction with the later DS delete,
so they aren't deleted if the DS delete fails.
if _, err := ds.ReqInfo.Tx.Tx.Exec(`DELETE FROM regex WHERE id IN
(SELECT regex FROM deliveryservice_regex WHERE deliveryservice=$1)`, *ds.ID);
err != nil {
- return nil, errors.New("TODeliveryService.Delete deleting
regexes for delivery service: " + err.Error()), http.StatusInternalServerError
+ return nil, fmt.Errorf("TODeliveryService.Delete deleting
regexes for delivery service: %w", err), http.StatusInternalServerError
}
if _, err := ds.ReqInfo.Tx.Tx.Exec(`DELETE FROM deliveryservice_regex
WHERE deliveryservice=$1`, *ds.ID); err != nil {
- return nil, errors.New("TODeliveryService.Delete deleting
delivery service regexes: " + err.Error()), http.StatusInternalServerError
+ return nil, fmt.Errorf("TODeliveryService.Delete deleting
delivery service regexes: %w", err), http.StatusInternalServerError
}
- userErr, sysErr, errCode := api.GenericDelete(ds)
+ userErr, sysErr, errCode = api.GenericDelete(ds)
if userErr != nil || sysErr != nil {
return userErr, sysErr, errCode
}
paramConfigFilePrefixes := []string{"hdr_rw_", "hdr_rw_mid_",
"regex_remap_", "cacheurl_"}
configFiles := []string{}
for _, prefix := range paramConfigFilePrefixes {
- configFiles = append(configFiles, prefix+*ds.XMLID+".config")
+ configFiles = append(configFiles, prefix+ds.XMLID+".config")
}
if _, err := ds.ReqInfo.Tx.Tx.Exec(`DELETE FROM parameter WHERE name =
'location' AND config_file = ANY($1)`, pq.Array(configFiles)); err != nil {
- return nil, errors.New("TODeliveryService.Delete deleting
delivery service parameteres: " + err.Error()), http.StatusInternalServerError
+ return nil, fmt.Errorf("TODeliveryService.Delete deleting
delivery service parameters: %w", err), http.StatusInternalServerError
}
return nil, nil, http.StatusOK
}
-func (v *TODeliveryService) DeleteQuery() string {
+// DeleteQuery implements part of the
+//
github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api.GenericDeleter
+// interface.
+func (*TODeliveryService) DeleteQuery() string {
return `DELETE FROM deliveryservice WHERE id = :id`
}
-func readGetDeliveryServices(h http.Header, params map[string]string, tx
*sqlx.Tx, user *auth.CurrentUser, useIMS bool) ([]tc.DeliveryServiceV4, error,
error, int, *time.Time) {
+// addActive adds the query parameter for a DS's "active" state for the
+// appropriate given API version to the provided 'WHERE' clause, and updates
+// queryValues with the client-provided value from params as necessary.
+func addActive(where string, params map[string]string, queryValues
map[string]interface{}, version api.Version) (string, error) {
+ active, ok := params["active"]
+ if !ok {
+ return where, nil
+ }
+ if version.Major < 5 {
+ b, err := strconv.ParseBool(active)
+ if err != nil {
+ return where, fmt.Errorf("active cannot parse to
boolean: %w", err)
+ }
+ if b {
+ return dbhelpers.AppendWhere(where, "ds.active =
'ACTIVE'"), nil
Review Comment:
`ACTIVE` should be `tc.DSActiveStateActive`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
