freeconf / yang

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

Augmenting is failing if target node is any substatment under a choice #76

Closed davidmat50 closed 11 months ago

davidmat50 commented 1 year ago

In the below exmaple, augment "/root-container/choice-init" works fine, But augment "/root-container/choice-init/case-A" fails. Any other augment with following augemnt statements als fails.

augment "/root-container/choice-init/case-A" fails

File used : augment-t3.yang

module augment-t3 {

    namespace "urn:params:basic_augment";
    prefix basic_augment;

    yang-version 1.1;

    augment "/root-container/choice-init/case-A" {
        choice choice-C {
            case case-C1 {
                leaf leaf-C1 {
                    type string;
                }
            }
        }
    }

    augment "/root-container/container-init" {
        choice choice-A {
            case case-A1 {
                leaf leaf-A1 {
                    type string;
                }
            }
        }
    }

    augment "/root-container/choice-init" {
        case case-C {
            leaf leaf-aug-1 {
                type string;
            }
        }
        case case-D {
            leaf-list leaf-list-aug-1{
                type string;
            }
        }
    }

    container root-container {
        container container-init{
            leaf leaf-init1 {
                type string;
            }
        }
        list list-init {
            leaf leaf-init2 {
                type string;
            }
        }
        choice choice-init {  
            case case-A {
                leaf leaf-init3 {
                    type string;
                }
            }
            case case-B {
                leaf leaf-init4 {
                    type string;
                }
            }
        }
    }
}
davidmat50 commented 1 year ago

Another detailed exmaple:

In this exmaple, augment "/root-container/choice-init/case-A" - FAILS augment "/root-container/choice-init/case-container" - FAILS augment "/root-container/choice-init/case-list" - FAILS

augment "/root-container/container-init" - SUCCESS augment "/root-container/list-init" - SUCCESS augment "/root-container/choice-init" - SUCCESS

File augment-t4.yang

module augment-t4 {

    namespace "urn:params:basic_augment";
    prefix basic_augment;

    yang-version 1.1;

    /* The firsts 3 augments here throw error*/

    augment "/root-container/choice-init/case-A" {
        choice choice-C {
            case case-C1 {
                leaf leaf-C1 {
                    type string;
                }
            }
        }
    }

    augment "/root-container/choice-init/case-container" {
        choice choice-E {
            case case-E1 {
                leaf leaf-E1 {
                    type string;
                }
            }
        }
    }

    augment "/root-container/choice-init/case-list" {
        choice choice-F {
            case case-F1 {
                leaf leaf-F1 {
                    type string;
                }
            }
        }
    }

    /* The below three AUGMENTS do not throw errors */

    augment "/root-container/container-init" {
        choice choice-A {
            case case-A1 {
                leaf leaf-A1 {
                    type string;
                }
            }
        }
    }
    augment "/root-container/list-init" {
        choice choice-B {
            case case-B1 {
                leaf leaf-B1 {
                    type string;
                }
            }
        }
    }

    augment "/root-container/choice-init" {
        case case-C {
            leaf leaf-aug-1 {
                type string;
            }
        }
        case case-D {
            leaf-list leaf-list-aug-1{
                type string;
            }
        }
    }

    container root-container {
        container container-init{
            leaf leaf-init1 {
                type string;
            }
        }
        list list-init {
            leaf leaf-init2 {
                type string;
            }
        }
        choice choice-init {
            case case-A {
                leaf leaf-init3 {
                    type string;
                }
            }
            case case-B {
                leaf leaf-init4 {
                    type string;
                }
            }

            container case-container {
                leaf leaf-init5 {
                    type string;
                }
            }

            list case-list {
                leaf leaf-init6 {
                    type string;
                }
            }
        }
    }
}
dhubler commented 1 year ago

i cannot seem to find an exact definition of schema paths. Are case id's and choice ids always required? what about implied case ids (no id). What about action/rpc input and output augments. Do you use "/my-rpc/input/foo" ? RFC seems vague here.

davidmat50 commented 1 year ago

I think you are asking about the schema node identifier for the augment's target node.

As per section, https://datatracker.ietf.org/doc/html/rfc7950#section-7.17 , the following was my understanding,

  1. The target node can be a container, list, choice, case, input, output, or notification node.
  2. RFC does not specifically mention about the whether the target node can be a shorthand case statement of a choice (section-7.9.2), but i assume it is allowed to keep short hand case statement as target, provided the target is one of the above 7 types.
dhubler commented 1 year ago

re: 1. Yes, I know what the targets are, just not clear the exact naming of the path. For rpc/action I think it is safe to assume .../output/... and ../input/...

To target container "q" in...

container x {
  choice y {
     case z {
         container q {}
     }
  }
}

We have .../x/y/z/q

One could argue .../y/z/... is extraneous because there can be only a single q target.

Furthermore

container x {
  choice y {
       container q {}
  }

Is this .../y/q or .../y/q/q because "case q" is implied

davidmat50 commented 1 year ago

For the last exmaple which you have mentioned. ie.

container x {
  choice y {
       container q {}
  }

In this case, i think augment target can be written as either .../y/q or .../y/q/q . Both may need to be supported as valid augment target. There by it does not break any RFC rules.

Also i just tested yang validation tool https://www.yangvalidator.com/yangvalidator suggested by IETF itself ( https://wiki.ietf.org/group/ops/yang-review-tools) . Both augment target path did not throw any error, even if it indictae the same target.

dhubler commented 1 year ago

https://datatracker.ietf.org/doc/html/rfc7950#section-7.9.2

As a shorthand, the "case" statement can be omitted if the branch contains a single "anydata", "anyxml", "choice", "container", "leaf", "list", or "leaf-list" statement. In this case, the case node still exists in the schema tree, and its identifier is the same as the identifier of the child node. Schema node identifiers (Section 6.5) MUST always explicitly include case node identifiers.

So it would be ONLY .../y/q/q according to spec if case statement was missing.

davidmat50 commented 11 months ago

Fixed. https://github.com/freeconf/yang/commit/505fe81ff1acfb4b5bc7cec8a69494cf90f79a89 https://github.com/freeconf/yang/commit/98d1cb7cbde32a421901287d309ed14ffa5ceeb0