Merge ~pappacena/launchpad:ocirecipe-args into launchpad:master

[Thiago F. Pappacena]

Adding OCIRecipe.build_args to allow ARG (--build-arg) options when building OCI images

[Thiago F. Pappacena (pappacena)]

db patch here: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/389748

7e10963... by Thiago F. Pappacena on 2020-08-27

Changing OCIRecipe.build_args datatype to jsonb

9f1937d... by Thiago F. Pappacena on 2020-08-27

Merge branch 'master' into ocirecipe-args

[Colin Watson (cjwatson)]

review: Approve

3680244... by Thiago F. Pappacena on 2020-08-28

py3 compat and better IOCIRecipe.build_args definition

[Thiago F. Pappacena (pappacena)]

Pushed the requested changes.

1d8dfd8... by Thiago F. Pappacena on 2020-09-30

Merge branch 'master' into ocirecipe-args

diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
index e185ea5..718cdd3 100644
--- a/lib/lp/oci/interfaces/ocirecipe.py
+++ b/lib/lp/oci/interfaces/ocirecipe.py
@@ -388,6 +388,15 @@ class IOCIRecipeEditableAttributes(IHasOwner):
         required=True,
         readonly=False))
 
+    build_args = exported(Dict(
+        title=_("Build ARG variables"),
+        description=_("The dictionary of ARG variables to be used when "
+                      "building this recipe."),
+        key_type=TextLine(title=_("ARG name")),
+        value_type=TextLine(title=_("ARG value")),
+        required=False,
+        readonly=False))
+
     build_path = exported(TextLine(
         title=_("Build directory context"),
         description=_("Directory to use for build context "
@@ -442,7 +451,7 @@ class IOCIRecipeSet(Interface):
     def new(name, registrant, owner, oci_project, git_ref, build_file,
             description=None, official=False, require_virtualized=True,
             build_daily=False, processors=None, date_created=DEFAULT,
-            allow_internet=True):
+            allow_internet=True, build_args=None):
         """Create an IOCIRecipe."""
 
     def exists(owner, oci_project, name):
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index ea03bf4..3ca817c 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -14,6 +14,8 @@ __all__ = [
 
 from lazr.lifecycle.event import ObjectCreatedEvent
 import pytz
+import six
+from storm.databases.postgres import JSON
 from storm.expr import (
     And,
     Desc,
@@ -146,6 +148,7 @@ class OCIRecipe(Storm, WebhookTargetMixin):
     git_path = Unicode(name="git_path", allow_none=True)
     build_file = Unicode(name="build_file", allow_none=False)
     build_path = Unicode(name="build_path", allow_none=False)
+    _build_args = JSON(name="build_args", allow_none=True)
 
     require_virtualized = Bool(name="require_virtualized", default=True,
                                allow_none=False)
@@ -157,7 +160,7 @@ class OCIRecipe(Storm, WebhookTargetMixin):
     def __init__(self, name, registrant, owner, oci_project, git_ref,
                  description=None, official=False, require_virtualized=True,
                  build_file=None, build_daily=False, date_created=DEFAULT,
-                 allow_internet=True, build_path=None):
+                 allow_internet=True, build_args=None, build_path=None):
         if not getFeatureFlag(OCI_RECIPE_ALLOW_CREATE):
             raise OCIRecipeFeatureDisabled()
         super(OCIRecipe, self).__init__()
@@ -174,6 +177,7 @@ class OCIRecipe(Storm, WebhookTargetMixin):
         self.date_last_modified = date_created
         self.git_ref = git_ref
         self.allow_internet = allow_internet
+        self.build_args = build_args or {}
         self.build_path = build_path
 
     def __repr__(self):
@@ -190,6 +194,16 @@ class OCIRecipe(Storm, WebhookTargetMixin):
         """See `IOCIProject.setOfficialRecipe` method."""
         return self._official
 
+    @property
+    def build_args(self):
+        return self._build_args or {}
+
+    @build_args.setter
+    def build_args(self, value):
+        assert value is None or isinstance(value, dict)
+        self._build_args = {k: six.text_type(v)
+                            for k, v in (value or {}).items()}
+
     def destroySelf(self):
         """See `IOCIRecipe`."""
         # XXX twom 2019-11-26 This needs to expand as more build artifacts
@@ -540,7 +554,7 @@ class OCIRecipeSet:
     def new(self, name, registrant, owner, oci_project, git_ref, build_file,
             description=None, official=False, require_virtualized=True,
             build_daily=False, processors=None, date_created=DEFAULT,
-            allow_internet=True, build_path=None):
+            allow_internet=True, build_args=None, build_path=None):
         """See `IOCIRecipeSet`."""
         if not registrant.inTeam(owner):
             if owner.is_team:
@@ -565,7 +579,7 @@ class OCIRecipeSet:
         oci_recipe = OCIRecipe(
             name, registrant, owner, oci_project, git_ref, description,
             official, require_virtualized, build_file, build_daily,
-            date_created, allow_internet, build_path)
+            date_created, allow_internet, build_args, build_path)
         store.add(oci_recipe)
 
         if processors is None:
diff --git a/lib/lp/oci/model/ocirecipebuildbehaviour.py b/lib/lp/oci/model/ocirecipebuildbehaviour.py
index 844a2c8..742c8a2 100644
--- a/lib/lp/oci/model/ocirecipebuildbehaviour.py
+++ b/lib/lp/oci/model/ocirecipebuildbehaviour.py
@@ -1,4 +1,4 @@
-# Copyright 2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2019-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """An `IBuildFarmJobBehaviour` for `OCIRecipeBuild`.
@@ -96,6 +96,7 @@ class OCIRecipeBuildBehaviour(SnapProxyMixin, BuildFarmJobBehaviourBase):
                 logger=logger))
 
         args['build_file'] = build.recipe.build_file
