Skip to content

Commit

Permalink
first stab at api re-abstraction
Browse files Browse the repository at this point in the history 
Signed-off-by: Jordan Keister <jordan@nimblewidget.com>
  • Loading branch information
@grokspawn
grokspawn committed Oct 14, 2024
1 parent 3686929 commit 926262f
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 49 deletions.
7 changes: 4 additions & 3 deletions cmd/manager/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,13 +252,14 @@ func main() {
os.Exit(1)
}

baseStorageURL, err := url.Parse(fmt.Sprintf("%s/catalogs/", externalAddr))
externalURL, err := url.Parse(externalAddr)
if err != nil {
setupLog.Error(err, "unable to create base storage URL")
setupLog.Error(err, fmt.Sprintf("unable to create base URL: %q", externalAddr))
os.Exit(1)
}
v1RawStorageURL := externalURL.JoinPath("api", "v1")

localStorage = storage.LocalDir{RootDir: storeDir, RootURL: baseStorageURL}
localStorage = storage.LocalDir{RootDir: storeDir, RootURL: v1RawStorageURL}

catalogServerConfig := serverutil.CatalogServerConfig{
ExternalAddr: externalAddr,
Expand Down
46 changes: 34 additions & 12 deletions internal/storage/localdir.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ type LocalDir struct {
RootURL *url.URL
}

const rawFileName = "all.json"

func (s LocalDir) Store(ctx context.Context, catalog string, fsys fs.FS) error {
fbcDir := filepath.Join(s.RootDir, catalog, "api", "v1")
fbcDir := filepath.Join(s.RootDir, catalog)
if err := os.MkdirAll(fbcDir, 0700); err != nil {
return err
}
Expand All @@ -40,10 +42,10 @@ func (s LocalDir) Store(ctx context.Context, catalog string, fsys fs.FS) error {
}
_, err = tempFile.Write(meta.Blob)
return err
}); err != nil {
}, declcfg.WithConcurrency(1)); err != nil {
return fmt.Errorf("error walking FBC root: %w", err)
}
fbcFile := filepath.Join(fbcDir, "all")
fbcFile := filepath.Join(fbcDir, rawFileName)
return os.Rename(tempFile.Name(), fbcFile)
}

Expand All @@ -52,7 +54,7 @@ func (s LocalDir) Delete(catalog string) error {
}

func (s LocalDir) ContentURL(catalog string) string {
return fmt.Sprintf("%s%s/api", s.RootURL, catalog)
return fmt.Sprintf("%s%s", s.RootURL, catalog)
}

func (s LocalDir) StorageServerHandler() http.Handler {
Expand All @@ -66,11 +68,12 @@ func (s LocalDir) StorageServerHandler() http.Handler {
gzHandler.ServeHTTP(w, r)
})
mux.Handle(s.RootURL.Path, typeHandler)

return mux
}

