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.

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 --devfile flag support #3118

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
7b39137
feat: add --devfile flag support
GeekArthur May 7, 2020
4f12152
fix: fix gosec G307 issue
GeekArthur May 7, 2020
301c3e6
fix: update error handling
GeekArthur May 7, 2020
ab94c3b
style: Update help and error messages
GeekArthur May 8, 2020
5b87a60
fix: Improve performance of error handling
GeekArthur May 8, 2020
ed1fbf3
style: Improve error message
GeekArthur May 9, 2020
e2118c3
fix: Fix remove file error on Windows
GeekArthur May 9, 2020
ab43b6d
fix: Use existing structure to handle temp file
GeekArthur May 9, 2020
dba1b4b
refactor: rebase onto master
GeekArthur May 12, 2020
b1a599d
refactor: improve code readability and structure
GeekArthur May 12, 2020
613dfae
docs: update variable comments
GeekArthur May 12, 2020
1475068
feat: --devfile value supports http(s)
GeekArthur May 12, 2020
520de2c
feat: remove --devfile for all other commands
GeekArthur May 12, 2020
4841a29
refactor: rebase onto master
GeekArthur May 20, 2020
e21b2c3
feat: add new design for existing devfile support
GeekArthur May 20, 2020
9a8dbcf
test: update TestCopyFile test cases
GeekArthur May 20, 2020
30e9db7
test: refactor file copy util function for test
GeekArthur May 20, 2020
4d42536
test: fix TestCopyFile test cases
GeekArthur May 20, 2020
9eb7f70
test: add integration tests for existing devfile
GeekArthur May 21, 2020
eb74657
refactor: rebase onto master
GeekArthur May 22, 2020
4583b7f
fix: gosec issue
GeekArthur May 22, 2020
20206d7
test: fix docker url integration tests
GeekArthur May 22, 2020
701a4ab
fix: context support
GeekArthur May 22, 2020
91b7537
test: fix docker url integration tests
GeekArthur May 22, 2020
c0dc7d7
fix: handle edge cases and add related tests
GeekArthur May 26, 2020
4bc977f
refactor: separate spinner for existing devfile
GeekArthur May 27, 2020
8d4a336
refactor: rebase onto master
GeekArthur May 28, 2020
6cff5e7
test: fix docker watch tests
GeekArthur May 29, 2020
20235df
refactor: rebase onto master
GeekArthur Jun 1, 2020
a770c83
Merge branch 'master' of github.com:openshift/odo into issue-2862-dev…
GeekArthur Jun 2, 2020
c2ae87c
test: fix url tests
GeekArthur Jun 2, 2020
5f9240a
Merge branch 'master' of github.com:openshift/odo into issue-2862-dev…
GeekArthur Jun 3, 2020
69ad017
Merge branch 'master' of github.com:openshift/odo into issue-2862-dev…
GeekArthur Jun 5, 2020
7ac65f2
Merge branch 'master' of github.com:openshift/odo into issue-2862-dev…
GeekArthur Jun 8, 2020
30a95c4
Merge branch 'master' of github.com:openshift/odo into issue-2862-dev…
GeekArthur Jun 9, 2020
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
2 changes: 1 addition & 1 deletion docs/dev/experimental-mode.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ Applying URL changes
✓ Changes successfully pushed to component
```

By default, `odo push` executes `devbuild` and `devrun` devfile commands. However, custom commands can be also be provided to `odo push` via flags `--build-command` & `--run-command`. `odo push` also provides a `--devfile` flag to execute push with a `devfile.yaml` in a custom path. These flags are only available in the experimental mode.
By default, `odo push` executes `devbuild` and `devrun` devfile commands. However, custom commands can be also be provided to `odo push` via flags `--build-command` & `--run-command`. These flags are only available in the experimental mode.
Copy link
Member

Choose a reason for hiding this comment

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

👍


##### Deleting a devfile component:
Delete a devfile component with `odo delete`. This deletes all the Kubernetes resources created during `odo push`. Use the `-all` flag to delete the Kubernetes resources and the local config at `.odo/env/env.yaml`
Expand Down
305 changes: 201 additions & 104 deletions pkg/odo/cli/component/create.go

Large diffs are not rendered by default.

7 changes: 1 addition & 6 deletions pkg/odo/cli/component/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func NewDeleteOptions() *DeleteOptions {

// Complete completes log args
func (do *DeleteOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) {
do.devfilePath = filepath.Join(do.componentContext, do.devfilePath)
do.devfilePath = filepath.Join(do.componentContext, DevfilePath)
johnmcollier marked this conversation as resolved.
Show resolved Hide resolved

// if experimental mode is enabled and devfile is present
if experimental.IsExperimentalModeEnabled() && util.CheckPathExists(do.devfilePath) {
Expand Down Expand Up @@ -260,11 +260,6 @@ func NewCmdDelete(name, fullName string) *cobra.Command {
},
}

// enable devfile flag if experimental mode is enabled
if experimental.IsExperimentalModeEnabled() {
componentDeleteCmd.Flags().StringVar(&do.devfilePath, "devfile", "./devfile.yaml", "Path to a devfile.yaml")
}

componentDeleteCmd.Flags().BoolVarP(&do.componentForceDeleteFlag, "force", "f", false, "Delete component without prompting")
componentDeleteCmd.Flags().BoolVarP(&do.componentDeleteAllFlag, "all", "a", false, "Delete component and local config")
componentDeleteCmd.Flags().BoolVarP(&do.componentDeleteWaitFlag, "wait", "w", false, "Wait for complete deletion of component and its dependent")
Expand Down
4 changes: 2 additions & 2 deletions pkg/odo/cli/component/devfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ use of Che devfiles in odo for performing various odo operations.
The devfile support progress can be tracked by:
https://github.com/openshift/odo/issues/2467

Please note that this feature is currently under development and the "--devfile"
flag is exposed only if the experimental mode in odo is enabled.
Please note that this feature is currently under development,
the feature will be available with experimental mode enabled.

The behaviour of this feature is subject to change as development for this
feature progresses.
Expand Down
4 changes: 1 addition & 3 deletions pkg/odo/cli/component/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ func NewPushOptions() *PushOptions {

// Complete completes push args
func (po *PushOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) {
po.DevfilePath = filepath.Join(po.componentContext, po.DevfilePath)

po.DevfilePath = filepath.Join(po.componentContext, DevfilePath)
// if experimental mode is enabled and devfile is present
if experimental.IsExperimentalModeEnabled() && util.CheckPathExists(po.DevfilePath) {
envInfo, err := envinfo.NewEnvSpecificInfo(po.componentContext)
Expand Down Expand Up @@ -174,7 +173,6 @@ func NewCmdPush(name, fullName string) *cobra.Command {

// enable devfile flag if experimental mode is enabled
if experimental.IsExperimentalModeEnabled() {
pushCmd.Flags().StringVar(&po.DevfilePath, "devfile", "./devfile.yaml", "Path to a devfile.yaml")
pushCmd.Flags().StringVar(&po.namespace, "namespace", "", "Namespace to push the component to")
pushCmd.Flags().StringVar(&po.devfileInitCommand, "init-command", "", "Devfile Init Command to execute")
pushCmd.Flags().StringVar(&po.devfileBuildCommand, "build-command", "", "Devfile Build Command to execute")
Expand Down
3 changes: 1 addition & 2 deletions pkg/odo/cli/component/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func NewWatchOptions() *WatchOptions {

// Complete completes watch args
func (wo *WatchOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) {
wo.devfilePath = filepath.Join(wo.componentContext, wo.devfilePath)
wo.devfilePath = filepath.Join(wo.componentContext, DevfilePath)

// if experimental mode is enabled and devfile is present
if experimental.IsExperimentalModeEnabled() && util.CheckPathExists(wo.devfilePath) {
Expand Down Expand Up @@ -285,7 +285,6 @@ func NewCmdWatch(name, fullName string) *cobra.Command {

// enable devfile flag if experimental mode is enabled
if experimental.IsExperimentalModeEnabled() {
watchCmd.Flags().StringVar(&wo.devfilePath, "devfile", "./devfile.yaml", "Path to a devfile.yaml")
watchCmd.Flags().StringVar(&wo.devfileInitCommand, "init-command", "", "Devfile Init Command to execute")
watchCmd.Flags().StringVar(&wo.devfileBuildCommand, "build-command", "", "Devfile Build Command to execute")
watchCmd.Flags().StringVar(&wo.devfileRunCommand, "run-command", "", "Devfile Run Command to execute")
Expand Down
3 changes: 2 additions & 1 deletion pkg/odo/cli/url/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ func NewURLCreateOptions() *URLCreateOptions {

// Complete completes URLCreateOptions after they've been Created
func (o *URLCreateOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) {
o.DevfilePath = clicomponent.DevfilePath
GeekArthur marked this conversation as resolved.
Show resolved Hide resolved

if experimental.IsExperimentalModeEnabled() && util.CheckPathExists(o.DevfilePath) {
o.Context = genericclioptions.NewDevfileContext(cmd)
} else if o.now {
Expand Down Expand Up @@ -321,7 +323,6 @@ func NewCmdURLCreate(name, fullName string) *cobra.Command {
urlCreateCmd.Flags().BoolVar(&o.wantIngress, "ingress", false, "Creates an ingress instead of Route on OpenShift clusters")
urlCreateCmd.Example = fmt.Sprintf(urlCreateExampleExperimental, fullName)
}
urlCreateCmd.Flags().StringVar(&o.DevfilePath, "devfile", "./devfile.yaml", "Path to a devfile.yaml")
} else {
urlCreateCmd.Flags().BoolVarP(&o.secureURL, "secure", "", false, "creates a secure https url")
urlCreateCmd.Example = fmt.Sprintf(urlCreateExample, fullName)
Expand Down
61 changes: 60 additions & 1 deletion pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,7 @@ func ValidateK8sResourceName(key string, value string) error {
_, err2 := strconv.ParseFloat(value, 64)

if err1 != nil || err2 == nil {
return errors.Errorf("%s is not valid, %s should conform the following requirements: %s", key, key, requirements)
return errors.Errorf("%s \"%s\" is not valid, %s should conform the following requirements: %s", key, value, key, requirements)
}

return nil
Expand Down Expand Up @@ -1046,6 +1046,65 @@ func ValidateURL(sourceURL string) error {
return nil
}

// ValidateFile validates the file
func ValidateFile(filePath string) error {
// Check if the file path exist
file, err := os.Stat(filePath)
if err != nil {
return err
}

if file.IsDir() {
return errors.Errorf("%s exists but it's not a file", filePath)
}

return nil
}

// CopyFile copies file from source path to destination path
func CopyFile(srcPath string, dstPath string, info os.FileInfo) error {
// Check if the source file path exists
err := ValidateFile(srcPath)
if err != nil {
return err
}

// Open source file
srcFile, err := os.Open(srcPath)
if err != nil {
return err
}
defer srcFile.Close() // #nosec G307

// Create destination file
dstFile, err := os.Create(dstPath)
if err != nil {
return err
}
defer dstFile.Close() // #nosec G307

// Ensure destination file has the same file mode with source file
err = os.Chmod(dstFile.Name(), info.Mode())
if err != nil {
return err
}

// Copy file
_, err = io.Copy(dstFile, srcFile)
if err != nil {
return err
}

return nil
}

// PathEqual compare the paths to determine if they are equal
func PathEqual(firstPath string, secondPath string) bool {
firstAbsPath, _ := GetAbsPath(firstPath)
secondAbsPath, _ := GetAbsPath(secondPath)
return firstAbsPath == secondAbsPath
}

// sliceContainsString checks for existence of given string in given slice
func sliceContainsString(str string, slice []string) bool {
for _, b := range slice {
Expand Down
147 changes: 147 additions & 0 deletions pkg/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1653,6 +1653,153 @@ func TestValidateURL(t *testing.T) {
}
}

func TestValidateFile(t *testing.T) {
// Create temp dir and temp file
tempDir, err := ioutil.TempDir("", "")
if err != nil {
t.Errorf("Failed to create temp dir: %s, error: %v", tempDir, err)
}
tempFile, err := ioutil.TempFile(tempDir, "")
if err != nil {
t.Errorf("Failed to create temp file: %s, error: %v", tempFile.Name(), err)
}
defer tempFile.Close()

tests := []struct {
name string
filePath string
wantErr bool
}{
{
name: "Case 1: Valid file path",
filePath: tempFile.Name(),
wantErr: false,
},
{
name: "Case 2: Invalid file path",
filePath: "!@#",
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotErr := false
err := ValidateFile(tt.filePath)
if err != nil {
gotErr = true
}
if !reflect.DeepEqual(gotErr, tt.wantErr) {
t.Errorf("Got error: %t, want error: %t", gotErr, tt.wantErr)
}
})
}
}

func TestCopyFile(t *testing.T) {
// Create temp dir
tempDir, err := ioutil.TempDir("", "")
if err != nil {
t.Errorf("Failed to create temp dir: %s, error: %v", tempDir, err)
}

// Create temp file under temp dir as source file
tempFile, err := ioutil.TempFile(tempDir, "")
if err != nil {
t.Errorf("Failed to create temp file: %s, error: %v", tempFile.Name(), err)
}
defer tempFile.Close()

srcPath := tempFile.Name()
fakePath := "!@#/**"
dstPath := filepath.Join(tempDir, "dstFile")
info, _ := os.Stat(srcPath)

tests := []struct {
name string
srcPath string
dstPath string
wantErr bool
}{
{
name: "Case 1: Copy successfully",
srcPath: srcPath,
dstPath: dstPath,
wantErr: false,
},
{
name: "Case 2: Invalid source path",
srcPath: fakePath,
dstPath: dstPath,
wantErr: true,
GeekArthur marked this conversation as resolved.
Show resolved Hide resolved
},
{
name: "Case 3: Invalid destination path",
srcPath: srcPath,
dstPath: fakePath,
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotErr := false
err = CopyFile(tt.srcPath, tt.dstPath, info)
if err != nil {
gotErr = true
}

if !reflect.DeepEqual(gotErr, tt.wantErr) {
t.Errorf("Got error: %t, want error: %t", gotErr, tt.wantErr)
}
})
}
}

func TestPathEqual(t *testing.T) {
currentDir, err := os.Getwd()
if err != nil {
t.Errorf("Can't get absolute path of current working directory with error: %v", err)
}
fileAbsPath := filepath.Join(currentDir, "file")
fileRelPath := filepath.Join(".", "file")

tests := []struct {
name string
firstPath string
secondPath string
want bool
}{
{
name: "Case 1: Two paths (two absolute paths) are equal",
firstPath: fileAbsPath,
secondPath: fileAbsPath,
want: true,
},
{
name: "Case 2: Two paths (one absolute path, one relative path) are equal",
firstPath: fileAbsPath,
secondPath: fileRelPath,
want: true,
},
{
name: "Case 3: Two paths are not equal",
firstPath: fileAbsPath,
secondPath: filepath.Join(fileAbsPath, "file"),
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := PathEqual(tt.firstPath, tt.secondPath)
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Got: %t, want %t", got, tt.want)
}
})
}
}

func TestSliceContainsString(t *testing.T) {
tests := []struct {
name string
Expand Down
28 changes: 3 additions & 25 deletions tests/helper/helper_filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package helper

import (
"fmt"
"io"
"io/ioutil"
"os"
"path/filepath"
Expand All @@ -11,6 +10,7 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/openshift/odo/pkg/util"
)

// CreateNewContext create new empty temporary directory
Expand Down Expand Up @@ -89,7 +89,7 @@ func CopyExampleDevFile(devfilePath, targetDst string) {
info, err := os.Stat(src)
Expect(err).NotTo(HaveOccurred())

err = copyFile(src, targetDst, info)
err = util.CopyFile(src, targetDst, info)
Expect(err).NotTo(HaveOccurred())
}

Expand Down Expand Up @@ -137,29 +137,7 @@ func copyDir(src string, dst string, info os.FileInfo) error {
return err
}

return copyFile(src, dst, info)
}

// copyFile copy one file to another location
func copyFile(src, dst string, info os.FileInfo) error {
dFile, err := os.Create(dst)
if err != nil {
return err
}
defer dFile.Close() // #nosec G307

sFile, err := os.Open(src)
if err != nil {
return err
}
defer sFile.Close() // #nosec G307

if err = os.Chmod(dFile.Name(), info.Mode()); err != nil {
return err
}

_, err = io.Copy(dFile, sFile)
return err
return util.CopyFile(src, dst, info)
}

// CreateFileWithContent creates a file at the given path and writes the given content
Expand Down
3 changes: 2 additions & 1 deletion tests/helper/kubernetes_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/openshift/odo/pkg/util"
)

// copyKubeConfigFile copies default kubeconfig file into current temporary context config file
func copyKubeConfigFile(kubeConfigFile, tempConfigFile string) {
info, err := os.Stat(kubeConfigFile)
Expect(err).NotTo(HaveOccurred())
err = copyFile(kubeConfigFile, tempConfigFile, info)
err = util.CopyFile(kubeConfigFile, tempConfigFile, info)
Expect(err).NotTo(HaveOccurred())
os.Setenv("KUBECONFIG", tempConfigFile)
fmt.Fprintf(GinkgoWriter, "Setting KUBECONFIG=%s\n", tempConfigFile)
Expand Down
Loading