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 872f8d0..cbc5c58 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.)
+    - 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

diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h
index 546acbb..7ea5ce6 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.
@@ -41,6 +41,7 @@ struct ovsdb_idl_row {
 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 *);
 };

diff --git a/ovsdb/execution.c b/ovsdb/execution.c
index 1aff0c5..300c247 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.
@@ -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) {
+        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);

diff --git a/ovsdb/mutation.c b/ovsdb/mutation.c
index 0dcd16f..5fd983a 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.
@@ -95,6 +95,12 @@ ovsdb_mutation_from_json(const struct ovsdb_table_schema *ts,
                                   "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]);

diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 0b1933b..478109a 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}
+            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 "    c->mutable = %s;" % mutable
             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 84cc272..e903619 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([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
 

diff --git a/tests/ovsdb-execution.at b/tests/ovsdb-execution.at
index 480f610..6a3b5d1 100644 (file)
--- a/tests/ovsdb-execution.at
+++ b/tests/ovsdb-execution.at
@@ -111,6 +111,15 @@ gc_schema () {
          "isRoot": false}}}
 EOF
 }
+
+immutable_schema () {
+    cat <<'EOF'
+{"name": "immutable",
+ "tables": {
+    "a": {
+        "columns": {"i": {"type": "integer", "mutable": false}}}}}
+EOF
+}
 ]
 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",[]]}]}]
 ]])
 
+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.

diff --git a/tests/ovsdb-mutation.at b/tests/ovsdb-mutation.at
index 3d75317..fc898b5 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_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":

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 0a7c2c6..e6bd63b 100644 (file)
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -2763,7 +2763,7 @@ 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)
@@ -2780,6 +2780,18 @@ pre_parse_column_key_value(struct vsctl_context *ctx,
 
     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
@@ -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++) {
-        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);
+    check_mutable(table, column);
 }
 
 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);
+    check_mutable(table, column);
 }
 
 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);
+        check_mutable(table, column);
     }
 }