Skip to content

Commit fda2dbe

Browse files
kwitschThinkChaos
kwitsch
and
ThinkChaos
authored
Refactoring Redis (#1271)
* RedisConfig -> Redis * moved redis config to seperate file * bugfix in config test during parallel processing * implement config.Configurable in Redis config * use Context in GetRedisCache * use Context in New * caching resolver test fix * use Context in PublishEnabled * use Context in getResponse * remove ctx field * bugfix in api interface test * propperly close channels * set ruler for go files from 80 to 111 * line break because function length is to long * only execute redis.New if it is enabled in config * stabilized flaky tests * Update config/redis.go Co-authored-by: ThinkChaos <ThinkChaos@users.noreply.github.com> * Update config/redis_test.go Co-authored-by: ThinkChaos <ThinkChaos@users.noreply.github.com> * Update config/redis_test.go Co-authored-by: ThinkChaos <ThinkChaos@users.noreply.github.com> * Update config/redis_test.go Co-authored-by: ThinkChaos <ThinkChaos@users.noreply.github.com> * Update config/redis.go Co-authored-by: ThinkChaos <ThinkChaos@users.noreply.github.com> * Update config/redis_test.go Co-authored-by: ThinkChaos <ThinkChaos@users.noreply.github.com> * fix ruler * redis test refactoring * vscode setting cleanup * removed else if chain * Update redis_test.go * context race fix * test fail on missing seintinel servers * cleanup context usage * cleanup2 * context fixes * added context util * disabled nil context rule for tests * copy paste error ctxSend -> CtxSend * use util.CtxSend * fixed comment * fixed flaky test * failsafe and tests --------- Co-authored-by: ThinkChaos <ThinkChaos@users.noreply.github.com>
1 parent 15bd383 commit fda2dbe

20 files changed

+571
-194
lines changed

‎.devcontainer/devcontainer.json

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,13 @@
3131
"GitHub.vscode-github-actions"
3232
],
3333
"settings": {
34-
"go.lintFlags": ["--config=${containerWorkspaceFolder}/.golangci.yml"],
34+
"go.lintFlags": [
35+
"--config=${containerWorkspaceFolder}/.golangci.yml",
36+
"--fast"
37+
],
3538
"go.alternateTools": {
3639
"go-langserver": "gopls"
3740
},
38-
"[go]": {
39-
"editor.defaultFormatter": "golang.go"
40-
},
41-
"[json][jsonc][github-actions-workflow]": {
42-
"editor.defaultFormatter": "esbenp.prettier-vscode"
43-
},
44-
"[markdown]": {
45-
"editor.defaultFormatter": "yzhang.markdown-all-in-one"
46-
},
4741
"markiscodecoverage.searchCriteria": "**/*.lcov",
4842
"runItOn": {
4943
"commands": [
@@ -52,6 +46,15 @@
5246
"cmd": "${workspaceRoot}/.devcontainer/scripts/runItOnGo.sh ${fileDirname} ${workspaceRoot}"
5347
}
5448
]
49+
},
50+
"[go]": {
51+
"editor.defaultFormatter": "golang.go"
52+
},
53+
"[json][jsonc][github-actions-workflow]": {
54+
"editor.defaultFormatter": "esbenp.prettier-vscode"
55+
},
56+
"[markdown]": {
57+
"editor.defaultFormatter": "yzhang.markdown-all-in-one"
5558
}
5659
}
5760
}

‎.golangci.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,7 @@ issues:
9898
- gochecknoinits
9999
- gochecknoglobals
100100
- gosec
101+
- path: _test\.go
102+
linters:
103+
- staticcheck
104+
text: "SA1012:"

‎.vscode/settings.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"source.organizeImports": true,
99
"source.fixAll": true
1010
},
11+
"editor.rulers": [120],
1112
"go.showWelcome": false,
1213
"go.survey.prompt": false,
1314
"go.useLanguageServer": true,

‎api/api_interface_impl.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ type BlockingStatus struct {
2929

3030
// BlockingControl interface to control the blocking status
3131
type BlockingControl interface {
32-
EnableBlocking()
33-
DisableBlocking(duration time.Duration, disableGroups []string) error
32+
EnableBlocking(ctx context.Context)
33+
DisableBlocking(ctx context.Context, duration time.Duration, disableGroups []string) error
3434
BlockingStatus() BlockingStatus
3535
}
3636

@@ -71,7 +71,7 @@ func NewOpenAPIInterfaceImpl(control BlockingControl,
7171
}
7272
}
7373

