Skip to content

Commit

Permalink
use catalogd HTTP server instead of CatalogMetadata API
Browse files Browse the repository at this point in the history 
Signed-off-by: Bryce Palmer <bpalmer@redhat.com>
  • Loading branch information
@everettraven
everettraven committed Sep 14, 2023
1 parent 3a8f2a0 commit f70e35b
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 198 deletions.
92 changes: 71 additions & 21 deletions internal/catalogmetadata/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package client
import (
"context"
"fmt"
"net/http"

"sigs.k8s.io/controller-runtime/pkg/client"

Expand All @@ -12,15 +13,46 @@ import (
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
)

func NewClient(cl client.Client) *Client {
return &Client{cl: cl}
// TODO: When a catalogd release containing https://github.com/operator-framework/catalogd/pull/168
// is made this can be removed in favor of parsing `Catalog.Status` for the
// appropriate URL for fetching catalog contents.
const catalogdOnClusterBaseURL = "http://catalogd-catalogserver.catalogd-system.svc"
const catalogdOnClusterURLTemplate = "%s/catalogs/%s/all.json"

type Opt func(c *Client)

func WithBaseURL(baseURL string) Opt {
return func(c *Client) {
c.baseURL = baseURL
}
}

func NewClient(cl client.Client, opts ...Opt) *Client {
cli := &Client{
cl: cl,
baseURL: catalogdOnClusterBaseURL,
}

for _, opt := range opts {
opt(cli)
}

return cli
}

// Client is reading catalog metadata
type Client struct {
// Note that eventually we will be reading from catalogd http API
// instead of kube API server. We will need to swap this implementation.
cl client.Client

// baseURL is the base URL used by the client when making a request
// to the catalogd HTTP server. When using NewClient() this defaults
// to http://catalogd-catalogserver.catalogd-system.svc
// TODO: When a catalogd release containing https://github.com/operator-framework/catalogd/pull/168
// is made this can be removed in favor of parsing `Catalog.Status` for the
// appropriate URL for fetching catalog contents.
baseURL string
}

func (c *Client) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) {
Expand All @@ -31,14 +63,47 @@ func (c *Client) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error)
return nil, err
}
for _, catalog := range catalogList.Items {
channels, err := fetchCatalogMetadata[catalogmetadata.Channel](ctx, c.cl, catalog.Name, declcfg.SchemaChannel)
channels := []*catalogmetadata.Channel{}
bundles := []*catalogmetadata.Bundle{}

// TODO: When a catalogd release containing https://github.com/operator-framework/catalogd/pull/168
// is made this should be updated in favor of parsing `Catalog.Status` for the
// appropriate URL for fetching catalog contents.
req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf(catalogdOnClusterURLTemplate, c.baseURL, catalog.Name), nil)
if err != nil {
return nil, err
return nil, fmt.Errorf("error forming request: %s", err)

Check warning on line 74 in internal/catalogmetadata/client/client.go

View check run for this annotation

Codecov / codecov/patch

internal/catalogmetadata/client/client.go#L74 

Added line #L74 was not covered by tests
}

bundles, err := fetchCatalogMetadata[catalogmetadata.Bundle](ctx, c.cl, catalog.Name, declcfg.SchemaBundle)
resp, err := http.DefaultClient.Do(req)
if err != nil {
return nil, err
return nil, fmt.Errorf("error performing request: %s", err)

Check warning on line 79 in internal/catalogmetadata/client/client.go

View check run for this annotation

Codecov / codecov/patch

internal/catalogmetadata/client/client.go#L79 

Added line #L79 was not covered by tests
}
defer resp.Body.Close()

err = declcfg.WalkMetasReader(resp.Body, func(meta *declcfg.Meta, err error) error {
if err != nil {
return err
}

switch meta.Schema {
case declcfg.SchemaChannel:
content, err := catalogmetadata.Unmarshal[catalogmetadata.Channel](meta)
if err != nil {
return fmt.Errorf("error unmarshalling catalog metadata: %s", err)

Check warning on line 92 in internal/catalogmetadata/client/client.go

View check run for this annotation

Codecov / codecov/patch

internal/catalogmetadata/client/client.go#L92 

Added line #L92 was not covered by tests
}
channels = append(channels, content)
case declcfg.SchemaBundle:
content, err := catalogmetadata.Unmarshal[catalogmetadata.Bundle](meta)
if err != nil {
return fmt.Errorf("error unmarshalling catalog metadata: %s", err)

Check warning on line 98 in internal/catalogmetadata/client/client.go

View check run for this annotation

Codecov / codecov/patch

internal/catalogmetadata/client/client.go#L98 

Added line #L98 was not covered by tests
}
bundles = append(bundles, content)
}

return nil
})
if err != nil {
return nil, fmt.Errorf("error processing response: %s", err)
}

