ONC: Fix Static-/Saved-/IPConfig.

ONC: Fix Static-/Saved-/IPConfig.

Before commit 41ec57e48ebd467c7a3b169a5598fb8038f54232,
IPConfigs was the field in ONC meant for configuring ip configuration of a network.
This was changed to:
- IPConfigs is the set of currently active ip configurations (it can be more than one if IPv4 and IPv6 are coexisting in parallel) reported by ChromeOS
- StaticIPConfig (note that it's a single configuration, not multiple as before) is the writable field that the user or policy can set
- SavedIPConfig is the configuration received via DHCP and reported by ChromeOS.

This change updates the different ONC functions to these new field definitions.

Effectively this also allows setting the IPConfig of a network through ONC import, the networkingPrivate API, and through policy (there's no server support though).

BUG=410877
Committed: https://crrev.com/faf50c40335fd2055c8beed69c0f630bd442d880
Cr-Commit-Position: refs/heads/master@{#293915}

[pneubeck (no reviews), 2014-09-05 13:29:41]

pneubeck@chromium.org changed reviewers:
+ stevenjb@chromium.org

[pneubeck (no reviews), 2014-09-05 13:34:02]

@Paul,

is it safe to assume that the service properties StaticIPConfig* and
SavedIPConfig* have always values of type IPv4 and never IPv6 ?

There is no 'type' or 'method' property that would indicate their type and
reconstructing the type from the values is not robust, so I'd like to avoid it.

[pneubeck (no reviews), 2014-09-05 13:37:46]

On 2014/09/05 13:34:02, pneubeck wrote:
> @Paul,
> 
> is it safe to assume that the service properties StaticIPConfig* and
> SavedIPConfig* have always values of type IPv4 and never IPv6 ?
> 
> There is no 'type' or 'method' property that would indicate their type and
> reconstructing the type from the values is not robust, so I'd like to avoid
it.

Sorry, I meant the properties StaticIP.* (like StaticIP.Address).

[stevenjb, 2014-09-05 15:44:39]

Thanks a ton for doing this, there are several details that I missed when I
tried which would have taken me a lot longer to track down.
LGTM

[pneubeck (no reviews), 2014-09-05 16:07:05]

On 2014/09/05 15:44:39, stevenjb wrote:
> Thanks a ton for doing this, there are several details that I missed when I
> tried which would have taken me a lot longer to track down.
> LGTM

yeah, I would primarily blame the missing documentation in this part of the
code... my fault, I should really add some more.

[pneubeck (no reviews), 2014-09-05 16:07:07]

On 2014/09/05 15:44:39, stevenjb wrote:
> Thanks a ton for doing this, there are several details that I missed when I
> tried which would have taken me a lot longer to track down.
> LGTM

yeah, I would primarily blame the missing documentation in this part of the
code... my fault, I should really add some more.

[Paul Stewart, 2014-09-05 18:19:27]

On 2014/09/05 13:34:02, pneubeck wrote:
> @Paul,
> 
> is it safe to assume that the service properties StaticIPConfig* and
> SavedIPConfig* have always values of type IPv4 and never IPv6 ?
> 
> There is no 'type' or 'method' property that would indicate their type and
> reconstructing the type from the values is not robust, so I'd like to avoid
it.

Yes, it's safe to assume the values in SavedIP.* and StaticIP.* will always be
IPv4.  There are no current plans to support static IP in IPv6, so although
there's an IPConfig object associated with the Device for IPv6 configurations,
there are no current plans to support static parameters.

[stevenjb, 2014-09-05 19:55:23]

So, I just noticed that the Managed format is different for IPConfigs vs
Saved/StaticIPConfig:

"IPConfigs": { "Active": [ { "IPAddress": "1.2.3.4", ... } ] }
"SavedIPConfig": { "IPAddress": { "Active": "1.2.3.4" }, ... }

I think that we should fix the translation so that all IPConfig dictionaries are
treated as a single entity from a policy perspective (i.e. use the IPConfigs
form).

[pneubeck (no reviews), 2014-09-08 08:23:37]

On 2014/09/05 19:55:23, stevenjb wrote:
> So, I just noticed that the Managed format is different for IPConfigs vs
> Saved/StaticIPConfig:
> 
> "IPConfigs": { "Active": [ { "IPAddress": "1.2.3.4", ... } ] }
> "SavedIPConfig": { "IPAddress": { "Active": "1.2.3.4" }, ... }
> 
> I think that we should fix the translation so that all IPConfig dictionaries
are
> treated as a single entity from a policy perspective (i.e. use the IPConfigs
> form).

Good point.
So for StaticIPConfig, which can be set by the user/policy, we should keep the
granularity: e.g. policy can preconfigure all but IPAddress, which the user can
set.
That means, there the dictionary will look like:
  "StaticIPConfig": { "IPAddress": { "UserSetting": "1.2.3.4", ... }, ... }

For the others, which are read-only, I wonder whether we should just remove the
augmentation completely, i.e. remove the dictionary with the 'Active' and
'Effective: Unmanaged' fields and just keep the plain value.
I could do that for _all_ non-ONC / read-only properties (i.e. all properties
that are not part of the network_configuration_signature) in a follow-up.

[pneubeck (no reviews), 2014-09-08 08:28:17]

sorry, didn't upload my latest local changes.
ptal at the opposite translation onc -> shill

i'll commit now anyways already.

[pneubeck (no reviews), 2014-09-08 08:28:31]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2014-09-08 08:29:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pneubeck@chromium.org/540333002/20001

[commit-bot: I haz the power, 2014-09-08 08:34:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-08 08:34:21]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[pneubeck (no reviews), 2014-09-08 08:46:06]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2014-09-08 08:47:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pneubeck@chromium.org/540333002/40001

[commit-bot: I haz the power, 2014-09-08 08:54:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-08 08:54:16]

Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[stevenjb, 2014-09-08 17:23:14]

Still lgtm

https://codereview.chromium.org/540333002/diff/40001/chromeos/network/onc/onc...
File chromeos/network/onc/onc_translator_onc_to_shill.cc (right):

https://codereview.chromium.org/540333002/diff/40001/chromeos/network/onc/onc...
chromeos/network/onc/onc_translator_onc_to_shill.cc:40: }
nit: Since this isn't someplace where it can be re-used, why not make this just
create the comma-separated string instead of calling JoinString below?

[pneubeck (no reviews), 2014-09-08 22:04:33]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2014-09-08 22:05:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pneubeck@chromium.org/540333002/40001

[commit-bot: I haz the power, 2014-09-08 22:33:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-08 22:33:27]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/53062)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/58053)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[pneubeck (no reviews), 2014-09-09 07:34:44]

