T2216: containers need to be added via "add container image" in advance before using them

8 files changed, 139 insertions, 51 deletions

diff --git a/data/configd-include.json b/data/configd-include.json
index 681685dd9..c65231e49 100644
--- a/data/configd-include.json
+++ b/data/configd-include.json
@@ -1,6 +1,7 @@
 [
 "arp.py",
 "bcast_relay.py",
+"containers.py",
 "conntrack.py",
 "conntrack_sync.py",
 "dhcp_relay.py",
diff --git a/data/templates/containers/registries.conf.j2 b/data/templates/containers/registries.conf.j2
new file mode 100644
index 000000000..4057bb452
--- /dev/null
+++ b/data/templates/containers/registries.conf.j2
@@ -0,0 +1,27 @@
+### Autogenerated by /usr/libexec/vyos/conf_mode/containers.py ###
+
+# For more information on this configuration file, see containers-registries.conf(5).
+#
+# NOTE: RISK OF USING UNQUALIFIED IMAGE NAMES
+# We recommend always using fully qualified image names including the registry
+# server (full dns name), namespace, image name, and tag
+# (e.g., registry.redhat.io/ubi8/ubi:latest). Pulling by digest (i.e.,
+# quay.io/repository/name@digest) further eliminates the ambiguity of tags.
+# When using short names, there is always an inherent risk that the image being
+# pulled could be spoofed. For example, a user wants to pull an image named
+# `foobar` from a registry and expects it to come from myregistry.com. If
+# myregistry.com is not first in the search list, an attacker could place a
+# different `foobar` image at a registry earlier in the search list. The user
+# would accidentally pull and run the attacker's image and code rather than the
+# intended content. We recommend only adding registries which are completely
+# trusted (i.e., registries which don't allow unknown or anonymous users to
+# create accounts with arbitrary names). This will prevent an image from being
+# spoofed, squatted or otherwise made insecure.  If it is necessary to use one
+# of these registries, it should be added at the end of the list.
+#
+# An array of host[:port] registries to try when pulling an unqualified image, in order.
+# unqualified-search-registries = ["example.com"]
+
+{% if registry is vyos_defined %}
+unqualified-search-registries = {{ registry }}
+{% endif %}
diff --git a/data/templates/containers/registry.tmpl b/data/templates/containers/registry.tmpl
deleted file mode 100644
index 0cbd9ecc2..000000000
--- a/data/templates/containers/registry.tmpl
+++ /dev/null
@@ -1,5 +0,0 @@
-### Autogenerated by /usr/libexec/vyos/conf_mode/containers.py ###
-
-{% if registry is vyos_defined %}
-unqualified-search-registries = {{ registry }}
-{% endif %}
diff --git a/data/templates/containers/storage.tmpl b/data/templates/containers/storage.conf.j2
index 3a69b7252..3a69b7252 100644
--- a/data/templates/containers/storage.tmpl
+++ b/data/templates/containers/storage.conf.j2
diff --git a/interface-definitions/containers.xml.in b/interface-definitions/containers.xml.in
index 13587711a..85231b50c 100644
--- a/interface-definitions/containers.xml.in
+++ b/interface-definitions/containers.xml.in
@@ -283,10 +283,10 @@
       </tagNode>
       <leafNode name="registry">
         <properties>
-          <help>Add registry</help>
+          <help>Registry Name</help>
           <multi/>
         </properties>
-        <defaultValue>docker.io</defaultValue>
+        <defaultValue>docker.io quay.io</defaultValue>
       </leafNode>
     </children>
   </node>
diff --git a/op-mode-definitions/containers.xml.in b/op-mode-definitions/containers.xml.in
index d4fd1abac..925d147fa 100644
--- a/op-mode-definitions/containers.xml.in
+++ b/op-mode-definitions/containers.xml.in
@@ -50,6 +50,63 @@
       </node>
     </children>
   </node>
+  <node name="generate">
+    <children>
+      <node name="container">
+        <properties>
+          <help>Generate Container Image</help>
+        </properties>
+        <children>
+          <tagNode name="image">
+            <properties>
+              <help>Name of container image (tag)</help>
+            </properties>
+            <children>
+              <tagNode name="path">
+                <properties>
+                  <help>Path to Dockerfile</help>
+                  <completionHelp>
+                    <list>&lt;filename&gt;</list>
+                  </completionHelp>
+                </properties>
+                <command>sudo podman build --layers --force-rm --tag "$4" $6</command>
+              </tagNode>
+            </children>
+          </tagNode>
+        </children>
+      </node>
+    </children>
+  </node>
+  <node name="monitor">
+    <children>
+      <node name="log">
+        <children>
+          <tagNode name="container">
+            <properties>
+              <help>Monitor last lines of container logs</help>
+              <completionHelp>
+                <path>container name</path>
+              </completionHelp>
+            </properties>
+            <command>sudo podman logs --follow --names "$4"</command>
+          </tagNode>
+        </children>
+      </node>
+      <node name="log">
+        <children>
+          <tagNode name="container">
+            <properties>
+              <help>Show logs from a given container</help>
+              <completionHelp>
+                <path>container name</path>
+              </completionHelp>
+            </properties>
+            <command>sudo podman logs --names "$4"</command>
+          </tagNode>
+        </children>
+      </node>
+    </children>
+  </node>
   <node name="show">
     <children>
       <node name="container">
diff --git a/src/conf_mode/containers.py b/src/conf_mode/containers.py
index 3c1a61f19..1cc6f5a35 100755
--- a/src/conf_mode/containers.py
+++ b/src/conf_mode/containers.py
@@ -41,6 +41,20 @@ airbag.enable()
 config_containers_registry = '/etc/containers/registries.conf'
 config_containers_storage = '/etc/containers/storage.conf'
 
+def _run_rerun(container_cmd):
+    counter = 0
+    while True:
+        if counter >= 10:
+            break
+        try:
+            _cmd(container_cmd)
+            break
+        except:
+            counter = counter +1
+            sleep(0.5)
+
+    return None
+
 def _cmd(command):
     if os.path.exists('/tmp/vyos.container.debug'):
         print(command)
@@ -92,6 +106,20 @@ def verify(container):
     # Add new container
     if 'name' in container:
         for name, container_config in container['name'].items():
+            # Container image is a mandatory option
+            if 'image' not in container_config:
+                raise ConfigError(f'Container image for "{name}" is mandatory!')
+
+            # verify container image exists locally
+            image = container_config['image']
+
+            # Check if requested container image exists locally. If it does not
+            # exist locally - inform the user.
+            if run(f'podman image exists {image}') != 0:
+                raise ConfigError(f'Image "{image}" used in contianer "{name}" does not exist '\
+                                  f'locally.\nPlease use "add container image {image}" to add it '\
+                                  'to the system!')
+
             if 'network' in container_config:
                 if len(container_config['network']) > 1:
                     raise ConfigError(f'Only one network can be specified for container "{name}"!')
@@ -150,10 +178,6 @@ def verify(container):
                     if not os.path.exists(source):
                         raise ConfigError(f'Volume "{volume}" source path "{source}" does not exist!')
 
-            # Container image is a mandatory option
-            if 'image' not in container_config:
-                raise ConfigError(f'Container image for "{name}" is mandatory!')
-
             # If 'allow-host-networks' or 'network' not set.
             if 'allow_host_networks' not in container_config and 'network' not in container_config:
                 raise ConfigError(f'Must either set "network" or "allow-host-networks" for container "{name}"!')
@@ -193,6 +217,10 @@ def verify(container):
 def generate(container):
     # bail out early - looks like removal from running config
     if not container:
+        if os.path.exists(config_containers_registry):
+            os.unlink(config_containers_registry)
+        if os.path.exists(config_containers_storage):
+            os.unlink(config_containers_storage)
         return None
 
     if 'network' in container:
@@ -226,8 +254,8 @@ def generate(container):
 
             write_file(f'/etc/cni/net.d/{network}.conflist', json_write(tmp, indent=2))
 
-    render(config_containers_registry, 'containers/registry.tmpl', container)
-    render(config_containers_storage, 'containers/storage.tmpl', container)
+    render(config_containers_registry, 'containers/registries.conf.j2', container)
+    render(config_containers_storage, 'containers/storage.conf.j2', container)
 
     return None
 
@@ -262,13 +290,6 @@ def apply(container):
             memory = container_config['memory']
             restart = container_config['restart']
 
-            # Check if requested container image exists locally. If it does not, we
-            # pull it. print() is the best way to have a good response from the
-            # polling process to the user to display progress. If the image exists
-            # locally, a user can update it running `update container image <name>`
-            tmp = run(f'podman image exists {image}')
-            if tmp != 0: print(os.system(f'podman pull {image}'))
-
             # Add capability options. Should be in uppercase
             cap_add = ''
             if 'cap_add' in container_config:
@@ -317,7 +338,7 @@ def apply(container):
                                  f'--memory {memory}m --memory-swap 0 --restart {restart} ' \
                                  f'--name {name} {device} {port} {volume} {env_opt}'
             if 'allow_host_networks' in container_config:
-                run(f'{container_base_cmd} --net host {image}')
+                _run_rerun(f'{container_base_cmd} --net host {image}')
             else:
                 for network in container_config['network']:
                     ipparam = ''
@@ -325,25 +346,10 @@ def apply(container):
                         address = container_config['network'][network]['address']
                         ipparam = f'--ip {address}'
 
-                    run(f'{container_base_cmd} --net {network} {ipparam} {image}')
+                    _run_rerun(f'{container_base_cmd} --net {network} {ipparam} {image}')
 
     return None
 
-def run(container_cmd):
-    counter = 0
-    while True:
-        if counter >= 10:
-            break
-        try:
-            _cmd(container_cmd)
-            break
-        except:
-            counter = counter +1
-            sleep(0.5)
-
-    return None
-
-
 if __name__ == '__main__':
     try:
         c = get_config()
diff --git a/src/op_mode/containers_op.py b/src/op_mode/containers_op.py
index bc317029c..c55a48b3c 100755
--- a/src/op_mode/containers_op.py
+++ b/src/op_mode/containers_op.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 #
-# Copyright (C) 2021 VyOS maintainers and contributors
+# Copyright (C) 2021-2022 VyOS maintainers and contributors
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 2 or later as
@@ -14,11 +14,14 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import os
 import argparse
 
 from getpass import getuser
 from vyos.configquery import ConfigTreeQuery
+from vyos.base import Warning
 from vyos.util import cmd
+from subprocess import STDOUT
 
 parser = argparse.ArgumentParser()
 parser.add_argument("-a", "--all", action="store_true", help="Show all containers")
@@ -30,9 +33,6 @@ parser.add_argument("-u", "--update", action="store", help="Update given contain
 
 config = ConfigTreeQuery()
 base = ['container']
-if not config.exists(base):
-    print('Containers not configured')
-    exit(0)
 
 if getuser() != 'root':
     raise OSError('This functions needs to be run as root to return correct results!')
@@ -42,26 +42,28 @@ if __name__ == '__main__':
 
     if args.all:
         print(cmd('podman ps --all'))
-
     elif args.image:
         print(cmd('podman image ls'))
-
     elif args.networks:
         print(cmd('podman network ls'))
 
     elif args.pull:
         image = args.pull
+        registry_config = '/etc/containers/registries.conf'
+        if not os.path.exists(registry_config):
+            Warning('No container registry configured. Please use full URL when '\
+                    'adding an image. E.g. prefix with docker.io/image-name.')
         try:
-            print(cmd(f'podman image pull {image}'))
-        except:
-            print(f'Can\'t find or download image "{image}"')
+            print(os.system(f'podman image pull {image}'))
+        except Exception  as e:
+            print(f'Unable to download image "{image}". {e}')
 
     elif args.remove:
         image = args.remove
         try:
-            print(cmd(f'podman image rm {image}'))
-        except:
-            print(f'Can\'t delete image "{image}"')
+            print(os.system(f'podman image rm {image}'))
+        except FileNotFoundError as e:
+            print(f'Unable to delete image "{image}". {e}')
 
     elif args.update:
         tmp = config.get_config_dict(base + ['name', args.update],
@@ -69,8 +71,8 @@ if __name__ == '__main__':
         try:
             image = tmp['image']
             print(cmd(f'podman image pull {image}'))
-        except:
-            print(f'Can\'t find or download image "{image}"')
+        except Exception  as e:
+            print(f'Unable to download image "{image}". {e}')
     else:
         parser.print_help()
         exit(1)