From 60bccc6d3b0deb65696b5c3266ffc85be2a10d92 Mon Sep 17 00:00:00 2001 From: Kim Christensen Date: Sat, 31 Aug 2024 22:48:20 +0200 Subject: [PATCH] Support custom templating in image section during lint Images are validated for valid digests during linting. If the image digest is templating and no default value exist in the bundle definition the linting will fail, because the digest will be invalid. One gotcha here is that if only part of the digest is templated linting will still fail. Signed-off-by: Kim Christensen --- pkg/cnab/config-adapter/helpers.go | 2 +- pkg/manifest/manifest.go | 16 +++++-- pkg/manifest/manifest_test.go | 26 +++++------ pkg/porter/lint.go | 2 +- pkg/porter/lint_test.go | 44 +++++++++++++++++++ .../porter-image-custom-data-default.yaml | 39 ++++++++++++++++ .../porter-image-custom-data-no-default.yaml | 36 +++++++++++++++ pkg/runtime/runtime_manifest_test.go | 2 +- tests/integration/schema_test.go | 2 +- 9 files changed, 149 insertions(+), 20 deletions(-) create mode 100644 pkg/porter/testdata/porter-image-custom-data-default.yaml create mode 100644 pkg/porter/testdata/porter-image-custom-data-no-default.yaml diff --git a/pkg/cnab/config-adapter/helpers.go b/pkg/cnab/config-adapter/helpers.go index 5e9a6ae2f..cd90b733f 100644 --- a/pkg/cnab/config-adapter/helpers.go +++ b/pkg/cnab/config-adapter/helpers.go @@ -13,7 +13,7 @@ import ( func LoadTestBundle(t *testing.T, config *config.Config, path string) cnab.ExtendedBundle { ctx := context.Background() - m, err := manifest.ReadManifest(config.Context, path, config) + m, err := manifest.ReadManifest(config.Context, path, config, false) require.NoError(t, err) b, err := ConvertToTestBundle(ctx, config, m) require.NoError(t, err) diff --git a/pkg/manifest/manifest.go b/pkg/manifest/manifest.go index e1ecfe46a..a6d3b43b1 100644 --- a/pkg/manifest/manifest.go +++ b/pkg/manifest/manifest.go @@ -1291,7 +1291,7 @@ func ReadManifestData(cxt *portercontext.Context, path string) ([]byte, error) { // ReadManifest determines if specified path is a URL or a filepath. // After reading the data in the path it returns a Manifest and any errors -func ReadManifest(cxt *portercontext.Context, path string, config *config.Config) (*Manifest, error) { +func ReadManifest(cxt *portercontext.Context, path string, config *config.Config, allowMissingVariables bool) (*Manifest, error) { data, err := ReadManifestData(cxt, path) if err != nil { return nil, err @@ -1307,7 +1307,7 @@ func ReadManifest(cxt *portercontext.Context, path string, config *config.Config if err != nil { return nil, fmt.Errorf("failed to marshal image section: %w", err) } - mustache.AllowMissingVariables = false + mustache.AllowMissingVariables = allowMissingVariables imageContext := map[string]interface{}{ "bundle": map[string]interface{}{ "custom": m.Custom, @@ -1444,10 +1444,20 @@ func (m *Manifest) getTemplateVariables(data string) (map[string]struct{}, error // LoadManifestFrom reads and validates the manifest at the specified location, // and returns a populated Manifest structure. func LoadManifestFrom(ctx context.Context, config *config.Config, file string) (*Manifest, error) { + return loadManifestFrom(ctx, config, file, false) +} + +// LoadManifestFromAllowMissingVariables reads and validates the manifest at the specified location, +// and returns a populated Manifest structure, while allowing missing variables. +func LoadManifestFromAllowMissingVariables(ctx context.Context, config *config.Config, file string) (*Manifest, error) { + return loadManifestFrom(ctx, config, file, true) +} + +func loadManifestFrom(ctx context.Context, config *config.Config, file string, allowMissingVariables bool) (*Manifest, error) { ctx, log := tracing.StartSpan(ctx) defer log.EndSpan() - m, err := ReadManifest(config.Context, file, config) + m, err := ReadManifest(config.Context, file, config, allowMissingVariables) if err != nil { return nil, err } diff --git a/pkg/manifest/manifest_test.go b/pkg/manifest/manifest_test.go index 872113c9c..754450c4a 100644 --- a/pkg/manifest/manifest_test.go +++ b/pkg/manifest/manifest_test.go @@ -263,7 +263,7 @@ func TestManifest_Validate_SchemaVersion(t *testing.T) { cfg.TestContext.UseFilesystem() cfg.Data.SchemaCheck = string(schema.CheckStrategyExact) - m, err := ReadManifest(cfg.Context, "testdata/porter.yaml", cfg.Config) + m, err := ReadManifest(cfg.Context, "testdata/porter.yaml", cfg.Config, false) require.NoError(t, err) err = m.Validate(ctx, cfg.Config) @@ -281,7 +281,7 @@ func TestManifest_Validate_SchemaVersion(t *testing.T) { cfg.TestContext.EditYaml("porter.yaml", func(yq *yaml.Editor) error { return yq.SetValue("schemaVersion", "1.1.0") }) - m, err := ReadManifest(cfg.Context, "porter.yaml", cfg.Config) + m, err := ReadManifest(cfg.Context, "porter.yaml", cfg.Config, false) require.NoError(t, err) err = m.Validate(ctx, cfg.Config) @@ -300,7 +300,7 @@ func TestManifest_Validate_SchemaVersion(t *testing.T) { defer span.EndSpan() cfg.Data.SchemaCheck = string(schema.CheckStrategyNone) - m, err := ReadManifest(cfg.Context, "testdata/porter.yaml", cfg.Config) + m, err := ReadManifest(cfg.Context, "testdata/porter.yaml", cfg.Config, false) require.NoError(t, err) m.SchemaVersion = "" @@ -415,7 +415,7 @@ func TestManifest_Validate_WrongSchema(t *testing.T) { func TestReadManifest_URL(t *testing.T) { cxt := portercontext.NewTestContext(t) url := "https://raw.githubusercontent.com/getporter/porter/v0.27.1/pkg/manifest/testdata/simple.porter.yaml" - m, err := ReadManifest(cxt.Context, url, config.NewTestConfig(t).Config) + m, err := ReadManifest(cxt.Context, url, config.NewTestConfig(t).Config, false) require.NoError(t, err) assert.Equal(t, "hello", m.Name) @@ -423,7 +423,7 @@ func TestReadManifest_URL(t *testing.T) { func TestReadManifest_Validate_InvalidURL(t *testing.T) { cxt := portercontext.NewTestContext(t) - _, err := ReadManifest(cxt.Context, "http://fake-example-porter", config.NewTestConfig(t).Config) + _, err := ReadManifest(cxt.Context, "http://fake-example-porter", config.NewTestConfig(t).Config, false) assert.Error(t, err) assert.Regexp(t, "could not reach url http://fake-example-porter", err) @@ -432,7 +432,7 @@ func TestReadManifest_Validate_InvalidURL(t *testing.T) { func TestReadManifest_File(t *testing.T) { cxt := portercontext.NewTestContext(t) cxt.AddTestFile("testdata/simple.porter.yaml", config.Name) - m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config) + m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config, false) require.NoError(t, err) assert.Equal(t, "hello", m.Name) @@ -562,7 +562,7 @@ func TestSetDefaults(t *testing.T) { func TestReadManifest_Validate_MissingFile(t *testing.T) { cxt := portercontext.NewTestContext(t) - _, err := ReadManifest(cxt.Context, "fake-porter.yaml", config.NewTestConfig(t).Config) + _, err := ReadManifest(cxt.Context, "fake-porter.yaml", config.NewTestConfig(t).Config, false) assert.EqualError(t, err, "the specified porter configuration file fake-porter.yaml does not exist") } @@ -570,7 +570,7 @@ func TestReadManifest_Validate_MissingFile(t *testing.T) { func TestMixinDeclaration_UnmarshalYAML(t *testing.T) { cxt := portercontext.NewTestContext(t) cxt.AddTestFile("testdata/mixin-with-config.yaml", config.Name) - m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config) + m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config, false) require.NoError(t, err) assert.Len(t, m.Mixins, 3, "expected 3 mixins") @@ -583,7 +583,7 @@ func TestMixinDeclaration_UnmarshalYAML(t *testing.T) { func TestMixinDeclaration_UnmarshalYAML_Invalid(t *testing.T) { cxt := portercontext.NewTestContext(t) cxt.AddTestFile("testdata/mixin-with-bad-config.yaml", config.Name) - _, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config) + _, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config, false) require.Error(t, err) assert.Contains(t, err.Error(), "mixin declaration contained more than one mixin") @@ -598,7 +598,7 @@ func TestCredentialsDefinition_UnmarshalYAML(t *testing.T) { t.Run("all credentials in the generated manifest file are required", func(t *testing.T) { cxt := portercontext.NewTestContext(t) cxt.AddTestFile("testdata/with-credentials.yaml", config.Name) - m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config) + m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config, false) require.NoError(t, err) assertAllCredentialsRequired(t, m.Credentials) @@ -896,7 +896,7 @@ func TestLoadManifestWithRequiredExtensions(t *testing.T) { func TestReadManifest_WithTemplateVariables(t *testing.T) { cxt := portercontext.NewTestContext(t) cxt.AddTestFile("testdata/porter-with-templating.yaml", config.Name) - m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config) + m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config, false) require.NoError(t, err, "ReadManifest failed") wantVars := []string{"bundle.dependencies.mysql.outputs.mysql-password", "bundle.outputs.msg", "bundle.outputs.name"} assert.Equal(t, wantVars, m.TemplateVariables) @@ -905,7 +905,7 @@ func TestReadManifest_WithTemplateVariables(t *testing.T) { func TestManifest_GetTemplatedOutputs(t *testing.T) { cxt := portercontext.NewTestContext(t) cxt.AddTestFile("testdata/porter-with-templating.yaml", config.Name) - m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config) + m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config, false) require.NoError(t, err, "ReadManifest failed") outputs := m.GetTemplatedOutputs() @@ -917,7 +917,7 @@ func TestManifest_GetTemplatedOutputs(t *testing.T) { func TestManifest_GetTemplatedDependencyOutputs(t *testing.T) { cxt := portercontext.NewTestContext(t) cxt.AddTestFile("testdata/porter-with-templating.yaml", config.Name) - m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config) + m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config, false) require.NoError(t, err, "ReadManifest failed") outputs := m.GetTemplatedDependencyOutputs() diff --git a/pkg/porter/lint.go b/pkg/porter/lint.go index 99ae97754..b55a56549 100644 --- a/pkg/porter/lint.go +++ b/pkg/porter/lint.go @@ -55,7 +55,7 @@ func (o *LintOptions) validateFile(cxt *portercontext.Context) error { // Lint porter.yaml for any problems and report the results. // This calls the mixins to analyze their sections of the manifest. func (p *Porter) Lint(ctx context.Context, opts LintOptions) (linter.Results, error) { - manifest, err := manifest.LoadManifestFrom(ctx, p.Config, opts.File) + manifest, err := manifest.LoadManifestFromAllowMissingVariables(ctx, p.Config, opts.File) if err != nil { return nil, err } diff --git a/pkg/porter/lint_test.go b/pkg/porter/lint_test.go index 0a6b31389..71bb32a31 100644 --- a/pkg/porter/lint_test.go +++ b/pkg/porter/lint_test.go @@ -67,6 +67,50 @@ func TestPorter_Lint(t *testing.T) { assert.Len(t, results, 1, "Lint returned the wrong number of results") } +func TestPorter_LintImageCustomDataWithDefault(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + + p.TestConfig.TestContext.AddTestFile("testdata/porter-image-custom-data-default.yaml", "porter.yaml") + + mixins := p.Mixins.(*mixin.TestMixinProvider) + mixins.LintResults = linter.Results{ + { + Level: linter.LevelError, + }, + } + + var opts LintOptions + err := opts.Validate(p.Context) + require.NoError(t, err, "Validate failed") + + results, err := p.Lint(context.Background(), opts) + require.NoError(t, err, "Lint failed") + assert.Len(t, results, 1, "Lint returned the wrong number of results") +} + +func TestPorter_LintImageCustomDataWithoutDefault(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + + p.TestConfig.TestContext.AddTestFile("testdata/porter-image-custom-data-no-default.yaml", "porter.yaml") + + mixins := p.Mixins.(*mixin.TestMixinProvider) + mixins.LintResults = linter.Results{ + { + Level: linter.LevelError, + }, + } + + var opts LintOptions + err := opts.Validate(p.Context) + require.NoError(t, err, "Validate failed") + + results, err := p.Lint(context.Background(), opts) + require.NoError(t, err, "Lint failed") + assert.Len(t, results, 1, "Lint returned the wrong number of results") +} + func TestPorter_PrintLintResults(t *testing.T) { lintResults := linter.Results{ { diff --git a/pkg/porter/testdata/porter-image-custom-data-default.yaml b/pkg/porter/testdata/porter-image-custom-data-default.yaml new file mode 100644 index 000000000..b6bc75b1d --- /dev/null +++ b/pkg/porter/testdata/porter-image-custom-data-default.yaml @@ -0,0 +1,39 @@ +schemaVersion: 1.0.0 +name: porter-hello +version: 0.1.0 +description: "A bundle with a custom action" +registry: "localhost:5000" + +custom: + digest: "sha256:6b5a28ccbb76f12ce771a23757880c6083234255c5ba191fca1c5db1f71c1687" + +images: + something: + description: "an image" + imageType: "docker" + repository: "getporter/boo" + digest: "${ bundle.custom.digest }" + +mixins: + - exec + +install: + - exec: + description: "Install Hello World" + command: echo + arguments: + - "Hello world" + +upgrade: + - exec: + description: "World 2.0" + command: bash + flags: + c: echo World 2.0 + +uninstall: + - exec: + description: "Uninstall Hello World" + command: bash + flags: + c: echo Goodbye World diff --git a/pkg/porter/testdata/porter-image-custom-data-no-default.yaml b/pkg/porter/testdata/porter-image-custom-data-no-default.yaml new file mode 100644 index 000000000..ffc7b6e84 --- /dev/null +++ b/pkg/porter/testdata/porter-image-custom-data-no-default.yaml @@ -0,0 +1,36 @@ +schemaVersion: 1.0.0 +name: porter-hello +version: 0.1.0 +description: "A bundle with a custom action" +registry: "localhost:5000" + +images: + something: + description: "an image" + imageType: "docker" + repository: "getporter/boo" + digest: "${ bundle.custom.digest }" + +mixins: + - exec + +install: + - exec: + description: "Install Hello World" + command: echo + arguments: + - "Hello world" + +upgrade: + - exec: + description: "World 2.0" + command: bash + flags: + c: echo World 2.0 + +uninstall: + - exec: + description: "Uninstall Hello World" + command: bash + flags: + c: echo Goodbye World diff --git a/pkg/runtime/runtime_manifest_test.go b/pkg/runtime/runtime_manifest_test.go index d56ad0dcf..4f75e8925 100644 --- a/pkg/runtime/runtime_manifest_test.go +++ b/pkg/runtime/runtime_manifest_test.go @@ -24,7 +24,7 @@ import ( func runtimeManifestFromStepYaml(t *testing.T, testConfig *config.TestConfig, stepYaml string) *RuntimeManifest { mContent := []byte(stepYaml) require.NoError(t, testConfig.FileSystem.WriteFile("/cnab/app/porter.yaml", mContent, pkg.FileModeWritable)) - m, err := manifest.ReadManifest(testConfig.Context, "/cnab/app/porter.yaml", testConfig.Config) + m, err := manifest.ReadManifest(testConfig.Context, "/cnab/app/porter.yaml", testConfig.Config, false) require.NoError(t, err, "ReadManifest failed") cfg := NewConfigFor(testConfig.Config) return NewRuntimeManifest(cfg, cnab.ActionInstall, m) diff --git a/tests/integration/schema_test.go b/tests/integration/schema_test.go index 5116a50e0..6eb7c5469 100644 --- a/tests/integration/schema_test.go +++ b/tests/integration/schema_test.go @@ -57,7 +57,7 @@ mixins.2.testmixin: Additional property missingproperty is not allowed`}, t.Run(tm.name, func(t *testing.T) { // Load the manifest as a go dump testManifestPath := tm.path - mani, err := manifest.ReadManifest(test.TestContext.Context, testManifestPath, config.NewTestConfig(t).Config) + mani, err := manifest.ReadManifest(test.TestContext.Context, testManifestPath, config.NewTestConfig(t).Config, false) maniYaml, err := yaml.Marshal(mani) require.NoError(t, err, "error marshaling manifest to yaml")