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

[Thiago F. Pappacena]

UI to manage build ARGs for OCI recipes

6919ec2... by Thiago F. Pappacena on 2020-08-27

Merge branch 'ocirecipe-args' into ui-ocirecipe-args

[Thiago F. Pappacena (pappacena)]

I forgot to add the screenshots for this. I'll do it in some minutes.

[Colin Watson (cjwatson)]

review: Approve

7c3074c... by Thiago F. Pappacena on 2020-08-28

Merge branch 'ocirecipe-args' into ui-ocirecipe-args

0c1d56d... by Thiago F. Pappacena on 2020-08-28

Small adjustments on build ARG presentation

2fb9015... by Thiago F. Pappacena on 2020-08-28

Fixing tag placement

04c9ed5... by Thiago F. Pappacena on 2020-09-30

Merge branch 'ocirecipe-args' into ui-ocirecipe-args

diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
index 5e6a50f..cf11bfb 100644
--- a/lib/lp/oci/browser/ocirecipe.py
+++ b/lib/lp/oci/browser/ocirecipe.py
@@ -23,9 +23,12 @@ from lazr.restful.interface import (
     copy_field,
     use_template,
     )
+import six
 from zope.component import getUtility
 from zope.formlib.form import FormFields
+from zope.formlib.textwidgets import TextAreaWidget
 from zope.formlib.widget import (
+    CustomWidgetFactory,
     DisplayWidget,
     renderElement,
     )
@@ -35,6 +38,7 @@ from zope.schema import (
     Choice,
     List,
     Password,
+    Text,
     TextLine,
     ValidationError,
     )
@@ -242,6 +246,12 @@ class OCIRecipeView(LaunchpadView):
         else:
             return "Built on request"
 
