Subject: [netmod] Y07: do not allow when or if-feature on key [Was: YANG 1.1 virtual interim 2014-11-19]

From: Martin Bjorklund <[email-masked]>
Date: Fri, 02 Jan 2015 14:14:24 +0100 (CET)

Phil Shafer <[email-masked]> wrote:
> * REVIEW :Y07: do not allow when or if-feature on keys
> 
> No objection, though I'm not sure why the exception for
> keys with identical conditions to their parents is worth
> the time/trouble to explain.  Forbidding it is simpler.

I agree.  Do we really want to allow:

  list x {
    if-feature "not (a or b)";
    key y;
    leaf y {
      if-feature "(not a) and (not b)";
      ...
    }
    ...
  } 

If we allow such constructs, do we really expect compilers to be able
to detect this?  In the case of "if-feature" it is doable, but not for
"when" in general.

Or are we saying that the argument strings must be
character-by-character equal?  What about whitespace?

I agree w/ Phil; we should simply reject "when" and "if-feature"  on
keys.

/martin

---

From: Ladislav Lhotka <[email-masked]>
Date: Fri, 2 Jan 2015 15:05:31 +0100

> On 02 Jan 2015, at 14:14, Martin Bjorklund <[email-masked]> wrote:
> 

I don’t see any reason for allowing it.

> I agree w/ Phil; we should simply reject "when" and "if-feature"  on

Yes, but what do you mean by rejecting? I think for compatibility reasons “when” and “if-feature” statements should be allowed but ignored on key leafs.

Lada

> 
> 
> /martin
> 
> _______________________________________________
> netmod mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netmod

--
Ladislav Lhotka, CZ.NIC Labs
PGP Key ID: E74E8C0C

---

From: Martin Bjorklund <[email-masked]>
Date: Fri, 02 Jan 2015 15:46:57 +0100 (CET)

Ladislav Lhotka <[email-masked]> wrote:
> Yes, but what do you mean by rejecting? I think for compatibility
> reasons “when” and “if-feature” statements should be allowed but
> ignored on key leafs.

Ok, ignored is ok as well.  It also makes it easier w/ groupings.

/martin

---

From: Andy Bierman <[email-masked]>
Date: Fri, 2 Jan 2015 06:49:23 -0800

Hi,

This would make valid YANG 1.0 modules be rejected.
I strongly object to this change.

Andy

---

From: Ladislav Lhotka <[email-masked]>
Date: Fri, 2 Jan 2015 15:52:44 +0100

> On 02 Jan 2015, at 15:49, Andy Bierman <[email-masked]> wrote:
> 
> Hi,
> 

No 1.0 module will be rejected if the statements are just ignored. It does mean a change in module semantics but then I guess this qualifies as a bug fix.

Lada

---

From: Andy Bierman <[email-masked]>
Date: Fri, 2 Jan 2015 08:24:26 -0800

> No 1.0 module will be rejected if the statements are just ignored. It does mean a change in module semantics but then I guess this qualifies as a bug fix.

IMO it is a really bad idea to allow tools to ignore any statements
in the base language.  This is only OK for external statements
defined with extensions.

It is really confusing to the writer and reader as well.

I agree that it is not feasible for a tool to compare XPath statements
with different context nodes to see if they always produce the
same results.  But it is easy to check if the feature-stmts are the same
or less than the parent.

When-stmt can be inherited from uses and augment.
Not sure if keys can be augmented into a list, but uses can
certainly pull in the keys.

Is module X valid:

module X {
   augment /my-list {
       leaf key1 { type string; }
   }

   list my-list {
      key "key1";
      leaf column1 { type string; }
   }
}

I know module Y is valid:

module Y {
   grouping key1 {
       leaf key1 { type string; }
   }

   list my-list {
      key "key1";
      uses key1;
      leaf column1 { type string; }
   }
}

Note that a when-stmt could appear in the uses-stmt or augment-stmts
making the key leaf conditional.  Also, if-feature-stmts could be present
as well.

I might be in favor of making key leafs that are conditional due to when-stmt
invalid and rejected with a compiler error. It's not OK to ignore this
condition.

Andy

---

From: Martin Bjorklund <[email-masked]>
Date: Fri, 02 Jan 2015 17:39:21 +0100 (CET)

> in the base language.  This is only OK for external statements

We already do this in a couple of instances; e.g. config statement in
rpc/notification, and default in a key leaf.

> certainly pull in the keys.

[...]

> invalid and rejected with a compiler error. It's not OK to ignore this

Consider this situation:

  grouping x {
    leaf a {
      if-feature "bar";
      type string;
    }
  }

  list y {
    key "a b";
    uses x;
    leaf b {
      type string;
    }
  }

Now, if the server doesn't advertise "bar", what does this mean?

YANG 1.0 doesn't say.

