From fd8cef22d29d18ad1b0ad7cb07f8545513533773 Mon Sep 17 00:00:00 2001 From: Anik Date: Thu, 14 Sep 2023 10:11:18 -0400 Subject: [PATCH] (server) Expose content URL on CR status closes #119 Signed-off-by: Anik --- api/core/v1alpha1/catalog_types.go | 1 + cmd/manager/main.go | 11 +++++++---- .../catalogd.operatorframework.io_catalogs.yaml | 2 ++ config/default/manager_auth_proxy_patch.yaml | 1 + pkg/controllers/core/catalog_controller.go | 15 ++++++++++----- pkg/controllers/core/catalog_controller_test.go | 6 +++--- pkg/storage/localdir.go | 15 ++++++++------- pkg/storage/localdir_test.go | 4 ++-- pkg/storage/storage.go | 2 +- test/e2e/unpack_test.go | 6 +++--- 10 files changed, 38 insertions(+), 25 deletions(-) diff --git a/api/core/v1alpha1/catalog_types.go b/api/core/v1alpha1/catalog_types.go index fc7e693a..7ca5ddef 100644 --- a/api/core/v1alpha1/catalog_types.go +++ b/api/core/v1alpha1/catalog_types.go @@ -81,6 +81,7 @@ type CatalogStatus struct { ResolvedSource *CatalogSource `json:"resolvedSource,omitempty"` Phase string `json:"phase,omitempty"` + 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..43782582 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -73,6 +73,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 +85,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://localhost:8080", "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 +137,7 @@ func main() { os.Exit(1) } - localStorage = storage.LocalDir{RootDir: storageDir} + localStorage = storage.LocalDir{RootDir: storageDir, URLPrefix: "/catalogs/"} shutdownTimeout := 30 * time.Second catalogServer := server.Server{ Kind: "catalogs", @@ -155,9 +157,10 @@ func main() { } if err = (&corecontrollers.CatalogReconciler{ - Client: mgr.GetClient(), - Unpacker: unpacker, - Storage: localStorage, + Client: mgr.GetClient(), + Unpacker: unpacker, + Storage: localStorage, + HTTPExternalAddr: httpExternalAddr, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Catalog") os.Exit(1) diff --git a/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml b/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml index a8367bc5..e9c5c1e3 100644 --- a/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml +++ b/config/crd/bases/catalogd.operatorframework.io_catalogs.yaml @@ -135,6 +135,8 @@ spec: - type type: object type: array + contentURL: + 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 b24c81b9..9532d1f3 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=CatalogMetadataAPI=true,HTTPServer=true" + - "--http-external-address=catalogd-catalogserver.catalogd-system.svc" diff --git a/pkg/controllers/core/catalog_controller.go b/pkg/controllers/core/catalog_controller.go index 31b4ca82..e2059e3a 100644 --- a/pkg/controllers/core/catalog_controller.go +++ b/pkg/controllers/core/catalog_controller.go @@ -50,8 +50,9 @@ const fbcDeletionFinalizer = "catalogd.operatorframework.io/delete-server-cache" // CatalogReconciler reconciles a Catalog object type CatalogReconciler struct { client.Client - Unpacker source.Unpacker - Storage storage.Instance + Unpacker source.Unpacker + Storage storage.Instance + HTTPExternalAddr string } //+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=catalogs,verbs=get;list;watch;create;update;patch;delete @@ -138,13 +139,16 @@ 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 { + location, 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 = fmt.Sprintf("%s%s", r.HTTPExternalAddr, location) } if features.CatalogdFeatureGate.Enabled(features.CatalogMetadataAPI) { if err = r.syncCatalogMetadata(ctx, unpackResult.FS, catalog); err != nil { @@ -152,7 +156,7 @@ func (r *CatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.Cat } } - 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)) @@ -181,8 +185,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 624b2064..7b0c7ced 100644 --- a/pkg/controllers/core/catalog_controller_test.go +++ b/pkg/controllers/core/catalog_controller_test.go @@ -54,11 +54,11 @@ type MockStore struct { shouldError bool } -func (m MockStore) Store(_ string, _ fs.FS) error { +func (m MockStore) Store(_ string, _ fs.FS) (string, error) { if m.shouldError { - return errors.New("mockstore store error") + return "", errors.New("mockstore store error") } - return nil + return "url", nil } func (m MockStore) Delete(_ string) error { diff --git a/pkg/storage/localdir.go b/pkg/storage/localdir.go index fbbcda9a..2a2bd429 100644 --- a/pkg/storage/localdir.go +++ b/pkg/storage/localdir.go @@ -16,17 +16,18 @@ import ( // done so that clients accessing the content stored in RootDir/catalogName have // atomic view of the content for a catalog. type LocalDir struct { - RootDir string + RootDir string + URLPrefix string } -func (s LocalDir) Store(catalog string, fsys fs.FS) error { +func (s LocalDir) Store(catalog string, fsys fs.FS) (string, error) { fbcDir := filepath.Join(s.RootDir, catalog) if err := os.MkdirAll(fbcDir, 0700); err != nil { - return err + return "", err } tempFile, err := os.CreateTemp(s.RootDir, fmt.Sprint(catalog)) if err != nil { - return err + return "", err } defer os.Remove(tempFile.Name()) err = declcfg.WalkMetasFS(fsys, func(path string, meta *declcfg.Meta, err error) error { @@ -37,10 +38,10 @@ func (s LocalDir) Store(catalog string, fsys fs.FS) error { return err }) if err != nil { - return err + return "", err } fbcFile := filepath.Join(fbcDir, "all.json") - return os.Rename(tempFile.Name(), fbcFile) + return fmt.Sprintf("%s%s/all.json", s.URLPrefix, catalog), os.Rename(tempFile.Name(), fbcFile) } func (s LocalDir) Delete(catalog string) error { @@ -49,7 +50,7 @@ func (s LocalDir) Delete(catalog string) error { 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.URLPrefix, http.StripPrefix(s.URLPrefix, 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..0869bd67 100644 --- a/pkg/storage/localdir_test.go +++ b/pkg/storage/localdir_test.go @@ -49,7 +49,7 @@ var _ = Describe("LocalDir Storage Test", func() { }) When("An unpacked FBC is stored using LocalDir", func() { BeforeEach(func() { - err := store.Store(catalog, unpackResultFS) + _, err := store.Store(catalog, unpackResultFS) Expect(err).To(Not(HaveOccurred())) }) It("should store the content in the RootDir correctly", func() { @@ -88,7 +88,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, URLPrefix: "/catalogs/"} testServer = httptest.NewServer(store.StorageServerHandler()) }) diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 1b244973..ce41463e 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -10,7 +10,7 @@ import ( // host's filesystem. It also a manager runnable object, that starts // a server to serve the content stored. type Instance interface { - Store(catalog string, fsys fs.FS) error + Store(catalog string, fsys fs.FS) (string, error) Delete(catalog string) error StorageServerHandler() http.Handler } diff --git a/test/e2e/unpack_test.go b/test/e2e/unpack_test.go index 444e0230..093ebdfc 100644 --- a/test/e2e/unpack_test.go +++ b/test/e2e/unpack_test.go @@ -175,8 +175,8 @@ var _ = Describe("Catalog Unpacking", func() { Expect(expectedBndl).To(komega.EqualObject(bndl, komega.IgnoreAutogeneratedMetadata)) 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", @@ -189,7 +189,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",