New policy protobuf protocol.

chrome/tools/build/generate_policy_source.py

 #!/usr/bin/python
 # Copyright (c) 2011 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 '''python %prog [options] platform template
 
 platform specifies which platform source is being generated for
   and can be one of (win, mac, linux).
 template is the path to a .json policy template file.'''
@@ skipping 11 matching lines @@
   parser = OptionParser(usage=__doc__);
   parser.add_option("--pch", "--policy-constants-header", dest="header_path",
                     help="generate header file of policy constants",
                     metavar="FILE");
   parser.add_option("--pcc", "--policy-constants-source", dest="source_path",
                     help="generate source file of policy constants",
                     metavar="FILE");
   parser.add_option("--pth", "--policy-type-header", dest="type_path",
                     help="generate header file for policy type enumeration",
                     metavar="FILE");
+  parser.add_option("--ppb", "--policy-protobuf", dest="proto_path",
+                    help="generate cloud policy protobuf file",
+                    metavar="FILE");
+  parser.add_option("--ppd", "--protobuf-decoder", dest="decoder_path",
+                    help="generate CPP code decoding the policy protobuf",

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

In my experience CPP most of the time refers to the C PreProcessor and most
people use CXX to refer to C++. But why not just say C++? ;)

[Jakob Kummerow, 2011/02/03 14:36:52]

Done.

+                    metavar="FILE");
 
   (opts, args) = parser.parse_args();
 
   if len(args) < 2 or len(args) > 2:
     print "exactly one platform and input file must be specified."
     parser.print_help()
     sys.exit(2)
   template_file_contents = _LoadJSONFile(args[1]);
   if opts.header_path is not None:
-    _WritePolicyConstantHeader(template_file_contents,
-                               args,
-                               opts);
+    _WritePolicyConstantHeader(template_file_contents, args, opts);
   if opts.source_path is not None:
-    _WritePolicyConstantSource(template_file_contents,
-                               args,
-                               opts);
+    _WritePolicyConstantSource(template_file_contents, args, opts);
   if opts.type_path is not None:
-    _WritePolicyTypeEnumerationHeader(template_file_contents,
-                                      args,
-                                      opts);
+    _WritePolicyTypeEnumerationHeader(template_file_contents, args, opts);
+  if opts.proto_path is not None:
+    _WriteProtobuf(template_file_contents, args, opts.proto_path)
+  if opts.decoder_path is not None:
+    _WriteProtobufParser(template_file_contents, args, opts.decoder_path)
 
 
+#------------------ shared helpers ---------------------------------#

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

Is that commonly used comment style in chromium python code?

[Jakob Kummerow, 2011/02/03 14:36:52]

No, I guess it's not. I just felt that this file would benefit from having clear
visual separations of its sections.

 def _OutputGeneratedWarningForC(f, template_file_path):
     f.write('//\n'
             '// DO NOT MODIFY THIS FILE DIRECTLY!\n'
             '// IT IS GENERATED BY generate_policy_source.py\n'
-            '// FROM ' + template_file_path + ' \n'
+            '// FROM ' + template_file_path + '\n'
             '//\n\n')
 
 
+def _GetPolicyNameList(template_file_contents):
+  policy_names = [];
+  for policy in template_file_contents['policy_definitions']:
+    if policy['type'] == 'group':
+      for sub_policy in policy['policies']:
+        policy_names.append(sub_policy['name'])
+    else:
+      policy_names.append(policy['name'])
+  policy_names.sort()
+  return policy_names
+
+
+def _LoadJSONFile(json_file):
+  with open(json_file, "r") as f:
+    text = f.read()
+  return eval(text)
+
+
+#------------------ policy constants header ------------------------#
 def _WritePolicyConstantHeader(template_file_contents, args, opts):
   platform = args[0];
   with open(opts.header_path, "w") as f:
     _OutputGeneratedWarningForC(f, args[1])
     f.write('#ifndef CHROME_COMMON_POLICY_CONSTANTS_H_\n'
             '#define CHROME_COMMON_POLICY_CONSTANTS_H_\n'
             '#pragma once\n'
             '\n'
             'namespace policy {\n\n')
     if platform == "win":
       f.write('// The windows registry path where policy configuration '
               'resides.\nextern const wchar_t kRegistrySubKey[];\n\n')
     f.write('// Key names for the policy settings.\n'
             'namespace key {\n\n')
     for policy_name in _GetPolicyNameList(template_file_contents):
       f.write('extern const char k' + policy_name + '[];\n')
     f.write('\n}  // namespace key\n\n'
             '}  // namespace policy\n\n'
             '#endif  // CHROME_COMMON_POLICY_CONSTANTS_H_\n')
 
 
+#------------------ policy constants source ------------------------#
 def _WritePolicyConstantSource(template_file_contents, args, opts):
   platform = args[0];
   with open(opts.source_path, "w") as f:
     _OutputGeneratedWarningForC(f, args[1])
     f.write('#include "policy/policy_constants.h"\n'
             '\n'
             'namespace policy {\n'
             '\n')
     if platform == "win":
       f.write('#if defined(GOOGLE_CHROME_BUILD)\n'
               'const wchar_t kRegistrySubKey[] = '
               'L"' + CHROME_SUBKEY + '";\n'
               '#else\n'
               'const wchar_t kRegistrySubKey[] = '
               'L"' + CHROMIUM_SUBKEY + '";\n'
               '#endif\n\n')
     f.write('namespace key {\n\n')
     for policy_name in _GetPolicyNameList(template_file_contents):
       f.write('const char k%s[] = "%s";\n' % (policy_name, policy_name))
     f.write('\n}  // namespace key\n\n'
             '}  // namespace policy\n')
 
 
+#------------------ policy type enumeration header -----------------#
 def _WritePolicyTypeEnumerationHeader(template_file_contents, args, opts):
   with open(opts.type_path, "w") as f:
     _OutputGeneratedWarningForC(f, args[1])
     f.write('#ifndef CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_TYPE_H_\n'
             '#define CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_TYPE_H_\n'
             '#pragma once\n'
             '\n'
             'namespace policy {\n'
             '\n'
             'enum ConfigurationPolicyType {\n')
     for policy_name in _GetPolicyNameList(template_file_contents):
       f.write('  kPolicy' + policy_name + ",\n");
     f.write('};\n\n'
             '}  // namespace policy\n\n'
             '#endif  // CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_TYPE_H_\n')
 
 
-def _GetPolicyNameList(template_file_contents):
-  policy_names = [];
-  for policy in template_file_contents['policy_definitions']:
-    if policy['type'] == 'group':
-      for sub_policy in policy['policies']:
-        policy_names.append(sub_policy['name'])
-    else:
-      policy_names.append(policy['name'])
-  policy_names.sort()
-  return policy_names
+#------------------ policy protobuf --------------------------------#
+PROTO_HEAD = '''
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+package enterprise_management;
+
+// Only included in serialized form in the PBs below.

