From 6f9382a594815dbeb13daf8e5b0329718fa0ef27 Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Tue, 25 Apr 2023 13:01:22 -0500 Subject: [PATCH] refactor to move more processing to the API (#1082) * refactor to move more processing to the template Signed-off-by: Jordan Keister * better diagnostics on custom exec error Signed-off-by: Jordan Keister * Remove obsolete fields Signed-off-by: Catherine Chan-Tse Signed-off-by: Jordan Keister * caching wip Signed-off-by: Jordan Keister * refactor a bit and update unit tests (#2) * refactor a bit and update unit tests Signed-off-by: Bryce Palmer * add more utests and minor tweaks Signed-off-by: Bryce Palmer --------- Signed-off-by: Bryce Palmer Signed-off-by: Jordan Keister * whitespace sanity hell Signed-off-by: Jordan Keister * drop STDERR for nominal case Signed-off-by: Jordan Keister --------- Signed-off-by: Jordan Keister Signed-off-by: Catherine Chan-Tse Signed-off-by: Bryce Palmer Co-authored-by: Catherine Chan-Tse Co-authored-by: Bryce Palmer --- .../alpha/template/composite/builder.go | 36 +- .../alpha/template/composite/builder_test.go | 192 +---- .../alpha/template/composite/composite.go | 226 +++++- .../template/composite/composite_test.go | 675 ++++++++++++++---- .../cmd/opm/alpha/template/composite.go | 165 +---- 5 files changed, 805 insertions(+), 489 deletions(-) diff --git a/staging/operator-registry/alpha/template/composite/builder.go b/staging/operator-registry/alpha/template/composite/builder.go index 3d1f7af95d..c62a82afcf 100644 --- a/staging/operator-registry/alpha/template/composite/builder.go +++ b/staging/operator-registry/alpha/template/composite/builder.go @@ -27,16 +27,9 @@ const ( CustomBuilderSchema = "olm.builder.custom" ) -type ContainerConfig struct { - ContainerTool string - BaseImage string - WorkingDir string -} - type BuilderConfig struct { - ContainerCfg ContainerConfig - OutputType string - InputDirectory string + WorkingDir string + OutputType string } type Builder interface { @@ -96,13 +89,13 @@ func (bb *BasicBuilder) Build(ctx context.Context, reg image.Registry, dir strin return fmt.Errorf("error rendering basic template: %v", err) } - destPath := path.Join(bb.builderCfg.ContainerCfg.WorkingDir, dir, basicConfig.Output) + destPath := path.Join(bb.builderCfg.WorkingDir, dir, basicConfig.Output) return build(dcfg, destPath, bb.builderCfg.OutputType) } func (bb *BasicBuilder) Validate(dir string) error { - return validate(bb.builderCfg.ContainerCfg, dir) + return validate(bb.builderCfg, dir) } type SemverBuilder struct { @@ -158,13 +151,13 @@ func (sb *SemverBuilder) Build(ctx context.Context, reg image.Registry, dir stri return fmt.Errorf("error rendering semver template: %v", err) } - destPath := path.Join(sb.builderCfg.ContainerCfg.WorkingDir, dir, semverConfig.Output) + destPath := path.Join(sb.builderCfg.WorkingDir, dir, semverConfig.Output) return build(dcfg, destPath, sb.builderCfg.OutputType) } func (sb *SemverBuilder) Validate(dir string) error { - return validate(sb.builderCfg.ContainerCfg, dir) + return validate(sb.builderCfg, dir) } type RawBuilder struct { @@ -218,13 +211,13 @@ func (rb *RawBuilder) Build(ctx context.Context, _ image.Registry, dir string, t return fmt.Errorf("error parsing raw input file: %s, %v", rawConfig.Input, err) } - destPath := path.Join(rb.builderCfg.ContainerCfg.WorkingDir, dir, rawConfig.Output) + destPath := path.Join(rb.builderCfg.WorkingDir, dir, rawConfig.Output) return build(dcfg, destPath, rb.builderCfg.OutputType) } func (rb *RawBuilder) Validate(dir string) error { - return validate(rb.builderCfg.ContainerCfg, dir) + return validate(rb.builderCfg, dir) } type CustomBuilder struct { @@ -268,13 +261,12 @@ func (cb *CustomBuilder) Build(ctx context.Context, reg image.Registry, dir stri } // build the command to execute cmd := exec.Command(customConfig.Command, customConfig.Args...) - cmd.Dir = cb.builderCfg.ContainerCfg.WorkingDir // custom template should output a valid FBC to STDOUT so we can // build the FBC just like all the other templates. v, err := cmd.Output() if err != nil { - return fmt.Errorf("running command %q: %v", cmd.String(), err) + return fmt.Errorf("running command %q: %v: %v", cmd.String(), err, v) } reader := bytes.NewReader(v) @@ -286,7 +278,7 @@ func (cb *CustomBuilder) Build(ctx context.Context, reg image.Registry, dir stri return fmt.Errorf("error parsing custom command output: %s, %v", strings.Join(cmdString, "'"), err) } - destPath := path.Join(cb.builderCfg.ContainerCfg.WorkingDir, dir, customConfig.Output) + destPath := path.Join(cb.builderCfg.WorkingDir, dir, customConfig.Output) // custom template should output a valid FBC to STDOUT so we can // build the FBC just like all the other templates. @@ -294,7 +286,7 @@ func (cb *CustomBuilder) Build(ctx context.Context, reg image.Registry, dir stri } func (cb *CustomBuilder) Validate(dir string) error { - return validate(cb.builderCfg.ContainerCfg, dir) + return validate(cb.builderCfg, dir) } func writeDeclCfg(dcfg declcfg.DeclarativeConfig, w io.Writer, output string) error { @@ -308,12 +300,12 @@ func writeDeclCfg(dcfg declcfg.DeclarativeConfig, w io.Writer, output string) er } } -func validate(containerCfg ContainerConfig, dir string) error { +func validate(builderCfg BuilderConfig, dir string) error { - path := path.Join(containerCfg.WorkingDir, dir) + path := path.Join(builderCfg.WorkingDir, dir) s, err := os.Stat(path) if err != nil { - return fmt.Errorf("directory not found. validation path needs to be composed of ContainerConfig.WorkingDir+Component[].Destination.Path: %q: %v", path, err) + return fmt.Errorf("directory not found. validation path needs to be composed of BuilderConfig.WorkingDir+Component[].Destination.Path: %q: %v", path, err) } if !s.IsDir() { return fmt.Errorf("%q is not a directory", path) diff --git a/staging/operator-registry/alpha/template/composite/builder_test.go b/staging/operator-registry/alpha/template/composite/builder_test.go index af7850b076..a011d47023 100644 --- a/staging/operator-registry/alpha/template/composite/builder_test.go +++ b/staging/operator-registry/alpha/template/composite/builder_test.go @@ -36,11 +36,7 @@ func TestBasicBuilder(t *testing.T) { name: "successful basic build yaml output", validate: true, basicBuilder: NewBasicBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -71,11 +67,7 @@ func TestBasicBuilder(t *testing.T) { name: "successful basic build json output", validate: true, basicBuilder: NewBasicBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "json", }), templateDefinition: TemplateDefinition{ @@ -106,11 +98,7 @@ func TestBasicBuilder(t *testing.T) { name: "invalid template configuration", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -129,11 +117,7 @@ func TestBasicBuilder(t *testing.T) { name: "invalid output type", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "invalid", }), templateDefinition: TemplateDefinition{ @@ -152,11 +136,7 @@ func TestBasicBuilder(t *testing.T) { name: "invalid schema", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -172,11 +152,7 @@ func TestBasicBuilder(t *testing.T) { name: "template config has empty input", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -197,11 +173,7 @@ func TestBasicBuilder(t *testing.T) { name: "template config has empty output", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -222,11 +194,7 @@ func TestBasicBuilder(t *testing.T) { name: "template config has empty input & output", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -244,7 +212,6 @@ func TestBasicBuilder(t *testing.T) { } for i, tc := range testCases { - tc.basicBuilder.builderCfg.InputDirectory = testDir t.Run(tc.name, func(t *testing.T) { outDir := fmt.Sprintf("basic-%d", i) outPath := path.Join(testDir, outDir) @@ -447,11 +414,7 @@ func TestSemverBuilder(t *testing.T) { name: "successful semver build yaml output", validate: true, semverBuilder: NewSemverBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -482,11 +445,7 @@ func TestSemverBuilder(t *testing.T) { name: "successful semver build json output", validate: true, semverBuilder: NewSemverBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "json", }), templateDefinition: TemplateDefinition{ @@ -517,11 +476,7 @@ func TestSemverBuilder(t *testing.T) { name: "invalid template configuration", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -540,11 +495,7 @@ func TestSemverBuilder(t *testing.T) { name: "invalid output type", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "invalid", }), templateDefinition: TemplateDefinition{ @@ -563,11 +514,7 @@ func TestSemverBuilder(t *testing.T) { name: "invalid schema", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -584,11 +531,7 @@ func TestSemverBuilder(t *testing.T) { name: "template config has empty input", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -609,11 +552,7 @@ func TestSemverBuilder(t *testing.T) { name: "template config has empty output", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -634,11 +573,7 @@ func TestSemverBuilder(t *testing.T) { name: "template config has empty input & output", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -656,7 +591,6 @@ func TestSemverBuilder(t *testing.T) { } for i, tc := range testCases { - tc.semverBuilder.builderCfg.InputDirectory = testDir t.Run(tc.name, func(t *testing.T) { outDir := fmt.Sprintf("semver-%d", i) outPath := path.Join(testDir, outDir) @@ -869,11 +803,7 @@ func TestRawBuilder(t *testing.T) { name: "successful raw build yaml output", validate: true, rawBuilder: NewRawBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -904,11 +834,7 @@ func TestRawBuilder(t *testing.T) { name: "successful raw build json output", validate: true, rawBuilder: NewRawBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "json", }), templateDefinition: TemplateDefinition{ @@ -939,11 +865,7 @@ func TestRawBuilder(t *testing.T) { name: "invalid template configuration", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -962,11 +884,7 @@ func TestRawBuilder(t *testing.T) { name: "invalid output type", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "invalid", }), templateDefinition: TemplateDefinition{ @@ -985,11 +903,7 @@ func TestRawBuilder(t *testing.T) { name: "invalid schema", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1005,11 +919,7 @@ func TestRawBuilder(t *testing.T) { name: "template config has empty input", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1030,11 +940,7 @@ func TestRawBuilder(t *testing.T) { name: "template config has empty output", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1055,11 +961,7 @@ func TestRawBuilder(t *testing.T) { name: "template config has empty input & output", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1077,7 +979,6 @@ func TestRawBuilder(t *testing.T) { } for i, tc := range testCases { - tc.rawBuilder.builderCfg.InputDirectory = testDir t.Run(tc.name, func(t *testing.T) { outDir := fmt.Sprintf("raw-%d", i) outPath := path.Join(testDir, outDir) @@ -1313,11 +1214,7 @@ func TestCustomBuilder(t *testing.T) { name: "successful custom build yaml output", validate: true, customBuilder: NewCustomBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1348,11 +1245,7 @@ func TestCustomBuilder(t *testing.T) { name: "successful custom build json output", validate: true, customBuilder: NewCustomBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "json", }), templateDefinition: TemplateDefinition{ @@ -1383,11 +1276,7 @@ func TestCustomBuilder(t *testing.T) { name: "invalid template configuration", validate: false, customBuilder: NewCustomBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1406,11 +1295,7 @@ func TestCustomBuilder(t *testing.T) { name: "invalid schema", validate: false, customBuilder: NewCustomBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1430,11 +1315,7 @@ func TestCustomBuilder(t *testing.T) { name: "template config has empty command", validate: false, customBuilder: NewCustomBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1456,11 +1337,7 @@ func TestCustomBuilder(t *testing.T) { name: "template config has empty output", validate: false, customBuilder: NewCustomBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1482,11 +1359,7 @@ func TestCustomBuilder(t *testing.T) { name: "template config has empty command & output", validate: false, customBuilder: NewCustomBuilder(BuilderConfig{ - ContainerCfg: ContainerConfig{ - ContainerTool: "docker", - BaseImage: "quay.io/operator-framework/opm:latest", - WorkingDir: testDir, - }, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1505,7 +1378,6 @@ func TestCustomBuilder(t *testing.T) { } for i, tc := range testCases { - tc.customBuilder.builderCfg.InputDirectory = testDir t.Run(tc.name, func(t *testing.T) { outDir := fmt.Sprintf("custom-%d", i) outPath := path.Join(testDir, outDir) @@ -1719,7 +1591,7 @@ const customBuiltFbcJson = `{ ` func TestValidateFailure(t *testing.T) { - err := validate(ContainerConfig{}, "") + err := validate(BuilderConfig{}, "") require.Error(t, err) require.Contains(t, err.Error(), "no such file or directory") } diff --git a/staging/operator-registry/alpha/template/composite/composite.go b/staging/operator-registry/alpha/template/composite/composite.go index 422017c974..fbd5e37c6e 100644 --- a/staging/operator-registry/alpha/template/composite/composite.go +++ b/staging/operator-registry/alpha/template/composite/composite.go @@ -2,28 +2,132 @@ package composite import ( "context" + "encoding/json" "fmt" + "io" + "net/http" + "net/url" + "os" "github.com/operator-framework/operator-registry/pkg/image" + "k8s.io/apimachinery/pkg/util/yaml" ) type BuilderMap map[string]Builder type CatalogBuilderMap map[string]BuilderMap +type builderFunc func(BuilderConfig) Builder + type Template struct { - CatalogBuilders CatalogBuilderMap - Registry image.Registry + catalogFile io.Reader + contributionFile io.Reader + validate bool + outputType string + registry image.Registry + registeredBuilders map[string]builderFunc +} + +type TemplateOption func(t *Template) + +func WithCatalogFile(catalogFile io.Reader) TemplateOption { + return func(t *Template) { + t.catalogFile = catalogFile + } +} + +func WithContributionFile(contribFile io.Reader) TemplateOption { + return func(t *Template) { + t.contributionFile = contribFile + } +} + +func WithOutputType(outputType string) TemplateOption { + return func(t *Template) { + t.outputType = outputType + } +} + +func WithRegistry(reg image.Registry) TemplateOption { + return func(t *Template) { + t.registry = reg + } +} + +func WithValidate(validate bool) TemplateOption { + return func(t *Template) { + t.validate = validate + } +} + +func NewTemplate(opts ...TemplateOption) *Template { + temp := &Template{ + // Default registered builders when creating a new Template + registeredBuilders: map[string]builderFunc{ + BasicBuilderSchema: func(bc BuilderConfig) Builder { return NewBasicBuilder(bc) }, + SemverBuilderSchema: func(bc BuilderConfig) Builder { return NewSemverBuilder(bc) }, + RawBuilderSchema: func(bc BuilderConfig) Builder { return NewRawBuilder(bc) }, + CustomBuilderSchema: func(bc BuilderConfig) Builder { return NewCustomBuilder(bc) }, + }, + } + + for _, opt := range opts { + opt(temp) + } + + return temp +} + +type HttpGetter interface { + Get(url string) (*http.Response, error) +} + +// FetchCatalogConfig will fetch the catalog configuration file from the given path. +// The path can be a local file path OR a URL that returns the raw contents of the catalog +// configuration file. +func FetchCatalogConfig(path string, httpGetter HttpGetter) (io.ReadCloser, error) { + var tempCatalog io.ReadCloser + catalogURI, err := url.ParseRequestURI(path) + if err != nil { + tempCatalog, err = os.Open(path) + if err != nil { + return nil, fmt.Errorf("opening catalog config file %q: %v", path, err) + } + } else { + tempResp, err := httpGetter.Get(catalogURI.String()) + if err != nil { + return nil, fmt.Errorf("fetching remote catalog config file %q: %v", path, err) + } + tempCatalog = tempResp.Body + } + + return tempCatalog, nil } // TODO(everettraven): do we need the context here? If so, how should it be used? -func (t *Template) Render(ctx context.Context, config *CompositeConfig, validate bool) error { +func (t *Template) Render(ctx context.Context, validate bool) error { + + catalogFile, err := t.parseCatalogsSpec() + if err != nil { + return err + } + + contributionFile, err := t.parseContributionSpec() + if err != nil { + return err + } + + catalogBuilderMap, err := t.newCatalogBuilderMap(catalogFile.Catalogs, t.outputType) + if err != nil { + return err + } + // TODO(everettraven): should we return aggregated errors? - for _, component := range config.Components { - if builderMap, ok := t.CatalogBuilders[component.Name]; ok { + for _, component := range contributionFile.Components { + if builderMap, ok := (*catalogBuilderMap)[component.Name]; ok { if builder, ok := builderMap[component.Strategy.Template.Schema]; ok { // run the builder corresponding to the schema - err := builder.Build(ctx, t.Registry, component.Destination.Path, component.Strategy.Template) + err := builder.Build(ctx, t.registry, component.Destination.Path, component.Strategy.Template) if err != nil { return fmt.Errorf("building component %q: %w", component.Name, err) } @@ -40,7 +144,7 @@ func (t *Template) Render(ctx context.Context, config *CompositeConfig, validate } } else { allowedComponents := []string{} - for k := range t.CatalogBuilders { + for k := range *catalogBuilderMap { allowedComponents = append(allowedComponents, k) } return fmt.Errorf("building component %q: component does not exist in the catalog configuration. Available components are: %s", component.Name, allowedComponents) @@ -48,3 +152,111 @@ func (t *Template) Render(ctx context.Context, config *CompositeConfig, validate } return nil } + +func (t *Template) builderForSchema(schema string, builderCfg BuilderConfig) (Builder, error) { + builderFunc, ok := t.registeredBuilders[schema] + if !ok { + return nil, fmt.Errorf("unknown schema %q", schema) + } + + return builderFunc(builderCfg), nil +} + +func (t *Template) parseCatalogsSpec() (*CatalogConfig, error) { + + // get catalog configurations + catalogConfig := &CatalogConfig{} + catalogDoc := json.RawMessage{} + catalogDecoder := yaml.NewYAMLOrJSONDecoder(t.catalogFile, 4096) + err := catalogDecoder.Decode(&catalogDoc) + if err != nil { + return nil, fmt.Errorf("decoding catalog config: %v", err) + } + err = json.Unmarshal(catalogDoc, catalogConfig) + if err != nil { + return nil, fmt.Errorf("unmarshalling catalog config: %v", err) + } + + if catalogConfig.Schema != CatalogSchema { + return nil, fmt.Errorf("catalog configuration file has unknown schema, should be %q", CatalogSchema) + } + + return catalogConfig, nil +} + +func (t *Template) parseContributionSpec() (*CompositeConfig, error) { + + // parse data to composite config + compositeConfig := &CompositeConfig{} + compositeDoc := json.RawMessage{} + compositeDecoder := yaml.NewYAMLOrJSONDecoder(t.contributionFile, 4096) + err := compositeDecoder.Decode(&compositeDoc) + if err != nil { + return nil, fmt.Errorf("decoding composite config: %v", err) + } + err = json.Unmarshal(compositeDoc, compositeConfig) + if err != nil { + return nil, fmt.Errorf("unmarshalling composite config: %v", err) + } + + if compositeConfig.Schema != CompositeSchema { + return nil, fmt.Errorf("composite configuration file has unknown schema, should be %q", CompositeSchema) + } + + return compositeConfig, nil +} + +func (t *Template) newCatalogBuilderMap(catalogs []Catalog, outputType string) (*CatalogBuilderMap, error) { + + catalogBuilderMap := make(CatalogBuilderMap) + + // setup the builders for each catalog + setupFailed := false + setupErrors := map[string][]string{} + for _, catalog := range catalogs { + errs := []string{} + if catalog.Destination.BaseImage == "" { + errs = append(errs, "destination.baseImage must not be an empty string") + } + + if catalog.Destination.WorkingDir == "" { + errs = append(errs, "destination.workingDir must not be an empty string") + } + + // check for validation errors and skip builder creation if there are any errors + if len(errs) > 0 { + setupFailed = true + setupErrors[catalog.Name] = errs + continue + } + + if _, ok := catalogBuilderMap[catalog.Name]; !ok { + builderMap := make(BuilderMap) + for _, schema := range catalog.Builders { + builder, err := t.builderForSchema(schema, BuilderConfig{ + OutputType: outputType, + }) + if err != nil { + return nil, fmt.Errorf("getting builder %q for catalog %q: %v", schema, catalog.Name, err) + } + builderMap[schema] = builder + } + catalogBuilderMap[catalog.Name] = builderMap + } + } + + // if there were errors validating the catalog configuration then exit + if setupFailed { + //build the error message + var errMsg string + for cat, errs := range setupErrors { + errMsg += fmt.Sprintf("\nCatalog %v:\n", cat) + for _, err := range errs { + errMsg += fmt.Sprintf(" - %v\n", err) + } + } + return nil, fmt.Errorf("catalog configuration file field validation failed: %s", errMsg) + } + + return &catalogBuilderMap, nil +} diff --git a/staging/operator-registry/alpha/template/composite/composite_test.go b/staging/operator-registry/alpha/template/composite/composite_test.go index c2b28fac7b..3e4618516e 100644 --- a/staging/operator-registry/alpha/template/composite/composite_test.go +++ b/staging/operator-registry/alpha/template/composite/composite_test.go @@ -2,44 +2,101 @@ package composite import ( "context" - "encoding/json" - "errors" "fmt" + "io" + "net/http" + "os" + "path" + "strings" "testing" "github.com/operator-framework/operator-registry/pkg/image" "github.com/stretchr/testify/require" ) -type TestBuilder struct { - buildError bool - validateError bool -} +var _ Builder = &TestBuilder{} -const buildErr = "TestBuilder Build() error" -const validateErr = "TestBuilder Validate() error" +var TestBuilderSchema = "olm.builder.test" -var _ Builder = &TestBuilder{} +type TestBuilder struct { + buildShouldError bool + validateShouldError bool +} -func (tb *TestBuilder) Build(ctx context.Context, reg image.Registry, dir string, vd TemplateDefinition) error { - if tb.buildError { - return errors.New(buildErr) +func (tb *TestBuilder) Build(ctx context.Context, reg image.Registry, dir string, td TemplateDefinition) error { + if tb.buildShouldError { + return fmt.Errorf("build error!") } return nil } func (tb *TestBuilder) Validate(dir string) error { - if tb.validateError { - return errors.New(validateErr) + if tb.validateShouldError { + return fmt.Errorf("validate error!") } return nil } +var renderValidCatalog = ` +schema: olm.composite.catalogs +catalogs: + - name: first-catalog + destination: + baseImage: quay.io/operator-framework/opm:latest + workingDir: contributions/first-catalog + builders: + - olm.builder.test +` + +var renderValidComposite = ` +schema: olm.composite +components: + - name: first-catalog + destination: + path: my-operator + strategy: + name: test + template: + schema: olm.builder.test + config: + input: components/contribution1.yaml + output: catalog.yaml +` + +var renderInvalidComponentComposite = ` +schema: olm.composite +components: + - name: missing-catalog + destination: + path: my-operator + strategy: + name: test + template: + schema: olm.builder.test + config: + input: components/contribution1.yaml + output: catalog.yaml +` + +var renderInvalidBuilderComposite = ` +schema: olm.composite +components: + - name: first-catalog + destination: + path: my-operator + strategy: + name: test + template: + schema: olm.builder.invalid + config: + input: components/contribution1.yaml + output: catalog.yaml +` + func TestCompositeRender(t *testing.T) { type testCase struct { name string compositeTemplate Template - compositeCfg CompositeConfig validate bool assertions func(t *testing.T, err error) } @@ -49,28 +106,10 @@ func TestCompositeRender(t *testing.T) { name: "successful render", validate: true, compositeTemplate: Template{ - CatalogBuilders: CatalogBuilderMap{ - "testcatalog": BuilderMap{ - "olm.builder.test": &TestBuilder{}, - }, - }, - }, - compositeCfg: CompositeConfig{ - Schema: CompositeSchema, - Components: []Component{ - { - Name: "testcatalog", - Destination: ComponentDestination{ - Path: "testcatalog/mypackage", - }, - Strategy: BuildStrategy{ - Name: "testbuild", - Template: TemplateDefinition{ - Schema: "olm.builder.test", - Config: json.RawMessage{}, - }, - }, - }, + catalogFile: strings.NewReader(renderValidCatalog), + contributionFile: strings.NewReader(renderValidComposite), + registeredBuilders: map[string]builderFunc{ + TestBuilderSchema: func(bc BuilderConfig) Builder { return &TestBuilder{} }, }, }, assertions: func(t *testing.T, err error) { @@ -78,181 +117,503 @@ func TestCompositeRender(t *testing.T) { }, }, { - name: "component not in catalog config", + name: "Component build failure", validate: true, compositeTemplate: Template{ - CatalogBuilders: CatalogBuilderMap{ - "testcatalog": BuilderMap{ - "olm.builder.test": &TestBuilder{}, - }, + catalogFile: strings.NewReader(renderValidCatalog), + contributionFile: strings.NewReader(renderValidComposite), + registeredBuilders: map[string]builderFunc{ + TestBuilderSchema: func(bc BuilderConfig) Builder { return &TestBuilder{buildShouldError: true} }, }, }, - compositeCfg: CompositeConfig{ - Schema: CompositeSchema, - Components: []Component{ - { - Name: "invalid", - Destination: ComponentDestination{ - Path: "testcatalog/mypackage", - }, - Strategy: BuildStrategy{ - Name: "testbuild", - Template: TemplateDefinition{ - Schema: "olm.builder.test", - Config: json.RawMessage{}, - }, - }, - }, + assertions: func(t *testing.T, err error) { + require.Error(t, err) + require.Equal(t, "building component \"first-catalog\": build error!", err.Error()) + }, + }, + { + name: "Component validate failure", + validate: true, + compositeTemplate: Template{ + catalogFile: strings.NewReader(renderValidCatalog), + contributionFile: strings.NewReader(renderValidComposite), + registeredBuilders: map[string]builderFunc{ + TestBuilderSchema: func(bc BuilderConfig) Builder { return &TestBuilder{validateShouldError: true} }, }, }, assertions: func(t *testing.T, err error) { require.Error(t, err) - expectedErr := fmt.Sprintf("building component %q: component does not exist in the catalog configuration. Available components are: %s", "invalid", []string{"testcatalog"}) - require.Equal(t, expectedErr, err.Error()) + require.Equal(t, "validating component \"first-catalog\": validate error!", err.Error()) }, }, { - name: "builder not in catalog config", - validate: true, + name: "Skipping validation", + validate: false, compositeTemplate: Template{ - CatalogBuilders: CatalogBuilderMap{ - "testcatalog": BuilderMap{ - "olm.builder.test": &TestBuilder{}, - }, + catalogFile: strings.NewReader(renderValidCatalog), + contributionFile: strings.NewReader(renderValidComposite), + registeredBuilders: map[string]builderFunc{ + TestBuilderSchema: func(bc BuilderConfig) Builder { return &TestBuilder{validateShouldError: true} }, }, }, - compositeCfg: CompositeConfig{ - Schema: CompositeSchema, - Components: []Component{ - { - Name: "testcatalog", - Destination: ComponentDestination{ - Path: "testcatalog/mypackage", - }, - Strategy: BuildStrategy{ - Name: "testbuild", - Template: TemplateDefinition{ - Schema: "olm.builder.invalid", - Config: json.RawMessage{}, - }, - }, - }, + assertions: func(t *testing.T, err error) { + // We are skipping validation so we shouldn't receive + // the validation error from the TestBuilder + require.NoError(t, err) + }, + }, + { + name: "component not in catalog config", + validate: true, + compositeTemplate: Template{ + catalogFile: strings.NewReader(renderValidCatalog), + contributionFile: strings.NewReader(renderInvalidComponentComposite), + registeredBuilders: map[string]builderFunc{ + TestBuilderSchema: func(bc BuilderConfig) Builder { return &TestBuilder{} }, }, }, assertions: func(t *testing.T, err error) { require.Error(t, err) - expectedErr := fmt.Sprintf("building component %q: no builder found for template schema %q", "testcatalog", "olm.builder.invalid") + expectedErr := fmt.Sprintf("building component %q: component does not exist in the catalog configuration. Available components are: %s", "missing-catalog", []string{"first-catalog"}) require.Equal(t, expectedErr, err.Error()) }, }, { - name: "build step error", + name: "builder not in catalog config", validate: true, compositeTemplate: Template{ - CatalogBuilders: CatalogBuilderMap{ - "testcatalog": BuilderMap{ - "olm.builder.test": &TestBuilder{buildError: true}, - }, + catalogFile: strings.NewReader(renderValidCatalog), + contributionFile: strings.NewReader(renderInvalidBuilderComposite), + registeredBuilders: map[string]builderFunc{ + TestBuilderSchema: func(bc BuilderConfig) Builder { return &TestBuilder{} }, }, }, - compositeCfg: CompositeConfig{ - Schema: CompositeSchema, - Components: []Component{ - { - Name: "testcatalog", - Destination: ComponentDestination{ - Path: "testcatalog/mypackage", - }, - Strategy: BuildStrategy{ - Name: "testbuild", - Template: TemplateDefinition{ - Schema: "olm.builder.test", - Config: json.RawMessage{}, - }, - }, - }, - }, + assertions: func(t *testing.T, err error) { + require.Error(t, err) + require.Equal(t, "building component \"first-catalog\": no builder found for template schema \"olm.builder.invalid\"", err.Error()) + }, + }, + { + name: "error parsing catalog spec", + validate: true, + compositeTemplate: Template{ + catalogFile: strings.NewReader(invalidSchemaCatalog), }, assertions: func(t *testing.T, err error) { require.Error(t, err) - expectedErr := fmt.Sprintf("building component %q: %s", "testcatalog", buildErr) - require.Equal(t, expectedErr, err.Error()) + require.Equal(t, "catalog configuration file has unknown schema, should be \"olm.composite.catalogs\"", err.Error()) }, }, { - name: "validate step error", + name: "error parsing contribution spec", validate: true, compositeTemplate: Template{ - CatalogBuilders: CatalogBuilderMap{ - "testcatalog": BuilderMap{ - "olm.builder.test": &TestBuilder{validateError: true}, + catalogFile: strings.NewReader(renderValidCatalog), + contributionFile: strings.NewReader(invalidSchemaComposite), + }, + assertions: func(t *testing.T, err error) { + require.Error(t, err) + require.Equal(t, "composite configuration file has unknown schema, should be \"olm.composite\"", err.Error()) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.compositeTemplate.Render(context.Background(), tc.validate) + tc.assertions(t, err) + }) + } +} + +func TestBuilderForSchema(t *testing.T) { + type testCase struct { + name string + builderSchema string + builderCfg BuilderConfig + assertions func(t *testing.T, builder Builder, err error) + } + + testCases := []testCase{ + { + name: "Basic Builder Schema", + builderSchema: BasicBuilderSchema, + builderCfg: BuilderConfig{}, + assertions: func(t *testing.T, builder Builder, err error) { + require.NoError(t, err) + require.IsType(t, &BasicBuilder{}, builder) + }, + }, + { + name: "Semver Builder Schema", + builderSchema: SemverBuilderSchema, + builderCfg: BuilderConfig{}, + assertions: func(t *testing.T, builder Builder, err error) { + require.NoError(t, err) + require.IsType(t, &SemverBuilder{}, builder) + }, + }, + { + name: "Raw Builder Schema", + builderSchema: RawBuilderSchema, + builderCfg: BuilderConfig{}, + assertions: func(t *testing.T, builder Builder, err error) { + require.NoError(t, err) + require.IsType(t, &RawBuilder{}, builder) + }, + }, + { + name: "Custom Builder Schema", + builderSchema: CustomBuilderSchema, + builderCfg: BuilderConfig{}, + assertions: func(t *testing.T, builder Builder, err error) { + require.NoError(t, err) + require.IsType(t, &CustomBuilder{}, builder) + }, + }, + { + name: "Invalid Builder Schema", + builderSchema: "invalid", + builderCfg: BuilderConfig{}, + assertions: func(t *testing.T, builder Builder, err error) { + require.Error(t, err) + require.Equal(t, fmt.Sprintf("unknown schema %q", "invalid"), err.Error()) + require.Nil(t, builder) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + template := NewTemplate() + builder, err := template.builderForSchema(tc.builderSchema, tc.builderCfg) + tc.assertions(t, builder, err) + }) + } + +} + +var validCatalog = ` +schema: olm.composite.catalogs +catalogs: + - name: first-catalog + destination: + baseImage: quay.io/operator-framework/opm:latest + workingDir: contributions/first-catalog + builders: + - olm.builder.semver + - olm.builder.basic + - name: second-catalog + destination: + baseImage: quay.io/operator-framework/opm:latest + workingDir: contributions/second-catalog + builders: + - olm.builder.semver + - name: test-catalog + destination: + baseImage: quay.io/operator-framework/opm:latest + workingDir: contributions/test-catalog + builders: + - olm.builder.custom` + +var unmarshalFail = ` +invalid +` + +var invalidSchemaCatalog = ` +schema: invalid +catalogs: + - name: first-catalog + destination: + baseImage: quay.io/operator-framework/opm:latest + workingDir: contributions/first-catalog + builders: + - olm.builder.semver + - olm.builder.basic +` + +func TestParseCatalogSpec(t *testing.T) { + type testCase struct { + name string + catalog string + assertions func(t *testing.T, catalog *CatalogConfig, err error) + } + + testCases := []testCase{ + { + name: "Valid catalog configuration", + catalog: validCatalog, + assertions: func(t *testing.T, catalog *CatalogConfig, err error) { + require.NoError(t, err) + require.Equal(t, 3, len(catalog.Catalogs)) + }, + }, + { + name: "Unmarshal failure", + catalog: unmarshalFail, + assertions: func(t *testing.T, catalog *CatalogConfig, err error) { + require.Error(t, err) + require.Equal(t, "unmarshalling catalog config: json: cannot unmarshal string into Go value of type composite.CatalogConfig", err.Error()) + }, + }, + { + name: "Invalid schema", + catalog: invalidSchemaCatalog, + assertions: func(t *testing.T, catalog *CatalogConfig, err error) { + require.Error(t, err) + require.Equal(t, fmt.Sprintf("catalog configuration file has unknown schema, should be %q", CatalogSchema), err.Error()) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + template := NewTemplate(WithCatalogFile(strings.NewReader(tc.catalog))) + catalog, err := template.parseCatalogsSpec() + tc.assertions(t, catalog, err) + }) + } +} + +var validComposite = ` +schema: olm.composite +components: + - name: first-catalog + destination: + path: my-operator + strategy: + name: semver + template: + schema: olm.builder.semver + config: + input: components/contribution1.yaml + output: catalog.yaml +` + +var invalidSchemaComposite = ` +schema: invalid +components: + - name: first-catalog + destination: + path: my-operator + strategy: + name: semver + template: + schema: olm.builder.semver + config: + input: components/contribution1.yaml + output: catalog.yaml +` + +func TestParseContributionSpec(t *testing.T) { + type testCase struct { + name string + composite string + assertions func(t *testing.T, composite *CompositeConfig, err error) + } + + testCases := []testCase{ + { + name: "Valid composite", + composite: validComposite, + assertions: func(t *testing.T, composite *CompositeConfig, err error) { + require.NoError(t, err) + require.Equal(t, 1, len(composite.Components)) + }, + }, + { + name: "Unmarshal failure", + composite: unmarshalFail, + assertions: func(t *testing.T, composite *CompositeConfig, err error) { + require.Error(t, err) + require.Equal(t, "unmarshalling composite config: json: cannot unmarshal string into Go value of type composite.CompositeConfig", err.Error()) + }, + }, + { + name: "Invalid schema", + composite: invalidSchemaComposite, + assertions: func(t *testing.T, composite *CompositeConfig, err error) { + require.Error(t, err) + require.Equal(t, fmt.Sprintf("composite configuration file has unknown schema, should be %q", CompositeSchema), err.Error()) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + template := NewTemplate(WithContributionFile(strings.NewReader(tc.composite))) + contrib, err := template.parseContributionSpec() + tc.assertions(t, contrib, err) + }) + } +} + +func TestNewCatalogBuilderMap(t *testing.T) { + type testCase struct { + name string + catalogs []Catalog + assertions func(t *testing.T, builderMap *CatalogBuilderMap, err error) + } + + testCases := []testCase{ + { + name: "Valid Catalogs", + catalogs: []Catalog{ + { + Name: "test-catalog", + Destination: CatalogDestination{ + WorkingDir: "/", + BaseImage: "base", + }, + Builders: []string{ + BasicBuilderSchema, }, }, }, - compositeCfg: CompositeConfig{ - Schema: CompositeSchema, - Components: []Component{ - { - Name: "testcatalog", - Destination: ComponentDestination{ - Path: "testcatalog/mypackage", - }, - Strategy: BuildStrategy{ - Name: "testbuild", - Template: TemplateDefinition{ - Schema: "olm.builder.test", - Config: json.RawMessage{}, - }, - }, + assertions: func(t *testing.T, builderMap *CatalogBuilderMap, err error) { + require.NoError(t, err) + //TODO: More assertions here + }, + }, + { + name: "Invalid Builder", + catalogs: []Catalog{ + { + Name: "test-catalog", + Destination: CatalogDestination{ + WorkingDir: "/", + BaseImage: "base", + }, + Builders: []string{ + "invalid", }, }, }, - assertions: func(t *testing.T, err error) { + assertions: func(t *testing.T, builderMap *CatalogBuilderMap, err error) { require.Error(t, err) - expectedErr := fmt.Sprintf("validating component %q: %s", "testcatalog", validateErr) - require.Equal(t, expectedErr, err.Error()) + require.Equal(t, "getting builder \"invalid\" for catalog \"test-catalog\": unknown schema \"invalid\"", err.Error()) }, }, { - name: "validation step skipped", - validate: false, - compositeTemplate: Template{ - CatalogBuilders: CatalogBuilderMap{ - "testcatalog": BuilderMap{ - "olm.builder.test": &TestBuilder{validateError: true}, + name: "BaseImage+WorkingDir invalid", + catalogs: []Catalog{ + { + Name: "test-catalog", + Destination: CatalogDestination{}, + Builders: []string{ + BasicBuilderSchema, }, }, }, - compositeCfg: CompositeConfig{ - Schema: CompositeSchema, - Components: []Component{ - { - Name: "testcatalog", - Destination: ComponentDestination{ - Path: "testcatalog/mypackage", - }, - Strategy: BuildStrategy{ - Name: "testbuild", - Template: TemplateDefinition{ - Schema: "olm.builder.test", - Config: json.RawMessage{}, - }, - }, - }, - }, + assertions: func(t *testing.T, builderMap *CatalogBuilderMap, err error) { + require.Error(t, err) + require.Equal(t, "catalog configuration file field validation failed: \nCatalog test-catalog:\n - destination.baseImage must not be an empty string\n - destination.workingDir must not be an empty string\n", err.Error()) }, - assertions: func(t *testing.T, err error) { - // the validate step would error but since - // we are skipping it we expect no error to occur + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + template := NewTemplate() + builderMap, err := template.newCatalogBuilderMap(tc.catalogs, "yaml") + tc.assertions(t, builderMap, err) + }) + } +} + +type fakeGetter struct { + catalog string + shouldError bool +} + +func (fg *fakeGetter) Get(url string) (*http.Response, error) { + if fg.shouldError { + return nil, fmt.Errorf("error!") + } + + return &http.Response{ + Body: io.NopCloser(strings.NewReader(fg.catalog)), + }, nil +} + +func TestFetchCatalogConfig(t *testing.T) { + type testCase struct { + name string + fakeGetter *fakeGetter + path string + createFile bool + assertions func(t *testing.T, rc io.ReadCloser, err error) + } + + testCases := []testCase{ + { + name: "Successful HTTP fetch", + path: "http://some-path.com", + fakeGetter: &fakeGetter{ + catalog: validCatalog, + }, + assertions: func(t *testing.T, rc io.ReadCloser, err error) { + require.NoError(t, err) + require.NotNil(t, rc) + }, + }, + { + name: "Failed HTTP fetch", + path: "http://some-path.com", + fakeGetter: &fakeGetter{ + catalog: validCatalog, + shouldError: true, + }, + assertions: func(t *testing.T, rc io.ReadCloser, err error) { + require.Error(t, err) + require.Equal(t, "fetching remote catalog config file \"http://some-path.com\": error!", err.Error()) + }, + }, + // TODO: for some reason this is triggering the fakeGetter.Get() function instead of using os.Open() + { + name: "Successful file fetch", + path: "file/test.yaml", + fakeGetter: &fakeGetter{ + catalog: validCatalog, + }, + createFile: true, + assertions: func(t *testing.T, rc io.ReadCloser, err error) { require.NoError(t, err) + require.NotNil(t, rc) + }, + }, + { + name: "Failed file fetch", + path: "file/test.yaml", + fakeGetter: &fakeGetter{ + catalog: validCatalog, + }, + createFile: false, + assertions: func(t *testing.T, rc io.ReadCloser, err error) { + require.Error(t, err) + require.Equal(t, "opening catalog config file \"file/test.yaml\": open file/test.yaml: no such file or directory", err.Error()) }, }, } + testDir := t.TempDir() + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err := tc.compositeTemplate.Render(context.Background(), &tc.compositeCfg, tc.validate) - tc.assertions(t, err) + filepath := tc.path + if tc.createFile { + err := os.MkdirAll(path.Join(testDir, path.Dir(tc.path)), 0o777) + require.NoError(t, err) + file, err := os.Create(path.Join(testDir, tc.path)) + require.NoError(t, err) + _, err = file.WriteString(tc.fakeGetter.catalog) + require.NoError(t, err) + + filepath = path.Join(testDir, tc.path) + } + + rc, err := FetchCatalogConfig(filepath, tc.fakeGetter) + tc.assertions(t, rc, err) }) } } diff --git a/staging/operator-registry/cmd/opm/alpha/template/composite.go b/staging/operator-registry/cmd/opm/alpha/template/composite.go index e1ac8aa4db..df44998766 100644 --- a/staging/operator-registry/cmd/opm/alpha/template/composite.go +++ b/staging/operator-registry/cmd/opm/alpha/template/composite.go @@ -1,16 +1,11 @@ package template import ( - "encoding/json" - "fmt" - "io" "log" "net/http" - "net/url" "os" "github.com/spf13/cobra" - "k8s.io/apimachinery/pkg/util/yaml" "github.com/operator-framework/operator-registry/alpha/template/composite" "github.com/operator-framework/operator-registry/cmd/opm/internal/util" @@ -18,9 +13,7 @@ import ( func newCompositeTemplateCmd() *cobra.Command { var ( - template composite.Template output string - containerTool string validate bool compositeFile string catalogFile string @@ -33,166 +26,52 @@ and a 'composite template' file`, and a 'composite template' file`, Args: cobra.MaximumNArgs(0), Run: func(cmd *cobra.Command, args []string) { - containerTool = "docker" - var tempCatalog io.ReadCloser - catalogURI, err := url.ParseRequestURI(catalogFile) - if err != nil { - tempCatalog, err = os.Open(catalogFile) - if err != nil { - log.Fatalf("opening catalog config file %q: %v", catalogFile, err) - } - defer tempCatalog.Close() - } else { - tempResp, err := http.Get(catalogURI.String()) - if err != nil { - log.Fatalf("fetching remote catalog config file %q: %v", catalogFile, err) - } - tempCatalog = tempResp.Body - defer tempCatalog.Close() - } - catalogData := tempCatalog - - // get catalog configurations - catalogConfig := &composite.CatalogConfig{} - catalogDoc := json.RawMessage{} - catalogDecoder := yaml.NewYAMLOrJSONDecoder(catalogData, 4096) - err = catalogDecoder.Decode(&catalogDoc) - if err != nil { - log.Fatalf("decoding catalog config: %v", err) - } - err = json.Unmarshal(catalogDoc, catalogConfig) - if err != nil { - log.Fatalf("unmarshalling catalog config: %v", err) - } - if catalogConfig.Schema != composite.CatalogSchema { - log.Fatalf("catalog configuration file has unknown schema, should be %q", composite.CatalogSchema) + switch output { + case "yaml": + // do nothing + case "json": + // do nothing + default: + log.Fatalf("invalid --output value %q, expected (json|yaml)", output) } - catalogBuilderMap := make(composite.CatalogBuilderMap) - - wd, err := os.Getwd() - if err != nil { - log.Fatalf("getting current working directory: %v", err) - } - - // setup the builders for each catalog - setupFailed := false - setupErrors := map[string][]string{} - for _, catalog := range catalogConfig.Catalogs { - errs := []string{} - if catalog.Destination.BaseImage == "" { - errs = append(errs, "destination.baseImage must not be an empty string") - } - - if catalog.Destination.WorkingDir == "" { - errs = append(errs, "destination.workingDir must not be an empty string") - } - - // check for validation errors and skip builder creation if there are any errors - if len(errs) > 0 { - setupFailed = true - setupErrors[catalog.Name] = errs - continue - } - - if _, ok := catalogBuilderMap[catalog.Name]; !ok { - builderMap := make(composite.BuilderMap) - for _, schema := range catalog.Builders { - builder, err := builderForSchema(schema, composite.BuilderConfig{ - ContainerCfg: composite.ContainerConfig{ - ContainerTool: containerTool, - BaseImage: catalog.Destination.BaseImage, - WorkingDir: catalog.Destination.WorkingDir, - }, - OutputType: output, - // BUGBUG: JEK: This is a strong assumption that input is always local to execution which we need to eliminate in a later PR - InputDirectory: wd, - }) - if err != nil { - log.Fatalf("getting builder %q for catalog %q: %v", schema, catalog.Name, err) - } - builderMap[schema] = builder - } - catalogBuilderMap[catalog.Name] = builderMap - } - } - - // if there were errors validating the catalog configuration then exit - if setupFailed { - //build the error message - var errMsg string - for cat, errs := range setupErrors { - errMsg += fmt.Sprintf("\nCatalog %v:\n", cat) - for _, err := range errs { - errMsg += fmt.Sprintf(" - %v\n", err) - } - } - log.Fatalf("catalog configuration file field validation failed: %s", errMsg) - } - - template.CatalogBuilders = catalogBuilderMap - reg, err := util.CreateCLIRegistry(cmd) if err != nil { log.Fatalf("creating containerd registry: %v", err) } defer reg.Destroy() - template.Registry = reg - - compositeData, err := os.Open(compositeFile) + // operator author's 'composite.yaml' file + compositeReader, err := os.Open(compositeFile) if err != nil { log.Fatalf("opening composite config file %q: %v", compositeFile, err) } - defer compositeData.Close() + defer compositeReader.Close() - // parse data to composite config - compositeConfig := &composite.CompositeConfig{} - compositeDoc := json.RawMessage{} - compositeDecoder := yaml.NewYAMLOrJSONDecoder(compositeData, 4096) - err = compositeDecoder.Decode(&compositeDoc) + // catalog maintainer's 'catalogs.yaml' file + tempCatalog, err := composite.FetchCatalogConfig(catalogFile, http.DefaultClient) if err != nil { - log.Fatalf("decoding composite config: %v", err) - } - err = json.Unmarshal(compositeDoc, compositeConfig) - if err != nil { - log.Fatalf("unmarshalling composite config: %v", err) + log.Fatalf(err.Error()) } + defer tempCatalog.Close() - if compositeConfig.Schema != composite.CompositeSchema { - log.Fatalf("%q has unknown schema, should be %q", compositeFile, composite.CompositeSchema) - } + template := composite.NewTemplate( + composite.WithCatalogFile(tempCatalog), + composite.WithContributionFile(compositeReader), + composite.WithOutputType(output), + composite.WithRegistry(reg), + ) - err = template.Render(cmd.Context(), compositeConfig, validate) + err = template.Render(cmd.Context(), validate) if err != nil { log.Fatalf("rendering the composite template: %v", err) } }, } cmd.Flags().StringVarP(&output, "output", "o", "json", "Output format (json|yaml)") - // TODO: Investigate ways to do this without using a cli tool like docker/podman - // cmd.Flags().StringVar(&containerTool, "container-tool", "docker", "container tool to be used when rendering templates (should be an equivalent replacement to docker - similar to podman)") cmd.Flags().BoolVar(&validate, "validate", true, "whether or not the created FBC should be validated (i.e 'opm validate')") - cmd.Flags().StringVarP(&compositeFile, "composite-config", "c", "catalog/config.yaml", "File to use as the composite configuration file") + cmd.Flags().StringVarP(&compositeFile, "composite-config", "c", "composite.yaml", "File to use as the composite configuration file") cmd.Flags().StringVarP(&catalogFile, "catalog-config", "f", "catalogs.yaml", "File to use as the catalog configuration file") return cmd } - -func builderForSchema(schema string, builderCfg composite.BuilderConfig) (composite.Builder, error) { - var builder composite.Builder - switch schema { - case composite.BasicBuilderSchema: - builder = composite.NewBasicBuilder(builderCfg) - case composite.SemverBuilderSchema: - builder = composite.NewSemverBuilder(builderCfg) - case composite.RawBuilderSchema: - builder = composite.NewRawBuilder(builderCfg) - case composite.CustomBuilderSchema: - builder = composite.NewCustomBuilder(builderCfg) - default: - return nil, fmt.Errorf("unknown schema %q", schema) - } - - return builder, nil -}