The CQ bit was checked by pneubeck@chromium.org

[pneubeck (no reviews), 2014-09-09 07:36:04]

https://codereview.chromium.org/540333002/diff/40001/chromeos/network/onc/onc...
File chromeos/network/onc/onc_translator_onc_to_shill.cc (right):

https://codereview.chromium.org/540333002/diff/40001/chromeos/network/onc/onc...
chromeos/network/onc/onc_translator_onc_to_shill.cc:40: }
On 2014/09/08 17:23:14, stevenjb wrote:
> nit: Since this isn't someplace where it can be re-used, why not make this
just
> create the comma-separated string instead of calling JoinString below?
> 

There are two other usages, which I can unify in a follow up:
https://code.google.com/p/chromium/codesearch#search/&q=ConvertListValueToStr...

[commit-bot: I haz the power, 2014-09-09 07:37:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pneubeck@chromium.org/540333002/60001

[commit-bot: I haz the power, 2014-09-09 08:51:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-09 08:51:58]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pneubeck (no reviews), 2014-09-09 09:48:45]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2014-09-09 09:50:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pneubeck@chromium.org/540333002/80001

[commit-bot: I haz the power, 2014-09-09 10:47:05]

Committed patchset #5 (id:80001) as dd2c7d520f2bb61970dea94e6357883d3ea4201c

[commit-bot: I haz the power, 2014-09-10 03:52:38]

Patchset 5 (id:??) landed as
https://crrev.com/faf50c40335fd2055c8beed69c0f630bd442d880
Cr-Commit-Position: refs/heads/master@{#293915}