Skip to content

Commit f94eb2e

Browse files
anik120anik120
committed
Tests
1 parent 5cc755d commit f94eb2e

File tree

5 files changed

+773
-397
lines changed

5 files changed

+773
-397
lines changed

‎catalogd/internal/serverutil/serverutil.go

+15-12
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package serverutil
22

33
import (
4-
"context"
54
"crypto/tls"
65
"fmt"
76
"io"
@@ -11,9 +10,9 @@ import (
1110

1211
"github.com/go-logr/logr"
1312
"github.com/gorilla/handlers"
13+
"github.com/klauspost/compress/gzhttp"
1414
ctrl "sigs.k8s.io/controller-runtime"
1515
"sigs.k8s.io/controller-runtime/pkg/certwatcher"
16-
"sigs.k8s.io/controller-runtime/pkg/log"
1716
"sigs.k8s.io/controller-runtime/pkg/manager"
1817

1918
catalogdmetrics "github.com/operator-framework/operator-controller/catalogd/internal/metrics"
@@ -44,20 +43,12 @@ func AddCatalogServerToManager(mgr ctrl.Manager, cfg CatalogServerConfig, tlsFil
4443
}
4544

4645
shutdownTimeout := 30 * time.Second
47-
48-
l := mgr.GetLogger().WithName("catalogd-http-server")
49-
handler := catalogdmetrics.AddMetricsToHandler(cfg.LocalStorage.StorageServerHandler())
50-
handler = logrLoggingHandler(l, handler)
51-
5246
catalogServer := manager.Server{
5347
Name: "catalogs",
5448
OnlyServeWhenLeader: true,
5549
Server: &http.Server{
56-
Addr: cfg.CatalogAddr,
57-
Handler: handler,
58-
BaseContext: func(_ net.Listener) context.Context {
59-
return log.IntoContext(context.Background(), mgr.GetLogger().WithName("http.catalogs"))
60-
},
50+
Addr: cfg.CatalogAddr,
51+
Handler: storageServerHandlerWrapped(mgr, cfg),
6152
ReadTimeout: 5 * time.Second,
6253
// TODO: Revert this to 10 seconds if/when the API
6354
// evolves to have significantly smaller responses
@@ -102,3 +93,15 @@ func logrLoggingHandler(l logr.Logger, handler http.Handler) http.Handler {
10293
l.Info("handled request", "host", host, "username", username, "method", params.Request.Method, "uri", uri, "protocol", params.Request.Proto, "status", params.StatusCode, "size", params.Size)
10394
})
10495
}
96+
97+
func storageServerHandlerWrapped(mgr ctrl.Manager, cfg CatalogServerConfig) http.Handler {
98+
99+
handler := cfg.LocalStorage.StorageServerHandler()
100+
handler = gzhttp.GzipHandler(handler)
101+
handler = catalogdmetrics.AddMetricsToHandler(handler)
102+
103+
l := mgr.GetLogger().WithName("catalogd-http-server")
104+
handler = logrLoggingHandler(l, handler)
105+
return handler
106+
107+
}

‎catalogd/internal/serverutil/serverutil_test.go

+254
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,254 @@
1+
package serverutil
2+
3+
import (
4+
"compress/gzip"
5+
"context"
6+
"io"
7+
"io/fs"
8+
"net/http"
9+
"net/http/httptest"
10+
"testing"
11+
12+
"github.com/stretchr/testify/require"
13+
ctrl "sigs.k8s.io/controller-runtime"
14+
)
15+
16+
func TestStorageServerHandlerWrapped_Gzip(t *testing.T) {
17+
tests := []struct {
18+
name string
19+
acceptEncoding string
20+
responseContent string
21+
expectCompressed bool
22+
expectedStatus int
23+
}{
24+
{
25+
name: "compresses large response when client accepts gzip",
26+
acceptEncoding: "gzip",
27+
responseContent: testCompressableJSON,
28+
expectCompressed: true,
29+
expectedStatus: http.StatusOK,
30+
},
31+
{
32+
name: "does not compress small response even when client accepts gzip",
33+
acceptEncoding: "gzip",
34+
responseContent: `{"foo":"bar"}`,
35+
expectCompressed: false,
36+
expectedStatus: http.StatusOK,
37+
},
38+
{
39+
name: "does not compress when client doesn't accept gzip",
40+
acceptEncoding: "",
41+
responseContent: testCompressableJSON,
42+
expectCompressed: false,
43+
expectedStatus: http.StatusOK,
44+
},
45+
}
46+
47+
for _, tt := range tests {
48+
t.Run(tt.name, func(t *testing.T) {
49+
// Create a mock storage instance that returns our test content
50+
mockStorage := &mockStorageInstance{
51+
content: tt.responseContent,
52+
}
53+
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{})
54+
require.NoError(t, err)
55+
56+
cfg := CatalogServerConfig{
57+
LocalStorage: mockStorage,
58+
}
59+
handler := storageServerHandlerWrapped(mgr, cfg)
60+
61+
// Create test request
62+
req := httptest.NewRequest("GET", "/test", nil)
63+
if tt.acceptEncoding != "" {
64+
req.Header.Set("Accept-Encoding", tt.acceptEncoding)
65+
}
66+
67+
// Create response recorder
68+
rec := httptest.NewRecorder()
69+
70+
// Handle the request
71+
handler.ServeHTTP(rec, req)
72+
73+
// Check status code
74+
require.Equal(t, tt.expectedStatus, rec.Code)
75+
76+
// Check if response was compressed
77+
wasCompressed := rec.Header().Get("Content-Encoding") == "gzip"
78+
require.Equal(t, tt.expectCompressed, wasCompressed)
79+
80+
// Get the response body
81+
var responseBody []byte
82+
if wasCompressed {
83+
// Decompress the response
84+
gzipReader, err := gzip.NewReader(rec.Body)
85+
require.NoError(t, err)
86+
responseBody, err = io.ReadAll(gzipReader)
87+
require.NoError(t, err)
88+
require.NoError(t, gzipReader.Close())
89+
} else {
90+
responseBody = rec.Body.Bytes()
91+
}
92+
93+
// Verify the response content
94+
require.Equal(t, tt.responseContent, string(responseBody))
95+
})
96+
}
97+
}
98+
99+
const testCompressableJSON = `{
100+
"defaultChannel": "stable-v6.x",
101+
"name": "cockroachdb",
102+
"schema": "olm.package"
103+
}
104+
{
105+
"entries": [
106+
{
107+
"name": "cockroachdb.v5.0.3"
108+
},
109+
{
110+
"name": "cockroachdb.v5.0.4",
111+
"replaces": "cockroachdb.v5.0.3"
112+
}
113+
],
114+
"name": "stable-5.x",
115+
"package": "cockroachdb",
116+
"schema": "olm.channel"
117+
}
118+
{
119+
"entries": [
120+
{
121+
"name": "cockroachdb.v6.0.0",
122+
"skipRange": "<6.0.0"
123+
}
124+
],
125+
"name": "stable-v6.x",
126+
"package": "cockroachdb",
127+
"schema": "olm.channel"
128+
}
129+
{
130+
"image": "quay.io/openshift-community-operators/cockroachdb@sha256:a5d4f4467250074216eb1ba1c36e06a3ab797d81c431427fc2aca97ecaf4e9d8",
131+
"name": "cockroachdb.v5.0.3",
132+
"package": "cockroachdb",
133+
"properties": [
134+
{
135+
"type": "olm.gvk",
136+
"value": {
137+
"group": "charts.operatorhub.io",
138+
"kind": "Cockroachdb",
139+
"version": "v1alpha1"
140+
}
141+
},
142+
{
143+
"type": "olm.package",
144+
"value": {
145+
"packageName": "cockroachdb",
146+
"version": "5.0.3"
147+
}
148+
}
149+
],
150+
"relatedImages": [
151+
{
152+
"name": "",
153+
"image": "quay.io/helmoperators/cockroachdb:v5.0.3"
154+
},
155+
{
156+
"name": "",
157+
"image": "quay.io/openshift-community-operators/cockroachdb@sha256:a5d4f4467250074216eb1ba1c36e06a3ab797d81c431427fc2aca97ecaf4e9d8"
158+
}
159+
],
160+
"schema": "olm.bundle"
161+
}
162+
{
163+
"image": "quay.io/openshift-community-operators/cockroachdb@sha256:f42337e7b85a46d83c94694638e2312e10ca16a03542399a65ba783c94a32b63",
164+
"name": "cockroachdb.v5.0.4",
165+
"package": "cockroachdb",
166+
"properties": [
167+
{
168+
"type": "olm.gvk",
169+
"value": {
170+
"group": "charts.operatorhub.io",
171+
"kind": "Cockroachdb",
172+
"version": "v1alpha1"
173+
}
174+
},
175+
{
176+
"type": "olm.package",
177+
"value": {
178+
"packageName": "cockroachdb",
179+
"version": "5.0.4"
180+
}
181+
}
182+
],
183+
"relatedImages": [
184+
{
185+
"name": "",
186+
"image": "quay.io/helmoperators/cockroachdb:v5.0.4"
187+
},
188+
{
189+
"name": "",
190+
"image": "quay.io/openshift-community-operators/cockroachdb@sha256:f42337e7b85a46d83c94694638e2312e10ca16a03542399a65ba783c94a32b63"
191+
}
192+
],
193+
"schema": "olm.bundle"
194+
}
195+
{
196+
"image": "quay.io/openshift-community-operators/cockroachdb@sha256:d3016b1507515fc7712f9c47fd9082baf9ccb070aaab58ed0ef6e5abdedde8ba",
197+
"name": "cockroachdb.v6.0.0",
198+
"package": "cockroachdb",
199+
"properties": [
200+
{
201+
"type": "olm.gvk",
202+
"value": {
203+
"group": "charts.operatorhub.io",
204+
"kind": "Cockroachdb",
205+
"version": "v1alpha1"
206+
}
207+
},
208+
{
209+
"type": "olm.package",
210+
"value": {
211+
"packageName": "cockroachdb",
212+
"version": "6.0.0"
213+
}
214+
}
215+
],
216+
"relatedImages": [
217+
{
218+
"name": "",
219+
"image": "quay.io/cockroachdb/cockroach-helm-operator:6.0.0"
220+
},
221+
{
222+
"name": "",
223+
"image": "quay.io/openshift-community-operators/cockroachdb@sha256:d3016b1507515fc7712f9c47fd9082baf9ccb070aaab58ed0ef6e5abdedde8ba"
224+
}
225+
],
226+
"schema": "olm.bundle"
227+
}
228+
`
229+
230+
// mockStorageInstance implements storage.Instance interface for testing
231+
type mockStorageInstance struct {
232+
content string
233+
}
234+
235+
func (m *mockStorageInstance) StorageServerHandler() http.Handler {
236+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
237+
w.Write([]byte(m.content))
238+
})
239+
}
240+
241+
func (m *mockStorageInstance) Store(ctx context.Context, catalogName string, fs fs.FS) error {
242+
return nil
243+
}
244+
245+
func (m *mockStorageInstance) Delete(catalogName string) error {
246+
return nil
247+
}
248+
249+
func (m *mockStorageInstance) ContentExists(catalog string) bool {
250+
return true
251+
}
252+
func (m *mockStorageInstance) BaseURL(catalog string) string {
253+
return ""
254+
}

