CESNET / libyang

YANG data modeling language library
BSD 3-Clause "New" or "Revised" License
369 stars 292 forks source link

Issue with augment of choice and uses clause #1989

Closed andre-d-brizido-alb closed 1 year ago

andre-d-brizido-alb commented 1 year ago

Hi

With yanglint 2.1.25 I'm getting the following error:

yanglint -F ietf-tcp-client: -F bbf-kafka-agent: -F bbf-network-function-client: -Fbbf-d-olt-pppoe-intermediate-agent: -t config \ -f xml -p ../../../nf-standard-adapters/bbf-nf-d_olt_pppoe_ia-standard-1.0/yang/ \ ../../../nf-standard-adapters/bbf-nf-d_olt_pppoe_ia-standard-1.0/yang/bbf-d-olt-pppoe-intermediate-agent.yang \ libyang err : Invalid augment of choice node which is not allowed to contain uses node "bbf-grpcc:grpc-client-transport". (/bbf-d-olt-pppoe-intermediate-agent:{uses='d-olt-pppoe-intermediate-agent'}/bbf-d-olt-pppoe-intermediate-agent:d-olt-pppoe-intermediate-agent/remote-nf/nf-client/{uses='bbf-nfc:nf-client'}/initiate/remote-server/transport/{augment='initiate/remote-server/transport'})

I was trying to convert a XML file into JSON, but the error seems to be related with the YANG file. However, the YANG files were correctly validated with pyang.

This is the YANG exceprt in cause:

          uses bbf-nfc:nf-client {
            augment initiate/remote-server {
              description
                "Insert an identifier for Mfc-SCi or Mfc-CPRi.";
              leaf mfc-type {
                type identityref {
                  base "bbf-d-olt-nft:d-olt-mfc";
                }
                description
                  "Type of Mfc.";
              }
            }
            augment initiate/remote-server/transport {
              description
                "The Mfc-CPRi and Mfc-SCi client endpoints.";
              uses bbf-grpcc:grpc-client-transport {
                augment "grpc-client/access-point/"+

                  "grpc-transport-parameters/tcp-client-options" {
                  description
                    "The gRPC client can use TCP and/or TLS.";
                  case tcp-or-tls-transport {
                    description
                      "Select TCP and optionally TLS transport";
                    container tcp-client-parameters {
                      description
                        "The TCP configuration.";
                      uses tcpc:tcp-client-grouping;
                      choice auth-type {
                        description
                          "A choice amongst supported authentication
                           mechanisms.";
                        case username-password {
                          if-feature "client-username-password";
                          container username-password {
                            leaf username {
                              type string;
                              mandatory true;
                              description
                                "The 'username' value to use for
                                 client identification.";
                            }
                            uses ct:password-grouping {
                              description
                                "The password to be used for client
                                 authentication.";
                            }
                            description
                              "The username/password configuration.";
                          }
                        }
                      }
                    }
                    container tls-client-parameters {
                      description
                        "The TLS configuration.";
                      uses tlsc:tls-client-grouping;
                    }
                  }
                }
              }
            }
          }

Indeed it has some some special tweaks, but they seem to be all legal according to RFC 7950. In attach you can find the complete files.

Best regards Andre

pppoeia-config.zip

yang.zip

michalvasko commented 1 year ago

Well, strictly complying with the RFC 7950, I would say it is not allowed:

   If the target node is a choice node, the "case" statement or a
   shorthand "case" statement (see Section 7.9.2) can be used within the
   "augment" statement.

You may argue that it is the same as when uses is used with these statements but another target has it mentioned explicitly:

   If the target node is a container, list, case, input, output, or
   notification node, the "container", "leaf", "list", "leaf-list",
   "uses", and "choice" statements can be used within the "augment"
   statement.

I am not sure whether this was intentional and what is the reason for leaving uses out but unless allowing it is confirmed on the NETMOD mailing list and ideally an errata is accepted, it will not be changed.

andre-d-brizido-alb commented 1 year ago

Indeed you are right. We saw the 7.17.1 table saying that "uses" is a valid substatement of "augment", but missed that detail in the text.

mallocfailed commented 1 year ago

Thanks for the extra pair of eyes, as always, @michalvasko -- there was indeed a simple fix by injecting a "case" before the subsequent "uses" of the desired grouping. With the multiple nested augments+uses, this was tricky to find.

The solution, for others who stumble upon "YANG augment with uses" threads later:

            augment initiate/remote-server/transport {
              description
                "The Mfc-CPRi and Mfc-SCi client endpoints.";
              case grpc {
                description
                  "Selection for gRPC client transport.";
                uses bbf-grpcc:grpc-client-transport {
                  augment "grpc-client/access-point/"+
                    "grpc-transport-parameters/tcp-client-options" {

Thanks; -JH