From 5d7eea74bae25262edcfa35c7b7f0e0a9de54fe0 Mon Sep 17 00:00:00 2001 From: Anik Date: Thu, 3 Aug 2023 14:41:41 -0400 Subject: [PATCH 01/10] Implement storage for fbc files obsoletes #113 Signed-off-by: Anik --- cmd/manager/main.go | 4 ++ go.mod | 12 +++--- go.sum | 32 +++++++++++--- pkg/controllers/core/catalog_controller.go | 5 +++ pkg/storage/storage.go | 50 ++++++++++++++++++++++ 5 files changed, 91 insertions(+), 12 deletions(-) create mode 100644 pkg/storage/storage.go diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 0214b526..b93fe859 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -39,6 +39,7 @@ import ( corecontrollers "github.com/operator-framework/catalogd/pkg/controllers/core" "github.com/operator-framework/catalogd/pkg/features" "github.com/operator-framework/catalogd/pkg/profile" + "github.com/operator-framework/catalogd/pkg/storage" //+kubebuilder:scaffold:imports "github.com/operator-framework/catalogd/api/core/v1alpha1" @@ -65,6 +66,7 @@ func main() { profiling bool catalogdVersion bool systemNamespace string + storageDir 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.") @@ -74,6 +76,7 @@ func main() { // TODO: should we move the unpacker to some common place? Or... hear me out... should catalogd just be a rukpak provisioner? flag.StringVar(&unpackImage, "unpack-image", "quay.io/operator-framework/rukpak:v0.12.0", "The unpack image to use when unpacking catalog images") flag.StringVar(&systemNamespace, "system-namespace", "", "The namespace catalogd uses for internal state, configuration, and workloads") + flag.StringVar(&storageDir, "catalogs-storage-dir", "/var/cache", "The directory in the filesystem where unpacked catalog content will be stored and served from") 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{ @@ -119,6 +122,7 @@ func main() { if err = (&corecontrollers.CatalogReconciler{ Client: mgr.GetClient(), Unpacker: unpacker, + Storage: storage.NewStorage(storageDir), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Catalog") os.Exit(1) diff --git a/go.mod b/go.mod index bd971964..f538dc28 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/onsi/gomega v1.27.7 github.com/operator-framework/operator-registry v1.27.1 github.com/spf13/pflag v1.0.5 - github.com/stretchr/testify v1.8.0 + github.com/stretchr/testify v1.8.1 k8s.io/api v0.26.1 k8s.io/apimachinery v0.26.1 k8s.io/client-go v0.26.1 @@ -19,15 +19,17 @@ require ( ) require ( + github.com/acomagu/bufpipe v1.0.3 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.1.2 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/emicklei/go-restful/v3 v3.9.0 // indirect + github.com/evanphx/json-patch v5.6.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.6.0 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/go-git/gcfg v1.5.0 // indirect - github.com/go-git/go-billy/v5 v5.1.0 // indirect - github.com/go-git/go-git/v5 v5.3.0 // indirect + github.com/go-git/go-billy/v5 v5.4.1 // indirect + github.com/go-git/go-git/v5 v5.4.2 // indirect github.com/go-logr/logr v1.2.4 // indirect github.com/go-logr/zapr v1.2.3 // indirect github.com/go-openapi/jsonpointer v0.19.5 // indirect @@ -39,12 +41,12 @@ require ( github.com/golang/protobuf v1.5.3 // indirect github.com/google/gnostic v0.5.7-v3refs // indirect github.com/google/go-cmp v0.5.9 // indirect - github.com/google/gofuzz v1.1.0 // indirect + github.com/google/gofuzz v1.2.0 // indirect github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 // indirect github.com/google/uuid v1.3.0 // indirect github.com/h2non/filetype v1.1.1 // indirect github.com/h2non/go-is-svg v0.0.0-20160927212452-35e8c4b0612c // indirect - github.com/imdario/mergo v0.3.12 // indirect + github.com/imdario/mergo v0.3.13 // indirect github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect github.com/joelanford/ignore v0.0.0-20210607151042-0d25dc18b62d // indirect github.com/josharian/intern v1.0.0 // indirect diff --git a/go.sum b/go.sum index 130830a4..cda71e23 100644 --- a/go.sum +++ b/go.sum @@ -35,6 +35,9 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/Microsoft/go-winio v0.4.14/go.mod h1:qXqCSQ3Xa7+6tgxaGTIe4Kpcdsi+P8jBhyzoq1bpyYA= github.com/Microsoft/go-winio v0.4.16/go.mod h1:XB6nPKklQyQ7GC9LdcBEcBl8PF76WugXOPRXwdLnMv0= +github.com/ProtonMail/go-crypto v0.0.0-20210428141323-04723f9f07d7/go.mod h1:z4/9nQmJSSwwds7ejkxaJwO37dru3geImFUdJlaLzQo= +github.com/acomagu/bufpipe v1.0.3 h1:fxAGrHZTgQ9w5QqVItgzwj235/uYZYgbXitB+dLupOk= +github.com/acomagu/bufpipe v1.0.3/go.mod h1:mxdxdup/WdsKVreO5GpW4+M/1CE2sMG4jeGJ2sYmHc4= github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7/go.mod h1:6zEj6s6u/ghQa61ZWa/C2Aw3RkjiTBOix7dkqa1VLIs= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= @@ -73,7 +76,8 @@ github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.m github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ= -github.com/evanphx/json-patch v4.12.0+incompatible h1:4onqiflcdA9EOZ4RxV643DvftH5pOlLGNtQ5lPWQu84= +github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U= +github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww= github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4= github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc= @@ -83,11 +87,16 @@ github.com/gliderlabs/ssh v0.2.2/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aev github.com/go-git/gcfg v1.5.0 h1:Q5ViNfGF8zFgyJWPqYwA7qGFoMTEiBmdlkcfRmpIMa4= github.com/go-git/gcfg v1.5.0/go.mod h1:5m20vg6GwYabIxaOonVkTdrILxQMpEShl1xiMF4ua+E= github.com/go-git/go-billy/v5 v5.0.0/go.mod h1:pmpqyWchKfYfrkb/UVH4otLvyi/5gJlGI4Hb3ZqZ3W0= -github.com/go-git/go-billy/v5 v5.1.0 h1:4pl5BV4o7ZG/lterP4S6WzJ6xr49Ba5ET9ygheTYahk= github.com/go-git/go-billy/v5 v5.1.0/go.mod h1:pmpqyWchKfYfrkb/UVH4otLvyi/5gJlGI4Hb3ZqZ3W0= +github.com/go-git/go-billy/v5 v5.2.0/go.mod h1:pmpqyWchKfYfrkb/UVH4otLvyi/5gJlGI4Hb3ZqZ3W0= +github.com/go-git/go-billy/v5 v5.3.1/go.mod h1:pmpqyWchKfYfrkb/UVH4otLvyi/5gJlGI4Hb3ZqZ3W0= +github.com/go-git/go-billy/v5 v5.4.1 h1:Uwp5tDRkPr+l/TnbHOQzp+tmJfLceOlbVucgpTz8ix4= +github.com/go-git/go-billy/v5 v5.4.1/go.mod h1:vjbugF6Fz7JIflbVpl1hJsGjSHNltrSw45YK/ukIvQg= github.com/go-git/go-git-fixtures/v4 v4.0.2-0.20200613231340-f56387b50c12/go.mod h1:m+ICp2rF3jDhFgEZ/8yziagdT1C+ZpZcrJjappBCDSw= -github.com/go-git/go-git/v5 v5.3.0 h1:8WKMtJR2j8RntEXR/uvTKagfEt4GYlwQ7mntE4+0GWc= +github.com/go-git/go-git-fixtures/v4 v4.2.1/go.mod h1:K8zd3kDUAykwTdDCr+I0per6Y6vMiRR/nnVTBtavnB0= github.com/go-git/go-git/v5 v5.3.0/go.mod h1:xdX4bWJ48aOrdhnl2XqHYstHbbp6+LFS4r4X+lNVprw= +github.com/go-git/go-git/v5 v5.4.2 h1:BXyZu9t0VkbiHtqrsvdq39UDhGJTl1h55VW6CSC4aY4= +github.com/go-git/go-git/v5 v5.4.2/go.mod h1:gQ1kArt6d+n+BGd+/B/I74HwRTLhth2+zti4ihgckDc= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= @@ -166,8 +175,8 @@ github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= -github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= +github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= +github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0= github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= @@ -192,8 +201,9 @@ github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= -github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU= github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA= +github.com/imdario/mergo v0.3.13 h1:lFzP57bqS/wsqKssCGmtLAb8A0wKjLGrve2q3PPVcBk= +github.com/imdario/mergo v0.3.13/go.mod h1:4lJ1jqUDcsbIECGy0RUJAXNIhg+6ocWgb1ALK2O4oXg= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo= github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= @@ -230,6 +240,8 @@ github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= github.com/mailru/easyjson v0.7.6 h1:8yTIVnZgCoiM1TgqoeTl+LfU5Jg6/xL3QhGQnimLYnA= github.com/mailru/easyjson v0.7.6/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= +github.com/matryer/is v1.2.0 h1:92UTHpy8CDwaJ08GqLDzhhuixiBUUD1p3AU6PHddz4A= +github.com/matryer/is v1.2.0/go.mod h1:2fLPjFQM9rhQ15aVEtbuwhJinnOqrmgXPNdZsdwlWXA= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo= github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= @@ -301,6 +313,7 @@ github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= @@ -308,8 +321,9 @@ github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/xanzy/ssh-agent v0.3.0/go.mod h1:3s9xbODqPuuhK9JV1R321M/FlMZSBvE5aY6eAcqrDh0= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= @@ -340,6 +354,7 @@ golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= +golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -469,6 +484,7 @@ golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210324051608-47abb6519492/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210502180810-71e4cd670f79/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -476,6 +492,7 @@ golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -654,6 +671,7 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/pkg/controllers/core/catalog_controller.go b/pkg/controllers/core/catalog_controller.go index ec11a16e..985a23a1 100644 --- a/pkg/controllers/core/catalog_controller.go +++ b/pkg/controllers/core/catalog_controller.go @@ -43,6 +43,7 @@ import ( "github.com/operator-framework/catalogd/internal/k8sutil" "github.com/operator-framework/catalogd/internal/source" "github.com/operator-framework/catalogd/pkg/features" + "github.com/operator-framework/catalogd/pkg/storage" ) // TODO (everettraven): Add unit tests for the CatalogReconciler @@ -51,6 +52,7 @@ import ( type CatalogReconciler struct { client.Client Unpacker source.Unpacker + Storage storage.Storage } //+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=catalogs,verbs=get;list;watch;create;update;patch;delete @@ -145,6 +147,9 @@ func (r *CatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.Cat // of the unpacking steps. fbc, err := declcfg.LoadFS(unpackResult.FS) + if err := r.Storage.Store(ctx, catalog, fbc); err != nil { + return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("storing fbc: %v", err)) + } if err != nil { return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("load FBC from filesystem: %v", err)) } diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go new file mode 100644 index 00000000..531fafe1 --- /dev/null +++ b/pkg/storage/storage.go @@ -0,0 +1,50 @@ +package storage + +import ( + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/operator-framework/operator-registry/alpha/declcfg" +) + +// Storage is a store of FBC content of catalogs added to a cluster. +// It can be used to Store or Delete FBC in the host's filesystem +type Storage struct { + RootDirectory string +} + +func NewStorage(rootDir string) Storage { + return Storage{ + RootDirectory: rootDir, + } +} + +func (s *Storage) Store(owner string, fbc *declcfg.DeclarativeConfig) error { + fbcFile, err := os.Create(s.fbcPath(owner)) + if err != nil { + return err + } + defer fbcFile.Close() + + if err := declcfg.WriteJSON(*fbc, fbcFile); err != nil { + return err + } + return nil +} + +func (s *Storage) Delete(owner string) error { + return ignoreNotExist(os.Remove(s.fbcPath(owner))) +} + +func (s *Storage) fbcPath(catalogName string) string { + return filepath.Join(s.RootDirectory, fmt.Sprintf("%s.json", catalogName)) +} + +func ignoreNotExist(err error) error { + if errors.Is(err, os.ErrNotExist) { + return nil + } + return err +} From 8afa23aff0f4ff15ce3cc163896d343ac96b0077 Mon Sep 17 00:00:00 2001 From: Anik Date: Sat, 12 Aug 2023 15:45:16 -0400 Subject: [PATCH 02/10] Serve stored FBC content Signed-off-by: Anik --- cmd/manager/main.go | 12 ++++- config/manager/manager.yaml | 12 +++++ pkg/catalogserver/server.go | 50 +++++++++++++++++++ pkg/controllers/core/catalog_controller.go | 29 +++++++++-- .../core/catalog_controller_test.go | 5 ++ pkg/storage/storage.go | 12 ++++- 6 files changed, 113 insertions(+), 7 deletions(-) create mode 100644 pkg/catalogserver/server.go diff --git a/cmd/manager/main.go b/cmd/manager/main.go index b93fe859..902017cf 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -36,6 +36,7 @@ import ( "github.com/operator-framework/catalogd/internal/source" "github.com/operator-framework/catalogd/internal/version" + catlaogserver "github.com/operator-framework/catalogd/pkg/catalogserver" corecontrollers "github.com/operator-framework/catalogd/pkg/controllers/core" "github.com/operator-framework/catalogd/pkg/features" "github.com/operator-framework/catalogd/pkg/profile" @@ -67,6 +68,7 @@ func main() { catalogdVersion bool systemNamespace string storageDir string + serverPort 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.") @@ -77,6 +79,7 @@ func main() { flag.StringVar(&unpackImage, "unpack-image", "quay.io/operator-framework/rukpak:v0.12.0", "The unpack image to use when unpacking catalog images") flag.StringVar(&systemNamespace, "system-namespace", "", "The namespace catalogd uses for internal state, configuration, and workloads") flag.StringVar(&storageDir, "catalogs-storage-dir", "/var/cache", "The directory in the filesystem where unpacked catalog content will be stored and served from") + flag.StringVar(&serverPort, "catalogs-server-port", ":8083", "The port where the unpacked catalogs' content will be accessible") 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{ @@ -119,10 +122,16 @@ func main() { os.Exit(1) } + catalogServer := catlaogserver.NewServer(storageDir, serverPort) + if err := mgr.Add(catalogServer); err != nil { + setupLog.Error(err, "unable to start catalog server") + os.Exit(1) + } + if err = (&corecontrollers.CatalogReconciler{ Client: mgr.GetClient(), Unpacker: unpacker, - Storage: storage.NewStorage(storageDir), + Storage: storage.NewStorage(storageDir, catalogServer.Mux), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Catalog") os.Exit(1) @@ -145,7 +154,6 @@ func main() { os.Exit(1) } } - setupLog.Info("starting manager") if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { setupLog.Error(err, "problem running manager") diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 73d00426..2cb84ab7 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -47,8 +47,17 @@ spec: - "./manager" args: - --leader-elect + - "--catalogs-storage-dir=/var/cache" + - "--catalogs-server-port=:8083" image: controller:latest name: manager + ports: + - containerPort: 8083 + protocol: TCP + name: https + volumeMounts: + - name: catalog-cache + mountPath: /var/cache/ securityContext: allowPrivilegeEscalation: false capabilities: @@ -73,3 +82,6 @@ spec: imagePullPolicy: IfNotPresent serviceAccountName: controller-manager terminationGracePeriodSeconds: 10 + volumes: + - name: catalog-cache + emptyDir: {} diff --git a/pkg/catalogserver/server.go b/pkg/catalogserver/server.go new file mode 100644 index 00000000..e8809a64 --- /dev/null +++ b/pkg/catalogserver/server.go @@ -0,0 +1,50 @@ +package catalogserver + +import ( + "context" + "fmt" + "net/http" + "os" + "path/filepath" + "time" +) + +// Server is a manager.Runnable Server, that serves the FBC +// content of the extension catalogs added to the cluster +type Server struct { + Dir string + Port string + Mux *http.ServeMux +} + +// NewServer takes directory and port number, and returns +// a Server that serves the FBC content stored in the +// directory on the given port number. +func NewServer(dir, port string) Server { + return Server{ + Dir: dir, + Port: port, + Mux: &http.ServeMux{}, + } +} + +func (s Server) Start(_ context.Context) error { + s.Mux.HandleFunc("/catalogs", func(w http.ResponseWriter, r *http.Request) { + files, err := os.ReadDir(s.Dir) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + fmt.Fprintf(w, "error reading catalog store directory: %v", err) + return + } + for _, file := range files { + name := file.Name() + fmt.Fprintf(w, "%v\n", name[:len(name)-len(filepath.Ext(name))]) + } + }) + server := &http.Server{ + Handler: s.Mux, + Addr: s.Port, + ReadHeaderTimeout: 3 * time.Second, + } + return server.ListenAndServe() +} diff --git a/pkg/controllers/core/catalog_controller.go b/pkg/controllers/core/catalog_controller.go index 985a23a1..ba477ada 100644 --- a/pkg/controllers/core/catalog_controller.go +++ b/pkg/controllers/core/catalog_controller.go @@ -36,6 +36,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -46,7 +47,7 @@ import ( "github.com/operator-framework/catalogd/pkg/storage" ) -// TODO (everettraven): Add unit tests for the CatalogReconciler +const fbcDeletionFinalizer = "operator-framework.catalogd/localstore" // CatalogReconciler reconciles a Catalog object type CatalogReconciler struct { @@ -81,7 +82,27 @@ func (r *CatalogReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct if err := r.Client.Get(ctx, req.NamespacedName, &existingCatsrc); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } - + if existingCatsrc.DeletionTimestamp.IsZero() { + if !controllerutil.ContainsFinalizer(&existingCatsrc, fbcDeletionFinalizer) { + controllerutil.AddFinalizer(&existingCatsrc, fbcDeletionFinalizer) + if err := r.Update(ctx, &existingCatsrc); err != nil { + return ctrl.Result{}, err + } + } + } else { + if controllerutil.ContainsFinalizer(&existingCatsrc, fbcDeletionFinalizer) { + err := r.Storage.Delete(existingCatsrc.Name) + if err != nil { + return ctrl.Result{}, err + } + // remove our finalizer from the list and update it. + controllerutil.RemoveFinalizer(&existingCatsrc, fbcDeletionFinalizer) + if err := r.Update(ctx, &existingCatsrc); err != nil { + return ctrl.Result{}, err + } + } + return ctrl.Result{}, nil + } reconciledCatsrc := existingCatsrc.DeepCopy() res, reconcileErr := r.reconcile(ctx, reconciledCatsrc) @@ -147,8 +168,8 @@ func (r *CatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.Cat // of the unpacking steps. fbc, err := declcfg.LoadFS(unpackResult.FS) - if err := r.Storage.Store(ctx, catalog, fbc); err != nil { - return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("storing fbc: %v", err)) + if err := r.Storage.Store(catalog.Name, fbc); err != nil { + return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("error storing fbc: %v", err)) } if err != nil { return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("load FBC from filesystem: %v", err)) diff --git a/pkg/controllers/core/catalog_controller_test.go b/pkg/controllers/core/catalog_controller_test.go index e4b82385..291548e3 100644 --- a/pkg/controllers/core/catalog_controller_test.go +++ b/pkg/controllers/core/catalog_controller_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net/http" "os" "testing/fstest" @@ -23,6 +24,7 @@ import ( "github.com/operator-framework/catalogd/internal/source" "github.com/operator-framework/catalogd/pkg/controllers/core" "github.com/operator-framework/catalogd/pkg/features" + "github.com/operator-framework/catalogd/pkg/storage" ) var _ source.Unpacker = &MockSource{} @@ -52,6 +54,8 @@ var _ = Describe("Catalogd Controller Test", func() { mockSource *MockSource ) BeforeEach(func() { + tmpDir, err := os.MkdirTemp(GinkgoT().TempDir(), "cache") + Expect(err).ToNot(HaveOccurred()) ctx = context.Background() mockSource = &MockSource{} reconciler = &core.CatalogReconciler{ @@ -61,6 +65,7 @@ var _ = Describe("Catalogd Controller Test", func() { v1alpha1.SourceTypeImage: mockSource, }, ), + Storage: storage.NewStorage(tmpDir, http.NewServeMux()), } }) diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 531fafe1..64bad7e8 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -3,6 +3,7 @@ package storage import ( "errors" "fmt" + "net/http" "os" "path/filepath" @@ -13,11 +14,13 @@ import ( // It can be used to Store or Delete FBC in the host's filesystem type Storage struct { RootDirectory string + ServerMux *http.ServeMux } -func NewStorage(rootDir string) Storage { +func NewStorage(rootDir string, mux *http.ServeMux) Storage { return Storage{ RootDirectory: rootDir, + ServerMux: mux, } } @@ -31,6 +34,7 @@ func (s *Storage) Store(owner string, fbc *declcfg.DeclarativeConfig) error { if err := declcfg.WriteJSON(*fbc, fbcFile); err != nil { return err } + s.registerFileForServing(s.ServerMux, owner) return nil } @@ -38,6 +42,12 @@ func (s *Storage) Delete(owner string) error { return ignoreNotExist(os.Remove(s.fbcPath(owner))) } +func (s *Storage) registerFileForServing(mux *http.ServeMux, name string) { + mux.HandleFunc(fmt.Sprintf("/catalogs/%s", name), func(w http.ResponseWriter, r *http.Request) { + http.ServeFile(w, r, filepath.Join(s.RootDirectory, fmt.Sprintf("%s.json", name))) + }) +} + func (s *Storage) fbcPath(catalogName string) string { return filepath.Join(s.RootDirectory, fmt.Sprintf("%s.json", catalogName)) } From 9a2a909633169e75a81f1254d7aee4eb6d119c6b Mon Sep 17 00:00:00 2001 From: Anik Date: Mon, 14 Aug 2023 13:09:15 -0400 Subject: [PATCH 03/10] put storage and server behind featuregate Signed-off-by: Anik --- config/default/manager_auth_proxy_patch.yaml | 2 +- pkg/controllers/core/catalog_controller.go | 45 +++++++++++--------- pkg/features/features.go | 7 ++- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/config/default/manager_auth_proxy_patch.yaml b/config/default/manager_auth_proxy_patch.yaml index 3bd84262..2f311a57 100644 --- a/config/default/manager_auth_proxy_patch.yaml +++ b/config/default/manager_auth_proxy_patch.yaml @@ -50,5 +50,5 @@ spec: - "--health-probe-bind-address=:8081" - "--metrics-bind-address=127.0.0.1:8080" - "--leader-elect" - - "--feature-gates=PackagesBundleMetadataAPIs=true,CatalogMetadataAPI=true" + - "--feature-gates=PackagesBundleMetadataAPIs=true,CatalogMetadataAPI=true,HTTPServer=false" diff --git a/pkg/controllers/core/catalog_controller.go b/pkg/controllers/core/catalog_controller.go index ba477ada..7781668d 100644 --- a/pkg/controllers/core/catalog_controller.go +++ b/pkg/controllers/core/catalog_controller.go @@ -82,26 +82,28 @@ func (r *CatalogReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct if err := r.Client.Get(ctx, req.NamespacedName, &existingCatsrc); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } - if existingCatsrc.DeletionTimestamp.IsZero() { - if !controllerutil.ContainsFinalizer(&existingCatsrc, fbcDeletionFinalizer) { - controllerutil.AddFinalizer(&existingCatsrc, fbcDeletionFinalizer) - if err := r.Update(ctx, &existingCatsrc); err != nil { - return ctrl.Result{}, err - } - } - } else { - if controllerutil.ContainsFinalizer(&existingCatsrc, fbcDeletionFinalizer) { - err := r.Storage.Delete(existingCatsrc.Name) - if err != nil { - return ctrl.Result{}, err + if features.CatalogdFeatureGate.Enabled(features.HTTPServer) { + if existingCatsrc.DeletionTimestamp.IsZero() { + if !controllerutil.ContainsFinalizer(&existingCatsrc, fbcDeletionFinalizer) { + controllerutil.AddFinalizer(&existingCatsrc, fbcDeletionFinalizer) + if err := r.Update(ctx, &existingCatsrc); err != nil { + return ctrl.Result{}, err + } } - // remove our finalizer from the list and update it. - controllerutil.RemoveFinalizer(&existingCatsrc, fbcDeletionFinalizer) - if err := r.Update(ctx, &existingCatsrc); err != nil { - return ctrl.Result{}, err + } else { + if controllerutil.ContainsFinalizer(&existingCatsrc, fbcDeletionFinalizer) { + err := r.Storage.Delete(existingCatsrc.Name) + if err != nil { + return ctrl.Result{}, err + } + // remove our finalizer from the list and update it. + controllerutil.RemoveFinalizer(&existingCatsrc, fbcDeletionFinalizer) + if err := r.Update(ctx, &existingCatsrc); err != nil { + return ctrl.Result{}, err + } } + return ctrl.Result{}, nil } - return ctrl.Result{}, nil } reconciledCatsrc := existingCatsrc.DeepCopy() res, reconcileErr := r.reconcile(ctx, reconciledCatsrc) @@ -168,13 +170,14 @@ func (r *CatalogReconciler) reconcile(ctx context.Context, catalog *v1alpha1.Cat // of the unpacking steps. fbc, err := declcfg.LoadFS(unpackResult.FS) - if err := r.Storage.Store(catalog.Name, fbc); err != nil { - return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("error storing fbc: %v", err)) - } if err != nil { return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("load FBC from filesystem: %v", err)) } - + if features.CatalogdFeatureGate.Enabled(features.HTTPServer) { + if err := r.Storage.Store(catalog.Name, fbc); err != nil { + return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("error storing fbc: %v", err)) + } + } if features.CatalogdFeatureGate.Enabled(features.PackagesBundleMetadataAPIs) { if err := r.syncPackages(ctx, fbc, catalog); err != nil { return ctrl.Result{}, updateStatusUnpackFailing(&catalog.Status, fmt.Errorf("create package objects: %v", err)) diff --git a/pkg/features/features.go b/pkg/features/features.go index 3fce4e59..1663d160 100644 --- a/pkg/features/features.go +++ b/pkg/features/features.go @@ -11,6 +11,7 @@ const ( CatalogMetadataAPI featuregate.Feature = "CatalogMetadataAPI" PackagesBundleMetadataAPIs featuregate.Feature = "PackagesBundleMetadataAPIs" + HTTPServer featuregate.Feature = "HTTPServer" ) var catalogdFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ @@ -18,10 +19,8 @@ var catalogdFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ // Ex: SomeFeature: {...} PackagesBundleMetadataAPIs: {Default: false, PreRelease: featuregate.Deprecated}, - - // Marking the CatalogMetadataAPI feature gate as Deprecated in the interest of introducing - // the HTTP Server functionality in the future and use it as a default method of serving the catalog contents. - CatalogMetadataAPI: {Default: false, PreRelease: featuregate.Deprecated}, + CatalogMetadataAPI: {Default: false, PreRelease: featuregate.Deprecated}, + HTTPServer: {Default: false, PreRelease: featuregate.Alpha}, } var CatalogdFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate() From e6b10ff9d0a8dc4d3434cf896c389cb268b8f82a Mon Sep 17 00:00:00 2001 From: Anik Date: Mon, 14 Aug 2023 14:31:39 -0400 Subject: [PATCH 04/10] add unit tests Signed-off-by: Anik --- cmd/manager/main.go | 4 +- pkg/controllers/core/catalog_controller.go | 33 +++++------- .../core/catalog_controller_test.go | 51 ++++++++++++++++++- 3 files changed, 63 insertions(+), 25 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 902017cf..f1bcc696 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -36,7 +36,7 @@ import ( "github.com/operator-framework/catalogd/internal/source" "github.com/operator-framework/catalogd/internal/version" - catlaogserver "github.com/operator-framework/catalogd/pkg/catalogserver" + catalogserver "github.com/operator-framework/catalogd/pkg/catalogserver" corecontrollers "github.com/operator-framework/catalogd/pkg/controllers/core" "github.com/operator-framework/catalogd/pkg/features" "github.com/operator-framework/catalogd/pkg/profile" @@ -122,7 +122,7 @@ func main() { os.Exit(1) } - catalogServer := catlaogserver.NewServer(storageDir, serverPort) + catalogServer := catalogserver.NewServer(storageDir, serverPort) if err := mgr.Add(catalogServer); err != nil { setupLog.Error(err, "unable to start catalog server") os.Exit(1) diff --git a/pkg/controllers/core/catalog_controller.go b/pkg/controllers/core/catalog_controller.go index 7781668d..07d0a2ef 100644 --- a/pkg/controllers/core/catalog_controller.go +++ b/pkg/controllers/core/catalog_controller.go @@ -82,28 +82,19 @@ func (r *CatalogReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct if err := r.Client.Get(ctx, req.NamespacedName, &existingCatsrc); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } - if features.CatalogdFeatureGate.Enabled(features.HTTPServer) { - if existingCatsrc.DeletionTimestamp.IsZero() { - if !controllerutil.ContainsFinalizer(&existingCatsrc, fbcDeletionFinalizer) { - controllerutil.AddFinalizer(&existingCatsrc, fbcDeletionFinalizer) - if err := r.Update(ctx, &existingCatsrc); err != nil { - return ctrl.Result{}, err - } - } - } else { - if controllerutil.ContainsFinalizer(&existingCatsrc, fbcDeletionFinalizer) { - err := r.Storage.Delete(existingCatsrc.Name) - if err != nil { - return ctrl.Result{}, err - } - // remove our finalizer from the list and update it. - controllerutil.RemoveFinalizer(&existingCatsrc, fbcDeletionFinalizer) - if err := r.Update(ctx, &existingCatsrc); err != nil { - return ctrl.Result{}, err - } - } - return ctrl.Result{}, nil + if features.CatalogdFeatureGate.Enabled(features.HTTPServer) && existingCatsrc.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(&existingCatsrc, fbcDeletionFinalizer) { + controllerutil.AddFinalizer(&existingCatsrc, fbcDeletionFinalizer) + if err := r.Update(ctx, &existingCatsrc); err != nil { + return ctrl.Result{}, err + } + } + if features.CatalogdFeatureGate.Enabled(features.HTTPServer) && !existingCatsrc.DeletionTimestamp.IsZero() && controllerutil.ContainsFinalizer(&existingCatsrc, fbcDeletionFinalizer) { + if err := r.Storage.Delete(existingCatsrc.Name); err != nil { + return ctrl.Result{}, err } + controllerutil.RemoveFinalizer(&existingCatsrc, fbcDeletionFinalizer) + err := r.Update(ctx, &existingCatsrc) + return ctrl.Result{}, err } reconciledCatsrc := existingCatsrc.DeepCopy() res, reconcileErr := r.reconcile(ctx, reconciledCatsrc) diff --git a/pkg/controllers/core/catalog_controller_test.go b/pkg/controllers/core/catalog_controller_test.go index 291548e3..c1fcefab 100644 --- a/pkg/controllers/core/catalog_controller_test.go +++ b/pkg/controllers/core/catalog_controller_test.go @@ -6,11 +6,13 @@ import ( "fmt" "net/http" "os" + "path/filepath" "testing/fstest" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/onsi/gomega/format" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -154,7 +156,13 @@ var _ = Describe("Catalogd Controller Test", func() { AfterEach(func() { By("tearing down cluster state") - Expect(cl.Delete(ctx, catalog)).To(Succeed()) + Eventually(func() error { + err := cl.Delete(ctx, catalog) + if !apierrors.IsNotFound(err) { + return err + } + return nil + }).Should(Succeed()) }) When("unpacker returns source.Result with state == 'Pending'", func() { @@ -278,11 +286,48 @@ var _ = Describe("Catalogd Controller Test", func() { Expect(cond.Status).To(Equal(metav1.ConditionTrue)) }) + When("HTTPServer feature gate is enabled", func() { + BeforeEach(func() { + Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{ + string(features.PackagesBundleMetadataAPIs): false, + string(features.CatalogMetadataAPI): false, + string(features.HTTPServer): true, + })).NotTo(HaveOccurred()) + + // reconcile + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + }) + It("should store the FBC in the cache directory", func() { + fbcFile := filepath.Join(reconciler.Storage.RootDirectory, fmt.Sprintf("%s.json", catalog.Name)) + _, err := os.Stat(fbcFile) + Expect(err).To(Not(HaveOccurred())) + }) + + When("the catalog is deleted", func() { + BeforeEach(func() { + Expect(cl.Delete(ctx, catalog)).Should(Succeed()) + }) + It("should delete the FBC from the cache directory", func() { + // reconcile + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + + fbcFile := filepath.Join(reconciler.Storage.RootDirectory, fmt.Sprintf("%s.json", catalog.Name)) + _, err = os.Stat(fbcFile) + Expect(err).To(HaveOccurred()) + Expect(os.IsNotExist(err)).To(BeTrue()) + }) + }) + }) When("PackagesBundleMetadataAPIs feature gate is enabled", func() { BeforeEach(func() { Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{ string(features.PackagesBundleMetadataAPIs): true, string(features.CatalogMetadataAPI): false, + string(features.HTTPServer): false, })).NotTo(HaveOccurred()) // reconcile @@ -417,7 +462,9 @@ var _ = Describe("Catalogd Controller Test", func() { When("the CatalogMetadataAPI feature gate is enabled", func() { BeforeEach(func() { Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{ - string(features.CatalogMetadataAPI): true, + string(features.CatalogMetadataAPI): true, + string(features.PackagesBundleMetadataAPIs): false, + string(features.HTTPServer): false, })).NotTo(HaveOccurred()) // reconcile From 9d60f8311fba161a9452eef9243b60a216bd8400 Mon Sep 17 00:00:00 2001 From: Anik Date: Wed, 16 Aug 2023 12:20:48 -0400 Subject: [PATCH 05/10] start server in goroutine and shutdown gracefully Signed-off-by: Anik --- cmd/manager/main.go | 19 +++++++++++++------ pkg/catalogserver/server.go | 35 ++++++++++++++++++++++++----------- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index f1bcc696..70bba894 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -19,6 +19,7 @@ package main import ( "flag" "fmt" + "net/http" "os" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) @@ -36,7 +37,7 @@ import ( "github.com/operator-framework/catalogd/internal/source" "github.com/operator-framework/catalogd/internal/version" - catalogserver "github.com/operator-framework/catalogd/pkg/catalogserver" + "github.com/operator-framework/catalogd/pkg/catalogserver" corecontrollers "github.com/operator-framework/catalogd/pkg/controllers/core" "github.com/operator-framework/catalogd/pkg/features" "github.com/operator-framework/catalogd/pkg/profile" @@ -68,7 +69,7 @@ func main() { catalogdVersion bool systemNamespace string storageDir string - serverPort string + catalogServerAddr 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.") @@ -78,8 +79,8 @@ func main() { // TODO: should we move the unpacker to some common place? Or... hear me out... should catalogd just be a rukpak provisioner? flag.StringVar(&unpackImage, "unpack-image", "quay.io/operator-framework/rukpak:v0.12.0", "The unpack image to use when unpacking catalog images") flag.StringVar(&systemNamespace, "system-namespace", "", "The namespace catalogd uses for internal state, configuration, and workloads") - flag.StringVar(&storageDir, "catalogs-storage-dir", "/var/cache", "The directory in the filesystem where unpacked catalog content will be stored and served from") - flag.StringVar(&serverPort, "catalogs-server-port", ":8083", "The port where the unpacked catalogs' content will be accessible") + 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", "127.0.0.1:8083", "The port where the unpacked catalogs' content will be accessible") 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{ @@ -122,16 +123,22 @@ func main() { os.Exit(1) } - catalogServer := catalogserver.NewServer(storageDir, serverPort) + serverMux := &http.ServeMux{} + catalogServer := catalogserver.New(storageDir, catalogServerAddr, serverMux) if err := mgr.Add(catalogServer); err != nil { setupLog.Error(err, "unable to start catalog server") os.Exit(1) } + if _, err := os.Stat(storageDir); os.IsNotExist(err) { + if err := os.MkdirAll(storageDir, 0700); err != nil { + setupLog.Error(err, "unable to create storage directory for catalogs") + } + } if err = (&corecontrollers.CatalogReconciler{ Client: mgr.GetClient(), Unpacker: unpacker, - Storage: storage.NewStorage(storageDir, catalogServer.Mux), + Storage: storage.NewStorage(storageDir, serverMux), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Catalog") os.Exit(1) diff --git a/pkg/catalogserver/server.go b/pkg/catalogserver/server.go index e8809a64..755098ab 100644 --- a/pkg/catalogserver/server.go +++ b/pkg/catalogserver/server.go @@ -9,26 +9,27 @@ import ( "time" ) -// Server is a manager.Runnable Server, that serves the FBC -// content of the extension catalogs added to the cluster -type Server struct { +// Instance is a manager.Runnable catalog server instance, +// that serves the FBC content of the extension catalogs +// added to the cluster +type Instance struct { Dir string Port string Mux *http.ServeMux } -// NewServer takes directory and port number, and returns -// a Server that serves the FBC content stored in the -// directory on the given port number. -func NewServer(dir, port string) Server { - return Server{ +// New returns an Instance of a catalog server that serves +// the FBC content stored in the given directory on the given +// http address. +func New(dir, port string, mux *http.ServeMux) Instance { + return Instance{ Dir: dir, Port: port, - Mux: &http.ServeMux{}, + Mux: mux, } } -func (s Server) Start(_ context.Context) error { +func (s Instance) Start(ctx context.Context) error { s.Mux.HandleFunc("/catalogs", func(w http.ResponseWriter, r *http.Request) { files, err := os.ReadDir(s.Dir) if err != nil { @@ -46,5 +47,17 @@ func (s Server) Start(_ context.Context) error { Addr: s.Port, ReadHeaderTimeout: 3 * time.Second, } - return server.ListenAndServe() + e := make(chan error) + go func(server *http.Server, e chan error) { + err := server.ListenAndServe() + e <- err + close(e) + }(server, e) + err := <-e + if err != nil { + return err + } + + <-ctx.Done() + return server.Shutdown(context.TODO()) } From 4e304f990cb19712ff3d85d30e6d957dba8e1aaf Mon Sep 17 00:00:00 2001 From: Anik Date: Wed, 16 Aug 2023 12:34:14 -0400 Subject: [PATCH 06/10] store catalogs in /var/cache/catalogs dir Signed-off-by: Anik --- config/manager/manager.yaml | 4 ++-- pkg/controllers/core/catalog_controller.go | 2 +- .../core/catalog_controller_test.go | 9 +------- pkg/storage/storage.go | 23 +++++++------------ 4 files changed, 12 insertions(+), 26 deletions(-) diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 2cb84ab7..fb440e40 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -47,8 +47,8 @@ spec: - "./manager" args: - --leader-elect - - "--catalogs-storage-dir=/var/cache" - - "--catalogs-server-port=:8083" + - "--catalogs-storage-dir=/var/cache/catalogs" + - "--catalogs-server-addr=127.0.0.1:8083" image: controller:latest name: manager ports: diff --git a/pkg/controllers/core/catalog_controller.go b/pkg/controllers/core/catalog_controller.go index 07d0a2ef..0af66824 100644 --- a/pkg/controllers/core/catalog_controller.go +++ b/pkg/controllers/core/catalog_controller.go @@ -47,7 +47,7 @@ import ( "github.com/operator-framework/catalogd/pkg/storage" ) -const fbcDeletionFinalizer = "operator-framework.catalogd/localstore" +const fbcDeletionFinalizer = "catalogd.operatorframework.io/delete-server-cache" // CatalogReconciler reconciles a Catalog object type CatalogReconciler struct { diff --git a/pkg/controllers/core/catalog_controller_test.go b/pkg/controllers/core/catalog_controller_test.go index c1fcefab..5f86c200 100644 --- a/pkg/controllers/core/catalog_controller_test.go +++ b/pkg/controllers/core/catalog_controller_test.go @@ -12,7 +12,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/onsi/gomega/format" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -156,13 +155,7 @@ var _ = Describe("Catalogd Controller Test", func() { AfterEach(func() { By("tearing down cluster state") - Eventually(func() error { - err := cl.Delete(ctx, catalog) - if !apierrors.IsNotFound(err) { - return err - } - return nil - }).Should(Succeed()) + Expect(client.IgnoreNotFound(cl.Delete(ctx, catalog))).To(Succeed()) }) When("unpacker returns source.Result with state == 'Pending'", func() { diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 64bad7e8..bcc5797d 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -34,27 +34,20 @@ func (s *Storage) Store(owner string, fbc *declcfg.DeclarativeConfig) error { if err := declcfg.WriteJSON(*fbc, fbcFile); err != nil { return err } - s.registerFileForServing(s.ServerMux, owner) + s.ServerMux.HandleFunc(fmt.Sprintf("/catalogs/%s", owner), func(w http.ResponseWriter, r *http.Request) { + http.ServeFile(w, r, filepath.Join(s.RootDirectory, fmt.Sprintf("%s.json", owner))) + }) return nil } func (s *Storage) Delete(owner string) error { - return ignoreNotExist(os.Remove(s.fbcPath(owner))) -} - -func (s *Storage) registerFileForServing(mux *http.ServeMux, name string) { - mux.HandleFunc(fmt.Sprintf("/catalogs/%s", name), func(w http.ResponseWriter, r *http.Request) { - http.ServeFile(w, r, filepath.Join(s.RootDirectory, fmt.Sprintf("%s.json", name))) - }) -} - -func (s *Storage) fbcPath(catalogName string) string { - return filepath.Join(s.RootDirectory, fmt.Sprintf("%s.json", catalogName)) -} - -func ignoreNotExist(err error) error { + err := os.Remove(s.fbcPath(owner)) if errors.Is(err, os.ErrNotExist) { return nil } return err } + +func (s *Storage) fbcPath(catalogName string) string { + return filepath.Join(s.RootDirectory, fmt.Sprintf("%s.json", catalogName)) +} From d9069e96add9cca61bcce91546d2281cae98e470 Mon Sep 17 00:00:00 2001 From: Anik Date: Wed, 16 Aug 2023 16:11:57 -0400 Subject: [PATCH 07/10] use httptest package to test server Signed-off-by: Anik --- .../core/catalog_controller_test.go | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/pkg/controllers/core/catalog_controller_test.go b/pkg/controllers/core/catalog_controller_test.go index 5f86c200..ee7ecb83 100644 --- a/pkg/controllers/core/catalog_controller_test.go +++ b/pkg/controllers/core/catalog_controller_test.go @@ -4,9 +4,10 @@ import ( "context" "errors" "fmt" + "io" "net/http" + "net/http/httptest" "os" - "path/filepath" "testing/fstest" . "github.com/onsi/ginkgo/v2" @@ -23,6 +24,7 @@ import ( "github.com/operator-framework/catalogd/api/core/v1alpha1" "github.com/operator-framework/catalogd/internal/k8sutil" "github.com/operator-framework/catalogd/internal/source" + "github.com/operator-framework/catalogd/pkg/catalogserver" "github.com/operator-framework/catalogd/pkg/controllers/core" "github.com/operator-framework/catalogd/pkg/features" "github.com/operator-framework/catalogd/pkg/storage" @@ -53,6 +55,7 @@ var _ = Describe("Catalogd Controller Test", func() { ctx context.Context reconciler *core.CatalogReconciler mockSource *MockSource + testServer *httptest.Server ) BeforeEach(func() { tmpDir, err := os.MkdirTemp(GinkgoT().TempDir(), "cache") @@ -68,8 +71,11 @@ var _ = Describe("Catalogd Controller Test", func() { ), Storage: storage.NewStorage(tmpDir, http.NewServeMux()), } + testServer = httptest.NewServer(catalogserver.MuxForServer(tmpDir)) + }) + AfterEach(func() { + testServer.Close() }) - When("the catalog does not exist", func() { It("returns no error", func() { res, err := reconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: "non-existent"}}) @@ -292,26 +298,33 @@ var _ = Describe("Catalogd Controller Test", func() { Expect(res).To(Equal(ctrl.Result{})) Expect(err).ToNot(HaveOccurred()) }) - It("should store the FBC in the cache directory", func() { - fbcFile := filepath.Join(reconciler.Storage.RootDirectory, fmt.Sprintf("%s.json", catalog.Name)) - _, err := os.Stat(fbcFile) + It("the catalog should become available at addr/catalogs", func() { + res, err := http.Get(testServer.URL + "/catalogs") + Expect(err).To(Not(HaveOccurred())) + catalogs, err := io.ReadAll(res.Body) + res.Body.Close() Expect(err).To(Not(HaveOccurred())) + //omitting trailing new line char from response + Expect(string(catalogs[:len(catalogs)-1])).To(Equal(catalogKey.Name)) }) When("the catalog is deleted", func() { BeforeEach(func() { Expect(cl.Delete(ctx, catalog)).Should(Succeed()) }) - It("should delete the FBC from the cache directory", func() { + It("should not be available at addr/catalogs anymore", func() { // reconcile res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey}) Expect(res).To(Equal(ctrl.Result{})) Expect(err).ToNot(HaveOccurred()) - fbcFile := filepath.Join(reconciler.Storage.RootDirectory, fmt.Sprintf("%s.json", catalog.Name)) - _, err = os.Stat(fbcFile) - Expect(err).To(HaveOccurred()) - Expect(os.IsNotExist(err)).To(BeTrue()) + newres, err := http.Get(testServer.URL + "/catalogs") + Expect(err).To(Not(HaveOccurred())) + catalogs, err := io.ReadAll(newres.Body) + newres.Body.Close() + Expect(err).To(Not(HaveOccurred())) + //omitting trailing new line char from response + Expect(string(catalogs)).To(Equal("")) }) }) }) From 50842c64af3fe3e4f8fbc199330673f41839b180 Mon Sep 17 00:00:00 2001 From: Anik Date: Wed, 16 Aug 2023 16:13:37 -0400 Subject: [PATCH 08/10] switch to using errgroup Signed-off-by: Anik --- cmd/manager/main.go | 3 +- go.mod | 1 + go.sum | 2 ++ pkg/catalogserver/server.go | 69 +++++++++++++++++++++++-------------- 4 files changed, 48 insertions(+), 27 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 70bba894..f9c67da5 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -19,7 +19,6 @@ package main import ( "flag" "fmt" - "net/http" "os" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) @@ -123,7 +122,7 @@ func main() { os.Exit(1) } - serverMux := &http.ServeMux{} + serverMux := catalogserver.MuxForServer(storageDir) catalogServer := catalogserver.New(storageDir, catalogServerAddr, serverMux) if err := mgr.Add(catalogServer); err != nil { setupLog.Error(err, "unable to start catalog server") diff --git a/go.mod b/go.mod index f538dc28..6411b9f1 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/operator-framework/operator-registry v1.27.1 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.1 + golang.org/x/sync v0.2.0 k8s.io/api v0.26.1 k8s.io/apimachinery v0.26.1 k8s.io/client-go v0.26.1 diff --git a/go.sum b/go.sum index cda71e23..a7eead23 100644 --- a/go.sum +++ b/go.sum @@ -446,6 +446,8 @@ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.2.0 h1:PUR+T4wwASmuSTYdKjYHI5TD22Wy5ogLU5qZCOLxBrI= +golang.org/x/sync v0.2.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= diff --git a/pkg/catalogserver/server.go b/pkg/catalogserver/server.go index 755098ab..f40c191d 100644 --- a/pkg/catalogserver/server.go +++ b/pkg/catalogserver/server.go @@ -7,31 +7,67 @@ import ( "os" "path/filepath" "time" + + "golang.org/x/sync/errgroup" ) // Instance is a manager.Runnable catalog server instance, // that serves the FBC content of the extension catalogs // added to the cluster type Instance struct { - Dir string - Port string - Mux *http.ServeMux + Dir string + Addr string + Mux *http.ServeMux + ShutdownTimeout time.Duration } // New returns an Instance of a catalog server that serves // the FBC content stored in the given directory on the given // http address. -func New(dir, port string, mux *http.ServeMux) Instance { +func New(dir, addr string, mux *http.ServeMux) Instance { return Instance{ Dir: dir, - Port: port, + Addr: addr, Mux: mux, } } func (s Instance) Start(ctx context.Context) error { - s.Mux.HandleFunc("/catalogs", func(w http.ResponseWriter, r *http.Request) { - files, err := os.ReadDir(s.Dir) + server := &http.Server{ + Handler: s.Mux, + Addr: s.Addr, + ReadHeaderTimeout: 3 * time.Second, + } + eg, ctx := errgroup.WithContext(ctx) + // run a server in a go routine + // server.ListenAndServer() returns under two circumstances + // 1. If there was an error starting the server + // 2. If the server was shut down (ErrServerClosed) + // i.e both are non-nil errors + eg.Go(func() error { return server.ListenAndServe() }) + // waiting for one of two things + // 1. a error is returned from the go routine + // 2. the runnable's context is cancelled + if err := eg.Wait(); err != nil && ctx.Err() == nil { + // we only get here if we're in case 1 (both case 1s) + return err + } + // if the ShutdownTimeout is zero, wait forever to shutdown + // otherwise force shut down when timeout expires + sc := context.Background() + if s.ShutdownTimeout > 0 { + var scc context.CancelFunc + sc, scc = context.WithTimeout(context.Background(), s.ShutdownTimeout) + defer scc() + } + // if the runnable's context was cancelled, shut down the server + return server.Shutdown(sc) +} + +func MuxForServer(dir string) *http.ServeMux { + m := &http.ServeMux{} + m.HandleFunc("/catalogs", func(w http.ResponseWriter, r *http.Request) { + files, err := os.ReadDir(dir) if err != nil { w.WriteHeader(http.StatusBadRequest) fmt.Fprintf(w, "error reading catalog store directory: %v", err) @@ -42,22 +78,5 @@ func (s Instance) Start(ctx context.Context) error { fmt.Fprintf(w, "%v\n", name[:len(name)-len(filepath.Ext(name))]) } }) - server := &http.Server{ - Handler: s.Mux, - Addr: s.Port, - ReadHeaderTimeout: 3 * time.Second, - } - e := make(chan error) - go func(server *http.Server, e chan error) { - err := server.ListenAndServe() - e <- err - close(e) - }(server, e) - err := <-e - if err != nil { - return err - } - - <-ctx.Done() - return server.Shutdown(context.TODO()) + return m } From 2d6410443b54757f716c875ff087b16414786716 Mon Sep 17 00:00:00 2001 From: Anik Date: Thu, 17 Aug 2023 12:05:41 -0400 Subject: [PATCH 09/10] switch to http.FileServer Signed-off-by: Anik --- cmd/manager/main.go | 11 ++--- pkg/catalogserver/server.go | 26 +++--------- .../core/catalog_controller_test.go | 40 ++++++++++++------- pkg/storage/storage.go | 8 +--- 4 files changed, 36 insertions(+), 49 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index f9c67da5..7fc807f8 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -122,22 +122,19 @@ func main() { os.Exit(1) } - serverMux := catalogserver.MuxForServer(storageDir) - catalogServer := catalogserver.New(storageDir, catalogServerAddr, serverMux) + catalogServer := catalogserver.New(storageDir, catalogServerAddr) if err := mgr.Add(catalogServer); err != nil { setupLog.Error(err, "unable to start catalog server") os.Exit(1) } - if _, err := os.Stat(storageDir); os.IsNotExist(err) { - if err := os.MkdirAll(storageDir, 0700); err != nil { - setupLog.Error(err, "unable to create storage directory for catalogs") - } + if err := os.MkdirAll(storageDir, 0700); err != nil { + setupLog.Error(err, "unable to create storage directory for catalogs") } if err = (&corecontrollers.CatalogReconciler{ Client: mgr.GetClient(), Unpacker: unpacker, - Storage: storage.NewStorage(storageDir, serverMux), + Storage: storage.NewStorage(storageDir), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Catalog") os.Exit(1) diff --git a/pkg/catalogserver/server.go b/pkg/catalogserver/server.go index f40c191d..8626a93a 100644 --- a/pkg/catalogserver/server.go +++ b/pkg/catalogserver/server.go @@ -2,10 +2,8 @@ package catalogserver import ( "context" - "fmt" "net/http" "os" - "path/filepath" "time" "golang.org/x/sync/errgroup" @@ -17,24 +15,22 @@ import ( type Instance struct { Dir string Addr string - Mux *http.ServeMux ShutdownTimeout time.Duration } // New returns an Instance of a catalog server that serves // the FBC content stored in the given directory on the given // http address. -func New(dir, addr string, mux *http.ServeMux) Instance { +func New(dir, addr string) Instance { return Instance{ Dir: dir, Addr: addr, - Mux: mux, } } func (s Instance) Start(ctx context.Context) error { server := &http.Server{ - Handler: s.Mux, + Handler: ServerHandler(s.Dir), Addr: s.Addr, ReadHeaderTimeout: 3 * time.Second, } @@ -64,19 +60,7 @@ func (s Instance) Start(ctx context.Context) error { return server.Shutdown(sc) } -func MuxForServer(dir string) *http.ServeMux { - m := &http.ServeMux{} - m.HandleFunc("/catalogs", func(w http.ResponseWriter, r *http.Request) { - files, err := os.ReadDir(dir) - if err != nil { - w.WriteHeader(http.StatusBadRequest) - fmt.Fprintf(w, "error reading catalog store directory: %v", err) - return - } - for _, file := range files { - name := file.Name() - fmt.Fprintf(w, "%v\n", name[:len(name)-len(filepath.Ext(name))]) - } - }) - return m +func ServerHandler(dir string) http.Handler { + fs := http.FileServer(http.FS(os.DirFS(dir))) + return fs } diff --git a/pkg/controllers/core/catalog_controller_test.go b/pkg/controllers/core/catalog_controller_test.go index ee7ecb83..f1aca34c 100644 --- a/pkg/controllers/core/catalog_controller_test.go +++ b/pkg/controllers/core/catalog_controller_test.go @@ -56,6 +56,7 @@ var _ = Describe("Catalogd Controller Test", func() { reconciler *core.CatalogReconciler mockSource *MockSource testServer *httptest.Server + httpclient *http.Client ) BeforeEach(func() { tmpDir, err := os.MkdirTemp(GinkgoT().TempDir(), "cache") @@ -69,9 +70,10 @@ var _ = Describe("Catalogd Controller Test", func() { v1alpha1.SourceTypeImage: mockSource, }, ), - Storage: storage.NewStorage(tmpDir, http.NewServeMux()), + Storage: storage.NewStorage(tmpDir), } - testServer = httptest.NewServer(catalogserver.MuxForServer(tmpDir)) + testServer = httptest.NewServer(catalogserver.ServerHandler(tmpDir)) + httpclient = &http.Client{} }) AfterEach(func() { testServer.Close() @@ -286,6 +288,7 @@ var _ = Describe("Catalogd Controller Test", func() { }) When("HTTPServer feature gate is enabled", func() { + var httpRequest *http.Request BeforeEach(func() { Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{ string(features.PackagesBundleMetadataAPIs): false, @@ -293,38 +296,43 @@ var _ = Describe("Catalogd Controller Test", func() { string(features.HTTPServer): true, })).NotTo(HaveOccurred()) + req, err := http.NewRequest("GET", testServer.URL, nil) + Expect(err).To(Not(HaveOccurred())) + req.Header.Set("Accept", "text/html") + httpRequest = req + // reconcile res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey}) Expect(res).To(Equal(ctrl.Result{})) Expect(err).ToNot(HaveOccurred()) }) - It("the catalog should become available at addr/catalogs", func() { - res, err := http.Get(testServer.URL + "/catalogs") + It("the catalog should become available at server endpoint", func() { + resp, err := httpclient.Do(httpRequest) Expect(err).To(Not(HaveOccurred())) - catalogs, err := io.ReadAll(res.Body) - res.Body.Close() + defer resp.Body.Close() + + catalogs, err := io.ReadAll(resp.Body) Expect(err).To(Not(HaveOccurred())) - //omitting trailing new line char from response - Expect(string(catalogs[:len(catalogs)-1])).To(Equal(catalogKey.Name)) + Expect(string(catalogs)).To(Equal(fmt.Sprintf(httpResponse, fmt.Sprintf("\n%s", catalogKey.Name+".json", catalogKey.Name+".json")))) }) When("the catalog is deleted", func() { BeforeEach(func() { Expect(cl.Delete(ctx, catalog)).Should(Succeed()) }) - It("should not be available at addr/catalogs anymore", func() { + It("should not be available at server endpoint anymore", func() { // reconcile res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey}) Expect(res).To(Equal(ctrl.Result{})) Expect(err).ToNot(HaveOccurred()) - newres, err := http.Get(testServer.URL + "/catalogs") + resp, err := httpclient.Do(httpRequest) Expect(err).To(Not(HaveOccurred())) - catalogs, err := io.ReadAll(newres.Body) - newres.Body.Close() + defer resp.Body.Close() + + catalogs, err := io.ReadAll(resp.Body) Expect(err).To(Not(HaveOccurred())) - //omitting trailing new line char from response - Expect(string(catalogs)).To(Equal("")) + Expect(string(catalogs)).To(Equal(fmt.Sprintf(httpResponse, ""))) }) }) }) @@ -598,3 +606,7 @@ name: %s entries: - name: %s ` + +const httpResponse = `
%s
+
+` diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index bcc5797d..c7533732 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -3,7 +3,6 @@ package storage import ( "errors" "fmt" - "net/http" "os" "path/filepath" @@ -14,13 +13,11 @@ import ( // It can be used to Store or Delete FBC in the host's filesystem type Storage struct { RootDirectory string - ServerMux *http.ServeMux } -func NewStorage(rootDir string, mux *http.ServeMux) Storage { +func NewStorage(rootDir string) Storage { return Storage{ RootDirectory: rootDir, - ServerMux: mux, } } @@ -34,9 +31,6 @@ func (s *Storage) Store(owner string, fbc *declcfg.DeclarativeConfig) error { if err := declcfg.WriteJSON(*fbc, fbcFile); err != nil { return err } - s.ServerMux.HandleFunc(fmt.Sprintf("/catalogs/%s", owner), func(w http.ResponseWriter, r *http.Request) { - http.ServeFile(w, r, filepath.Join(s.RootDirectory, fmt.Sprintf("%s.json", owner))) - }) return nil } From e59cdff0fe72a4913d64e02a1c98def0c85420e1 Mon Sep 17 00:00:00 2001 From: Anik Date: Thu, 17 Aug 2023 15:05:37 -0400 Subject: [PATCH 10/10] minor refractors Signed-off-by: Anik --- cmd/manager/main.go | 4 +-- config/manager/manager.yaml | 2 +- pkg/catalogserver/server.go | 10 -------- pkg/controllers/core/catalog_controller.go | 2 +- .../core/catalog_controller_test.go | 2 +- pkg/storage/storage.go | 25 ++++++------------- 6 files changed, 13 insertions(+), 32 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 7fc807f8..0168ed56 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -122,7 +122,7 @@ func main() { os.Exit(1) } - catalogServer := catalogserver.New(storageDir, catalogServerAddr) + catalogServer := catalogserver.Instance{Dir: storageDir, Addr: catalogServerAddr} if err := mgr.Add(catalogServer); err != nil { setupLog.Error(err, "unable to start catalog server") os.Exit(1) @@ -134,7 +134,7 @@ func main() { if err = (&corecontrollers.CatalogReconciler{ Client: mgr.GetClient(), Unpacker: unpacker, - Storage: storage.NewStorage(storageDir), + Storage: storage.Instance{RootDirectory: storageDir}, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Catalog") os.Exit(1) diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index fb440e40..2a6b67f8 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -57,7 +57,7 @@ spec: name: https volumeMounts: - name: catalog-cache - mountPath: /var/cache/ + mountPath: /var/cache/catalogs securityContext: allowPrivilegeEscalation: false capabilities: diff --git a/pkg/catalogserver/server.go b/pkg/catalogserver/server.go index 8626a93a..3aac9a01 100644 --- a/pkg/catalogserver/server.go +++ b/pkg/catalogserver/server.go @@ -18,16 +18,6 @@ type Instance struct { ShutdownTimeout time.Duration } -// New returns an Instance of a catalog server that serves -// the FBC content stored in the given directory on the given -// http address. -func New(dir, addr string) Instance { - return Instance{ - Dir: dir, - Addr: addr, - } -} - func (s Instance) Start(ctx context.Context) error { server := &http.Server{ Handler: ServerHandler(s.Dir), diff --git a/pkg/controllers/core/catalog_controller.go b/pkg/controllers/core/catalog_controller.go index 0af66824..0ba80215 100644 --- a/pkg/controllers/core/catalog_controller.go +++ b/pkg/controllers/core/catalog_controller.go @@ -53,7 +53,7 @@ const fbcDeletionFinalizer = "catalogd.operatorframework.io/delete-server-cache" type CatalogReconciler struct { client.Client Unpacker source.Unpacker - Storage storage.Storage + Storage storage.Instance } //+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=catalogs,verbs=get;list;watch;create;update;patch;delete diff --git a/pkg/controllers/core/catalog_controller_test.go b/pkg/controllers/core/catalog_controller_test.go index f1aca34c..9864b89b 100644 --- a/pkg/controllers/core/catalog_controller_test.go +++ b/pkg/controllers/core/catalog_controller_test.go @@ -70,7 +70,7 @@ var _ = Describe("Catalogd Controller Test", func() { v1alpha1.SourceTypeImage: mockSource, }, ), - Storage: storage.NewStorage(tmpDir), + Storage: storage.Instance{RootDirectory: tmpDir}, } testServer = httptest.NewServer(catalogserver.ServerHandler(tmpDir)) httpclient = &http.Client{} diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index c7533732..afbdfb24 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -9,32 +9,23 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" ) -// Storage is a store of FBC content of catalogs added to a cluster. -// It can be used to Store or Delete FBC in the host's filesystem -type Storage struct { +// Instance is a storage instance that stores FBC content of catalogs +// added to a cluster. It can be used to Store or Delete FBC in the +// host's filesystem +type Instance struct { RootDirectory string } -func NewStorage(rootDir string) Storage { - return Storage{ - RootDirectory: rootDir, - } -} - -func (s *Storage) Store(owner string, fbc *declcfg.DeclarativeConfig) error { +func (s *Instance) Store(owner string, fbc *declcfg.DeclarativeConfig) error { fbcFile, err := os.Create(s.fbcPath(owner)) if err != nil { return err } defer fbcFile.Close() - - if err := declcfg.WriteJSON(*fbc, fbcFile); err != nil { - return err - } - return nil + return declcfg.WriteJSON(*fbc, fbcFile) } -func (s *Storage) Delete(owner string) error { +func (s *Instance) Delete(owner string) error { err := os.Remove(s.fbcPath(owner)) if errors.Is(err, os.ErrNotExist) { return nil @@ -42,6 +33,6 @@ func (s *Storage) Delete(owner string) error { return err } -func (s *Storage) fbcPath(catalogName string) string { +func (s *Instance) fbcPath(catalogName string) string { return filepath.Join(s.RootDirectory, fmt.Sprintf("%s.json", catalogName)) }