Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .github/workflows/verify-config.yaml
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,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
Expand Down
4 changes: 2 additions & 2 deletions examples/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
3 changes: 1 addition & 2 deletions examples/config-example.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down
3 changes: 1 addition & 2 deletions examples/config-example.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 27 additions & 4 deletions pkg/cli/server/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import (
"context"
"errors"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -856,15 +857,37 @@
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")

Check warning on line 867 in pkg/cli/server/root.go

View check run for this annotation

Codecov / codecov/patch

pkg/cli/server/root.go#L867

Added line #L867 was not covered by tests

return config.LDAPCredentials{}, errors.Join(zerr.ErrBadConfig, err)

Check warning on line 869 in pkg/cli/server/root.go

View check run for this annotation

Codecov / codecov/patch

pkg/cli/server/root.go#L869

Added line #L869 was not covered by tests
}

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
Expand Down
195 changes: 193 additions & 2 deletions pkg/cli/server/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
Expand Down
Loading