Add accounting-method and authorization-method leaf-lists inside the /system/aaa/accounting/events/event and /system/aaa/authorization/events/event lists, respectively.

[LeonGWang]

(M) system/openconfig-aaa.yang

Change Scope

AAA accounting-method and authorization-method should be able to be configured per event-type.
For example event-type=AAA_ACCOUNTING_EVENT_COMMAND can have a different accounting-method configured vs. event-type=AAA_ACCOUNTING_EVENT_LOGIN.

This change is an addition of AAA capabilities for accounting-method and authorization-method and is a non-breaking (backwards-compatible) change.

Platform Implementations

Arista AAA configuration:
accounting and authorization can have different accounting-methods and authorization-methods configured (e.g. group tacacs+, group radius, local, logging) per event type (e.g. commands, exec, etc.).

(config)#aaa accounting commands all console start-stop group tacacs+
(config)#aaa accounting exec console start-stop group radius logging
(config)#aaa authorization commands all default group radius
(config)#aaa authorization exec default group tacacs+ local

Arista documentation for authorization

Arista documentation for accounting

Pyang output for the new model:

module: openconfig-system
  +--rw system
     +--rw aaa
        +--rw config
        +--ro state
        +--rw authorization
        |  +--rw config
        |  |  +--rw authorization-method*   union [DEPRECATED]
        |  +--ro state
        |  |  +--ro authorization-method*   union [DEPRECATED]
        |  +--rw events
        |     +--rw event* [event-type]
        |        +--rw event-type    -> ../config/event-type
        |        +--rw config
        |        |  +--rw event-type?             identityref
        |        |  +--rw authorization-method*   union [NEW]
        |        +--ro state
        |           +--ro event-type?             identityref
        |           +--ro authorization-method*   union [NEW]
        +--rw accounting
        |  +--rw config
        |  |  +--rw accounting-method*   union [DEPRECATED]
        |  +--ro state
        |  |  +--ro accounting-method*   union [DEPRECATED]
        |  +--rw events
        |     +--rw event* [event-type]
        |        +--rw event-type    -> ../config/event-type
        |        +--rw config
        |        |  +--rw event-type?          identityref
        |        |  +--rw record?              enumeration
        |        |  +--rw accounting-method*   union [NEW]
        |        +--ro state
        |           +--ro event-type?          identityref
        |           +--ro record?              enumeration
        |           +--ro accounting-method*   union [NEW]

---

P. S. As an aside (not the topic of this PR), certain event types (e.g. commands) can have privilege levels associated, usually between 0 (unprivileged) to 15 (full privilege). To accommodate privilege levels, we can either:

1. add more event-type identities with privilege level suffix attached such as AAA_ACCOUNTING_EVENT_COMMAND_07 and AAA_ACCOUNTING_EVENT_COMMAND_15 and keep the current tree structure (i.e. backwards compatible)
2. or add an additional list such as privilege-level[level=?] under .../events/event/..., such as

        |     +--rw event* [event-type]
        |        +--rw event-type    -> ../config/event-type
        |        +--rw privilege-level* [level]
        |        |  +--rw level
        |        |  +--rw config
        |        |    +--rw event-type?          identityref
        |        |    +--rw record?              enumeration
        |        |    +--rw accounting-method*   union
        |        |  +--ro state
        |        |    +--ro event-type?          identityref
        |        |    +--ro record?              enumeration
        |        |    +--ro accounting-method*   union

For event-type with only 1 privilege-level, the key could be level=all, and for event-type with multiple privilege levels, the level can be more customized such as level=5. This better delineates between event-type and privilege-level but is a breaking change.

Without either of the 2 changes above, the YANG model would not be able to configure or represent full Aaa accounting and authorization on a vendor switch.

[dplore]

Hi @LeonGWang , thanks for this PR. Please refactor to make this a non-breaking change. One way you could do this is by introducing only new containers/leaves. (ie: instead of move, simply add the authorization-method to the events/event list.).

[dplore]

/gcbrun

[OpenConfigBot]

No major YANG version changes in commit 9f151cb

[LeonGWang]

Hi @LeonGWang , thanks for this PR. Please refactor to make this a non-breaking change. One way you could do this is by introducing only new containers/leaves. (ie: instead of move, simply add the authorization-method to the events/event list.).

Thanks for the quick review @dplore. I've added back the original leaf-lists but marked them as deprecated. This is now a non-breaking changes.

I have also changed the title, the description, and the pyang output.

[dplore]

/gcbrun

[LeonGWang]

Updated the submodules openconfig-aaa-radius.yang and openconfig-aaa-tacacs.yang to version 1.1.0 as well. This should satisfy the failing check.

