ocket8888 commented on a change in pull request #4700:
URL: https://github.com/apache/trafficcontrol/pull/4700#discussion_r433389303



##########
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:
       Yeah, that should be impossible, barring direct database manipulation.




----------------------------------------------------------------
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]


Reply via email to