Early Review of draft-ietf-opsawg-teas-attachment-circuit-03
review-ietf-opsawg-teas-attachment-circuit-03-yangdoctors-early-aries-2024-01-10-00

2 modules in this draft:
- ietf-ac-svc@2023-11-13.yang
- ietf-bearer-svc@2023-11-13.yang

YANG compiler errors or warnings (pyang 2.6.0, yanglint 2.1.128, yangson 1.4.19)
- No compiler errors or warnings for tree outputs

NOTE: These modules were reviewed and validated (stub instance-data) in
conjunction with draft-ietf-opsawg-teas-common-ac-02 and I did my best to
separate comments out to each even though validation crosses the 2 reviews

General comments on the draft:
- Section 5.1/5.2: Move the "file" declaration in <CODE BEGINS> up to align
  and quote the filename otherwise published IETF tooling will fail to parse
  correctly

General comments on the modules:
- Similar comment to that in the `ietf-ac-common` review in that if there is
  intention for other modules to import and use then ensure any must/when
  statements are fully qualified.  L#272-273 in `ietf-bearer-svc` are one such
  example.
- For `status/admin-status/last-change`, this leaf is `r/w` and while I
  realize this is reuse from `ietf-vpn-common`, it seems that this is
  incorrect and should be reflected as pure `r/o` state.  A client is not
  going to "write" this value to a server however this is an inheritance/reuse
  issue if you agree

Example Validated Instance Data (post qualification fixes):

<key-chains xmlns="urn:ietf:params:xml:ns:yang:ietf-key-chain">
  <key-chain>
    <name>KC1</name>
    <description>KC1 Description</description>
    <key>
      <key-id>131001</key-id>
      <lifetime>
        <send-accept-lifetime>
          <always/>
        </send-accept-lifetime>
      </lifetime>
      <crypto-algorithm>hmac-sha-512</crypto-algorithm>
    </key>
  </key-chain>
</key-chains>
<attachment-circuits xmlns="urn:ietf:params:xml:ns:yang:ietf-ac-svc">
  <ac-group-profile>
    <name>AGP1</name>
    <service-profile>SPP1</service-profile>
    <service-profile>SPP2</service-profile>
    <l2-connection>
      <encapsulation>
        <type
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:ethernet-type</type>
      </encapsulation>
    </l2-connection>
  </ac-group-profile>
  <ac-group-profile>
    <name>AGP2</name>
    <service-profile>SPP1</service-profile>
    <ip-connection>
      <ipv4>
        <local-address>1.1.1.1</local-address>
        <virtual-address>2.2.2.2</virtual-address>
        <prefix-length>31</prefix-length>
        <address-allocation-type
        xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac-common">ac-common:static-address</address-allocation-type>
        <address>
          <address-id>ID1</address-id>
          <customer-address>10.1.1.1</customer-address>
        </address>
      </ipv4>
      <ipv6>
        <local-address>2001:db8:1000::1</local-address>
        <virtual-address>2001:db8:ffff::ffff</virtual-address>
        <prefix-length>127</prefix-length>
        <address-allocation-type
        xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac-common">ac-common:static-address</address-allocation-type>
        <address>
          <address-id>ID1</address-id>
          <customer-address>2001:db8:dead::beef</customer-address>
        </address>
      </ipv6>
    </ip-connection>
    <routing-protocols>
      <routing-protocol>
        <id>RP1</id>
        <type
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:bgp-routing</type>
        <routing-profiles>
          <id>EPI5</id>
          <type
          xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:import-export</type>
        </routing-profiles>
        <bgp>
          <peer-groups>
            <peer-group>
              <name>PG1</name>
              <local-as>65000</local-as>
              <peer-as>65001</peer-as>
              <address-family
              xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:ipv4</address-family>
              <local-address>10.1.1.1</local-address> <authentication>
                <enable>true</enable>
                <keying-material>
                  <enable-ao>true</enable-ao>
                  <ao-keychain>KC1</ao-keychain>
                </keying-material>
              </authentication>
            </peer-group>
          </peer-groups>
          <neighbor>
            <id>N1</id>
            <remote-address>10.2.2.2</remote-address>
            <local-address>10.1.1.1</local-address>
            <peer-group>PG1</peer-group>
            <status>
              <admin-status>
                <status
                xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:admin-up</status>
                <last-change>2023-12-30T15:02:11.353Z</last-change>
              </admin-status>
              <oper-status>
                <status
                xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:op-up</status>
                <last-change>2023-12-30T15:02:11.353Z</last-change>
              </oper-status>
            </status>
          </neighbor>
        </bgp>
      </routing-protocol>
    </routing-protocols>
    <oam>
      <bfd>
        <profile>EPI3</profile>
        <holdtime>180</holdtime>
        <status>
          <admin-status>
            <status
            xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:admin-up</status>
            <last-change>2023-12-30T15:02:11.353Z</last-change>
          </admin-status>
          <oper-status>
            <status
            xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:op-up</status>
            <last-change>2023-12-30T15:02:11.353Z</last-change>
          </oper-status>
        </status>
      </bfd>
    </oam>
    <security>
      <encryption>
        <enabled>true</enabled>
        <layer>layer3</layer>
      </encryption>
      <encryption-profile>
        <customer-key-chain>KC1</customer-key-chain>
      </encryption-profile>
    </security>
    <service>
      <svc-pe-to-ce-bandwidth>
        <bandwidth>
          <bw-type
          xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:bw-per-service</bw-type>
          <cir>10000</cir> <cbs>10000</cbs> <eir>10000</eir> <ebs>10000</ebs>
          <pir>10000</pir> <pbs>10000</pbs>
        </bandwidth>
      </svc-pe-to-ce-bandwidth>
    </service>
  </ac-group-profile>
  <placement-constraints>
    <constraint>
      <constraint-type
      xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:pop-diverse</constraint-type>
      <target>
        <group>
          <group-id>GID1</group-id>
        </group>
      </target>
    </constraint>
  </placement-constraints>
  <ac>
    <name>AC1</name>
    <customer-name>CUSTOMER1</customer-name>
    <description>Attachment Circuit #1</description>
    <requested-start>2023-12-30T14:52:51.353Z</requested-start>
    <requested-stop>2025-12-30T00:00:00.000Z</requested-stop>
    <actual-start>2023-12-30T15:02:10.003Z</actual-start>
    <peer-sap-id>PSID1</peer-sap-id>
    <ac-group-profile>AGP2</ac-group-profile>
    <ac-parent-ref>AC2</ac-parent-ref>
    <group>
      <group-id>GID1</group-id>
      <precedence
      xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac-common">ac-common:primary</precedence>
    </group>
    <service-ref>
      <service-type
      xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:l3vpn</service-type>
      <service-id>SID1</service-id>
    </service-ref>
    <service-profile>SPP1</service-profile>
    <ip-connection>
      <ipv6>
        <local-address>2001:db8::1</local-address>
        <virtual-address>2001:db8::2</virtual-address>
        <prefix-length>128</prefix-length>
        <address-allocation-type
        xmlns:ac-common="urn:ietf:params:xml:ns:yang:ietf-ac-common">ac-common:static-address</address-allocation-type>
      </ipv6>
    </ip-connection>
    <service>
      <qos>
        <qos-profiles>
          <qos-profile>
            <profile>EPI2</profile>
            <direction
            xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:both</direction>
          </qos-profile>
        </qos-profiles>
      </qos>
      <access-control-list>
        <acl-profiles>
          <acl-profile>
            <profile>EPI4</profile>
          </acl-profile>
        </acl-profiles>
      </access-control-list>
    </service>
  </ac>
  <ac>
    <name>AC2</name>
  </ac>
