Skip to content

Commit

Permalink
Refactoring: FileDownloader (#1281)
Browse files Browse the repository at this point in the history 
* enabled containedctx & contextcheck

* enabled noctx

* less background context

* context metrics test

* use ginkgo context instead of background

* fix redis e2e tests

* made downloader context aware
  • Loading branch information
@kwitsch
kwitsch authored Nov 29, 2023
1 parent 976d619 commit 3378316
Show file tree
Hide file tree
Showing 15 changed files with 235 additions and 215 deletions.
3 changes: 2 additions & 1 deletion .golangci.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ linters:
- whitespace
- wsl
- ginkgolinter
disable:
- noctx
- containedctx
- contextcheck
disable:
- scopelint
- structcheck
- deadcode
Expand Down
60 changes: 30 additions & 30 deletions e2e/basic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ var _ = Describe("Basic functional tests", func() {
var err error

Describe("Container start", func() {
BeforeEach(func() {
moka, err = createDNSMokkaContainer("moka1", `A google/NOERROR("A 1.2.3.4 123")`)
BeforeEach(func(ctx context.Context) {
moka, err = createDNSMokkaContainer(ctx, "moka1", `A google/NOERROR("A 1.2.3.4 123")`)

Expect(err).Should(Succeed())
DeferCleanup(moka.Terminate)
})
When("wrong port configuration is provided", func() {
BeforeEach(func() {
blocky, err = createBlockyContainer(tmpDir,
BeforeEach(func(ctx context.Context) {
blocky, err = createBlockyContainer(ctx, tmpDir,
"upstreams:",
" groups:",
" default:",
Expand All @@ -38,22 +38,22 @@ var _ = Describe("Basic functional tests", func() {
Expect(err).Should(HaveOccurred())

// check container exit status
state, err := blocky.State(context.Background())
state, err := blocky.State(ctx)
Expect(err).Should(Succeed())
Expect(state.ExitCode).Should(Equal(1))

DeferCleanup(blocky.Terminate)
})
It("should fail to start", func() {
It("should fail to start", func(ctx context.Context) {
Eventually(blocky.IsRunning, "5s", "2ms").Should(BeFalse())

Expect(getContainerLogs(blocky)).
Expect(getContainerLogs(ctx, blocky)).
Should(ContainElement(ContainSubstring("address already in use")))
})
})
When("Minimal configuration is provided", func() {
BeforeEach(func() {
blocky, err = createBlockyContainer(tmpDir,
BeforeEach(func(ctx context.Context) {
blocky, err = createBlockyContainer(ctx, tmpDir,
"upstreams:",
" groups:",
" default:",
Expand All @@ -63,19 +63,19 @@ var _ = Describe("Basic functional tests", func() {
Expect(err).Should(Succeed())
DeferCleanup(blocky.Terminate)
})
It("Should start and answer DNS queries", func() {
It("Should start and answer DNS queries", func(ctx context.Context) {
msg := util.NewMsgWithQuestion("google.de.", A)

Expect(doDNSRequest(blocky, msg)).
Expect(doDNSRequest(ctx, blocky, msg)).
Should(
SatisfyAll(
BeDNSRecord("google.de.", A, "1.2.3.4"),
HaveTTL(BeNumerically("==", 123)),
))
})
It("should return 'healthy' container status (healthcheck)", func() {
It("should return 'healthy' container status (healthcheck)", func(ctx context.Context) {
Eventually(func(g Gomega) string {
state, err := blocky.State(context.Background())
state, err := blocky.State(ctx)
g.Expect(err).NotTo(HaveOccurred())

return state.Health.Status
Expand All @@ -84,8 +84,8 @@ var _ = Describe("Basic functional tests", func() {
})
Context("http port configuration", func() {
When("'httpPort' is not defined", func() {
BeforeEach(func() {
blocky, err = createBlockyContainer(tmpDir,
BeforeEach(func(ctx context.Context) {
blocky, err = createBlockyContainer(ctx, tmpDir,
"upstreams:",
" groups:",
" default:",
Expand All @@ -96,17 +96,17 @@ var _ = Describe("Basic functional tests", func() {
DeferCleanup(blocky.Terminate)
})

It("should not open http port", func() {
host, port, err := getContainerHostPort(blocky, "4000/tcp")
It("should not open http port", func(ctx context.Context) {
host, port, err := getContainerHostPort(ctx, blocky, "4000/tcp")
Expect(err).Should(Succeed())

_, err = http.Get(fmt.Sprintf("http://%s", net.JoinHostPort(host, port)))
Expect(err).Should(HaveOccurred())
})
})
When("'httpPort' is defined", func() {
BeforeEach(func() {
blocky, err = createBlockyContainer(tmpDir,
BeforeEach(func(ctx context.Context) {
blocky, err = createBlockyContainer(ctx, tmpDir,
"upstreams:",
" groups:",
" default:",
Expand All @@ -118,8 +118,8 @@ var _ = Describe("Basic functional tests", func() {
Expect(err).Should(Succeed())
DeferCleanup(blocky.Terminate)
})
It("should serve http content", func() {
host, port, err := getContainerHostPort(blocky, "4000/tcp")
It("should serve http content", func(ctx context.Context) {
host, port, err := getContainerHostPort(ctx, blocky, "4000/tcp")
Expect(err).Should(Succeed())
url := fmt.Sprintf("http://%s", net.JoinHostPort(host, port))

Expand All @@ -142,15 +142,15 @@ var _ = Describe("Basic functional tests", func() {
})

Describe("Logging", func() {
BeforeEach(func() {
moka, err = createDNSMokkaContainer("moka1", `A google/NOERROR("A 1.2.3.4 123")`)
BeforeEach(func(ctx context.Context) {
moka, err = createDNSMokkaContainer(ctx, "moka1", `A google/NOERROR("A 1.2.3.4 123")`)

Expect(err).Should(Succeed())
DeferCleanup(moka.Terminate)
})
When("log privacy is enabled", func() {
BeforeEach(func() {
blocky, err = createBlockyContainer(tmpDir,
BeforeEach(func(ctx context.Context) {
blocky, err = createBlockyContainer(ctx, tmpDir,
"upstreams:",
" groups:",
" default:",
Expand All @@ -162,27 +162,27 @@ var _ = Describe("Basic functional tests", func() {
Expect(err).Should(Succeed())
DeferCleanup(blocky.Terminate)
})
It("should not log answers and questions", func() {
It("should not log answers and questions", func(ctx context.Context) {
msg := util.NewMsgWithQuestion("google.com.", A)

// do 2 requests

Expect(doDNSRequest(blocky, msg)).
Expect(doDNSRequest(ctx, blocky, msg)).
Should(
SatisfyAll(
BeDNSRecord("google.com.", A, "1.2.3.4"),
HaveTTL(BeNumerically("==", 123)),
))

Expect(doDNSRequest(blocky, msg)).
Expect(doDNSRequest(ctx, blocky, msg)).
Should(
SatisfyAll(
BeDNSRecord("google.com.", A, "1.2.3.4"),
HaveTTL(BeNumerically("<=", 123)),
))

Expect(getContainerLogs(blocky)).Should(Not(ContainElement(ContainSubstring("google.com"))))
Expect(getContainerLogs(blocky)).Should(Not(ContainElement(ContainSubstring("1.2.3.4"))))
Expect(getContainerLogs(ctx, blocky)).Should(Not(ContainElement(ContainSubstring("google.com"))))
Expect(getContainerLogs(ctx, blocky)).Should(Not(ContainElement(ContainSubstring("1.2.3.4"))))
})
})
})
Expand Down
36 changes: 18 additions & 18 deletions e2e/blocking_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ import (
var _ = Describe("External lists and query blocking", func() {
var blocky, httpServer, moka testcontainers.Container
var err error
BeforeEach(func() {
moka, err = createDNSMokkaContainer("moka", `A google/NOERROR("A 1.2.3.4 123")`)
BeforeEach(func(ctx context.Context) {
moka, err = createDNSMokkaContainer(ctx, "moka", `A google/NOERROR("A 1.2.3.4 123")`)

Expect(err).Should(Succeed())
DeferCleanup(moka.Terminate)
})
Describe("List download on startup", func() {
When("external blacklist ist not available", func() {
Context("loading.strategy = blocking", func() {
BeforeEach(func() {
blocky, err = createBlockyContainer(tmpDir,
BeforeEach(func(ctx context.Context) {
blocky, err = createBlockyContainer(ctx, tmpDir,
"log:",
" level: warn",
"upstreams:",
Expand All @@ -45,22 +45,22 @@ var _ = Describe("External lists and query blocking", func() {
DeferCleanup(blocky.Terminate)
})

It("should start with warning in log work without errors", func() {
It("should start with warning in log work without errors", func(ctx context.Context) {
msg := util.NewMsgWithQuestion("google.com.", A)

Expect(doDNSRequest(blocky, msg)).
Expect(doDNSRequest(ctx, blocky, msg)).
Should(
SatisfyAll(
BeDNSRecord("google.com.", A, "1.2.3.4"),
HaveTTL(BeNumerically("==", 123)),
))

Expect(getContainerLogs(blocky)).Should(ContainElement(ContainSubstring("cannot open source: ")))
Expect(getContainerLogs(ctx, blocky)).Should(ContainElement(ContainSubstring("cannot open source: ")))
})
})
Context("loading.strategy = failOnError", func() {
BeforeEach(func() {
blocky, err = createBlockyContainer(tmpDir,
BeforeEach(func(ctx context.Context) {
blocky, err = createBlockyContainer(ctx, tmpDir,
"log:",
" level: warn",
"upstreams:",
Expand All @@ -81,31 +81,31 @@ var _ = Describe("External lists and query blocking", func() {
Expect(err).Should(HaveOccurred())

// check container exit status
state, err := blocky.State(context.Background())
state, err := blocky.State(ctx)
Expect(err).Should(Succeed())
Expect(state.ExitCode).Should(Equal(1))

DeferCleanup(blocky.Terminate)
})

It("should fail to start", func() {
It("should fail to start", func(ctx context.Context) {
Eventually(blocky.IsRunning, "5s", "2ms").Should(BeFalse())

Expect(getContainerLogs(blocky)).
Expect(getContainerLogs(ctx, blocky)).
Should(ContainElement(ContainSubstring("Error: can't start server: 1 error occurred")))
})
})
})
})
Describe("Query blocking against external blacklists", func() {
When("external blacklists are defined and available", func() {
BeforeEach(func() {
httpServer, err = createHTTPServerContainer("httpserver", tmpDir, "list.txt", "blockeddomain.com")
BeforeEach(func(ctx context.Context) {
httpServer, err = createHTTPServerContainer(ctx, "httpserver", tmpDir, "list.txt", "blockeddomain.com")

Expect(err).Should(Succeed())
DeferCleanup(httpServer.Terminate)

blocky, err = createBlockyContainer(tmpDir,
blocky, err = createBlockyContainer(ctx, tmpDir,
"log:",
" level: warn",
"upstreams:",
Expand All @@ -124,17 +124,17 @@ var _ = Describe("External lists and query blocking", func() {
Expect(err).Should(Succeed())
DeferCleanup(blocky.Terminate)
})
It("should download external list on startup and block queries", func() {
It("should download external list on startup and block queries", func(ctx context.Context) {
msg := util.NewMsgWithQuestion("blockeddomain.com.", A)

Expect(doDNSRequest(blocky, msg)).
Expect(doDNSRequest(ctx, blocky, msg)).
Should(
SatisfyAll(
BeDNSRecord("blockeddomain.com.", A, "0.0.0.0"),
HaveTTL(BeNumerically("==", 6*60*60)),
))

Expect(getContainerLogs(blocky)).Should(BeEmpty())
Expect(getContainerLogs(ctx, blocky)).Should(BeEmpty())
})
})
})
Expand Down
Loading

0 comments on commit 3378316

Please sign in to comment.