ovsdb: Enforce immutability of immutable columns.

OVSDB has always had the ability to mark a column as "immutable", so that
its value cannot be changed in a given row after that row is initially
inserted.  However, we discovered recently that ovsdb-server has never
enforced this constraint.  This commit implements enforcement.

Reported-by: Paul Ingram <paul@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>

diff --git a/NEWS b/NEWS
index 872f8d0d85a81a1034c4249cd0af984e5490f4c8..cbc5c58627f0697935b8657e420bb207747cc00b 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,8 @@ post-v1.8.0
       are true, but because we do not know of any users for this
       feature it seems better on balance to remove it.  (The ovs-pki-cgi
       program was not included in distribution packaging.)
       are true, but because we do not know of any users for this
       feature it seems better on balance to remove it.  (The ovs-pki-cgi
       program was not included in distribution packaging.)

+    - ovsdb-server now enforces the immutability of immutable columns.  This
+      was not enforced in earlier versions due to an oversight.

     - Stable bond mode is deprecated and will be removed no earlier than
       February 2013.  Please email dev@openvswitch.org with concerns.
     - The autopath action is deprecated and will be removed no earlier than
     - Stable bond mode is deprecated and will be removed no earlier than
       February 2013.  Please email dev@openvswitch.org with concerns.
     - The autopath action is deprecated and will be removed no earlier than

diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h
index 546acbb84698e5ddce519803dbc08ea8455fc1ea..7ea5ce6902d887ee6668d4e5fe45fd84a7e8fbad 100644 (file)
--- a/lib/ovsdb-idl-provider.h
+++ b/lib/ovsdb-idl-provider.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.

  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.


@@ -41,6 +41,7 @@ struct ovsdb_idl_row {
 struct ovsdb_idl_column {
     char *name;
     struct ovsdb_type type;
 struct ovsdb_idl_column {
     char *name;
     struct ovsdb_type type;

+    bool mutable;

     void (*parse)(struct ovsdb_idl_row *, const struct ovsdb_datum *);
     void (*unparse)(struct ovsdb_idl_row *);
 };
     void (*parse)(struct ovsdb_idl_row *, const struct ovsdb_datum *);
     void (*unparse)(struct ovsdb_idl_row *);
 };

diff --git a/ovsdb/execution.c b/ovsdb/execution.c
index 1aff0c51eaca917d2c2d8cc86b7c629050bed06e..300c247a3d98a3485b217006c5eb9c978e07229a 100644 (file)
--- a/ovsdb/execution.c
+++ b/ovsdb/execution.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.

  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.


@@ -434,6 +434,22 @@ ovsdb_execute_update(struct ovsdb_execution *x, struct ovsdb_parser *parser,
     if (!error) {
         error = parse_row(row_json, table, x->symtab, &row, &columns);
     }
     if (!error) {
         error = parse_row(row_json, table, x->symtab, &row, &columns);
     }

+    if (!error) {
+        size_t i;
+
+        for (i = 0; i < columns.n_columns; i++) {
+            const struct ovsdb_column *column = columns.columns[i];
+
+            if (!column->mutable) {
+                error = ovsdb_syntax_error(parser->json,
+                                           "constraint violation",
+                                           "Cannot update immutable column %s "
+                                           "in table %s.",
+                                           column->name, table->schema->name);
+                break;
+            }
+        }
+    }

     if (!error) {
         error = ovsdb_condition_from_json(table->schema, where, x->symtab,
                                           &condition);
     if (!error) {
         error = ovsdb_condition_from_json(table->schema, where, x->symtab,
                                           &condition);

diff --git a/ovsdb/mutation.c b/ovsdb/mutation.c
index 0dcd16fec9b54b38d04be7439fcdac887f918752..5fd983a4badfa634707db5017761a9c6eb158710 100644 (file)
--- a/ovsdb/mutation.c
+++ b/ovsdb/mutation.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.

  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.


@@ -95,6 +95,12 @@ ovsdb_mutation_from_json(const struct ovsdb_table_schema *ts,
                                   "No column %s in table %s.",
                                   column_name, ts->name);
     }
                                   "No column %s in table %s.",
                                   column_name, ts->name);
     }

+    if (!m->column->mutable) {
+        return ovsdb_syntax_error(json, "constraint violation",
+                                  "Cannot mutate immutable column %s in "
+                                  "table %s.", column_name, ts->name);
+    }
+

     ovsdb_type_clone(&m->type, &m->column->type);
 
     mutator_name = json_string(array->elems[1]);
     ovsdb_type_clone(&m->type, &m->column->type);
 
     mutator_name = json_string(array->elems[1]);

diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 0b1933b3598dd95c73d911f01d57d42b5a11af4c..478109af639d176167e7b5b2a67b24b7dbb4b51b 100755 (executable)
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -577,11 +577,16 @@ static void\n%s_columns_init(void)
         for columnName, column in sorted(table.columns.iteritems()):
             cs = "%s_col_%s" % (structName, columnName)
             d = {'cs': cs, 'c': columnName, 's': structName}
         for columnName, column in sorted(table.columns.iteritems()):
             cs = "%s_col_%s" % (structName, columnName)
             d = {'cs': cs, 'c': columnName, 's': structName}

+            if column.mutable:
+                mutable = "true"
+            else:
+                mutable = "false"

             print
             print "    /* Initialize %(cs)s. */" % d
             print "    c = &%(cs)s;" % d
             print "    c->name = \"%(c)s\";" % d
             print column.type.cInitType("    ", "c->type")
             print
             print "    /* Initialize %(cs)s. */" % d
             print "    c = &%(cs)s;" % d
             print "    c->name = \"%(c)s\";" % d
             print column.type.cInitType("    ", "c->type")

+            print "    c->mutable = %s;" % mutable

             print "    c->parse = %(s)s_parse_%(c)s;" % d
             print "    c->unparse = %(s)s_unparse_%(c)s;" % d
         print "}"
             print "    c->parse = %(s)s_parse_%(c)s;" % d
             print "    c->unparse = %(s)s_unparse_%(c)s;" % d
         print "}"

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 84cc272d706e918e315a494613054795e22a563f..e90361915a5ca4d2b3befc9f44301f78b67b3904 100644 (file)
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -736,6 +736,18 @@ AT_CHECK([RUN_OVS_VSCTL([clear netflow `cat netflow-uuid` targets])],
 AT_CHECK([RUN_OVS_VSCTL([destroy b br2])], 
   [1], [], [ovs-vsctl: no row "br2" in table Bridge
 ], [OVS_VSCTL_CLEANUP])
 AT_CHECK([RUN_OVS_VSCTL([destroy b br2])], 
   [1], [], [ovs-vsctl: no row "br2" in table Bridge
 ], [OVS_VSCTL_CLEANUP])

+AT_CHECK([RUN_OVS_VSCTL([add i br1 name x])],
+  [1], [], [ovs-vsctl: cannot modify read-only column name in table Interface
+], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([set port br1 name br2])],
+  [1], [], [ovs-vsctl: cannot modify read-only column name in table Port
+], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([remove b br1 name br1])],
+  [1], [], [ovs-vsctl: cannot modify read-only column name in table Bridge
+], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([clear b br1 name])],
+  [1], [], [ovs-vsctl: cannot modify read-only column name in table Bridge
+], [OVS_VSCTL_CLEANUP])

 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 

