Fixed lastUpdated timestamp format in api v5 federation resolvers (#7596

)

* fr changes

* fr changes

* changelog added

* type import correction

* fr codefix

* fr code changes

* comment correction

* fixes

* comments addressed

* changelog correction

* fr changed to give rfc format res

* removed unused function

* version revert for deletion

* space added

* comments addressed

* comments addressed

* reference mistake correction

CHANGELOG.md

@@ -79,7 +79,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#7544](https://github.com/apache/trafficcontrol/issues/7544) *Traffic Ops* Fixes stats_summary v5 apis to respond with RFC3339 date/time Format.
 - [#7542](https://github.com/apache/trafficcontrol/pull/7542) *Traffic Ops* Fixed `CDN Locks` documentation to reflect the correct RFC3339 timestamps.
 - [#6340](https://github.com/apache/trafficcontrol/issues/6340) *Traffic Ops* Fixed alert messages for POST and PUT invalidation job APIs.
-- [#7519] (https://github.com/apache/trafficcontrol/issues/7519) *Traffic Ops* Fixed TO API /servers/{id}/deliveryservices endpoint to responding with all DS's on cache that are directly assigned and inherited through topology.
+- [#7519](https://github.com/apache/trafficcontrol/issues/7519) *Traffic Ops* Fixed TO API /servers/{id}/deliveryservices endpoint to responding with all DS's on cache that are directly assigned and inherited through topology.
 - [#7511](https://github.com/apache/trafficcontrol/pull/7511) *Traffic Ops* Fixed the changelog registration message to include the username instead of duplicate email entry.
 - [#7465](https://github.com/apache/trafficcontrol/issues/7465) *Traffic Ops* Fixes server_capabilities v5 apis to respond with RFC3339 date/time Format
 - [#7441](https://github.com/apache/trafficcontrol/pull/7441) *Traffic Ops* Fixed the invalidation jobs endpoint to respect CDN locks.
@@ -134,6 +134,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#7590](https://github.com/apache/trafficcontrol/issues/7590) *Traffic Control Cache Config (t3c)* Fixed issue with git detected dubious ownership in repository.
 - [#7575](https://github.com/apache/trafficcontrol/pull/7575) *Traffic Ops* Fixes `types` v5 apis to respond with `RFC3339` date/time Format.
 - [#7628](https://github.com/apache/trafficcontrol/pull/7628) *Traffic Ops* Fixes an issue where certificate chain validation failed based on leading or trailing whitespace.
+- [#7596](https://github.com/apache/trafficcontrol/pull/7596) *Traffic Ops* Fixes `federation_resolvers` v5 apis to respond with `RFC3339` date/time Format.
 
 ### Removed
 - [#7271](https://github.com/apache/trafficcontrol/pull/7271) Remove components in `infrastructre/docker/`, not in use as cdn-in-a-box performs the same functionality.

docs/source/api/v5/federation_resolvers.rst

@@ -70,7 +70,7 @@ Response Structure
 ------------------
 :id:          The integral, unique identifier of the resolver
 :ipAddress:   The IP address or :abbr:`CIDR (Classless Inter-Domain Routing)`-notation subnet of the resolver - may be IPv4 or IPv6
-:lastUpdated: The date and time at which this resolver was last updated, in :ref:`non-rfc-datetime`
+:lastUpdated: The date and time at which this resolver was last updated, in :rfc:`3339`
 :type:        The :term:`Type` of the resolver
 
 .. code-block:: http
@@ -92,7 +92,7 @@ Response Structure
 		{
 			"id": 1,
 			"ipAddress": "::1/1",
-			"lastUpdated": "2019-11-06 00:00:40+00",
+			"lastUpdated": "2019-11-06T15:18:14.952814+05:30",
 			"type": "RESOLVE6"
 		}
 	]}

lib/go-tc/federation_resolver.go

@@ -23,6 +23,7 @@ import (
 	"database/sql"
 	"errors"
 	"net"
+	"time"
 
 	validation "github.com/go-ozzo/ozzo-validation"
 )
@@ -50,6 +51,36 @@ type FederationResolver struct {
 	TypeID      *uint      `json:"typeId,omitempty" db:"type"`
 }
 
+// FederationResolverV5 - is an alias for the Federal Resolver struct response used for the latest minor version associated with APIv5.
+type FederationResolverV5 = FederationResolverV50
+
+// FederationResolverV50 - is used for RFC3339 format timestamp in FederationResolver which represents a resolver record for a CDN Federation for APIv50.
+type FederationResolverV50 struct {
+	ID          *uint      `json:"id" db:"id"`
+	IPAddress   *string    `json:"ipAddress" db:"ip_address"`
+	LastUpdated *time.Time `json:"lastUpdated,omitempty" db:"last_updated"`
+	Type        *string    `json:"type"`
+	TypeID      *uint      `json:"typeId,omitempty" db:"type"`
+}
+
+// FederationResolversResponseV5 - an alias for the Federation Resolver's struct response used for the latest minor version associated with APIv5.
+type FederationResolversResponseV5 = FederationResolversResponseV50
+
+// FederationResolversResponseV50 - GET request to its /federation_resolvers endpoint for APIv50.
+type FederationResolversResponseV50 struct {
+	Alerts
+	Response []FederationResolverV5 `json:"response"`
+}
+
+// FederationResolverResponseV50 - represents struct response used for the latest minor version associated with APIv5.
+type FederationResolverResponseV5 = FederationResolverResponseV50
+
+// FederationResolverResponseV50 - POST request to its /federation_resolvers endpoint APIv50.
+type FederationResolverResponseV50 struct {
+	Alerts
+	Response FederationResolverV5 `json:"response"`
+}
+
 // Validate implements the github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api.ParseValidator
 // interface.
 func (fr *FederationResolver) Validate(tx *sql.Tx) error {
@@ -71,3 +102,19 @@ func (fr *FederationResolver) Validate(tx *sql.Tx) error {
 		validation.Field(&fr.TypeID, validation.Required),
 	)
 }
+
+// UpgradeToFederationResolverV5 upgrades an APIv4 Federal Resolver into an APIv5 Federal Resolver of
+// the latest minor version.
+func UpgradeToFederationResolverV5(fr FederationResolver) *FederationResolverV5 {
+	upgraded := FederationResolverV5{
+		ID:        fr.ID,
+		IPAddress: fr.IPAddress,
+		LastUpdated: func() *time.Time {
+			lastUpdated := time.Unix(fr.LastUpdated.Unix(), 0)
+			return &lastUpdated
+		}(),
+		Type: fr.Type,
+	}
+
+	return &upgraded
+}

traffic_ops/testing/api/v5/federation_resolvers_test.go

@@ -38,7 +38,7 @@ func TestFederationResolvers(t *testing.T) {
 		currentTime := time.Now().UTC().Add(-15 * time.Second)
 		tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123)
 
-		methodTests := utils.TestCase[client.Session, client.RequestOptions, tc.FederationResolver]{
+		methodTests := utils.TestCase[client.Session, client.RequestOptions, tc.FederationResolverV5]{
 			"GET": {
 				"NOT MODIFIED when NO CHANGES made": {
 					ClientSession: TOSession,
@@ -111,12 +111,12 @@ func TestFederationResolvers(t *testing.T) {
 			"POST": {
 				"BAD REQUEST when MISSING IPADDRESS and TYPE FIELDS": {
 					ClientSession: TOSession,
-					RequestBody:   tc.FederationResolver{},
+					RequestBody:   tc.FederationResolverV5{},
 					Expectations:  utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
 				},
 				"BAD REQUEST when INVALID IP ADDRESS": {
 					ClientSession: TOSession,
-					RequestBody: tc.FederationResolver{
+					RequestBody: tc.FederationResolverV5{
 						IPAddress: util.Ptr("not a valid IP address"),
 						TypeID:    util.Ptr((uint)(GetTypeId(t, "RESOLVE4"))),
 					},
@@ -167,7 +167,7 @@ func TestFederationResolvers(t *testing.T) {
 func validateFederationResolverFields(expectedResp map[string]interface{}) utils.CkReqFunc {
 	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
 		assert.RequireNotNil(t, resp, "Expected Federation Resolver response to not be nil.")
-		frResp := resp.([]tc.FederationResolver)
+		frResp := resp.([]tc.FederationResolverV5)
 		for field, expected := range expectedResp {
 			for _, fr := range frResp {
 				switch field {
@@ -190,7 +190,7 @@ func validateFederationResolverFields(expectedResp map[string]interface{}) utils
 
 func validateFederationResolverPagination(paginationParam string) utils.CkReqFunc {
 	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
-		paginationResp := resp.([]tc.FederationResolver)
+		paginationResp := resp.([]tc.FederationResolverV5)
 
 		opts := client.NewRequestOptions()
 		opts.QueryParameters.Set("orderby", "id")
@@ -214,7 +214,7 @@ func validateFederationResolverIDSort() utils.CkReqFunc {
 	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, alerts tc.Alerts, _ error) {
 		assert.RequireNotNil(t, resp, "Expected Federation Resolver response to not be nil.")
 		var frIDs []int
-		frResp := resp.([]tc.FederationResolver)
+		frResp := resp.([]tc.FederationResolverV5)
 		for _, fr := range frResp {
 			frIDs = append(frIDs, int(*fr.ID))
 		}
@@ -225,7 +225,7 @@ func validateFederationResolverIDSort() utils.CkReqFunc {
 func validateFederationResolverDescSort() utils.CkReqFunc {
 	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, alerts tc.Alerts, _ error) {
 		assert.RequireNotNil(t, resp, "Expected FederationResolver response to not be nil.")
-		frDescResp := resp.([]tc.FederationResolver)
+		frDescResp := resp.([]tc.FederationResolverV5)
 		var descSortedList []uint
 		var ascSortedList []uint
 		assert.RequireGreaterOrEqual(t, len(frDescResp), 2, "Need at least 2 Federation Resolvers in Traffic Ops to test desc sort, found: %d", len(frDescResp))

traffic_ops/testing/api/v5/traffic_control_test.go

@@ -36,7 +36,7 @@ type TrafficControl struct {
 	TopologyBasedDeliveryServicesRequiredCapabilities []tc.DeliveryServicesRequiredCapability `json:"topologyBasedDeliveryServicesRequiredCapabilities"`
 	Divisions                                         []tc.DivisionV5                         `json:"divisions"`
 	Federations                                       []tc.CDNFederation                      `json:"federations"`
-	FederationResolvers                               []tc.FederationResolver                 `json:"federation_resolvers"`
+	FederationResolvers                               []tc.FederationResolverV5               `json:"federation_resolvers"`
 	Jobs                                              []tc.InvalidationJobCreateV4            `json:"jobs"`
 	Origins                                           []tc.Origin                             `json:"origins"`
 	Profiles                                          []tc.Profile                            `json:"profiles"`

traffic_ops/traffic_ops_golang/federation_resolvers/federation_resolvers.go

@@ -34,8 +34,6 @@ import (
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
-
-	"github.com/jmoiron/sqlx"
 )
 
 const insertFederationResolverQuery = `
@@ -136,13 +134,17 @@ func Read(w http.ResponseWriter, r *http.Request) {
 	} else {
 		log.Warnf("Couldn't get config %v", e)
 	}
+
+	// Based on version we load types - for version 5 and above we use FederationResolverV5
+	var resolvers []interface{}
+
 	if useIMS {
-		runSecond, maxTime = TryIfModifiedSinceQuery(r.Header, inf.Tx, where, orderBy, pagination, queryValues)
+		runSecond, maxTime = ims.TryIfModifiedSinceQuery(inf.Tx, r.Header, queryValues, SelectMaxLastUpdatedQuery(where, "federation_resolver"))
 		if !runSecond {
 			log.Debugln("IMS HIT")
 			api.AddLastModifiedHdr(w, maxTime)
 			w.WriteHeader(http.StatusNotModified)
-			api.WriteResp(w, r, []tc.FederationResolver{})
+			api.WriteResp(w, r, resolvers)
 			return
 		}
 		log.Debugln("IMS MISS")
@@ -162,7 +164,6 @@ func Read(w http.ResponseWriter, r *http.Request) {
 	}
 	defer rows.Close()
 
-	var resolvers = []tc.FederationResolver{}
 	for rows.Next() {
 		var resolver tc.FederationResolver
 		if err := rows.Scan(&resolver.ID, &resolver.IPAddress, &resolver.LastUpdated, &resolver.Type); err != nil {
@@ -174,7 +175,16 @@ func Read(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		resolvers = append(resolvers, resolver)
+		// Based on version we load types - for version 5 and above we use FederationResolverV5
+		if inf.Version.GreaterThanOrEqualTo(&api.Version{Major: 5, Minor: 0}) {
+
+			// Convert FederationResolver fields to FederationResolverV5 fields
+			v5Resolver := tc.UpgradeToFederationResolverV5(resolver)
+
+			resolvers = append(resolvers, v5Resolver)
+		} else {
+			resolvers = append(resolvers, resolver)
+		}
 	}
 
 	if api.SetLastModifiedHeader(r, useIMS) {
@@ -185,52 +195,6 @@ func Read(w http.ResponseWriter, r *http.Request) {
 	api.WriteResp(w, r, resolvers)
 }
 
-func TryIfModifiedSinceQuery(header http.Header, tx *sqlx.Tx, where string, orderBy string, pagination string, queryValues map[string]interface{}) (bool, time.Time) {
-	var max time.Time
-	var imsDate time.Time
-	var ok bool
-	imsDateHeader := []string{}
-	runSecond := true
-	dontRunSecond := false
-	if header == nil {
-		return runSecond, max
-	}
-	imsDateHeader = header[rfc.IfModifiedSince]
-	if len(imsDateHeader) == 0 {
-		return runSecond, max
-	}
-	if imsDate, ok = rfc.ParseHTTPDate(imsDateHeader[0]); !ok {
-		log.Warnf("IMS request header date '%s' not parsable", imsDateHeader[0])
-		return runSecond, max
-	}
-	query := SelectMaxLastUpdatedQuery(where, "federation_resolver")
-	rows, err := tx.NamedQuery(query, queryValues)
-	if err != nil {
-		log.Warnf("Couldn't get the max last updated time: %v", err)
-		return runSecond, max
-	}
-	if err == sql.ErrNoRows {
-		return dontRunSecond, max
-	}
-	defer rows.Close()
-	// This should only ever contain one row
-	if rows.Next() {
-		v := &ims.LatestTimestamp{}
-		if err = rows.StructScan(v); err != nil || v == nil {
-			log.Warnf("Failed to parse the max time stamp into a struct %v", err)
-			return runSecond, max
-		}
-		if v.LatestTime != nil {
-			max = v.LatestTime.Time
-			// The request IMS time is later than the max of (lastUpdated, deleted_time)
-			if imsDate.After(v.LatestTime.Time) {
-				return dontRunSecond, max
-			}
-		}
-	}
-	return runSecond, max
-}
-
 func SelectMaxLastUpdatedQuery(where string, tableName string) string {
 	return `SELECT max(t) from (
 		SELECT max(last_updated) as t from ` + tableName + where +

traffic_ops/v5-client/federation_resolver.go

@@ -27,15 +27,15 @@ import (
 const apiFederationResolvers = "/federation_resolvers"
 
 // GetFederationResolvers retrieves Federation Resolvers from Traffic Ops.
-func (to *Session) GetFederationResolvers(opts RequestOptions) (tc.FederationResolversResponse, toclientlib.ReqInf, error) {
-	var data tc.FederationResolversResponse
+func (to *Session) GetFederationResolvers(opts RequestOptions) (tc.FederationResolversResponseV5, toclientlib.ReqInf, error) {
+	var data tc.FederationResolversResponseV5
 	inf, err := to.get(apiFederationResolvers, opts, &data)
 	return data, inf, err
 }
 
 // CreateFederationResolver creates the Federation Resolver 'fr'.
-func (to *Session) CreateFederationResolver(fr tc.FederationResolver, opts RequestOptions) (tc.FederationResolverResponse, toclientlib.ReqInf, error) {
-	var response tc.FederationResolverResponse
+func (to *Session) CreateFederationResolver(fr tc.FederationResolverV5, opts RequestOptions) (tc.FederationResolverResponseV5, toclientlib.ReqInf, error) {
+	var response tc.FederationResolverResponseV5
 	reqInf, err := to.post(apiFederationResolvers, opts, fr, &response)
 	return response, reqInf, err
 }