diff --git a/api/core/v1alpha1/clustercatalog_types.go b/api/core/v1alpha1/clustercatalog_types.go index d44d7d6a..895f4068 100644 --- a/api/core/v1alpha1/clustercatalog_types.go +++ b/api/core/v1alpha1/clustercatalog_types.go @@ -147,17 +147,21 @@ type ClusterCatalogStatus struct { // type: Image // +optional ResolvedSource *ResolvedCatalogSource `json:"resolvedSource,omitempty"` - // catalogURLs contains the URLs that can be used to access the catalog. + // urls contains the URLs that can be used to access the catalog. // +optional - URLs *CatalogURLs `json:"urls,omitempty"` + URLs *ClusterCatalogURLs `json:"urls,omitempty"` // lastUnpacked represents the time when the // ClusterCatalog object was last unpacked successfully. // +optional LastUnpacked metav1.Time `json:"lastUnpacked,omitempty"` } -type CatalogURLs struct { - // base is a required cluster-internal URL from which on-cluster components can access the API endpoint for this catalog +// ClusterCatalogURLs contains the URLs that can be used to access the catalog. +type ClusterCatalogURLs struct { + // base is a required cluster-internal URL which provides API access for this ClusterCatalog. + // A suffix API access path can be added to retrieve catalog data for the ClusterCatalog. + // Currently, a 'v1' API access provides complete FBC retrival via the path "/api/v1/all", with the general form `{base}/api/v1/all`. + // +kubebuilder:validation:Required Base string `json:"base"` } diff --git a/api/core/v1alpha1/zz_generated.deepcopy.go b/api/core/v1alpha1/zz_generated.deepcopy.go index 727dabf6..24f7bbfc 100644 --- a/api/core/v1alpha1/zz_generated.deepcopy.go +++ b/api/core/v1alpha1/zz_generated.deepcopy.go @@ -45,21 +45,6 @@ func (in *CatalogSource) DeepCopy() *CatalogSource { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *CatalogURLs) DeepCopyInto(out *CatalogURLs) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CatalogURLs. -func (in *CatalogURLs) DeepCopy() *CatalogURLs { - if in == nil { - return nil - } - out := new(CatalogURLs) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterCatalog) DeepCopyInto(out *ClusterCatalog) { *out = *in @@ -152,7 +137,7 @@ func (in *ClusterCatalogStatus) DeepCopyInto(out *ClusterCatalogStatus) { } if in.URLs != nil { in, out := &in.URLs, &out.URLs - *out = new(CatalogURLs) + *out = new(ClusterCatalogURLs) **out = **in } in.LastUnpacked.DeepCopyInto(&out.LastUnpacked) @@ -168,6 +153,21 @@ func (in *ClusterCatalogStatus) DeepCopy() *ClusterCatalogStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterCatalogURLs) DeepCopyInto(out *ClusterCatalogURLs) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterCatalogURLs. +func (in *ClusterCatalogURLs) DeepCopy() *ClusterCatalogURLs { + if in == nil { + return nil + } + out := new(ClusterCatalogURLs) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ImageSource) DeepCopyInto(out *ImageSource) { *out = *in diff --git a/cmd/manager/main.go b/cmd/manager/main.go index ee981ad1..b39fd795 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, RootURL: baseStorageURL} + localStorage = storage.LocalDirV1{RootDir: storeDir, RootURL: baseStorageURL} // Config for the the catalogd web server catalogServerConfig := serverutil.CatalogServerConfig{ diff --git a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml index cb202ce1..a3a10dc9 100644 --- a/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml +++ b/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml @@ -274,12 +274,14 @@ spec: - message: source type 'Image' requires image field rule: self.type == 'Image' && has(self.image) urls: - description: catalogURLs contains the URLs that can be used to access - the catalog. + description: urls contains the URLs that can be used to access the + catalog. properties: base: - description: base is a required cluster-internal URL from which - on-cluster components can access the API endpoint for this catalog + description: |- + base is a required cluster-internal URL which provides API access for this ClusterCatalog. + A suffix API access path can be added to retrieve catalog data for the ClusterCatalog. + Currently, a 'v1' API access provides complete FBC retrival via the path "/api/v1/all", with the general form `{base}/api/v1/all`. type: string required: - base diff --git a/docs/fetching-catalog-contents.md b/docs/fetching-catalog-contents.md index 0ecc7931..2c577e53 100644 --- a/docs/fetching-catalog-contents.md +++ b/docs/fetching-catalog-contents.md @@ -1,5 +1,5 @@ # `ClusterCatalog` Interface -`catalogd` serves catalog content via a catalog-specific, versioned HTTP(S) endpoint. Clients access catalog information via this API endpoint and a versioned reference of the desired format. Current support includes only a complete catalog download, indicated by the path "v1/all", for example if `status.urls.base` is `https://catalogd-service.olmv1-system.svc/catalogs/operatorhub/api` then `https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio/api/vi/all` would receive the complete FBC for the catalog `operatorhubio`. +`catalogd` serves catalog content via a catalog-specific, versioned HTTP(S) endpoint. Clients access catalog information via this API endpoint and a versioned reference of the desired format. Current support includes only a complete catalog download, indicated by the path "api/v1/all", for example if `status.urls.base` is `https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio` then `https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio/api/vi/all` would receive the complete FBC for the catalog `operatorhubio`. ## Response Format @@ -80,13 +80,14 @@ For example purposes we make the following assumption: For local development, consider skipping TLS verification, such as `curl -k`, or reference external material on self-signed certificate verification. -`ClusterCatalog` CRs have a status.baseURL field which identifies the catalog-specific API to access the catalog content: +`ClusterCatalog` CRs have a `status.urls.base` field which identifies the catalog-specific API to access the catalog content: ```yaml status: . . - baseURL: https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio/api/ + urls: + base: https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio resolvedSource: image: ref: quay.io/operatorhubio/catalog@sha256:e53267559addc85227c2a7901ca54b980bc900276fc24d3f4db0549cb38ecf76 @@ -96,9 +97,9 @@ on self-signed certificate verification. ## On cluster When making a request for the complete contents of the `operatorhubio` `ClusterCatalog` from within -the cluster, clients would combine `status.baseURL` with the desired API service and issue an HTTP GET request for the URL. +the cluster, clients would combine `status.urls.base` with the desired API service and issue an HTTP GET request for the URL. -For example, to receive the complete catalog data for the `operatorhubio` catalog indicated above, the client would append the service point designator `v1/all`, like: +For example, to receive the complete catalog data for the `operatorhubio` catalog indicated above, the client would append the service point designator `api/v1/all`, like: `https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio/api/v1/all`. diff --git a/internal/controllers/core/clustercatalog_controller.go b/internal/controllers/core/clustercatalog_controller.go index f714076e..f0c880d4 100644 --- a/internal/controllers/core/clustercatalog_controller.go +++ b/internal/controllers/core/clustercatalog_controller.go @@ -159,7 +159,7 @@ func (r *ClusterCatalogReconciler) SetupWithManager(mgr ctrl.Manager) error { func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.ClusterCatalog) (ctrl.Result, error) { l := log.FromContext(ctx) // Check if the catalog availability is set to disabled, if true then - // unset content URL, delete it from the cache and set appropriate status + // unset base URL, delete it from the cache and set appropriate status if catalog.Spec.Availability == v1alpha1.AvailabilityDisabled { // Delete the catalog from local cache err := r.deleteCatalogCache(ctx, catalog) @@ -244,10 +244,10 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *v1alp updateStatusProgressing(&catalog.Status, catalog.GetGeneration(), storageErr) return ctrl.Result{}, storageErr } - contentURL := r.Storage.ContentURL(catalog.Name) + baseURL := r.Storage.BaseURL(catalog.Name) updateStatusProgressing(&catalog.Status, catalog.GetGeneration(), nil) - updateStatusServing(&catalog.Status, *unpackResult, contentURL, catalog.GetGeneration()) + updateStatusServing(&catalog.Status, *unpackResult, baseURL, catalog.GetGeneration()) default: panic(fmt.Sprintf("unknown unpack state %q", unpackResult.State)) } @@ -271,7 +271,7 @@ func (r *ClusterCatalogReconciler) getCurrentState(catalog *v1alpha1.ClusterCata // Set expected status based on what we see in the stored catalog clearUnknownConditions(expectedStatus) if hasStoredCatalog && r.Storage.ContentExists(catalog.Name) { - updateStatusServing(expectedStatus, storedCatalog.unpackResult, r.Storage.ContentURL(catalog.Name), storedCatalog.observedGeneration) + updateStatusServing(expectedStatus, storedCatalog.unpackResult, r.Storage.BaseURL(catalog.Name), storedCatalog.observedGeneration) updateStatusProgressing(expectedStatus, storedCatalog.observedGeneration, nil) } @@ -323,12 +323,12 @@ func updateStatusProgressing(status *v1alpha1.ClusterCatalogStatus, generation i meta.SetStatusCondition(&status.Conditions, progressingCond) } -func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result source.Result, contentURL string, generation int64) { +func updateStatusServing(status *v1alpha1.ClusterCatalogStatus, result source.Result, baseURL string, generation int64) { status.ResolvedSource = result.ResolvedSource if status.URLs == nil { - status.URLs = &v1alpha1.CatalogURLs{} + status.URLs = &v1alpha1.ClusterCatalogURLs{} } - status.URLs.Base = contentURL + status.URLs.Base = baseURL status.LastUnpacked = metav1.NewTime(result.UnpackTime) meta.SetStatusCondition(&status.Conditions, metav1.Condition{ Type: v1alpha1.TypeServing, @@ -352,10 +352,7 @@ func updateStatusCatalogDisabled(status *v1alpha1.ClusterCatalogStatus, generati func updateStatusNotServing(status *v1alpha1.ClusterCatalogStatus, generation int64) { status.ResolvedSource = nil - if status.URLs == nil { - status.URLs = &v1alpha1.CatalogURLs{} - } - status.URLs.Base = "" + status.URLs = nil 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 04d20e29..62cc66ad 100644 --- a/internal/controllers/core/clustercatalog_controller_test.go +++ b/internal/controllers/core/clustercatalog_controller_test.go @@ -70,7 +70,7 @@ func (m MockStore) Delete(_ string) error { return nil } -func (m MockStore) ContentURL(_ string) string { +func (m MockStore) BaseURL(_ string) string { return "URL" } @@ -261,7 +261,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - URLs: &catalogdv1alpha1.CatalogURLs{Base: "URL"}, + URLs: &catalogdv1alpha1.ClusterCatalogURLs{Base: "URL"}, Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeServing, @@ -393,7 +393,6 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - URLs: &catalogdv1alpha1.CatalogURLs{}, LastUnpacked: metav1.Time{}, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, @@ -430,7 +429,6 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - URLs: &catalogdv1alpha1.CatalogURLs{}, Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeServing, @@ -473,7 +471,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - URLs: &catalogdv1alpha1.CatalogURLs{}, + URLs: &catalogdv1alpha1.ClusterCatalogURLs{Base: "URL"}, Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeProgressing, @@ -503,7 +501,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - URLs: &catalogdv1alpha1.CatalogURLs{}, + URLs: &catalogdv1alpha1.ClusterCatalogURLs{Base: "URL"}, Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeProgressing, @@ -573,7 +571,6 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - URLs: &catalogdv1alpha1.CatalogURLs{}, Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeProgressing, @@ -590,7 +587,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, { - name: "catalog availability set to disabled, ContentURL should get unset", + name: "catalog availability set to disabled, status.urls should get unset", source: &MockSource{ result: &source.Result{ State: source.StateUnpacked, @@ -612,7 +609,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { Availability: "Disabled", }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - URLs: &catalogdv1alpha1.CatalogURLs{Base: "URL"}, + URLs: &catalogdv1alpha1.ClusterCatalogURLs{Base: "URL"}, LastUnpacked: metav1.Time{}, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, @@ -648,7 +645,6 @@ func TestCatalogdControllerReconcile(t *testing.T) { Availability: "Disabled", }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - URLs: &catalogdv1alpha1.CatalogURLs{}, Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeServing, @@ -688,7 +684,7 @@ func TestCatalogdControllerReconcile(t *testing.T) { Availability: "Disabled", }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - URLs: &catalogdv1alpha1.CatalogURLs{Base: "URL"}, + URLs: &catalogdv1alpha1.ClusterCatalogURLs{Base: "URL"}, LastUnpacked: metav1.Time{}, ResolvedSource: &catalogdv1alpha1.ResolvedCatalogSource{ Type: catalogdv1alpha1.SourceTypeImage, @@ -725,7 +721,6 @@ func TestCatalogdControllerReconcile(t *testing.T) { Availability: "Disabled", }, Status: catalogdv1alpha1.ClusterCatalogStatus{ - URLs: &catalogdv1alpha1.CatalogURLs{}, Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeServing, @@ -848,7 +843,7 @@ func TestPollingReconcilerUnpack(t *testing.T) { successfulObservedGeneration := int64(2) successfulUnpackStatus := func(mods ...func(status *catalogdv1alpha1.ClusterCatalogStatus)) catalogdv1alpha1.ClusterCatalogStatus { s := catalogdv1alpha1.ClusterCatalogStatus{ - URLs: &catalogdv1alpha1.CatalogURLs{Base: "URL"}, + URLs: &catalogdv1alpha1.ClusterCatalogURLs{Base: "URL"}, Conditions: []metav1.Condition{ { Type: catalogdv1alpha1.TypeProgressing, diff --git a/internal/storage/localdir.go b/internal/storage/localdir.go index 4d104b8e..dd06729e 100644 --- a/internal/storage/localdir.go +++ b/internal/storage/localdir.go @@ -14,12 +14,12 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" ) -// LocalDir is a storage Instance. When Storing a new FBC contained in +// LocalDirV1 is a storage Instance. When Storing a new FBC contained in // fs.FS, the content is first written to a temporary file, after which // it is copied to its final destination in RootDir/catalogName/. This is // done so that clients accessing the content stored in RootDir/catalogName have // atomic view of the content for a catalog. -type LocalDir struct { +type LocalDirV1 struct { RootDir string RootURL *url.URL } @@ -29,7 +29,7 @@ const ( v1ApiData = "all" ) -func (s LocalDir) Store(ctx context.Context, catalog string, fsys fs.FS) error { +func (s LocalDirV1) Store(ctx context.Context, catalog string, fsys fs.FS) error { fbcDir := filepath.Join(s.RootDir, catalog, v1ApiPath) if err := os.MkdirAll(fbcDir, 0700); err != nil { return err @@ -52,15 +52,15 @@ func (s LocalDir) Store(ctx context.Context, catalog string, fsys fs.FS) error { return os.Rename(tempFile.Name(), fbcFile) } -func (s LocalDir) Delete(catalog string) error { +func (s LocalDirV1) Delete(catalog string) error { return os.RemoveAll(filepath.Join(s.RootDir, catalog)) } -func (s LocalDir) ContentURL(catalog string) string { - return s.RootURL.JoinPath(catalog, "api").String() +func (s LocalDirV1) BaseURL(catalog string) string { + return s.RootURL.JoinPath(catalog).String() } -func (s LocalDir) StorageServerHandler() http.Handler { +func (s LocalDirV1) StorageServerHandler() http.Handler { mux := http.NewServeMux() fsHandler := http.FileServer(http.FS(&filesOnlyFilesystem{os.DirFS(s.RootDir)})) spHandler := http.StripPrefix(s.RootURL.Path, fsHandler) @@ -74,7 +74,7 @@ func (s LocalDir) StorageServerHandler() http.Handler { return mux } -func (s LocalDir) ContentExists(catalog string) bool { +func (s LocalDirV1) ContentExists(catalog string) bool { file, err := os.Stat(filepath.Join(s.RootDir, catalog, v1ApiPath, v1ApiData)) if err != nil { return false diff --git a/internal/storage/localdir_test.go b/internal/storage/localdir_test.go index 7344e4e5..d36d655f 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, RootURL: baseURL} + store = LocalDirV1{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}, @@ -82,7 +82,7 @@ var _ = Describe("LocalDir Storage Test", func() { Expect(diff).To(Equal("")) }) It("should form the content URL correctly", func() { - Expect(store.ContentURL(catalog)).To(Equal(baseURL.JoinPath(catalog, "api").String())) + Expect(store.BaseURL(catalog)).To(Equal(baseURL.JoinPath(catalog).String())) }) It("should report content exists", func() { Expect(store.ContentExists(catalog)).To(BeTrue()) @@ -108,13 +108,13 @@ var _ = Describe("LocalDir Storage Test", func() { var _ = Describe("LocalDir Server Handler tests", func() { var ( testServer *httptest.Server - store LocalDir + store LocalDirV1 ) BeforeEach(func() { d, err := os.MkdirTemp(GinkgoT().TempDir(), "cache") Expect(err).ToNot(HaveOccurred()) Expect(os.MkdirAll(filepath.Join(d, "test-catalog", v1ApiPath), 0700)).To(Succeed()) - store = LocalDir{RootDir: d, RootURL: &url.URL{Path: urlPrefix}} + store = LocalDirV1{RootDir: d, RootURL: &url.URL{Path: urlPrefix}} testServer = httptest.NewServer(store.StorageServerHandler()) }) diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 09fbded5..458ff040 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -13,7 +13,7 @@ import ( type Instance interface { Store(ctx context.Context, catalog string, fsys fs.FS) error Delete(catalog string) error - ContentURL(catalog string) string + BaseURL(catalog string) string StorageServerHandler() http.Handler ContentExists(catalog string) bool } diff --git a/test/e2e/util.go b/test/e2e/util.go index e9390fdf..e5784fb0 100644 --- a/test/e2e/util.go +++ b/test/e2e/util.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "net/url" - "path" "strings" "k8s.io/client-go/kubernetes" @@ -19,14 +18,14 @@ func ReadTestCatalogServerContents(ctx context.Context, catalog *catalogd.Cluste return nil, fmt.Errorf("cannot read nil catalog") } if catalog.Status.URLs == nil { - return nil, fmt.Errorf("catalog %s has no catalog urls", catalog.Name) + return nil, fmt.Errorf("catalog %q has no catalog urls", catalog.Name) } url, err := url.Parse(catalog.Status.URLs.Base) if err != nil { - return nil, fmt.Errorf("error parsing clustercatalog url %s: %v", catalog.Status.URLs.Base, err) + return nil, fmt.Errorf("error parsing clustercatalog url %q: %v", catalog.Status.URLs.Base, err) } // url is expected to be in the format of - // http://{service_name}.{namespace}.svc/{catalog_name}/api + // http://{service_name}.{namespace}.svc/catalogs/{catalog_name}/ // 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] @@ -41,7 +40,7 @@ func ReadTestCatalogServerContents(ctx context.Context, catalog *catalogd.Cluste port = "80" } } - resp := kubeClient.CoreV1().Services(ns).ProxyGet(url.Scheme, name, port, path.Join(url.Path, "v1", "all"), map[string]string{}) + resp := kubeClient.CoreV1().Services(ns).ProxyGet(url.Scheme, name, port, url.JoinPath("api", "v1", "all").Path, map[string]string{}) rc, err := resp.Stream(ctx) if err != nil { return nil, err