From 20d83c1ee6b5a1d4005c2d1657dc243dad04a35b Mon Sep 17 00:00:00 2001 From: Anik Bhattacharjee Date: Mon, 18 Sep 2023 16:45:24 -0400 Subject: [PATCH] (server) Expose content URL on CR status (#168) closes #119 Signed-off-by: Anik --- api/core/v1alpha1/catalog_types.go | 3 ++ cmd/manager/main.go | 10 +++++- ...atalogd.operatorframework.io_catalogs.yaml | 4 +++ config/default/manager_auth_proxy_patch.yaml | 1 + docs/fetching-catalog-contents.md | 31 ++++++++++++++----- pkg/controllers/core/catalog_controller.go | 10 ++++-- .../core/catalog_controller_test.go | 4 +++ pkg/storage/localdir.go | 8 ++++- pkg/storage/localdir_test.go | 12 +++++-- pkg/storage/storage.go | 1 + test/e2e/unpack_test.go | 7 ++--- 11 files changed, 73 insertions(+), 18 deletions(-) diff --git a/api/core/v1alpha1/catalog_types.go b/api/core/v1alpha1/catalog_types.go index fc7e693a..f523d362 100644 --- a/api/core/v1alpha1/catalog_types.go +++ b/api/core/v1alpha1/catalog_types.go @@ -81,6 +81,9 @@ type CatalogStatus struct { ResolvedSource *CatalogSource `json:"resolvedSource,omitempty"` Phase string `json:"phase,omitempty"` + // ContentURL is a cluster-internal address that on-cluster components + // can read the content of a catalog from + ContentURL string `json:"contentURL,omitempty"` } // CatalogSource contains the sourcing information for a Catalog diff --git a/cmd/manager/main.go b/cmd/manager/main.go index f4b0f2da..edabbe66 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -20,6 +20,7 @@ import ( "flag" "fmt" "net/http" + "net/url" "os" "time" @@ -73,6 +74,7 @@ func main() { systemNamespace string storageDir string catalogServerAddr string + httpExternalAddr string ) flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") @@ -84,6 +86,7 @@ func main() { flag.StringVar(&systemNamespace, "system-namespace", "", "The namespace catalogd uses for internal state, configuration, and workloads") flag.StringVar(&storageDir, "catalogs-storage-dir", "/var/cache/catalogs", "The directory in the filesystem where unpacked catalog content will be stored and served from") flag.StringVar(&catalogServerAddr, "catalogs-server-addr", ":8083", "The address where the unpacked catalogs' content will be accessible") + flag.StringVar(&httpExternalAddr, "http-external-address", "http://catalogd-catalogserver.catalogd-system.svc", "The external address at which the http server is reachable.") flag.BoolVar(&profiling, "profiling", false, "enable profiling endpoints to allow for using pprof") flag.BoolVar(&catalogdVersion, "version", false, "print the catalogd version and exit") opts := zap.Options{ @@ -135,7 +138,12 @@ func main() { os.Exit(1) } - localStorage = storage.LocalDir{RootDir: storageDir} + baseStorageURL, err := url.Parse(fmt.Sprintf("%s/catalogs/", httpExternalAddr)) + if err != nil { + setupLog.Error(err, "unable to create base storage URL") + os.Exit(1) + } + localStorage = storage.LocalDir{RootDir: storageDir, BaseURL: baseStorageURL} shutdownTimeout := 30 * time.Second catalogServer := server.Server{ Kind: "catalogs", diff --git a/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml b/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml index a8367bc5..928e9ad6 100644 --- a/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml +++ b/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml @@ -135,6 +135,10 @@ spec: - type type: object type: array + contentURL: + description: ContentURL is a cluster-internal address that on-cluster + components can read the content of a catalog from + type: string phase: type: string resolvedSource: diff --git a/config/default/manager_auth_proxy_patch.yaml b/config/default/manager_auth_proxy_patch.yaml index 9fde74dd..bdd36ec0 100644 --- a/config/default/manager_auth_proxy_patch.yaml +++ b/config/default/manager_auth_proxy_patch.yaml @@ -52,4 +52,5 @@ spec: - "--leader-elect" - "--catalogs-storage-dir=/var/cache/catalogs" - "--feature-gates=HTTPServer=true" + - "--http-external-address=http://catalogd-catalogserver.catalogd-system.svc" diff --git a/docs/fetching-catalog-contents.md b/docs/fetching-catalog-contents.md index fe7e46a0..ba19320d 100644 --- a/docs/fetching-catalog-contents.md +++ b/docs/fetching-catalog-contents.md @@ -3,15 +3,32 @@ This document covers how to fetch the contents for a `Catalog` from the Catalogd HTTP Server that runs when the `HTTPServer` feature-gate is enabled (enabled by default). -All `Catalog`s currently have their contents served via the following endpoint pattern: -`http://{httpServerBaseUrl}/catalogs/{Catalog.Name}/all.json` +For example purposes we make the following assumption: +- A `Catalog` named `operatorhubio` has been created and successfully unpacked +(denoted in the `Catalog.Status`) + +`Catalog` CRs have a status.contentURL field whose value is the location where the content +of a catalog can be read from: + +```yaml + status: + conditions: + - lastTransitionTime: "2023-09-14T15:21:18Z" + message: successfully unpacked the catalog image "quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76" + reason: UnpackSuccessful + status: "True" + type: Unpacked + contentURL: http://catalogd-catalogserver.catalogd-system.svc/catalogs/operatorhubio/all.json + phase: Unpacked + resolvedSource: + image: + ref: quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76 + type: image +``` All responses will be a JSON stream where each JSON object is a File-Based Catalog (FBC) object. -For example purposes we make the following assumption: -- A `Catalog` named `operatorhubio` has been created and successfully unpacked -(denoted in the `Catalog.Status`) ## On cluster @@ -30,11 +47,11 @@ When making a request for the contents of the `operatorhubio` `Catalog` from out the cluster, we have to perform an extra step: 1. Port forward the `catalogd-catalogserver` service in the `catalogd-system` namespace: ```sh -kubectl -n catalogd-system port-forward svc/catalogd-catalogserver :80 +kubectl -n catalogd-system port-forward svc/catalogd-catalogserver 8080:80 ``` Once the service has been successfully forwarded to a localhost port, issue a HTTP `GET` -request to `http://localhost:/catalogs/operatorhubio/all.json` +request to `http://localhost:8080/catalogs/operatorhubio/all.json` An example `curl` request that assumes the port-forwarding is mapped to port 8080 on the local machine: ```sh diff --git a/pkg/controllers/core/catalog_controller.go b/pkg/controllers/core/catalog_controller.go index 7b4bb881..b69750c6 100644 --- a/pkg/controllers/core/catalog_controller.go +++ b/pkg/controllers/core/catalog_controller.go @@ -128,16 +128,19 @@ func (r *CatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.Cat updateStatusUnpacking(&catalog.Status, unpackResult) return ctrl.Result{}, nil case source.StateUnpacked: + contentURL := "" // TODO: We should check to see if the unpacked result has the same content // as the already unpacked content. If it does, we should skip this rest // of the unpacking steps. if features.CatalogdFeatureGate.Enabled(features.HTTPServer) { - if err := r.Storage.Store(catalog.Name, unpackResult.FS); err != nil { + err := r.Storage.Store(catalog.Name, unpackResult.FS) + if err != nil { return ctrl.Result{}, updateStatusStorageError(&catalog.Status, fmt.Errorf("error storing fbc: %v", err)) } + contentURL = r.Storage.ContentURL(catalog.Name) } - updateStatusUnpacked(&catalog.Status, unpackResult) + updateStatusUnpacked(&catalog.Status, unpackResult, contentURL) return ctrl.Result{}, nil default: return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("unknown unpack state %q: %v", unpackResult.State, err)) @@ -166,8 +169,9 @@ func updateStatusUnpacking(status *v1alpha1.CatalogStatus, result *source.Result }) } -func updateStatusUnpacked(status *v1alpha1.CatalogStatus, result *source.Result) { +func updateStatusUnpacked(status *v1alpha1.CatalogStatus, result *source.Result, contentURL string) { status.ResolvedSource = result.ResolvedSource + status.ContentURL = contentURL status.Phase = v1alpha1.PhaseUnpacked meta.SetStatusCondition(&status.Conditions, metav1.Condition{ Type: v1alpha1.TypeUnpacked, diff --git a/pkg/controllers/core/catalog_controller_test.go b/pkg/controllers/core/catalog_controller_test.go index 89f9f274..ba2f26bb 100644 --- a/pkg/controllers/core/catalog_controller_test.go +++ b/pkg/controllers/core/catalog_controller_test.go @@ -68,6 +68,10 @@ func (m MockStore) Delete(_ string) error { return nil } +func (m MockStore) ContentURL(_ string) string { + return "URL" +} + func (m MockStore) StorageServerHandler() http.Handler { panic("not needed") } diff --git a/pkg/storage/localdir.go b/pkg/storage/localdir.go index fbbcda9a..76ffca46 100644 --- a/pkg/storage/localdir.go +++ b/pkg/storage/localdir.go @@ -4,6 +4,7 @@ import ( "fmt" "io/fs" "net/http" + "net/url" "os" "path/filepath" @@ -17,6 +18,7 @@ import ( // atomic view of the content for a catalog. type LocalDir struct { RootDir string + BaseURL *url.URL } func (s LocalDir) Store(catalog string, fsys fs.FS) error { @@ -47,9 +49,13 @@ func (s LocalDir) Delete(catalog string) error { return os.RemoveAll(filepath.Join(s.RootDir, catalog)) } +func (s LocalDir) ContentURL(catalog string) string { + return fmt.Sprintf("%s%s/all.json", s.BaseURL, catalog) +} + func (s LocalDir) StorageServerHandler() http.Handler { mux := http.NewServeMux() - mux.Handle("/catalogs/", http.StripPrefix("/catalogs/", http.FileServer(http.FS(&filesOnlyFilesystem{os.DirFS(s.RootDir)})))) + mux.Handle(s.BaseURL.Path, http.StripPrefix(s.BaseURL.Path, http.FileServer(http.FS(&filesOnlyFilesystem{os.DirFS(s.RootDir)})))) return mux } diff --git a/pkg/storage/localdir_test.go b/pkg/storage/localdir_test.go index aec96c65..ee9f1436 100644 --- a/pkg/storage/localdir_test.go +++ b/pkg/storage/localdir_test.go @@ -6,6 +6,7 @@ import ( "io/fs" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "testing/fstest" @@ -16,11 +17,14 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" ) +const urlPrefix = "/catalogs/" + var _ = Describe("LocalDir Storage Test", func() { var ( catalog = "test-catalog" store Instance rootDir string + baseURL *url.URL testBundleName = "bundle.v0.0.1" testBundleImage = "quaydock.io/namespace/bundle:0.0.3" testBundleRelatedImageName = "test" @@ -40,7 +44,8 @@ var _ = Describe("LocalDir Storage Test", func() { Expect(err).ToNot(HaveOccurred()) rootDir = d - store = LocalDir{RootDir: rootDir} + baseURL = &url.URL{Scheme: "http", Host: "test-addr", Path: urlPrefix} + store = LocalDir{RootDir: rootDir, BaseURL: baseURL} unpackResultFS = &fstest.MapFS{ "bundle.yaml": &fstest.MapFile{Data: []byte(testBundle), Mode: os.ModePerm}, "package.yaml": &fstest.MapFile{Data: []byte(testPackage), Mode: os.ModePerm}, @@ -64,6 +69,9 @@ var _ = Describe("LocalDir Storage Test", func() { diff := cmp.Diff(gotConfig, storedConfig) Expect(diff).To(Equal("")) }) + It("should form the content URL correctly", func() { + Expect(store.ContentURL(catalog)).To(Equal(fmt.Sprintf("%s%s/all.json", baseURL, catalog))) + }) When("The stored content is deleted", func() { BeforeEach(func() { err := store.Delete(catalog) @@ -88,7 +96,7 @@ var _ = Describe("LocalDir Server Handler tests", func() { d, err := os.MkdirTemp(GinkgoT().TempDir(), "cache") Expect(err).ToNot(HaveOccurred()) Expect(os.Mkdir(filepath.Join(d, "test-catalog"), 0700)).To(Succeed()) - store = LocalDir{RootDir: d} + store = LocalDir{RootDir: d, BaseURL: &url.URL{Path: urlPrefix}} testServer = httptest.NewServer(store.StorageServerHandler()) }) diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 1b244973..7a7e57ee 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -12,5 +12,6 @@ import ( type Instance interface { Store(catalog string, fsys fs.FS) error Delete(catalog string) error + ContentURL(catalog string) string StorageServerHandler() http.Handler } diff --git a/test/e2e/unpack_test.go b/test/e2e/unpack_test.go index 73939365..0e9c9b55 100644 --- a/test/e2e/unpack_test.go +++ b/test/e2e/unpack_test.go @@ -2,7 +2,6 @@ package e2e import ( "context" - "fmt" "io" "os" @@ -81,8 +80,8 @@ var _ = Describe("Catalog Unpacking", func() { }).Should(Succeed()) By("Making sure the catalog content is available via the http server") - // (TODO): Get the URL from the CR once https://github.com/operator-framework/catalogd/issues/119 is done - catalogURL := fmt.Sprintf("%s.%s.svc/catalogs/%s/all.json", "catalogd-catalogserver", defaultSystemNamespace, catalogName) + err = c.Get(ctx, types.NamespacedName{Name: catalog.Name}, catalog) + Expect(err).ToNot(HaveOccurred()) job = batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Name: "test-svr-job", @@ -95,7 +94,7 @@ var _ = Describe("Catalog Unpacking", func() { { Name: "test-svr", Image: "curlimages/curl", - Command: []string{"sh", "-c", "curl --silent --show-error --location -o - " + catalogURL}, + Command: []string{"sh", "-c", "curl --silent --show-error --location -o - " + catalog.Status.ContentURL}, }, }, RestartPolicy: "Never",