74-
func (i *OpenAPIInterfaceImpl) DisableBlocking(_ context.Context,
74+
func (i *OpenAPIInterfaceImpl) DisableBlocking(ctx context.Context,
7575
request DisableBlockingRequestObject,
7676
) (DisableBlockingResponseObject, error) {
7777
var (
@@ -91,7 +91,7 @@ func (i *OpenAPIInterfaceImpl) DisableBlocking(_ context.Context,
9191
groups = strings.Split(*request.Params.Groups, ",")
9292
}
9393

94-
err = i.control.DisableBlocking(duration, groups)
94+
err = i.control.DisableBlocking(ctx, duration, groups)
9595

9696
if err != nil {
9797
return DisableBlocking400TextResponse(log.EscapeInput(err.Error())), nil
@@ -100,9 +100,9 @@ func (i *OpenAPIInterfaceImpl) DisableBlocking(_ context.Context,
100100
return DisableBlocking200Response{}, nil
101101
}
102102

103-
func (i *OpenAPIInterfaceImpl) EnableBlocking(_ context.Context, _ EnableBlockingRequestObject,
103+
func (i *OpenAPIInterfaceImpl) EnableBlocking(ctx context.Context, _ EnableBlockingRequestObject,
104104
) (EnableBlockingResponseObject, error) {
105-
i.control.EnableBlocking()
105+
i.control.EnableBlocking(ctx)
106106

107107
return EnableBlocking200Response{}, nil
108108
}

‎api/api_interface_impl_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ func (m *ListRefreshMock) RefreshLists() error {
3737
return args.Error(0)
3838
}
3939

40-
func (m *BlockingControlMock) EnableBlocking() {
40+
func (m *BlockingControlMock) EnableBlocking(_ context.Context) {
4141
_ = m.Called()
4242
}
4343

44-
func (m *BlockingControlMock) DisableBlocking(t time.Duration, g []string) error {
44+
func (m *BlockingControlMock) DisableBlocking(_ context.Context, t time.Duration, g []string) error {
4545
args := m.Called(t, g)
4646

4747
return args.Error(0)

‎config/config.go

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ type Config struct {
219219
Caching CachingConfig `yaml:"caching"`
220220
QueryLog QueryLogConfig `yaml:"queryLog"`
221221
Prometheus MetricsConfig `yaml:"prometheus"`
222-
Redis RedisConfig `yaml:"redis"`
222+
Redis Redis `yaml:"redis"`
223223
Log log.Config `yaml:"log"`
224224
Ports PortsConfig `yaml:"ports"`
225225
MinTLSServeVer TLSVersion `yaml:"minTlsServeVersion" default:"1.2"`
@@ -280,20 +280,6 @@ type (
280280
}
281281
)
282282

283-
// RedisConfig configuration for the redis connection
284-
type RedisConfig struct {
285-
Address string `yaml:"address"`
286-
Username string `yaml:"username" default:""`
287-
Password string `yaml:"password" default:""`
288-
Database int `yaml:"database" default:"0"`
289-
Required bool `yaml:"required" default:"false"`
290-
ConnectionAttempts int `yaml:"connectionAttempts" default:"3"`
291-
ConnectionCooldown Duration `yaml:"connectionCooldown" default:"1s"`
292-
SentinelUsername string `yaml:"sentinelUsername" default:""`
293-
SentinelPassword string `yaml:"sentinelPassword" default:""`
294-
SentinelAddresses []string `yaml:"sentinelAddresses"`
295-
}
296-
297283
type (
298284
FQDNOnly = toEnable
299285
EDE = toEnable

‎config/config_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ var _ = Describe("Config", func() {
169169
err = writeConfigDir(tmpDir)
170170
Expect(err).Should(Succeed())
171171

172-
_, err := LoadConfig(tmpDir.Path, true)
172+
c, err = LoadConfig(tmpDir.Path, true)
173173
Expect(err).Should(Succeed())
174174

175175
defaultTestFileConfig(c)

‎config/redis.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package config
2+
3+
import (
4+
"strings"
5+
6+
"github.com/sirupsen/logrus"
7+
)
8+
9+
// Redis configuration for the redis connection
10+
type Redis struct {
11+
Address string `yaml:"address"`
12+
Username string `yaml:"username" default:""`
13+
Password string `yaml:"password" default:""`
14+
Database int `yaml:"database" default:"0"`
15+
Required bool `yaml:"required" default:"false"`
16+
ConnectionAttempts int `yaml:"connectionAttempts" default:"3"`
17+
ConnectionCooldown Duration `yaml:"connectionCooldown" default:"1s"`
18+
SentinelUsername string `yaml:"sentinelUsername" default:""`
19+
SentinelPassword string `yaml:"sentinelPassword" default:""`
20+
SentinelAddresses []string `yaml:"sentinelAddresses"`
21+
}
22+
23+
// IsEnabled implements `config.Configurable`
24+
func (c *Redis) IsEnabled() bool {
25+
return c.Address != ""
26+
}
27+
28+
// LogConfig implements `config.Configurable`
29+
func (c *Redis) LogConfig(logger *logrus.Entry) {
30+
if len(c.SentinelAddresses) == 0 {
31+
logger.Info("address: ", c.Address)
32+
}
33+
34+
logger.Info("username: ", c.Username)
35+
logger.Info("password: ", obfuscatePassword(c.Password))
36+
logger.Info("database: ", c.Database)
37+
logger.Info("required: ", c.Required)
38+
logger.Info("connectionAttempts: ", c.ConnectionAttempts)
39+
logger.Info("connectionCooldown: ", c.ConnectionCooldown)
40+
41+
if len(c.SentinelAddresses) > 0 {
42+
logger.Info("sentinel:")
43+
logger.Info(" master: ", c.Address)
44+
logger.Info(" username: ", c.SentinelUsername)
45+
logger.Info(" password: ", obfuscatePassword(c.SentinelPassword))
46+
logger.Info(" addresses:")
47+
48+
for _, addr := range c.SentinelAddresses {
49+
logger.Info(" - ", addr)
50+
}
51+
}
52+
}
53+
54+
// obfuscatePassword replaces all characters of a password except the first and last with *
55+
func obfuscatePassword(pass string) string {
56+
return strings.Repeat("*", len(pass))
57+
}

‎config/redis_test.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
package config
2+
3+
import (
4+
"github.com/0xERR0R/blocky/log"
5+
"github.com/creasty/defaults"
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
)
9+
10+
var _ = Describe("Redis", func() {
11+
var (
12+
c Redis
13+
err error
14+
)
15+
16+
suiteBeforeEach()
17+
18+
BeforeEach(func() {
19+
err = defaults.Set(&c)
20+
Expect(err).Should(Succeed())
21+
})
22+
23+
Describe("IsEnabled", func() {
24+
When("all fields are default", func() {
25+
It("should be disabled", func() {
26+
Expect(c.IsEnabled()).Should(BeFalse())
27+
})
28+
})
29+
30+
When("Address is set", func() {
31+
BeforeEach(func() {
32+
c.Address = "localhost:6379"
33+
})
34+
35+
It("should be enabled", func() {
36+
Expect(c.IsEnabled()).Should(BeTrue())
37+
})
38+
})
39+
})
40+
41+
Describe("LogConfig", func() {
42+
BeforeEach(func() {
43+
logger, hook = log.NewMockEntry()
44+
})
45+
46+
When("all fields are default", func() {
47+
It("should log default values", func() {
48+
c.LogConfig(logger)
49+
50+
Expect(hook.Messages).Should(
51+
SatisfyAll(ContainElement(ContainSubstring("address: ")),
52+
ContainElement(ContainSubstring("username: ")),
53+
ContainElement(ContainSubstring("password: ")),
54+
ContainElement(ContainSubstring("database: ")),
55+
ContainElement(ContainSubstring("required: ")),
56+
ContainElement(ContainSubstring("connectionAttempts: ")),
57+
ContainElement(ContainSubstring("connectionCooldown: "))))
58+
})
59+
})
60+
61+
When("Address is set", func() {
62+
BeforeEach(func() {
63+
c.Address = "localhost:6379"
64+
})
65+
66+
It("should log address", func() {
67+
c.LogConfig(logger)
68+
69+
Expect(hook.Messages).Should(ContainElement(ContainSubstring("address: localhost:6379")))
70+
})
71+
})
72+
73+
When("SentinelAddresses is set", func() {
74+
BeforeEach(func() {
75+
c.SentinelAddresses = []string{"localhost:26379", "localhost:26380"}
76+
})
77+
78+
It("should log sentinel addresses", func() {
79+
c.LogConfig(logger)
80+
81+
Expect(hook.Messages).Should(
82+
SatisfyAll(
83+
ContainElement(ContainSubstring("sentinel:")),
84+
ContainElement(ContainSubstring(" addresses:")),
85+
ContainElement(ContainSubstring(" - localhost:26379")),
86+
ContainElement(ContainSubstring(" - localhost:26380"))))
87+
})
88+
})
89+
})
90+
91+
Describe("obfuscatePassword", func() {
92+
When("password is empty", func() {
93+
It("should return empty string", func() {
94+
Expect(obfuscatePassword("")).Should(Equal(""))
95+
})
96+
})
97+
98+
When("password is not empty", func() {
99+
It("should return obfuscated password", func() {
100+
Expect(obfuscatePassword("test123")).Should(Equal("*******"))
101+
})
102+
})
103+
})
104+
})

0 commit comments

Comments
 (0)