diff --git a/Dockerfile.downloads b/Dockerfile.downloads index 4b596209232..2929de86560 100644 --- a/Dockerfile.downloads +++ b/Dockerfile.downloads @@ -1,6 +1,6 @@ -FROM quay.io/openshift/origin-cli-artifacts:4.18 AS origincli +FROM quay.io/openshift/origin-cli-artifacts:4.22 AS origincli -FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.22-openshift-4.18 AS gobuilder +FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.25-openshift-4.22 AS gobuilder ENV HOME=/go/src/github.com/openshift/console/ RUN mkdir -p ${HOME} @@ -14,7 +14,7 @@ COPY .git/ /${HOME}.git/ WORKDIR ${HOME} RUN ./build-downloads.sh -FROM registry.ci.openshift.org/ocp/4.18:base-rhel9 +FROM registry.access.redhat.com/ubi9/ubi-minimal:latest RUN mkdir -p /opt/downloads COPY --from=gobuilder /go/src/github.com/openshift/console/bin/downloads /opt/downloads @@ -30,7 +30,7 @@ CMD ["/opt/downloads/downloads", "--config-path=/opt/downloads/defaultArtifacts LABEL \ io.k8s.display-name="CLI Artifacts Downloads Server" \ io.k8s.description="This is a component of OpenShift Container Platform and provides a Golang server that serves 'oc' binaries for various platforms." \ - maintainer="Marek Ziska " \ + maintainer="Jakub Hadvig " \ License="Apache 2.0" \ vendor="Red Hat" \ io.openshift.tags="openshift,cli-artifacts" \ \ No newline at end of file diff --git a/cmd/downloads/config/defaultArtifactsConfig.yaml b/cmd/downloads/config/defaultArtifactsConfig.yaml index bbbdf8959a2..d86a7c674ad 100644 --- a/cmd/downloads/config/defaultArtifactsConfig.yaml +++ b/cmd/downloads/config/defaultArtifactsConfig.yaml @@ -2,6 +2,12 @@ defaultArtifactsConfig: - arch: amd64 operatingSystem: linux path: /usr/share/openshift/linux_amd64/oc + - arch: amd64 + operatingSystem: linux + path: /usr/share/openshift/linux_amd64/oc.rhel8 + - arch: amd64 + operatingSystem: linux + path: /usr/share/openshift/linux_amd64/oc.rhel9 - arch: amd64 operatingSystem: mac path: /usr/share/openshift/mac/oc @@ -11,12 +17,30 @@ defaultArtifactsConfig: - arch: arm64 operatingSystem: linux path: /usr/share/openshift/linux_arm64/oc + - arch: arm64 + operatingSystem: linux + path: /usr/share/openshift/linux_arm64/oc.rhel8 + - arch: arm64 + operatingSystem: linux + path: /usr/share/openshift/linux_arm64/oc.rhel9 - arch: arm64 operatingSystem: mac path: /usr/share/openshift/mac_arm64/oc - arch: ppc64le operatingSystem: linux path: /usr/share/openshift/linux_ppc64le/oc + - arch: ppc64le + operatingSystem: linux + path: /usr/share/openshift/linux_ppc64le/oc.rhel8 + - arch: ppc64le + operatingSystem: linux + path: /usr/share/openshift/linux_ppc64le/oc.rhel9 + - arch: s390x + operatingSystem: linux + path: /usr/share/openshift/linux_s390x/oc + - arch: s390x + operatingSystem: linux + path: /usr/share/openshift/linux_s390x/oc.rhel8 - arch: s390x operatingSystem: linux - path: /usr/share/openshift/linux_s390x/oc \ No newline at end of file + path: /usr/share/openshift/linux_s390x/oc.rhel9 \ No newline at end of file diff --git a/cmd/downloads/config/downloads_config.go b/cmd/downloads/config/downloads_config.go index 1719cbf7041..e61ec00239e 100644 --- a/cmd/downloads/config/downloads_config.go +++ b/cmd/downloads/config/downloads_config.go @@ -5,8 +5,11 @@ import ( "archive/zip" "fmt" "io" + "net/http" "os" "path/filepath" + "strings" + "sync" "text/template" "gopkg.in/yaml.v3" @@ -55,6 +58,8 @@ type DownloadsServerConfig struct { Spec []ArtifactSpec TempDir string TemplateHTML *template.Template + // archivesReady is closed once all background archive creation goroutines complete. + archivesReady chan struct{} } const indexFileName = "index.html" @@ -210,6 +215,18 @@ func addFileToZip(zw *zip.Writer, filename string) error { return nil } +// displayName returns a human-readable label like "amd64 linux" or "amd64 linux - RHEL 8". +func displayName(arch, os, basename string) string { + name := fmt.Sprintf("%s %s", arch, os) + base := strings.TrimSuffix(basename, ".exe") + if strings.HasSuffix(base, ".rhel8") { + name += " - RHEL 8" + } else if strings.HasSuffix(base, ".rhel9") { + name += " - RHEL 9" + } + return name +} + func configureArchivePath(pathToTargetFile string) string { if filepath.Ext(pathToTargetFile) == ".exe" { // Remove the .exe extension from the path @@ -323,29 +340,66 @@ func (downloadsConfig *DownloadsServerConfig) generateDirFileContents() ([]ListI return nil, err } - err = createArchive(artifactPath, ".tar") - if err != nil { - return nil, err - } - err = createArchive(artifactPath, ".zip") - if err != nil { - return nil, err - } - - pathToArchive := configureArchivePath(artifactPath) - // append new entry in the list of links to artifacts + relPath := filepath.Join(spec.Arch, spec.OperatingSystem, basename) + relArchivePath := configureArchivePath(relPath) content = append(content, ListItemLink{ Type: Binary, - URL: artifactPath, - Name: fmt.Sprintf("%s %s", spec.Arch, spec.OperatingSystem), - TarURL: fmt.Sprintf("%s.tar", pathToArchive), - ZipURL: fmt.Sprintf("%s.zip", pathToArchive), + URL: relPath, + Name: displayName(spec.Arch, spec.OperatingSystem, basename), + TarURL: fmt.Sprintf("%s.tar", relArchivePath), + ZipURL: fmt.Sprintf("%s.zip", relArchivePath), }) } return content, nil } +// CreateArchivesInBackground spawns goroutines to create tar and zip archives +// for every artifact concurrently. It closes archivesReady when all archives +// have been written so that Handler() can unblock waiting requests. +func (c *DownloadsServerConfig) CreateArchivesInBackground() { + c.archivesReady = make(chan struct{}) + var wg sync.WaitGroup + + for _, spec := range c.Spec { + basename := filepath.Base(spec.Path) + artifactPath := filepath.Join(c.TempDir, spec.Arch, spec.OperatingSystem, basename) + + wg.Add(2) + go func(p string) { + defer wg.Done() + if err := createArchive(p, ".tar"); err != nil { + klog.Errorf("Failed to create tar archive for %s: %v", p, err) + } + }(artifactPath) + go func(p string) { + defer wg.Done() + if err := createArchive(p, ".zip"); err != nil { + klog.Errorf("Failed to create zip archive for %s: %v", p, err) + } + }(artifactPath) + } + + go func() { + wg.Wait() + close(c.archivesReady) + klog.Info("All archives created successfully") + }() +} + +// Handler returns an http.Handler that serves files from TempDir. Requests for +// .tar or .zip archives block until background archive creation is complete. +func (c *DownloadsServerConfig) Handler() http.Handler { + fs := http.FileServer(http.Dir(c.TempDir)) + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ext := filepath.Ext(r.URL.Path) + if ext == ".tar" || ext == ".zip" { + <-c.archivesReady + } + fs.ServeHTTP(w, r) + }) +} + // setupArtifactsDirectory creates the root HTML file and directories, files and archives for artifacts func (artifactsConfig *DownloadsServerConfig) setupArtifactsDirectory() error { // symlink file in the temporary directory that points to the openshift license diff --git a/cmd/downloads/config/downloads_config_test.go b/cmd/downloads/config/downloads_config_test.go new file mode 100644 index 00000000000..1b5b3b00bd3 --- /dev/null +++ b/cmd/downloads/config/downloads_config_test.go @@ -0,0 +1,269 @@ +package config + +import ( + "archive/tar" + "archive/zip" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "text/template" + "time" +) + +func TestDisplayName(t *testing.T) { + tests := []struct { + arch, os, basename, expected string + }{ + {"amd64", "linux", "oc", "amd64 linux"}, + {"amd64", "linux", "oc.rhel8", "amd64 linux - RHEL 8"}, + {"amd64", "linux", "oc.rhel9", "amd64 linux - RHEL 9"}, + {"arm64", "linux", "oc.rhel8", "arm64 linux - RHEL 8"}, + {"ppc64le", "linux", "oc.rhel9", "ppc64le linux - RHEL 9"}, + {"amd64", "mac", "oc", "amd64 mac"}, + {"amd64", "windows", "oc.exe", "amd64 windows"}, + } + + for _, tt := range tests { + got := displayName(tt.arch, tt.os, tt.basename) + if got != tt.expected { + t.Errorf("displayName(%q, %q, %q) = %q, want %q", tt.arch, tt.os, tt.basename, got, tt.expected) + } + } +} + +func TestConfigureArchivePath(t *testing.T) { + tests := []struct { + input, expected string + }{ + {"/tmp/amd64/linux/oc", "/tmp/amd64/linux/oc"}, + {"/tmp/amd64/linux/oc.rhel8", "/tmp/amd64/linux/oc.rhel8"}, + {"/tmp/amd64/linux/oc.rhel9", "/tmp/amd64/linux/oc.rhel9"}, + {"/tmp/amd64/windows/oc.exe", "/tmp/amd64/windows/oc"}, + } + + for _, tt := range tests { + got := configureArchivePath(tt.input) + if got != tt.expected { + t.Errorf("configureArchivePath(%q) = %q, want %q", tt.input, got, tt.expected) + } + } +} + +func newTestConfig(t *testing.T, specs []ArtifactSpec) *DownloadsServerConfig { + t.Helper() + tempDir := t.TempDir() + + tmpl, err := template.New("artifacts").Parse(templateStringHTML) + if err != nil { + t.Fatalf("failed to parse template: %v", err) + } + + for _, spec := range specs { + if err := os.MkdirAll(filepath.Dir(spec.Path), 0755); err != nil { + t.Fatalf("failed to create source dir: %v", err) + } + if err := os.WriteFile(spec.Path, []byte("fake-binary"), 0755); err != nil { + t.Fatalf("failed to write fake binary: %v", err) + } + } + + return &DownloadsServerConfig{ + Port: "0", + Spec: specs, + TempDir: tempDir, + TemplateHTML: tmpl, + } +} + +func TestGenerateDirFileContents_RelativeURLs(t *testing.T) { + srcDir := t.TempDir() + specs := []ArtifactSpec{ + {Arch: "amd64", OperatingSystem: "linux", Path: filepath.Join(srcDir, "oc")}, + {Arch: "amd64", OperatingSystem: "windows", Path: filepath.Join(srcDir, "oc.exe")}, + } + cfg := newTestConfig(t, specs) + + content, err := cfg.generateDirFileContents() + if err != nil { + t.Fatalf("generateDirFileContents() error: %v", err) + } + + for _, item := range content { + if item.Type == License { + continue + } + for _, u := range []string{item.URL, item.TarURL, item.ZipURL} { + if strings.Contains(u, cfg.TempDir) { + t.Errorf("URL %q contains TempDir %q; expected relative path", u, cfg.TempDir) + } + if filepath.IsAbs(u) { + t.Errorf("URL %q is an absolute path; expected relative", u) + } + } + } + + linux := content[1] + if linux.URL != "amd64/linux/oc" { + t.Errorf("expected URL %q, got %q", "amd64/linux/oc", linux.URL) + } + if linux.TarURL != "amd64/linux/oc.tar" { + t.Errorf("expected TarURL %q, got %q", "amd64/linux/oc.tar", linux.TarURL) + } + + win := content[2] + if win.URL != "amd64/windows/oc.exe" { + t.Errorf("expected URL %q, got %q", "amd64/windows/oc.exe", win.URL) + } + if win.TarURL != "amd64/windows/oc.tar" { + t.Errorf("expected TarURL %q, got %q", "amd64/windows/oc.tar", win.TarURL) + } + if win.ZipURL != "amd64/windows/oc.zip" { + t.Errorf("expected ZipURL %q, got %q", "amd64/windows/oc.zip", win.ZipURL) + } +} + +func TestCreateArchivesInBackground(t *testing.T) { + srcDir := t.TempDir() + specs := []ArtifactSpec{ + {Arch: "amd64", OperatingSystem: "linux", Path: filepath.Join(srcDir, "oc")}, + {Arch: "arm64", OperatingSystem: "mac", Path: filepath.Join(srcDir, "oc-mac")}, + } + cfg := newTestConfig(t, specs) + + if _, err := cfg.generateDirFileContents(); err != nil { + t.Fatalf("generateDirFileContents() error: %v", err) + } + + cfg.CreateArchivesInBackground() + + select { + case <-cfg.archivesReady: + case <-time.After(10 * time.Second): + t.Fatal("timed out waiting for archives to complete") + } + + for _, spec := range specs { + basename := filepath.Base(spec.Path) + base := filepath.Join(cfg.TempDir, spec.Arch, spec.OperatingSystem, basename) + + tarPath := base + ".tar" + if _, err := os.Stat(tarPath); err != nil { + t.Errorf("tar archive not found at %s: %v", tarPath, err) + continue + } + f, _ := os.Open(tarPath) + tr := tar.NewReader(f) + hdr, err := tr.Next() + f.Close() + if err != nil { + t.Errorf("failed to read tar %s: %v", tarPath, err) + } else if hdr.Name != basename { + t.Errorf("tar entry name = %q, want %q", hdr.Name, basename) + } + + zipPath := base + ".zip" + if _, err := os.Stat(zipPath); err != nil { + t.Errorf("zip archive not found at %s: %v", zipPath, err) + continue + } + zr, err := zip.OpenReader(zipPath) + if err != nil { + t.Errorf("failed to open zip %s: %v", zipPath, err) + } else { + if len(zr.File) != 1 || zr.File[0].Name != basename { + t.Errorf("zip entry name = %q, want %q", zr.File[0].Name, basename) + } + zr.Close() + } + } +} + +func TestHandler_BlocksUntilArchivesReady(t *testing.T) { + tmpDir := t.TempDir() + if err := os.WriteFile(filepath.Join(tmpDir, "index.html"), []byte("ok"), 0644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(tmpDir, "amd64", "linux"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "amd64", "linux", "oc.tar"), []byte("archive"), 0644); err != nil { + t.Fatal(err) + } + + cfg := &DownloadsServerConfig{ + TempDir: tmpDir, + archivesReady: make(chan struct{}), + } + + handler := cfg.Handler() + + t.Run("non-archive requests are served immediately", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Errorf("expected 200 for /, got %d", rec.Code) + } + }) + + t.Run("archive request blocks then succeeds", func(t *testing.T) { + done := make(chan int, 1) + go func() { + req := httptest.NewRequest(http.MethodGet, "/amd64/linux/oc.tar", nil) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + done <- rec.Code + }() + + select { + case <-done: + t.Fatal("archive request returned before archivesReady was closed") + case <-time.After(100 * time.Millisecond): + } + + close(cfg.archivesReady) + + select { + case code := <-done: + if code != http.StatusOK { + t.Errorf("expected 200 for oc.tar after ready, got %d", code) + } + case <-time.After(5 * time.Second): + t.Fatal("archive request did not complete after archivesReady was closed") + } + }) +} + +func TestHandler_ServesArchiveImmediatelyWhenReady(t *testing.T) { + tmpDir := t.TempDir() + if err := os.MkdirAll(filepath.Join(tmpDir, "amd64", "linux"), 0755); err != nil { + t.Fatal(err) + } + archiveContent := []byte("archive-data") + if err := os.WriteFile(filepath.Join(tmpDir, "amd64", "linux", "oc.tar"), archiveContent, 0644); err != nil { + t.Fatal(err) + } + + ready := make(chan struct{}) + close(ready) + cfg := &DownloadsServerConfig{ + TempDir: tmpDir, + archivesReady: ready, + } + + req := httptest.NewRequest(http.MethodGet, "/amd64/linux/oc.tar", nil) + rec := httptest.NewRecorder() + cfg.Handler().ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Errorf("expected 200, got %d", rec.Code) + } + body, _ := io.ReadAll(rec.Result().Body) + if string(body) != string(archiveContent) { + t.Errorf("expected body %q, got %q", archiveContent, body) + } +} diff --git a/cmd/downloads/main.go b/cmd/downloads/main.go index 53598a36d49..f04e1b88d34 100644 --- a/cmd/downloads/main.go +++ b/cmd/downloads/main.go @@ -16,7 +16,7 @@ func main() { klog.InitFlags(fs) defer klog.Flush() - fPort := fs.Int("port", 8081, "Port number used to start the downloads server.") + fPort := fs.Int("port", 8080, "Port number used to start the downloads server.") fPathToArtifactsFileConfig := fs.String("config-path", "/opt/downloads/defaultArtifactsConfig.yaml", "Path to the configuration file of available 'oc' artifacts.") if err := fs.Parse(os.Args[1:]); err != nil { @@ -30,13 +30,15 @@ func main() { } defer os.RemoveAll(downloadsServerConfig.TempDir) + downloadsServerConfig.CreateArchivesInBackground() + // Listen for incoming connections klog.Infof("Server started. Listening on http://0.0.0.0:%s", downloadsServerConfig.Port) // Serve the files and listen for incoming connections downlsrv := &http.Server{ Addr: fmt.Sprintf("0.0.0.0:%s", downloadsServerConfig.Port), - Handler: http.FileServer(http.Dir(downloadsServerConfig.TempDir)), + Handler: downloadsServerConfig.Handler(), } klog.Fatal(downlsrv.ListenAndServe())