bundles, err = populateExtraFields(catalog.Name, channels, bundles)
Expand All @@ -52,21 +117,6 @@ func (c *Client) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error)
return allBundles, nil
}

func fetchCatalogMetadata[T catalogmetadata.Schemas](ctx context.Context, cl client.Client, catalogName, schema string) ([]*T, error) {
var cmList catalogd.CatalogMetadataList
err := cl.List(ctx, &cmList, client.MatchingLabels{"catalog": catalogName, "schema": schema})
if err != nil {
return nil, err
}

content, err := catalogmetadata.Unmarshal[T](cmList.Items)
if err != nil {
return nil, fmt.Errorf("error unmarshalling catalog metadata: %s", err)
}

return content, nil
}

func populateExtraFields(catalogName string, channels []*catalogmetadata.Channel, bundles []*catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
bundlesMap := map[string]*catalogmetadata.Bundle{}
for i := range bundles {
Expand Down
150 changes: 41 additions & 109 deletions internal/catalogmetadata/client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ package client_test
import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -33,7 +37,7 @@ func TestClient(t *testing.T) {
t.Run("Bundles", func(t *testing.T) {
for _, tt := range []struct {
name string
fakeCatalog func() ([]client.Object, []*catalogmetadata.Bundle)
fakeCatalog func() ([]client.Object, []*catalogmetadata.Bundle, map[string][]byte)
wantErr string
}{
{
Expand All @@ -42,16 +46,10 @@ func TestClient(t *testing.T) {
},
{
name: "channel has a ref to a missing bundle",
fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle) {
objs, _ := defaultFakeCatalog()
fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string][]byte) {
objs, _, catalogContentMap := defaultFakeCatalog()

objs = append(objs, &catalogd.CatalogMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: "catalog-1-fake1-channel-with-missing-bundle",
Labels: map[string]string{"schema": declcfg.SchemaChannel, "catalog": "catalog-1"},
},
Spec: catalogd.CatalogMetadataSpec{
Content: json.RawMessage(`{
catalogContentMap["catalog-1"] = append(catalogContentMap["catalog-1"], []byte(`{
"schema": "olm.channel",
"name": "channel-with-missing-bundle",
"package": "fake1",
Expand All @@ -60,59 +58,51 @@ func TestClient(t *testing.T) {
"name": "fake1.v9.9.9"
}
]
}`),
},
})
}`)...)

return objs, nil
return objs, nil, catalogContentMap
},
wantErr: `bundle "fake1.v9.9.9" not found in catalog "catalog-1" (package "fake1", channel "channel-with-missing-bundle")`,
},
{
name: "invalid bundle",
fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle) {
objs, _ := defaultFakeCatalog()
fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string][]byte) {
objs, _, catalogContentMap := defaultFakeCatalog()

objs = append(objs, &catalogd.CatalogMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: "catalog-1-broken-bundle",
Labels: map[string]string{"schema": declcfg.SchemaBundle, "catalog": "catalog-1"},
},
Spec: catalogd.CatalogMetadataSpec{
Content: json.RawMessage(`{"name":123123123}`),
},
})
catalogContentMap["catalog-1"] = append(catalogContentMap["catalog-1"], []byte(`{"schema": "olm.bundle", "name":123123123}`)...)

return objs, nil
return objs, nil, catalogContentMap
},
wantErr: "error unmarshalling catalog metadata: json: cannot unmarshal number into Go struct field Bundle.name of type string",
wantErr: "error processing response: expected value for key \"name\" to be a string, got %!t(float64=1.23123123e+08): 1.23123123e+08",
},
{
name: "invalid channel",
fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle) {
objs, _ := defaultFakeCatalog()
fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string][]byte) {
objs, _, catalogContentMap := defaultFakeCatalog()

objs = append(objs, &catalogd.CatalogMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: "catalog-1-fake1-broken-channel",
Labels: map[string]string{"schema": declcfg.SchemaChannel, "catalog": "catalog-1"},
},
Spec: catalogd.CatalogMetadataSpec{
Content: json.RawMessage(`{"name":123123123}`),
},
})
catalogContentMap["catalog-1"] = append(catalogContentMap["catalog-1"], []byte(`{"schema": "olm.channel", "name":123123123}`)...)

return objs, nil
return objs, nil, catalogContentMap
},
wantErr: "error unmarshalling catalog metadata: json: cannot unmarshal number into Go struct field Channel.name of type string",
wantErr: "error processing response: expected value for key \"name\" to be a string, got %!t(float64=1.23123123e+08): 1.23123123e+08",
},
} {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
objs, expectedBundles := tt.fakeCatalog()
objs, expectedBundles, catalogContentMap := tt.fakeCatalog()

mux := http.NewServeMux()
for k, v := range catalogContentMap {
content := v
mux.HandleFunc(fmt.Sprintf("/catalogs/%s/all.json", k), func(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write(content)
})
}
srv := httptest.NewServer(mux)

fakeCatalogClient := catalogClient.NewClient(
fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build(),
catalogClient.WithBaseURL(srv.URL),
)

bundles, err := fakeCatalogClient.Bundles(ctx)
Expand All @@ -127,9 +117,9 @@ func TestClient(t *testing.T) {
})
}