diff --git a/tests/ovsdb-execution.at b/tests/ovsdb-execution.at
index 480f61016ffa26a252b2c191b91edd8cf7a705d1..6a3b5d157a3cab1c3ac5351afec23914a01aaa8e 100644 (file)
--- a/tests/ovsdb-execution.at
+++ b/tests/ovsdb-execution.at
@@ -111,6 +111,15 @@ gc_schema () {
          "isRoot": false}}}
 EOF
 }
          "isRoot": false}}}
 EOF
 }

+
+immutable_schema () {
+    cat <<'EOF'
+{"name": "immutable",
+ "tables": {
+    "a": {
+        "columns": {"i": {"type": "integer", "mutable": false}}}}}
+EOF
+}

 ]
 m4_divert_pop([PREPARE_TESTS])
 
 ]
 m4_divert_pop([PREPARE_TESTS])
 


@@ -908,6 +917,40 @@ OVSDB_CHECK_EXECUTION([weak references],
 [{"rows":[{"_uuid":["uuid","<3>"],"b":2,"b2a":["set",[]]},{"_uuid":["uuid","<4>"],"b":3,"b2a":["set",[]]}]}]
 ]])
 
 [{"rows":[{"_uuid":["uuid","<3>"],"b":2,"b2a":["set",[]]},{"_uuid":["uuid","<4>"],"b":3,"b2a":["set",[]]}]}]
 ]])
 

