Skip to content

Commit

Permalink
Merge branch 'main' into signBundlesPublishedFromArchives
Browse files Browse the repository at this point in the history
  • Loading branch information
kichristensen committed Sep 24, 2024
2 parents be8ac58 + 7538008 commit d38a546
Show file tree
Hide file tree
Showing 8 changed files with 251 additions and 7 deletions.
38 changes: 38 additions & 0 deletions pkg/linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"get.porter.sh/porter/pkg/portercontext"
"get.porter.sh/porter/pkg/tracing"
"get.porter.sh/porter/pkg/yaml"
"github.com/Masterminds/semver/v3"
"github.com/dustin/go-humanize"
)

Expand Down Expand Up @@ -264,9 +265,46 @@ func (l *Linter) Lint(ctx context.Context, m *manifest.Manifest, config *config.
results = append(results, r...)
}

span.Debug("Getting versions for each mixin used in the manifest...")
err = l.validateVersionNumberConstraints(ctx, m)
if err != nil {
return nil, span.Error(err)
}

return results, nil
}

func (l *Linter) validateVersionNumberConstraints(ctx context.Context, m *manifest.Manifest) error {
for _, mixin := range m.Mixins {
if mixin.Version != nil {
installedMeta, err := l.Mixins.GetMetadata(ctx, mixin.Name)
if err != nil {
return fmt.Errorf("unable to get metadata from mixin %s: %w", mixin.Name, err)
}
installedVersion := installedMeta.GetVersionInfo().Version

err = validateSemverConstraint(mixin.Name, installedVersion, mixin.Version)
if err != nil {
return err
}
}
}

return nil
}

func validateSemverConstraint(name string, installedVersion string, versionConstraint *semver.Constraints) error {
v, err := semver.NewVersion(installedVersion)
if err != nil {
return fmt.Errorf("invalid version number from mixin %s: %s. %w", name, installedVersion, err)
}

if !versionConstraint.Check(v) {
return fmt.Errorf("mixin %s is installed at version %s but your bundle requires version %s", name, installedVersion, versionConstraint)
}
return nil
}

func validateParamsAppliesToAction(m *manifest.Manifest, steps manifest.Steps, tmplParams manifest.ParameterDefinitions, actionName string, config *config.Config) (Results, error) {
var results Results
for stepNumber, step := range steps {
Expand Down
105 changes: 105 additions & 0 deletions pkg/linter/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import (
"get.porter.sh/porter/pkg/config"
"get.porter.sh/porter/pkg/manifest"
"get.porter.sh/porter/pkg/mixin"
"get.porter.sh/porter/pkg/pkgmgmt"
"get.porter.sh/porter/pkg/portercontext"
"get.porter.sh/porter/tests"
"github.com/Masterminds/semver/v3"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -363,3 +366,105 @@ func TestLinter_DependencyMultipleTimes(t *testing.T) {
require.Len(t, results, 0, "linter should have returned 0 result")
})
}

func TestLinter_Lint_MissingMixin(t *testing.T) {
cxt := portercontext.NewTestContext(t)
mixins := mixin.NewTestMixinProvider()
l := New(cxt.Context, mixins)
testConfig := config.NewTestConfig(t).Config

mixinName := "made-up-mixin-that-is-not-installed"

m := &manifest.Manifest{
Mixins: []manifest.MixinDeclaration{
{
Name: mixinName,
},
},
}

mixins.RunAssertions = append(mixins.RunAssertions, func(mixinCxt *portercontext.Context, mixinName string, commandOpts pkgmgmt.CommandOptions) error {
return fmt.Errorf("%s not installed", mixinName)
})

_, err := l.Lint(context.Background(), m, testConfig)
require.Error(t, err, "Linting should return an error")
tests.RequireOutputContains(t, err.Error(), fmt.Sprintf("%s is not currently installed", mixinName))
}

func TestLinter_Lint_MixinVersions(t *testing.T) {
cxt := portercontext.NewTestContext(t)
mixinProvider := mixin.NewTestMixinProvider()
l := New(cxt.Context, mixinProvider)
testConfig := config.NewTestConfig(t).Config

exampleMixinVersion := mixin.ExampleMixinSemver.String()

// build up some test semvers
patchDifferenceSemver := fmt.Sprintf("%d.%d.%d", mixin.ExampleMixinSemver.Major(), mixin.ExampleMixinSemver.Minor(), mixin.ExampleMixinSemver.Patch()+1)
anyPatchAccepted := fmt.Sprintf("%d.%d.x", mixin.ExampleMixinSemver.Major(), mixin.ExampleMixinSemver.Minor())
lessThanNextMajor := fmt.Sprintf("<%d.%d", mixin.ExampleMixinSemver.Major()+1, mixin.ExampleMixinSemver.Minor())

exampleMixinVersionConstraint, _ := semver.NewConstraint(exampleMixinVersion)
patchDifferenceSemverConstraint, _ := semver.NewConstraint(patchDifferenceSemver)
anyPatchAcceptedConstraint, _ := semver.NewConstraint(anyPatchAccepted)
lessThanNextMajorConstraint, _ := semver.NewConstraint(lessThanNextMajor)

testCases := []struct {
name string
errExpected bool
mixins []manifest.MixinDeclaration
}{
{"exact-semver", false, []manifest.MixinDeclaration{
{
Name: mixin.ExampleMixinName,
Version: exampleMixinVersionConstraint,
},
}},
{"different-patch", true, []manifest.MixinDeclaration{
{
Name: mixin.ExampleMixinName,
Version: patchDifferenceSemverConstraint,
},
}},
{"accept-different-patch", false, []manifest.MixinDeclaration{
{
Name: mixin.ExampleMixinName,
Version: anyPatchAcceptedConstraint,
},
}},
{"accept-less-than-versions", false, []manifest.MixinDeclaration{
{
Name: mixin.ExampleMixinName,
Version: lessThanNextMajorConstraint,
},
}},
{"no-version-provided", false, []manifest.MixinDeclaration{
{
Name: mixin.ExampleMixinName,
},
}},
}

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
m := &manifest.Manifest{
Mixins: testCase.mixins,
}
results, err := l.Lint(context.Background(), m, testConfig)
if testCase.errExpected {
require.Error(t, err, "Linting should return an error")
tests.RequireOutputContains(t, err.Error(), fmt.Sprintf(
"mixin %s is installed at version v%s but your bundle requires version %s",
mixin.ExampleMixinName,
exampleMixinVersion,
testCase.mixins[0].Version.String(),
))
} else {
require.NoError(t, err, "Linting should not return an error")
}
require.Len(t, results, 0, "linter should have returned 0 result")
})
}

}
48 changes: 44 additions & 4 deletions pkg/manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -640,8 +640,30 @@ func (l Location) IsEmpty() bool {
}