‎catalogd/internal/storage/localdir.go

+13-2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ type LocalDirV1 struct {
3434
sf singleflight.Group
3535
}
3636

37+
var (
38+
_ Instance = &LocalDirV1{}
39+
ErrInvalidParams = errors.New("invalid parameters")
40+
)
41+
3742
func (s *LocalDirV1) Store(ctx context.Context, catalog string, fsys fs.FS) error {
3843
s.m.Lock()
3944
defer s.m.Unlock()
@@ -56,9 +61,13 @@ func (s *LocalDirV1) Store(ctx context.Context, catalog string, fsys fs.FS) erro
5661
eg, egCtx = errgroup.WithContext(ctx)
5762
metaChans []chan *declcfg.Meta
5863
)
59-
for i, f := range storeMetaFuncs {
64+
for range storeMetaFuncs {
6065
metaChans = append(metaChans, make(chan *declcfg.Meta, 1))
61-
eg.Go(func() error { return f(tmpCatalogDir, metaChans[i]) })
66+
}
67+
for i, f := range storeMetaFuncs {
68+
eg.Go(func() error {
69+
return f(tmpCatalogDir, metaChans[i])
70+
})
6271
}
6372
err = declcfg.WalkMetasFS(egCtx, fsys, func(path string, meta *declcfg.Meta, err error) error {
6473
if err != nil {
@@ -249,6 +258,8 @@ func httpError(w http.ResponseWriter, err error) {
249258
code = http.StatusNotFound
250259
case errors.Is(err, fs.ErrPermission):
251260
code = http.StatusForbidden
261+
case errors.Is(err, ErrInvalidParams):
262+
code = http.StatusBadRequest
252263
default:
253264
code = http.StatusInternalServerError
254265
}

0 commit comments

Comments
 (0)