freeconf / yang

Standards-based management for Golang microservices
Apache License 2.0
38 stars 15 forks source link

No access to meta.List's key when module defines list in augment #110

Open berupp opened 6 months ago

berupp commented 6 months ago

I am trying to work with the AST of a module. I am loading the following YANG using parser.LoadModule

module myModule {

    yang-version "1.1";

    namespace "http://myModule.com/ns/yang/eth_dm";

    prefix "myModule";

    organization "myOrg";

    revision 2024-02-26 {
        description
            "Newly generated revision";
        reference "1.0.0";
    }
       augment "/otherModule:object-type" {
        when "/otherModule:object-type/otherModule:typename = 'eth-dm'";
        container eth-dm {
            uses monitored-objects;
        }
    }

    grouping monitored-objects {
        container monitored-objects {
            list monitored-object {
                key "monitored-object-id monitored-object-name";
            }
        }
    }
}

Trying to access they key of the monitored-objects list is not possible

module.Groupings()["monitored-objects"].DataDefinitions()[0].(*meta.Container).DataDefinitions()[0].(*meta.List).KeyMeta()

is returning nil.

The private key property of meta.List is populated, but I noticed that the entire container monitored-objects is not compiled because it is only referenced by the augment and not a by container rooted in the module. Therefore, the code which is populating the keyMeta property of meta.List isn't executed. Which also means that I can provide a key that is referencing a non-existent leaf without getting any compile errors on LoadModule.

dhubler commented 6 months ago

How did you load module?

While this is a little surprising, it is possible error checking doesn't happen until the group is used somewhere.

On Thu, Feb 29, 2024 at 11:16 AM berupp @.***> wrote:

I am trying to work with the AST of a module. I am loading the following YANG using parser.LoadModule

module myModule {

yang-version "1.1";

namespace "http://myModule.com/ns/yang/eth_dm";

prefix "myModule";

organization "myOrg";

revision 2024-02-26 {
    description
        "Newly generated revision";
    reference "1.0.0";
}
   augment "/otherModule:object-type" {
  when "/otherModule:object-type/otherModule:typename = 'eth-dm'";
  container eth-dm {
      uses monitored-objects;
  }

}

grouping monitored-objects { container monitored-objects { list monitored-object { key "monitored-object-id monitored-object-name"; } } } }

Trying to access they key of the monitored-objects list is not possible

module.Groupings()["monitored-objects"].DataDefinitions()[0].(meta.Container).DataDefinitions()[0].(meta.List).KeyMeta()

is returning nil.

The private key property of meta.List is populated, but I noticed that the entire container monitored-objects is not compiled because it is only referenced by the augment and not a by container rooted in the module. Therefore, the code which is populating the keyMeta property of meta.List isn't executed. Which also means that I can provide a key that is referencing a non-existent leaf without getting any compile errors on LoadModule.

— Reply to this email directly, view it on GitHub https://github.com/freeconf/yang/issues/110, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7QMCOFFE7Y52SKR7Z3YV5J47AVCNFSM6AAAAABEAHNLFOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGE3DCNRSGIZTGMQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

berupp commented 6 months ago
p := source.Path(directoryPath)
module, err := parser.LoadModule(p, "myModule")
if err != nil {
    return fmt.Errorf("unable to load module %s: %w", "myModule", err)
}

The grouping is used in the

container eth-dm {
    uses monitored-objects;
}

which is defined in the augment.

It seems the augment is parsed but not compiled at all

dhubler commented 6 months ago

I'll try to factor out into test.

It's there a reason you're trying to get keys on grouping and not final container?

On Thu, Feb 29, 2024, 4:26 PM berupp @.***> wrote:

p := source.Path(directoryPath) module, err := parser.LoadModule(p, "myModule") if err != nil { return fmt.Errorf("unable to load module %s: %w", "myModule", err) }

The grouping is used in the

container eth-dm { uses monitored-objects; }

which is defined in the augment

— Reply to this email directly, view it on GitHub https://github.com/freeconf/yang/issues/110#issuecomment-1971988598, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7VZVZE5M6EIMN6SDFTYV6OGNAVCNFSM6AAAAABEAHNLFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZRHE4DQNJZHA . You are receiving this because you commented.Message ID: @.***>

