From 593dc4514192354491473ba00e1b9104f456c50c Mon Sep 17 00:00:00 2001
From: Maximilian Bosch <maximilian@mbosch.me>
Date: Mon, 26 Mar 2018 13:54:01 +0200
Subject: [PATCH] nixos/docker-registry: cleanup module definition & enhance
 testcase

The following changes have been applied:

- the property `http.headers.X-Content-Type-Options` must a list of
  strings rather than a serialized list
- instead of `/etc/docker/registry/config.yml` the configuration will be
  written with `pkgs.writeText` and the store path will be used to run
  the registry. This reduces the risk of possible impurities by relying
  on the Nix store only.
- cleaned up the property paths to easy readability and reduce the
  verbosity.
- enhanced the testcase to ensure that digests can be deleted as well
- the `services.docker-registry.extraConfig` object will be merged with
  `registryConfig`

/cc @ironpinguin
---
 nixos/doc/manual/release-notes/rl-1809.xml    |  7 +++
 .../modules/services/misc/docker-registry.nix | 57 ++++++-------------
 nixos/tests/docker-registry.nix               |  8 ++-
 3 files changed, 32 insertions(+), 40 deletions(-)

diff --git a/nixos/doc/manual/release-notes/rl-1809.xml b/nixos/doc/manual/release-notes/rl-1809.xml
index 62f1b769463..5ff5caaf255 100644
--- a/nixos/doc/manual/release-notes/rl-1809.xml
+++ b/nixos/doc/manual/release-notes/rl-1809.xml
@@ -77,6 +77,13 @@ following incompatible changes:</para>
       accepted by the nc command.
     </para>
   </listitem>
+  <listitem>
+    <para>
+      The <varname>services.docker-registry.extraConfig</varname> object doesn't contain
+      environment variables anymore. Instead it needs to provide an object structure
+      that can be mapped onto the YAML configuration defined in <link xlink:href="https://github.com/docker/distribution/blob/v2.6.2/docs/configuration.md">the <varname>docker/distribution</varname> docs</link>.
+    </para>
+  </listitem>
 </itemizedlist>
 
 </section>
diff --git a/nixos/modules/services/misc/docker-registry.nix b/nixos/modules/services/misc/docker-registry.nix
index 4866ecf7793..c0dbcf380db 100644
--- a/nixos/modules/services/misc/docker-registry.nix
+++ b/nixos/modules/services/misc/docker-registry.nix
@@ -5,40 +5,26 @@ with lib;
 let
   cfg = config.services.dockerRegistry;
 
-  blogCache = if cfg.enableRedisCache
-      then "redis"
-      else "inmemory";
+  blobCache = if cfg.enableRedisCache
+    then "redis"
+    else "inmemory";
 
   registryConfig = {
     version =  "0.1";
-    log = {
-      fields = {
-        service = "registry";
-      };
-    };
+    log.fields.service = "registry";
     storage = {
-      cache = {
-        blobdescriptor = "${blogCache}";
-      };
-      filesystem = {
-        rootdirectory = "/var/lib/registry";
-      };
-      delete = {
-        enabled = cfg.enableDelete;
-      };
+      cache.blobdescriptor = blobCache;
+      filesystem.rootdirectory = cfg.storagePath;
+      delete.enabled = cfg.enableDelete;
     };
     http = {
-      addr = ":5000";
-      headers = {
-        X-Content-Type-Options = "[nosniff]";
-      };
+      addr = ":${builtins.toString cfg.port}";
+      headers.X-Content-Type-Options = ["nosniff"];
     };
-    health = {
-      storagedriver = {
-        enabled = true;
-        interval = "10s";
-        threshold = 3;
-      };
+    health.storagedriver = {
+      enabled = true;
+      interval = "10s";
+      threshold = 3;
     };
   };
 
@@ -98,7 +84,7 @@ in {
 
     redisPassword = mkOption {
       type = types.str;
-      default = "asecret";
+      default = "";
       description = "Set redis password.";
     };
 
@@ -112,21 +98,14 @@ in {
   };
 
   config = mkIf cfg.enable {
-    environment.etc."docker/registry/config.yml".text = builtins.toJSON registryConfig;
-
     systemd.services.docker-registry = {
       description = "Docker Container Registry";
       wantedBy = [ "multi-user.target" ];
       after = [ "network.target" ];
-
-      environment = {
-        REGISTRY_HTTP_ADDR = "${cfg.listenAddress}:${toString cfg.port}";
-        REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY = cfg.storagePath;
-      } // cfg.extraConfig;
-
-      script = ''
-        ${pkgs.docker-distribution}/bin/registry serve \
-          /etc/docker/registry/config.yml
+      script = let
+        configFile = pkgs.writeText "docker-registry-config.yml" (builtins.toJSON (registryConfig // cfg.extraConfig));
+      in ''
+        ${pkgs.docker-distribution}/bin/registry serve ${configFile}
       '';
 
       serviceConfig = {
diff --git a/nixos/tests/docker-registry.nix b/nixos/tests/docker-registry.nix
index 109fca440e5..943773ee391 100644
--- a/nixos/tests/docker-registry.nix
+++ b/nixos/tests/docker-registry.nix
@@ -3,12 +3,13 @@
 import ./make-test.nix ({ pkgs, ...} : {
   name = "docker-registry";
   meta = with pkgs.stdenv.lib.maintainers; {
-    maintainers = [ globin ];
+    maintainers = [ globin ma27 ];
   };
 
   nodes = {
     registry = { config, pkgs, ... }: {
       services.dockerRegistry.enable = true;
+      services.dockerRegistry.enableDelete = true;
       services.dockerRegistry.port = 8080;
       services.dockerRegistry.listenAddress = "0.0.0.0";
       networking.firewall.allowedTCPPorts = [ 8080 ];
@@ -22,6 +23,7 @@ import ./make-test.nix ({ pkgs, ...} : {
     client2 = { config, pkgs, ...}: {
       virtualisation.docker.enable = true;
       virtualisation.docker.extraOptions = "--insecure-registry registry:8080";
+      environment.systemPackages = [ pkgs.jq ];
     };
   };
 
@@ -39,5 +41,9 @@ import ./make-test.nix ({ pkgs, ...} : {
     $client2->waitForUnit("docker.service");
     $client2->succeed("docker pull registry:8080/scratch");
     $client2->succeed("docker images | grep scratch");
+
+    $client2->succeed(
+      'curl -fsS -X DELETE registry:8080/v2/scratch/manifests/$(curl registry:8080/v2/scratch/manifests/latest | jq ".fsLayers[0].blobSum" | sed -e \'s/"//g\')'
+    );
   '';
 })