+    @property
+    def build_args(self):
+        return "\n".join(
+            "%s=%s" % (k, v)
+            for k, v in sorted(self.context.build_args.items()))
+
 
 def builds_for_recipe(recipe):
     """A list of interesting builds.
@@ -676,6 +686,7 @@ class IOCIRecipeEditSchema(Interface):
         "description",
         "git_ref",
         "build_file",
+        "build_args",
         "build_path",
         "build_daily",
         "require_virtualized",
@@ -683,7 +694,45 @@ class IOCIRecipeEditSchema(Interface):
         ])
 
 
-class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin):
+class OCIRecipeFormMixin:
+    """Mixin with common processing for both edit and add views."""
+    custom_widget_build_args = CustomWidgetFactory(
+        TextAreaWidget, height=5, width=100)
+
+    def createBuildArgsField(self):
+        """Create a form field for OCIRecipe.build_args attribute."""
+        if IOCIRecipe.providedBy(self.context):
+            default = "\n".join(
+                "%s=%s" % (k, v)
+                for k, v in sorted(self.context.build_args.items()))
+        else:
+            default = ""
+        return FormFields(Text(
+            __name__='build_args',
+            title=u'Build-time ARG variables',
+            description=("One per line. Each ARG should be in the format "
+                         "of ARG_KEY=arg_value."),
+            default=default,
+            required=False, readonly=False))
+
+    def validateBuildArgs(self, data):
+        field_value = data.get('build_args')
+        if not field_value:
+            return
+        build_args = {}
+        for i, line in enumerate(field_value.split("\n")):
+            if '=' not in line:
+                msg = ("'%s' at line %s is not a valid KEY=value pair." %
+                       (line, i + 1))
+                self.setFieldError("build_args", six.text_type(msg))
+                return
+            k, v = line.split('=', 1)
+            build_args[k] = v
+        data['build_args'] = build_args
+
+
+class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin,
+                       OCIRecipeFormMixin):
     """View for creating OCI recipes."""
 
     page_title = label = "Create a new OCI recipe"
@@ -708,6 +757,7 @@ class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin):
     def setUpFields(self):
         """See `LaunchpadFormView`."""
         super(OCIRecipeAddView, self).setUpFields()
+        self.form_fields += self.createBuildArgsField()
         self.form_fields += self.createEnabledProcessors(
             getUtility(IProcessorSet).getAll(),
             "The architectures that this OCI recipe builds for. Some "
@@ -748,15 +798,16 @@ class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin):
                     "There is already an OCI recipe owned by %s in %s with "
                     "this name." % (
                         owner.display_name, self.context.display_name))
+        self.validateBuildArgs(data)
 
     @action("Create OCI recipe", name="create")
     def create_action(self, action, data):
         recipe = getUtility(IOCIRecipeSet).new(
             name=data["name"], registrant=self.user, owner=data["owner"],
             oci_project=self.context, git_ref=data["git_ref"],
-            build_file=data["build_file"], build_path=data["build_path"],
-            description=data["description"],
-            build_daily=data["build_daily"], processors=data["processors"])
+            build_file=data["build_file"], description=data["description"],
+            build_daily=data["build_daily"], build_args=data["build_args"],
+            build_path=data["build_path"], processors=data["processors"])
         self.next_url = canonical_url(recipe)
 
 
@@ -798,7 +849,8 @@ class OCIRecipeAdminView(BaseOCIRecipeEditView):
     field_names = ("require_virtualized", "allow_internet")
 
 
-class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin):
+class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin,
+                        OCIRecipeFormMixin):
     """View for editing OCI recipes."""
 
     @property
@@ -821,6 +873,7 @@ class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin):
     def setUpFields(self):
         """See `LaunchpadFormView`."""
         super(OCIRecipeEditView, self).setUpFields()
+        self.form_fields += self.createBuildArgsField()
         self.form_fields += self.createEnabledProcessors(
             self.context.available_processors,
             "The architectures that this OCI recipe builds for. Some "
@@ -860,7 +913,7 @@ class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin):
                         # This processor is restricted and currently
                         # enabled. Leave it untouched.
                         data["processors"].append(processor)
-
+        self.validateBuildArgs(data)
 
 class OCIRecipeDeleteView(BaseOCIRecipeEditView):
     """View for deleting OCI recipes."""
diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index c854d14..9a3f0ea 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -215,6 +215,26 @@ class TestOCIRecipeAddView(BaseTestOCIRecipeView):
             "Build schedule:\nBuilt on request\nEdit OCI recipe\n",
             MatchesTagText(content, "build-schedule"))
 
+    def test_create_new_recipe_with_build_args(self):
+        oci_project = self.factory.makeOCIProject()
+        [git_ref] = self.factory.makeGitRefs()
+        browser = self.getViewBrowser(
+            oci_project, view_name="+new-recipe", user=self.person)
+        browser.getControl(name="field.name").value = "recipe-name"
+        browser.getControl("Description").value = "Recipe description"
+        browser.getControl("Git repository").value = (
+            git_ref.repository.identity)
+        browser.getControl("Git branch").value = git_ref.path
+        browser.getControl("Build-time ARG variables").value = (
+            "VAR1=10\nVAR2=20")
+        browser.getControl("Create OCI recipe").click()
+
+        content = find_main_content(browser.contents)
+        self.assertEqual("recipe-name", extract_text(content.h1))
+        self.assertThat(
+            "Build-time\nARG variables:\nVAR1=10\nVAR2=20",
+            MatchesTagText(content, "build-args"))
+
     def test_create_new_recipe_users_teams_as_owner_options(self):
         # Teams that the user is in are options for the OCI recipe owner.
         self.factory.makeTeam(
@@ -466,6 +486,47 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
         login_person(self.person)
         self.assertRecipeProcessors(recipe, ["386", "amd64"])
 
+    def test_edit_build_args(self):
+        self.setUpDistroSeries()
+        oci_project = self.factory.makeOCIProject(pillar=self.distribution)
+        recipe = self.factory.makeOCIRecipe(
+            registrant=self.person, owner=self.person,
+            oci_project=oci_project, build_args={"VAR1": "xxx", "VAR2": "uu"})
+        browser = self.getViewBrowser(
+            recipe, view_name="+edit", user=recipe.owner)
+        args = browser.getControl(name="field.build_args")
+        self.assertContentEqual("VAR1=xxx\r\nVAR2=uu", args.value)
+        args.value = "VAR=aa\nANOTHER_VAR=bbb"
+        browser.getControl("Update OCI recipe").click()
+        login_person(self.person)
+        IStore(recipe).reload(recipe)
+        self.assertEqual(
+            {"VAR": "aa", "ANOTHER_VAR": "bbb"}, recipe.build_args)
+
+    def test_edit_build_args_invalid_content(self):
+        self.setUpDistroSeries()
+        oci_project = self.factory.makeOCIProject(pillar=self.distribution)
+        recipe = self.factory.makeOCIRecipe(
+            registrant=self.person, owner=self.person,
+            oci_project=oci_project, build_args={"VAR1": "xxx", "VAR2": "uu"})
+        browser = self.getViewBrowser(
+            recipe, view_name="+edit", user=recipe.owner)
+        args = browser.getControl(name="field.build_args")
+        self.assertContentEqual("VAR1=xxx\r\nVAR2=uu", args.value)
+        args.value = "VAR=aa\nmessed up text"
+        browser.getControl("Update OCI recipe").click()
+
+        # Error message should be shown.
+        content = find_main_content(browser.contents)
+        self.assertIn(
+            "'messed up text' at line 2 is not a valid KEY=value pair.",
+            extract_text(content))
+
+        # Assert that recipe still have the original build_args.
+        login_person(self.person)
+        IStore(recipe).reload(recipe)
+        self.assertEqual({"VAR1": "xxx", "VAR2": "uu"}, recipe.build_args)
+
     def test_edit_with_invisible_processor(self):
         # It's possible for existing recipes to have an enabled processor
         # that's no longer usable with the current distroseries, which will
@@ -705,6 +766,37 @@ class TestOCIRecipeView(BaseTestOCIRecipeView):
             """ % (oci_project_name, oci_project_display),
             self.getMainText(build.recipe))
 
