Skip to content

Commit

Permalink
syncclusterconfig: handle missing --users-directory
Browse files Browse the repository at this point in the history
Prior to this commit the syncclusterconfig command would fail if the directory
specified by `--users-directory` did not exist. As the flag had a default value
and our helm chart always specifies the flag, this command could frequently
fail with no solution.

This commit updates the command to handle non-existent directories by simply
skipping the check.
  • Loading branch information
chrisseto committed Nov 14, 2024
1 parent 79f9708 commit 7532af7
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 5 deletions.
4 changes: 4 additions & 0 deletions operator/cmd/syncclusterconfig/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ func loadBoostrapYAML(path string) (map[string]any, error) {
func loadUsersFiles(ctx context.Context, path string) (map[string][]byte, error) {
files, err := os.ReadDir(path)
if err != nil {
if os.IsNotExist(err) {
log.FromContext(ctx).Info(fmt.Sprintf("users directory doesn't exist; skipping: %q", path))
return nil, nil
}
return nil, err
}

Expand Down
32 changes: 27 additions & 5 deletions operator/cmd/syncclusterconfig/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"context"
"encoding/json"
"fmt"
"os"
"path/filepath"
"strings"
"testing"
Expand Down Expand Up @@ -87,13 +88,15 @@ func TestSync(t *testing.T) {
usersTxtYAMLPath := testutils.WriteFile(t, "users-*.txt", []byte(strings.Join([]string{admin, password, saslMechanism}, ":")))

cases := []struct {
Config map[string]any
Expected map[string]any
Config map[string]any
Expected map[string]any
UsersTXTDir string
}{
{
// No superusers entry, the value just gets pulled from
// what we initialized in our redpanda.Run call above.
Config: map[string]any{},
Config: map[string]any{},
UsersTXTDir: filepath.Dir(usersTxtYAMLPath),
Expected: map[string]any{
"admin_api_require_auth": true,
"superusers": []any{user},
Expand All @@ -109,6 +112,7 @@ func TestSync(t *testing.T) {
Config: map[string]any{
"superusers": []string{user},
},
UsersTXTDir: filepath.Dir(usersTxtYAMLPath),
Expected: map[string]any{
"admin_api_require_auth": true,
"superusers": []any{admin, user},
Expand All @@ -119,6 +123,7 @@ func TestSync(t *testing.T) {
"abort_index_segment_size": 10,
"audit_queue_drain_interval_ms": 60,
},
UsersTXTDir: filepath.Dir(usersTxtYAMLPath),
Expected: map[string]any{
"abort_index_segment_size": 10,
"admin_api_require_auth": true,
Expand All @@ -133,6 +138,7 @@ func TestSync(t *testing.T) {
// Showcasing that settings are not unset if/when they're removed.
// This is to showcase feature parity with the helm chart's job(s).
// Improvements are welcome.
UsersTXTDir: filepath.Dir(usersTxtYAMLPath),
Expected: map[string]any{
"abort_index_segment_size": 10,
"admin_api_require_auth": true,
Expand All @@ -144,6 +150,7 @@ func TestSync(t *testing.T) {
Config: map[string]any{
"audit_queue_drain_interval_ms": 70,
},
UsersTXTDir: filepath.Dir(usersTxtYAMLPath),
Expected: map[string]any{
"abort_index_segment_size": 10,
"admin_api_require_auth": true,
Expand All @@ -157,21 +164,36 @@ func TestSync(t *testing.T) {
// auth is ignored if it's not required.
"admin_api_require_auth": false,
},
UsersTXTDir: filepath.Dir(usersTxtYAMLPath),
Expected: map[string]any{
"abort_index_segment_size": 10,
"audit_queue_drain_interval_ms": 70,
"superusers": []any{admin, user},
},
},
{
Config: map[string]any{
"admin_api_require_auth": false,
"superusers": []string{user},
},
UsersTXTDir: os.TempDir() + "/this-path-does-not-exist",
Expected: map[string]any{
"abort_index_segment_size": 10,
"audit_queue_drain_interval_ms": 70,
"superusers": []any{user},
},
},
}

for _, tc := range cases {
for i, tc := range cases {
t.Logf("case %d", i)

configBytes, err := yaml.Marshal(tc.Config)
require.NoError(t, err)

cmd := Command()
cmd.SetArgs([]string{
"--users-directory", filepath.Dir(usersTxtYAMLPath),
"--users-directory", tc.UsersTXTDir,
"--redpanda-yaml", redpandaYAMLPath,
"--bootstrap-yaml", testutils.WriteFile(t, "bootstrap-*.yaml", configBytes),
})
Expand Down

0 comments on commit 7532af7

Please sign in to comment.