dhubler commented 6 months ago

While I was able to duplicate no error, when you augment nodes in other modules and then those augmented nodes are not used in your original module, the net result is no nodes in your original module and that is why validator doesn't complain.

module issue-110 { yang-version "1.1"; namespace "freeconf.org/ns/issue-110"; prefix "i";

import issue-110-other {
    prefix "o";
}

augment "/o:c1" {
    when "/o:c1/o:l2 = 'x'";
    container c3 {
        uses g1;
    }
}

grouping g1 {
    container c2 {
        list l1 {
            key "k1 k2";
        }
    }
}

}

module issue-110-other { yang-version "1.1"; namespace "freeconf.org/ns/issue-110"; prefix "i";

container c1 {
    leaf l2 {
        type string;
    }
}

}

If I move c1 into original module and get rid of import, I get the error that keys are missing.

On Thu, Feb 29, 2024 at 8:59 PM Douglas Hubler @.***> wrote:

I'll try to factor out into test.

It's there a reason you're trying to get keys on grouping and not final container?

On Thu, Feb 29, 2024, 4:26 PM berupp @.***> wrote:

p := source.Path(directoryPath) module, err := parser.LoadModule(p, "myModule") if err != nil { return fmt.Errorf("unable to load module %s: %w", "myModule", err) }

The grouping is used in the

container eth-dm { uses monitored-objects; }

which is defined in the augment

— Reply to this email directly, view it on GitHub https://github.com/freeconf/yang/issues/110#issuecomment-1971988598, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7VZVZE5M6EIMN6SDFTYV6OGNAVCNFSM6AAAAABEAHNLFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZRHE4DQNJZHA . You are receiving this because you commented.Message ID: @.***>

berupp commented 6 months ago

Thank you so much for looking into this!

If I move c1 into original module and get rid of import, I get the error

I have made the same observation. But the way my models are structured is to have one module (the main module) with a container object-type and then I have separate models for each object type that define an augment for said object-type container in the main module, just like you have in your example above (c1).

There are other things defined in the main module (typedefs, common groupings etc). I like this structure, because it allows me to add more object types without forcing consumers to recompile the main module, as they'd would have to do if I used a module-submodule relationship.

It's there a reason you're trying to get keys on grouping and not final container?

I don't know how. My models look pretty much like your example. If I load issue-110-other, it doesn't know about the augment in issue-110 and any of its definitions.

If I load issue-110, it doesn't know about c1. I only have access to the groupings, augment etc defined in issue-110.

Is there a way to load issue-110-other and issue-110 as a whole? And the basically have c2 appear under c1? I looked at the API but none of the parser.LoadModuleXXX functions seems to support that

dhubler commented 6 months ago

Parents (main module) know about children (imports) but children don't know about parents. Given this, children cannot augment parents. While this seems limited, it keeps children from "monkey patching" parents which can lead to confusion and keeps children modules reusable w/other parents.

I came up with this which is close. The module issue-110-other can be used w/o issue-110. At first glance you are "polluting" main module with details of children, but it is easier to see what is in the main module and you did accomplish the goal of decoupling object-type from what is defined in other modules.

module issue-110 { yang-version "1.1"; namespace "freeconf.org/ns/issue-110"; prefix "i";

import issue-110-other {
    prefix "o";
}

container object-type {
    uses o:guts {
        when "l2 = 'x'";
    }
    leaf l2 {
        type string;
    }
}

}

module issue-110-other { yang-version "1.1"; namespace "freeconf.org/ns/issue-110"; prefix "o";

grouping guts {
    container c3 {
        container c2 {
            list l1 {
                key "k1 k2";
            }
        }
    }
}

}

Unless I'm wrong this isn't really about FreeCONF's parser limitations, this is really just structuring yang to a non-empty result.

On Fri, Mar 1, 2024 at 11:13 AM berupp @.***> wrote:

Thank you so much for looking into this!

If I move c1 into original module and get rid of import, I get the error

I have made the same observation. But the way my models are structured is to have one module (the main module) with a container object-type and then I have separate models for each object type that define an augment for said object-type container in the main module, just like you have in your example above (c1).