func defaultFakeCatalog() ([]client.Object, []*catalogmetadata.Bundle) {
func defaultFakeCatalog() ([]client.Object, []*catalogmetadata.Bundle, map[string][]byte) {
package1 := `{
"schema": "olm.bundle",
"schema": "olm.package",
"name": "fake1"
}`

Expand Down Expand Up @@ -179,69 +169,6 @@ func defaultFakeCatalog() ([]client.Object, []*catalogmetadata.Bundle) {
Name: "catalog-2",
},
},
&catalogd.CatalogMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: "catalog-1-fake1",
Labels: map[string]string{"schema": declcfg.SchemaPackage, "catalog": "catalog-1"},
},
Spec: catalogd.CatalogMetadataSpec{
Content: json.RawMessage(package1),
},
},
&catalogd.CatalogMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: "catalog-1-fake1-channel-stable",
Labels: map[string]string{"schema": declcfg.SchemaChannel, "catalog": "catalog-1"},
},
Spec: catalogd.CatalogMetadataSpec{
Content: json.RawMessage(stableChannel),
},
},
&catalogd.CatalogMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: "catalog-1-fake1-channel-beta",
Labels: map[string]string{"schema": declcfg.SchemaChannel, "catalog": "catalog-1"},
},
Spec: catalogd.CatalogMetadataSpec{
Content: json.RawMessage(betaChannel),
},
},
&catalogd.CatalogMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: "catalog-1-fake1-bundle-1",
Labels: map[string]string{"schema": declcfg.SchemaBundle, "catalog": "catalog-1"},
},
Spec: catalogd.CatalogMetadataSpec{
Content: json.RawMessage(bundle1),
},
},
&catalogd.CatalogMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: "catalog-2-fake1",
Labels: map[string]string{"schema": declcfg.SchemaPackage, "catalog": "catalog-2"},
},
Spec: catalogd.CatalogMetadataSpec{
Content: json.RawMessage(package1),
},
},
&catalogd.CatalogMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: "catalog-2-fake1-channel-stable",
Labels: map[string]string{"schema": declcfg.SchemaChannel, "catalog": "catalog-2"},
},
Spec: catalogd.CatalogMetadataSpec{
Content: json.RawMessage(stableChannel),
},
},
&catalogd.CatalogMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: "catalog-2-fake1-bundle-1",
Labels: map[string]string{"schema": declcfg.SchemaBundle, "catalog": "catalog-2"},
},
Spec: catalogd.CatalogMetadataSpec{
Content: json.RawMessage(bundle1),
},
},
}

expectedBundles := []*catalogmetadata.Bundle{
Expand All @@ -263,7 +190,7 @@ func defaultFakeCatalog() ([]client.Object, []*catalogmetadata.Bundle) {
{
Channel: declcfg.Channel{
Schema: declcfg.SchemaChannel,
Name: "beta",
Name: "stable",
Package: "fake1",
Entries: []declcfg.ChannelEntry{
{
Expand All @@ -275,7 +202,7 @@ func defaultFakeCatalog() ([]client.Object, []*catalogmetadata.Bundle) {
{
Channel: declcfg.Channel{
Schema: declcfg.SchemaChannel,
Name: "stable",
Name: "beta",
Package: "fake1",
Entries: []declcfg.ChannelEntry{
{
Expand Down Expand Up @@ -317,5 +244,10 @@ func defaultFakeCatalog() ([]client.Object, []*catalogmetadata.Bundle) {
},
}

return objs, expectedBundles
catalogContents := map[string][]byte{
"catalog-1": []byte(strings.Join([]string{package1, bundle1, stableChannel, betaChannel}, "\n")),
"catalog-2": []byte(strings.Join([]string{package1, bundle1, stableChannel}, "\n")),
}

return objs, expectedBundles, catalogContents
}
14 changes: 6 additions & 8 deletions internal/catalogmetadata/unmarshal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,16 @@ package catalogmetadata
import (
"encoding/json"

catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
"github.com/operator-framework/operator-registry/alpha/declcfg"
)

func Unmarshal[T Schemas](cm []catalogd.CatalogMetadata) ([]*T, error) {
contents := make([]*T, 0, len(cm))
for _, cm := range cm {
var content T
if err := json.Unmarshal(cm.Spec.Content, &content); err != nil {
func Unmarshal[T Schemas](meta *declcfg.Meta) (*T, error) {
var content T
if meta != nil {
if err := json.Unmarshal(meta.Blob, &content); err != nil {
return nil, err
}
contents = append(contents, &content)
}

return contents, nil
return &content, nil
}
Loading

0 comments on commit f70e35b

Please sign in to comment.