ONC: Add IPConfigType.

components/onc/docs/onc_spec.html

Index: components/onc/docs/onc_spec.html
diff --git a/components/onc/docs/onc_spec.html b/components/onc/docs/onc_spec.html
index f4e70c808c956715b81a6564d755b8b156681511..1b7d3381a24cdc845f4773035b5063c404da2355 100644
--- a/components/onc/docs/onc_spec.html
+++ b/components/onc/docs/onc_spec.html
@@ -251,6 +251,24 @@
       string.
     </dd>
 
+    <dt class="field">IPConfigType</dt>
+    <dd>
+      <span class="field_meta">
+        (optional if <span class="field">Remove</span> is
+        <span class="value">false</span>, otherwise ignored. Defaults to
+        <span class="value">DHCP</span>)

[stevenjb, 2014/12/04 00:14:30]

Actually, e.g., ethernet.onc contains a StaticIPConfig entry, but no
IPConfigType. Should that be valid, i.e. should the presence of a StaticIPConfig
struct imply IPConfigType = 'Static' if not specified? 

The thing that makes this complicated is the need (or at least, desire) to
support partial ONC configurations. We are kind of committed to that since
networkingPrivate supports it in the API description and I believe that existing
extensions already make use of that. It is certainly more convenient than
requiring a round trip + merge resolution with existing settings.

So, I believe there are three valid scenarios we need to consider:

a) IPConfigType = DHCP : Clear any existing StaticIPConfig properties.
StaticIPConfig must not be present.
b) IPConfigType = Static: Apply the StaticIPConfig properties which must be
valid.
c) IPConfigType unspecified, StaticIPConfig exists: treat as (b).
d) IPConfigType unspecified, no StaticIPConfig entry: No change to the current
configuration.

If we want to support c), I believe that IPConfigType needs to default to
undefined instead of 'DHCP'.

[pneubeck (no reviews), 2014/12/04 12:17:42]

Depending on the source/usage of ONC, we remove 'irrelevant'
values/subdictionaries.
The Validator doesn't care about 'irrelevant' or 'ignored' fields, it also
doesn't remove them.
The Normalizer takes care of removing these irrelevant fields and is applied for
example to policies.

Currently, we apply the Normalizer to any ONC before we convert it to the Shill
dictionary (e.g. in CreateShillConfiguration:
https://code.google.com/p/chromium/codesearch#chromium/src/chromeos/network/p...)
One reason for that was: if a 'type' field changes (e.g. EAP type), any
properties that were required by the old type shouldn't be written anymore to
Shill, independent on whether we remember the old properties for the user or
not.
Similarly, we could end-up in situations where the user setting says
'EAP.Outer=LEAP' and the policy says 'EAP.Outer=EAP-TLS', in which case LEAP
specific settings shouldn't be sent to Shill.
Supporting partial updates to consumers of the API, I think, is fine.
But just sending these partial dictionaries to Shill is AFAIU rather difficult,
the moment policies come into play. We could have different control flows
depending on whether a policy exists or not, but I think that would be an
avoidance strategy rather than a solution.

I didn't finish that implementation, but my original plan was to handle a
SetProperties call by:
1) obtain the old user settings
2) merge the new (partial) user settings update with the old user settings
3) merge with policy
4) convert to Shill and send to Shill. This step should include any cleanup work
like the 'Clear IPConfig' mechanism or the ONCNormalizer that removes
irrelevant/ignored fields.

Step 1) and 2) are not implemented. I didn't get to it and wanted to discuss
this with you further when I'm in MTV.

One nice side effect would be that you can very easily compare the old and the
new ONC settings and trigger ipconfig updates.
IMO, a partial update can be partial in every sense. The partial update should
simply be merged with old settings and policy.
After the merging however, the settings must be sufficiently complete. E.g. the
IPConfigType=Static could have come from policy and the user only has to specify
the content of StaticIPConfig.

I think we should not support c) _after_ we merged the settings, because then we
would be back to the problem of how to transition from Static to DHCP settings
and we would again have to consider special 'NULL' values which we already found
out to be in conflict with Recommended policy settings.
It must default to DHCP because that is the current behavior of policy that we
have to continue to support.

+        <span class="type">string</span>
+      </span>
+      <span class="rule">
+        <span class="rule_id"></span>
+        Allowed values are <span class="value">DHCP</span> and
+        <span class="value">Static</span>.
+      </span>
+      Determines whether the IP configuration is statically configured, see
+      <span class="field">StaticIPConfig</span> or automatically configured
+      using DHCP.
+    </dd>
+
     <dt class="field">IPConfigs</dt>
     <dd>
       <span class="field_meta">
@@ -263,8 +281,8 @@
     <dt class="field">StaticIPConfig</dt>
     <dd>
       <span class="field_meta">
-        (optional if <span class="field">Remove</span> is
-        <span class="value">false</span>, otherwise ignored)
+        (required if <span class="field">IPConfigType</span> is
+        <span class="value">DHCP</span>)

[stevenjb, 2014/12/04 00:14:30]

Shouldn't this be Static?

[pneubeck (no reviews), 2014/12/04 12:17:42]

doh, of course,

         <span class="type">IPConfig</span>
       </span>
       Each property set in this IPConfig object overrides the respective