+        args['build_args'] = build.recipe.build_args
         args['build_path'] = build.recipe.build_path
 
         if build.recipe.git_ref is not None:
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index 30c547f..76997a7 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -62,6 +62,7 @@ from lp.services.database.constants import (
     ONE_DAY_AGO,
     UTC_NOW,
     )
+from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import flush_database_caches
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.runner import JobRunner
@@ -700,6 +701,43 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
             [recipe3, recipe1, recipe2],
             list(oci_project.searchRecipes(u"a")))
 
+    def test_build_args_dict(self):
+        args = {"MY_VERSION": "1.0.3", "ANOTHER_VERSION": "2.9.88"}
+        recipe = self.factory.makeOCIRecipe(build_args=args)
+        # Force fetch it from database
+        store = IStore(recipe)
+        store.invalidate(recipe)
+        self.assertEqual(args, recipe.build_args)
+
+    def test_build_args_not_dict(self):
+        invalid_build_args_set = [
+            [1, 2, 3],
+            "some string",
+            123,
+        ]
+        for invalid_build_args in invalid_build_args_set:
+            self.assertRaises(
+                AssertionError, self.factory.makeOCIRecipe,
+                build_args=invalid_build_args)
+
+    def test_build_args_flatten_dict(self):
+        # Makes sure we only store one level of key=pair, flattening to
+        # string every value.
+        args = {
+            "VAR1": {b"something": [1, 2, 3]},
+            "VAR2": 123,
+            "VAR3": "A string",
+        }
+        recipe = self.factory.makeOCIRecipe(build_args=args)
+        # Force fetch it from database
+        store = IStore(recipe)
+        store.invalidate(recipe)
+        self.assertEqual({
+            "VAR1": "{'something': [1, 2, 3]}",
+            "VAR2": "123",
+            "VAR3": "A string",
+        }, recipe.build_args)
+
 
 class TestOCIRecipeProcessors(TestCaseWithFactory):
 
@@ -1024,7 +1062,8 @@ class TestOCIRecipeWebservice(OCIConfigHelperMixin, TestCaseWithFactory):
             oci_project = self.factory.makeOCIProject(
                 registrant=self.person)
             recipe = self.factory.makeOCIRecipe(
-                oci_project=oci_project)
+                oci_project=oci_project,
+                build_args={"VAR_A": "123"})
             url = api_url(recipe)
 
         ws_recipe = self.load_from_api(url)
@@ -1043,8 +1082,9 @@ class TestOCIRecipeWebservice(OCIConfigHelperMixin, TestCaseWithFactory):
                 git_ref_link=Equals(self.getAbsoluteURL(recipe.git_ref)),
                 description=Equals(recipe.description),
                 build_file=Equals(recipe.build_file),
+                build_args=Equals({"VAR_A": "123"}),
                 build_daily=Equals(recipe.build_daily),
-                build_path=Equals(recipe.build_path)
+                build_path=Equals(recipe.build_path),
                 )))
 
     def test_api_patch_oci_recipe(self):
@@ -1108,6 +1148,7 @@ class TestOCIRecipeWebservice(OCIConfigHelperMixin, TestCaseWithFactory):
             "owner": person_url,
             "git_ref": git_ref_url,
             "build_file": "./Dockerfile",
+            "build_args": {"VAR": "VAR VALUE"},
             "description": "My recipe"}
 
         resp = self.webservice.named_post(oci_project_url, "newRecipe", **obj)
@@ -1125,6 +1166,7 @@ class TestOCIRecipeWebservice(OCIConfigHelperMixin, TestCaseWithFactory):
                 description=Equals(obj["description"]),
                 owner_link=Equals(self.getAbsoluteURL(self.person)),
                 registrant_link=Equals(self.getAbsoluteURL(self.person)),
