Skip to content

Commit

Permalink
fix dirApp pitfalls (#167)
Browse files Browse the repository at this point in the history 
* check if an identical appName already exist in the project
* empty path will be persisted as "."
  • Loading branch information
@ATGardner
ATGardner authored Aug 29, 2021
1 parent 6ecdf93 commit 96837ab
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 41 deletions.
28 changes: 12 additions & 16 deletions pkg/application/application.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package application

import (
"encoding/json"
"errors"
"fmt"
"io/ioutil"
Expand Down Expand Up @@ -43,7 +42,6 @@ var (
ErrEmptyProjectName = errors.New("project name can not be empty, please specificy project name with: --project")
ErrAppAlreadyInstalledOnProject = errors.New("application already installed on project")
ErrAppCollisionWithExistingBase = errors.New("an application with the same name and a different base already exists, consider choosing a different name")
ErrAppBaseOnDifferentRepo = errors.New("an application with the same name already exists in a different repo, consider choosing a different name")
ErrUnknownAppType = errors.New("unknown application type")
)

Expand Down Expand Up @@ -360,13 +358,8 @@ func kustCreateFiles(app *kustApp, repofs fs.FS, appsfs fs.FS, projectName strin
configPath = repofs.Join(appPath, projectName, "config.json")
}

config, err := json.Marshal(app.config)
if err != nil {
return fmt.Errorf("failed to marshal app config.json: %w", err)
}

if _, err = writeFile(repofs, configPath, "config", config); err != nil {
return err
if err = repofs.WriteJson(configPath, app.config); err != nil {
return fmt.Errorf("failed to write app config.json: %w", err)
}

return nil
Expand Down Expand Up @@ -405,6 +398,10 @@ func newDirApp(opts *CreateOptions) *dirApp {

host, orgRepo, path, gitRef, _, suffix, _ := util.ParseGitUrl(opts.AppSpecifier)
url := host + orgRepo + suffix
if path == "" {
path = "."
}

app.config = &Config{
AppName: opts.AppName,
UserGivenName: opts.AppName,
Expand All @@ -420,15 +417,14 @@ func newDirApp(opts *CreateOptions) *dirApp {
}

func (app *dirApp) CreateFiles(repofs fs.FS, appsfs fs.FS, projectName string) error {
basePath := repofs.Join(store.Default.AppsDir, app.opts.AppName, projectName)
configPath := repofs.Join(basePath, "config.json")
config, err := json.Marshal(app.config)
if err != nil {
return fmt.Errorf("failed to marshal app config.json: %w", err)
appPath := repofs.Join(store.Default.AppsDir, app.opts.AppName, projectName)
if repofs.ExistsOrDie(appPath) {
return ErrAppAlreadyInstalledOnProject
}

if _, err = writeFile(repofs, configPath, "config", config); err != nil {
return err
configPath := repofs.Join(appPath, "config.json")
if err := repofs.WriteJson(configPath, app.config); err != nil {
return fmt.Errorf("failed to write app config.json: %w", err)
}

clusterName, err := getClusterName(repofs, app.opts.DestServer)
Expand Down
86 changes: 61 additions & 25 deletions pkg/application/application_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package application

import (
"encoding/json"
"errors"
"io/ioutil"
"path/filepath"
Expand All @@ -21,17 +20,12 @@ import (
kusttypes "sigs.k8s.io/kustomize/api/types"
)

func bootstrapMockFS(t *testing.T, repofs fs.FS) {
func bootstrapMockFS() fs.FS {
repofs := fs.Create(memfs.New())
clusterResConf := &ClusterResConfig{Name: store.Default.ClusterContextName, Server: store.Default.DestServer}
clusterResConfJSON, err := json.Marshal(clusterResConf)
assert.NoError(t, err)
err = billyUtils.WriteFile(
repofs,
repofs.Join(store.Default.BootsrtrapDir, store.Default.ClusterResourcesDir, store.Default.ClusterContextName+".json"),
clusterResConfJSON,
0644,
)
assert.NoError(t, err)
clusterResPath := repofs.Join(store.Default.BootsrtrapDir, store.Default.ClusterResourcesDir, store.Default.ClusterContextName+".json")
_ = repofs.WriteYamls(clusterResPath, clusterResConf)
return repofs
}

func Test_newKustApp(t *testing.T) {
Expand Down Expand Up @@ -286,7 +280,7 @@ func Test_kustCreateFiles(t *testing.T) {
},
},
}
repofs := fs.Create(memfs.New())
repofs := bootstrapMockFS()
return app, repofs, repofs, "project"
},
assertFn: func(t *testing.T, repofs fs.FS, _ fs.FS, err error) {
Expand All @@ -309,7 +303,7 @@ func Test_kustCreateFiles(t *testing.T) {
},
},
}
repofs := fs.Create(memfs.New())
repofs := bootstrapMockFS()
appsfs := fs.Create(memfs.New())
return app, repofs, appsfs, "project"
},
Expand Down Expand Up @@ -339,7 +333,7 @@ func Test_kustCreateFiles(t *testing.T) {
},
manifests: []byte("some manifests"),
}
repofs := fs.Create(memfs.New())
repofs := bootstrapMockFS()
return app, repofs, repofs, "project"
},
assertFn: func(t *testing.T, repofs fs.FS, _ fs.FS, err error) {
Expand All @@ -361,7 +355,7 @@ func Test_kustCreateFiles(t *testing.T) {
},
namespace: kube.GenerateNamespace("foo"),
}
repofs := fs.Create(memfs.New())
repofs := bootstrapMockFS()
return app, repofs, repofs, "project"
},
assertFn: func(t *testing.T, repofs fs.FS, _ fs.FS, err error) {
Expand All @@ -384,7 +378,7 @@ func Test_kustCreateFiles(t *testing.T) {
},
},
}
repofs := fs.Create(memfs.New())
repofs := bootstrapMockFS()
return app, repofs, repofs, "project"
},
assertFn: func(t *testing.T, _ fs.FS, _ fs.FS, err error) {
Expand Down Expand Up @@ -415,7 +409,7 @@ func Test_kustCreateFiles(t *testing.T) {
},
Resources: []string{"github.com/owner/different_repo?ref=v1.2.3"},
}
repofs := fs.Create(memfs.New())
repofs := bootstrapMockFS()
_ = repofs.WriteYamls(repofs.Join(store.Default.AppsDir, "app", store.Default.BaseDir, "kustomization.yaml"), origBase)
return app, repofs, repofs, "project"
},
Expand Down Expand Up @@ -448,7 +442,7 @@ func Test_kustCreateFiles(t *testing.T) {
},
base: origBase,
}
repofs := fs.Create(memfs.New())
repofs := bootstrapMockFS()
_ = repofs.WriteYamls(repofs.Join(store.Default.AppsDir, "app", store.Default.BaseDir, "kustomization.yaml"), origBase)
_ = repofs.WriteYamls(repofs.Join(store.Default.AppsDir, "app", store.Default.OverlaysDir, "project", "kustomization.yaml"), overlay)
return app, repofs, repofs, "project"
Expand All @@ -466,7 +460,7 @@ func Test_kustCreateFiles(t *testing.T) {
},
},
}
repofs := fs.Create(memfs.New())
repofs := bootstrapMockFS()
appsfs := fs.Create(memfs.New())
_ = repofs.MkdirAll(repofs.Join(store.Default.AppsDir, "app"), 0666)
return app, repofs, appsfs, "project"
Expand All @@ -488,7 +482,7 @@ func Test_kustCreateFiles(t *testing.T) {
AppName: "app",
SrcRepoURL: "https://github.com/owner/other_name.git",
}
repofs := fs.Create(memfs.New())
repofs := bootstrapMockFS()
appsfs := fs.Create(memfs.New())
_ = repofs.WriteJson(repofs.Join(store.Default.AppsDir, "app", store.Default.OverlaysDir, "project", "config.json"), config)
return app, repofs, appsfs, "project"
Expand All @@ -501,7 +495,6 @@ func Test_kustCreateFiles(t *testing.T) {
for tname, tt := range tests {
t.Run(tname, func(t *testing.T) {
app, repofs, appsfs, projectName := tt.beforeFn()
bootstrapMockFS(t, repofs)
err := app.CreateFiles(repofs, appsfs, projectName)
tt.assertFn(t, repofs, appsfs, err)
})
Expand Down Expand Up @@ -692,7 +685,7 @@ func Test_newDirApp(t *testing.T) {
opts *CreateOptions
want *dirApp
}{
"Basic": {
"Should create a simple app with requested fields": {
opts: &CreateOptions{
AppName: "fooapp",
AppSpecifier: "github.com/foo/bar/somepath/in/repo?ref=v0.1.2",
Expand All @@ -711,6 +704,23 @@ func Test_newDirApp(t *testing.T) {
},
},
},
"Should use the correct path, when no path is supplied": {
opts: &CreateOptions{
AppName: "fooapp",
AppSpecifier: "github.com/foo/bar",
},
want: &dirApp{
config: &Config{
AppName: "fooapp",
UserGivenName: "fooapp",
DestNamespace: "",
DestServer: "",
SrcRepoURL: "https://github.com/foo/bar.git",
SrcTargetRevision: "",
SrcPath: ".",
},
},
},
}
for tname, tt := range tests {
t.Run(tname, func(t *testing.T) {
Expand All @@ -725,8 +735,31 @@ func Test_dirApp_CreateFiles(t *testing.T) {
tests := map[string]struct {
projectName string
app *dirApp
beforeFn func() fs.FS
assertFn func(*testing.T, fs.FS, error)
}{
"Should fail if an app with the same name already exist": {
projectName: "project",
app: &dirApp{
baseApp: baseApp{
opts: &CreateOptions{
AppName: "foo",
AppSpecifier: "github.com/foo/bar/path",
DestNamespace: "default",
DestServer: store.Default.DestServer,
},
},
},
beforeFn: func() fs.FS {
fs := bootstrapMockFS()
appPath := fs.Join(store.Default.AppsDir, "foo", "project", "DUMMY")
_ = billyUtils.WriteFile(fs, appPath, []byte{}, 0666)
return fs
},
assertFn: func(t *testing.T, _ fs.FS, err error) {
assert.Error(t, err, ErrAppAlreadyInstalledOnProject)
},
},
"Should not create namespace if app namespace is 'default'": {
app: &dirApp{
baseApp: baseApp{
Expand All @@ -738,7 +771,9 @@ func Test_dirApp_CreateFiles(t *testing.T) {
},
},
},
assertFn: func(t *testing.T, repofs fs.FS, _ error) {
beforeFn: bootstrapMockFS,
assertFn: func(t *testing.T, repofs fs.FS, err error) {
assert.NoError(t, err)
exists, err := repofs.Exists(repofs.Join(
store.Default.BootsrtrapDir,
store.Default.ClusterResourcesDir,
Expand All @@ -760,6 +795,7 @@ func Test_dirApp_CreateFiles(t *testing.T) {
},
},
},
beforeFn: bootstrapMockFS,
assertFn: func(t *testing.T, _ fs.FS, err error) {
assert.Error(t, err, "cluster 'some.new.server' is not configured yet, you need to create a project that uses this cluster first")
},
Expand All @@ -775,6 +811,7 @@ func Test_dirApp_CreateFiles(t *testing.T) {
},
},
},
beforeFn: bootstrapMockFS,
assertFn: func(t *testing.T, repofs fs.FS, _ error) {
exists, err := repofs.Exists(repofs.Join(
store.Default.BootsrtrapDir,
Expand All @@ -789,8 +826,7 @@ func Test_dirApp_CreateFiles(t *testing.T) {
}
for tname, tt := range tests {
t.Run(tname, func(t *testing.T) {
repofs := fs.Create(memfs.New())
bootstrapMockFS(t, repofs)
repofs := tt.beforeFn()
tt.assertFn(t, repofs, tt.app.CreateFiles(repofs, repofs, tt.projectName))
})
}
Expand Down

0 comments on commit 96837ab

Please sign in to comment.