type MixinDeclaration struct {
Name string
Config interface{}
Name string
Version *semver.Constraints
Config interface{}
}

func extractVersionFromName(name string) (string, *semver.Constraints, error) {
parts := strings.Split(name, "@")

// if there isn't a version in the name, just stop!
if len(parts) == 1 {
return name, nil, nil
}

// if we somehow got more parts than expected!
if len(parts) != 2 {
return "", nil, fmt.Errorf("expected name@version, got: %s", name)
}

version, err := semver.NewConstraint(parts[1])
if err != nil {
return "", nil, err
}

return parts[0], version, nil
}

// UnmarshalYAML allows mixin declarations to either be a normal list of strings
Expand All @@ -652,12 +674,25 @@ type MixinDeclaration struct {
// - az:
// extensions:
// - iot
//
// for each type, we can optionally support a version number in the name field
// mixins:
// - exec@2.1.1
// or
// - az@2.1.1
// extensions:
// - iot
func (m *MixinDeclaration) UnmarshalYAML(unmarshal func(interface{}) error) error {
// First try to just read the mixin name
var mixinNameOnly string
err := unmarshal(&mixinNameOnly)
if err == nil {
m.Name = mixinNameOnly
name, version, err := extractVersionFromName(mixinNameOnly)
if err != nil {
return fmt.Errorf("invalid mixin name/version: %w", err)
}
m.Name = name
m.Version = version
m.Config = nil
return nil
}
Expand All @@ -676,7 +711,12 @@ func (m *MixinDeclaration) UnmarshalYAML(unmarshal func(interface{}) error) erro
}

for mixinName, config := range mixinWithConfig {
m.Name = mixinName
name, version, err := extractVersionFromName(mixinName)
if err != nil {
return fmt.Errorf("invalid mixin name/version: %w", err)
}
m.Name = name
m.Version = version
m.Config = config
break // There is only one mixin anyway but break for clarity
}
Expand Down
38 changes: 38 additions & 0 deletions pkg/manifest/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"get.porter.sh/porter/pkg/portercontext"
"get.porter.sh/porter/pkg/schema"
"get.porter.sh/porter/pkg/yaml"
"github.com/Masterminds/semver/v3"
"github.com/cnabio/cnab-go/bundle/definition"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -589,6 +590,43 @@ func TestMixinDeclaration_UnmarshalYAML_Invalid(t *testing.T) {
assert.Contains(t, err.Error(), "mixin declaration contained more than one mixin")
}

func TestMixinDeclaration_UnmarshalYAML_Versions(t *testing.T) {
cxt := portercontext.NewTestContext(t)
cxt.AddTestFile("testdata/mixin-with-versions.yaml", config.Name)
m, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config)

execVersion, _ := semver.NewConstraint("1")
axVersion, _ := semver.NewConstraint("1.1.X")
terraformVersoin, _ := semver.NewConstraint(">=2")

require.NoError(t, err)
assert.Len(t, m.Mixins, 3, "expected 3 mixins")
assert.Equal(t, "exec", m.Mixins[0].Name)
assert.Equal(t, "az", m.Mixins[1].Name)
assert.Equal(t, "terraform", m.Mixins[2].Name)
assert.Equal(t, execVersion, m.Mixins[0].Version)
assert.Equal(t, axVersion, m.Mixins[1].Version)
assert.Equal(t, terraformVersoin, m.Mixins[2].Version)
}