We can make it illegal in 1.1, or we can say that the if-feature/when
statements are ignored.

If we make it illegal, it means that there is no way list y can use
the grouping x.

I think Lada's suggestion was to ignore if-feature/when b/c of
backwards compatibility reasons.

This would be legal in 1.1, if we say that we ignore the if-feature:

  list y {
    if-feature "bar";
    key "a b";
    uses x;
    leaf b {
      type string;
    }
  }

/martin

---

From: Andy Bierman <[email-masked]>
Date: Fri, 2 Jan 2015 09:02:41 -0800

> rpc/notification, and default in a key leaf.

And now we will change the rules in YANG 1.1 so default-stmt
is not ignored in a leaf?  How is that backward-compatible?

> Now, if the server doesn't advertise "bar", what does this mean?

As I said before, it is an error for the leaf to have if-feature-stmts
that are not also in the parent node.  Our compiler would flag leaf "a"
as an error.

If an "if-feature bar;" stmt appeared in the parent list y,
then this is not an error at all.

> If we make it illegal, it means that there is no way list y can use

sure it can -- add "if-feature bar;"

> This would be legal in 1.1, if we say that we ignore the if-feature:

This is legal now -- our tools support it and check for key leafs
that are more conditional than their parent. The statements are
validated

Andy

---

From: Martin Bjorklund <[email-masked]>
Date: Sun, 04 Jan 2015 11:09:16 +0100 (CET)

[...]

> >> same results.  But it is easy to check if the feature-stmts are the same

This is exactly the problem that I tried to describe orginally in this
email thread.

The current approach to Y07 is to say that it is ok to have
if-feature/when on key leafs, as long as they are the same as the
parent list node.  This sounds simple, but my point is - and it seems
you agree - that it is not simple.  For boolean expressions in
if-feature it is doable, but for XPath expressions in when it is not.

So how do we solve this?

First of all, this is mostly an issue w/ using leafs defined in a
groping as a key in a list.  If the key leaf is defined "inline", it
is not necessary to allow if-feature/when on the key leaf (*).

1.  Require the expression to be *exactly* the same string, including
    whitespaces.

    There are some corner cases where some groupings cannot be used in
    this case (**).

2.  Require the expression to be logically the same - but this is
    difficult and will probably lead to tools not checking this.

    (Or rather, the expression on the list can be more restrictive
    than the expression on the key; even more difficult to check.)

3.  Ignore the if-feature/when on the key leaf.  This is possibly
    confusing.

4.  Make if-feature/when on the key leaf illegal.  This means that
    certain groupings cannot be used.

5.  Have different rules for if-feature and when.

(*) one interesting case is Y07 combined with Y09, for example:

  list address {
    key "ip vrf";
    leaf ip { ... }
    leaf vrf {
      if-feature vrf;
      mandatory false;
      ...
    }
    ...
  }

(**)

  grouping a {
    leaf a {
      when "/properties/a":
      ...
    }
  }

  grouping b {
    leaf b {
      when "/properties/b":
      ...
    }
  }

  list x {
    when "/properties/a and /properties/b";
    key "a b";
    uses a;
    uses b;
  }

> that are more conditional than their parent.

But this behavior isn't specified in RFC 6020 - and this is the
problem that we want to solve.

/martin

---

From: Ladislav Lhotka <[email-masked]>
Date: Sun, 4 Jan 2015 12:53:11 +0100

> On 04 Jan 2015, at 11:09, Martin Bjorklund <[email-masked]> wrote:
> 

This could possibly work only for a small subset of XPath expressions because of different context nodes. Also, it would be extremely brittle.

>    difficult and will probably lead to tools not checking this.

Yes.

> 3.  Ignore the if-feature/when on the key leaf.  This is possibly

A “default” statement on a key is also ignored.

>    certain groupings cannot be used.

… and makes some 1.0 modules illegal.

> 5.  Have different rules for if-feature and when.

This is IMO too complicated given that such cases should be quite rare.

> (*) one interesting case is Y07 combined with Y09, for example:

This actually makes a good sense and should be allowed if Y09 makes it into 1.1.

Lada

---

From: Andy Bierman <[email-masked]>
Date: Sun, 4 Jan 2015 09:00:10 -0800

> This could possibly work only for a small subset of XPath expressions because of different context nodes. Also, it would be extremely brittle.

This is not very robust.
It only works if the context node has no affect on the expression,
which is a corner-case, and not the way XPath is really used.
(So this solution is out)

>>    difficult and will probably lead to tools not checking this.

This is too hard to implement (except if the context node
is irrelevant -- see previous point).

> … and makes some 1.0 modules illegal.

which modules?

if-feature is not a problem at all.
The solution is clear and simple to implement for if-feature.
Only the whrn-stmt is a problem.  So which YANG modules
currently have when-stmts on key leafs?

