From ac8be0a1056b343fa6ae69d49338d22070e2f3bc Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Thu, 13 Aug 2026 08:26:17 +1000 Subject: [PATCH] remotes: add mirrorlist for round-robin + failover across mirrors (rpm/deb/apk) OS package remotes (rpm/deb/apk) fetch many small files and benefit from spreading upstream load across mirrors and surviving a mirror outage. A remote may now set a `mirrorlist` of additional upstream base URLs; the effective upstream pool is [base_url] + mirrorlist, which the shared proxy engine load-balances round-robin and, on a network error/timeout/5xx, fails over to the next mirror before returning an error. Because selection happens in the engine, it works for every provider that reaches upstream. Backward compatible: `base_url` stays a plain string that providers read unchanged, and a remote with no mirrorlist behaves exactly as today (single attempt, same error path). - add models.Remote.Mirrorlist ([]string, json "mirrorlist,omitempty") and UpstreamPool() = [base_url] + mirrorlist; ValidateMirrorlist enforces remote repo_type + package_type in {rpm, deb, alpine} and http/https URLs - v2 create/update: reject a mirrorlist on any other repo (400); base_url remains required for remotes - persist mirrorlist in a new additive `mirrorlist TEXT[]` column (remoteCols/scanRemote/CreateRemote/UpdateRemote); base_url column unchanged - engine: per-remote round-robin cursor over the pool; wrap the fetch/head/revalidate upstream calls in a failover loop that narrows the remote to one selected mirror per attempt; only network errors and 5xx fail over (404/403/... return as-is); circuit breaker stays keyed per remote and trips only after all mirrors fail - tests: model JSON round-trip + validation gating, engine round-robin/failover/no-mirrorlist-unchanged, DB mirrorlist round-trip, and a docker acceptance suite (round-robin across two mock upstreams, failover past a dead primary, no-mirrorlist regression, and a real dnf makecache+install through a two-mirror rpm remote whose base_url is dead) Least-connections and a per-remote strategy selector are a follow-up PR. --- docker-compose.e2e.yml | 16 ++ e2e-docker/README.md | 6 + .../Packages/e2e-testpkg-1.0-1.noarch.rpm | Bin 0 -> 6838 bytes ...7d3c31a2579ea95e31a4c3a320d85-other.xml.gz | Bin 0 -> 296 bytes ...ce173fd7a589d647918fd7da4-other.sqlite.bz2 | Bin 0 -> 738 bytes ...c5f217f5b87144f465e57-filelists.sqlite.bz2 | Bin 0 -> 764 bytes ...0d75734e50d0f8afdcbc6261b08-primary.xml.gz | Bin 0 -> 631 bytes ...f436ef239fa46827f966222be-filelists.xml.gz | Bin 0 -> 256 bytes ...21338ef740dc35eb85b9b71-primary.sqlite.bz2 | Bin 0 -> 1740 bytes .../fixtures/rpm-mirror/repodata/repomd.xml | 55 +++++ e2e-docker/mirror-conf/a.conf | 9 + e2e-docker/mirror-conf/b.conf | 8 + e2e-docker/multibaseurl_test.go | 163 +++++++++++++++ internal/api/v2/remotes.go | 8 + internal/database/database_test.go | 33 +++ internal/database/postgres.go | 2 + internal/database/remotes.go | 13 +- internal/proxy/engine.go | 119 +++++++++++ internal/proxy/multibaseurl_test.go | 188 ++++++++++++++++++ pkg/models/remote.go | 56 +++++- pkg/models/remote_test.go | 84 +++++++- scripts/docker-e2e.sh | 16 +- 22 files changed, 763 insertions(+), 13 deletions(-) create mode 100644 e2e-docker/fixtures/rpm-mirror/Packages/e2e-testpkg-1.0-1.noarch.rpm create mode 100644 e2e-docker/fixtures/rpm-mirror/repodata/8510c74a6f288828bbc92abee5d0d8ae9687d3c31a2579ea95e31a4c3a320d85-other.xml.gz create mode 100644 e2e-docker/fixtures/rpm-mirror/repodata/ba593cd8ab5ec1e127888707c1fd882920996f1ce173fd7a589d647918fd7da4-other.sqlite.bz2 create mode 100644 e2e-docker/fixtures/rpm-mirror/repodata/be3c6e4c7a13ece48bd5d6a4d6d5e6a2395fe006ef9c5f217f5b87144f465e57-filelists.sqlite.bz2 create mode 100644 e2e-docker/fixtures/rpm-mirror/repodata/d82f717e4da1afe96b8e7857de9e852f5785c0d75734e50d0f8afdcbc6261b08-primary.xml.gz create mode 100644 e2e-docker/fixtures/rpm-mirror/repodata/daa313cc5eeb7df556e1d4885d7701b10b9f012f436ef239fa46827f966222be-filelists.xml.gz create mode 100644 e2e-docker/fixtures/rpm-mirror/repodata/f6bd7755da13d9726381048f467992869104a4c5521338ef740dc35eb85b9b71-primary.sqlite.bz2 create mode 100644 e2e-docker/fixtures/rpm-mirror/repodata/repomd.xml create mode 100644 e2e-docker/mirror-conf/a.conf create mode 100644 e2e-docker/mirror-conf/b.conf create mode 100644 e2e-docker/multibaseurl_test.go create mode 100644 internal/proxy/multibaseurl_test.go diff --git a/docker-compose.e2e.yml b/docker-compose.e2e.yml index 8bfa2d6..9590563 100644 --- a/docker-compose.e2e.yml +++ b/docker-compose.e2e.yml @@ -9,6 +9,18 @@ services: # No host port needed: only the artifactapi container talks to it, and the # tests compare served bytes against the on-disk fixtures. + # Two constant-body upstreams for the multi-base_url suite: each returns a + # distinct, upstream-identifying body for any path, so round-robin + # distribution across a two-mirror remote is directly observable. + mockupstreama: + image: nginx:alpine + volumes: + - ./e2e-docker/mirror-conf/a.conf:/etc/nginx/conf.d/default.conf:ro,z + mockupstreamb: + image: nginx:alpine + volumes: + - ./e2e-docker/mirror-conf/b.conf:/etc/nginx/conf.d/default.conf:ro,z + artifactapi: # The host port is set via ARTIFACTAPI_PORT (see scripts/docker-e2e.sh), # defaulting to 8000; the e2e run uses 8001 to avoid colliding with a @@ -16,3 +28,7 @@ services: depends_on: mockupstream: condition: service_started + mockupstreama: + condition: service_started + mockupstreamb: + condition: service_started diff --git a/e2e-docker/README.md b/e2e-docker/README.md index 3a7773a..f6ab089 100644 --- a/e2e-docker/README.md +++ b/e2e-docker/README.md @@ -30,6 +30,12 @@ already-running stack. index), rpm (real package + **automatic repodata** generation). - **Virtual repositories** — pypi simple-index merge and helm `index.yaml` merge across two members. +- **Mirrorlist** — an rpm remote with a `mirrorlist` of extra upstream mirrors + (pool = `base_url` + `mirrorlist`): round-robin distribution across both mirrors + (constant-body `mockupstreama` / `mockupstreamb`), failover past a dead primary, + no-mirrorlist regression, and a real `dnf` (stock `rockylinux:9` container) + `makecache` + `install` through a two-mirror rpm remote whose `base_url` is dead + — a dead mirror must not break the client. ## Fixtures diff --git a/e2e-docker/fixtures/rpm-mirror/Packages/e2e-testpkg-1.0-1.noarch.rpm b/e2e-docker/fixtures/rpm-mirror/Packages/e2e-testpkg-1.0-1.noarch.rpm new file mode 100644 index 0000000000000000000000000000000000000000..1977530453b8eab78a6c875b8f82a512a20eb059 GIT binary patch literal 6838 zcmeI1TZkM*6ozZElZ%Na8)MLjNP~!3NzL|c`XX-3x|d|4n=IMHMA5|R>Z+a2_H;Mh zJ=t9`5<$=h5haKU5@Hk)R1hx^(Fc7{^g-VQLD2|`iFid35#xH!Ohwl`>H9QWedeoE zb^6p_)x&b;yXU|Eq>sS0AWT6^QIy%tG&O2EHL&;pT@|wQ{0R@ec)FtcmJ`zdDtz7y z4L?5vwx5v7MPQdf!wbe%)`CSI=yKm_uvKWW;%~vie3C(f`F%ftMgI|KFu(6du;{-J z+?weJ+%|kEgzgxQ>G;fd99O!5VR@m$UFqAp?^`|zT|?Gw)6*R{@N6lay5zcFH$&;@ zZs-^`H$z8Sy0kd6J#N@x-Euv{()G}>Ezwo$f-B2+-yT@|b#3$PZ^O47y!)P6Na+g5 zrFZod=qb=spr=4jft~_A1$qkf6zD0?Q=q3nPl28SJq0>7k%fhYhd~OcX%MogZsHKK z)Q7Gcpg~<@Db_`BObBfSG`z6xfprqBn=Da$M)7LJ-zdIVG1gHofOd)E?-XMx1LJ)U z7V9d{DaJYr#(zQaPhioHH60xPD_HcORQwxQjQ^(MvtZHxp5i|hf7sg4It!@B|{!H=Zia%0}d0_lsz+!x_+h6%ZG446c$L;L+ zuTy-b;;mpIpCz{9ZHh4t#%qAZ`PVDnr~F$K-wGD}ql#|_i}7w&e24O%Tkn3wdzC)` z3;8ce6^C7};y8T1P|Oue#ZASr;u*yySa^Q*KE-Xt4=6qa7V@n=*yVLcmH%?IOPx~7g<_F zWtvB#&{z_7Izm=5ZCWHEXJwQoHAwW}`2K;GC|Qus0g?WR#%{7FDKfz$8Htd@QPQ3x zNy>8G!~t;i-}?;A8{K!rb1f-#S8%9z3a8Gb@I7vamf@LtopIs0Ec8R^xxN*;OmfS& z49m1FlY6!a713@uLh>{%ySGJ}b!*18BImU{YZ3S^TU=2_!B;MBFeZ+IjU#C*%S8d& zjYqNA!CImiAj*e5qtP^o{!iZ|n~zi08C+~*hs0zqWHk5p6y2JN z2e#haxN&6Pz5~NkQ)4h^=Z-0j;>jz9I?IIp9E7bK%thE_r7{%O9>NI@Ddz^Pswm<8 zp;?}mu*@{4MTv?2hd3-r(f}5QVKmxuY`EbeADZ(V+M%+Yh@sBg5^;DEE9Mj4gy?Cz zpzUNfNf4)FK(4_K||Ph$aZKtY2&Ve%xNnssm$TH(kzXV z5?*lHI@ccyCjaduJd09t7o16A`0C}PU72X-g{2T0k5Km-o*HR1;M-;nzY*bFwM$7o zVoBS!JYiU&?r_cmgS)l}eMi`iVYxy#O~-+sFI>i%2^lzbNBYwCrEdm2G#GvhLoBE6 zO6Ga4kb&g|x~>OdAOkyupEUv&>K>P-DO}UE1L;V?JsBE?4?l2_U|L_0*{Nca<-)Nu!AOHda06Yu; z0ssL301Q9^U9zQ%WerS}+D1)JMD;W@44#NG0im@tG}BE6We+IO13+jr8X*k^LTHA8 zpwmDA0001F05k@GXmG@7rh`Bl0000Q41-MrKn9HfVj2M?ghbgROhDBAKm$_<8hQbs z5MmiU08CF%dM1#K|KtFESP$3l`E$-Bm1>1XX(W9VkSN&*75fneF^pp=y$PFRNdh7h z9m9yu&&S(Q1k!qx2~QV;-nTw!XqCTqUkb@nm5y@Gvlu$ljvW_cpIT|`#eVuQORIwC z>YNh@h=_=ZaZ?=0)R|0p)j$}Y7~(MoUW>py67GSWs^2sI)g~gyB?l~bAywW%SW7-s zh!HX`CPKa;64$Exrk9dorbET1Cq@S>)|<_4b8JwzasW%|D)5u9U@S48On|VwAr)Rs zkihkcc?88Ah<`{dDhLBa0b&IQ%t8UF#fL+|Dij?H=m-%9(j^k8MXv}$YalGvAcQ)G z2%`=qVM0LVnptzkBJnpFbqb|KAnu~Z?AtbKL4me4QPK4%0qBinAi(c@M@$GFONfXE zba&bq7fidF-4YXpvL9DD7AYy^uHOkpOhJN6prLLgyai6(x|(~TK~I3p69jM+1uIvh zT%f&0n+>j_(JXwHnW|h#pj=Almx@NEoBx8OZ4QExt=KuPs1~9sDC7kHGLG!&4zmrkWEI zBS2`;h-A>v$kRh02AXD|$+aVvE_5UttRs(;5A&B4i7cC?)?$hvM#w@^Fd-|<3J`=L z2x!=#wNyw*R+}mA)z-|~j9IHTXCAMk__A8Wrf`9d2ubnDOq}=c%@oiz)?+n<>~S=z zdt6d$ie$)NOgi7?#&)Ben}7fS030PGE-%)dXjn%EY?Q+#!vLxR_*SIiDa2)91i@_x zx*?G$TCngce5g$=MmfTZSSGs~Y)Q(5gW5X4XNf>n(*=q0#utqBA#=L(6?{9m2G7%`|A3}LN4SSF%Dza1U4*7){)%pX_~rb(s`(JCEqzx+eh@=@}5Q**e*o{W`ymMNQ#C$ zSMj1sZj$Dhr_1`?n#Zpo=qwM7x#`Mlk(`0y@fiOw?9Zj~floiFv1seb^Ir`}}yfsP|)M4_FMkrsV9 z%*>F@bVof^q9RC>nc}TZ=X6S{dc9*FBSN<)Ysgq2KA7PEoECuhR7N4i`GqqWxb@}4 uJT2LE2E+@`5m&cMb+;Q)v36_eut>uk4jjWoz#%yQi@744C`eNXSNH%9CSi#H literal 0 HcmV?d00001 diff --git a/e2e-docker/fixtures/rpm-mirror/repodata/d82f717e4da1afe96b8e7857de9e852f5785c0d75734e50d0f8afdcbc6261b08-primary.xml.gz b/e2e-docker/fixtures/rpm-mirror/repodata/d82f717e4da1afe96b8e7857de9e852f5785c0d75734e50d0f8afdcbc6261b08-primary.xml.gz new file mode 100644 index 0000000000000000000000000000000000000000..5526cb4f1e0932ebaf3b423f27e2cd51b3107244 GIT binary patch literal 631 zcmV--0*L(|iwFP!000001BFyeZ=)~}zV}yHz9&uy0a5`~4_&E;O5H~fdo=bCtbmPd z6D9lWcg&-Sb`L!O_UCVA{LO>>>C(024Av>53%XrzC;=^uRQg!Zk6+(b8GR}r@(w(g z+;f6-?F!m>-#;?eDm`4*ayY>n3@D{U4j@xVbY`v#mV=+Nad1Dng z!nGWMvFbJF9 zj!<+sB>QM7g;RfE(N)kS+hvp}?L2Q=2z)F{sINNXX_UpYRnQUcMH&lrLEq z21Wn@2AG%xz!L;%rT_%M38A5hfB*mhMiGEUn3)U&8e(7*08A04m;e(1CWeM400001 z7)AjaVq`D|r00000000CUVqg;hOcAD-022TvhK42p0003P zMgbaPWH1Dg36KOP2qB`Mpd>vY@`KbFGgC*VrZqJ3fHqKQ4XA0FA?kW$YHdu^unXCe zZl8`~QbY&XHlMiKwB2^^D0%&3 zb8n@VA0MU}9$ zv0;Cdi8zU*r0S(%p&v(Xmt4q@^#Tlo4_amJ?1lUJanqFigxs`zl#X~>Mg#t4dV9l( zt&jD1J&VOqQJI;824+MB;}6xR85pe*Ydq$WvtztTt(~iEXkUWBL1ADdF}qXyZ1wX} z;Jo7cm$TQtmFqHosKJ<33Vu%}Gbm`7b_8y)gZH1+g_*mFBh*018Il45JysI*!1#P< zA>XmAsvG}}BuqtA@25OI{iNe2ENCQi9al+#06Y|Q5d*X0_^ZCCH^S4J3#}2Ax*j!s zmxsS=LQ;|qJjrS$ATRK^DN{ckhVNRgtF&J3P5jSOyjWO7_l;dxPU3}zpeyyZ z$1&3;U|?228`^5rDL5JnWb!K}M6U75?JK@S;c+4MR%fOKFDo)GwnT=w#CaTq8IXOx zfx+A1NSFHJ`e&Nr2`f{c84x*c$Z_22GyK!qrght^U$o~g@*JYo8=lye!~xta)C_A)BL7l&GUy@(;s;1nKKFzlui!%&4vP#HHU@L%$E9>oc ziI{H{s1)(jKoWKViw4c5=pd9tdBsbm284>h*=1b8o#2B#<9!@&FR%7-*Czu2Dqz4$ z`bHQq8~OM&Q+;l5yRZxcu>5X7)j%LCk3jbRH{abf?bdwGGJXsflM#(Z;tj_&0Ru~J zv>T2(2LX!5ICk0uXlqZt+?fGJX&|hG92B^+I5%pWS386>E2jlgBuJ4dNuCjunc)Hu zgrzw~QiQUD%2OUCQ)o$=qNorclmLk)0_udk zB#P2^9xNe%a=#cG0thXr2M*%v?FD_nY@rpY^`PAa#@?AX#EKPiJ&b3Le~sUq##)S* zp3o6!V~8tv4xB3CW!L$A4y9=aEhLus<+o`z0l}pM8;Cj^?5zSLp@cCYG|UQgc>|r= z^)L+9bFira0svqH5yLyciAL(Tjnc;N+GUDpcCm zC^I#CW`cli0_2DQ$-te{rv|F^4hwf)?dKB=^Ur*O5bckGBejwSL9GMU%*_{eW@ct) zY(Tvtw2Oe1WE?cK;SCN8Vrg@}`1p6f{9H8-8ltORjEu%sf if&&IOrM=~^FR3zqR|h(bApo5J;_gVN3K9itiS+>AlK%|= literal 0 HcmV?d00001 diff --git a/e2e-docker/fixtures/rpm-mirror/repodata/repomd.xml b/e2e-docker/fixtures/rpm-mirror/repodata/repomd.xml new file mode 100644 index 0000000..46ab470 --- /dev/null +++ b/e2e-docker/fixtures/rpm-mirror/repodata/repomd.xml @@ -0,0 +1,55 @@ + + + 1786573032 + + d82f717e4da1afe96b8e7857de9e852f5785c0d75734e50d0f8afdcbc6261b08 + 3345bb631380ae6c0620fe2a29cf7dff2ed4e28c5cb06c91e8a1628ba1979bcb + + 1786573032 + 631 + 1192 + + + daa313cc5eeb7df556e1d4885d7701b10b9f012f436ef239fa46827f966222be + 648bd0ce00fda09abbc6e9c3ff3278518a76f258576ac24cd10c12e41e0e5bd7 + + 1786573032 + 256 + 338 + + + 8510c74a6f288828bbc92abee5d0d8ae9687d3c31a2579ea95e31a4c3a320d85 + c42cfd3843e9c53a60ad84bded44aa46c65faae7b3da99a099a9ca0b018872a9 + + 1786573032 + 296 + 399 + + + f6bd7755da13d9726381048f467992869104a4c5521338ef740dc35eb85b9b71 + c45c85d12ccb0f8172b7bfae466362c08ac1867559574a9fb9cb2118c04daddb + + 1786573032 + 1740 + 106496 + 10 + + + be3c6e4c7a13ece48bd5d6a4d6d5e6a2395fe006ef9c5f217f5b87144f465e57 + 1ccfa3dff532d782ce3225aae807506a4ce4534291386f1c47455dcc6b70cfd6 + + 1786573032 + 764 + 28672 + 10 + + + ba593cd8ab5ec1e127888707c1fd882920996f1ce173fd7a589d647918fd7da4 + 5d4d38380f0e359bfc0a50033d4faa84c75c5ae8fe2ef11c1c82d64748e9b8e2 + + 1786573032 + 738 + 24576 + 10 + + diff --git a/e2e-docker/mirror-conf/a.conf b/e2e-docker/mirror-conf/a.conf new file mode 100644 index 0000000..288a624 --- /dev/null +++ b/e2e-docker/mirror-conf/a.conf @@ -0,0 +1,9 @@ +# Mock upstream A for the multi-base_url e2e: any path returns a constant, +# upstream-identifying body so round-robin distribution is observable. +server { + listen 80; + location / { + default_type text/plain; + return 200 "UPSTREAM-A"; + } +} diff --git a/e2e-docker/mirror-conf/b.conf b/e2e-docker/mirror-conf/b.conf new file mode 100644 index 0000000..0d84602 --- /dev/null +++ b/e2e-docker/mirror-conf/b.conf @@ -0,0 +1,8 @@ +# Mock upstream B for the multi-base_url e2e (see a.conf). +server { + listen 80; + location / { + default_type text/plain; + return 200 "UPSTREAM-B"; + } +} diff --git a/e2e-docker/multibaseurl_test.go b/e2e-docker/multibaseurl_test.go new file mode 100644 index 0000000..a88aa48 --- /dev/null +++ b/e2e-docker/multibaseurl_test.go @@ -0,0 +1,163 @@ +//go:build dockere2e + +package e2edocker + +import ( + "fmt" + "net/http" + "os" + "os/exec" + "strings" + "testing" +) + +// mockUpstreamA/B are the constant-body upstreams (see docker-compose.e2e.yml) +// that let the round-robin test observe which mirror served each request. +func mockUpstreamA() string { + if v := os.Getenv("MOCK_UPSTREAM_A_INTERNAL"); v != "" { + return strings.TrimRight(v, "/") + } + return "http://mockupstreama" +} + +func mockUpstreamB() string { + if v := os.Getenv("MOCK_UPSTREAM_B_INTERNAL"); v != "" { + return strings.TrimRight(v, "/") + } + return "http://mockupstreamb" +} + +// TestMultiBaseURLRoundRobin configures an rpm remote with base_url = mirror A +// and mirrorlist = [mirror B] and drives distinct paths through it, asserting +// both mirrors serve traffic. Each path is a cache miss, so every request reaches +// upstream and the round-robin cursor alternates mirrors. +func TestMultiBaseURLRoundRobin(t *testing.T) { + name := "e2e-rr" + createRepo(t, fmt.Sprintf(`{ + "name": %q, + "package_type": "rpm", + "repo_type": "remote", + "base_url": %q, + "mirrorlist": [%q], + "stale_on_error": false + }`, name, mockUpstreamA(), mockUpstreamB())) + defer deleteRepo(t, name) + + seenA, seenB := false, false + const n = 12 + for i := 0; i < n; i++ { + url := api(fmt.Sprintf("/api/v1/remote/%s/rr/%d", name, i)) + resp, body := doRequest(t, http.MethodGet, url, nil, "") + if resp.StatusCode != http.StatusOK { + t.Fatalf("request %d: status %d: %s", i, resp.StatusCode, body) + } + switch strings.TrimSpace(string(body)) { + case "UPSTREAM-A": + seenA = true + case "UPSTREAM-B": + seenB = true + default: + t.Fatalf("request %d: unexpected body %q", i, body) + } + } + if !seenA || !seenB { + t.Fatalf("round-robin did not reach both upstreams: A=%v B=%v", seenA, seenB) + } +} + +// TestMultiBaseURLFailover points a two-mirror remote at a dead primary and a +// healthy secondary and asserts every request still succeeds via the secondary. +func TestMultiBaseURLFailover(t *testing.T) { + name := "e2e-failover" + createRepo(t, fmt.Sprintf(`{ + "name": %q, + "package_type": "rpm", + "repo_type": "remote", + "base_url": "http://mockupstream-dead:80", + "mirrorlist": [%q], + "stale_on_error": false + }`, name, mockUpstreamB())) + defer deleteRepo(t, name) + + for i := 0; i < 6; i++ { + url := api(fmt.Sprintf("/api/v1/remote/%s/fo/%d", name, i)) + resp, body := doRequest(t, http.MethodGet, url, nil, "") + if resp.StatusCode != http.StatusOK { + t.Fatalf("request %d: dead primary broke fetch: status %d: %s", i, resp.StatusCode, body) + } + if got := strings.TrimSpace(string(body)); got != "UPSTREAM-B" { + t.Fatalf("request %d: body %q, want UPSTREAM-B (served via failover)", i, got) + } + } +} + +// TestSingleBaseURLRegression asserts a remote with no mirrorlist works exactly +// as before the mirrorlist change. +func TestSingleBaseURLRegression(t *testing.T) { + name := "e2e-single" + createRepo(t, fmt.Sprintf(`{ + "name": %q, + "package_type": "rpm", + "repo_type": "remote", + "base_url": %q, + "stale_on_error": false + }`, name, mockUpstreamA())) + defer deleteRepo(t, name) + + resp, body := doRequest(t, http.MethodGet, api("/api/v1/remote/"+name+"/solo/0"), nil, "") + if resp.StatusCode != http.StatusOK { + t.Fatalf("single-url fetch: status %d: %s", resp.StatusCode, body) + } + if got := strings.TrimSpace(string(body)); got != "UPSTREAM-A" { + t.Fatalf("single-url body %q, want UPSTREAM-A", got) + } +} + +// TestMultiBaseURLDnfFailover drives a real dnf (stock rockylinux container) at +// a two-mirror rpm remote whose primary is dead: makecache + install must +// succeed via the live secondary mirror, proving a dead mirror does not break a +// real package-manager client. Requires the compose network and internal API +// URL exported by scripts/docker-e2e.sh; skipped when run standalone. +func TestMultiBaseURLDnfFailover(t *testing.T) { + network := os.Getenv("COMPOSE_NETWORK") + internal := os.Getenv("ARTIFACTAPI_INTERNAL") + if network == "" || internal == "" { + t.Skip("COMPOSE_NETWORK/ARTIFACTAPI_INTERNAL not set; run via scripts/docker-e2e.sh") + } + if _, err := exec.LookPath("docker"); err != nil { + t.Skip("docker not available on the test host") + } + + name := "e2e-dnf-failover" + // Primary base_url is dead; the live mirror serves the real yum repo under + // fixtures/rpm-mirror via the shared mock upstream. + createRepo(t, fmt.Sprintf(`{ + "name": %q, + "package_type": "rpm", + "repo_type": "remote", + "base_url": "http://mockupstream-dead:80", + "mirrorlist": [%q], + "stale_on_error": false + }`, name, mockUpstream())) + defer deleteRepo(t, name) + + repoURL := strings.TrimRight(internal, "/") + "/api/v1/remote/" + name + "/rpm-mirror" + repoConf := fmt.Sprintf("[dnffo]\nname=dnffo\nbaseurl=%s\nenabled=1\ngpgcheck=0\nsslverify=0\nmetadata_expire=0\n", repoURL) + script := "set -euo pipefail; " + + "printf '%s' \"$REPO\" > /etc/yum.repos.d/dnffo.repo; " + + "dnf -y --disablerepo='*' --enablerepo=dnffo makecache; " + + "dnf -y --disablerepo='*' --enablerepo=dnffo install e2e-testpkg; " + + "rpm -q e2e-testpkg" + + cmd := exec.Command("docker", "run", "--rm", + "--network", network, + "-e", "REPO="+repoConf, + "rockylinux:9", "bash", "-c", script) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("real dnf install through a dead primary mirror failed: %v\n%s", err, out) + } + if !strings.Contains(string(out), "e2e-testpkg-1.0-1") { + t.Fatalf("dnf did not install the expected package via failover; output:\n%s", out) + } +} diff --git a/internal/api/v2/remotes.go b/internal/api/v2/remotes.go index b8d1e36..23d1ce7 100644 --- a/internal/api/v2/remotes.go +++ b/internal/api/v2/remotes.go @@ -78,6 +78,10 @@ func (h *RemotesHandler) create(w http.ResponseWriter, r *http.Request) { http.Error(w, "base_url is required for remote repositories", http.StatusBadRequest) return } + if err := remote.ValidateMirrorlist(); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } if err := remote.ValidatePatterns(); err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return @@ -102,6 +106,10 @@ func (h *RemotesHandler) update(w http.ResponseWriter, r *http.Request) { return } remote.Name = name + if err := remote.ValidateMirrorlist(); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } if err := remote.ValidatePatterns(); err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return diff --git a/internal/database/database_test.go b/internal/database/database_test.go index af3746e..fa72d54 100644 --- a/internal/database/database_test.go +++ b/internal/database/database_test.go @@ -99,6 +99,39 @@ func TestRemotesCRUD(t *testing.T) { } } +func TestRemoteMirrorlistRoundTrip(t *testing.T) { + requireDB(t) + mirrors := []string{"https://b.example", "https://c.example"} + if err := testDB.CreateRemote(ctx(), &models.Remote{ + Name: "r-mirror", PackageType: models.PackageRPM, RepoType: models.RepoTypeRemote, + BaseURL: "https://a.example", Mirrorlist: mirrors, MutableTTL: 3600, + }); err != nil { + t.Fatalf("create mirrorlist remote: %v", err) + } + defer testDB.DeleteRemote(ctx(), "r-mirror") + + got, err := testDB.GetRemote(ctx(), "r-mirror") + if err != nil { + t.Fatalf("get: %v", err) + } + if got.BaseURL != "https://a.example" { + t.Fatalf("BaseURL = %q, want https://a.example", got.BaseURL) + } + if len(got.Mirrorlist) != 2 || got.Mirrorlist[0] != mirrors[0] || got.Mirrorlist[1] != mirrors[1] { + t.Fatalf("Mirrorlist round-trip = %v, want %v", got.Mirrorlist, mirrors) + } + + // Clearing the mirrorlist on update persists an empty list. + got.Mirrorlist = nil + if err := testDB.UpdateRemote(ctx(), got); err != nil { + t.Fatalf("update clearing mirrorlist: %v", err) + } + got, _ = testDB.GetRemote(ctx(), "r-mirror") + if len(got.Mirrorlist) != 0 { + t.Fatalf("mirrorlist after clear = %v, want empty", got.Mirrorlist) + } +} + func TestArtifactsAndBlobs(t *testing.T) { requireDB(t) seedRemote(t, "r-art") diff --git a/internal/database/postgres.go b/internal/database/postgres.go index e434d05..d4aa3c8 100644 --- a/internal/database/postgres.go +++ b/internal/database/postgres.go @@ -44,6 +44,7 @@ func (db *DB) migrate() error { package_type TEXT NOT NULL, repo_type TEXT DEFAULT 'remote', base_url TEXT NOT NULL DEFAULT '', + mirrorlist TEXT[] DEFAULT '{}', description TEXT DEFAULT '', username TEXT DEFAULT '', password TEXT DEFAULT '', @@ -124,6 +125,7 @@ func (db *DB) migrate() error { CREATE INDEX IF NOT EXISTS idx_access_log_remote_time ON access_log(remote_name, created_at); ALTER TABLE remotes ADD COLUMN IF NOT EXISTS repo_type TEXT DEFAULT 'remote'; + ALTER TABLE remotes ADD COLUMN IF NOT EXISTS mirrorlist TEXT[] DEFAULT '{}'; ALTER TABLE remotes ADD COLUMN IF NOT EXISTS upstream_dial_timeout INTEGER DEFAULT 0; ALTER TABLE remotes ADD COLUMN IF NOT EXISTS upstream_tls_timeout INTEGER DEFAULT 0; ALTER TABLE remotes ADD COLUMN IF NOT EXISTS upstream_response_header_timeout INTEGER DEFAULT 0; diff --git a/internal/database/remotes.go b/internal/database/remotes.go index a7d2b62..5eadfda 100644 --- a/internal/database/remotes.go +++ b/internal/database/remotes.go @@ -6,7 +6,7 @@ import ( "git.unkin.net/unkin/artifactapi/pkg/models" ) -const remoteCols = `name, package_type, repo_type, base_url, description, username, password, +const remoteCols = `name, package_type, repo_type, base_url, mirrorlist, description, username, password, immutable_ttl, mutable_ttl, check_mutable, patterns, blocklist, mutable_patterns, immutable_patterns, ban_tags_enabled, ban_tags, @@ -17,7 +17,7 @@ const remoteCols = `name, package_type, repo_type, base_url, description, userna func scanRemote(scanner interface{ Scan(...any) error }, r *models.Remote) error { return scanner.Scan( - &r.Name, &r.PackageType, &r.RepoType, &r.BaseURL, &r.Description, &r.Username, &r.Password, + &r.Name, &r.PackageType, &r.RepoType, &r.BaseURL, &r.Mirrorlist, &r.Description, &r.Username, &r.Password, &r.ImmutableTTL, &r.MutableTTL, &r.CheckMutable, &r.Patterns, &r.Blocklist, &r.MutablePatterns, &r.ImmutablePatterns, &r.BanTagsEnabled, &r.BanTags, @@ -58,16 +58,16 @@ func (db *DB) ListRemotes(ctx context.Context) ([]models.Remote, error) { func (db *DB) CreateRemote(ctx context.Context, r *models.Remote) error { _, err := db.Pool.Exec(ctx, ` INSERT INTO remotes ( - name, package_type, repo_type, base_url, description, username, password, + name, package_type, repo_type, base_url, mirrorlist, description, username, password, immutable_ttl, mutable_ttl, check_mutable, patterns, blocklist, mutable_patterns, immutable_patterns, ban_tags_enabled, ban_tags, quarantine_enabled, quarantine_days, stale_on_error, releases_remote, managed_by, upstream_dial_timeout, upstream_tls_timeout, upstream_response_header_timeout - ) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24) + ) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25) `, - r.Name, r.PackageType, r.RepoType, r.BaseURL, r.Description, r.Username, r.Password, + r.Name, r.PackageType, r.RepoType, r.BaseURL, r.Mirrorlist, r.Description, r.Username, r.Password, r.ImmutableTTL, r.MutableTTL, r.CheckMutable, r.Patterns, r.Blocklist, r.MutablePatterns, r.ImmutablePatterns, r.BanTagsEnabled, r.BanTags, @@ -81,7 +81,7 @@ func (db *DB) CreateRemote(ctx context.Context, r *models.Remote) error { func (db *DB) UpdateRemote(ctx context.Context, r *models.Remote) error { _, err := db.Pool.Exec(ctx, ` UPDATE remotes SET - package_type=$2, repo_type=$3, base_url=$4, description=$5, username=$6, password=$7, + package_type=$2, repo_type=$3, base_url=$4, mirrorlist=$25, description=$5, username=$6, password=$7, immutable_ttl=$8, mutable_ttl=$9, check_mutable=$10, patterns=$11, blocklist=$12, mutable_patterns=$13, immutable_patterns=$14, ban_tags_enabled=$15, ban_tags=$16, @@ -98,6 +98,7 @@ func (db *DB) UpdateRemote(ctx context.Context, r *models.Remote) error { r.QuarantineEnabled, r.QuarantineDays, r.StaleOnError, r.ReleasesRemote, r.ManagedBy, r.UpstreamDialTimeout, r.UpstreamTLSTimeout, r.UpstreamResponseHeaderTimeout, + r.Mirrorlist, ) return err } diff --git a/internal/proxy/engine.go b/internal/proxy/engine.go index 662d04d..b7b89bb 100644 --- a/internal/proxy/engine.go +++ b/internal/proxy/engine.go @@ -11,6 +11,8 @@ import ( "log/slog" "net/http" "strings" + "sync" + "sync/atomic" "time" "git.unkin.net/unkin/artifactapi/internal/cache" @@ -35,6 +37,10 @@ type Engine struct { cas *storage.CAS circuit *CircuitBreaker accessLog chan database.AccessLogEntry + // rrCounters holds a per-remote round-robin cursor (remoteName -> + // *atomic.Uint64) used to rotate the starting mirror across upstream base + // URLs. Distribution is per-replica and approximate, which is fine. + rrCounters sync.Map } func NewEngine(db *database.DB, c *cache.Redis, s *storage.S3) *Engine { @@ -222,7 +228,30 @@ func (e *Engine) Head(ctx context.Context, remote models.Remote, path string, pr return e.headUpstream(ctx, remote, path, prov) } +// headUpstream issues an upstream HEAD, load-balancing across the remote's base +// URLs and failing over to the next mirror on a network error or 5xx. func (e *Engine) headUpstream(ctx context.Context, remote models.Remote, path string, prov provider.Provider) (*HeadResult, error) { + order := e.baseURLAttemptOrder(remote) + if len(order) == 0 { + return nil, &ProxyError{Status: http.StatusBadGateway, Message: "no upstream base_url configured"} + } + var lastErr error + for i, url := range order { + result, err := e.headUpstreamOnce(ctx, withBaseURL(remote, url), path, prov) + if err == nil { + return result, nil + } + lastErr = err + if i < len(order)-1 && shouldFailover(err) { + slog.Warn("upstream HEAD failed, failing over", "remote", remote.Name, "base_url", url, "error", err) + continue + } + return nil, err + } + return nil, lastErr +} + +func (e *Engine) headUpstreamOnce(ctx context.Context, remote models.Remote, path string, prov provider.Provider) (*HeadResult, error) { url := prov.UpstreamURL(remote, path) authHeaders, err := prov.AuthHeaders(ctx, remote) @@ -277,7 +306,31 @@ func (e *Engine) headUpstream(ctx context.Context, remote models.Remote, path st return &HeadResult{ContentType: contentType, Size: resp.ContentLength, Source: "remote"}, nil } +// fetchFromUpstream fetches an artifact from upstream, load-balancing across the +// remote's base URLs and failing over to the next mirror on a network error or +// 5xx before returning an error. func (e *Engine) fetchFromUpstream(ctx context.Context, remote models.Remote, path string, prov provider.Provider, class Classification, ttl time.Duration, clientHeaders http.Header) (*FetchResult, error) { + order := e.baseURLAttemptOrder(remote) + if len(order) == 0 { + return nil, &ProxyError{Status: http.StatusBadGateway, Message: "no upstream base_url configured"} + } + var lastErr error + for i, url := range order { + result, err := e.fetchFromUpstreamOnce(ctx, withBaseURL(remote, url), path, prov, class, ttl, clientHeaders) + if err == nil { + return result, nil + } + lastErr = err + if i < len(order)-1 && shouldFailover(err) { + slog.Warn("upstream fetch failed, failing over", "remote", remote.Name, "base_url", url, "error", err) + continue + } + return nil, err + } + return nil, lastErr +} + +func (e *Engine) fetchFromUpstreamOnce(ctx context.Context, remote models.Remote, path string, prov provider.Provider, class Classification, ttl time.Duration, clientHeaders http.Header) (*FetchResult, error) { url := prov.UpstreamURL(remote, path) authHeaders, err := prov.AuthHeaders(ctx, remote) @@ -454,7 +507,31 @@ func (e *Engine) serveFromStore(ctx context.Context, remote models.Remote, path }, nil } +// checkUpstream issues a conditional upstream HEAD (If-None-Match), load +// balancing across the remote's base URLs and failing over to the next mirror on +// a network error or 5xx. func (e *Engine) checkUpstream(ctx context.Context, remote models.Remote, path, etag string, prov provider.Provider) (bool, error) { + order := e.baseURLAttemptOrder(remote) + if len(order) == 0 { + return false, &ProxyError{Status: http.StatusBadGateway, Message: "no upstream base_url configured"} + } + var lastErr error + for i, url := range order { + notModified, err := e.checkUpstreamOnce(ctx, withBaseURL(remote, url), path, etag, prov) + if err == nil { + return notModified, nil + } + lastErr = err + if i < len(order)-1 && shouldFailover(err) { + slog.Warn("upstream revalidation failed, failing over", "remote", remote.Name, "base_url", url, "error", err) + continue + } + return false, err + } + return false, lastErr +} + +func (e *Engine) checkUpstreamOnce(ctx context.Context, remote models.Remote, path, etag string, prov provider.Provider) (bool, error) { url := prov.UpstreamURL(remote, path) req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil) @@ -649,3 +726,45 @@ func isNetworkError(err error) bool { var ue *UpstreamError return errors.As(err, &ue) } + +// baseURLAttemptOrder returns the ordered upstream base URLs to try for a single +// request, drawn from the remote's pool ([base_url] + mirrorlist). A multi-mirror +// remote starts at the next round-robin position and advances linearly for +// failover; a remote with no mirrorlist yields exactly [base_url], preserving the +// original single-attempt behavior. +func (e *Engine) baseURLAttemptOrder(remote models.Remote) []string { + urls := remote.UpstreamPool() + if len(urls) <= 1 { + return urls + } + v, _ := e.rrCounters.LoadOrStore(remote.Name, new(atomic.Uint64)) + start := int(v.(*atomic.Uint64).Add(1) - 1) + ordered := make([]string, len(urls)) + for i := range urls { + ordered[i] = urls[(start+i)%len(urls)] + } + return ordered +} + +// withBaseURL narrows a remote's active BaseURL to a single selected mirror so +// providers (UpstreamURL/AuthHeaders/RewriteResponse) operate on exactly that +// upstream for this attempt. +func withBaseURL(remote models.Remote, url string) models.Remote { + remote.BaseURL = url + remote.Mirrorlist = nil + return remote +} + +// shouldFailover reports whether an upstream attempt error is worth retrying +// against the next mirror: network errors/timeouts and upstream 5xx responses. +// Definitive statuses (404/403/401/...) are returned to the caller unchanged. +func shouldFailover(err error) bool { + if isNetworkError(err) { + return true + } + var pe *ProxyError + if errors.As(err, &pe) { + return pe.Status >= 500 + } + return false +} diff --git a/internal/proxy/multibaseurl_test.go b/internal/proxy/multibaseurl_test.go new file mode 100644 index 0000000..d165c1c --- /dev/null +++ b/internal/proxy/multibaseurl_test.go @@ -0,0 +1,188 @@ +package proxy + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "git.unkin.net/unkin/artifactapi/pkg/models" +) + +// TestFetchMultiBaseURLRoundRobin drives distinct artifact paths through a +// remote configured with two upstreams and asserts both receive traffic. +func TestFetchMultiBaseURLRoundRobin(t *testing.T) { + requireStack(t) + ctx := context.Background() + + var hitsA, hitsB atomic.Int64 + upA := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hitsA.Add(1) + w.Write([]byte("A")) + })) + defer upA.Close() + upB := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hitsB.Add(1) + w.Write([]byte("B")) + })) + defer upB.Close() + + r := seed(t, models.Remote{ + Name: "eng-rr", + PackageType: models.PackageGeneric, + RepoType: models.RepoTypeRemote, + BaseURL: upA.URL, + Mirrorlist: []string{upB.URL}, + StaleOnError: true, + }) + p := prov(t, models.PackageGeneric) + + const n = 10 + for i := 0; i < n; i++ { + res, err := testEngine.Fetch(ctx, r, fmt.Sprintf("rr-%d.bin", i), p) + if err != nil { + t.Fatalf("fetch %d: %v", i, err) + } + res.Reader.Close() + } + + if hitsA.Load() == 0 || hitsB.Load() == 0 { + t.Fatalf("round-robin did not spread across both upstreams: A=%d B=%d", hitsA.Load(), hitsB.Load()) + } + if total := hitsA.Load() + hitsB.Load(); total != n { + t.Fatalf("expected %d upstream hits total, got %d (A=%d B=%d)", n, total, hitsA.Load(), hitsB.Load()) + } +} + +// TestFetchMultiBaseURLFailover asserts that a dead/erroring primary mirror +// transparently fails over to a healthy secondary, for both a 5xx primary and a +// network-unreachable primary. +func TestFetchMultiBaseURLFailover(t *testing.T) { + requireStack(t) + ctx := context.Background() + + var hitsB atomic.Int64 + upB := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hitsB.Add(1) + w.Write([]byte("served-by-B")) + })) + defer upB.Close() + up500 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer up500.Close() + + p := prov(t, models.PackageGeneric) + + // Primary returns 5xx: every request must still succeed via the secondary. + r5xx := seed(t, models.Remote{ + Name: "eng-failover-5xx", + PackageType: models.PackageGeneric, + RepoType: models.RepoTypeRemote, + BaseURL: up500.URL, + Mirrorlist: []string{upB.URL}, + }) + for i := 0; i < 6; i++ { + res, err := testEngine.Fetch(ctx, r5xx, fmt.Sprintf("fo5-%d.bin", i), p) + if err != nil { + t.Fatalf("5xx failover fetch %d: %v", i, err) + } + if got := readAll(t, res); got != "served-by-B" { + t.Fatalf("5xx failover fetch %d body=%q, want served-by-B", i, got) + } + } + + // Primary is network-unreachable: failover must still reach the secondary. + rNet := seed(t, models.Remote{ + Name: "eng-failover-net", + PackageType: models.PackageGeneric, + RepoType: models.RepoTypeRemote, + BaseURL: "http://127.0.0.1:1", + Mirrorlist: []string{upB.URL}, + }) + res, err := testEngine.Fetch(ctx, rNet, "fonet.bin", p) + if err != nil { + t.Fatalf("network failover fetch: %v", err) + } + if got := readAll(t, res); got != "served-by-B" { + t.Fatalf("network failover body=%q, want served-by-B", got) + } + if hitsB.Load() == 0 { + t.Fatal("secondary upstream never served during failover") + } +} + +// TestFetchDefinitiveStatusNoFailover asserts a definitive 404 from the first +// mirror is returned as-is (not failed over): a missing artifact is not a mirror +// outage. The remote is fresh so its round-robin cursor starts at index 0. +func TestFetchDefinitiveStatusNoFailover(t *testing.T) { + requireStack(t) + ctx := context.Background() + + var hitsB atomic.Int64 + up404 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer up404.Close() + upB := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hitsB.Add(1) + w.Write([]byte("B")) + })) + defer upB.Close() + + r := seed(t, models.Remote{ + Name: "eng-no-failover-404", + PackageType: models.PackageGeneric, + RepoType: models.RepoTypeRemote, + BaseURL: up404.URL, + Mirrorlist: []string{upB.URL}, + }) + _, err := testEngine.Fetch(ctx, r, "missing.bin", prov(t, models.PackageGeneric)) + var pe *ProxyError + if err == nil || !asProxyError(err, &pe) || pe.Status != http.StatusNotFound { + t.Fatalf("expected 404 ProxyError without failover, got %v", err) + } + if hitsB.Load() != 0 { + t.Fatalf("404 from primary must not fail over, but secondary was hit %d times", hitsB.Load()) + } +} + +// TestFetchSingleBaseURLUnchanged asserts a single-URL remote behaves exactly as +// before: one healthy URL succeeds, and one dead URL errors with no failover. +func TestFetchSingleBaseURLUnchanged(t *testing.T) { + requireStack(t) + ctx := context.Background() + + upB := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("solo")) + })) + defer upB.Close() + + p := prov(t, models.PackageGeneric) + + rOK := seed(t, models.Remote{ + Name: "eng-solo", + PackageType: models.PackageGeneric, + RepoType: models.RepoTypeRemote, + BaseURL: upB.URL, + }) + res, err := testEngine.Fetch(ctx, rOK, "solo.bin", p) + if err != nil { + t.Fatalf("single-url fetch: %v", err) + } + if got := readAll(t, res); got != "solo" { + t.Fatalf("single-url body=%q, want solo", got) + } + + rDead := seed(t, models.Remote{ + Name: "eng-solo-dead", + PackageType: models.PackageGeneric, + RepoType: models.RepoTypeRemote, + BaseURL: "http://127.0.0.1:1", + }) + if _, err := testEngine.Fetch(ctx, rDead, "x.bin", p); err == nil { + t.Fatal("single dead upstream should error, not succeed") + } +} diff --git a/pkg/models/remote.go b/pkg/models/remote.go index 5516a25..2b50891 100644 --- a/pkg/models/remote.go +++ b/pkg/models/remote.go @@ -2,6 +2,7 @@ package models import ( "fmt" + "net/url" "regexp" "time" ) @@ -39,9 +40,13 @@ type Remote struct { PackageType PackageType `json:"package_type"` RepoType RepoType `json:"repo_type"` BaseURL string `json:"base_url"` - Description string `json:"description,omitempty"` - Username string `json:"-"` - Password string `json:"-"` + // Mirrorlist holds additional upstream mirror base URLs. The effective + // upstream pool is [base_url] + mirrorlist, load-balanced round-robin with + // failover by the proxy engine. Only valid on remote rpm/deb/apk repos. + Mirrorlist []string `json:"mirrorlist,omitempty"` + Description string `json:"description,omitempty"` + Username string `json:"-"` + Password string `json:"-"` ImmutableTTL int `json:"immutable_ttl"` MutableTTL int `json:"mutable_ttl"` @@ -72,6 +77,51 @@ type Remote struct { UpdatedAt time.Time `json:"updated_at"` } +// mirrorlistPackageTypes are the package types for which a mirrorlist is +// allowed: OS package repos (rpm, deb, apk/alpine) that fetch many small files +// and benefit most from mirror load-balancing and failover. +var mirrorlistPackageTypes = map[PackageType]bool{ + PackageRPM: true, + PackageDeb: true, + PackageAlpine: true, +} + +// UpstreamPool returns the ordered upstream base URLs for this remote: the +// primary base_url first, followed by any mirrorlist entries. The proxy engine +// load-balances round-robin across the pool and fails over between them. +func (r Remote) UpstreamPool() []string { + pool := make([]string, 0, 1+len(r.Mirrorlist)) + if r.BaseURL != "" { + pool = append(pool, r.BaseURL) + } + pool = append(pool, r.Mirrorlist...) + return pool +} + +// ValidateMirrorlist enforces that a mirrorlist is only configured on remote +// rpm/deb/apk repositories and that every entry is a parseable http/https URL. +func (r *Remote) ValidateMirrorlist() error { + if len(r.Mirrorlist) == 0 { + return nil + } + if r.RepoType != RepoTypeRemote { + return fmt.Errorf("mirrorlist is only allowed on remote repositories") + } + if !mirrorlistPackageTypes[r.PackageType] { + return fmt.Errorf("mirrorlist is only allowed for rpm, deb and alpine package types, not %q", r.PackageType) + } + for _, u := range r.Mirrorlist { + parsed, err := url.ParseRequestURI(u) + if err != nil { + return fmt.Errorf("invalid mirrorlist url %q: %w", u, err) + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return fmt.Errorf("mirrorlist url %q must be http or https", u) + } + } + return nil +} + // ValidatePatterns ensures every configured regex compiles. Storing an // invalid pattern would otherwise be silently dropped at match time, which // for the blocklist is a fail-open: a mistyped deny rule becomes a no-op. diff --git a/pkg/models/remote_test.go b/pkg/models/remote_test.go index e46de7b..c5749ac 100644 --- a/pkg/models/remote_test.go +++ b/pkg/models/remote_test.go @@ -1,6 +1,10 @@ package models -import "testing" +import ( + "encoding/json" + "strings" + "testing" +) func TestRemote_ValidatePatterns(t *testing.T) { valid := &Remote{ @@ -17,3 +21,81 @@ func TestRemote_ValidatePatterns(t *testing.T) { t.Fatal("expected error for invalid blocklist regex, got nil") } } + +func TestRemoteMirrorlistJSON(t *testing.T) { + // base_url stays a plain string; mirrorlist round-trips as an array. + var r Remote + body := `{"name":"x","package_type":"rpm","repo_type":"remote","base_url":"https://a.example","mirrorlist":["https://b.example","https://c.example"]}` + if err := json.Unmarshal([]byte(body), &r); err != nil { + t.Fatal(err) + } + if r.BaseURL != "https://a.example" { + t.Errorf("BaseURL = %q, want https://a.example", r.BaseURL) + } + if len(r.Mirrorlist) != 2 || r.Mirrorlist[0] != "https://b.example" || r.Mirrorlist[1] != "https://c.example" { + t.Errorf("Mirrorlist = %v, want two entries", r.Mirrorlist) + } + + out, err := json.Marshal(r) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(out), `"base_url":"https://a.example"`) { + t.Errorf("marshal lost base_url: %s", out) + } + if !strings.Contains(string(out), `"mirrorlist":["https://b.example","https://c.example"]`) { + t.Errorf("marshal lost mirrorlist: %s", out) + } +} + +func TestRemoteMirrorlistOmitempty(t *testing.T) { + out, err := json.Marshal(Remote{Name: "x", PackageType: PackageRPM, RepoType: RepoTypeRemote, BaseURL: "https://a.example"}) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(out), "mirrorlist") { + t.Errorf("empty mirrorlist should be omitted: %s", out) + } +} + +func TestUpstreamPool(t *testing.T) { + // base_url first, then mirrorlist. + r := Remote{BaseURL: "https://a.example", Mirrorlist: []string{"https://b.example", "https://c.example"}} + pool := r.UpstreamPool() + want := []string{"https://a.example", "https://b.example", "https://c.example"} + if strings.Join(pool, ",") != strings.Join(want, ",") { + t.Errorf("UpstreamPool = %v, want %v", pool, want) + } + + // No mirrorlist ⇒ pool is just [base_url]. + solo := Remote{BaseURL: "https://a.example"} + if got := solo.UpstreamPool(); len(got) != 1 || got[0] != "https://a.example" { + t.Errorf("solo UpstreamPool = %v, want [base_url]", got) + } +} + +func TestValidateMirrorlist(t *testing.T) { + cases := []struct { + name string + remote Remote + wantErr bool + }{ + {"empty is ok on anything", Remote{RepoType: RepoTypeRemote, PackageType: PackageGeneric}, false}, + {"rpm remote ok", Remote{RepoType: RepoTypeRemote, PackageType: PackageRPM, Mirrorlist: []string{"https://m.example"}}, false}, + {"deb remote ok", Remote{RepoType: RepoTypeRemote, PackageType: PackageDeb, Mirrorlist: []string{"http://m.example"}}, false}, + {"alpine remote ok", Remote{RepoType: RepoTypeRemote, PackageType: PackageAlpine, Mirrorlist: []string{"https://m.example"}}, false}, + {"generic remote rejected", Remote{RepoType: RepoTypeRemote, PackageType: PackageGeneric, Mirrorlist: []string{"https://m.example"}}, true}, + {"docker remote rejected", Remote{RepoType: RepoTypeRemote, PackageType: PackageDocker, Mirrorlist: []string{"https://m.example"}}, true}, + {"local rpm rejected", Remote{RepoType: RepoTypeLocal, PackageType: PackageRPM, Mirrorlist: []string{"https://m.example"}}, true}, + {"bad scheme rejected", Remote{RepoType: RepoTypeRemote, PackageType: PackageRPM, Mirrorlist: []string{"ftp://m.example"}}, true}, + {"unparseable rejected", Remote{RepoType: RepoTypeRemote, PackageType: PackageRPM, Mirrorlist: []string{"://nope"}}, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := tc.remote.ValidateMirrorlist() + if (err != nil) != tc.wantErr { + t.Errorf("ValidateMirrorlist() err = %v, wantErr = %v", err, tc.wantErr) + } + }) + } +} diff --git a/scripts/docker-e2e.sh b/scripts/docker-e2e.sh index 80c4f15..0318a8d 100755 --- a/scripts/docker-e2e.sh +++ b/scripts/docker-e2e.sh @@ -17,8 +17,8 @@ cleanup() { } trap cleanup EXIT -echo "==> building and starting stack (postgres, redis, minio, mockupstream, artifactapi)" -"${COMPOSE[@]}" up -d --build postgres redis minio mockupstream artifactapi +echo "==> building and starting stack (postgres, redis, minio, mockupstream(s), artifactapi)" +"${COMPOSE[@]}" up -d --build postgres redis minio mockupstream mockupstreama mockupstreamb artifactapi echo "==> waiting for artifactapi health at ${API_URL}" for i in $(seq 1 60); do @@ -34,7 +34,17 @@ for i in $(seq 1 60); do sleep 1 done -echo "==> running dockerised e2e suite" +# Resolve the compose network the artifactapi container is attached to, so the +# real-package-manager test can launch a stock distro container on the same +# network and reach artifactapi by service name. +API_CID="$("${COMPOSE[@]}" ps -q artifactapi)" +COMPOSE_NETWORK="$(docker inspect -f '{{range $k,$_ := .NetworkSettings.Networks}}{{$k}}{{end}}' "${API_CID}" 2>/dev/null || true)" + +echo "==> running dockerised e2e suite (compose network: ${COMPOSE_NETWORK:-unknown})" ARTIFACTAPI_URL="${API_URL}" \ MOCK_UPSTREAM_INTERNAL="${MOCK_UPSTREAM_INTERNAL:-http://mockupstream}" \ +MOCK_UPSTREAM_A_INTERNAL="${MOCK_UPSTREAM_A_INTERNAL:-http://mockupstreama}" \ +MOCK_UPSTREAM_B_INTERNAL="${MOCK_UPSTREAM_B_INTERNAL:-http://mockupstreamb}" \ +ARTIFACTAPI_INTERNAL="${ARTIFACTAPI_INTERNAL:-http://artifactapi:8000}" \ +COMPOSE_NETWORK="${COMPOSE_NETWORK}" \ go test -tags=dockere2e -count=1 -timeout=10m -v ./e2e-docker/...