From 73e514ec55cfa881914fcbd8b5c25fd81c2fe3f2 Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Wed, 6 Jul 2022 16:41:38 -0500 Subject: [PATCH 1/2] Fix yaml tag for dependency version There was a typo in the yaml tag for a dependency's version in porter.yaml. It has always been version, and we intended to just move reference and version underneath a new bundle field. It was accidentally set to versions, but none of the other code, tests or examples were updated to that value (yay!) This reverts the yaml tag back to version (singular). I'm not bumping the schema version since this actually matches what the schema has always been, it's just fixing a bug where version completely didn't work for a bit. Signed-off-by: Carolyn Van Slyck --- pkg/manifest/manifest.go | 2 +- pkg/porter/testdata/schema.json | 2 +- pkg/schema/manifest.schema.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/manifest/manifest.go b/pkg/manifest/manifest.go index 4d8e8c9ca..725ead784 100644 --- a/pkg/manifest/manifest.go +++ b/pkg/manifest/manifest.go @@ -609,7 +609,7 @@ type BundleCriteria struct { // This includes considering prereleases to be invalid if the ranges does not include one. // If you want to have it include pre-releases a simple solution is to include -0 in your range." // https://github.com/Masterminds/semver/blob/master/README.md#checking-version-constraints - Version string `yaml:"versions,omitempty"` + Version string `yaml:"version,omitempty"` } func (d *RequiredDependency) Validate(cxt *portercontext.Context) error { diff --git a/pkg/porter/testdata/schema.json b/pkg/porter/testdata/schema.json index 156f8e49e..1d48618ed 100644 --- a/pkg/porter/testdata/schema.json +++ b/pkg/porter/testdata/schema.json @@ -27,7 +27,7 @@ "type": "string" }, "version": { - "description": "Bundle version contstraint for version matching, see https://github.com/Masterminds/semver/blob/master/README.md#checking-version-constraints", + "description": "Bundle version constraint for version matching, see https://github.com/Masterminds/semver/blob/master/README.md#checking-version-constraints", "type": "string" } }, diff --git a/pkg/schema/manifest.schema.json b/pkg/schema/manifest.schema.json index 3ac81b7bf..c4ac00120 100644 --- a/pkg/schema/manifest.schema.json +++ b/pkg/schema/manifest.schema.json @@ -191,7 +191,7 @@ "type": "string" }, "version": { - "description": "Bundle version contstraint for version matching, see https://github.com/Masterminds/semver/blob/master/README.md#checking-version-constraints", + "description": "Bundle version constraint for version matching, see https://github.com/Masterminds/semver/blob/master/README.md#checking-version-constraints", "type": "string" } }, From 7bc74f9ddfd8aae37dd872aea9fe0ded0b31a29f Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Wed, 6 Jul 2022 16:43:47 -0500 Subject: [PATCH 2/2] Set AllowPrerelease on deps v1 extension When we are converting a porter.yaml to a bundle.json and populating the v1 dependencies extension metadata, we accidentally stopped populating AllowPrerelease (which was in use previously). Before we updated the dependency schema in preparation for a v2 of the dependency extension this value was set explicitly in the porter.yaml. When we removed it, we decided to use the mastermind/semver convention of "if a version constraint includes a prerelease, then we will match against releases", i.e. 1.0.0-0 would allow prereleases, but we missed updating the conversion logic to explicitly set AllowPrerelease in that situation. I've updated our testdata to work with the new logic and verified that we are now properly setting AllowPrerelease again. Signed-off-by: Carolyn Van Slyck --- pkg/cnab/config-adapter/adapter.go | 29 ++++++++++++++----- pkg/cnab/config-adapter/adapter_test.go | 20 ++++++------- .../testdata/porter-with-deps.yaml | 4 +-- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/pkg/cnab/config-adapter/adapter.go b/pkg/cnab/config-adapter/adapter.go index e8a5eb7bc..917a875c6 100644 --- a/pkg/cnab/config-adapter/adapter.go +++ b/pkg/cnab/config-adapter/adapter.go @@ -11,6 +11,7 @@ import ( "get.porter.sh/porter/pkg/manifest" "get.porter.sh/porter/pkg/mixin" "get.porter.sh/porter/pkg/tracing" + "github.com/Masterminds/semver/v3" "github.com/cnabio/cnab-go/bundle" "github.com/cnabio/cnab-go/bundle/definition" ) @@ -71,7 +72,11 @@ func (c *ManifestConverter) ToBundle(ctx context.Context) (cnab.ExtendedBundle, b.Outputs = c.generateBundleOutputs(ctx, &b.Definitions) b.Credentials = c.generateBundleCredentials() b.Images = c.generateBundleImages() - b.Custom = c.generateCustomExtensions(&b) + custom, err := c.generateCustomExtensions(&b) + if err != nil { + return cnab.ExtendedBundle{}, err + } + b.Custom = custom b.RequiredExtensions = c.generateRequiredExtensions(b) b.Custom[config.CustomPorterKey] = stamp @@ -401,10 +406,9 @@ func (c *ManifestConverter) generateBundleImages() map[string]bundle.Image { return images } -func (c *ManifestConverter) generateDependencies() *cnab.Dependencies { - +func (c *ManifestConverter) generateDependencies() (*cnab.Dependencies, error) { if len(c.Manifest.Dependencies.RequiredDependencies) == 0 { - return nil + return nil, nil } deps := &cnab.Dependencies{ @@ -421,12 +425,18 @@ func (c *ManifestConverter) generateDependencies() *cnab.Dependencies { dependencyRef.Version = &cnab.DependencyVersion{ Ranges: []string{dep.Bundle.Version}, } + + // If we can detect that prereleases are used in the version, then set AllowPrereleases to true + v, err := semver.NewVersion(dep.Bundle.Version) + if err == nil { + dependencyRef.Version.AllowPrereleases = v.Prerelease() != "" + } } deps.Sequence = append(deps.Sequence, dep.Name) deps.Requires[dep.Name] = dependencyRef } - return deps + return deps, nil } func (c *ManifestConverter) generateParameterSources(b *cnab.ExtendedBundle) cnab.ParameterSources { @@ -576,7 +586,7 @@ func toFloat(v float64) *float64 { return &v } -func (c *ManifestConverter) generateCustomExtensions(b *cnab.ExtendedBundle) map[string]interface{} { +func (c *ManifestConverter) generateCustomExtensions(b *cnab.ExtendedBundle) (map[string]interface{}, error) { customExtensions := map[string]interface{}{ cnab.FileParameterExtensionKey: struct{}{}, } @@ -587,7 +597,10 @@ func (c *ManifestConverter) generateCustomExtensions(b *cnab.ExtendedBundle) map } // Add the dependency extension - deps := c.generateDependencies() + deps, err := c.generateDependencies() + if err != nil { + return nil, err + } if deps != nil && len(deps.Requires) > 0 { customExtensions[cnab.DependenciesExtensionKey] = deps } @@ -603,7 +616,7 @@ func (c *ManifestConverter) generateCustomExtensions(b *cnab.ExtendedBundle) map customExtensions[lookupExtensionKey(ext.Name)] = ext.Config } - return customExtensions + return customExtensions, nil } func (c *ManifestConverter) generateRequiredExtensions(b cnab.ExtendedBundle) []string { diff --git a/pkg/cnab/config-adapter/adapter_test.go b/pkg/cnab/config-adapter/adapter_test.go index 67174c054..208f36261 100644 --- a/pkg/cnab/config-adapter/adapter_test.go +++ b/pkg/cnab/config-adapter/adapter_test.go @@ -544,11 +544,12 @@ func TestManifestConverter_generateDependencies(t *testing.T) { Name: "mysql", Bundle: "getporter/azure-mysql:5.7", }}, - {"no-ranges", cnab.Dependency{ + {"no-ranges, uses prerelease", cnab.Dependency{ Name: "ad", Bundle: "getporter/azure-active-directory", Version: &cnab.DependencyVersion{ AllowPrereleases: true, + Ranges: []string{"1.0.0-0"}, }, }}, {"with-ranges", cnab.Dependency{ @@ -556,21 +557,17 @@ func TestManifestConverter_generateDependencies(t *testing.T) { Bundle: "getporter/azure-blob-storage", Version: &cnab.DependencyVersion{ Ranges: []string{ - "1.x - 2", - "2.1 - 3.x", + "1.x - 2,2.1 - 3.x", }, }, }}, - {"with-tag", cnab.Dependency{ - Name: "dep-with-tag", - Bundle: "getporter/dep-bun:v0.1.0", - }}, } for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { t.Parallel() - tc := tc c := config.NewTestConfig(t) c.TestContext.AddTestFile("testdata/porter-with-deps.yaml", config.Name) @@ -581,9 +578,10 @@ func TestManifestConverter_generateDependencies(t *testing.T) { a := NewManifestConverter(c.Config, m, nil, nil) - deps := a.generateDependencies() - require.Len(t, deps.Requires, 4, "incorrect number of dependencies were generated") - require.Equal(t, []string{"mysql", "ad", "storage", "dep-with-tag"}, deps.Sequence, "incorrect sequence was generated") + deps, err := a.generateDependencies() + require.NoError(t, err, "generateDependencies failed") + require.Len(t, deps.Requires, 3, "incorrect number of dependencies were generated") + require.Equal(t, []string{"mysql", "ad", "storage"}, deps.Sequence, "incorrect sequence was generated") var dep *cnab.Dependency for _, d := range deps.Requires { diff --git a/pkg/cnab/config-adapter/testdata/porter-with-deps.yaml b/pkg/cnab/config-adapter/testdata/porter-with-deps.yaml index b738bff2b..0590b59c2 100644 --- a/pkg/cnab/config-adapter/testdata/porter-with-deps.yaml +++ b/pkg/cnab/config-adapter/testdata/porter-with-deps.yaml @@ -12,13 +12,11 @@ dependencies: - name: ad bundle: reference: "getporter/azure-active-directory" + version: 1.0.0-0 - name: storage bundle: reference: "getporter/azure-blob-storage" version: 1.x - 2,2.1 - 3.x - - name: dep-with-tag - bundle: - reference: "getporter/dep-bun:v0.1.0" mixins: - exec