+OVSDB_CHECK_EXECUTION([immutable columns],
+  [immutable_schema],
+  [[[["immutable",
+      {"op": "insert",
+       "table": "a",
+       "row": {"i": 5},
+       "uuid-name": "row1"}]]],
+   [[["immutable",
+      {"op": "update",
+       "table": "a",
+       "row": {"i": 10},
+       "where": []}]]],
+   [[["immutable",
+      {"op": "update",
+       "table": "a",
+       "row": {"i": 5},
+       "where": []}]]],
+   [[["immutable",
+      {"op": "mutate",
+       "table": "a",
+       "where": [],
+       "mutations": [["i", "-=", 5]]}]]],
+   [[["immutable",
+      {"op": "mutate",
+       "table": "a",
+       "where": [],
+       "mutations": [["i", "*=", 1]]}]]]],
+  [[[{"uuid":["uuid","<0>"]}]
+[{"details":"Cannot update immutable column i in table a.","error":"constraint violation","syntax":"{\"op\":\"update\",\"row\":{\"i\":10},\"table\":\"a\",\"where\":[]}"}]
+[{"details":"Cannot update immutable column i in table a.","error":"constraint violation","syntax":"{\"op\":\"update\",\"row\":{\"i\":5},\"table\":\"a\",\"where\":[]}"}]
+[{"details":"Cannot mutate immutable column i in table a.","error":"constraint violation","syntax":"[\"i\",\"-=\",5]"}]
+[{"details":"Cannot mutate immutable column i in table a.","error":"constraint violation","syntax":"[\"i\",\"*=\",1]"}]
+]])
+

 OVSDB_CHECK_EXECUTION([garbage collection],
   [gc_schema],
   [dnl Check that inserting a row without any references is a no-op.
 OVSDB_CHECK_EXECUTION([garbage collection],
   [gc_schema],
   [dnl Check that inserting a row without any references is a no-op.

diff --git a/tests/ovsdb-mutation.at b/tests/ovsdb-mutation.at
index 3d753176ddb152cca674d79a3dfd717513ce1056..fc898b56d92ac8f76d0b8667d3de7c392c3bff07 100644 (file)
--- a/tests/ovsdb-mutation.at
+++ b/tests/ovsdb-mutation.at
@@ -99,6 +99,18 @@ test-ovsdb: syntax "["u","delete",["uuid","9179ca6d-6d65-400a-b455-3ad92783a099"
 ]])
 AT_CLEANUP
 
 ]])
 AT_CLEANUP
 

+AT_SETUP([disallowed mutations on immutable columns])
+AT_KEYWORDS([ovsdb negative mutation])
+AT_CHECK([[test-ovsdb parse-mutations \
+    '{"columns":
+        {"i": {"type": "integer", "mutable": false}}}' \
+    '[["i", "+=", 1]]'
+]],
+  [1], [],
+  [[test-ovsdb: syntax "["i","+=",1]": constraint violation: Cannot mutate immutable column i in table mytable.
+]])
+AT_CLEANUP
+

 OVSDB_CHECK_POSITIVE([mutations on sets],
   [[parse-mutations \
     '{"columns":
 OVSDB_CHECK_POSITIVE([mutations on sets],
   [[parse-mutations \
     '{"columns":

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 0a7c2c64ef4e20bc8adecfc4172d4dcc682325d4..e6bd63bd285ac833767811fd705c8a07d3c82416 100644 (file)
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -2763,7 +2763,7 @@ error:
     return error;
 }
 
     return error;
 }
 

-static void
+static const struct ovsdb_idl_column *

 pre_parse_column_key_value(struct vsctl_context *ctx,
                            const char *arg,
                            const struct vsctl_table_class *table)
 pre_parse_column_key_value(struct vsctl_context *ctx,
                            const char *arg,
                            const struct vsctl_table_class *table)


@@ -2780,6 +2780,18 @@ pre_parse_column_key_value(struct vsctl_context *ctx,
 
     pre_get_column(ctx, table, column_name, &column);
     free(column_name);
 
     pre_get_column(ctx, table, column_name, &column);
     free(column_name);

+
+    return column;
+}
+
+static void
+check_mutable(const struct vsctl_table_class *table,
+              const struct ovsdb_idl_column *column)
+{
+    if (!column->mutable) {
+        vsctl_fatal("cannot modify read-only column %s in table %s",
+                    column->name, table->class->name);
+    }

 }
 
 static void
 }
 
 static void


@@ -3112,7 +3124,10 @@ pre_cmd_set(struct vsctl_context *ctx)
 
     table = pre_get_table(ctx, table_name);
     for (i = 3; i < ctx->argc; i++) {
 
     table = pre_get_table(ctx, table_name);
     for (i = 3; i < ctx->argc; i++) {

-        pre_parse_column_key_value(ctx, ctx->argv[i], table);
+        const struct ovsdb_idl_column *column;
+
+        column = pre_parse_column_key_value(ctx, ctx->argv[i], table);
+        check_mutable(table, column);

     }
 }
 
     }
 }
 


@@ -3195,6 +3210,7 @@ pre_cmd_add(struct vsctl_context *ctx)
 
     table = pre_get_table(ctx, table_name);
     pre_get_column(ctx, table, column_name, &column);
 
     table = pre_get_table(ctx, table_name);
     pre_get_column(ctx, table, column_name, &column);

+    check_mutable(table, column);

 }
 
 static void
 }
 
 static void


@@ -3251,6 +3267,7 @@ pre_cmd_remove(struct vsctl_context *ctx)
 
     table = pre_get_table(ctx, table_name);
     pre_get_column(ctx, table, column_name, &column);
 
     table = pre_get_table(ctx, table_name);
     pre_get_column(ctx, table, column_name, &column);

+    check_mutable(table, column);

 }
 
 static void
 }
 
 static void


@@ -3316,6 +3333,7 @@ pre_cmd_clear(struct vsctl_context *ctx)
         const struct ovsdb_idl_column *column;
 
         pre_get_column(ctx, table, ctx->argv[i], &column);
         const struct ovsdb_idl_column *column;
 
         pre_get_column(ctx, table, ctx->argv[i], &column);

+        check_mutable(table, column);

     }
 }
 
     }
 }