+    def test_index_with_build_args(self):
+        oci_project = self.factory.makeOCIProject(
+            pillar=self.distroseries.distribution)
+        oci_project_name = oci_project.name
+        oci_project_display = oci_project.display_name
+        [ref] = self.factory.makeGitRefs(
+            owner=self.person, target=self.person, name="recipe-repository",
+            paths=["refs/heads/master"])
+        recipe = self.makeOCIRecipe(
+            oci_project=oci_project, git_ref=ref, build_file="Dockerfile",
+            build_args={"VAR1": "123", "VAR2": "XXX"})
+        build = self.makeBuild(
+            recipe=recipe, status=BuildStatus.FULLYBUILT,
+            duration=timedelta(minutes=30))
+        self.assertTextMatchesExpressionIgnoreWhitespace("""\
+            %s OCI project
+            recipe-name
+            .*
+            OCI recipe information
+            Owner: Test Person
+            OCI project: %s
+            Source: ~test-person/\\+git/recipe-repository:master
+            Build file path: Dockerfile
+            Build schedule: Built on request
+            Build-time\nARG variables: VAR1=123 VAR2=XXX
+            Latest builds
+            Status When complete Architecture
+            Successfully built 30 minutes ago 386
+            """ % (oci_project_name, oci_project_display),
+            self.getMainText(build.recipe))
+
     def test_index_success_with_buildlog(self):
         # The build log is shown if it is there.
         build = self.makeBuild(
diff --git a/lib/lp/oci/templates/ocirecipe-index.pt b/lib/lp/oci/templates/ocirecipe-index.pt
index c2ef69e..5c3ca56 100644
--- a/lib/lp/oci/templates/ocirecipe-index.pt
+++ b/lib/lp/oci/templates/ocirecipe-index.pt
@@ -69,6 +69,18 @@
           <a tal:replace="structure view/menu:overview/edit/fmt:icon"/>
         </dd>
       </dl>
+      <dl id="build-args"
+          tal:define="build_args view/build_args"
+          tal:condition="build_args">
+        <dt>
+          Build-time
+          <a href="https://docs.docker.com/engine/reference/commandline/build/#set-build-time-variables---build-arg"
+             target="_blank">ARG variables</a>:
+        </dt>
+        <dd>
+          <pre tal:content="build_args" />
+        </dd>
+      </dl>
     </div>
 
     <h2>Latest builds</h2>
diff --git a/lib/lp/oci/templates/ocirecipe-new.pt b/lib/lp/oci/templates/ocirecipe-new.pt
index 0bc156a..1cae71e 100644
--- a/lib/lp/oci/templates/ocirecipe-new.pt
+++ b/lib/lp/oci/templates/ocirecipe-new.pt
@@ -35,6 +35,9 @@
         <tal:widget define="widget nocall:view/widgets/build_daily">
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>
+        <tal:widget define="widget nocall:view/widgets/build_args">
+          <metal:block use-macro="context/@@launchpad_form/widget_row" />
+        </tal:widget>
         <tal:widget define="widget nocall:view/widgets/processors">
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>