ocket8888 commented on a change in pull request #4700:
URL: https://github.com/apache/trafficcontrol/pull/4700#discussion_r433407930
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -139,94 +366,213 @@ func (s *TOServer) Validate() error {
validateErrs["ip6Address"] = validation.Validate(s.IP6Address,
validation.By(tovalidate.IsValidIPv6CIDROrAddress))
}
errs = append(errs, tovalidate.ToErrors(validateErrs)...)
- if len(errs) > 0 {
- return util.JoinErrs(errs)
- }
+ errs = append(errs, validateCommon(s.CommonServerProperties, tx)...)
+
+ return util.JoinErrs(errs)
+}
+
+func validateV2(s *tc.ServerNullableV2, tx *sql.Tx) error {
+ var errs []error
- if _, err := tc.ValidateTypeID(s.ReqInfo.Tx.Tx, s.TypeID, "server");
err != nil {
+ if err := validateV1(s.ServerNullableV11, tx); err != nil {
return err
}
- rows, err := s.ReqInfo.Tx.Tx.Query("select cdn from profile where
id=$1", s.ProfileID)
- if err != nil {
- log.Error.Printf("could not execute select cdnID from profile:
%s\n", err)
- errs = append(errs, tc.DBError)
- return util.JoinErrs(errs)
+ // default boolean value is false
+ if s.IPIsService == nil {
+ s.IPIsService = new(bool)
}
- defer rows.Close()
- var cdnID int
- for rows.Next() {
- if err := rows.Scan(&cdnID); err != nil {
- log.Error.Printf("could not scan cdnID from profile:
%s\n", err)
- errs = append(errs, errors.New("associated profile must
have a cdn associated"))
- return util.JoinErrs(errs)
- }
+ if s.IP6IsService == nil {
+ s.IP6IsService = new(bool)
}
- log.Infof("got cdn id: %d from profile and cdn id: %d from server",
cdnID, *s.CDNID)
- if cdnID != *s.CDNID {
- errs = append(errs, errors.New(fmt.Sprintf("CDN id '%d' for
profile '%d' does not match Server CDN '%d'", cdnID, *s.ProfileID, *s.CDNID)))
+
+ if !*s.IPIsService && !*s.IP6IsService {
+ errs = append(errs, tc.NeedsAtLeastOneServiceAddressError)
}
- return util.JoinErrs(errs)
-}
-// ChangeLogMessage implements the api.ChangeLogger interface for a custom log
message
-func (s TOServer) ChangeLogMessage(action string) (string, error) {
+ if *s.IPIsService && s.IPAddress == nil {
+ errs = append(errs, tc.EmptyAddressCannotBeAServiceAddressError)
+ }
- var status string
- if s.Status != nil {
- status = *s.Status
+ if *s.IP6IsService && s.IP6Address == nil {
+ errs = append(errs, tc.EmptyAddressCannotBeAServiceAddressError)
}
+ return util.JoinErrs(errs)
+}
- var hostName string
- if s.HostName != nil {
- hostName = *s.HostName
+func validateMTU(mtu interface{}) error {
+ m := mtu.(*uint64)
+ if m == nil {
+ return nil
}
- var domainName string
- if s.DomainName != nil {
- domainName = *s.DomainName
+ if *m < 1280 {
+ return errors.New("must be at least 1280")
}
+ return nil
+}
+
+func validateV3(s tc.ServerNullable, tx *sql.Tx) (string, error) {
- var serverID string
- if s.ID != nil {
- serverID = strconv.Itoa(*s.ID)
+ if len(s.Interfaces) == 0 {
+ return "", errors.New("a server must have at least one
interface")
}
+ var errs []error
+ var serviceAddrV4Found bool
+ var serviceAddrV6Found bool
+ var serviceInterface string
+ for _, iface := range s.Interfaces {
+
+ ruleName := fmt.Sprintf("interface '%s' ", iface.Name)
+ errs = append(errs, tovalidate.ToErrors(validation.Errors{
+ ruleName + "name":
validation.Validate(iface.Name, validation.Required),
+ ruleName + "mtu":
validation.Validate(iface.MaxBandwidth, validation.By(validateMTU)),
+ ruleName + "ipAddresses":
validation.Validate(iface.IPAddresses, validation.Required),
+ })...)
+
+ for _, addr := range iface.IPAddresses {
+ ruleName += fmt.Sprintf("address '%s'", addr.Address)
+
+ var parsedIP net.IP
+ var err error
+ if parsedIP, _, err = net.ParseCIDR(addr.Address); err
!= nil {
+ if parsedIP = net.ParseIP(addr.Address);
parsedIP == nil {
+ errs = append(errs, fmt.Errorf("%s:
address: %v", ruleName, err))
+ continue
+ }
+ }
- message := action + ` ` + status + ` server: { "hostName":"` + hostName
+ `", "domainName":"` + domainName + `", id:` + serverID + ` }`
+ if addr.Gateway != nil {
+ if gateway := net.ParseIP(*addr.Gateway);
gateway == nil {
+ errs = append(errs, fmt.Errorf("%s:
gateway: could not parse '%s' as a network gateway", ruleName, *addr.Gateway))
+ } else if (gateway.To4() == nil &&
parsedIP.To4() != nil) || (gateway.To4() != nil && parsedIP.To4() == nil) {
+ errs = append(errs,
errors.New(ruleName+": address family mismatch between address and gateway"))
+ }
+ }
- return message, nil
+ if addr.ServiceAddress {
+ if serviceInterface != "" && serviceInterface
!= iface.Name {
+ errs = append(errs,
fmt.Errorf("interfaces: both %s and %s interfaces contain service addresses -
only one service-address-containing-interface is allowed", serviceInterface,
iface.Name))
+ }
+ serviceInterface = iface.Name
+ if parsedIP.To4() != nil {
+ if serviceAddrV4Found {
+ errs = append(errs,
fmt.Errorf("interfaces: address '%s' of interface '%s' is marked as a service
address, but an IPv4 service address appears earlier in the list",
addr.Address, iface.Name))
+ }
+ serviceAddrV4Found = true
+ } else {
+ if serviceAddrV6Found {
+ errs = append(errs,
fmt.Errorf("interfaces: address '%s' of interface '%s' is marked as a service
address, but an IPv6 service address appears earlier in the list",
addr.Address, iface.Name))
+ }
+ serviceAddrV6Found = true
+ }
+ }
+ }
+ }
+
+ errs = append(errs, validateCommon(s.CommonServerProperties, tx)...)
+ return serviceInterface, util.JoinErrs(errs)
}
-func (s *TOServer) Read() ([]interface{}, error, error, int) {
- version := s.APIInfo().Version
- if version == nil {
- return nil, nil, errors.New("TOServer.Read called with nil API
version"), http.StatusInternalServerError
+func Read(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
}
+ defer inf.Close()
- returnable := []interface{}{}
+ // Middleware should've already handled this, so idk why this is a
pointer at all tbh
+ version := inf.Version
+ if version == nil {
+ middleware.NotImplementedHandler().ServeHTTP(w, r)
+ return
+ }
- servers, userErr, sysErr, errCode := getServers(s.ReqInfo.Params,
s.ReqInfo.Tx, s.ReqInfo.User)
+ servers := []tc.ServerNullable{}
+ var unfiltered uint64
+ servers, unfiltered, userErr, sysErr, errCode = getServers(inf.Params,
inf.Tx, inf.User)
if userErr != nil || sysErr != nil {
- return nil, userErr, sysErr, errCode
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if version.Major >= 3 {
+ api.WriteRespWithSummary(w, r, servers, unfiltered)
+ return
}
+ if version.Major <= 1 {
+ legacyServers := make([]tc.ServerNullableV11, 0, len(servers))
+ for _, server := range servers {
+ legacyServer, err := server.ToServerV2()
Review comment:
I guess also I should mention that even through direct database
manipulation you can't _break_ the V1/V2 APIs, just make them not return
IPAddress values even if the server has configured addresses - though they'd be
non-service addresses so that's still technically correct.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
