Skip to content

Commit

Permalink
change catalog-specific URL from full path to API endpoint ref
Browse files Browse the repository at this point in the history
Signed-off-by: Jordan Keister <jordan@nimblewidget.com>
  • Loading branch information
grokspawn committed Oct 10, 2024
1 parent b1b145a commit 4fd529a
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 43 deletions.
6 changes: 2 additions & 4 deletions api/core/v1alpha1/clustercatalog_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions internal/controllers/core/clustercatalog_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
14 changes: 7 additions & 7 deletions internal/controllers/core/clustercatalog_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func TestCatalogdControllerReconcile(t *testing.T) {
},
},
Status: catalogdv1alpha1.ClusterCatalogStatus{
ContentURL: "URL",
BaseURL: "URL",
Conditions: []metav1.Condition{
{
Type: catalogdv1alpha1.TypeServing,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -430,7 +430,7 @@ func TestCatalogdControllerReconcile(t *testing.T) {
},
},
Status: catalogdv1alpha1.ClusterCatalogStatus{
ContentURL: "",
BaseURL: "",
Conditions: []metav1.Condition{
{
Type: catalogdv1alpha1.TypeServing,
Expand Down Expand Up @@ -473,7 +473,7 @@ func TestCatalogdControllerReconcile(t *testing.T) {
},
},
Status: catalogdv1alpha1.ClusterCatalogStatus{
ContentURL: "URL",
BaseURL: "URL",
Conditions: []metav1.Condition{
{
Type: catalogdv1alpha1.TypeProgressing,
Expand Down Expand Up @@ -503,7 +503,7 @@ func TestCatalogdControllerReconcile(t *testing.T) {
},
},
Status: catalogdv1alpha1.ClusterCatalogStatus{
ContentURL: "URL",
BaseURL: "URL",
Conditions: []metav1.Condition{
{
Type: catalogdv1alpha1.TypeProgressing,
Expand Down Expand Up @@ -544,7 +544,7 @@ func TestCatalogdControllerReconcile(t *testing.T) {
},
},
Status: catalogdv1alpha1.ClusterCatalogStatus{
ContentURL: "URL",
BaseURL: "URL",
Conditions: []metav1.Condition{
{
Type: catalogdv1alpha1.TypeProgressing,
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 7 additions & 7 deletions internal/storage/localdir.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
}

Expand All @@ -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
}
Expand Down
29 changes: 15 additions & 14 deletions internal/storage/localdir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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())
Expand Down Expand Up @@ -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())

})
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,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]
Expand Down

0 comments on commit 4fd529a

Please sign in to comment.