There are other things defined in the main module (typedefs, common groupings etc). I like this structure, because it allows me to add more object types without forcing consumers to recompile the main module, as they'd would have to do if I used a module-submodule relationship.

It's there a reason you're trying to get keys on grouping and not final container?

I don't know how. My models look pretty much like your example. If I load issue-110-other, it doesn't know about the augment in issue-110 and any of its definitions.

If I load issue-110, it doesn't know about c1. I only have access to the groupings, augment etc defined in issue-110.

Is there a way to load issue-110-other and issue-110 as a whole? And the basically have c2 appear under c1? I looked at the API but none of the parser.LoadModuleXXX functions seems to support that

— Reply to this email directly, view it on GitHub https://github.com/freeconf/yang/issues/110#issuecomment-1973468190, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7QTI7JHSA45X4BG6TLYWCSJRAVCNFSM6AAAAABEAHNLFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZTGQ3DQMJZGA . You are receiving this because you commented.Message ID: @.***>

berupp commented 6 months ago

The "augment" statement allows a module or submodule to add to a schema tree defined in an external module, or in the current module and its submodules

Based on my interpretation, augments exist for the purpose of monkey patching, especially when it comes to external modules.

Otherwise, the above suggestion isn't bad. It just removes the advantage of using an augment versus using a parent module with submodules for each object type, because I will have to release the parent module after each addition of an object type.

Would it be feasible to add a function Load to parser that allows loading multiple YANG files? That would allow the application of the augment to container c1 and be present in the compiled module?

dhubler commented 6 months ago

If you add keys to the list so parsers like pyang complete, does it generate a structure you are looking for?

If it does, then it's possible FreeCONF has a bug. If it does not, this is just YANG's limitation.

On Mon, Mar 4, 2024 at 10:05 AM berupp @.***> wrote:

The "augment" statement allows a module or submodule to add to a schema tree defined in an external module, or in the current module and its submodules

Based on my interpretation, augments exist for the purpose of monkey patching, especially when it comes to external modules.

Otherwise, the above suggestion isn't bad. It just removes the advantage of using an augment versus using a parent module with submodules for each object type, because I will have to release the parent module after each addition of an object type.

— Reply to this email directly, view it on GitHub https://github.com/freeconf/yang/issues/110#issuecomment-1976791758, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7QW6JOZUU6C7VREKN3YWSERRAVCNFSM6AAAAABEAHNLFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZWG44TCNZVHA . You are receiving this because you commented.Message ID: @.***>

berupp commented 6 months ago

Yes pyang parses the model flawlessly. I am not posting the full model here because proprietary :) But let's take your example, which essentially looks like mine in a nutshell:

module issue-110-other {
    yang-version "1.1";
    namespace "http://freeconf.org/ns/issue-110-other";
    prefix "i";

    container c1 {
        leaf l2 {
            type string;
        }
    }
}
module issue-110 {
    yang-version "1.1";
    namespace "http://freeconf.org/ns/issue-110";
    prefix "i";

    import issue-110-other {
        prefix "o";
    }

    augment "/o:c1" {
        when "/o:c1/o:l2 = 'x'";
        container c3 {
            uses g1;
        }
    }

    grouping g1 {
        container c2 {
            list l1 {
                key "k1 k2";
                leaf k1 {
                  type string;
                }
                leaf k2 {
                  type string;
                }
            }
        }
    }
}

The result of pyang -f tree * (with both yang files in the directory)


module: issue-110-other
  +--rw c1
     +--rw l2?     string
     +--rw i:c3
        +--rw i:c2
           +--rw i:l1* [k1 k2]
              +--rw i:k1    string
              +--rw i:k2    string
dhubler commented 6 months ago

re:pyang -f tree * Interesting. We could get FreeCONF to support the merging of modules into one module but what practical use is this. If I was developing a device I would need to pick one module or the other, not the merging of multiple modules unless I am missing something.

Also, What do you make of this?

$ pyang -f tree issue-110.yang module: issue-110

augment /o:c1: +--rw c3 +--rw c2 +--rw l1* [k1 k2] +--rw k1 string +--rw k2 string

