search

openconfig / ygot

A YANG-centric Go toolkit - Go/Protobuf Code Generation; Validation; Marshaling/Unmarshaling
Apache License 2.0
284 stars 106 forks source link

Add support for YinstanceIdentifier: "instance-identifier" #398

Open jmacauley opened 4 years ago

jmacauley commented 4 years ago

I am using a YANG definition from a hardware vendor that makes use of the "instance-identifier" type in a leaf definition. The generator will compile the YANG however I encounter a runtime error when parsing the result of a RESTCONF GET.

Here is the error encountered:

ERROR: logging before flag.Parse: E0602 21:27:55.439979   88429 util_types.go:192] unexpected type instance-identifier in yangToJSONType
ERROR: logging before flag.Parse: E0602 21:27:55.439997   88429 util_types.go:192] unexpected type instance-identifier in yangToJSONType
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x98 pc=0x14a3d80]

goroutine 1 [running]:
github.com/openconfig/ygot/ytypes.sanitizeJSON(0x17e95c0, 0xc000ea29c0, 0xc000675800, 0x16b8f9f, 0x8, 0x1701520, 0xc004335c10, 0xc000e154a0, 0x20300000000000, 0x333ffff, ...)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/leaf.go:690 +0x10e9
github.com/openconfig/ygot/ytypes.unmarshalScalar(0x17e95c0, 0xc000ea29c0, 0xc000675800, 0x16b8f9f, 0x8, 0x1701520, 0xc004335c10, 0x0, 0x8, 0x10, ...)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/leaf.go:666 +0x3ad
github.com/openconfig/ygot/ytypes.unmarshalLeaf(0xc000675800, 0x17e95c0, 0xc000ea29c0, 0x1701520, 0xc004335c10, 0x0, 0x4, 0x203000)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/leaf.go:370 +0x295
github.com/openconfig/ygot/ytypes.unmarshalGeneric(0xc000675800, 0x17e95c0, 0xc000ea29c0, 0x1701520, 0xc004335c10, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/unmarshal.go:92 +0x5e9
github.com/openconfig/ygot/ytypes.unmarshalStruct(0xc000675080, 0x17e95c0, 0xc000ea29c0, 0xc00433e000, 0x0, 0x0, 0x0, 0x0, 0x0, 0x20)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/container.go:206 +0x863
github.com/openconfig/ygot/ytypes.unmarshalList(0xc000675080, 0x17451c0, 0xc00433ebd0, 0x16edc40, 0xc0011c49a0, 0x0, 0x0, 0x0, 0x0, 0x100f890, ...)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/list.go:333 +0x4c9
github.com/openconfig/ygot/ytypes.unmarshalGeneric(0xc000675080, 0x17451c0, 0xc00433ebd0, 0x16edc40, 0xc0011c49a0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/unmarshal.go:96 +0x4e3
github.com/openconfig/ygot/ytypes.unmarshalStruct(0xc0000abc80, 0x18095a0, 0xc004352740, 0xc0011d62a0, 0x0, 0x0, 0x0, 0x0, 0xc0001c0f30, 0x100e196)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/container.go:206 +0x863
github.com/openconfig/ygot/ytypes.unmarshalContainer(0xc0000abc80, 0x18095a0, 0xc004352740, 0x174b520, 0xc0011d62a0, 0x0, 0x0, 0x0, 0x0, 0x2259ea0, ...)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/container.go:138 +0x2c5
github.com/openconfig/ygot/ytypes.unmarshalGeneric(0xc0000abc80, 0x18095a0, 0xc004352740, 0x174b520, 0xc0011d62a0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/unmarshal.go:100 +0x2f3
github.com/openconfig/ygot/ytypes.unmarshalStruct(0xc0000abb00, 0x17e9480, 0xc001165080, 0xc000e63a10, 0x0, 0x0, 0x0, 0x0, 0xc0001c1490, 0x100e196)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/container.go:206 +0x863
github.com/openconfig/ygot/ytypes.unmarshalContainer(0xc0000abb00, 0x17e9480, 0xc001165080, 0x174b520, 0xc000e63a10, 0x0, 0x0, 0x0, 0x0, 0x2259ea0, ...)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/container.go:138 +0x2c5
github.com/openconfig/ygot/ytypes.unmarshalGeneric(0xc0000abb00, 0x17e9480, 0xc001165080, 0x174b520, 0xc000e63a10, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/unmarshal.go:100 +0x2f3
github.com/openconfig/ygot/ytypes.unmarshalStruct(0xc0000ab980, 0x17ae6a0, 0xc000e639d8, 0xc000e639e0, 0x0, 0x0, 0x0, 0x0, 0xc0001c19f0, 0x100e196)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/container.go:206 +0x863
github.com/openconfig/ygot/ytypes.unmarshalContainer(0xc0000ab980, 0x17ae6a0, 0xc000e639d8, 0x174b520, 0xc000e639e0, 0x0, 0x0, 0x0, 0x0, 0x40, ...)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/container.go:138 +0x2c5
github.com/openconfig/ygot/ytypes.unmarshalGeneric(0xc0000ab980, 0x17ae6a0, 0xc000e639d8, 0x174b520, 0xc000e639e0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/unmarshal.go:100 +0x2f3
github.com/openconfig/ygot/ytypes.Unmarshal(0xc0000ab980, 0x17ae6a0, 0xc000e639d8, 0x174b520, 0xc000e639e0, 0x0, 0x0, 0x0, 0x1050668, 0x24248144c480b)
        /Users/hacksaw/go/src/github.com/openconfig/ygot/ytypes/unmarshal.go:47 +0x8d
github.com/jmacauley/topology/g30/models/fp4_1_1.Unmarshal(0xc000ee4000, 0x12e607, 0x12e807, 0x1c98680, 0xc000e639d8, 0x0, 0x0, 0x0, 0x203000, 0x203000)
        /Users/hacksaw/go/src/github.com/jmacauley/topology/g30/models/fp4_1_1/fp4_1_1.go:90 +0x1b5

A quickly placed debug print shows this is the field and value causing the issue:

fieldName = FmEntity
value = /ne:ne/shelf[shelf-id='1']/slot[slot-id='1']/card/port[port-id='1']/och-os

The associated YANG definition is as follows:

    leaf "fm-entity" {
      type "instance-identifier" {
        require-instance "true";
      }

      description
        "The management object instance which the alarm or condition is reported against.";
    }

Here is the instance causing the failure:

      "standing-condition":  [{
          "fm-entity":  "/ne:ne/shelf[shelf-id='1']/slot[slot-id='1']/card/port[port-id='1']/och-os",
          "condition-type":  "LOS",
          "location":  "near-end",
          "direction":  "ingress",
          "time-period":  "not-applicable",
          "service-affect":  "SA",
          "severity-level":  "critical",
          "occurrence-date-time":  "2020-06-01T17:45:49+01:00",
          "condition-description":  "Loss Of Signal",
          "fm-entity-type":  "OCH-OS",
          "alarm-id":  "00BFF29F60ED224A04EF882F5F2F980C00121100"
        },
wenovus commented 4 years ago

Just took a cursory look, it looks like JSON unmarshalling doesn't support instance-identifier. I'm not familiar with this so it might take me some time to figure this out. @robshakir are you more familiar with this?

jmacauley commented 4 years ago

I also dug into this last night to see if I could shim in something to get around the error. Syntactically the identifier is a string, but behaviourally it is an identifier that references an instance node in the data tree. Not sure what type of validation you might want to put on this when doing model validation, but from my perspective getting the identifier parsed successfully is about all I need at this moment. In fact, until this fixed I will probably change the YANG to type string just to get it parsing.

@wenovus thank you for the support.

robshakir commented 4 years ago

hi folks!

Yes, today we don't support bits or instance-identifier types (supported types are in this table). There's no real reason that they can't be supported -- just we didn't really have folks that were using these types that had thoughts on what the representation in Go would be.

We could generate these as a *string in the generated code, and then see whether we can re-use the leafref validation (and when validation going forward) to see whether they're valid instances in the schema. Adding the first part of this is relatively easy to do with changes to ygen.

Cheers! r.

jmacauley commented 4 years ago

@robshakir generating them as strings would be great. Given the YANG I am using has some bugs and fails validation I am really only looking for the ability to parse it at the moment.

saintnoah commented 4 years ago

@wenovus sorry can I ask a side question.. Does protogenerator support YANG notification statement? I was trying to use protogenerator to convert yang(with notification statement inside) to proto, but got error below:

protogenerator.go:128] unknown type of entry Directory in findMappableEntities for xxxx

Thanks!

robshakir commented 4 years ago

It does not. No ygot tool currently supports the YANG Notification or RPC statement.

r.

On Fri, 12 Jun 2020 at 18:52, saintnoah notifications@github.com wrote:

@wenovus https://github.com/wenovus sorry can I ask a side question.. Does protogenerator support YANG notification statement? https://tools.ietf.org/html/rfc6020#section-7.14.1 I was trying to use protogenerator to convert yang(with notification statement inside) to proto, but got error below:

protogenerator.go:128] unknown type of entry Directory in findMappableEntities for xxxx

Thanks!

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/openconfig/ygot/issues/398#issuecomment-643551652, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABE43VMFMQIKQ2UUUIXZGCDRWLLVPANCNFSM4NR4GHJA .

saintnoah commented 4 years ago

thanks Rob for the confirmation.