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 46a2792 Fix federations API when using same resolvers for multiple
DSes (#6109)
46a2792 is described below
commit 46a279204af553c51cee69bd03a6fbba2589208b
Author: Rawlin Peters <[email protected]>
AuthorDate: Mon Aug 16 18:01:41 2021 -0600
Fix federations API when using same resolvers for multiple DSes (#6109)
Instead of allowing the query to return no row on conflict, ensure it
always returns a row so that we can associate the resolver ID to the
federation.
Closes: #6104
---
CHANGELOG.md | 1 +
traffic_ops/testing/api/v4/federations_test.go | 109 +++++++++++++++++++++
.../traffic_ops_golang/federations/federations.go | 14 ++-
3 files changed, 116 insertions(+), 8 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b4539b7..bbc8cac 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -70,6 +70,7 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
- [#5609](https://github.com/apache/trafficcontrol/issues/5609) - Fixed GET
/servercheck filter for an extra query param.
- [#5954](https://github.com/apache/trafficcontrol/issues/5954) - Traffic Ops
HTTP response write errors are ignored
- [#6048](https://github.com/apache/trafficcontrol/issues/6048) - TM sometimes
sets cachegroups to unavailable even though all caches are online
+- [#6104](https://github.com/apache/trafficcontrol/issues/6104) - PUT
/api/x/federations only respects first item in request payload
- [#5288](https://github.com/apache/trafficcontrol/issues/5288) - Fixed the
ability to create and update a server with MTU value >= 1280.
- [#5284](https://github.com/apache/trafficcontrol/issues/5284) - Fixed error
message when creating a server with non-existent profile
- Fixed a NullPointerException in TR when a client passes a null SNI hostname
in a TLS request
diff --git a/traffic_ops/testing/api/v4/federations_test.go
b/traffic_ops/testing/api/v4/federations_test.go
index 071bb3d..c1958c6 100644
--- a/traffic_ops/testing/api/v4/federations_test.go
+++ b/traffic_ops/testing/api/v4/federations_test.go
@@ -19,6 +19,8 @@ import (
"errors"
"fmt"
"net/http"
+ "reflect"
+ "sort"
"testing"
"time"
@@ -33,6 +35,7 @@ func TestFederations(t *testing.T) {
GetTestFederations(t)
GetTestFederationsIMS(t)
AddFederationResolversForCurrentUserTest(t)
+ ReplaceFederationResolversForCurrentUserTest(t)
RemoveFederationResolversForCurrentUserTest(t)
})
}
@@ -268,3 +271,109 @@ func AddFederationResolversForCurrentUserTest(t
*testing.T) {
}
}
}
+
+func ReplaceFederationResolversForCurrentUserTest(t *testing.T) {
+ fedID, ds, ds1, err := createFederationToDeliveryServiceAssociation()
+ if err != nil {
+ t.Fatal(err.Error())
+ }
+
+ // need to assign myself the federation to set its mappings
+ me, _, err := TOSession.GetUserCurrent(client.RequestOptions{})
+ if err != nil {
+ t.Fatalf("Couldn't figure out who I am: %v - alerts: %+v", err,
me.Alerts)
+ }
+ if me.Response.ID == nil {
+ t.Fatal("Current user has no ID, cannot continue.")
+ }
+
+ fedUsers, _, err := TOSession.GetFederationUsers(fedID,
client.RequestOptions{})
+ if err != nil {
+ t.Fatalf("unexpected error getting federation users: %v", err)
+ }
+ foundFedUser := false
+ for _, fedUser := range fedUsers.Response {
+ if *fedUser.ID == *me.Response.ID {
+ foundFedUser = true
+ break
+ }
+ }
+ if !foundFedUser {
+ alerts, _, err := TOSession.CreateFederationUsers(fedID,
[]int{*me.Response.ID}, false, client.RequestOptions{})
+ if err != nil {
+ t.Fatalf("Failed to assign Federation to current user:
%v - alerts: %+v", err, alerts.Alerts)
+ }
+ }
+ expectedResolve4 := []string{"192.0.2.0/25", "192.0.2.128/25"}
+ expectedResolve6 := []string{"2001:db8::/33", "2001:db8:8000::/33"}
+ sort.Strings(expectedResolve4)
+ sort.Strings(expectedResolve6)
+
+ mappings := tc.DeliveryServiceFederationResolverMappingRequest{
+ tc.DeliveryServiceFederationResolverMapping{
+ DeliveryService: *ds.XMLID,
+ Mappings: tc.ResolverMapping{
+ Resolve4: expectedResolve4,
+ Resolve6: expectedResolve6,
+ },
+ },
+ // for the purpose of this test, it's important that at least
two different mappings have the same resolvers
+ tc.DeliveryServiceFederationResolverMapping{
+ DeliveryService: *ds1.XMLID,
+ Mappings: tc.ResolverMapping{
+ Resolve4: expectedResolve4,
+ Resolve6: expectedResolve6,
+ },
+ },
+ }
+
+ alerts, _, err :=
TOSession.ReplaceFederationResolverMappingsForCurrentUser(mappings,
client.RequestOptions{})
+ if err != nil {
+ t.Fatalf("Unexpected error replacing Federation Resolver
mappings for the current user: %v - alerts: %+v", err, alerts.Alerts)
+ }
+ for _, a := range alerts.Alerts {
+ if a.Level == tc.ErrorLevel.String() {
+ t.Errorf("Unexpected error-level alert from replacing
Federation Resolver mappings for the current user: %s", a.Text)
+ }
+ }
+
+ feds, _, err := TOSession.Federations(client.RequestOptions{})
+ if err != nil {
+ t.Fatalf("unexpected error getting federations: %v", err)
+ }
+ for _, fed := range feds.Response {
+ if fed.DeliveryService.String() == *ds.XMLID ||
fed.DeliveryService.String() == *ds1.XMLID {
+ if len(fed.Mappings) != 1 {
+ t.Fatalf("expected 1 mapping, got %d",
len(fed.Mappings))
+ }
+ sort.Strings(fed.Mappings[0].Resolve4)
+ sort.Strings(fed.Mappings[0].Resolve6)
+ if !reflect.DeepEqual(expectedResolve4,
fed.Mappings[0].Resolve4) {
+ t.Errorf("checking federation resolver
mappings, expected: %+v, actual: %+v", expectedResolve4,
fed.Mappings[0].Resolve4)
+ }
+ if !reflect.DeepEqual(expectedResolve6,
fed.Mappings[0].Resolve6) {
+ t.Errorf("checking federation resolver
mappings, expected: %+v, actual: %+v", expectedResolve6,
fed.Mappings[0].Resolve6)
+ }
+ }
+ }
+
+ mappings = tc.DeliveryServiceFederationResolverMappingRequest{
+ tc.DeliveryServiceFederationResolverMapping{
+ DeliveryService: "aoeuhtns",
+ Mappings: tc.ResolverMapping{
+ Resolve4: []string{},
+ Resolve6: []string{"dead::beef",
"f1d0::f00d/82"},
+ },
+ },
+ }
+
+ alerts, _, err =
TOSession.ReplaceFederationResolverMappingsForCurrentUser(mappings,
client.RequestOptions{})
+ if err == nil {
+ t.Fatal("Expected error replacing Federation Resolver mappings
for the current user, but didn't get one")
+ }
+ for _, a := range alerts.Alerts {
+ if a.Level == tc.SuccessLevel.String() {
+ t.Errorf("Unexpected success from replacing Federation
Resolver mappings for the current user: %s", a.Text)
+ }
+ }
+}
diff --git a/traffic_ops/traffic_ops_golang/federations/federations.go
b/traffic_ops/traffic_ops_golang/federations/federations.go
index 8bef373..7d462c4 100644
--- a/traffic_ops/traffic_ops_golang/federations/federations.go
+++ b/traffic_ops/traffic_ops_golang/federations/federations.go
@@ -49,7 +49,7 @@ VALUES ($1, (
FROM type
WHERE type.name = $2
))
-ON CONFLICT DO NOTHING
+ON CONFLICT (ip_address) DO UPDATE SET ip_address =
federation_resolver.ip_address
RETURNING federation_resolver.ip_address, federation_resolver.id
`
@@ -324,7 +324,7 @@ WHERE
return feds, nil, http.StatusOK, &maxTime
}
-// AddFederationResorverMappingsForCurrentUser is the handler for a POST
request to /federations.
+// AddFederationResolverMappingsForCurrentUser is the handler for a POST
request to /federations.
// Confusingly, it does not create a federation, but is instead used to
manipulate the resolvers
// used by one or more particular Delivery Services for one or more particular
Federations.
func AddFederationResolverMappingsForCurrentUser(w http.ResponseWriter, r
*http.Request) {
@@ -426,14 +426,12 @@ func addFederationResolver(res []string, t
tc.FederationResolverType, fedID uint
for _, r := range res {
var ip string
var id uint
- if err := tx.QueryRow(insertResolverQuery, r, t).Scan(&ip,
&id); err != nil && err != sql.ErrNoRows {
+ if err := tx.QueryRow(insertResolverQuery, r, t).Scan(&ip,
&id); err != nil {
return nil, err
}
- if ip != "" && id > 0 {
- inserted = append(inserted, ip)
- if _, err :=
tx.Exec(associateFederationWithResolverQuery, fedID, id); err != nil {
- return nil, err
- }
+ inserted = append(inserted, ip)
+ if _, err := tx.Exec(associateFederationWithResolverQuery,
fedID, id); err != nil {
+ return nil, err
}
}