</attachment-circuits>
<specific-provisioning-profiles xmlns="urn:ietf:params:xml:ns:yang:ietf-ac-svc">
  <valid-provider-identifiers>
    <encryption-profile-identifier>
      <id>EPI1</id>
    </encryption-profile-identifier>
    <qos-profile-identifier>
      <id>EPI2</id>
    </qos-profile-identifier>
    <bfd-profile-identifier>
      <id>EPI3</id>
    </bfd-profile-identifier>
    <forwarding-profile-identifier>
      <id>EPI4</id>
    </forwarding-profile-identifier>
    <routing-profile-identifier>
      <id>EPI5</id>
    </routing-profile-identifier>
  </valid-provider-identifiers>
</specific-provisioning-profiles>
<service-provisioning-profiles xmlns="urn:ietf:params:xml:ns:yang:ietf-ac-svc">
  <service-profile-identifier>
    <id>SPP1</id>
  </service-profile-identifier>
  <service-profile-identifier>
    <id>SPP2</id>
  </service-profile-identifier>
</service-provisioning-profiles>
<bearers xmlns="urn:ietf:params:xml:ns:yang:ietf-bearer-svc">
  <placement-constraints>
    <constraint>
      <constraint-type>network-termination-hint</constraint-type>
      <target>
        <group>
          <group-id>G1</group-id>
        </group>
      </target>
    </constraint>
    <constraint>
      <constraint-type
      xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:pop-diverse</constraint-type>
      <target>
        <all-other-bearers/>
      </target>
    </constraint>
    <constraint>
      <constraint-type
      xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:pe-diverse</constraint-type>
      <target>
        <all-other-groups/>
      </target>
    </constraint>
  </placement-constraints>
  <bearer>
    <id>B1</id>
    <description>Description for B1</description>
    <groups>
      <group>
        <group-id>G1</group-id>
      </group>
    </groups>
    <op-comment>Op comment</op-comment>
    <customer-point>
      <identified-by>site-and-device-id</identified-by>
      <device>
        <device-id>devid1</device-id>
        <location>
          <location-name>SJC01</location-name>
          <address>555 Anystreet</address>
          <postal-code>95123</postal-code>
          <state>CA</state>
          <city>San Jose</city>
          <country-code>US</country-code>
        </location>
      </device>
    </customer-point>
    <requested-type>ethernet</requested-type>
    <ac-svc-ref>AC1</ac-svc-ref>
    <requested-start>2023-12-30T14:52:51.353Z</requested-start>
    <requested-stop>2025-12-30T00:00:00.000Z</requested-stop>
    <actual-start>2023-12-30T15:02:10.003Z</actual-start>
    <status>
      <admin-status>
        <status
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:admin-up</status>
        <last-change>2023-12-30T15:02:11.353Z</last-change>
      </admin-status>
      <oper-status>
        <status
        xmlns:vpn-common="urn:ietf:params:xml:ns:yang:ietf-vpn-common">vpn-common:op-up</status>
        <last-change>2023-12-30T15:02:11.353Z</last-change>
      </oper-status>
    </status>
  </bearer>
</bearers>