From 86c886c4bdcdc457c53749e62c002d0164b6987d Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Thu, 10 Oct 2024 09:55:38 -0500 Subject: [PATCH] change catalog-specific URL from full path to API endpoint ref Signed-off-by: Jordan Keister --- api/core/v1alpha1/clustercatalog_types.go | 6 ++-- cmd/manager/main.go | 2 +- ....operatorframework.io_clustercatalogs.yaml | 9 +++--- .../core/clustercatalog_controller.go | 4 +-- .../core/clustercatalog_controller_test.go | 14 ++++----- internal/storage/localdir.go | 14 ++++----- internal/storage/localdir_test.go | 29 ++++++++++--------- test/e2e/util.go | 9 +++--- test/upgrade/unpack_test.go | 2 +- 9 files changed, 44 insertions(+), 45 deletions(-) diff --git a/api/core/v1alpha1/clustercatalog_types.go b/api/core/v1alpha1/clustercatalog_types.go index ecc88403..8290633e 100644 --- a/api/core/v1alpha1/clustercatalog_types.go +++ b/api/core/v1alpha1/clustercatalog_types.go @@ -129,11 +129,9 @@ type ClusterCatalogStatus struct { // type: Image // +optional ResolvedSource *ResolvedCatalogSource `json:"resolvedSource,omitempty"` - // contentURL is a cluster-internal URL from which on-cluster components - // can read the content of a catalog + // baseURL is a cluster-internal URL from which on-cluster components can access the API endpoint for this catalog // +optional - ContentURL string `json:"contentURL,omitempty"` - + BaseURL string `json:"baseURL,omitempty"` // lastUnpacked represents the time when the // ClusterCatalog object was last unpacked successfully. // +optional diff --git a/cmd/manager/main.go b/cmd/manager/main.go index a2d0175a..b4cfee79 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -258,7 +258,7 @@ func main() { os.Exit(1) } - localStorage = storage.LocalDir{RootDir: storeDir, BaseURL: baseStorageURL} + localStorage = storage.LocalDir{RootDir: storeDir, RootURL: baseStorageURL} catalogServerConfig := serverutil.CatalogServerConfig{ ExternalAddr: externalAddr, diff --git a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml index 67a0d774..a7a9a4a6 100644 --- a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml +++ b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml @@ -130,6 +130,10 @@ spec: status: description: ClusterCatalogStatus defines the observed state of ClusterCatalog properties: + baseURL: + description: baseURL is a cluster-internal URL from which on-cluster + components can access the API endpoint for this catalog + type: string conditions: description: |- conditions is a representation of the current state for this ClusterCatalog. @@ -206,11 +210,6 @@ spec: - type type: object type: array - contentURL: - description: |- - contentURL is a cluster-internal URL from which on-cluster components - can read the content of a catalog - type: string lastUnpacked: description: |- lastUnpacked represents the time when the diff --git a/internal/controllers/core/clustercatalog_controller.go b/internal/controllers/core/clustercatalog_controller.go index a8ad497c..7ec49093 100644 --- a/internal/controllers/core/clustercatalog_controller.go +++ b/internal/controllers/core/clustercatalog_controller.go @@ -304,7 +304,7 @@ func updateStatusProgressing(status *v1alpha1.ClusterCatalogStatus, generation i func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result source.Result, contentURL string, generation int64) { status.ResolvedSource = result.ResolvedSource - status.ContentURL = contentURL + status.BaseURL = contentURL status.LastUnpacked = metav1.NewTime(result.UnpackTime) meta.SetStatusCondition(&status.Conditions, metav1.Condition{ Type: v1alpha1.TypeServing, @@ -317,7 +317,7 @@ func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result source.Re func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus, generation int64) { status.ResolvedSource = nil - status.ContentURL = "" + status.BaseURL = "" status.LastUnpacked = metav1.Time{} meta.SetStatusCondition(&status.Conditions, metav1.Condition{ Type: v1alpha1.TypeServing, diff --git a/internal/controllers/core/clustercatalog_controller_test.go b/internal/controllers/core/clustercatalog_controller_test.go index 347fb8c6..09fb5c20 100644 --- a/internal/controllers/core/clustercatalog_controller_test.go +++ b/internal/controllers/core/clustercatalog_controller_test.go @@ -261,7 +261,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - ContentURL: "URL", + BaseURL: "URL", Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeServing, @@ -393,7 +393,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - ContentURL: "URL", + BaseURL: "URL", LastUnpacked: metav1.Time{}, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, @@ -430,7 +430,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - ContentURL: "", + BaseURL: "", Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeServing, @@ -473,7 +473,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - ContentURL: "URL", + BaseURL: "URL", Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeProgressing, @@ -503,7 +503,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - ContentURL: "URL", + BaseURL: "URL", Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeProgressing, @@ -544,7 +544,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - ContentURL: "URL", + BaseURL: "URL", Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeProgressing, @@ -696,7 +696,7 @@ func TestPollingReconcilerUnpack(t *testing.T) { successfulObservedGeneration := int64(2) successfulUnpackStatus := func(mods ...func(status *catalogdv1alpha1.ClusterCatalogStatus)) catalogdv1alpha1.ClusterCatalogStatus { s := catalogdv1alpha1.ClusterCatalogStatus{ - ContentURL: "URL", + BaseURL: "URL", Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeProgressing, diff --git a/internal/storage/localdir.go b/internal/storage/localdir.go index 93a82b71..8d134ca3 100644 --- a/internal/storage/localdir.go +++ b/internal/storage/localdir.go @@ -21,11 +21,11 @@ import ( // atomic view of the content for a catalog. type LocalDir struct { RootDir string - BaseURL *url.URL + RootURL *url.URL } func (s LocalDir) Store(ctx context.Context, catalog string, fsys fs.FS) error { - fbcDir := filepath.Join(s.RootDir, catalog) + fbcDir := filepath.Join(s.RootDir, catalog, "api", "v1") if err := os.MkdirAll(fbcDir, 0700); err != nil { return err } @@ -43,7 +43,7 @@ func (s LocalDir) Store(ctx context.Context, catalog string, fsys fs.FS) error { }); err != nil { return fmt.Errorf("error walking FBC root: %w", err) } - fbcFile := filepath.Join(fbcDir, "all.json") + fbcFile := filepath.Join(fbcDir, "all") return os.Rename(tempFile.Name(), fbcFile) } @@ -52,25 +52,25 @@ func (s LocalDir) Delete(catalog string) error { } func (s LocalDir) ContentURL(catalog string) string { - return fmt.Sprintf("%s%s/all.json", s.BaseURL, catalog) + return fmt.Sprintf("%s%s/api", s.RootURL, catalog) } func (s LocalDir) StorageServerHandler() http.Handler { mux := http.NewServeMux() fsHandler := http.FileServer(http.FS(&filesOnlyFilesystem{os.DirFS(s.RootDir)})) - spHandler := http.StripPrefix(s.BaseURL.Path, fsHandler) + spHandler := http.StripPrefix(s.RootURL.Path, fsHandler) gzHandler := gzhttp.GzipHandler(spHandler) typeHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Add("Content-Type", "application/jsonl") gzHandler.ServeHTTP(w, r) }) - mux.Handle(s.BaseURL.Path, typeHandler) + mux.Handle(s.RootURL.Path, typeHandler) return mux } func (s LocalDir) ContentExists(catalog string) bool { - file, err := os.Stat(filepath.Join(s.RootDir, catalog, "all.json")) + file, err := os.Stat(filepath.Join(s.RootDir, catalog, "api", "v1", "all")) if err != nil { return false } diff --git a/internal/storage/localdir_test.go b/internal/storage/localdir_test.go index fd20d125..ee8cc05e 100644 --- a/internal/storage/localdir_test.go +++ b/internal/storage/localdir_test.go @@ -56,7 +56,7 @@ var _ = Describe("LocalDir Storage Test", func() { rootDir = d baseURL = &url.URL{Scheme: "http", Host: "test-addr", Path: urlPrefix} - store = LocalDir{RootDir: rootDir, BaseURL: baseURL} + store = LocalDir{RootDir: rootDir, RootURL: baseURL} unpackResultFS = &fstest.MapFS{ "bundle.yaml": &fstest.MapFile{Data: []byte(testBundle), Mode: os.ModePerm}, "package.yaml": &fstest.MapFile{Data: []byte(testPackage), Mode: os.ModePerm}, @@ -69,19 +69,20 @@ var _ = Describe("LocalDir Storage Test", func() { Expect(err).To(Not(HaveOccurred())) }) It("should store the content in the RootDir correctly", func() { - fbcFile := filepath.Join(rootDir, catalog, "all.json") + fbcDir := filepath.Join(rootDir, catalog, "api", "v1") + fbcFile := filepath.Join(fbcDir, "all") _, err := os.Stat(fbcFile) Expect(err).To(Not(HaveOccurred())) gotConfig, err := declcfg.LoadFS(ctx, unpackResultFS) Expect(err).To(Not(HaveOccurred())) - storedConfig, err := declcfg.LoadFile(os.DirFS(filepath.Join(rootDir, catalog)), "all.json") + storedConfig, err := declcfg.LoadFile(os.DirFS(fbcDir), "all") Expect(err).To(Not(HaveOccurred())) 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))) + Expect(store.ContentURL(catalog)).To(Equal(fmt.Sprintf("%s%s/api", baseURL, catalog))) }) It("should report content exists", func() { Expect(store.ContentExists(catalog)).To(BeTrue()) @@ -112,8 +113,8 @@ var _ = Describe("LocalDir Server Handler tests", func() { BeforeEach(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, BaseURL: &url.URL{Path: urlPrefix}} + Expect(os.MkdirAll(filepath.Join(d, "test-catalog", "api", "v1"), 0700)).To(Succeed()) + store = LocalDir{RootDir: d, RootURL: &url.URL{Path: urlPrefix}} testServer = httptest.NewServer(store.StorageServerHandler()) }) @@ -144,15 +145,15 @@ var _ = Describe("LocalDir Server Handler tests", func() { Expect(os.WriteFile(filepath.Join(store.RootDir, "test-catalog", "foo.txt"), expectedContent, 0600)).To(Succeed()) expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/test-catalog/foo.txt"), expectedContent) }) - It("ignores accept-encoding for the path /catalogs/test-catalog/all.json with size < 1400 bytes", func() { + It("ignores accept-encoding for the path /catalogs/test-catalog/api/v1/all with size < 1400 bytes", func() { expectedContent := []byte("bar") - Expect(os.WriteFile(filepath.Join(store.RootDir, "test-catalog", "all.json"), expectedContent, 0600)).To(Succeed()) - expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/test-catalog/all.json"), expectedContent) + Expect(os.WriteFile(filepath.Join(store.RootDir, "test-catalog", "api", "v1", "all"), expectedContent, 0600)).To(Succeed()) + expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/test-catalog/api/v1/all"), expectedContent) }) - It("provides gzipped content for the path /catalogs/test-catalog/all.json with size > 1400 bytes", func() { + It("provides gzipped content for the path /catalogs/test-catalog/api/v1/all with size > 1400 bytes", func() { expectedContent := []byte(testCompressableJSON) - Expect(os.WriteFile(filepath.Join(store.RootDir, "test-catalog", "all.json"), expectedContent, 0600)).To(Succeed()) - expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/test-catalog/all.json"), expectedContent) + Expect(os.WriteFile(filepath.Join(store.RootDir, "test-catalog", "api", "v1", "all"), expectedContent, 0600)).To(Succeed()) + expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/test-catalog/api/v1/all"), expectedContent) }) It("provides json-lines format for the served JSON catalog", func() { catalog := "test-catalog" @@ -164,7 +165,7 @@ var _ = Describe("LocalDir Server Handler tests", func() { expectedContent, err := generateJSONLines([]byte(testCompressableJSON)) Expect(err).To(Not(HaveOccurred())) - expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/test-catalog/all.json"), []byte(expectedContent)) + expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/test-catalog/api/v1/all"), []byte(expectedContent)) }) It("provides json-lines format for the served YAML catalog", func() { catalog := "test-catalog" @@ -178,7 +179,7 @@ var _ = Describe("LocalDir Server Handler tests", func() { expectedContent, err := generateJSONLines(yamlData) Expect(err).To(Not(HaveOccurred())) - expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/test-catalog/all.json"), []byte(expectedContent)) + expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/test-catalog/api/v1/all"), []byte(expectedContent)) }) AfterEach(func() { testServer.Close() diff --git a/test/e2e/util.go b/test/e2e/util.go index a5a6ac1d..689737ee 100644 --- a/test/e2e/util.go +++ b/test/e2e/util.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/url" + "path" "strings" "k8s.io/client-go/kubernetes" @@ -17,12 +18,12 @@ func ReadTestCatalogServerContents(ctx context.Context, catalog *catalogd.Cluste if catalog == nil { return nil, fmt.Errorf("cannot read nil catalog") } - url, err := url.Parse(catalog.Status.ContentURL) + url, err := url.Parse(catalog.Status.BaseURL) if err != nil { - return nil, fmt.Errorf("error parsing clustercatalog url %s: %v", catalog.Status.ContentURL, err) + return nil, fmt.Errorf("error parsing clustercatalog url %s: %v", catalog.Status.BaseURL, err) } // url is expected to be in the format of - // http://{service_name}.{namespace}.svc/{catalog_name}/all.json + // http://{service_name}.{namespace}.svc/api/{catalog_name}/v1/all // so to get the namespace and name of the service we grab only // the hostname and split it on the '.' character ns := strings.Split(url.Hostname(), ".")[1] @@ -37,7 +38,7 @@ func ReadTestCatalogServerContents(ctx context.Context, catalog *catalogd.Cluste port = "80" } } - resp := kubeClient.CoreV1().Services(ns).ProxyGet(url.Scheme, name, port, url.Path, map[string]string{}) + resp := kubeClient.CoreV1().Services(ns).ProxyGet(url.Scheme, name, port, path.Join(url.Path, "v1", "all"), map[string]string{}) rc, err := resp.Stream(ctx) if err != nil { return nil, err diff --git a/test/upgrade/unpack_test.go b/test/upgrade/unpack_test.go index 2cc368d3..2f9a9a53 100644 --- a/test/upgrade/unpack_test.go +++ b/test/upgrade/unpack_test.go @@ -76,7 +76,7 @@ var _ = Describe("ClusterCatalog Unpacking", func() { g.Expect(cond.Reason).To(Equal(catalogd.ReasonSucceeded)) }).Should(Succeed()) - expectedFBC, err := os.ReadFile("../../testdata/catalogs/test-catalog/expected_all.json") + expectedFBC, err := os.ReadFile("../../testdata/catalogs/test-catalog/api/v1/expected_all.json") Expect(err).To(Not(HaveOccurred())) By("Making sure the catalog content is available via the http server")