Skip to content

Commit b0b2616

Browse files
chengfangimmanuwellclaude
authored
fix(test): correct assert.Equal arg order and wrong webhook type in quay test (#1663) (#1687); fix(webhook-quay): Self-hosted Quay images redirect to quay.io, not self-hosted URL (#1683) (#1689) (#1697)
Signed-off-by: immanuwell <pchpr.00@list.ru> Co-authored-by: Immanuel Tikhonov <122638311+immanuwell@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7a27e11 commit b0b2616

3 files changed

Lines changed: 146 additions & 29 deletions

File tree

‎pkg/webhook/quay.go‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"fmt"
66
"io"
77
"net/http"
8+
"net/url"
9+
"strings"
810

911
"github.com/argoproj-labs/argocd-image-updater/pkg/argocd"
1012
)
@@ -75,9 +77,32 @@ func (q *QuayWebhook) Parse(r *http.Request) (*argocd.WebhookEvent, error) {
7577
}
7678
tag = payload.UpdatedTags[0]
7779

80+
registryURL := "quay.io"
81+
if payload.DockerUrl != "" {
82+
registryURL = extractRegistryFromDockerURL(payload.DockerUrl)
83+
}
84+
7885
return &argocd.WebhookEvent{
79-
RegistryURL: "quay.io",
86+
RegistryURL: registryURL,
8087
Repository: payload.Repository,
8188
Tag: tag,
8289
}, nil
8390
}
91+
92+
// extractRegistryFromDockerURL extracts the registry hostname from a docker_url
93+
// field such as "quay.apps.example.com/namespace/repo"
94+
func extractRegistryFromDockerURL(dockerURL string) string {
95+
raw := dockerURL
96+
if !strings.HasPrefix(dockerURL, "http://") && !strings.HasPrefix(dockerURL, "https://") {
97+
dockerURL = "https://" + dockerURL
98+
}
99+
if parsed, err := url.Parse(dockerURL); err == nil && parsed.Host != "" {
100+
return parsed.Host
101+
}
102+
// Fallback: try to extract host manually by splitting on the first '/'
103+
parts := strings.Split(raw, "/")
104+
if len(parts) > 0 && strings.Contains(parts[0], ".") {
105+
return parts[0]
106+
}
107+
return "quay.io"
108+
}

‎pkg/webhook/quay_test.go‎

