From 5dd53c1258ea6f7bf2d8dd6a4e09644ee92a3c9b Mon Sep 17 00:00:00 2001 From: Andrea Bolognani Date: Tue, 28 Feb 2023 12:19:09 +0100 Subject: [PATCH 1/2] Make it possible for RPMRule.SetURLs() to fail This can't happen at the moment, but we're about to introduce a possible failure state. Signed-off-by: Andrea Bolognani --- cmd/rpmtree.go | 5 ++++- pkg/bazel/bazel.go | 12 +++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/cmd/rpmtree.go b/cmd/rpmtree.go index a1127ec..ebda4e6 100644 --- a/cmd/rpmtree.go +++ b/cmd/rpmtree.go @@ -72,7 +72,10 @@ func NewRpmTreeCmd() *cobra.Command { if err != nil { return err } - bazel.AddRPMs(workspace, install, rpmtreeopts.arch) + err = bazel.AddRPMs(workspace, install, rpmtreeopts.arch) + if err != nil { + return err + } bazel.AddTree(rpmtreeopts.name, build, install, rpmtreeopts.arch, rpmtreeopts.public) bazel.PruneRPMs(build, workspace) logrus.Info("Writing bazel files.") diff --git a/pkg/bazel/bazel.go b/pkg/bazel/bazel.go index 86aeee3..a036c95 100644 --- a/pkg/bazel/bazel.go +++ b/pkg/bazel/bazel.go @@ -63,7 +63,7 @@ func GetRPMs(workspace *build.File) (rpms []*RPMRule) { return } -func AddRPMs(workspace *build.File, pkgs []*api.Package, arch string) { +func AddRPMs(workspace *build.File, pkgs []*api.Package, arch string) error { rpms := map[string]*RPMRule{} @@ -83,7 +83,10 @@ func AddRPMs(workspace *build.File, pkgs []*api.Package, arch string) { rule.SetSHA256(pkg.Checksum.Text) urls := rule.URLs() if len(urls) == 0 { - rule.SetURLs(pkg.Repository.Mirrors, pkg.Location.Href) + err := rule.SetURLs(pkg.Repository.Mirrors, pkg.Location.Href) + if err != nil { + return err + } } } @@ -100,6 +103,8 @@ func AddRPMs(workspace *build.File, pkgs []*api.Package, arch string) { for _, rule := range rules { workspace.Stmt = edit.InsertAtEnd(workspace.Stmt, rule.Call) } + + return nil } func AddTar2Files(name string, rpmtree string, buildfile *build.File, files []string, public bool) { @@ -231,13 +236,14 @@ func (r *RPMRule) URLs() []string { return nil } -func (r *RPMRule) SetURLs(urls []string, href string) { +func (r *RPMRule) SetURLs(urls []string, href string) error { urlsAttr := []build.Expr{} for _, url := range urls { u := strings.TrimSuffix(url, "/") + "/" + strings.TrimSuffix(href, "/") urlsAttr = append(urlsAttr, &build.StringExpr{Value: u}) } r.Rule.SetAttr("urls", &build.ListExpr{List: urlsAttr}) + return nil } func (r *RPMRule) SetName(name string) { From ced018c4df15716a3a4d1b5f018fa377786b7d21 Mon Sep 17 00:00:00 2001 From: Andrea Bolognani Date: Tue, 28 Feb 2023 12:20:06 +0100 Subject: [PATCH 2/2] Escape URLs before adding them to bazel rules Currently we produce the final URL by performing string concatenation, which works fine in most cases. The URL, however, might contain special characters that bazel is later unable to process correctly. One such example: Error downloading passt-0^20221110.g4129764-1.el9.x86_64.rpm: Illegal character in path at index 78: https://mirror.stream.centos.org/9-stream/AppStream/x86_64/os/Packages/passt-0^20221110.g4129764-1.el9.x86_64.rpm This issue made it necessary to manually tweak the WORKSPACE file generated by bazeldnf: https://github.com/kubevirt/kubevirt/commit/8052a7f3e30b221550bc8d49d74b77eaf5b4998a To solve this entire class of issues, use the functionality offered by the net/url package to manipulare URLs. This will automatically take care of performing all the necessary escaping, as well as avoiding unnecessary slashes. Signed-off-by: Andrea Bolognani --- pkg/bazel/bazel.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/bazel/bazel.go b/pkg/bazel/bazel.go index a036c95..c44695e 100644 --- a/pkg/bazel/bazel.go +++ b/pkg/bazel/bazel.go @@ -3,6 +3,7 @@ package bazel import ( "fmt" "io/ioutil" + "net/url" "path/filepath" "sort" "strings" @@ -236,11 +237,15 @@ func (r *RPMRule) URLs() []string { return nil } -func (r *RPMRule) SetURLs(urls []string, href string) error { +func (r *RPMRule) SetURLs(mirrors []string, href string) error { urlsAttr := []build.Expr{} - for _, url := range urls { - u := strings.TrimSuffix(url, "/") + "/" + strings.TrimSuffix(href, "/") - urlsAttr = append(urlsAttr, &build.StringExpr{Value: u}) + for _, mirror := range mirrors { + u, err := url.Parse(mirror) + if err != nil { + return err + } + u = u.JoinPath(href) + urlsAttr = append(urlsAttr, &build.StringExpr{Value: u.String()}) } r.Rule.SetAttr("urls", &build.ListExpr{List: urlsAttr}) return nil