On Thu, Feb 06, 2014 at 05:24:11PM +0100, Santi Raffa wrote:
> This commit adds the private containers to Python and Haskell.
>
> Signed-off-by: Santi Raffa <[email protected]>
> ---
> lib/cli.py | 12 ++++
> lib/config.py | 5 +-
> lib/ht.py | 19 ++++-
> lib/serializer.py | 155
> +++++++++++++++++++++++++++++++++++++++++
> src/Ganeti/JSON.hs | 1 +
> src/Ganeti/Types.hs | 39 ++++++++++-
> test/hs/Test/Ganeti/Objects.hs | 8 ++-
> test/hs/Test/Ganeti/OpCodes.hs | 10 +++
> 8 files changed, 244 insertions(+), 5 deletions(-)
>
> diff --git a/lib/cli.py b/lib/cli.py
> index cb28c09..3bdd2e2 100644
> --- a/lib/cli.py
> +++ b/lib/cli.py
> @@ -44,6 +44,7 @@ from ganeti import netutils
> from ganeti import qlang
> from ganeti import objects
> from ganeti import pathutils
> +from ganeti import serializer
>
> from ganeti.runtime import (GetClient)
>
> @@ -679,6 +680,15 @@ def check_key_val(option, opt, value): # pylint:
> disable=W0613
> return _SplitKeyVal(opt, value, True)
>
>
> +def check_key_private_val(option, opt, value): # pylint: disable=W0613
> + """Custom parser class for private and secret key=val,key=val options.
> +
> + This will store the parsed values as a dict {key: val}.
> +
> + """
> + return serializer.PrivateDict(_SplitKeyVal(opt, value, True))
> +
> +
> def _SplitListKeyVal(opt, value):
> retval = {}
> for elem in value.split("/"):
> @@ -781,6 +791,7 @@ class CliOption(Option):
> "multilistidentkeyval",
> "identkeyval",
> "keyval",
> + "keyprivateval",
> "unit",
> "bool",
> "list",
> @@ -790,6 +801,7 @@ class CliOption(Option):
> TYPE_CHECKER["multilistidentkeyval"] = check_multilist_ident_key_val
> TYPE_CHECKER["identkeyval"] = check_ident_key_val
> TYPE_CHECKER["keyval"] = check_key_val
> + TYPE_CHECKER["keyprivateval"] = check_key_private_val
> TYPE_CHECKER["unit"] = check_unit
> TYPE_CHECKER["bool"] = check_bool
> TYPE_CHECKER["list"] = check_list
> diff --git a/lib/config.py b/lib/config.py
> index fcdefc3..4e146c4 100644
> --- a/lib/config.py
> +++ b/lib/config.py
> @@ -2488,7 +2488,10 @@ class ConfigWriter(object):
> if destination is None:
> destination = self._cfg_file
> self._BumpSerialNo()
> - txt = serializer.Dump(self._config_data.ToDict())
> + txt = serializer.DumpJson(
> + self._config_data.ToDict(_with_private=True),
> + private_encoder=serializer.EncodeWithPrivateFields
> + )
>
> getents = self._getents()
> try:
> diff --git a/lib/ht.py b/lib/ht.py
> index 500c391..c7688ba 100644
> --- a/lib/ht.py
> +++ b/lib/ht.py
> @@ -29,7 +29,7 @@ from ganeti import compat
> from ganeti import utils
> from ganeti import constants
> from ganeti import objects
> -
> +from ganeti.serializer import Private
>
> _PAREN_RE = re.compile("^[a-zA-Z0-9_-]+$")
>
> @@ -77,6 +77,9 @@ class _DescWrapper(_WrapperBase):
> def __str__(self):
> return self._text
>
> + def __repr__(self):
> + return "<%s %r>" % (self._text, self._fn)
> +
>
> class _CommentWrapper(_WrapperBase):
> """Wrapper class for comment.
> @@ -269,6 +272,8 @@ def TTuple(val):
> def TDict(val):
> """Checks if the given value is a dictionary.
>
> + Note that L{PrivateDict}s subclass dict and pass this check.
> +
> """
> return isinstance(val, dict)
>
> @@ -416,6 +421,18 @@ def TInstanceOf(cls):
> return desc(lambda val: isinstance(val, cls))
>
>
> +def TPrivate(val_type):
> + """Checks if a given value is an instance of Private.
> +
> + """
> + def fn(val):
> + return isinstance(val, Private) and val_type(val.Get())
> +
> + desc = WithDesc("Private %s" % Parens(val_type))
> +
> + return desc(fn)
> +
> +
> def TListOf(my_type):
> """Checks if a given value is a list with all elements of the same type.
>
> diff --git a/lib/serializer.py b/lib/serializer.py
> index 87217f3..1b3be77 100644
> --- a/lib/serializer.py
> +++ b/lib/serializer.py
> @@ -166,3 +166,158 @@ Dump = DumpJson
> Load = LoadJson
> DumpSigned = DumpSignedJson
> LoadSigned = LoadSignedJson
> +
> +
> +class Private(object):
> + """Wrap a value so it is hard to leak it accidentally.
> +
> + >>> x = Private("foo")
> + >>> print "Value: %s" % x
> + Value: <redacted>
> + >>> print "Value: {0}".format(x)
> + Value: <redacted>
> + >>> x.upper() == "FOO"
> + True
Indentation should be on the first column, as discussed offline.
> +
> + """
> +
Unnecessary newline.
> + def __init__(self, item, descr="redacted"):
> + if isinstance(item, Private):
> + raise ValueError("Attempted to nest Private values.")
> + self._item = item
> + self._descr = descr
> +
> + def Get(self):
> + "Return the wrapped value."
> + return self._item
> +
> + def __str__(self):
> + return "<{._descr}>".format(self)
> +
> + def __repr__(self):
> + return "Private(?, descr='{._descr}')".format(self)
> +
> + # pylint: disable=W0212
> + # If it doesn't access _item directly, the call will go through __getattr__
> + # because this class defines __slots__ and "item" is not in it.
> + # OTOH, if we do add it there, we'd risk shadowing an "item" attribute.
> + def __eq__(self, other):
> + if isinstance(other, Private):
> + return self._item == other._item
> + else:
> + return self._item == other
> +
> + def __hash__(self):
> + return hash(self._item)
> +
> + def __format__(self, *_1, **_2):
> + return self.__str__()
> +
> + def __getattr__(self, attr):
> + return Private(getattr(self._item, attr),
> + descr="%s.%s" % (self._descr, attr))
> +
> + def __call__(self, *args, **kwargs):
> + return Private(self._item(*args, **kwargs),
> + descr="%s()" % self._descr)
> +
> + # pylint: disable=R0201
> + # While this could get away with being a function, it needs to be a method.
> + # Required by the copy.deepcopy function used by FillDict.
> + def __getnewargs__(self):
> + return tuple()
> +
> + def __nonzero__(self):
> + return bool(self._item)
> +
> + # Get in the way of Pickle by implementing __slots__ but not __getstate__
> + # ...and get a performance boost, too.
> + __slots__ = ["_item", "_descr"]
> +
> +
> +class PrivateDict(dict):
> + """A dictionary that turns its values to private fields.
> +
> + >>> PrivateDict()
> + {}
> + >>> supersekkrit = PrivateDict({"password": "foobar"})
> + >>> print supersekkrit["password"]
> + <password>
> + >>> supersekkrit["password"].Get()
> + 'foobar'
> + >>> supersekkrit.GetPrivate("password")
> + 'foobar'
> + >>> supersekkrit["user"] = "eggspam"
> + >>> supersekkrit.Unprivate()
> + {'password': 'foobar', 'user': 'eggspam'}
> +
Indentation as discussed offline.
> + """
> +
Unnecessary newline.
> + def __init__(self, data=None):
> + dict.__init__(self)
> + self.update(data)
> +
> + def __setitem__(self, item, value):
> + if not isinstance(value, Private):
> + if not isinstance(item, dict):
> + value = Private(value, descr=item)
> + else:
> + value = PrivateDict(value)
> + dict.__setitem__(self, item, value)
> +
> + # The actual conversion to Private containers is done by __setitem__
> +
> + # copied straight from cpython/Lib/UserDict.py
> + # Copyright (c) 2001-2014 Python Software Foundation; All Rights Reserved
> + def update(self, other=None, **kwargs):
> + # Make progressively weaker assumptions about "other"
> + if other is None:
> + pass
> + elif hasattr(other, 'iteritems'): # iteritems saves memory and lookups
> + for k, v in other.iteritems():
> + self[k] = v
> + elif hasattr(other, 'keys'):
> + for k in other.keys():
> + self[k] = other[k]
> + else:
> + for k, v in other:
> + self[k] = v
> + if kwargs:
> + self.update(kwargs)
> +
> + def GetPrivate(self, *args):
> + """Like dict.get, but extracting the value in the process.
> +
> + Arguments are semantically equivalent to ``dict.get``
> +
> + >>> PrivateDict({"foo": "bar"}).GetPrivate("foo")
> + 'bar'
> + >>> PrivateDict({"foo": "bar"}).GetPrivate("baz", "spam")
> + 'spam'
> +
Indentation.
Rest LGTM. No need to resend.
Thanks,
Jose
> + """
> + if len(args) == 1:
> + key, = args
> + return self[key].Get()
> + elif len(args) == 2:
> + key, default = args
> + if key not in self:
> + return default
> + else:
> + return self[key].Get()
> + else:
> + raise TypeError("GetPrivate() takes 2 arguments (%d given)" %
> len(args))
> +
> + def Unprivate(self):
> + """Turn this dict of Private() values to a dict of values.
> +
> + >>> PrivateDict({"foo": "bar"}).Unprivate()
> + {'foo': 'bar'}
> +
> + @rtype: dict
> +
> + """
> + returndict = {}
> + for key in self:
> + returndict[key] = self[key].Get()
> + return returndict
> diff --git a/src/Ganeti/JSON.hs b/src/Ganeti/JSON.hs
> index c1daa5a..fe84d8a 100644
> --- a/src/Ganeti/JSON.hs
> +++ b/src/Ganeti/JSON.hs
> @@ -48,6 +48,7 @@ module Ganeti.JSON
> , toArray
> , optionalJSField
> , optFieldsToObj
> + , readContainer
> , HasStringRepr(..)
> , GenericContainer(..)
> , Container
> diff --git a/src/Ganeti/Types.hs b/src/Ganeti/Types.hs
> index f3bd4af..5b6bcb7 100644
> --- a/src/Ganeti/Types.hs
> +++ b/src/Ganeti/Types.hs
> @@ -159,6 +159,8 @@ module Ganeti.Types
> , hotplugTargetToRaw
> , HotplugAction(..)
> , hotplugActionToRaw
> + , Private(..)
> + , showPrivateJSObject
> ) where
>
> import Control.Monad (liftM)
> @@ -689,7 +691,7 @@ instance JSON.JSON JobIdDep where
> absoluteJobIdDep :: (Monad m) => JobIdDep -> JobId -> m JobIdDep
> absoluteJobIdDep (JobDepAbsolute jid) _ = return $ JobDepAbsolute jid
> absoluteJobIdDep (JobDepRelative rjid) jid =
> - liftM JobDepAbsolute . makeJobId $ fromJobId jid + fromNegative rjid
> + liftM JobDepAbsolute . makeJobId $ fromJobId jid + fromNegative rjid
>
> -- | Job Dependency type.
> data JobDependency = JobDependency JobIdDep [FinalizedJobStatus]
> @@ -702,7 +704,7 @@ instance JSON JobDependency where
> -- | From job dependency and job id compute an absolute job dependency.
> absoluteJobDependency :: (Monad m) => JobDependency -> JobId -> m
> JobDependency
> absoluteJobDependency (JobDependency jdep fstats) jid =
> - liftM (flip JobDependency fstats) $ absoluteJobIdDep jdep jid
> + liftM (flip JobDependency fstats) $ absoluteJobIdDep jdep jid
>
> -- | Valid opcode priorities for submit.
> $(THH.declareIADT "OpSubmitPriority"
> @@ -889,3 +891,36 @@ $(THH.declareLADT ''String "HotplugTarget"
> , ("HTNic", "hotnic")
> ])
> $(THH.makeJSONInstance ''HotplugTarget)
> +
> +-- * Private type and instances
> +
> +-- | A container for values that should be happy to be manipulated yet
> +-- refuses to be shown unless explicitly requested.
> +newtype Private a = Private { getPrivate :: a }
> + deriving Eq
> +
> +instance (Show a, JSON.JSON a) => JSON.JSON (Private a) where
> + readJSON = liftM Private . JSON.readJSON
> + showJSON (Private x) = JSON.showJSON x
> +
> +-- | "Show" the value of the field.
> +--
> +-- It would be better not to implement this at all.
> +-- Alas, Show OpCode requires Show Private.
> +instance Show a => Show (Private a) where
> + show _ = "<redacted>"
> +
> +instance THH.PyValue a => THH.PyValue (Private a) where
> + showValue (Private x) = "Private(" ++ THH.showValue x ++ ")"
> +
> +instance Functor Private where
> + fmap f (Private x) = Private $ f x
> +
> +instance Monad Private where
> + (Private x) >>= f = f x
> + return = Private
> +
> +showPrivateJSObject :: (JSON.JSON a) =>
> + [(String, a)] -> JSON.JSObject (Private JSON.JSValue)
> +showPrivateJSObject value = JSON.toJSObject $ map f value
> + where f (k, v) = (k, Private $ JSON.showJSON v)
> diff --git a/test/hs/Test/Ganeti/Objects.hs b/test/hs/Test/Ganeti/Objects.hs
> index 96ba802..a07a95b 100644
> --- a/test/hs/Test/Ganeti/Objects.hs
> +++ b/test/hs/Test/Ganeti/Objects.hs
> @@ -219,6 +219,12 @@ instance Arbitrary ClusterNicParams where
> instance Arbitrary OsParams where
> arbitrary = (GenericContainer . Map.fromList) <$> arbitrary
>
> +instance Arbitrary Objects.ClusterOsParamsPrivate where
> + arbitrary = (GenericContainer . Map.fromList) <$> arbitrary
> +
> +instance Arbitrary a => Arbitrary (Private a) where
> + arbitrary = Private <$> arbitrary
> +
> instance Arbitrary ClusterOsParams where
> arbitrary = (GenericContainer . Map.fromList) <$> arbitrary
>
> @@ -552,7 +558,7 @@ caseIncludeLogicalIdDrbd :: HUnit.Assertion
> caseIncludeLogicalIdDrbd =
> let vg_name = "xenvg" :: String
> lv_name = "1234sdf-qwef-2134-asff-asd2-23145d.data" :: String
> - d =
> + d =
> Disk
> (LIDDrbd8 "node1.example.com" "node2.example.com" 2000 1 5
> "secret")
> [ Disk (LIDPlain "onevg" "onelv") [] "disk1" 1000 DiskRdWr Nothing
> diff --git a/test/hs/Test/Ganeti/OpCodes.hs b/test/hs/Test/Ganeti/OpCodes.hs
> index 861e72d..4a502f1 100644
> --- a/test/hs/Test/Ganeti/OpCodes.hs
> +++ b/test/hs/Test/Ganeti/OpCodes.hs
> @@ -431,6 +431,16 @@ genMacPrefix = do
> octets <- vectorOf 3 $ choose (0::Int, 255)
> mkNonEmpty . intercalate ":" $ map (printf "%02x") octets
>
> +-- | JSObject of arbitrary data.
> +--
> +-- Since JSValue does not implement Arbitrary, I'll simply generate
> +-- (String, String) objects.
> +arbitraryPrivateJSObj :: Gen (J.JSObject (Private J.JSValue))
> +arbitraryPrivateJSObj =
> + constructor <$> (fromNonEmpty <$> genNameNE)
> + <*> (fromNonEmpty <$> genNameNE)
> + where constructor k v = showPrivateJSObject [(k, v)]
> +
> -- | Arbitrary instance for MetaOpCode, defined here due to TH ordering.
> $(genArbitrary ''OpCodes.MetaOpCode)
>
> --
> 1.9.0.rc1.175.g0b1dcb5
>
--
Jose Antonio Lopes
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