Lines changed: 110 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,18 @@ import (
1010

1111
func TestNewQuayWebhook(t *testing.T) {
1212
secret := "test"
13-
webhook := NewDockerHubWebhook(secret)
13+
webhook := NewQuayWebhook(secret)
1414

1515
assert.NotNil(t, webhook, "webhook was nil")
16-
assert.Equal(t, webhook.secret, secret, "Secret is not the same expected %s but got %s", secret, webhook.secret)
16+
assert.Equal(t, secret, webhook.secret, "Secret is not the same expected %s but got %s", secret, webhook.secret)
1717
}
1818

1919
func TestQuayWebhook_GetRegistryType(t *testing.T) {
2020
webhook := NewQuayWebhook("")
2121
registryType := webhook.GetRegistryType()
2222

2323
assert.NotNil(t, webhook, "Webhook was nil")
24-
assert.Equal(t, registryType, "quay.io", "Registry type is not quay.io got: %s", registryType)
24+
assert.Equal(t, "quay.io", registryType, "Registry type is not quay.io got: %s", registryType)
2525
}
2626

2727
func TestQuayWebhook_Validate(t *testing.T) {
@@ -92,14 +92,15 @@ func TestQuayWebhook_Parse(t *testing.T) {
9292
webhook := NewQuayWebhook("")
9393

9494
tests := []struct {
95-
name string
96-
payload string
97-
expectedRepo string
98-
expectedTag string
99-
expectError bool
95+
name string
96+
payload string
97+
expectedRegistry string
98+
expectedRepo string
99+
expectedTag string
100+
expectError bool
100101
}{
101102
{
102-
name: "valid payload",
103+
name: "valid payload with quay.io docker_url",
103104
payload: `{
104105
"name": "repository",
105106
"repository": "mynamespace/repository",
@@ -110,9 +111,75 @@ func TestQuayWebhook_Parse(t *testing.T) {
110111
"latest"
111112
]
112113
}`,
113-
expectedRepo: "mynamespace/repository",
114-
expectedTag: "latest",
115-
expectError: false,
114+
expectedRegistry: "quay.io",
115+
expectedRepo: "mynamespace/repository",
116+
expectedTag: "latest",
117+
expectError: false,
118+
},
119+
{
120+
name: "private quay registry in docker_url",
121+
payload: `{
122+
"name": "repository",
123+
"repository": "mynamespace/repository",
124+
"namespace": "mynamespace",
125+
"docker_url": "quay.apps.example.com/mynamespace/repository",
126+
"homepage": "https://quay.apps.example.com/repository/mynamespace/repository",
127+
"updated_tags": [
128+
"dev"
129+
]
130+
}`,
131+
expectedRegistry: "quay.apps.example.com",
132+
expectedRepo: "mynamespace/repository",
133+
expectedTag: "dev",
134+
expectError: false,
135+
},
136+
{
137+
name: "private quay registry with https scheme in docker_url",
138+
payload: `{
139+
"name": "repository",
140+
"repository": "ariss/alrl",
141+
"namespace": "ariss",
142+
"docker_url": "https://quay.internal.corp/ariss/alrl",
143+
"homepage": "https://quay.internal.corp/repository/ariss/alrl",
144+
"updated_tags": [
145+
"v1.0"
146+
]
147+
}`,
148+
expectedRegistry: "quay.internal.corp",
149+
expectedRepo: "ariss/alrl",
150+
expectedTag: "v1.0",
151+
expectError: false,
152+
},
153+
{
154+
name: "empty docker_url falls back to quay.io",
155+
payload: `{
156+
"name": "repository",
157+
"repository": "mynamespace/repository",
158+
"namespace": "mynamespace",
159+
"docker_url": "",
160+
"updated_tags": [
161+
"latest"
162+
]
163+
}`,
164+
expectedRegistry: "quay.io",
165+
expectedRepo: "mynamespace/repository",
166+
expectedTag: "latest",
167+
expectError: false,
168+
},
169+
{
170+
name: "missing docker_url falls back to quay.io",
171+
payload: `{
172+
"name": "repository",
173+
"repository": "mynamespace/repository",
174+
"namespace": "mynamespace",
175+
"updated_tags": [
176+
"latest"
177+
]
178+
}`,
179+
expectedRegistry: "quay.io",
180+
expectedRepo: "mynamespace/repository",
181+
expectedTag: "latest",
182+
expectError: false,
116183
},
117184
{
118185
name: "valid payload with multiple tags",
@@ -127,9 +194,10 @@ func TestQuayWebhook_Parse(t *testing.T) {
127194
"v1.0"
128195
]
129196
}`,
130-
expectedRepo: "mynamespace/repository",
131-
expectedTag: "latest",
132-
expectError: false,
197+
expectedRegistry: "quay.io",
198+
expectedRepo: "mynamespace/repository",
199+
expectedTag: "latest",
200+
expectError: false,
133201
},
134202
{
135203
name: "valid payload with no tags",
@@ -186,9 +254,33 @@ func TestQuayWebhook_Parse(t *testing.T) {
186254
}
187255

188256
assert.NotNil(t, event, "Event was nil")
189-
assert.Equal(t, event.RegistryURL, "quay.io", "Expected repository url to be %s, but got %s", "quay.io", event.RegistryURL)
190-
assert.Equal(t, event.Repository, tt.expectedRepo, "Expect repository to be %s, but got %s", tt.expectedRepo, event.Repository)
191-
assert.Equal(t, event.Tag, tt.expectedTag, "Expected tag to be %s, but got %s", tt.expectedTag, event.Tag)
257+
assert.Equal(t, tt.expectedRegistry, event.RegistryURL)
258+
assert.Equal(t, tt.expectedRepo, event.Repository)
259+
assert.Equal(t, tt.expectedTag, event.Tag)
260+
})
261+
}
262+
}
263+
264+
func TestExtractRegistryFromDockerURL(t *testing.T) {
265+
tests := []struct {
266+
name string
267+
input string
268+
expected string
269+
}{
270+
{"public quay.io", "quay.io/ns/repo", "quay.io"},
271+
{"private hostname", "quay.apps.example.com/ns/repo", "quay.apps.example.com"},
272+
{"with https scheme", "https://quay.internal.corp/ns/repo", "quay.internal.corp"},
273+
{"with http scheme", "http://quay.internal.corp/ns/repo", "quay.internal.corp"},
274+
{"hostname with port", "quay.internal.corp:8443/ns/repo", "quay.internal.corp:8443"},
275+
{"bare hostname no path", "quay.apps.example.com", "quay.apps.example.com"},
276+
{"manual split fallback", "quay.custom.io:bad:port/ns/repo", "quay.custom.io:bad:port"},
277+
{"unparseable url falls back to quay.io", "/ns/repo", "quay.io"},
278+
}
279+
280+
for _, tt := range tests {
281+
t.Run(tt.name, func(t *testing.T) {
282+
result := extractRegistryFromDockerURL(tt.input)
283+
assert.Equal(t, tt.expected, result)
192284
})
193285
}
194286
}

‎pkg/webhook/server_test.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func testEndpointConnectivity(t *testing.T, url string, expectedStatus int) {
7878

7979
res, err := client.Get(url)
8080
if res != nil {
81-
assert.Equal(t, res.StatusCode, expectedStatus, "Did not receive the expected status of %d got: %d", expectedStatus, res.StatusCode)
81+
assert.Equal(t, expectedStatus, res.StatusCode, "Did not receive the expected status of %d got: %d", expectedStatus, res.StatusCode)
8282
defer res.Body.Close()
8383
}
8484
assert.NotNil(t, res, "No body received so server is not alive")
@@ -92,8 +92,8 @@ func TestNewWebhookServer(t *testing.T) {
9292
server := NewWebhookServer(8080, handler, reconciler)
9393

9494
assert.NotNil(t, server, "Server was nil")
95-
assert.Equal(t, server.Port, 8080, "Port is not 8080 got %d", server.Port)
96-
assert.Equal(t, server.Handler, handler, "Handler is not equal")
95+
assert.Equal(t, 8080, server.Port, "Port is not 8080 got %d", server.Port)
96+
assert.Equal(t, handler, server.Handler, "Handler is not equal")
9797
assert.NotNil(t, server.Reconciler, "Reconciler was nil")
9898

9999
}
@@ -166,8 +166,8 @@ func TestWebhookServerHandleHealth(t *testing.T) {
166166
body, err := io.ReadAll(res.Body)
167167
assert.NoError(t, err, "Error while parsing body")
168168

169-
assert.Equal(t, res.StatusCode, http.StatusOK, "Did not receive the correct status code got: %d", res.StatusCode)
170-
assert.Equal(t, string(body), "OK", "Did not receive the correct health message")
169+
assert.Equal(t, http.StatusOK, res.StatusCode, "Did not receive the correct status code got: %d", res.StatusCode)
170+
assert.Equal(t, "OK", string(body), "Did not receive the correct health message")
171171
}
172172

173173
// TestWebhookServerHealthEndpoint ensures that the health endpoint of the server is working properly
@@ -195,8 +195,8 @@ func TestWebhookServerHealthEndpoint(t *testing.T) {
195195

196196
body, err := io.ReadAll(res.Body)
197197
assert.NoError(t, err)
198-
assert.Equal(t, string(body), "OK", "Did not receive 'OK' got: %s", string(body))
199-
assert.Equal(t, res.StatusCode, http.StatusOK, "Did not receive status 200 got: %d", res.StatusCode)
198+
assert.Equal(t, "OK", string(body), "Did not receive 'OK' got: %s", string(body))
199+
assert.Equal(t, http.StatusOK, res.StatusCode, "Did not receive status 200 got: %d", res.StatusCode)
200200
}
201201
}
202202

@@ -248,7 +248,7 @@ func TestWebhookServerHandleWebhook(t *testing.T) {
248248
res := rec.Result()
249249
defer res.Body.Close()
250250

251-
assert.Equal(t, res.StatusCode, tt.expectedStatus, "Did not receive ok status")
251+
assert.Equal(t, tt.expectedStatus, res.StatusCode, "Did not receive ok status")
252252
})
253253
}
254254

@@ -317,7 +317,7 @@ func TestWebhookServerWebhookEndpoint(t *testing.T) {
317317
defer res.Body.Close()
318318

319319
assert.NoError(t, err)
320-
assert.Equal(t, res.StatusCode, http.StatusOK, "Did not receive status 200 got: %d", res.StatusCode)
320+
assert.Equal(t, http.StatusOK, res.StatusCode, "Did not receive status 200 got: %d", res.StatusCode)
321321
}
322322

323323
body2 := `{}`
@@ -329,7 +329,7 @@ func TestWebhookServerWebhookEndpoint(t *testing.T) {
329329
defer res2.Body.Close()
330330

331331
assert.NoError(t, err)
332-
assert.Equal(t, res2.StatusCode, http.StatusBadRequest, "Did not receive status 400 got: %d", res.StatusCode)
332+
assert.Equal(t, http.StatusBadRequest, res2.StatusCode, "Did not receive status 400 got: %d", res2.StatusCode)
333333
}
334334
}
335335

0 commit comments

Comments
 (0)