func TestMixinDeclaration_UnmarshalYAML_Versions_Empty(t *testing.T) {
cxt := portercontext.NewTestContext(t)
cxt.AddTestFile("testdata/mixin-with-empty-version.yaml", config.Name)
_, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config)

require.Error(t, err)
assert.Contains(t, err.Error(), "invalid mixin name/version: improper constraint:")
}

func TestMixinDeclaration_UnmarshalYAML_Versions_Invalid(t *testing.T) {
cxt := portercontext.NewTestContext(t)
cxt.AddTestFile("testdata/mixin-with-invalid-version.yaml", config.Name)
_, err := ReadManifest(cxt.Context, config.Name, config.NewTestConfig(t).Config)

require.Error(t, err)
assert.Contains(t, err.Error(), "invalid mixin name/version: expected name@version, got: az@this@that")
}

func TestCredentialsDefinition_UnmarshalYAML(t *testing.T) {
assertAllCredentialsRequired := func(t *testing.T, creds CredentialDefinitions) {
for _, cred := range creds {
Expand Down
6 changes: 6 additions & 0 deletions pkg/manifest/testdata/mixin-with-empty-version.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
mixins:
- exec@
- az:
extensions:
- iot
- terraform
6 changes: 6 additions & 0 deletions pkg/manifest/testdata/mixin-with-invalid-version.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
mixins:
- exec
- az@this@that:
extensions:
- iot
- terraform
6 changes: 6 additions & 0 deletions pkg/manifest/testdata/mixin-with-versions.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
mixins:
- exec@1
- az@1.1.X:
extensions:
- iot
- terraform@>=2
11 changes: 8 additions & 3 deletions pkg/mixin/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"get.porter.sh/porter/pkg/pkgmgmt"
"get.porter.sh/porter/pkg/pkgmgmt/client"
"get.porter.sh/porter/pkg/portercontext"
"github.com/Masterminds/semver/v3"
)

type TestMixinProvider struct {
Expand All @@ -24,6 +25,10 @@ type TestMixinProvider struct {
ReturnBuildError bool
}

// hoist these into variables so tests can reference them safely
var ExampleMixinName = "testmixin"
var ExampleMixinSemver = semver.New(0, 1, 0, "", "")

// NewTestMixinProvider helps us test Porter.Mixins in our unit tests without actually hitting any real plugins on the file system.
func NewTestMixinProvider() *TestMixinProvider {
packages := []pkgmgmt.PackageMetadata{
Expand All @@ -36,9 +41,9 @@ func NewTestMixinProvider() *TestMixinProvider {
},
},
&Metadata{
Name: "testmixin",
Name: ExampleMixinName,
VersionInfo: pkgmgmt.VersionInfo{
Version: "v0.1.0",
Version: fmt.Sprintf("v%s", ExampleMixinSemver.String()),
Commit: "abc123",
Author: "Porter Authors",
},
Expand Down Expand Up @@ -78,7 +83,7 @@ func (p *TestMixinProvider) GetSchema(ctx context.Context, name string) (string,
switch name {
case "exec":
schemaFile = "../exec/schema/exec.json"
case "testmixin":
case ExampleMixinName:
schemaFile = "../../cmd/testmixin/schema.json"
default:
return "", nil
Expand Down

0 comments on commit d38a546

Please sign in to comment.