From 49927453077abfa2885680e97d682484b90097b3 Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Tue, 28 Mar 2023 15:45:25 -0500 Subject: [PATCH 1/7] refactor to move more processing to the template Signed-off-by: Jordan Keister --- alpha/template/composite/builder.go | 1 - alpha/template/composite/builder_test.go | 4 - alpha/template/composite/composite.go | 181 ++++++++++++++++++++++- cmd/opm/alpha/template/composite.go | 158 +------------------- 4 files changed, 181 insertions(+), 163 deletions(-) diff --git a/alpha/template/composite/builder.go b/alpha/template/composite/builder.go index 3d1f7af95..80d7abdf2 100644 --- a/alpha/template/composite/builder.go +++ b/alpha/template/composite/builder.go @@ -36,7 +36,6 @@ type ContainerConfig struct { type BuilderConfig struct { ContainerCfg ContainerConfig OutputType string - InputDirectory string } type Builder interface { diff --git a/alpha/template/composite/builder_test.go b/alpha/template/composite/builder_test.go index af7850b07..17e2aa55a 100644 --- a/alpha/template/composite/builder_test.go +++ b/alpha/template/composite/builder_test.go @@ -244,7 +244,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) @@ -656,7 +655,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) @@ -1077,7 +1075,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) @@ -1505,7 +1502,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) diff --git a/alpha/template/composite/composite.go b/alpha/template/composite/composite.go index 422017c97..d1af1a026 100644 --- a/alpha/template/composite/composite.go +++ b/alpha/template/composite/composite.go @@ -2,9 +2,15 @@ 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 @@ -12,15 +18,34 @@ type BuilderMap map[string]Builder type CatalogBuilderMap map[string]BuilderMap type Template struct { - CatalogBuilders CatalogBuilderMap - Registry image.Registry + CatalogFile string + ContributionFile string + Validate bool + OutputType string + Registry image.Registry } // 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) @@ -40,7 +65,7 @@ func (t *Template) Render(ctx context.Context, config *CompositeConfig, validate } } else { allowedComponents := []string{} - for k := range t.CatalogBuilders { + for k := range builderMap { 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 +73,147 @@ func (t *Template) Render(ctx context.Context, config *CompositeConfig, validate } return nil } + +func builderForSchema(schema string, builderCfg BuilderConfig) (Builder, error) { + var builder Builder + switch schema { + case BasicBuilderSchema: + builder = NewBasicBuilder(builderCfg) + case SemverBuilderSchema: + builder = NewSemverBuilder(builderCfg) + case RawBuilderSchema: + builder = NewRawBuilder(builderCfg) + case CustomBuilderSchema: + builder = NewCustomBuilder(builderCfg) + default: + return nil, fmt.Errorf("unknown schema %q", schema) + } + + return builder, nil +} + +func (t *Template) parseCatalogsSpec() (*CatalogConfig, error) { + var tempCatalog io.ReadCloser + catalogURI, err := url.ParseRequestURI(t.CatalogFile) + if err != nil { + tempCatalog, err = os.Open(t.CatalogFile) + if err != nil { + return nil, fmt.Errorf("opening catalog config file %q: %v", t.CatalogFile, err) + } + defer tempCatalog.Close() + } else { + tempResp, err := http.Get(catalogURI.String()) + if err != nil { + return nil, fmt.Errorf("fetching remote catalog config file %q: %v", t.CatalogFile, err) + } + tempCatalog = tempResp.Body + defer tempCatalog.Close() + } + catalogData := tempCatalog + + // get catalog configurations + catalogConfig := &CatalogConfig{} + catalogDoc := json.RawMessage{} + catalogDecoder := yaml.NewYAMLOrJSONDecoder(catalogData, 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) { + + compositeData, err := os.Open(t.ContributionFile) + if err != nil { + return nil, fmt.Errorf("opening composite config file %q: %v", t.ContributionFile, err) + } + defer compositeData.Close() + + // parse data to composite config + compositeConfig := &CompositeConfig{} + compositeDoc := json.RawMessage{} + compositeDecoder := yaml.NewYAMLOrJSONDecoder(compositeData, 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("%q has unknown schema, should be %q", t.ContributionFile, 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 := builderForSchema(schema, BuilderConfig{ + ContainerCfg: ContainerConfig{ + BaseImage: catalog.Destination.BaseImage, + WorkingDir: catalog.Destination.WorkingDir, + }, + 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/cmd/opm/alpha/template/composite.go b/cmd/opm/alpha/template/composite.go index e1ac8aa4d..05a9aae32 100644 --- a/cmd/opm/alpha/template/composite.go +++ b/cmd/opm/alpha/template/composite.go @@ -1,16 +1,9 @@ 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 +11,7 @@ import ( func newCompositeTemplateCmd() *cobra.Command { var ( - template composite.Template output string - containerTool string validate bool compositeFile string catalogFile string @@ -33,105 +24,6 @@ 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) - } - - 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 { @@ -139,60 +31,22 @@ and a 'composite template' file`, } defer reg.Destroy() - template.Registry = reg - - compositeData, err := os.Open(compositeFile) - if err != nil { - log.Fatalf("opening composite config file %q: %v", compositeFile, err) + template := composite.Template{ + Registry: reg, + CatalogFile: catalogFile, + ContributionFile: compositeFile, + OutputType: output, } - defer compositeData.Close() - // parse data to composite config - compositeConfig := &composite.CompositeConfig{} - compositeDoc := json.RawMessage{} - compositeDecoder := yaml.NewYAMLOrJSONDecoder(compositeData, 4096) - err = compositeDecoder.Decode(&compositeDoc) - 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) - } - - if compositeConfig.Schema != composite.CompositeSchema { - log.Fatalf("%q has unknown schema, should be %q", compositeFile, composite.CompositeSchema) - } - - 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(&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 -} From 6b4a6a5a491cfa32c4e28b4d05863fea66da1d31 Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Fri, 31 Mar 2023 13:11:33 -0500 Subject: [PATCH 2/7] better diagnostics on custom exec error Signed-off-by: Jordan Keister --- alpha/template/composite/builder.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/alpha/template/composite/builder.go b/alpha/template/composite/builder.go index 80d7abdf2..4273fa746 100644 --- a/alpha/template/composite/builder.go +++ b/alpha/template/composite/builder.go @@ -34,8 +34,8 @@ type ContainerConfig struct { } type BuilderConfig struct { - ContainerCfg ContainerConfig - OutputType string + ContainerCfg ContainerConfig + OutputType string } type Builder interface { @@ -267,13 +267,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() + v, err := cmd.CombinedOutput() 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) From 8f129e9839ddbf09396f3040e5cfd87765ae0371 Mon Sep 17 00:00:00 2001 From: Catherine Chan-Tse Date: Tue, 11 Apr 2023 18:30:24 -0400 Subject: [PATCH 3/7] Remove obsolete fields Signed-off-by: Catherine Chan-Tse Signed-off-by: Jordan Keister --- alpha/template/composite/builder.go | 30 ++-- alpha/template/composite/builder_test.go | 188 ++++----------------- alpha/template/composite/composite.go | 4 - alpha/template/composite/composite_test.go | 2 +- 4 files changed, 45 insertions(+), 179 deletions(-) diff --git a/alpha/template/composite/builder.go b/alpha/template/composite/builder.go index 4273fa746..dadb0f753 100644 --- a/alpha/template/composite/builder.go +++ b/alpha/template/composite/builder.go @@ -27,14 +27,8 @@ const ( CustomBuilderSchema = "olm.builder.custom" ) -type ContainerConfig struct { - ContainerTool string - BaseImage string - WorkingDir string -} - type BuilderConfig struct { - ContainerCfg ContainerConfig + WorkingDir string OutputType string } @@ -95,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 { @@ -157,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 { @@ -217,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 { @@ -284,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. @@ -292,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 { @@ -306,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/alpha/template/composite/builder_test.go b/alpha/template/composite/builder_test.go index 17e2aa55a..c8427dc28 100644 --- a/alpha/template/composite/builder_test.go +++ b/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{ @@ -446,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{ @@ -481,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{ @@ -516,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{ @@ -539,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{ @@ -562,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{ @@ -583,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{ @@ -608,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{ @@ -633,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{ @@ -867,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{ @@ -902,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{ @@ -937,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{ @@ -960,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{ @@ -983,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{ @@ -1003,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{ @@ -1028,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{ @@ -1053,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{ @@ -1310,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{ @@ -1345,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{ @@ -1380,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{ @@ -1403,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{ @@ -1427,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{ @@ -1453,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{ @@ -1479,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{ @@ -1715,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/alpha/template/composite/composite.go b/alpha/template/composite/composite.go index d1af1a026..93c10845d 100644 --- a/alpha/template/composite/composite.go +++ b/alpha/template/composite/composite.go @@ -187,10 +187,6 @@ func (t *Template) newCatalogBuilderMap(catalogs []Catalog, outputType string) ( builderMap := make(BuilderMap) for _, schema := range catalog.Builders { builder, err := builderForSchema(schema, BuilderConfig{ - ContainerCfg: ContainerConfig{ - BaseImage: catalog.Destination.BaseImage, - WorkingDir: catalog.Destination.WorkingDir, - }, OutputType: outputType, }) if err != nil { diff --git a/alpha/template/composite/composite_test.go b/alpha/template/composite/composite_test.go index c2b28fac7..6984448da 100644 --- a/alpha/template/composite/composite_test.go +++ b/alpha/template/composite/composite_test.go @@ -251,7 +251,7 @@ func TestCompositeRender(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err := tc.compositeTemplate.Render(context.Background(), &tc.compositeCfg, tc.validate) + err := tc.compositeTemplate.Render(context.Background(), tc.validate) tc.assertions(t, err) }) } From 3dac14cb12ab82a7e49507ff6e85cfb28aed79ee Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Tue, 18 Apr 2023 10:42:42 -0500 Subject: [PATCH 4/7] caching wip Signed-off-by: Jordan Keister --- alpha/template/composite/composite.go | 38 +---- alpha/template/composite/composite_test.go | 186 +++++++++++++++------ cmd/opm/alpha/template/composite.go | 44 ++++- 3 files changed, 180 insertions(+), 88 deletions(-) diff --git a/alpha/template/composite/composite.go b/alpha/template/composite/composite.go index 93c10845d..a30b0bb07 100644 --- a/alpha/template/composite/composite.go +++ b/alpha/template/composite/composite.go @@ -5,9 +5,6 @@ import ( "encoding/json" "fmt" "io" - "net/http" - "net/url" - "os" "github.com/operator-framework/operator-registry/pkg/image" "k8s.io/apimachinery/pkg/util/yaml" @@ -18,8 +15,8 @@ type BuilderMap map[string]Builder type CatalogBuilderMap map[string]BuilderMap type Template struct { - CatalogFile string - ContributionFile string + CatalogFile io.Reader + ContributionFile io.Reader Validate bool OutputType string Registry image.Registry @@ -93,29 +90,12 @@ func builderForSchema(schema string, builderCfg BuilderConfig) (Builder, error) } func (t *Template) parseCatalogsSpec() (*CatalogConfig, error) { - var tempCatalog io.ReadCloser - catalogURI, err := url.ParseRequestURI(t.CatalogFile) - if err != nil { - tempCatalog, err = os.Open(t.CatalogFile) - if err != nil { - return nil, fmt.Errorf("opening catalog config file %q: %v", t.CatalogFile, err) - } - defer tempCatalog.Close() - } else { - tempResp, err := http.Get(catalogURI.String()) - if err != nil { - return nil, fmt.Errorf("fetching remote catalog config file %q: %v", t.CatalogFile, err) - } - tempCatalog = tempResp.Body - defer tempCatalog.Close() - } - catalogData := tempCatalog // get catalog configurations catalogConfig := &CatalogConfig{} catalogDoc := json.RawMessage{} - catalogDecoder := yaml.NewYAMLOrJSONDecoder(catalogData, 4096) - err = catalogDecoder.Decode(&catalogDoc) + catalogDecoder := yaml.NewYAMLOrJSONDecoder(t.CatalogFile, 4096) + err := catalogDecoder.Decode(&catalogDoc) if err != nil { return nil, fmt.Errorf("decoding catalog config: %v", err) } @@ -133,17 +113,11 @@ func (t *Template) parseCatalogsSpec() (*CatalogConfig, error) { func (t *Template) parseContributionSpec() (*CompositeConfig, error) { - compositeData, err := os.Open(t.ContributionFile) - if err != nil { - return nil, fmt.Errorf("opening composite config file %q: %v", t.ContributionFile, err) - } - defer compositeData.Close() - // parse data to composite config compositeConfig := &CompositeConfig{} compositeDoc := json.RawMessage{} - compositeDecoder := yaml.NewYAMLOrJSONDecoder(compositeData, 4096) - err = compositeDecoder.Decode(&compositeDoc) + compositeDecoder := yaml.NewYAMLOrJSONDecoder(t.ContributionFile, 4096) + err := compositeDecoder.Decode(&compositeDoc) if err != nil { return nil, fmt.Errorf("decoding composite config: %v", err) } diff --git a/alpha/template/composite/composite_test.go b/alpha/template/composite/composite_test.go index 6984448da..940775930 100644 --- a/alpha/template/composite/composite_test.go +++ b/alpha/template/composite/composite_test.go @@ -1,39 +1,116 @@ package composite import ( + "bytes" "context" "encoding/json" - "errors" "fmt" "testing" - "github.com/operator-framework/operator-registry/pkg/image" "github.com/stretchr/testify/require" ) -type TestBuilder struct { - buildError bool - validateError bool -} +var catalogs = []byte(` +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`) -const buildErr = "TestBuilder Build() error" -const validateErr = "TestBuilder Validate() error" +var composite = []byte(` +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 _ Builder = &TestBuilder{} +var testCompositeFormat = []byte(` +- name: test-catalog + destination: + path: my-package + strategy: + name: custom + template: + schema: olm.builder.custom + config: + command: cat + args: + - "components/v4.13.yaml" + output: catalog.yaml +`) -func (tb *TestBuilder) Build(ctx context.Context, reg image.Registry, dir string, vd TemplateDefinition) error { - if tb.buildError { - return errors.New(buildErr) - } - return nil -} +var semverContribution = []byte(`--- +schema: olm.semver +stable: + bundles: + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.4.0 + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.4.1 + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.4.2 + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.4.3 + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.4.4 + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.5.0 + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.5.1 + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.5.2 + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.6.0 + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.7.0 + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.8.0 + - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.8.1 +`) -func (tb *TestBuilder) Validate(dir string) error { - if tb.validateError { - return errors.New(validateErr) - } - return nil -} +var basicContribution = []byte(`--- +--- +schema: olm.package +name: kubevirt-hyperconverged +defaultChannel: stable +--- +schema: olm.channel +package: kubevirt-hyperconverged +name: stable +entries: +- name: kubevirt-hyperconverged-operator.v4.10.0 +- name: kubevirt-hyperconverged-operator.v4.10.1 + replaces: kubevirt-hyperconverged-operator.v4.10.0 +- name: kubevirt-hyperconverged-operator.v4.10.2 + replaces: kubevirt-hyperconverged-operator.v4.10.1 +- name: kubevirt-hyperconverged-operator.v4.10.3 + replaces: kubevirt-hyperconverged-operator.v4.10.2 +--- +schema: olm.bundle +image: registry.redhat.io/container-native-virtualization/hco-bundle-registry@sha256:35b29e8eb48d9818a1217d5b89e4dcb7a900c5c5e6ae3745683813c5708c86e9 +--- +schema: olm.bundle +image: registry.redhat.io/container-native-virtualization/hco-bundle-registry@sha256:ac8b60a91411c0fcc4ab2c52db8b6e7682ee747c8969dde9ad8d1a5aa7d44772 +--- +schema: olm.bundle +image: registry.redhat.io/container-native-virtualization/hco-bundle-registry@sha256:9c10a5c4e5ffad632bc8b4289fe2bc0c181bc7c4c8270a356ac21ebff568a45e +--- +schema: olm.bundle +image: registry.redhat.io/container-native-virtualization/hco-bundle-registry@sha256:34cf9e0dd3cc07c487b364c30b3f95bf352be8ca4fe89d473fc624ad7283651d +`) func TestCompositeRender(t *testing.T) { type testCase struct { @@ -45,38 +122,41 @@ func TestCompositeRender(t *testing.T) { } testCases := []testCase{ - { - 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{}, - }, - }, - }, - }, - }, - assertions: func(t *testing.T, err error) { - require.NoError(t, err) - }, - }, + // { + // name: "successful render", + // validate: true, + // compositeTemplate: Template{ + // CatalogFile: bytes.NewReader(catalogs), + // ContributionFile: bytes.NewReader(composite), + + // 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{}, + // }, + // }, + // }, + // }, + // }, + // assertions: func(t *testing.T, err error) { + // require.NoError(t, err) + // }, + // }, { name: "component not in catalog config", validate: true, diff --git a/cmd/opm/alpha/template/composite.go b/cmd/opm/alpha/template/composite.go index 05a9aae32..e2f9e7df6 100644 --- a/cmd/opm/alpha/template/composite.go +++ b/cmd/opm/alpha/template/composite.go @@ -1,7 +1,11 @@ package template import ( + "io" "log" + "net/http" + "net/url" + "os" "github.com/spf13/cobra" @@ -25,16 +29,50 @@ and a 'composite template' file`, Args: cobra.MaximumNArgs(0), Run: func(cmd *cobra.Command, args []string) { + switch output { + case "yaml": + // do nothing + case "json": + // do nothing + default: + log.Fatalf("invalid --output value %q, expected (json|yaml)", output) + } + reg, err := util.CreateCLIRegistry(cmd) if err != nil { log.Fatalf("creating containerd registry: %v", err) } defer reg.Destroy() + // 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 compositeReader.Close() + + // catalog maintainer's 'catalogs.yaml' file + 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() + } + template := composite.Template{ Registry: reg, - CatalogFile: catalogFile, - ContributionFile: compositeFile, + CatalogFile: tempCatalog, + ContributionFile: compositeReader, OutputType: output, } @@ -46,7 +84,7 @@ and a 'composite template' file`, } cmd.Flags().StringVarP(&output, "output", "o", "json", "Output format (json|yaml)") 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 } From 4593d4a40aa6693a52701b2135eb09351f79234f Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Wed, 19 Apr 2023 08:36:23 -0400 Subject: [PATCH 5/7] 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 --- alpha/template/composite/composite.go | 123 +++- alpha/template/composite/composite_test.go | 757 ++++++++++++++------- cmd/opm/alpha/template/composite.go | 31 +- 3 files changed, 626 insertions(+), 285 deletions(-) diff --git a/alpha/template/composite/composite.go b/alpha/template/composite/composite.go index a30b0bb07..fbd5e37c6 100644 --- a/alpha/template/composite/composite.go +++ b/alpha/template/composite/composite.go @@ -5,6 +5,9 @@ import ( "encoding/json" "fmt" "io" + "net/http" + "net/url" + "os" "github.com/operator-framework/operator-registry/pkg/image" "k8s.io/apimachinery/pkg/util/yaml" @@ -14,12 +17,91 @@ type BuilderMap map[string]Builder type CatalogBuilderMap map[string]BuilderMap +type builderFunc func(BuilderConfig) Builder + type Template struct { - CatalogFile io.Reader - ContributionFile io.Reader - Validate bool - OutputType string - 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? @@ -35,7 +117,7 @@ func (t *Template) Render(ctx context.Context, validate bool) error { return err } - catalogBuilderMap, err := t.newCatalogBuilderMap(catalogFile.Catalogs, t.OutputType) + catalogBuilderMap, err := t.newCatalogBuilderMap(catalogFile.Catalogs, t.outputType) if err != nil { return err } @@ -45,7 +127,7 @@ func (t *Template) Render(ctx context.Context, validate bool) error { 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) } @@ -62,7 +144,7 @@ func (t *Template) Render(ctx context.Context, validate bool) error { } } else { allowedComponents := []string{} - for k := range builderMap { + 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) @@ -71,22 +153,13 @@ func (t *Template) Render(ctx context.Context, validate bool) error { return nil } -func builderForSchema(schema string, builderCfg BuilderConfig) (Builder, error) { - var builder Builder - switch schema { - case BasicBuilderSchema: - builder = NewBasicBuilder(builderCfg) - case SemverBuilderSchema: - builder = NewSemverBuilder(builderCfg) - case RawBuilderSchema: - builder = NewRawBuilder(builderCfg) - case CustomBuilderSchema: - builder = NewCustomBuilder(builderCfg) - default: +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 builder, nil + return builderFunc(builderCfg), nil } func (t *Template) parseCatalogsSpec() (*CatalogConfig, error) { @@ -94,7 +167,7 @@ func (t *Template) parseCatalogsSpec() (*CatalogConfig, error) { // get catalog configurations catalogConfig := &CatalogConfig{} catalogDoc := json.RawMessage{} - catalogDecoder := yaml.NewYAMLOrJSONDecoder(t.CatalogFile, 4096) + catalogDecoder := yaml.NewYAMLOrJSONDecoder(t.catalogFile, 4096) err := catalogDecoder.Decode(&catalogDoc) if err != nil { return nil, fmt.Errorf("decoding catalog config: %v", err) @@ -116,7 +189,7 @@ func (t *Template) parseContributionSpec() (*CompositeConfig, error) { // parse data to composite config compositeConfig := &CompositeConfig{} compositeDoc := json.RawMessage{} - compositeDecoder := yaml.NewYAMLOrJSONDecoder(t.ContributionFile, 4096) + compositeDecoder := yaml.NewYAMLOrJSONDecoder(t.contributionFile, 4096) err := compositeDecoder.Decode(&compositeDoc) if err != nil { return nil, fmt.Errorf("decoding composite config: %v", err) @@ -127,7 +200,7 @@ func (t *Template) parseContributionSpec() (*CompositeConfig, error) { } if compositeConfig.Schema != CompositeSchema { - return nil, fmt.Errorf("%q has unknown schema, should be %q", t.ContributionFile, CompositeSchema) + return nil, fmt.Errorf("composite configuration file has unknown schema, should be %q", CompositeSchema) } return compositeConfig, nil @@ -160,7 +233,7 @@ func (t *Template) newCatalogBuilderMap(catalogs []Catalog, outputType string) ( if _, ok := catalogBuilderMap[catalog.Name]; !ok { builderMap := make(BuilderMap) for _, schema := range catalog.Builders { - builder, err := builderForSchema(schema, BuilderConfig{ + builder, err := t.builderForSchema(schema, BuilderConfig{ OutputType: outputType, }) if err != nil { diff --git a/alpha/template/composite/composite_test.go b/alpha/template/composite/composite_test.go index 940775930..3e4618516 100644 --- a/alpha/template/composite/composite_test.go +++ b/alpha/template/composite/composite_test.go @@ -1,16 +1,43 @@ package composite import ( - "bytes" "context" - "encoding/json" "fmt" + "io" + "net/http" + "os" + "path" + "strings" "testing" + "github.com/operator-framework/operator-registry/pkg/image" "github.com/stretchr/testify/require" ) -var catalogs = []byte(` +var _ Builder = &TestBuilder{} + +var TestBuilderSchema = "olm.builder.test" + +type TestBuilder struct { + buildShouldError bool + validateShouldError bool +} + +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.validateShouldError { + return fmt.Errorf("validate error!") + } + return nil +} + +var renderValidCatalog = ` schema: olm.composite.catalogs catalogs: - name: first-catalog @@ -18,321 +45,575 @@ catalogs: baseImage: quay.io/operator-framework/opm:latest workingDir: contributions/first-catalog builders: - - olm.builder.semver - - olm.builder.basic - - name: second-catalog + - olm.builder.test +` + +var renderValidComposite = ` +schema: olm.composite +components: + - name: first-catalog destination: - baseImage: quay.io/operator-framework/opm:latest - workingDir: contributions/second-catalog - builders: - - olm.builder.semver - - name: test-catalog + 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: - baseImage: quay.io/operator-framework/opm:latest - workingDir: contributions/test-catalog - builders: - - olm.builder.custom`) + path: my-operator + strategy: + name: test + template: + schema: olm.builder.test + config: + input: components/contribution1.yaml + output: catalog.yaml +` -var composite = []byte(` +var renderInvalidBuilderComposite = ` schema: olm.composite components: - name: first-catalog destination: path: my-operator strategy: - name: semver + name: test template: - schema: olm.builder.semver + schema: olm.builder.invalid config: input: components/contribution1.yaml output: catalog.yaml -`) - -var testCompositeFormat = []byte(` -- name: test-catalog - destination: - path: my-package - strategy: - name: custom - template: - schema: olm.builder.custom - config: - command: cat - args: - - "components/v4.13.yaml" - output: catalog.yaml -`) - -var semverContribution = []byte(`--- -schema: olm.semver -stable: - bundles: - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.4.0 - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.4.1 - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.4.2 - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.4.3 - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.4.4 - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.5.0 - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.5.1 - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.5.2 - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.6.0 - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.7.0 - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.8.0 - - image: quay.io/openshift-community-operators/community-kubevirt-hyperconverged:v1.8.1 -`) - -var basicContribution = []byte(`--- ---- -schema: olm.package -name: kubevirt-hyperconverged -defaultChannel: stable ---- -schema: olm.channel -package: kubevirt-hyperconverged -name: stable -entries: -- name: kubevirt-hyperconverged-operator.v4.10.0 -- name: kubevirt-hyperconverged-operator.v4.10.1 - replaces: kubevirt-hyperconverged-operator.v4.10.0 -- name: kubevirt-hyperconverged-operator.v4.10.2 - replaces: kubevirt-hyperconverged-operator.v4.10.1 -- name: kubevirt-hyperconverged-operator.v4.10.3 - replaces: kubevirt-hyperconverged-operator.v4.10.2 ---- -schema: olm.bundle -image: registry.redhat.io/container-native-virtualization/hco-bundle-registry@sha256:35b29e8eb48d9818a1217d5b89e4dcb7a900c5c5e6ae3745683813c5708c86e9 ---- -schema: olm.bundle -image: registry.redhat.io/container-native-virtualization/hco-bundle-registry@sha256:ac8b60a91411c0fcc4ab2c52db8b6e7682ee747c8969dde9ad8d1a5aa7d44772 ---- -schema: olm.bundle -image: registry.redhat.io/container-native-virtualization/hco-bundle-registry@sha256:9c10a5c4e5ffad632bc8b4289fe2bc0c181bc7c4c8270a356ac21ebff568a45e ---- -schema: olm.bundle -image: registry.redhat.io/container-native-virtualization/hco-bundle-registry@sha256:34cf9e0dd3cc07c487b364c30b3f95bf352be8ca4fe89d473fc624ad7283651d -`) +` func TestCompositeRender(t *testing.T) { type testCase struct { name string compositeTemplate Template - compositeCfg CompositeConfig validate bool assertions func(t *testing.T, err error) } testCases := []testCase{ - // { - // name: "successful render", - // validate: true, - // compositeTemplate: Template{ - // CatalogFile: bytes.NewReader(catalogs), - // ContributionFile: bytes.NewReader(composite), - - // 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{}, - // }, - // }, - // }, - // }, - // }, - // assertions: func(t *testing.T, err error) { - // require.NoError(t, err) - // }, - // }, { - name: "component not in catalog config", + name: "successful render", 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{} }, }, }, - 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.NoError(t, err) + }, + }, + { + name: "Component build failure", + validate: true, + compositeTemplate: Template{ + catalogFile: strings.NewReader(renderValidCatalog), + contributionFile: strings.NewReader(renderValidComposite), + registeredBuilders: map[string]builderFunc{ + TestBuilderSchema: func(bc BuilderConfig) Builder { return &TestBuilder{buildShouldError: 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, "building component \"first-catalog\": build error!", err.Error()) }, }, { - name: "builder not in catalog config", + name: "Component validate 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{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) { + require.Error(t, err) + require.Equal(t, "validating component \"first-catalog\": validate error!", err.Error()) + }, + }, + { + name: "Skipping validation", + validate: false, + 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) { + // 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.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/cmd/opm/alpha/template/composite.go b/cmd/opm/alpha/template/composite.go index e2f9e7df6..df4499876 100644 --- a/cmd/opm/alpha/template/composite.go +++ b/cmd/opm/alpha/template/composite.go @@ -1,10 +1,8 @@ package template import ( - "io" "log" "net/http" - "net/url" "os" "github.com/spf13/cobra" @@ -52,29 +50,18 @@ and a 'composite template' file`, defer compositeReader.Close() // catalog maintainer's 'catalogs.yaml' file - var tempCatalog io.ReadCloser - catalogURI, err := url.ParseRequestURI(catalogFile) + tempCatalog, err := composite.FetchCatalogConfig(catalogFile, http.DefaultClient) 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() + log.Fatalf(err.Error()) } + defer tempCatalog.Close() - template := composite.Template{ - Registry: reg, - CatalogFile: tempCatalog, - ContributionFile: compositeReader, - OutputType: output, - } + template := composite.NewTemplate( + composite.WithCatalogFile(tempCatalog), + composite.WithContributionFile(compositeReader), + composite.WithOutputType(output), + composite.WithRegistry(reg), + ) err = template.Render(cmd.Context(), validate) if err != nil { From 135969c8db8e5b2c116b59562a64248b2f0dce71 Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Wed, 19 Apr 2023 07:45:49 -0500 Subject: [PATCH 6/7] whitespace sanity hell Signed-off-by: Jordan Keister --- alpha/template/composite/builder.go | 4 +- alpha/template/composite/builder_test.go | 62 ++++++++++++------------ 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/alpha/template/composite/builder.go b/alpha/template/composite/builder.go index dadb0f753..80a7d995d 100644 --- a/alpha/template/composite/builder.go +++ b/alpha/template/composite/builder.go @@ -28,8 +28,8 @@ const ( ) type BuilderConfig struct { - WorkingDir string - OutputType string + WorkingDir string + OutputType string } type Builder interface { diff --git a/alpha/template/composite/builder_test.go b/alpha/template/composite/builder_test.go index c8427dc28..a011d4702 100644 --- a/alpha/template/composite/builder_test.go +++ b/alpha/template/composite/builder_test.go @@ -36,7 +36,7 @@ func TestBasicBuilder(t *testing.T) { name: "successful basic build yaml output", validate: true, basicBuilder: NewBasicBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -67,7 +67,7 @@ func TestBasicBuilder(t *testing.T) { name: "successful basic build json output", validate: true, basicBuilder: NewBasicBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "json", }), templateDefinition: TemplateDefinition{ @@ -98,7 +98,7 @@ func TestBasicBuilder(t *testing.T) { name: "invalid template configuration", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -117,7 +117,7 @@ func TestBasicBuilder(t *testing.T) { name: "invalid output type", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "invalid", }), templateDefinition: TemplateDefinition{ @@ -136,7 +136,7 @@ func TestBasicBuilder(t *testing.T) { name: "invalid schema", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -152,7 +152,7 @@ func TestBasicBuilder(t *testing.T) { name: "template config has empty input", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -173,7 +173,7 @@ func TestBasicBuilder(t *testing.T) { name: "template config has empty output", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -194,7 +194,7 @@ func TestBasicBuilder(t *testing.T) { name: "template config has empty input & output", validate: false, basicBuilder: NewBasicBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -414,7 +414,7 @@ func TestSemverBuilder(t *testing.T) { name: "successful semver build yaml output", validate: true, semverBuilder: NewSemverBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -445,7 +445,7 @@ func TestSemverBuilder(t *testing.T) { name: "successful semver build json output", validate: true, semverBuilder: NewSemverBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "json", }), templateDefinition: TemplateDefinition{ @@ -476,7 +476,7 @@ func TestSemverBuilder(t *testing.T) { name: "invalid template configuration", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -495,7 +495,7 @@ func TestSemverBuilder(t *testing.T) { name: "invalid output type", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "invalid", }), templateDefinition: TemplateDefinition{ @@ -514,7 +514,7 @@ func TestSemverBuilder(t *testing.T) { name: "invalid schema", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -531,7 +531,7 @@ func TestSemverBuilder(t *testing.T) { name: "template config has empty input", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -552,7 +552,7 @@ func TestSemverBuilder(t *testing.T) { name: "template config has empty output", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -573,7 +573,7 @@ func TestSemverBuilder(t *testing.T) { name: "template config has empty input & output", validate: false, semverBuilder: NewSemverBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -803,7 +803,7 @@ func TestRawBuilder(t *testing.T) { name: "successful raw build yaml output", validate: true, rawBuilder: NewRawBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -834,7 +834,7 @@ func TestRawBuilder(t *testing.T) { name: "successful raw build json output", validate: true, rawBuilder: NewRawBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "json", }), templateDefinition: TemplateDefinition{ @@ -865,7 +865,7 @@ func TestRawBuilder(t *testing.T) { name: "invalid template configuration", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -884,7 +884,7 @@ func TestRawBuilder(t *testing.T) { name: "invalid output type", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "invalid", }), templateDefinition: TemplateDefinition{ @@ -903,7 +903,7 @@ func TestRawBuilder(t *testing.T) { name: "invalid schema", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -919,7 +919,7 @@ func TestRawBuilder(t *testing.T) { name: "template config has empty input", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -940,7 +940,7 @@ func TestRawBuilder(t *testing.T) { name: "template config has empty output", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -961,7 +961,7 @@ func TestRawBuilder(t *testing.T) { name: "template config has empty input & output", validate: false, rawBuilder: NewRawBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1214,7 +1214,7 @@ func TestCustomBuilder(t *testing.T) { name: "successful custom build yaml output", validate: true, customBuilder: NewCustomBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1245,7 +1245,7 @@ func TestCustomBuilder(t *testing.T) { name: "successful custom build json output", validate: true, customBuilder: NewCustomBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "json", }), templateDefinition: TemplateDefinition{ @@ -1276,7 +1276,7 @@ func TestCustomBuilder(t *testing.T) { name: "invalid template configuration", validate: false, customBuilder: NewCustomBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1295,7 +1295,7 @@ func TestCustomBuilder(t *testing.T) { name: "invalid schema", validate: false, customBuilder: NewCustomBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1315,7 +1315,7 @@ func TestCustomBuilder(t *testing.T) { name: "template config has empty command", validate: false, customBuilder: NewCustomBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1337,7 +1337,7 @@ func TestCustomBuilder(t *testing.T) { name: "template config has empty output", validate: false, customBuilder: NewCustomBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ @@ -1359,7 +1359,7 @@ func TestCustomBuilder(t *testing.T) { name: "template config has empty command & output", validate: false, customBuilder: NewCustomBuilder(BuilderConfig{ - WorkingDir: testDir, + WorkingDir: testDir, OutputType: "yaml", }), templateDefinition: TemplateDefinition{ From 097ed196e19c0804f86d0daf27e583c3ab732165 Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Mon, 24 Apr 2023 15:22:31 -0500 Subject: [PATCH 7/7] drop STDERR for nominal case Signed-off-by: Jordan Keister --- alpha/template/composite/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/alpha/template/composite/builder.go b/alpha/template/composite/builder.go index 80a7d995d..c62a82afc 100644 --- a/alpha/template/composite/builder.go +++ b/alpha/template/composite/builder.go @@ -264,7 +264,7 @@ func (cb *CustomBuilder) Build(ctx context.Context, reg image.Registry, dir stri // custom template should output a valid FBC to STDOUT so we can // build the FBC just like all the other templates. - v, err := cmd.CombinedOutput() + v, err := cmd.Output() if err != nil { return fmt.Errorf("running command %q: %v: %v", cmd.String(), err, v) }