pyang shows the augment /o:c1. Does indicate a device that used this module could emit data like

{ "c1": ... }

I'm considering consulting the IETF mailing list for this.

On Wed, Mar 6, 2024 at 8:53 AM berupp @.***> wrote:

Yes pyang parses the model flawlessly. I am not posting the full model here because proprietary :) But let's take your example, which essentially looks like mine in a nutshell:

module issue-110-other { yang-version "1.1"; namespace "http://freeconf.org/ns/issue-110-other"; prefix "i";

container c1 {
    leaf l2 {
        type string;
    }
}

}

module issue-110 { yang-version "1.1"; namespace "http://freeconf.org/ns/issue-110"; prefix "i";

import issue-110-other {
    prefix "o";
}

augment "/o:c1" {
    when "/o:c1/o:l2 = 'x'";
    container c3 {
        uses g1;
    }
}

grouping g1 {
    container c2 {
        list l1 {
            key "k1 k2";
            leaf k1 {
              type string;
            }
            leaf k2 {
              type string;
            }
        }
    }
}

}

The result of pyang -f tree * (with both yang files in the directory)

module: issue-110-other +--rw c1 +--rw l2? string +--rw i:c3 +--rw i:c2 +--rw i:l1* [k1 k2] +--rw i:k1 string +--rw i:k2 string

— Reply to this email directly, view it on GitHub https://github.com/freeconf/yang/issues/110#issuecomment-1980925076, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7RNIR4J3Q2CWDEARXLYW4NVBAVCNFSM6AAAAABEAHNLFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBQHEZDKMBXGY . You are receiving this because you commented.Message ID: @.***>

berupp commented 6 months ago

If I was developing a device I would need to pick one module or the other, not the merging of multiple modules unless I am missing something

Let's say you device supports module issue-110-other. But the augmented version, where the augment is defined in issue-110, then device supports:

module: issue-110-other
  +--rw c1
     +--rw l2?     string
     +--rw i:c3
        +--rw i:c2
           +--rw i:l1* [k1 k2]
              +--rw i:k1    string
              +--rw i:k2    string

It makes complete sense to me. Because I understand augments as monkey patch to other existing modules. I think that is where our understanding is different. Once you see an augment as an addition to an existing module, which could or could not be supported by certain devices, this all makes sense

berupp commented 5 months ago

Would it be possible to add an accessor for the private field keys in *meta.List so I have access to the defined keys. This would fix they issue I am currently having without 'compiling' the module.

Right now the keys are in a private field, with no accessor

dhubler commented 5 months ago

It is available @.***/meta#List.KeyMeta

If I recall, you want to "merge" several modules into one kinda like how pyang lets you, so you can test module completeness. Even tho this has no production scope, I can see the benefit for testing. So maybe we go that route, I think it is one of the original suggestions you had.

On Mon, Mar 25, 2024 at 1:03 PM berupp @.***> wrote:

Would it be possible to add an accessor for the private field keys in *meta.List so I have access to the defined keys. This would fix they issue I am currently having without 'compiling' the module.

Right now the keys are in a private field, with no accessor

— Reply to this email directly, view it on GitHub https://github.com/freeconf/yang/issues/110#issuecomment-2018482441, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACA7QXAMYKD7752TMK2ZDY2BKE5AVCNFSM6AAAAABEAHNLFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJYGQ4DENBUGE . You are receiving this because you commented.Message ID: @.***>

berupp commented 5 months ago
Screenshot 2024-03-27 at 9 47 01 AM

It is not in keyMeta because the augment is not compiled. This is my problem that I outlined in the initial comment on this issue.

So problem 1) I do not have access to the key. This I really need, I need access to the defined keys of a list. I forked and added a simple accessor and it works, but I'd rather not fork :) 2) Due to the lack of execution of freeconf's 'compile' (for a augment structure as discussed above), I can technically add key definitions that have no leaf representation and freeconf/yang is fine with it. This should be fixed IMO, but isn't really a pressing issue for me. If the 'compile was executed, keyMeta would be populated as well.

By adding the accessor for the key field in *meta.List, you'd make my day :)