Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add optional [bundle-name] for porter create [bundle-name] #2892

Merged
merged 13 commits into from
Sep 6, 2023
Merged

add optional [bundle-name] for porter create [bundle-name] #2892

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1c81209
adding functionality to port create in a specified directory
sbdtu5498 Jun 9, 2023
ecea4ef
add warning when overwriting file + fix comments
ludfjig Sep 1, 2023
851b923
fixes
ludfjig Sep 1, 2023
3771be2
fix
ludfjig Sep 1, 2023
4f10522
forgot this file
ludfjig Sep 1, 2023
f7f2395
fix
ludfjig Sep 1, 2023
60a9ce0
fixed tests
ludfjig Sep 5, 2023
7fa75da
feedback
ludfjig Sep 5, 2023
9a6666d
fixed unittest
ludfjig Sep 5, 2023
f5c75d2
added test for nested name
ludfjig Sep 5, 2023
bc4b1b0
updated to use the porter io writer for stderr
ludfjig Sep 5, 2023
a764dbd
fixed perm code + test
ludfjig Sep 5, 2023
dbfac3b
feedback
ludfjig Sep 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions cmd/porter/bundle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,18 @@ func buildBundleCommands(p *porter.Porter) *cobra.Command {

func buildBundleCreateCommand(p *porter.Porter) *cobra.Command {
return &cobra.Command{
Use: "create",
Use: "create [bundle-name]",
Short: "Create a bundle",
Long: "Create a bundle. This generates a porter bundle in the current directory.",
Long: "Create a bundle. This command creates a new porter bundle with the specified bundle-name, in the directory with the specified bundle-name. If no bundle-name is provided, the bundle will be created in current directory and the bundle name will be 'porter-hello'.",
Args: cobra.MaximumNArgs(1), // Expect at most one argument for the bundle name
RunE: func(cmd *cobra.Command, args []string) error {
return p.Create()
// By default we create the bundle in the current directory
bundleName := ""
schristoff marked this conversation as resolved.
Show resolved Hide resolved
if len(args) > 0 {
ludfjig marked this conversation as resolved.
Show resolved Hide resolved
bundleName = args[0]
}

return p.Create(bundleName)
},
}
}
Expand Down
4 changes: 2 additions & 2 deletions docs/content/references/cli/bundles_create.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ Create a bundle

### Synopsis

Create a bundle. This generates a porter bundle in the current directory.
Create a bundle. This generates a porter bundle in the directory with the specified name or in the current directory if no name is provided.

```
porter bundles create [flags]
porter bundles create [bundle-name] [flags]
```

### Options
Expand Down
4 changes: 2 additions & 2 deletions docs/content/references/cli/create.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ Create a bundle

### Synopsis

Create a bundle. This generates a porter bundle in the current directory.
Create a bundle. This generates a porter bundle in the directory with the specified name or in the current directory if no name is provided.

```
porter create [flags]
porter create [bundle-name] [flags]
```

### Options
Expand Down
4 changes: 2 additions & 2 deletions pkg/porter/build_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func TestPorter_LintDuringBuild(t *testing.T) {
testMixins := p.Mixins.(*mixin.TestMixinProvider)
testMixins.LintResults = lintResults

err := p.Create()
err := p.Create("mybundle")
require.NoError(t, err, "Create failed")

opts := BuildOptions{NoLint: false}
Expand All @@ -163,7 +163,7 @@ func TestPorter_LintDuringBuild(t *testing.T) {
testMixins := p.Mixins.(*mixin.TestMixinProvider)
testMixins.LintResults = lintResults

err := p.Create()
err := p.Create("mybundle")
require.NoError(t, err, "Create failed")

opts := BuildOptions{NoLint: true}
Expand Down
49 changes: 41 additions & 8 deletions pkg/porter/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,43 +1,73 @@
package porter

import (
"errors"
"fmt"
"os"
"path/filepath"
"strings"

"get.porter.sh/porter/pkg"
"get.porter.sh/porter/pkg/config"
)

func (p *Porter) Create() error {
fmt.Fprintln(p.Out, "creating porter configuration in the current directory")
// Create creates a new bundle configuration with the specified bundleName. A directory with the given bundleName will be created if it does not already exist.
// If bundleName is the empty string, the configuration will be created in the current directory, and the name will be "porter-hello".
func (p *Porter) Create(bundleName string) error {
// Normalize the bundleName by removing trailing slashes
bundleName = strings.TrimSuffix(bundleName, "/")

err := p.CopyTemplate(p.Templates.GetManifest, config.Name)
// If given a bundleName, create directory if it doesn't exist
ludfjig marked this conversation as resolved.
Show resolved Hide resolved
if bundleName != "" {
_, err := os.Stat(bundleName)
if err != nil && errors.Is(err, os.ErrNotExist) {
err = os.Mkdir(bundleName, os.ModePerm)
ludfjig marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your error is not nil so you don’t have to check it again. Your logic above can be changed to check if it is nil and the. Check if it is the error you are looking for. If it is still nil, you can fall through to the not nil case and not re check the error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies to line 23

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something like this?

	if err != nil {
		if errors.Is(err, os.ErrNotExist) {
			err = os.MkdirAll(dir, os.ModePerm)
		}
		if err != nil {
			return fmt.Errorf("failed to create directory for bundle: %w", err)
		}
	}

return fmt.Errorf("failed to create directory for bundle: %w", err)
}
}
}

var err error
if bundleName == "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your logic is repeating itself if it falls through the bundleName case being empty. Returning early with nil would make this check implicit if it isn’t filled out here.

The logic is the same in both cases it’s just where it gets executed. This can make it easier to understand and less things to write.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hopefully this is addressed now that it's 2 different functions

// create bundle with default name "porter_hello"
err = p.CopyTemplate(p.Templates.GetManifest, filepath.Join(bundleName, config.Name))
} else {
// create bundle with given name
err = p.CopyTemplate(func() ([]byte, error) {
content, err := p.Templates.GetManifest()
if err != nil {
return nil, err
}
content = []byte(strings.ReplaceAll(string(content), "porter-hello", bundleName))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we removing the porter-hello hardcoded?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, the porter-hello will be replaced

return content, nil
}, filepath.Join(bundleName, config.Name))
}
if err != nil {
return err
}

err = p.CopyTemplate(p.Templates.GetManifestHelpers, "helpers.sh")
err = p.CopyTemplate(p.Templates.GetManifestHelpers, filepath.Join(bundleName, "helpers.sh"))
if err != nil {
return err
}

err = p.CopyTemplate(p.Templates.GetReadme, "README.md")
err = p.CopyTemplate(p.Templates.GetReadme, filepath.Join(bundleName, "README.md"))
if err != nil {
return err
}

err = p.CopyTemplate(p.Templates.GetDockerfileTemplate, "template.Dockerfile")
err = p.CopyTemplate(p.Templates.GetDockerfileTemplate, filepath.Join(bundleName, "template.Dockerfile"))
if err != nil {
return err
}

err = p.CopyTemplate(p.Templates.GetDockerignore, ".dockerignore")
err = p.CopyTemplate(p.Templates.GetDockerignore, filepath.Join(bundleName, ".dockerignore"))
if err != nil {
return err
}

return p.CopyTemplate(p.Templates.GetGitignore, ".gitignore")
return p.CopyTemplate(p.Templates.GetGitignore, filepath.Join(bundleName, ".gitignore"))
}

func (p *Porter) CopyTemplate(getTemplate func() ([]byte, error), dest string) error {
Expand All @@ -51,6 +81,9 @@ func (p *Porter) CopyTemplate(getTemplate func() ([]byte, error), dest string) e
mode = pkg.FileModeExecutable
}

if _, err := os.Stat(dest); err == nil {
fmt.Fprintf(os.Stderr, "WARNING: File %q already exists. Overwriting.\n", dest)
}
err = p.FileSystem.WriteFile(dest, tmpl, mode)
if err != nil {
return fmt.Errorf("failed to write template to %s: %w", dest, err)
Expand Down
50 changes: 47 additions & 3 deletions pkg/porter/create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,29 @@
package porter

import (
"get.porter.sh/porter/pkg/manifest"
"get.porter.sh/porter/pkg/yaml"
"github.com/stretchr/testify/assert"
"path/filepath"
"testing"

"get.porter.sh/porter/pkg"
"get.porter.sh/porter/tests"
"github.com/stretchr/testify/require"
)

func TestCreate(t *testing.T) {
func TestCreateInWorkingDirectory(t *testing.T) {
p := NewTestPorter(t)
defer p.Close()

err := p.Create()
err := p.Create("")
require.NoError(t, err)

// Verify that files are present in the root directory
configFileStats, err := p.FileSystem.Stat("porter.yaml")
require.NoError(t, err)
tests.AssertFilePermissionsEqual(t, "porter.yaml", pkg.FileModeWritable, configFileStats.Mode())

// Verify that helpers is present and executable
helperFileStats, err := p.FileSystem.Stat("helpers.sh")
require.NoError(t, err)
tests.AssertFilePermissionsEqual(t, "helpers.sh", pkg.FileModeExecutable, helperFileStats.Mode())
Expand All @@ -39,5 +43,45 @@ func TestCreate(t *testing.T) {
dockerignoreStats, err := p.FileSystem.Stat(".dockerignore")
require.NoError(t, err)
tests.AssertFilePermissionsEqual(t, ".dockerignore", pkg.FileModeWritable, dockerignoreStats.Mode())
}

// tests to ensure behavior similarity with helm create
func TestCreateWithBundleName(t *testing.T) {
bundleName := "mybundle"

p := NewTestPorter(t)
err := p.Create(bundleName)
require.NoError(t, err)

// Verify that files are present in the "mybundle" directory
configFileStats, err := p.FileSystem.Stat(filepath.Join(bundleName, "porter.yaml"))
require.NoError(t, err)
tests.AssertFilePermissionsEqual(t, filepath.Join(bundleName, "porter.yaml"), pkg.FileModeWritable, configFileStats.Mode())

helperFileStats, err := p.FileSystem.Stat(filepath.Join(bundleName, "helpers.sh"))
require.NoError(t, err)
tests.AssertFilePermissionsEqual(t, filepath.Join(bundleName, "helpers.sh"), pkg.FileModeExecutable, helperFileStats.Mode())

dockerfileStats, err := p.FileSystem.Stat(filepath.Join(bundleName, "template.Dockerfile"))
require.NoError(t, err)
tests.AssertFilePermissionsEqual(t, filepath.Join(bundleName, "template.Dockerfile"), pkg.FileModeWritable, dockerfileStats.Mode())

readmeStats, err := p.FileSystem.Stat(filepath.Join(bundleName, "README.md"))
require.NoError(t, err)
tests.AssertFilePermissionsEqual(t, filepath.Join(bundleName, "README.md"), pkg.FileModeWritable, readmeStats.Mode())

gitignoreStats, err := p.FileSystem.Stat(filepath.Join(bundleName, ".gitignore"))
require.NoError(t, err)
tests.AssertFilePermissionsEqual(t, filepath.Join(bundleName, ".gitignore"), pkg.FileModeWritable, gitignoreStats.Mode())

dockerignoreStats, err := p.FileSystem.Stat(filepath.Join(bundleName, ".dockerignore"))
require.NoError(t, err)
tests.AssertFilePermissionsEqual(t, filepath.Join(bundleName, ".dockerignore"), pkg.FileModeWritable, dockerignoreStats.Mode())

// verify "name" inside porter.yaml is set to "mybundle"
porterYaml := &manifest.Manifest{}
data, err := p.FileSystem.ReadFile(filepath.Join(bundleName, "porter.yaml"))
assert.NoError(t, err)
assert.NoError(t, yaml.Unmarshal(data, &porterYaml))
assert.True(t, porterYaml.Name == bundleName)
}
2 changes: 1 addition & 1 deletion pkg/porter/lifecycle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestInstallFromTagIgnoresCurrentBundle(t *testing.T) {
p := NewTestPorter(t)
defer p.Close()

err := p.Create()
err := p.Create("")
require.NoError(t, err)

installOpts := NewInstallOptions()
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/invoke_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestInvokeCustomAction(t *testing.T) {
ctx := p.SetupIntegrationTest()

// Install a bundle with a custom action defined
err := p.Create()
err := p.Create("")
require.NoError(t, err)

bundleName := p.AddTestBundleDir("testdata/bundles/bundle-with-custom-action", true)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/outputs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func CleanupCurrentBundle(ctx context.Context, p *porter.TestPorter) {
}

func installExecOutputsBundle(ctx context.Context, p *porter.TestPorter) string {
err := p.Create()
err := p.Create("")
require.NoError(p.T(), err)

bundleName := p.AddTestBundleDir("testdata/bundles/exec-outputs", true)
Expand Down
Loading