Add a job to check zot config examples (and fix existing examples) (#…

…2322)

* fix: Add credentials config verification

(cherry picked from commit e7fdfa0)
Signed-off-by: Andrei Aaron <[email protected]>

* fix: Update golang version to 1.21.x

Signed-off-by: onidoru <[email protected]>
Signed-off-by: Nikita Kotikov <[email protected]>
(cherry picked from commit cbc0f89)
Signed-off-by: Andrei Aaron <[email protected]>

* fix: LDAP credentials files are now required, add more tests

Signed-off-by: onidoru <[email protected]>
Signed-off-by: Nikita Kotikov <[email protected]>
(cherry picked from commit b74366d)
Signed-off-by: Andrei Aaron <[email protected]>

* fix: Update error handling, add more tests

Signed-off-by: onidoru <[email protected]>
Signed-off-by: Nikita Kotikov <[email protected]>
(cherry picked from commit 8a61bbc)
Signed-off-by: Andrei Aaron <[email protected]>

* fix: Add coverage

Signed-off-by: Andrei Aaron <[email protected]>

---------

Signed-off-by: onidoru <[email protected]>
Signed-off-by: Nikita Kotikov <[email protected]>
Signed-off-by: Andrei Aaron <[email protected]>
Co-authored-by: onidoru <[email protected]>
Co-authored-by: Nikita Kotikov <[email protected]>

.github/workflows/verify-config.yaml

@@ -0,0 +1,45 @@
+name: "Verify Example Config Files"
+
+# Validate all example config files are relevant and valid.
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+    branches: [main]
+  release:
+    types:
+      - published
+
+permissions: read-all
+
+jobs:
+  verify-config:
+    name: Verify Config Files
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Install go
+        uses: actions/setup-go@v5
+        with:
+          cache: false
+          go-version: 1.21.x
+      - name: Cache go dependencies
+        id: cache-go-dependencies
+        uses: actions/cache@v4
+        with:
+          path: |
+            ~/go/pkg/mod
+          key: ${{ runner.os }}-go-mod-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ runner.os }}-go-mod-
+      - name: Install go dependencies
+        if: steps.cache-go-dependencies.outputs.cache-hit != 'true'
+        run: |
+          cd $GITHUB_WORKSPACE
+          go mod download
+      - name: run verify-config
+        run: |
+          cd $GITHUB_WORKSPACE
+          make verify-config

Makefile

@@ -391,7 +391,7 @@ verify-config: _verify-config verify-config-warnings verify-config-commited
 .PHONY: _verify-config
 _verify-config: binary
 	rm -f output.txt
-	$(foreach file, $(wildcard examples/config-*), ./bin/zot-$(OS)-$(ARCH) verify $(file) 2>&1 | tee -a output.txt || exit 1;)
+	$(foreach file, $(filter-out examples/config-ldap-credentials.json, $(wildcard examples/config-*)), ./bin/zot-$(OS)-$(ARCH) verify $(file) 2>&1 | tee -a output.txt || exit 1;)
 
 .PHONY: verify-config-warnings
 verify-config-warnings: _verify-config

examples/README.md

@@ -225,14 +225,14 @@ authentication:
         "startTLS":false,
         "baseDN":"ou=Users,dc=example,dc=org",
         "userAttribute":"uid",
