Skip to content

Commit

Permalink
Add --devfile flag support (redhat-developer#3118)
Browse files Browse the repository at this point in the history
* feat: add --devfile flag support

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* fix: fix gosec G307 issue

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* fix: update error handling

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* style: Update help and error messages

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* fix: Improve performance of error handling

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* style: Improve error message

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* fix: Fix remove file error on Windows

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* fix: Use existing structure to handle temp file

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* refactor: improve code readability and structure

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* docs: update variable comments

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* feat: --devfile value supports http(s)

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* feat: remove --devfile for all other commands

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* feat: add new design for existing devfile support

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* test: update TestCopyFile test cases

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* test: refactor file copy util function for test

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* test: fix TestCopyFile test cases

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* test: add integration tests for existing devfile

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* fix: gosec issue

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* test: fix docker url integration tests

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* fix: context support

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* test: fix docker url integration tests

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* fix: handle edge cases and add related tests

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* refactor: separate spinner for existing devfile

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* test: fix docker watch tests

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

* test: fix url tests

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>
  • Loading branch information
GeekArthur authored and cdrage committed Jun 17, 2020
1 parent b37843d commit 6b82233
Show file tree
Hide file tree
Showing 22 changed files with 574 additions and 267 deletions.
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.

##### 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)

// 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

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,
},
{
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

0 comments on commit 6b82233

Please sign in to comment.