[gfeher, 2011/02/02 08:42:45]

Please update comment.

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

and can't we just switch to repeated string instead of wrapping this in
StringList?

[Jakob Kummerow, 2011/02/03 14:36:52]

Done.

[Jakob Kummerow, 2011/02/03 14:36:52]

As discussed offline: Yes, we could, but we would lose the ability to
differentiate between empty lists and non-existing lists, so we don't.

+message StringList {
+  repeated string entries = 1;
+}
+
+message PolicyOptions {
+  enum PolicyMode {
+    // The user may choose to override the given settings.
+    RECOMMENDED = 1;
+    // The given settings are applied regardless of user choice.
+    MANDATORY = 2;
+  }
+  optional PolicyMode mode = 1;
+}
+

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

Can we have this in a separate .proto that includes the generated stuff?

[Jakob Kummerow, 2011/02/03 14:36:52]

As discussed offline: Yes, we could, but since it's not much code, it's unclear
whether that would be better, so we're leaving it in here for now. If we ever
need more static code here, we can always factor it out into its own file.

+'''
 
 
-def _LoadJSONFile(json_file):
-  with open(json_file, "r") as f:
-    text = f.read()
-  return eval(text)
+def _PolicyTypeToProtobufType(type):
+  if type == 'main':
+    return 'bool'
+  elif type in ('string', 'string-enum'):
+    return 'string'
+  elif type in ('int', 'int-enum'):
+    return 'int64'
+  elif type == 'list':
+    return 'StringList'
+  else:
+    raise NotImplementedError()
 
 
+def _WriteProtobuf(data, args, outfilepath):

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

What's data? Can we have a better name?

[Jakob Kummerow, 2011/02/03 14:36:52]

Done.

+  with open(outfilepath, 'w') as f:
+    _OutputGeneratedWarningForC(f, args[1])
+    f.write(PROTO_HEAD)
+
+    # Write protobufs for individual settings/groups.
+    f.write('// PBs for individual settings.\n\n')
+    for policy in data['policy_definitions']:
+      f.write('message %sProto {\n' % policy['name'])
+      f.write('  optional PolicyOptions policy_options = 1;\n')
+      if policy['type'] == 'group':
+        for sub_policy in policy['policies']:
+          f.write('  optional %s %s = %s;\n' %
+                  (_PolicyTypeToProtobufType(sub_policy['type']),
+                   sub_policy['name'], (sub_policy['protobuf_id'] + 1)))

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

hm, we might want to name the id just ID and not protobuf_id, because of the +1
(and the json file shouldn't know it's being used for proto generation).

[Jakob Kummerow, 2011/02/03 14:36:52]

Done.

+      else:
+        f.write('  optional %s %s = 2;\n' %
+                (_PolicyTypeToProtobufType(policy['type']), policy['name']))

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

Seems like you could use a _WritePolicy function and call it from the two
branches.

[Jakob Kummerow, 2011/02/03 14:36:52]

Done.

+      f.write('}\n\n')
+    # end of: for policy in data['policy_definitions'].
+
+    f.write('// --------------------------------------------------\n'
+            '// Big wrapper PB containing the above groups.\n\n'
+            'message CloudPolicySettings {\n')
+    for policy in data['policy_definitions']:
+      f.write('  optional %sProto %s = %s;\n' %
+              (policy['name'], policy['name'], policy['protobuf_id']))
+    f.write('}\n\n')
+
+
+#------------------ protobuf decoder -------------------------------#
+CPP_HEAD = '''
+#include <limits>
+#include <map>
+#include <string>
+
+#include "base/logging.h"
+#include "base/values.h"
+#include "chrome/browser/policy/proto/device_management_backend.pb.h"
+#include "policy/configuration_policy_type.h"
+
+using google::protobuf::RepeatedPtrField;
+
+namespace policy {
+
+namespace em = enterprise_management;
+
+typedef std::map<ConfigurationPolicyType, Value*> PolicyMapType;
+
+Value* DecodeIntegerValue(google::protobuf::int64 value) {
+  if (value < std::numeric_limits<int>::min() ||
+      value > std::numeric_limits<int>::max()) {
+    LOG(WARNING) << "Integer value " << value
+                 << " out of numeric limits, ignoring.";
+    return NULL;
+  }
+
+  return Value::CreateIntegerValue(static_cast<int>(value));
+}
+
+ListValue* DecodeStringList(const em::StringList& string_list) {
+  ListValue* list_value = new ListValue;
+  RepeatedPtrField<std::string>::const_iterator entry;
+  for (entry = string_list.entries().begin();
+       entry != string_list.entries().end(); ++entry) {
+    list_value->Append(Value::CreateStringValue(*entry));
+  }
+  return list_value;
+}
+
+void DecodePolicy(const em::CloudPolicySettings& policy,
+                  PolicyMapType* mandatory, PolicyMapType* recommended) {
+  DCHECK(mandatory);
+  DCHECK(recommended);
+'''

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

I wouldn't worry to much about this boilerplate header because you need to
generate the proper #includes anyway. Another approach I can see would be to
generate only the DecodePolicy function definition and #include that in some .cc
file that contains the boilerplate.

[Jakob Kummerow, 2011/02/03 14:36:52]

Same as above: as discussed offline, we'll leave this as is for now.

+
+
+CPP_FOOT = '''}
+
+}  // namespace policy
+'''
+
+
+def _CreateValue(type):
+  if type == 'main':
+    return "Value::CreateBooleanValue"
+  elif type in ('int', 'int-enum'):
+    return "DecodeIntegerValue"
+  elif type in ('string', 'string-enum'):
+    return "Value::CreateStringValue"
+  elif type == 'list':
+    return "DecodeStringList"
+  else:
+    raise NotImplementedError()
+
+
+def _GeneratePolicyCode(policy, protoname=None):
+  membername = policy['name'].lower()
+  if protoname is None:
+    protoname = "%s_proto" % policy['name'].lower()

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

can use membername instead of the repeated policy['name'].lower()

[Jakob Kummerow, 2011/02/03 14:36:52]

Done.

+  code = '    if (%s.has_%s()) {\n' % (protoname, membername)
+  code += ('      Value* value = %s(%s.%s());\n' %
+           (_CreateValue(policy['type']), protoname, membername))
+  code += ('      PolicyMapType* result = mandatory;\n'

[gfeher, 2011/02/02 08:42:45]

Nit: rename |result| to |destination|?

[Jakob Kummerow, 2011/02/03 14:36:52]

Done.

+           '      if (%s.has_policy_options() &&\n'
+           '          (%s.policy_options().mode() == '
+                      'em::PolicyOptions::RECOMMENDED)) {\n' %

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

I think I would rather have used a switch(%s.policy_options().mode()) here,
which is future-proof in a way that the compiler will notice if we ever add a
third enum value. WDYT?

[Jakob Kummerow, 2011/02/03 14:36:52]

Done.

+           (protoname, protoname))
+  code += ('        result = recommended;\n'
+           '      }\n'
+           '      result->insert(std::make_pair(kPolicy%s, value));\n' %
+           policy['name'])
+  code += '    }\n'
+  return code
+
+
+def _WriteProtobufParser(data, args, outfilepath):
+  with open(outfilepath, 'w') as f:
+    _OutputGeneratedWarningForC(f, args[1])
+    f.write(CPP_HEAD)
+    for policy in data['policy_definitions']:
+      policyname = policy['name']
+      membername = policyname.lower()
+      proto_type = "%sProto" % policyname
+      protoname = "%s_proto" % policyname.lower()

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

why not use membername?

[Jakob Kummerow, 2011/02/03 14:36:52]

Done.

+      f.write('  if (policy.has_%s()) {\n' % membername)
+      f.write('    const em::%s& %s = policy.%s();\n' %
+              (proto_type, protoname, membername))
+      if policy['type'] == 'group':
+        for sub_policy in policy['policies']:
+          f.write(_GeneratePolicyCode(sub_policy, protoname))

[Mattias Nissler (ping if slow), 2011/02/02 12:27:55]

Why not just pass f to the function instead of building a string and returning
it?

[Jakob Kummerow, 2011/02/03 14:36:52]

Done.

+      else:
+        f.write(_GeneratePolicyCode(policy))
+      f.write('  }\n')
+    f.write(CPP_FOOT)
+
+
+#------------------ main() -----------------------------------------#
 if __name__ == '__main__':
   main();