func (s LocalDir) ContentExists(catalog string) bool {
file, err := os.Stat(filepath.Join(s.RootDir, catalog, "api", "v1", "all"))
file, err := os.Stat(filepath.Join(s.RootDir, catalog, rawFileName))
if err != nil {
return false
}
Expand All @@ -88,9 +91,11 @@ type filesOnlyFilesystem struct {
FS fs.FS
}

// Open opens a named file from the underlying filesystem. If the file
// is not a regular file, it return os.ErrNotExists. Callers are resposible
// for closing the file returned.
// Open accesses a catalog from the underlying filesystem to fulfill the v1 'raw' API IFF:
// 1. the request refers to a directory (catalog name), AND
// 2. the directory contains a file matching `rawFileName` variable
// otherwise the function will return the encountered error, or os.ErrNotExist
// Callers are resposible for closing the file returned.
func (f *filesOnlyFilesystem) Open(name string) (fs.File, error) {
file, err := f.FS.Open(name)
if err != nil {
Expand All @@ -101,9 +106,26 @@ func (f *filesOnlyFilesystem) Open(name string) (fs.File, error) {
_ = file.Close()
return nil, err
}
if !stat.Mode().IsRegular() {
_ = file.Close()
return nil, os.ErrNotExist
// if the name is a directory, it should be the name of the catalog we want to access.
// test for the existence of the raw FBC file in that dir, and return it if found.
// otherwise, return os.ErrNotExist.
if stat.IsDir() {
defer file.Close()
rawFile, err := f.FS.Open(filepath.Join(name, rawFileName))
if err != nil {
_ = file.Close()
return nil, os.ErrNotExist
}
filestat, err := rawFile.Stat()
if err != nil {
_ = file.Close()
return nil, err
}
if !filestat.Mode().IsRegular() {
_ = file.Close()
return nil, os.ErrNotExist
}
return rawFile, nil
}
return file, nil
return nil, os.ErrNotExist
}
69 changes: 35 additions & 34 deletions internal/storage/localdir_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ import (
"github.com/operator-framework/operator-registry/alpha/declcfg"
)

const urlPrefix = "/catalogs/"
const fsPrefix = "/catalogs/"
const rawURLPrefix = "/api/v1/"

var ctx = context.Background()

Expand All @@ -53,9 +54,9 @@ var _ = Describe("LocalDir Storage Test", func() {
BeforeEach(func() {
d, err := os.MkdirTemp(GinkgoT().TempDir(), "cache")
Expect(err).ToNot(HaveOccurred())
rootDir = d
rootDir = fmt.Sprintf("%s%s", d, fsPrefix)

baseURL = &url.URL{Scheme: "http", Host: "test-addr", Path: urlPrefix}
baseURL = &url.URL{Scheme: "http", Host: "test-addr", Path: rawURLPrefix}
store = LocalDir{RootDir: rootDir, RootURL: baseURL}
unpackResultFS = &fstest.MapFS{
"bundle.yaml": &fstest.MapFile{Data: []byte(testBundle), Mode: os.ModePerm},
Expand All @@ -67,22 +68,23 @@ var _ = Describe("LocalDir Storage Test", func() {
BeforeEach(func() {
err := store.Store(context.Background(), catalog, unpackResultFS)
Expect(err).To(Not(HaveOccurred()))
exists := store.ContentExists(catalog)
Expect(exists).To(BeTrue())
})
It("should store the content in the RootDir correctly", func() {
fbcDir := filepath.Join(rootDir, catalog, "api", "v1")
fbcFile := filepath.Join(fbcDir, "all")
fbcDir := filepath.Join(rootDir, catalog)
fbcFile := filepath.Join(fbcDir, rawFileName)
_, 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(fbcDir), "all")
storedConfig, err := declcfg.LoadFile(os.DirFS(fbcDir), rawFileName)
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/api", baseURL, catalog)))
Expect(store.ContentURL(catalog)).To(Equal(fmt.Sprintf("%s%s", baseURL, catalog)))
})
It("should report content exists", func() {
Expect(store.ContentExists(catalog)).To(BeTrue())
Expand Down Expand Up @@ -113,10 +115,9 @@ var _ = Describe("LocalDir Server Handler tests", func() {
BeforeEach(func() {
d, err := os.MkdirTemp(GinkgoT().TempDir(), "cache")
Expect(err).ToNot(HaveOccurred())
Expect(os.MkdirAll(filepath.Join(d, "test-catalog", "api", "v1"), 0700)).To(Succeed())
store = LocalDir{RootDir: d, RootURL: &url.URL{Path: urlPrefix}}
Expect(os.MkdirAll(filepath.Join(d, "test-catalog"), 0700)).To(Succeed())
store = LocalDir{RootDir: d, RootURL: &url.URL{Path: rawURLPrefix}}
testServer = httptest.NewServer(store.StorageServerHandler())

})
It("gets 404 for the path /", func() {
expectNotFound(testServer.URL)
Expand All @@ -127,33 +128,33 @@ var _ = Describe("LocalDir Server Handler tests", func() {
It("gets 404 for the path /catalogs/test-catalog/", func() {
expectNotFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/test-catalog/"))
})
It("gets 404 for the path /test-catalog/foo.txt", func() {
// This ensures that even if the file exists, the URL must contain the /catalogs/ prefix
It("gets 404 for the path /api/v1/test-catalog when the expected raw FBC file does not exist", func() {
// This ensures that even if there is content, if it does not follow the expected format it will fail the access
Expect(os.WriteFile(filepath.Join(store.RootDir, "test-catalog", "foo.txt"), []byte("bar"), 0600)).To(Succeed())
expectNotFound(fmt.Sprintf("%s/%s", testServer.URL, "/test-catalog/foo.txt"))
})
It("gets 404 for the path /catalogs/test-catalog/non-existent.txt", func() {
expectNotFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/test-catalog/non-existent.txt"))
expectNotFound(fmt.Sprintf("%s/%s", testServer.URL, "/api/v1/test-catalog/foo.txt"))
})
It("gets 200 for the path /catalogs/foo.txt", func() {
expectedContent := []byte("bar")
Expect(os.WriteFile(filepath.Join(store.RootDir, "foo.txt"), expectedContent, 0600)).To(Succeed())
expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/foo.txt"), expectedContent)
})
It("gets 200 for the path /catalogs/test-catalog/foo.txt", func() {
expectedContent := []byte("bar")
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("gets 404 for an unknown catalog", func() {
expectNotFound(fmt.Sprintf("%s/%s", testServer.URL, "/api/vi/unknown-catalog"))
})
It("ignores accept-encoding for the path /catalogs/test-catalog/api/v1/all with size < 1400 bytes", func() {
// It("gets 200 for the path /catalogs/foo.txt", func() {
// expectedContent := []byte("bar")
// Expect(os.WriteFile(filepath.Join(store.RootDir, "foo.txt"), expectedContent, 0600)).To(Succeed())
// expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/catalogs/foo.txt"), expectedContent)
// })
// It("gets 200 for the path /catalogs/test-catalog/foo.txt", func() {
// expectedContent := []byte("bar")
// 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 raw FBC access to test-catalog with size < 1400 bytes", func() {
expectedContent := []byte("bar")
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)
Expect(os.WriteFile(filepath.Join(store.RootDir, "test-catalog", rawFileName), expectedContent, 0600)).To(Succeed())
expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/api/v1/test-catalog"), expectedContent)
})
It("provides gzipped content for the path /catalogs/test-catalog/api/v1/all with size > 1400 bytes", func() {
It("provides gzipped content for raw FBC access to test-catalog with size > 1400 bytes", func() {
expectedContent := []byte(testCompressableJSON)
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)
Expect(os.WriteFile(filepath.Join(store.RootDir, "test-catalog", rawFileName), expectedContent, 0600)).To(Succeed())
expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/api/v1/test-catalog"), expectedContent)
})
It("provides json-lines format for the served JSON catalog", func() {
catalog := "test-catalog"
Expand All @@ -165,7 +166,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/api/v1/all"), []byte(expectedContent))
expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/api/v1/test-catalog"), []byte(expectedContent))
})
It("provides json-lines format for the served YAML catalog", func() {
catalog := "test-catalog"
Expand All @@ -179,7 +180,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/api/v1/all"), []byte(expectedContent))
expectFound(fmt.Sprintf("%s/%s", testServer.URL, "/api/v1/test-catalog"), []byte(expectedContent))
})
AfterEach(func() {
testServer.Close()
Expand Down

0 comments on commit 926262f

Please sign in to comment.