-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preserve an instance's enabled status for its two ports #59
Changes from all commits
9352b64
df904f6
2b3ac93
6e72b26
f1f7c88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,42 +5,66 @@ package fargo | |
import ( | ||
"encoding/json" | ||
"encoding/xml" | ||
"fmt" | ||
"io" | ||
"strconv" | ||
) | ||
|
||
// Temporary structs used for GetAppsResponse unmarshalling | ||
type getAppsResponseArray GetAppsResponse | ||
type getAppsResponseSingle struct { | ||
Application *Application `json:"application"` | ||
AppsHashcode string `json:"apps__hashcode"` | ||
VersionsDelta int `json:"versions__delta"` | ||
func intFromJSONNumberOrString(jv interface{}, description string) (int, error) { | ||
switch v := jv.(type) { | ||
case float64: | ||
return int(v), nil | ||
case string: | ||
n, err := strconv.Atoi(v) | ||
if err != nil { | ||
return 0, err | ||
} | ||
return n, nil | ||
default: | ||
return 0, fmt.Errorf("unexpected %s: %[2]v (type %[2]T)", description, jv) | ||
} | ||
} | ||
|
||
// UnmarshalJSON is a custom JSON unmarshaler for GetAppsResponse to deal with | ||
// sometimes non-wrapped Application arrays when there is only a single Application item. | ||
func (r *GetAppsResponse) UnmarshalJSON(b []byte) error { | ||
marshalLog.Debugf("GetAppsResponse.UnmarshalJSON b:%s\n", string(b)) | ||
var err error | ||
resolveDelta := func(d interface{}) (int, error) { | ||
return intFromJSONNumberOrString(d, "versions delta") | ||
} | ||
|
||
// Normal array case | ||
var ra getAppsResponseArray | ||
if err = json.Unmarshal(b, &ra); err == nil { | ||
marshalLog.Debug("GetAppsResponse.UnmarshalJSON ra:%+v\n", ra) | ||
*r = GetAppsResponse(ra) | ||
return nil | ||
type getAppsResponse GetAppsResponse | ||
auxArray := struct { | ||
*getAppsResponse | ||
VersionsDelta interface{} `json:"versions__delta"` | ||
}{ | ||
getAppsResponse: (*getAppsResponse)(r), | ||
} | ||
var err error | ||
if err = json.Unmarshal(b, &auxArray); err == nil { | ||
marshalLog.Debugf("GetAppsResponse.UnmarshalJSON array:%+v\n", auxArray) | ||
r.VersionsDelta, err = resolveDelta(auxArray.VersionsDelta) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a little bit confused here. Previously we were unmarshaling here with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note line 42, where we populate the "auxArray" variable's "getAppsResponse" (anonymous|embedded) field with variable "r." When we supply "auxArray' to Here the variable name "auxArray" means "auxiliary structure for the "array of applications" case, versus the "single application" case handled at line 52. The variable name "aux" comes up frequently with this encoding or decoding technique in Go, where one introduces some separate, temporary, and only locally significant struct to handle the finicky details that Go's struct tags can't accommodate. |
||
return err | ||
} | ||
|
||
// Bogus non-wrapped case | ||
var rs getAppsResponseSingle | ||
if err = json.Unmarshal(b, &rs); err == nil { | ||
marshalLog.Debug("GetAppsResponse.UnmarshalJSON rs:%+v\n", rs) | ||
r.Applications = make([]*Application, 1, 1) | ||
r.Applications[0] = rs.Application | ||
r.AppsHashcode = rs.AppsHashcode | ||
r.VersionsDelta = rs.VersionsDelta | ||
return nil | ||
auxSingle := struct { | ||
Application *Application `json:"application"` | ||
AppsHashcode string `json:"apps__hashcode"` | ||
VersionsDelta interface{} `json:"versions__delta"` | ||
}{} | ||
if err := json.Unmarshal(b, &auxSingle); err != nil { | ||
return err | ||
} | ||
return err | ||
marshalLog.Debugf("GetAppsResponse.UnmarshalJSON single:%+v\n", auxSingle) | ||
if r.VersionsDelta, err = resolveDelta(auxSingle.VersionsDelta); err != nil { | ||
return err | ||
} | ||
r.Applications = make([]*Application, 1, 1) | ||
r.Applications[0] = auxSingle.Application | ||
r.AppsHashcode = auxSingle.AppsHashcode | ||
return nil | ||
} | ||
|
||
// Temporary structs used for Application unmarshalling | ||
|
@@ -59,15 +83,15 @@ func (a *Application) UnmarshalJSON(b []byte) error { | |
// Normal array case | ||
var aa applicationArray | ||
if err = json.Unmarshal(b, &aa); err == nil { | ||
marshalLog.Debug("Application.UnmarshalJSON aa:%+v\n", aa) | ||
marshalLog.Debugf("Application.UnmarshalJSON aa:%+v\n", aa) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thx for finding that ⭐️ |
||
*a = Application(aa) | ||
return nil | ||
} | ||
|
||
// Bogus non-wrapped case | ||
var as applicationSingle | ||
if err = json.Unmarshal(b, &as); err == nil { | ||
marshalLog.Debug("Application.UnmarshalJSON as:%+v\n", as) | ||
marshalLog.Debugf("Application.UnmarshalJSON as:%+v\n", as) | ||
a.Name = as.Name | ||
a.Instances = make([]*Instance, 1, 1) | ||
a.Instances[0] = as.Instance | ||
|
@@ -76,30 +100,140 @@ func (a *Application) UnmarshalJSON(b []byte) error { | |
return err | ||
} | ||
|
||
type instance Instance | ||
func stringAsBool(s string) bool { | ||
return s == "true" | ||
} | ||
|
||
// UnmarshalJSON is a custom JSON unmarshaler for Instance to deal with the | ||
// different Port encodings between XML and JSON. Here we copy the values from the JSON | ||
// Port struct into the simple XML int field. | ||
// UnmarshalJSON is a custom JSON unmarshaler for Instance, transcribing the two composite port | ||
// specifications up to top-level fields. | ||
func (i *Instance) UnmarshalJSON(b []byte) error { | ||
// Preclude recursive calls to MarshalJSON. | ||
type instance Instance | ||
// inboundJSONFormatPort describes an instance's network port, including whether its registrant | ||
// considers the port to be enabled or disabled. | ||
// | ||
// Example JSON encoding: | ||
// | ||
// Eureka versions 1.2.1 and prior: | ||
// "port":{"@enabled":"true", "$":"7101"} | ||
// | ||
// Eureka version 1.2.2 and later: | ||
// "port":{"@enabled":"true", "$":7101} | ||
// | ||
// Note that later versions of Eureka write the port number as a JSON number rather than as a | ||
// decimal-formatted string. We accept it as either an integer or a string. Strangely, the | ||
// "@enabled" field remains a string. | ||
type inboundJSONFormatPort struct { | ||
Number interface{} `json:"$"` | ||
Enabled bool `json:"@enabled,string"` | ||
} | ||
aux := struct { | ||
*instance | ||
Port inboundJSONFormatPort `json:"port"` | ||
SecurePort inboundJSONFormatPort `json:"securePort"` | ||
}{ | ||
instance: (*instance)(i), | ||
} | ||
if err := json.Unmarshal(b, &aux); err != nil { | ||
return err | ||
} | ||
resolvePort := func(port interface{}) (int, error) { | ||
return intFromJSONNumberOrString(port, "port number") | ||
} | ||
var err error | ||
var ii instance | ||
if err = json.Unmarshal(b, &ii); err == nil { | ||
marshalLog.Debug("Instance.UnmarshalJSON ii:%+v\n", ii) | ||
*i = Instance(ii) | ||
i.Port = parsePort(ii.PortJ.Number) | ||
i.SecurePort = parsePort(ii.SecurePortJ.Number) | ||
return nil | ||
if i.Port, err = resolvePort(aux.Port.Number); err != nil { | ||
return err | ||
} | ||
return err | ||
i.PortEnabled = aux.Port.Enabled | ||
if i.SecurePort, err = resolvePort(aux.SecurePort.Number); err != nil { | ||
return err | ||
} | ||
i.SecurePortEnabled = aux.SecurePort.Enabled | ||
return nil | ||
} | ||
|
||
func parsePort(s string) int { | ||
n, err := strconv.Atoi(s) | ||
if err != nil { | ||
log.Warningf("Invalid port error: %s", err.Error()) | ||
// MarshalJSON is a custom JSON marshaler for Instance, adapting the top-level raw port values to | ||
// the composite port specifications. | ||
func (i *Instance) MarshalJSON() ([]byte, error) { | ||
// Preclude recursive calls to MarshalJSON. | ||
type instance Instance | ||
// outboundJSONFormatPort describes an instance's network port, including whether its registrant | ||
// considers the port to be enabled or disabled. | ||
// | ||
// Example JSON encoding: | ||
// | ||
// "port":{"@enabled":"true", "$":"7101"} | ||
// | ||
// Note that later versions of Eureka write the port number as a JSON number rather than as a | ||
// decimal-formatted string. We emit the port number as a string, not knowing the Eureka | ||
// server's version. Strangely, the "@enabled" field remains a string. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a thinking exercise. How hard will it be to detect eureka version and serve what's the most appropriate version of date depending on version. Will that be even cleaner? Are we worried that eureka might stop accepting old version? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The server doesn't send anything indicative of its version—not even in an HTTP header. If we had already received some responses from a server and been able to inspect the fields that differ, we could then remember which "style" or "version" of the encoding we've seen the server use and then mimic that in our future requests. There's still that problem of how our client starts that conversation safely. As for whether Eureka will stop accepting the old version, I think they'd have to introduce some new resource paths in order to be fair to all the possible clients out there of various versions. My impression is that their handling of the encoding here has been haphazard; it came down to whatever XStream happened to do, and the Java clients could still talk well enough with the servers, so it doesn't appear that they cared enough to document or even prevent these format changes. I don't think they were deliberate. |
||
type outboundJSONFormatPort struct { | ||
Number int `json:"$,string"` | ||
Enabled bool `json:"@enabled,string"` | ||
} | ||
aux := struct { | ||
*instance | ||
Port outboundJSONFormatPort `json:"port"` | ||
SecurePort outboundJSONFormatPort `json:"securePort"` | ||
}{ | ||
(*instance)(i), | ||
outboundJSONFormatPort{i.Port, i.PortEnabled}, | ||
outboundJSONFormatPort{i.SecurePort, i.SecurePortEnabled}, | ||
} | ||
return n | ||
return json.Marshal(&aux) | ||
} | ||
|
||
// xmlFormatPort describes an instance's network port, including whether its registrant considers | ||
// the port to be enabled or disabled. | ||
// | ||
// Example XML encoding: | ||
// | ||
// <port enabled="true">7101</port> | ||
type xmlFormatPort struct { | ||
Number int `xml:",chardata"` | ||
Enabled bool `xml:"enabled,attr"` | ||
} | ||
|
||
// UnmarshalXML is a custom XML unmarshaler for Instance, transcribing the two composite port | ||
// specifications up to top-level fields. | ||
func (i *Instance) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { | ||
type instance Instance | ||
aux := struct { | ||
*instance | ||
Port xmlFormatPort `xml:"port"` | ||
SecurePort xmlFormatPort `xml:"securePort"` | ||
}{ | ||
instance: (*instance)(i), | ||
} | ||
if err := d.DecodeElement(&aux, &start); err != nil { | ||
return err | ||
} | ||
i.Port = aux.Port.Number | ||
i.PortEnabled = aux.Port.Enabled | ||
i.SecurePort = aux.SecurePort.Number | ||
i.SecurePortEnabled = aux.SecurePort.Enabled | ||
return nil | ||
} | ||
|
||
// startLocalName creates a start-tag of an XML element with the given local name and no namespace name. | ||
func startLocalName(local string) xml.StartElement { | ||
return xml.StartElement{Name: xml.Name{Space: "", Local: local}} | ||
} | ||
|
||
// MarshalXML is a custom XML marshaler for Instance, adapting the top-level raw port values to | ||
// the composite port specifications. | ||
func (i *Instance) MarshalXML(e *xml.Encoder, start xml.StartElement) error { | ||
type instance Instance | ||
aux := struct { | ||
*instance | ||
Port xmlFormatPort `xml:"port"` | ||
SecurePort xmlFormatPort `xml:"securePort"` | ||
}{ | ||
instance: (*instance)(i), | ||
Port: xmlFormatPort{i.Port, i.PortEnabled}, | ||
SecurePort: xmlFormatPort{i.SecurePort, i.SecurePortEnabled}, | ||
} | ||
return e.EncodeElement(&aux, startLocalName("instance")) | ||
} | ||
|
||
// UnmarshalJSON is a custom JSON unmarshaler for InstanceMetadata to handle squirreling away | ||
|
@@ -123,11 +257,6 @@ func (i *InstanceMetadata) MarshalJSON() ([]byte, error) { | |
return i.Raw, nil | ||
} | ||
|
||
// startLocalName creates a start-tag of an XML element with the given local name and no namespace name. | ||
func startLocalName(local string) xml.StartElement { | ||
return xml.StartElement{Name: xml.Name{Space: "", Local: local}} | ||
} | ||
|
||
// MarshalXML is a custom XML marshaler for InstanceMetadata. | ||
func (i InstanceMetadata) MarshalXML(e *xml.Encoder, start xml.StartElement) error { | ||
tokens := []xml.Token{start} | ||
|
@@ -191,7 +320,7 @@ func (m metadataMap) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error | |
return nil | ||
} | ||
|
||
func metadataValue(i DataCenterInfo) interface{} { | ||
func metadataValue(i *DataCenterInfo) interface{} { | ||
if i.Name == Amazon { | ||
return i.Metadata | ||
} | ||
|
@@ -205,7 +334,7 @@ var ( | |
|
||
// MarshalXML is a custom XML marshaler for DataCenterInfo, writing either Metadata or AlternateMetadata | ||
// depending on the type of data center indicated by the Name. | ||
func (i DataCenterInfo) MarshalXML(e *xml.Encoder, start xml.StartElement) error { | ||
func (i *DataCenterInfo) MarshalXML(e *xml.Encoder, start xml.StartElement) error { | ||
if err := e.EncodeToken(start); err != nil { | ||
return err | ||
} | ||
|
@@ -222,6 +351,7 @@ func (i DataCenterInfo) MarshalXML(e *xml.Encoder, start xml.StartElement) error | |
|
||
type preliminaryDataCenterInfo struct { | ||
Name string `xml:"name" json:"name"` | ||
Class string `xml:"-" json:"@class"` | ||
Metadata metadataMap `xml:"metadata" json:"metadata"` | ||
} | ||
|
||
|
@@ -247,8 +377,9 @@ func populateAmazonMetadata(dst *AmazonMetadataType, src map[string]string) { | |
bindValue(&dst.InstanceType, src, "instance-type") | ||
} | ||
|
||
func adaptDataCenterInfo(dst *DataCenterInfo, src preliminaryDataCenterInfo) { | ||
func adaptDataCenterInfo(dst *DataCenterInfo, src *preliminaryDataCenterInfo) { | ||
dst.Name = src.Name | ||
dst.Class = src.Class | ||
if src.Name == Amazon { | ||
populateAmazonMetadata(&dst.Metadata, src.Metadata) | ||
} else { | ||
|
@@ -265,37 +396,76 @@ func (i *DataCenterInfo) UnmarshalXML(d *xml.Decoder, start xml.StartElement) er | |
if err := d.DecodeElement(&p, &start); err != nil { | ||
return err | ||
} | ||
adaptDataCenterInfo(i, p) | ||
adaptDataCenterInfo(i, &p) | ||
return nil | ||
} | ||
|
||
// MarshalJSON is a custom JSON marshaler for DataCenterInfo, writing either Metadata or AlternateMetadata | ||
// depending on the type of data center indicated by the Name. | ||
func (i DataCenterInfo) MarshalJSON() ([]byte, error) { | ||
func (i *DataCenterInfo) MarshalJSON() ([]byte, error) { | ||
type named struct { | ||
Name string `json:"name"` | ||
Name string `json:"name"` | ||
Class string `json:"@class"` | ||
} | ||
if i.Name == Amazon { | ||
return json.Marshal(struct { | ||
named | ||
Metadata AmazonMetadataType `json:"metadata"` | ||
}{named{i.Name}, i.Metadata}) | ||
}{ | ||
named{i.Name, "com.netflix.appinfo.AmazonInfo"}, | ||
i.Metadata, | ||
}) | ||
} | ||
class := "com.netflix.appinfo.MyDataCenterInfo" | ||
if i.Name != MyOwn { | ||
class = i.Class | ||
} | ||
return json.Marshal(struct { | ||
named | ||
Metadata map[string]string `json:"metadata"` | ||
}{named{i.Name}, i.AlternateMetadata}) | ||
Metadata map[string]string `json:"metadata,omitempty"` | ||
}{ | ||
named{i.Name, class}, | ||
i.AlternateMetadata, | ||
}) | ||
} | ||
|
||
func jsonValueAsString(i interface{}) string { | ||
switch v := i.(type) { | ||
case string: | ||
return v | ||
case float64: | ||
return fmt.Sprintf("%.f", v) | ||
case bool: | ||
return strconv.FormatBool(v) | ||
case []interface{}, map[string]interface{}: | ||
// Don't bother trying to decode these. | ||
return "" | ||
case nil: | ||
return "" | ||
default: | ||
panic("type of unexpected value") | ||
} | ||
} | ||
|
||
// UnmarshalJSON is a custom JSON unmarshaler for DataCenterInfo, populating either Metadata or AlternateMetadata | ||
// depending on the type of data center indicated by the Name. | ||
func (i *DataCenterInfo) UnmarshalJSON(b []byte) error { | ||
p := preliminaryDataCenterInfo{ | ||
Metadata: make(map[string]string, 11), | ||
// The Eureka server will mistakenly convert metadata values that look like numbers to JSON numbers. | ||
// Convert them back to strings. | ||
aux := struct { | ||
*preliminaryDataCenterInfo | ||
PreliminaryMetadata map[string]interface{} `json:"metadata"` | ||
}{ | ||
PreliminaryMetadata: make(map[string]interface{}, 11), | ||
} | ||
if err := json.Unmarshal(b, &p); err != nil { | ||
if err := json.Unmarshal(b, &aux); err != nil { | ||
return err | ||
} | ||
adaptDataCenterInfo(i, p) | ||
metadata := make(map[string]string, len(aux.PreliminaryMetadata)) | ||
for k, v := range aux.PreliminaryMetadata { | ||
metadata[k] = jsonValueAsString(v) | ||
} | ||
aux.Metadata = metadata | ||
adaptDataCenterInfo(i, aux.preliminaryDataCenterInfo) | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this float64 and not int? Have you checked it's coming as a fload? Any thoughts of adding inte there as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per the documentation for
json.Unmarshal
, numbers coming from JSON into Go are always of typefloat64
—barring use ofjson.(*Decoder).UseNumber
. (That's implemented here.) As it stands, fargo does not set this option on aDecoder
when decoding afargo.GetAppsResponse
.