-        "bindDN":"cn=ldap-searcher,ou=Users,dc=example,dc=org",
-        "bindPassword":"ldap-searcher-password",
+        "credentialsFile": "config-ldap-credentials.json",
         "skipVerify":false,
         "subtreeSearch":true
       },
 ```
 
 NOTE: When both htpasswd and LDAP configuration are specified, LDAP authentication is given preference.
+NOTE: The separate file for storing DN and password credentials must be created. You can see example in `examples/config-ldap-credentials.json` file.
 
 **OAuth2 authentication** (client credentials grant type) support via _Bearer Token_ configured with:
 

examples/config-example.json

@@ -18,8 +18,7 @@
         "startTLS": false,
         "baseDN": "ou=Users,dc=example,dc=org",
         "userAttribute": "uid",
-        "bindDN": "cn=ldap-searcher,ou=Users,dc=example,dc=org",
-        "bindPassword": "ldap-searcher-password",
+        "credentialsFile": "examples/config-ldap-credentials.json",
         "skipVerify": false,
         "subtreeSearch": true
       },

examples/config-example.yaml

@@ -8,8 +8,7 @@ http:
     ldap:
       address: ldap.example.org
       basedn: ou=Users,dc=example,dc=org
-      binddn: cn=ldap-searcher,ou=Users,dc=example,dc=org
-      bindpassword: ldap-searcher-password
+      credentialsFile: examples/config-ldap-credentials.json
       port: 389
       skipverify: false
       starttls: false

pkg/cli/server/root.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -856,15 +857,37 @@ func readLDAPCredentials(ldapConfigPath string) (config.LDAPCredentials, error)
 	if err := viperInstance.ReadInConfig(); err != nil {
 		log.Error().Err(err).Msg("failed to read configuration")
 
-		return config.LDAPCredentials{}, err
+		return config.LDAPCredentials{}, errors.Join(zerr.ErrBadConfig, err)
 	}
 
 	var ldapCredentials config.LDAPCredentials
 
-	if err := viperInstance.Unmarshal(&ldapCredentials); err != nil {
-		log.Error().Err(err).Msg("failed to unmarshal new config")
+	metaData := &mapstructure.Metadata{}
+	if err := viperInstance.Unmarshal(&ldapCredentials, metadataConfig(metaData)); err != nil {
+		log.Error().Err(err).Msg("failed to unmarshal ldap credentials config")
+
+		return config.LDAPCredentials{}, errors.Join(zerr.ErrBadConfig, err)
+	}
+
+	if len(metaData.Keys) == 0 {
+		log.Error().Err(zerr.ErrBadConfig).
+			Msg("failed to load ldap credentials config due to the absence of any key:value pair")
+
+		return config.LDAPCredentials{}, zerr.ErrBadConfig
+	}
+
+	if len(metaData.Unused) > 0 {
+		log.Error().Err(zerr.ErrBadConfig).Strs("keys", metaData.Unused).
+			Msg("failed to load ldap credentials config due to unknown keys")
+
+		return config.LDAPCredentials{}, zerr.ErrBadConfig
+	}
+
+	if len(metaData.Unset) > 0 {
+		log.Error().Err(zerr.ErrBadConfig).Strs("keys", metaData.Unset).
+			Msg("failed to load ldap credentials config due to unset keys")
 
-		return config.LDAPCredentials{}, err
+		return config.LDAPCredentials{}, zerr.ErrBadConfig
 	}
 
 	return ldapCredentials, nil

pkg/cli/server/root_test.go

@@ -1159,7 +1159,7 @@ storage:
 		content := []byte(`{"distSpecVersion":"1.1.0","storage":{"rootDirectory":"/tmp/zot"},
 							"http":{"address":"127.0.0.1","port":"8080","realm":"zot",
 							"auth":{"openid":{"providers":{"oidc":{"issuer":"http://127.0.0.1:5556/dex",
-						    "clientid":"client_id","scopes":["openid"]}}}}},
+							"clientid":"client_id","scopes":["openid"]}}}}},
 							"log":{"level":"debug"}}`)
 		_, err = tmpfile.Write(content)
 		So(err, ShouldBeNil)
@@ -1236,6 +1236,197 @@ storage:
 		err = cli.NewServerRootCmd().Execute()
 		So(err, ShouldBeNil)
 	})
+
+	Convey("Test verify good ldap config", t, func(c C) {
+		tmpFile, err := os.CreateTemp("", "zot-test*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpFile.Name())
+
+		tmpCredsFile, err := os.CreateTemp("", "zot-cred*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpCredsFile.Name())
+
+		content := []byte(`{
+			"bindDN":"cn=ldap-searcher,ou=Users,dc=example,dc=org",
+			"bindPassword":"ldap-searcher-password"
+		}`)
+
+		_, err = tmpCredsFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpCredsFile.Close()
+		So(err, ShouldBeNil)
+
+		content = []byte(fmt.Sprintf(`{ "distSpecVersion": "1.1.0-dev", 
+			"storage": { "rootDirectory": "/tmp/zot" }, "http": { "address": "127.0.0.1", "port": "8080", 
+			"auth": { "ldap": { "credentialsFile": "%v", "address": "ldap.example.org", "port": 389, 
+			"startTLS": false, "baseDN": "ou=Users,dc=example,dc=org", 
+			"userAttribute": "uid", "userGroupAttribute": "memberOf", "skipVerify": true, "subtreeSearch": true }, 
+			"failDelay": 5 } }, "log": { "level": "debug" } }`,
+			tmpCredsFile.Name()),
+		)
+
+		_, err = tmpFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpFile.Close()
+		So(err, ShouldBeNil)
+
+		os.Args = []string{"cli_test", "verify", tmpFile.Name()}
+		err = cli.NewServerRootCmd().Execute()
+		So(err, ShouldBeNil)
+	})
+
+	Convey("Test verify bad ldap config: key is missing", t, func(c C) {
+		tmpFile, err := os.CreateTemp("", "zot-test*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpFile.Name())
+
+		tmpCredsFile, err := os.CreateTemp("", "zot-cred*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpCredsFile.Name())
+
+		// `bindDN` key is missing
+		content := []byte(`{
+			"bindPassword":"ldap-searcher-password"
+		}`)
+
+		_, err = tmpCredsFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpCredsFile.Close()
+		So(err, ShouldBeNil)
+
+		content = []byte(fmt.Sprintf(`{ "distSpecVersion": "1.1.0-dev", 
+			"storage": { "rootDirectory": "/tmp/zot" }, "http": { "address": "127.0.0.1", "port": "8080", 
+			"auth": { "ldap": { "credentialsFile": "%v", "address": "ldap.example.org", "port": 389, 
+			"startTLS": false, "baseDN": "ou=Users,dc=example,dc=org", 
+			"userAttribute": "uid", "userGroupAttribute": "memberOf", "skipVerify": true, "subtreeSearch": true }, 
+			"failDelay": 5 } }, "log": { "level": "debug" } }`,
+			tmpCredsFile.Name()),
+		)
+
+		_, err = tmpFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpFile.Close()
+		So(err, ShouldBeNil)
+
+		os.Args = []string{"cli_test", "verify", tmpFile.Name()}
+		err = cli.NewServerRootCmd().Execute()
+		So(err, ShouldNotBeNil)
+		So(err.Error(), ShouldContainSubstring, "invalid server config")
+	})
+
+	Convey("Test verify bad ldap config: unused key", t, func(c C) {
+		tmpFile, err := os.CreateTemp("", "zot-test*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpFile.Name())
+
+		tmpCredsFile, err := os.CreateTemp("", "zot-cred*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpCredsFile.Name())
+
+		content := []byte(`{
+			"bindDN":"cn=ldap-searcher,ou=Users,dc=example,dc=org",
+			"bindPassword":"ldap-searcher-password",
+			"extraKey": "extraValue"
+		}`)
+
+		_, err = tmpCredsFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpCredsFile.Close()
+		So(err, ShouldBeNil)
+
+		content = []byte(fmt.Sprintf(`{ "distSpecVersion": "1.1.0-dev", 
+			"storage": { "rootDirectory": "/tmp/zot" }, "http": { "address": "127.0.0.1", "port": "8080", 
+			"auth": { "ldap": { "credentialsFile": "%v", "address": "ldap.example.org", "port": 389, 
+			"startTLS": false, "baseDN": "ou=Users,dc=example,dc=org", 
+			"userAttribute": "uid", "userGroupAttribute": "memberOf", "skipVerify": true, "subtreeSearch": true }, 
+			"failDelay": 5 } }, "log": { "level": "debug" } }`,
+			tmpCredsFile.Name()),
+		)
+
+		_, err = tmpFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpFile.Close()
+		So(err, ShouldBeNil)
+
+		os.Args = []string{"cli_test", "verify", tmpFile.Name()}
+		err = cli.NewServerRootCmd().Execute()
+		So(err, ShouldNotBeNil)
+		So(err.Error(), ShouldContainSubstring, "invalid server config")
+	})
+
+	Convey("Test verify bad ldap config: empty credentials file", t, func(c C) {
+		tmpFile, err := os.CreateTemp("", "zot-test*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpFile.Name())
+
+		tmpCredsFile, err := os.CreateTemp("", "zot-cred*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpCredsFile.Name())
+
+		// `bindDN` key is missing
+		content := []byte(``)
+
+		_, err = tmpCredsFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpCredsFile.Close()
+		So(err, ShouldBeNil)
+
+		content = []byte(fmt.Sprintf(`{ "distSpecVersion": "1.1.0-dev", 
+			"storage": { "rootDirectory": "/tmp/zot" }, "http": { "address": "127.0.0.1", "port": "8080", 
+			"auth": { "ldap": { "credentialsFile": "%v", "address": "ldap.example.org", "port": 389, 
+			"startTLS": false, "baseDN": "ou=Users,dc=example,dc=org", 
+			"userAttribute": "uid", "userGroupAttribute": "memberOf", "skipVerify": true, "subtreeSearch": true }, 
+			"failDelay": 5 } }, "log": { "level": "debug" } }`,
+			tmpCredsFile.Name()),
+		)
+
+		_, err = tmpFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpFile.Close()
+		So(err, ShouldBeNil)
+
+		os.Args = []string{"cli_test", "verify", tmpFile.Name()}
+		err = cli.NewServerRootCmd().Execute()
+		So(err, ShouldNotBeNil)
+		So(err.Error(), ShouldContainSubstring, "invalid server config")
+	})
+
+	Convey("Test verify bad ldap config: no keys set in credentials file", t, func(c C) {
+		tmpFile, err := os.CreateTemp("", "zot-test*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpFile.Name())
+
+		tmpCredsFile, err := os.CreateTemp("", "zot-cred*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpCredsFile.Name())
+
+		// empty json
+		content := []byte(`{}`)
+
+		_, err = tmpCredsFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpCredsFile.Close()
+		So(err, ShouldBeNil)
+
+		content = []byte(fmt.Sprintf(`{ "distSpecVersion": "1.1.0-dev", 
+			"storage": { "rootDirectory": "/tmp/zot" }, "http": { "address": "127.0.0.1", "port": "8080", 
+			"auth": { "ldap": { "credentialsFile": "%v", "address": "ldap.example.org", "port": 389, 
+			"startTLS": false, "baseDN": "ou=Users,dc=example,dc=org", 
+			"userAttribute": "uid", "userGroupAttribute": "memberOf", "skipVerify": true, "subtreeSearch": true }, 
+			"failDelay": 5 } }, "log": { "level": "debug" } }`,
+			tmpCredsFile.Name()),
+		)
+
+		_, err = tmpFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpFile.Close()
+		So(err, ShouldBeNil)
+
+		os.Args = []string{"cli_test", "verify", tmpFile.Name()}
+		err = cli.NewServerRootCmd().Execute()
+		So(err, ShouldNotBeNil)
+		So(err.Error(), ShouldContainSubstring, "invalid server config")
+	})
 }
 
 func TestApiKeyConfig(t *testing.T) {
@@ -1248,7 +1439,7 @@ func TestApiKeyConfig(t *testing.T) {
 		content := []byte(`{"distSpecVersion":"1.1.0","storage":{"rootDirectory":"/tmp/zot"},
 							"http":{"address":"127.0.0.1","port":"8080","realm":"zot",
 							"auth":{"openid":{"providers":{"oidc":{"issuer":"http://127.0.0.1:5556/dex",
-						    "clientid":"client_id","scopes":["openid"]}}}}},
+							"clientid":"client_id","scopes":["openid"]}}}}},
 							"log":{"level":"debug"}}`)
 
 		err = os.WriteFile(tmpfile.Name(), content, 0o0600)