[jsterne]

Should we not be referencing at least a few other references to implementations with this granularity in this PR?

Although this is not currently a non-backwards-compatible change, by marking the current approach deprecated we are signalling the intent to remove it later (and break current implementations).

[LeonGWang]

Thanks for the feedback @jsterne

Should we not be referencing at least a few other references to implementations with this granularity in this PR?

As per company (Arista) policy, unfortunately I am unable to provide references outside of Arista. Usually @dplore is kind enough to provide additional references should that be required.

Although this is not currently a non-backwards-compatible change, by marking the current approach deprecated we are signalling the intent to remove it later (and break current implementations).

This is intended. The newly added *-method leaf-lists at a more granular level provides a functional superset of the original, so there is no reason to keep the original leaf-lists.

[LeonGWang]

/gcbrun

[dplore]

/gcbrun

[LeonGWang]

/gcbrun

[dplore]

/gcbrun

[ElodinLaarz]

Discussed at OC Operators Meeting Oct 07:

Can we (1) get a better diff of the change (marking which leaves are deprecated and highlighting the leaves that have moved), and (2) @dplore (or anyone) can include a second vendor's implementation to elaborate on the need to move the configuration under events?

[LeonGWang]

P. S. As an aside (not the topic of this PR), certain event types (e.g. commands) can have privilege levels associated, usually between 0 (unprivileged) to 15 (full privilege). To accommodate privilege levels, we can either:

1. add more event-type identities with privilege level suffix attached such as AAA_ACCOUNTING_EVENT_COMMAND_07 and AAA_ACCOUNTING_EVENT_COMMAND_15 and keep the current tree structure (i.e. backwards compatible)
2. or add an additional list such as privilege-level[level=?] under .../events/event/..., such as

        |     +--rw event* [event-type]
        |        +--rw event-type    -> ../config/event-type
        |        +--rw privilege-level* [level]
        |        |  +--rw level
        |        |  +--rw config
        |        |    +--rw event-type?          identityref
        |        |    +--rw record?              enumeration
        |        |    +--rw accounting-method*   union
        |        |  +--ro state
        |        |    +--ro event-type?          identityref
        |        |    +--ro record?              enumeration
        |        |    +--ro accounting-method*   union

For event-type with only 1 privilege-level, the key could be level=all, and for event-type with multiple privilege levels, the level can be more customized such as level=5. This better delineates between event-type and privilege-level but is a breaking change.

Without either of the 2 changes above, the YANG model would not be able to configure or represent full Aaa accounting and authorization on a vendor switch.

[jsterne]

I think we still need to show how any of these changes are necessary on more than 1 implementation. Isn't that supposed to be done by the person proposing these types of changes? The intent here is to eventually break the current model so we should only be doing that if it is needed across a broad set of implementations.

[LeonGWang]

Discussed at OC Operators Meeting Oct 07:

Can we (1) get a better diff of the change (marking which leaves are deprecated and highlighting the leaves that have moved), and (2) @dplore (or anyone) can include a second vendor's implementation to elaborate on the need to move the configuration under events?

Hi I updated the diff in the original comment to annotate deprecated and new leaves with [DEPRECATED] and [NEW]. Below is a copy of the edited version (now also in the original comment):

module: openconfig-system
  +--rw system
     +--rw aaa
        +--rw config
        +--ro state
        +--rw authorization
        |  +--rw config
        |  |  +--rw authorization-method*   union [DEPRECATED]
        |  +--ro state
        |  |  +--ro authorization-method*   union [DEPRECATED]
        |  +--rw events
        |     +--rw event* [event-type]
        |        +--rw event-type    -> ../config/event-type
        |        +--rw config
        |        |  +--rw event-type?             identityref
        |        |  +--rw authorization-method*   union [NEW]
        |        +--ro state
        |           +--ro event-type?             identityref
        |           +--ro authorization-method*   union [NEW]
        +--rw accounting
        |  +--rw config
        |  |  +--rw accounting-method*   union [DEPRECATED]
        |  +--ro state
        |  |  +--ro accounting-method*   union [DEPRECATED]
        |  +--rw events
        |     +--rw event* [event-type]
        |        +--rw event-type    -> ../config/event-type
        |        +--rw config
        |        |  +--rw event-type?          identityref
        |        |  +--rw record?              enumeration
        |        |  +--rw accounting-method*   union [NEW]
        |        +--ro state
        |           +--ro event-type?          identityref
        |           +--ro record?              enumeration
        |           +--ro accounting-method*   union [NEW]

As for other vendor implementation, @dplore can you check if other vendors have similar privilege level configurations for Aaa?