From 080ced82794e266abe6579dde62f34dbec4a95bd Mon Sep 17 00:00:00 2001 From: Mustafa Gezen Date: Sun, 3 Sep 2023 10:09:43 +0200 Subject: [PATCH] Add more tests --- tools/mothership/worker_server/BUILD | 5 +- tools/mothership/worker_server/entry.go | 15 +-- tools/mothership/worker_server/entry_test.go | 4 +- tools/mothership/worker_server/forge_test.go | 60 ++++++++++- tools/mothership/worker_server/process_rpm.go | 26 +---- .../worker_server/process_rpm_test.go | 97 ++++++++++++++++++ .../testdata/basesystem-11-5.el8.src.rpm | Bin 0 -> 10758 bytes tools/mothership/worker_server/utils.go | 31 ++++++ 8 files changed, 196 insertions(+), 42 deletions(-) create mode 100644 tools/mothership/worker_server/testdata/basesystem-11-5.el8.src.rpm create mode 100644 tools/mothership/worker_server/utils.go diff --git a/tools/mothership/worker_server/BUILD b/tools/mothership/worker_server/BUILD index 7d5de840..05d1e461 100644 --- a/tools/mothership/worker_server/BUILD +++ b/tools/mothership/worker_server/BUILD @@ -20,6 +20,7 @@ go_library( "entry.go", "process_rpm.go", "retract_entry.go", + "utils.go", "worker.go", "workflows.go", ], @@ -58,7 +59,6 @@ go_test( "main_test.go", "process_rpm_test.go", "retract_entry_test.go", - "worker_test.go", "workflows_test.go", ], data = glob(["testdata/**"]), @@ -72,7 +72,10 @@ go_test( "//tools/mothership/proto/v1:pb", "//tools/mothership/worker_server/forge", "//vendor/github.com/go-git/go-billy/v5/osfs", + "//vendor/github.com/go-git/go-git/v5:go-git", + "//vendor/github.com/go-git/go-git/v5/plumbing/cache", "//vendor/github.com/go-git/go-git/v5/plumbing/transport/http", + "//vendor/github.com/go-git/go-git/v5/storage/filesystem", "//vendor/github.com/stretchr/testify/mock", "//vendor/github.com/stretchr/testify/require", "//vendor/github.com/stretchr/testify/suite", diff --git a/tools/mothership/worker_server/entry.go b/tools/mothership/worker_server/entry.go index 39dffc08..f7f2eedd 100644 --- a/tools/mothership/worker_server/entry.go +++ b/tools/mothership/worker_server/entry.go @@ -26,7 +26,6 @@ import ( mothershippb "go.resf.org/peridot/tools/mothership/pb" "go.temporal.io/sdk/temporal" "io" - "net/url" "os" "path/filepath" "time" @@ -78,19 +77,9 @@ func (w *Worker) SetEntryIDFromRPM(entry string, uri string, checksumSha256 stri } defer os.RemoveAll(tempDir) - // Parse uri - parsed, err := url.Parse(uri) + object, err := getObjectPath(uri) if err != nil { - return nil, errors.Wrap(err, "failed to parse resource URI") - } - - // Download the resource to the temporary directory - // S3 for example must include bucket, while memory:// does not. - // So memory://test.rpm would be parsed as host=test.rpm, path="". - // While s3://mship/test.rpm would be parsed as host=mship, path=test.rpm. - object := parsed.Path - if object == "" { - object = parsed.Host + return nil, err } err = w.storage.Download(object, filepath.Join(tempDir, "resource.rpm")) diff --git a/tools/mothership/worker_server/entry_test.go b/tools/mothership/worker_server/entry_test.go index 6b1f815d..af5c56d1 100644 --- a/tools/mothership/worker_server/entry_test.go +++ b/tools/mothership/worker_server/entry_test.go @@ -125,7 +125,7 @@ func TestWorker_SetEntryState(t *testing.T) { importRpmRes := &mothershippb.ImportRPMResponse{ CommitHash: "123", - CommitUri: "https://forge.resf.org/peridot/efi-rpm-macros/commit/123", + CommitUri: "https://testforge.resf.org/peridot/efi-rpm-macros/commit/123", CommitBranch: "el-8.8", CommitTag: "imports/el-8.8/efi-rpm-macros-3-3.el8", Nevra: "efi-rpm-macros-0:3-3.el8.aarch64", @@ -136,7 +136,7 @@ func TestWorker_SetEntryState(t *testing.T) { require.NotNil(t, entry) require.Equal(t, mothershippb.Entry_ARCHIVED, entry.State) require.Equal(t, "123", entry.CommitHash) - require.Equal(t, "https://forge.resf.org/peridot/efi-rpm-macros/commit/123", entry.CommitUri) + require.Equal(t, "https://testforge.resf.org/peridot/efi-rpm-macros/commit/123", entry.CommitUri) require.Equal(t, "el-8.8", entry.CommitBranch) require.Equal(t, "imports/el-8.8/efi-rpm-macros-3-3.el8", entry.CommitTag) require.Equal(t, "efi-rpm-macros", entry.Pkg) diff --git a/tools/mothership/worker_server/forge_test.go b/tools/mothership/worker_server/forge_test.go index 9d2d0b07..dffd106d 100644 --- a/tools/mothership/worker_server/forge_test.go +++ b/tools/mothership/worker_server/forge_test.go @@ -15,20 +15,28 @@ package mothership_worker_server import ( + "errors" "fmt" + "github.com/go-git/go-billy/v5/osfs" + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/cache" transport_http "github.com/go-git/go-git/v5/plumbing/transport/http" + "github.com/go-git/go-git/v5/storage/filesystem" "go.resf.org/peridot/tools/mothership/worker_server/forge" + "path/filepath" "time" ) type inMemoryForge struct { - localTempDir string - repos map[string]bool - remoteBaseURL string + localTempDir string + repos map[string]bool + remoteBaseURL string + invalidUsernamePass bool + noAuthMethod bool } func (f *inMemoryForge) GetAuthenticator() (*forge.Authenticator, error) { - return &forge.Authenticator{ + ret := &forge.Authenticator{ AuthMethod: &transport_http.BasicAuth{ Username: "user", Password: "pass", @@ -36,7 +44,18 @@ func (f *inMemoryForge) GetAuthenticator() (*forge.Authenticator, error) { AuthorName: "Test User", AuthorEmail: "test@resf.org", Expires: time.Now().Add(time.Hour), - }, nil + } + + if f.noAuthMethod { + ret.AuthMethod = nil + } else if f.invalidUsernamePass { + ret.AuthMethod = &transport_http.BasicAuth{ + Username: "invalid", + Password: "invalid", + } + } + + return ret, nil } func (f *inMemoryForge) GetRemote(repo string) string { @@ -48,6 +67,37 @@ func (f *inMemoryForge) GetCommitViewerURL(repo string, commit string) string { } func (f *inMemoryForge) EnsureRepositoryExists(auth *forge.Authenticator, repo string) error { + // Try casting auth.AuthMethod to *transport_http.BasicAuth + // If it fails, return an error + authx, ok := auth.AuthMethod.(*transport_http.BasicAuth) + if !ok { + return errors.New("auth failed") + } + if authx.Username != "user" || authx.Password != "pass" { + return errors.New("username or password incorrect") + } + + if f.repos[repo] { + return nil + } + + osfsTemp := osfs.New(filepath.Join(f.localTempDir, repo)) + dot, err := osfsTemp.Chroot(".git") + if err != nil { + return err + } + + filesystemTemp := filesystem.NewStorage(dot, cache.NewObjectLRUDefault()) + err = filesystemTemp.Init() + if err != nil { + return err + } + + _, err = git.Init(filesystemTemp, nil) + if err != nil { + return err + } + f.repos[repo] = true return nil } diff --git a/tools/mothership/worker_server/process_rpm.go b/tools/mothership/worker_server/process_rpm.go index 513ac78a..b60a5554 100644 --- a/tools/mothership/worker_server/process_rpm.go +++ b/tools/mothership/worker_server/process_rpm.go @@ -26,7 +26,6 @@ import ( "go.resf.org/peridot/tools/mothership/worker_server/srpm_import" "go.temporal.io/sdk/temporal" "io" - "net/url" "os" "path/filepath" "strings" @@ -48,24 +47,9 @@ func (w *Worker) VerifyResourceExists(uri string) error { ) } - // Get object name from URI. - // Check if object exists. - // If not, return error. - parsed, err := url.Parse(uri) + object, err := getObjectPath(uri) if err != nil { - return temporal.NewNonRetryableApplicationError( - "could not parse resource URI", - "couldNotParseResourceURI", - errors.Wrap(err, "failed to parse resource URI"), - ) - } - - // S3 for example must include bucket, while memory:// does not. - // So memory://test.rpm would be parsed as host=test.rpm, path="". - // While s3://mship/test.rpm would be parsed as host=mship, path=test.rpm. - object := parsed.Path - if object == "" { - object = parsed.Host + return err } exists, err := w.storage.Exists(object) @@ -94,13 +78,13 @@ func (w *Worker) ImportRPM(uri string, checksumSha256 string, osRelease string) defer os.RemoveAll(tempDir) // Parse uri - parsed, err := url.Parse(uri) + object, err := getObjectPath(uri) if err != nil { - return nil, errors.Wrap(err, "failed to parse resource URI") + return nil, err } // Download the resource to the temporary directory - err = w.storage.Download(parsed.Path, filepath.Join(tempDir, "resource.rpm")) + err = w.storage.Download(object, filepath.Join(tempDir, "resource.rpm")) if err != nil { return nil, errors.Wrap(err, "failed to download resource") } diff --git a/tools/mothership/worker_server/process_rpm_test.go b/tools/mothership/worker_server/process_rpm_test.go index 77e38509..67f342a8 100644 --- a/tools/mothership/worker_server/process_rpm_test.go +++ b/tools/mothership/worker_server/process_rpm_test.go @@ -34,3 +34,100 @@ func TestWorker_VerifyResourceExists_CannotRead(t *testing.T) { require.NotNil(t, err) require.Contains(t, err.Error(), "client submitted a resource URI that cannot be read by server") } + +func TestWorker_ImportRPM(t *testing.T) { + require.False(t, inmf.repos["efi-rpm-macros"]) + + res, err := testW.ImportRPM( + "memory://efi-rpm-macros-3-3.el8.src.rpm", + "518a9418fec1deaeb4c636615d8d81fb60146883c431ea15ab1127893d075d28", + "Rocky Linux release 8.8 (Green Obsidian)", + ) + require.Nil(t, err) + require.NotNil(t, res) + require.Equal(t, "efi-rpm-macros", res.Pkg) + require.Equal(t, "efi-rpm-macros-0:3-3.el8.noarch.rpm", res.Nevra) + + require.True(t, inmf.repos["efi-rpm-macros"]) +} + +func TestWorker_ImportRPM_Existing(t *testing.T) { + require.False(t, inmf.repos["basesystem"]) + + res, err := testW.ImportRPM( + "memory://basesystem-11-5.el8.src.rpm", + "6beff4cbfd5425e2c193312a9a184969a27d6bbd2d4cc29d7ce72dbe3d9f6416", + "Rocky Linux release 8.8 (Green Obsidian)", + ) + require.Nil(t, err) + require.NotNil(t, res) + + require.True(t, inmf.repos["basesystem"]) + + res, err = testW.ImportRPM( + "memory://basesystem-11-5.el8.src.rpm", + "6beff4cbfd5425e2c193312a9a184969a27d6bbd2d4cc29d7ce72dbe3d9f6416", + "Rocky Linux release 8.8 (Green Obsidian)", + ) + require.Nil(t, err) + require.NotNil(t, res) + + require.True(t, inmf.repos["basesystem"]) + + remote := testW.forge.GetRemote("basesystem") + + repo, err := getRepo(remote) + require.Nil(t, err) + + commitIter, err := repo.CommitObjects() + require.Nil(t, err) + c, err := commitIter.Next() + require.Nil(t, err) + require.NotNil(t, c) + require.Equal(t, "import basesystem-11-5.el8", c.Message) + c, err = commitIter.Next() + require.Nil(t, err) + require.NotNil(t, c) + require.Equal(t, "import basesystem-11-5.el8", c.Message) +} + +func TestWorker_ImportRPM_ChecksumDoesntMatch(t *testing.T) { + res, err := testW.ImportRPM( + "memory://efi-rpm-macros-3-3.el8.src.rpm", + "518a9418fec1deaeb4c636615d8d81fb60146883c431ea15ab1127893d075d27", + "Rocky Linux release 8.8 (Green Obsidian)", + ) + require.NotNil(t, err) + require.Nil(t, res) + require.Contains(t, err.Error(), "checksum does not match") +} + +func TestWorker_ImportRPM_AuthError(t *testing.T) { + inmf.noAuthMethod = true + + res, err := testW.ImportRPM( + "memory://efi-rpm-macros-3-3.el8.src.rpm", + "518a9418fec1deaeb4c636615d8d81fb60146883c431ea15ab1127893d075d28", + "Rocky Linux release 8.8 (Green Obsidian)", + ) + require.NotNil(t, err) + require.Nil(t, res) + require.Contains(t, err.Error(), "auth failed") + + inmf.noAuthMethod = false +} + +func TestWorker_ImportRPM_InvalidCredentials(t *testing.T) { + inmf.invalidUsernamePass = true + + res, err := testW.ImportRPM( + "memory://efi-rpm-macros-3-3.el8.src.rpm", + "518a9418fec1deaeb4c636615d8d81fb60146883c431ea15ab1127893d075d28", + "Rocky Linux release 8.8 (Green Obsidian)", + ) + require.NotNil(t, err) + require.Nil(t, res) + require.Contains(t, err.Error(), "username or password incorrect") + + inmf.invalidUsernamePass = false +} diff --git a/tools/mothership/worker_server/testdata/basesystem-11-5.el8.src.rpm b/tools/mothership/worker_server/testdata/basesystem-11-5.el8.src.rpm new file mode 100644 index 0000000000000000000000000000000000000000..29463506deb173819be6bab5e85203df35cc7a26 GIT binary patch literal 10758 zcmeHNdss~C_upNRlF)@imn}t6)66t8&1BScRiV2g!k(EuH8stQnW^MflzelkbL1|U zQslT)R7f#M91*7+MdTJmN-nvUzVBO`FX#N8bDVQ}p5H(A^Xysg=Y7{&d%f$uuJugo z{ zbO`wLpk54q5mg4j+Mv+Cw+!q;RE83GE}#IbH3O;++IJE737|lqYId)T+S6XFeiKlj zU+o#7fL|TfL-}^19Z;?HDXO;Wh>8YcWH4G?>Q^q^XZb3y>ZILK{k+fvQHqdx{Z!}q zI6g--2M+mEGi|v}w?f(f#*8cHPb#*~O1qwK-hHWgRenPGE8YD;T$%Rd+(jvqnn%UI ze^^@kX6Yv9ig&lDk*(%?hVSGIe>}kZjlq;bb4GnUiP(e6$`45dly=G+B>ZN`9)ruR&e_Wowe#4W72H=27B0vJxyI(r|JY2o;2>i>WW$2 zmd;eSH!`L0=G8{YjiQ2$o4UsNTLsp-RX$K8z7HN=+Ak>FB

d?x-UNeD!iA7FrjN zKYBO4rR4IOQxPK@9yH;hB{NOlH%2)6uIl>b)yoN$OQun7+AgiHTG6@hh@tq}V3*}t zkB>Cm7?jT(ms@l07LtGHykXssMT@A0u4C^XvkN-1esTW$hYoMr;thvJu8=j|`8Y2$ z|NQyY15N9at2z_A)KKTKHJetaoqJNcMiTS$g`&8ZiB5T8u5B|WO7HGX+OjHRBH5#H z(B)alSjT_*p?-NuhHAC-* z84Nm;&Z6Er@|`7B2Zb3w*WG6|9V1uGJy%#ix%pj-`551u8MJ#(i#f9mhD9z`9jP(V zxX{M=@Uo+CN$A_I)Qgr{SGqaH$-rQm=+l}~TKWym=-@l}K zaB{ubkNRUR%4V)Kp&q|={?^#&=2)_^`L;{-gZ5TS=NQ^eV&07zJl67gxyzuzvLByKMgr!%HT-t~%T~?aFd?ollhByuc`}Ca(rj?i(?!$a;nSLOv zFPup(KK;Y9Ny%~BR@TIvD)!sF`N}>|hntLkS$Cc2Z?r#L6lLY=8pNg6TKL|-VVMG^ z5;6?#wcnrb9{BEo?;iN>f$tvp?t$+f`0j!49{BEo|6dREjMn=4T<&<1V#Y`W1{{8ppf5!z#|C^{Ri7& zJ?MjKIG|8J6Hus63g{S6WC050I|*#xiw9x^cIw4CK(g9as;)?@zQ(B`_RUDDO+)P(UGnAc5x)m``9x;rDR@Lke*OfdvGX^kQ8& z?ohviz!gOM0s=Pz3ct6Mzz+e1{7Q&jkpJZ`5efzA&0hwFIJNjN&o4EDjXDR0j>|HQNfs#C%k!&?IEUkR3|O@F+F>tV0$!A5iP?xzmpE=NhYIi;uxV!B8~#Cvw?j>dh{b= z2`1x(_vl}i4Q=CQG3g*%X2+s41T=;%m5ej&SOQx<3nbQ9OeSVWr*Ltc!J<(FIL@*q z+p<`E8iPxx&{$L^l?gI!J?0_^8U;-4=^6q{2m?j?xWJLvt8*oDuLtMKZoNpxRT?oHeNZ+r$Ol@%;I=vr71l+WlFc^ zp-R`nMvlvt`AR20A%|mC%5fmAR8Hb0C@1OabJ$OBDA{bXk|hdNve->Z#;ZFVM%!d% z&$~)rNxw?v&|T#m+PFN9JI(W8=PBUW zI-u4lO!FiELYVe%gl+$gFct9;^JRDh8U$8hB+8b=K+2Kf{BTS`0!!5ac8>B_pRk6m z5edOSlm2f9k>Qy4ks5NJSi|ZQEzCaAV%H~H7~e3Ka2)<}s{ZL{_KB%~IcNXW1i0d0 zN2rKzxKM;n5esE10CdluYS_0t$)IHGIJpfh2NhM&+>?m{wjcf^S z*L*(82Mb++h5e)PqtN@H&(uE9@rOC!933S^L9if)0SXKvWIgMg-?Pq%+aZMvd_kjA zs7w}|zAoURDUOz+QV?Q%igRRmj1Z3nY*Z?pL1scWLeb}v;x9{FXbaE;7>U(|f41lE z*MUIg_a^X3XmB`=<0L}cCk7Z`!Kh6Py3o65@&AP_s0F-5!>ct7c8f0&k5d4_`O!id zh_9qks0CMsW07*O2S{--9~blDP+m9)kb0f)nGeYJ>V`Wg2q!*c83+LZ070Nw-Ww^( zK@h_i3WT8hkeLsHN`VXnmNLK=h4I1Wg7yYbpZxzRp87lS9ZF$P=yo&~%NC{@V9ek1 zeky+vQIp^~i6A_wP=bJSQi{M_1bj526#{D}!exQKH}IGkj`8W39h1etF)Eczqmfy5 zAZDY}1QfCzm%`;z=s1fGDyUodK9@UtrN*N0vy z9qK%?GCgUcnXkRGc94^JNY-pE)`|lEjp{F~b{~!Cau}t%pPe%AX*SZZ>*NRTjNQ|h zeo#HOhBHGF<6d&*w&&V?C8u+}W5;&z3)gJ758s}W73z`vW1)e)ykv_sGCJqAv+mIs zJ_nziTzx!)o3*FGqE4TRl;t}9)W?*N?tWB>8PQRZ%b^nAx zQK63?Ps#mh%(aj?YpYlx1{ub@NZXn_^|RhBeHJu(b=k1T)aE%mWIR%3x*~I3jpq)| z)tC(Xw0qZXr3Qqps3|a5v14<0xcAytW>M{vefgnh=SMgu2jm;=eWbb9FJpM|%SEXH zT2=h)=b8@wS3}QoZXfH|P+{??*!!n<)yr&En~vw0VQwCW7EgX2>9@DsxQevvLgFpw zWhy~eXFX)`%7S*+9wK$QskW@&?D(t8WlPh{X1k7#C+#gu*A}$Ws%QLqp`Tyu(~BCB z4%!prC&q}ZPjoG|f0W%c^{71!5I)Uu|? z8%xM*+*BnwSCd2PoMJ-Tfxc=aQ%O)@R_BeVSd^4OB%8CP4UH*K_k zICAmuNsZ2Ycb%{H+b^0X~-c#4Md~fYP zRN-~9gcLi$LT_MnP^`!MS$FKq^Q}%Sjnu`oRAQ1_bfb<4z~{;+TR^NdZxlLIriPpNfM<;HaYMppE1Js+?Az^tM&5}j;EjA zQ+~e6`hr!Za=q42N0psL<44@KOQ;m??Gzh0*8j43LMHA07`|2M#g&;s;$wx4PZQ@m zbfp!Zt*y+=Z2q`JW~ESV^{=vd=MuQmhhlnt_29(pWAjW~56{UPkVntacB)-`cAOYB zn!37Ul;h5Vkn-`Dr-?_T>$yC1l(N{)IAfU05&N*V_T148dTDn}&EtxkZX z-rCNazBXGk*Z3yKauU;jjC4w+Z8-E(kh-mM)`_eQE9#D~ot@8E9DD4<=(G6sh}tgJ zfTnkAHgLVA)MdK*Qy%bVug$xzDdpW|?`;d?Z}g6Pzhn2M{)#4>M-zXb-SrAcJ=ou0 zKP5%EF3?r&rEAUA15LM9DXOK_6CHo`xj5!YdssjhTez?xHNN}J=F9Oj-W7$h@2shM Z(;XM9`1th35u=442S~Nil0m}|