> This is IMO too complicated given that such cases should be quite rare.

This is not too complicated.
YANG features are static and the values are not dependent on
the datastore node values.  A key leaf MUST NOT have
if-feature-stmts that do not also apply to its parent.

IMO we should make any when-stmt that applies to a key-leaf illegal,
just to make it clear that conditional keys are not allowed.

> This actually makes a good sense and should be allowed if Y09 makes it into 1.1.

IMO optional keys do not work well at all, and they require a base type to
be changed, which is unacceptable.

Allowing default-stmt on a key leaf does not work because a YANG 1.0 module
clearly ignores the default.

IMO YANG backward compatibility is easy to define:

  Axiom 1: Within YANG 1.0 there are statements with clear behavior and some
  behavior that is under-defined.  Backward compatibility only applies to the
  behavior that is clearly defined.

  Axiom 2: There MUST NOT be any changes in clearly defined behavior
  when a YANG 1.0 module is parsed  as if it were a YANG 1.1 module

Andy

---

From: Ladislav Lhotka <[email-masked]>
Date: Sun, 4 Jan 2015 20:50:52 +0100

> On 04 Jan 2015, at 18:00, Andy Bierman <[email-masked]> wrote:
> 

...

>> … and makes some 1.0 modules illegal.

Those that have if-feature or when on a list key - as Martin wrote, this is not forbidden by RFC 6020.

> just to make it clear that conditional keys are not allowed.

This would still violate the first charter requirement (all compliant 1.0 modules must remain compliant).

>  when a YANG 1.0 module is parsed  as if it were a YANG 1.1 module

The charter says something else. Above the charter text, I think it is also reasonable to avoid changes in the semantics of existing statements unless it is a bug fix. But this case clearly qualifies as a bug fix.

Lada

---

From: Andy Bierman <[email-masked]>
Date: Sun, 4 Jan 2015 12:20:17 -0800

> Those that have if-feature or when on a list key - as Martin wrote, this is not forbidden by RFC 6020.

So none.
There are no modules that have key leafs that have if-feature-stmts
that are not in the parent.  There is nothing in YANG 1.0 that even
suggests that a key leaf is allowed to be absent in a list.

The mechanics of the if-feature-stmt and when-stmt
may inadvertently allow it, but it is disallowed in other text so
this overrides it.

> This would still violate the first charter requirement (all compliant 1.0 modules must remain compliant).

So then the only solution is to say (even though it is already clear)
that key leafs are never optional.  It is an error if an edit would
require the server to remove a key leaf and not its parent.
The server must reject such changes.

> The charter says something else. Above the charter text, I think it is also reasonable to avoid changes in the semantics of existing statements unless it is a bug fix. But this case clearly qualifi…

We do not agree on the bug or the fix.

Andy

---

From: Ladislav Lhotka <[email-masked]>
Date: Mon, 5 Jan 2015 10:28:43 +0100

> On 04 Jan 2015, at 21:20, Andy Bierman <[email-masked]> wrote:
> 

How can you be so sure? I can write one if you wish.

> may inadvertently allow it, but it is disallowed in other text so

OK, so let’s move Y07 to DEAD and forget about it.

Lada

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Wed, 7 Jan 2015 17:45:59 +0100

Hi,

we briefly discussed Y07 during the virtual interim meeting today and
I have re-read this email thread as well. The original proposal was to
adopt Y07-01 and this was verified on the mailing list.

  Make "if-feature" and "when" illegal on leafs that are key, unless
  the condition matches the condition on the parent.

I think the intention here was to be able to detect "if-feature" and
"when" statements on key leafs that are different from the parent's
restriction at compile time. Martin then figured out that it is not
easy to decide this at compile time in the general case for when
statements.

It seems a number of options have meanwhile surfaced:

a) Make if-feature and when statements on key leafs illegal since they
   can at best provide redundant information (and a compiler can easily
   detect the presense of these statements for key leafs and abort with
   an error). (Y07-02)

b) Ignore if-feature and when statements on key leafs (the idea is to
   make it easier to reuse groupings that may have if-feature / when
   constraints inside). (Y07-03)

c) Make if-feature and when statements on key leafs illegal if the
   condition does not match the condition of the parent. (Y07-01)
   As explained above, this requires to be able to determine whether
   conditions are equal, which is difficult at compile-time for when
   expressions.

d) Do not do anything about this in YANG 1.1 but add text to the
   guidelines document explaining why if-feature and when statements
   on key leafs can be problematic, encouraging tools to warn about
   them. (Y07-04)

During today's meeting, there was a movement towards d). Please let me
know if that works for you. Also let the WG know if you think one of
the other options is the way to go or there is yet another option to
consider.

/js

PS: I might add a), b), and d) as possible solutions to the issues
    list

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1, 28759 Bremen, Germany
Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>