+                build_args=Equals({"VAR": "VAR VALUE"})
             )))
 
     def test_api_create_oci_recipe_non_legitimate_user(self):
diff --git a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
index bbc1ee6..6397405 100644
--- a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
+++ b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
@@ -111,6 +111,7 @@ class MakeOCIBuildMixin:
                 build = self.factory.makeOCIRecipeBuild(
                     recipe=recipe, **kwargs)
         build.recipe.git_ref = git_ref
+        build.recipe.build_args = {"BUILD_VAR": "123"}
 
         job = IBuildFarmJobBehaviour(build)
         builder = MockBuilder()
@@ -242,6 +243,7 @@ class TestAsyncOCIRecipeBuildBehaviour(MakeOCIBuildMixin, TestCaseWithFactory):
             "archives": Equals(expected_archives),
             "arch_tag": Equals("i386"),
             "build_file": Equals(job.build.recipe.build_file),
+            "build_args": Equals({"BUILD_VAR": "123"}),
             "build_path": Equals(job.build.recipe.build_path),
             "build_url": Equals(canonical_url(job.build)),
             "fast_cleanup": Is(True),
@@ -273,6 +275,7 @@ class TestAsyncOCIRecipeBuildBehaviour(MakeOCIBuildMixin, TestCaseWithFactory):
             "archives": Equals(expected_archives),
             "arch_tag": Equals("i386"),
             "build_file": Equals(job.build.recipe.build_file),
+            "build_args": Equals({"BUILD_VAR": "123"}),
             "build_path": Equals(job.build.recipe.build_path),
             "build_url": Equals(canonical_url(job.build)),
             "fast_cleanup": Is(True),
diff --git a/lib/lp/registry/interfaces/ociproject.py b/lib/lp/registry/interfaces/ociproject.py
index 2454662..e72a4cf 100644
--- a/lib/lp/registry/interfaces/ociproject.py
+++ b/lib/lp/registry/interfaces/ociproject.py
@@ -33,6 +33,7 @@ from zope.interface import Interface
 from zope.schema import (
     Bool,
     Datetime,
+    Dict,
     Int,
     Text,
     TextLine,
@@ -164,6 +165,13 @@ class IOCIProjectLegitimate(Interface):
                 "branch that defines how to build the recipe."),
             constraint=path_does_not_escape,
             required=True),
+        build_args=Dict(
+            title=_("Build ARGs to be used when building the recipe"),
+            description=_(
+                "A dict of VARIABLE=VALUE to be used as ARG when building "
+                "the recipe."
+            ),
+            required=False),
         description=Text(
             title=_("Description for this recipe."),
             description=_("A short description of this recipe."),
@@ -174,7 +182,7 @@ class IOCIProjectLegitimate(Interface):
     @operation_for_version("devel")
     def newRecipe(name, registrant, owner, git_ref, build_file,
                   description=None, build_daily=False,
-                  require_virtualized=True):
+                  require_virtualized=True, build_args=None):
         """Create an IOCIRecipe for this project."""
 
 
diff --git a/lib/lp/registry/model/ociproject.py b/lib/lp/registry/model/ociproject.py
index 28b3dc0..c27f2c6 100644
--- a/lib/lp/registry/model/ociproject.py
+++ b/lib/lp/registry/model/ociproject.py
@@ -130,7 +130,7 @@ class OCIProject(BugTargetBase, StormBase):
 
     def newRecipe(self, name, registrant, owner, git_ref,
                   build_file, description=None, build_daily=False,
-                  require_virtualized=True):
+                  require_virtualized=True, build_args=None):
         return getUtility(IOCIRecipeSet).new(
             name=name,
             registrant=registrant,
@@ -138,6 +138,7 @@ class OCIProject(BugTargetBase, StormBase):
             oci_project=self,
             git_ref=git_ref,
             build_file=build_file,
+            build_args=build_args,
             description=description,
             require_virtualized=require_virtualized,
             build_daily=build_daily,
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index d469c5a..57babc5 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -4961,7 +4961,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
                       oci_project=None, git_ref=None, description=None,
                       official=False, require_virtualized=True,
                       build_file=None, date_created=DEFAULT,
-                      allow_internet=True, build_path=None):
+                      allow_internet=True, build_args=None, build_path=None):
         """Make a new OCIRecipe."""
         if name is None:
             name = self.getUniqueString(u"oci-recipe-name")
@@ -4991,7 +4991,8 @@ class BareLaunchpadObjectFactory(ObjectFactory):
             official=official,
             require_virtualized=require_virtualized,
             date_created=date_created,
-            allow_internet=allow_internet)
+            allow_internet=allow_internet,
+            build_args=build_args)
 
     def makeOCIRecipeArch(self, recipe=None, processor=None):
         """Make a new OCIRecipeArch."""