Added the option to enable Gitlab Container Registry #77639
Conversation
to pointing to cert like in rest of package
ofborg
bot
added
6.topic: nixos
8.has: module (update)
2.status: merge conflict
10.rebuild-darwin: 0
10.rebuild-linux: 0
and removed
2.status: merge conflict
labels
|
@flokli unfortunately my only |
|
Sorry for the delay, I hope to take a look at this till the end of the upcoming week. |
There was a problem hiding this comment.
First of all, thank you for putting in the work and sorry for the delay! I haven't had time to try this out, but I've left some comments on things I would like to see fixed. In addition to these fixes, could you squash some of your commits? Everything, except maybe the maintainer-list addition, might as well be the same commit.
| host = mkOption { | ||
| type = types.str; | ||
| default = "registry." + config.networking.hostName; | ||
| description = "Gitlab-Contaier Registry host name."; |
There was a problem hiding this comment.
Minor spelling and style nitpicks. Applies to the other descriptions as well.
| description = "Gitlab-Contaier Registry host name."; | |
| description = "GitLab container registry host name."; |
| }; | ||
| host = mkOption { | ||
| type = types.str; | ||
| default = "registry." + config.networking.hostName; |
There was a problem hiding this comment.
Maybe host should default to services.gitlab.host, since it's running on the same machine?
| @@ -633,6 +669,22 @@ in { | |||
| # Use postfix to send out mails. | |||
| services.postfix.enable = mkDefault true; | |||
|
|
|||
| # Enable Docker Registry, if GitLab-Container Registry is enabled | |||
| services.dockerRegistry = { | |||
There was a problem hiding this comment.
We don't want any of this to apply unless services.gitlab.registry.enable is set to true.
| services.dockerRegistry = { | |
| services.dockerRegistry = optionalAttrs cfg.registry.enable { |
|
I applied the suggestions from above. The squash is still to be done, but maybe you can re-review my changes in the meantime. |
| host = mkOption { | ||
| type = types.str; | ||
| default = config.services.gitlab.host; | ||
| description = "GitLab container registry host name."; | ||
| }; | ||
| port = mkOption { | ||
| type = types.int; | ||
| default = 4567; | ||
| description = "GitLab container registry port."; | ||
| }; | ||
| certFile = mkOption { | ||
| type = types.path; | ||
| default = null; | ||
| description = "Path to gitLab container registry certificate."; | ||
| }; | ||
| keyFile = mkOption { | ||
| type = types.path; | ||
| default = null; | ||
| description = "Path to gitLab container registry certificate-key."; | ||
| }; |
There was a problem hiding this comment.
see my note from above regarding duplication of configuration options.
Options not existing there but set in the gitlab module should probably be added to the dockerRegistry module.
There was a problem hiding this comment.
These are the options, which control the endpoint, which is presented to the user. I think this should stay there.
| port = 5000; | ||
| enableDelete = true; # This must be true, otherwise GitLab won't manage it correctly | ||
| extraConfig = { | ||
| REGISTRY_LOG_LEVEL = "info"; |
There was a problem hiding this comment.
Deletion is not the default. When disabled, GitLab will throw errors, because it cannot fully delete projects.
There was a problem hiding this comment.
I was referring to the log level, at that line.
| @@ -633,6 +684,19 @@ in { | |||
| # Use postfix to send out mails. | |||
| services.postfix.enable = mkDefault true; | |||
|
|
|||
| # Enable Docker Registry, if GitLab-Container Registry is enabled | |||
| services.dockerRegistry = optionalAttrs cfg.registry.enable { | |||
| enable = cfg.registry.enable; | |||
There was a problem hiding this comment.
this can be true, as the whole attrset is inside a optionalAttrs cfg.registry.enable.
|
Can you add registry usage to the VM test, so people see how to use it, and we make sure it works (well, at least ensure the registry service starts successfully)? |
| @@ -32,6 +32,94 @@ import ./make-test-python.nix ({ pkgs, lib, ...} : with lib; { | |||
| databasePasswordFile = pkgs.writeText "dbPassword" "xo0daiF4"; | |||
| initialRootPasswordFile = pkgs.writeText "rootPassword" initialRootPassword; | |||
| smtp.enable = true; | |||
| registry = { | |||
| enable = true; | |||
| certFile = pkgs.writeText "registry.crt" '' | |||
There was a problem hiding this comment.
can you create certs on the fly, like in taskserver.nix? This should be way more readable than dumping ~80lines here ;-)
Motivation for this change
In the current state, it is not possible to easily use the Gitlab Container-Registry on NixOS like the rest of Gitlab. Documentation on how to configure it is very rare. To ease the configuration for future Gitlab users on NixOS I implemented most of the required options and serivces into the Gitlab service. The only thing that has to be done manually is the root certificate generation.
Things done
Tested using sandboxing (nix.useSandbox on NixOS, or option
sandboxinnix.confon non-NixOS linux)Built on platform(s)
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using
nix-shell -p nixpkgs-review --run "nixpkgs-review wip"Tested execution of all binary files (usually in
./result/bin/)Determined the impact on package closure size (by running
nix path-info -Sbefore and after)Ensured that relevant documentation is up to date
Fits CONTRIBUTING.md.
Not mentioned in the template: I tested this inside a VM.