Skip to content

Commit

Permalink
fix a bug regarding panics with checking apps (#170)
Browse files Browse the repository at this point in the history
  • Loading branch information
djeebus authored Mar 25, 2024
1 parent 946b84b commit e525f84
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 4 deletions.
27 changes: 27 additions & 0 deletions cmd/locations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,33 @@ func (f *fakeCloner) Clone(ctx context.Context, cloneUrl, branchName string) (*g
return f.result, f.err
}

func TestMaybeCloneGitUrl_NonGitUrl(t *testing.T) {
ctx := context.TODO()

type testcase struct {
name, input string
}

testcases := []testcase{
{
name: "https url",
input: "https://raw.githubusercontent.com/datreeio/CRDs-catalog/main/{{.Group}}/{{.ResourceKind}}_{{.ResourceAPIVersion}}.json",
},
}

for _, tc := range testcases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
fc := &fakeCloner{result: nil, err: nil}
actual, err := maybeCloneGitUrl(ctx, fc, time.Duration(0), tc.input, testUsername)
require.NoError(t, err)
assert.Equal(t, "", fc.branchName)
assert.Equal(t, "", fc.cloneUrl)
assert.Equal(t, tc.input, actual)
})
}
}

const testRoot = "/tmp/path"
const testUsername = "username"

Expand Down
10 changes: 6 additions & 4 deletions pkg/app_watcher/app_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func (ctrl *ApplicationWatcher) newApplicationInformerAndLister(refreshTimeout t
func canProcessApp(obj interface{}) (*appv1alpha1.Application, bool) {
app, ok := obj.(*appv1alpha1.Application)
if !ok {
return &appv1alpha1.Application{}, false
return nil, false
}

for _, src := range app.Spec.Sources {
Expand All @@ -152,11 +152,13 @@ func canProcessApp(obj interface{}) (*appv1alpha1.Application, bool) {
}
}

if !isGitRepo(app.Spec.Source.RepoURL) {
return app, false
if app.Spec.Source != nil {
if isGitRepo(app.Spec.Source.RepoURL) {
return app, true
}
}

return app, true
return app, false
}

func isGitRepo(url string) bool {
Expand Down
102 changes: 102 additions & 0 deletions pkg/app_watcher/app_watcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,105 @@ func TestIsGitRepo(t *testing.T) {
}
}
}

func TestCanProcessApp(t *testing.T) {
tests := []struct {
name string
resource interface{}
expectedApp *v1alpha1.Application
returnApp, canProcessApp bool
}{
{
name: "nil resource",
resource: nil,
expectedApp: nil,
returnApp: false,
canProcessApp: false,
},
{
name: "not an app",
resource: new(string),
expectedApp: nil,
returnApp: false,
canProcessApp: false,
},
{
name: "empty app",
resource: new(v1alpha1.Application),
expectedApp: nil,
returnApp: true,
canProcessApp: false,
},
{
name: "single source without git repo",
resource: &v1alpha1.Application{
Spec: v1alpha1.ApplicationSpec{
Source: &v1alpha1.ApplicationSource{
RepoURL: "file://../../../",
},
},
},
returnApp: true,
canProcessApp: false,
},
{
name: "single source without git repo",
resource: &v1alpha1.Application{
Spec: v1alpha1.ApplicationSpec{
Source: &v1alpha1.ApplicationSource{
RepoURL: "[email protected]:user/repo.git",
},
},
},
returnApp: true,
canProcessApp: true,
},
{
name: "multi source without git repo",
resource: &v1alpha1.Application{
Spec: v1alpha1.ApplicationSpec{
Sources: v1alpha1.ApplicationSources{
{
RepoURL: "file://../../../",
},
},
},
},
returnApp: true,
canProcessApp: false,
},
{
name: "multi source with git repo",
resource: &v1alpha1.Application{
Spec: v1alpha1.ApplicationSpec{
Sources: v1alpha1.ApplicationSources{
{
RepoURL: "[email protected]:user/repo.git",
},
},
},
},
returnApp: true,
canProcessApp: true,
},
}

for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
app, canProcess := canProcessApp(tc.resource)

if tc.canProcessApp {
assert.True(t, canProcess)
} else {
assert.False(t, canProcess)
}

if tc.returnApp {
assert.Equal(t, tc.resource, app)
} else {
assert.Nil(t, app)
}
})
}
}

0 comments on commit e525f84

Please sign in to comment.