srijeet0406 commented on code in PR #7596:
URL: https://github.com/apache/trafficcontrol/pull/7596#discussion_r1253626135
##########
lib/go-tc/federation_resolver.go:
##########
@@ -50,6 +51,36 @@ type FederationResolver struct {
TypeID *uint `json:"typeId,omitempty" db:"type"`
}
+// [V5] FederationResolverV5 is an alias for the Federal Resolver struct
response used for the latest minor version associated with api major version 5.
+type FederationResolverV5 = FederationResolverV50
+
+// [V50] FederationResolverV50 is used for RFC3339 format timestamp in
FederationResolver which represents a resolver record for a CDN Federation.
Review Comment:
Same comment here about godocs.
##########
lib/go-tc/federation_resolver.go:
##########
@@ -50,6 +51,36 @@ type FederationResolver struct {
TypeID *uint `json:"typeId,omitempty" db:"type"`
}
+// [V5] FederationResolverV5 is an alias for the Federal Resolver struct
response used for the latest minor version associated with api major version 5.
Review Comment:
The GoDoc comment should be gin with the name of the function/ struct that
it applies to.
##########
lib/go-tc/federation_resolver.go:
##########
@@ -50,6 +51,36 @@ type FederationResolver struct {
TypeID *uint `json:"typeId,omitempty" db:"type"`
}
+// [V5] FederationResolverV5 is an alias for the Federal Resolver struct
response used for the latest minor version associated with api major version 5.
+type FederationResolverV5 = FederationResolverV50
+
+// [V50] FederationResolverV50 is used for RFC3339 format timestamp in
FederationResolver which represents a resolver record for a CDN Federation.
+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"`
+}
+
+// [V5] FederationResolversV5Response is an alias for the FederationResolvers
struct response used for the latest minor version associated with api major
version 5.
+type FederationResolversV5Response = FederationResolversV50Response
+
+// [V50] GET request to its /federation_resolvers endpoint for APIv5.
+type FederationResolversV50Response struct {
+ Alerts
+ Response []FederationResolverV5 `json:"response"`
+}
+
+// [V5] FederationResolverV5Response represents struct response used for the
latest minor version associated with api major version 5.
Review Comment:
Same comment here
##########
lib/go-tc/federation_resolver.go:
##########
@@ -50,6 +51,36 @@ type FederationResolver struct {
TypeID *uint `json:"typeId,omitempty" db:"type"`
}
+// [V5] FederationResolverV5 is an alias for the Federal Resolver struct
response used for the latest minor version associated with api major version 5.
+type FederationResolverV5 = FederationResolverV50
+
+// [V50] FederationResolverV50 is used for RFC3339 format timestamp in
FederationResolver which represents a resolver record for a CDN Federation.
+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"`
+}
+
+// [V5] FederationResolversV5Response is an alias for the FederationResolvers
struct response used for the latest minor version associated with api major
version 5.
+type FederationResolversV5Response = FederationResolversV50Response
+
+// [V50] GET request to its /federation_resolvers endpoint for APIv5.
+type FederationResolversV50Response struct {
+ Alerts
+ Response []FederationResolverV5 `json:"response"`
+}
+
+// [V5] FederationResolverV5Response represents struct response used for the
latest minor version associated with api major version 5.
+type FederationResolverV5Response = FederationResolverV50Response
Review Comment:
V5 should be at the end of the names of the structs
##########
traffic_ops/traffic_ops_golang/routing/routes.go:
##########
@@ -466,8 +466,8 @@ func Routes(d ServerData) ([]Route, http.Handler, error) {
{Version: api.Version{Major: 5, Minor: 0}, Method:
http.MethodDelete, Path: `federations/{id}/deliveryservices/{dsID}/?$`,
Handler: api.DeleteHandler(&federations.TOFedDSes{}), RequiredPrivLevel:
auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:UPDATE",
"DELIVERY-SERVICE:UPDATE", "FEDERATION:READ", "DELIVERY-SERVICE:READ"},
Authenticated: Authenticated, Middlewares: nil, ID: 441740257031},
// Federation Resolvers
- {Version: api.Version{Major: 5, Minor: 0}, Method:
http.MethodPost, Path: `federation_resolvers/?$`, Handler:
federation_resolvers.Create, RequiredPrivLevel: auth.PrivLevelAdmin,
RequiredPermissions: []string{"FEDERATION-RESOLVER:CREATE", "TYPE:READ"},
Authenticated: Authenticated, Middlewares: nil, ID: 413437366131},
- {Version: api.Version{Major: 5, Minor: 0}, Method:
http.MethodGet, Path: `federation_resolvers/?$`, Handler:
federation_resolvers.Read, RequiredPrivLevel: auth.PrivLevelReadOnly,
RequiredPermissions: []string{"FEDERATION-RESOLVER:READ", "TYPE:READ"},
Authenticated: Authenticated, Middlewares: nil, ID: 45660875931},
+ {Version: api.Version{Major: 5, Minor: 0}, Method:
http.MethodPost, Path: `federation_resolvers/?$`, Handler:
federation_resolvers.CreateFederationResolver, RequiredPrivLevel:
auth.PrivLevelAdmin, RequiredPermissions:
[]string{"FEDERATION-RESOLVER:CREATE", "TYPE:READ"}, Authenticated:
Authenticated, Middlewares: nil, ID: 413437366131},
Review Comment:
If you just used the same functions with the api version checks in there,
you wouldn't need to change anything in this file.
##########
CHANGELOG.md:
##########
@@ -59,6 +59,7 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
- [#7469](https://github.com/apache/trafficcontrol/pull/7469) *Traffic Ops*
Changed logic to not report empty or missing cookies into TO error.log.
- [#7586](https://github.com/apache/trafficcontrol/pull/7586) *Traffic Ops*
Add permission to Operations Role to read from dnsseckeys endpoint.
- [#7600](https://github.com/apache/trafficcontrol/pull/7600) *t3c* changed
default go-direct command line arg to be old to avoid unexpected config changes
upon upgrade.
+- [#7596](https://github.com/apache/trafficcontrol/pull/7596) *Traffic Ops*
Fixes `federation_resolvers` v5 apis to respond with `RFC3339` date/time Format.
Review Comment:
This should be a part of the "fixed" section.
##########
lib/go-tc/federation_resolver.go:
##########
@@ -50,6 +51,36 @@ type FederationResolver struct {
TypeID *uint `json:"typeId,omitempty" db:"type"`
}
+// [V5] FederationResolverV5 is an alias for the Federal Resolver struct
response used for the latest minor version associated with api major version 5.
+type FederationResolverV5 = FederationResolverV50
+
+// [V50] FederationResolverV50 is used for RFC3339 format timestamp in
FederationResolver which represents a resolver record for a CDN Federation.
+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"`
+}
+
+// [V5] FederationResolversV5Response is an alias for the FederationResolvers
struct response used for the latest minor version associated with api major
version 5.
+type FederationResolversV5Response = FederationResolversV50Response
+
+// [V50] GET request to its /federation_resolvers endpoint for APIv5.
Review Comment:
Same comment here
##########
lib/go-tc/tovalidate/rules.go:
##########
@@ -217,3 +217,14 @@ func StringIsValidFloat() *validation.StringRule {
return err == nil && !math.IsNaN(validated)
}, "must be a valid float")
}
+
+// IsValidIPorCIDR returns whether the input is a valid IP address or CIDR
notation subnet
+func IsValidIPorCIDR(input string) bool {
Review Comment:
nit: The name should be `IsValidIPOrCIDR`, instead of `IsValidIPorCIDR`
Note that the `O` needs to be capitalized
##########
lib/go-tc/federation_resolver.go:
##########
@@ -50,6 +51,36 @@ type FederationResolver struct {
TypeID *uint `json:"typeId,omitempty" db:"type"`
}
+// [V5] FederationResolverV5 is an alias for the Federal Resolver struct
response used for the latest minor version associated with api major version 5.
+type FederationResolverV5 = FederationResolverV50
+
+// [V50] FederationResolverV50 is used for RFC3339 format timestamp in
FederationResolver which represents a resolver record for a CDN Federation.
+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"`
+}
+
+// [V5] FederationResolversV5Response is an alias for the FederationResolvers
struct response used for the latest minor version associated with api major
version 5.
+type FederationResolversV5Response = FederationResolversV50Response
+
+// [V50] GET request to its /federation_resolvers endpoint for APIv5.
+type FederationResolversV50Response struct {
+ Alerts
+ Response []FederationResolverV5 `json:"response"`
+}
+
+// [V5] FederationResolverV5Response represents struct response used for the
latest minor version associated with api major version 5.
+type FederationResolverV5Response = FederationResolverV50Response
+
+// [V50] POST request to its /federation_resolvers endpoint APIv5.
+type FederationResolverV50Response struct {
Review Comment:
Same comment here
##########
lib/go-tc/federation_resolver.go:
##########
@@ -50,6 +51,36 @@ type FederationResolver struct {
TypeID *uint `json:"typeId,omitempty" db:"type"`
}
+// [V5] FederationResolverV5 is an alias for the Federal Resolver struct
response used for the latest minor version associated with api major version 5.
+type FederationResolverV5 = FederationResolverV50
+
+// [V50] FederationResolverV50 is used for RFC3339 format timestamp in
FederationResolver which represents a resolver record for a CDN Federation.
+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"`
+}
+
+// [V5] FederationResolversV5Response is an alias for the FederationResolvers
struct response used for the latest minor version associated with api major
version 5.
+type FederationResolversV5Response = FederationResolversV50Response
Review Comment:
Same comment here
##########
lib/go-tc/tovalidate/rules.go:
##########
@@ -217,3 +217,14 @@ func StringIsValidFloat() *validation.StringRule {
return err == nil && !math.IsNaN(validated)
}, "must be a valid float")
}
+
+// IsValidIPorCIDR returns whether the input is a valid IP address or CIDR
notation subnet
+func IsValidIPorCIDR(input string) bool {
+ ip := net.ParseIP(input)
+ if ip != nil {
+ return true // Valid IP address
+ }
+
+ _, _, err := net.ParseCIDR(input)
+ return err == nil // Valid CIDR notation subnet
Review Comment:
Should we return the error from here?
##########
lib/go-tc/federation_resolver.go:
##########
@@ -50,6 +51,36 @@ type FederationResolver struct {
TypeID *uint `json:"typeId,omitempty" db:"type"`
}
+// [V5] FederationResolverV5 is an alias for the Federal Resolver struct
response used for the latest minor version associated with api major version 5.
+type FederationResolverV5 = FederationResolverV50
+
+// [V50] FederationResolverV50 is used for RFC3339 format timestamp in
FederationResolver which represents a resolver record for a CDN Federation.
+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"`
+}
+
+// [V5] FederationResolversV5Response is an alias for the FederationResolvers
struct response used for the latest minor version associated with api major
version 5.
+type FederationResolversV5Response = FederationResolversV50Response
+
+// [V50] GET request to its /federation_resolvers endpoint for APIv5.
+type FederationResolversV50Response struct {
Review Comment:
V50 should be at the end of the name of the struct here
##########
traffic_ops/traffic_ops_golang/federation_resolvers/federation_resolvers.go:
##########
@@ -301,3 +306,153 @@ func deleteFederationResolver(inf *api.APIInfo)
(tc.Alert, tc.FederationResolver
return alert, result, userErr, sysErr, statusCode
}
+
+// GetFederationResolvers [V5] - get list of federation resolver for APIv5
Review Comment:
Since federation_resolvers is not using the CRUDer interface, it will be
much easier (and would involve lesser lines of code) if we could just check the
api version in the `Get` and `Create` methods of federation_resolvers, and
return the appropriate structs.
You can take a look at this PR for reference:
https://github.com/apache/trafficcontrol/pull/7547
##########
lib/go-tc/federation_resolver.go:
##########
@@ -50,6 +51,36 @@ type FederationResolver struct {
TypeID *uint `json:"typeId,omitempty" db:"type"`
}
+// [V5] FederationResolverV5 is an alias for the Federal Resolver struct
response used for the latest minor version associated with api major version 5.
+type FederationResolverV5 = FederationResolverV50
+
+// [V50] FederationResolverV50 is used for RFC3339 format timestamp in
FederationResolver which represents a resolver record for a CDN Federation.
+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"`
+}
+
+// [V5] FederationResolversV5Response is an alias for the FederationResolvers
struct response used for the latest minor version associated with api major
version 5.
+type FederationResolversV5Response = FederationResolversV50Response
+
+// [V50] GET request to its /federation_resolvers endpoint for APIv5.
+type FederationResolversV50Response struct {
+ Alerts
+ Response []FederationResolverV5 `json:"response"`
+}
+
+// [V5] FederationResolverV5Response represents struct response used for the
latest minor version associated with api major version 5.
+type FederationResolverV5Response = FederationResolverV50Response
+
+// [V50] POST request to its /federation_resolvers endpoint APIv5.
+type FederationResolverV50Response struct {
Review Comment:
Also, following the conventions in our repo, the struct name should end with
`V50`
--
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: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
