Skip to content

Commit 15e64db

Browse files
ddymkoddymko
authored
refactor: streamline githook validation (#63)
* refactor: streamline githook validation * test: add test for allowedgithooks
1 parent 4feb038 commit 15e64db

6 files changed

Lines changed: 159 additions & 130 deletions

File tree

‎internal/commands/root.go‎

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"os"
88
"sort"
9-
"strings"
109

1110
"github.com/devbytes-cloud/freight/internal/blueprint"
1211
"github.com/devbytes-cloud/freight/internal/config"
@@ -18,14 +17,6 @@ import (
1817
"gopkg.in/yaml.v3"
1918
)
2019

21-
var allowHooks = map[string]struct{}{
22-
"pre-commit": {},
23-
"prepare-commit-msg": {},
24-
"commit-msg": {},
25-
"post-commit": {},
26-
"post-checkout": {},
27-
}
28-
2920
// Execute runs the root command and handles any errors that occur during execution.
3021
func Execute() {
3122
if err := NewRootCmd().Execute(); err != nil {
@@ -84,7 +75,7 @@ func NewRootCmd() *cobra.Command {
8475
} else if len(freightConfig.Allow) == 0 {
8576
// If no user allow and no existing allow, use default all hooks
8677
var allHooks []string
87-
for h := range allowHooks {
78+
for h := range githooks.AllowedGitHooks() {
8879
allHooks = append(allHooks, h)
8980
}
9081
sort.Strings(allHooks)
@@ -98,7 +89,7 @@ func NewRootCmd() *cobra.Command {
9889
pterm.Debug.Printfln("Effective allow: %v", freightConfig.Allow)
9990
pterm.Debug.Printfln("Hooks to setup: %v", hooksToSetup)
10091

101-
validatedAllow, err := validateAllowHooks(hooksToSetup)
92+
validatedAllow, err := validate.GitHooks(hooksToSetup)
10293
if err != nil {
10394
cmd.PrintErrln(err)
10495
os.Exit(1)
@@ -224,26 +215,3 @@ func installBinary() error {
224215
pterm.Success.Println("✔ Installed conductor successfully")
225216
return nil
226217
}
227-
228-
// validateAllowHooks validates the provided allow hooks and returns a map of valid hooks.
229-
func validateAllowHooks(allow []string) (map[string]struct{}, error) {
230-
if len(allow) == 0 {
231-
pterm.Debug.Println("No hooks provided, using default allowed hooks")
232-
return allowHooks, nil
233-
}
234-
235-
inputHooks := map[string]struct{}{}
236-
var invalidHooks []string
237-
for _, v := range allow {
238-
if _, ok := allowHooks[v]; !ok {
239-
invalidHooks = append(invalidHooks, v)
240-
}
241-
inputHooks[v] = struct{}{}
242-
}
243-
244-
if len(invalidHooks) > 0 {
245-
return nil, fmt.Errorf("invalid hook types: %s", strings.Join(invalidHooks, ", "))
246-
}
247-
248-
return inputHooks, nil
249-
}

‎internal/commands/root_test.go‎

Lines changed: 0 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,97 +1 @@
11
package commands
2-
3-
import (
4-
"fmt"
5-
"testing"
6-
7-
"github.com/stretchr/testify/assert"
8-
)
9-
10-
type hookStructure struct {
11-
inputHooks []string
12-
expectedHooks map[string]struct{}
13-
expectedErr bool
14-
expectedErrMgs error
15-
}
16-
17-
func TestValidateAllowHooks(t *testing.T) {
18-
testData := map[string]hookStructure{
19-
"no input hooks": {
20-
expectedErr: false,
21-
inputHooks: []string{},
22-
expectedHooks: map[string]struct{}{
23-
"pre-commit": {},
24-
"prepare-commit-msg": {},
25-
"commit-msg": {},
26-
"post-commit": {},
27-
"post-checkout": {},
28-
},
29-
},
30-
"only pre-commit hook": {
31-
expectedErr: false,
32-
inputHooks: []string{"pre-commit"},
33-
expectedHooks: map[string]struct{}{
34-
"pre-commit": {},
35-
},
36-
},
37-
"invalid hook name hook": {
38-
expectedErr: true,
39-
inputHooks: []string{"invalid hook name"},
40-
expectedErrMgs: fmt.Errorf("invalid hook types: invalid hook name"),
41-
expectedHooks: nil,
42-
},
43-
"multiple valid hooks": {
44-
expectedErr: false,
45-
inputHooks: []string{"pre-commit", "commit-msg", "post-checkout"},
46-
expectedHooks: map[string]struct{}{
47-
"pre-commit": {},
48-
"commit-msg": {},
49-
"post-checkout": {},
50-
},
51-
},
52-
"multiple invalid hooks": {
53-
expectedErr: true,
54-
inputHooks: []string{"invalid1", "invalid2"},
55-
expectedErrMgs: fmt.Errorf("invalid hook types: invalid1, invalid2"),
56-
expectedHooks: nil,
57-
},
58-
"mix of valid and invalid hooks": {
59-
expectedErr: true,
60-
inputHooks: []string{"pre-commit", "invalid-hook"},
61-
expectedErrMgs: fmt.Errorf("invalid hook types: invalid-hook"),
62-
expectedHooks: nil,
63-
},
64-
"all valid hooks explicitly provided": {
65-
expectedErr: false,
66-
inputHooks: []string{"pre-commit", "prepare-commit-msg", "commit-msg", "post-commit", "post-checkout"},
67-
expectedHooks: map[string]struct{}{
68-
"pre-commit": {},
69-
"prepare-commit-msg": {},
70-
"commit-msg": {},
71-
"post-commit": {},
72-
"post-checkout": {},
73-
},
74-
},
75-
"duplicate valid hooks": {
76-
expectedErr: false,
77-
inputHooks: []string{"pre-commit", "pre-commit"},
78-
expectedHooks: map[string]struct{}{
79-
"pre-commit": {},
80-
},
81-
},
82-
}
83-
84-
for name, test := range testData {
85-
t.Run(name, func(t *testing.T) {
86-
resp, err := validateAllowHooks(test.inputHooks)
87-
88-
if test.expectedErr {
89-
assert.Error(t, err)
90-
assert.EqualError(t, err, test.expectedErrMgs.Error())
91-
} else {
92-
assert.NoError(t, err)
93-
assert.Equal(t, test.expectedHooks, resp)
94-
}
95-
})
96-
}
97-
}

‎internal/githooks/githooks.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,21 @@ func generateHooks(hook []string) []GitHook {
5353
func hookPath(hookType string) string {
5454
return filepath.Join(hooksBaseDir, hookType)
5555
}
56+
57+
// AllowedGitHooks returns a slice of allowed Git hook types.
58+
func AllowedGitHooks() map[string]struct{} {
59+
checkout := getCheckoutHooks()
60+
commit := getCommitHook()
61+
62+
allowedHooks := make(map[string]struct{}, len(checkout)+len(commit))
63+
64+
for _, hook := range checkout {
65+
allowedHooks[hook] = struct{}{}
66+
}
67+
68+
for _, hook := range commit {
69+
allowedHooks[hook] = struct{}{}
70+
}
71+
72+
return allowedHooks
73+
}

‎internal/githooks/githooks_test.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,23 @@ func TestNewGitHooks(t *testing.T) {
5555
assert.Equal(t, gitHookTemplate, hook.Template, "checkout hook Template should match gitHookTemplate")
5656
}
5757
}
58+
59+
// TestAllowedGitHooks verifies that AllowedGitHooks returns the expected map of allowed Git hooks.
60+
func TestAllowedGitHooks(t *testing.T) {
61+
allowedHooks := AllowedGitHooks()
62+
63+
expectedHooks := []string{
64+
PreCommit,
65+
PrepareCommitMsg,
66+
CommitMsg,
67+
PostCommit,
68+
PostCheckout,
69+
}
70+
71+
assert.Len(t, allowedHooks, len(expectedHooks), "AllowedGitHooks should return the correct number of hooks")
72+
73+
for _, hook := range expectedHooks {
74+
_, exists := allowedHooks[hook]
75+
assert.True(t, exists, "Hook %s should be in the allowed hooks map", hook)
76+
}
77+
}

‎internal/validate/validate.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import (
44
"fmt"
55
"os"
66
"path/filepath"
7+
"strings"
8+
9+
"github.com/devbytes-cloud/freight/internal/githooks"
10+
"github.com/pterm/pterm"
711
)
812

913
var (
@@ -35,3 +39,28 @@ func CurrentWD() (string, error) {
3539

3640
return dir, nil
3741
}
42+
43+
// GitHooks validates the provided allow hooks and returns a map of valid hooks.
44+
func GitHooks(allow []string) (map[string]struct{}, error) {
45+
allowedGitHooks := githooks.AllowedGitHooks()
46+
if len(allow) == 0 {
47+
pterm.Debug.Println("No hooks provided, using default allowed hooks")
48+
return allowedGitHooks, nil
49+
}
50+
51+
inputHooks := map[string]struct{}{}
52+
var invalidHooks []string
53+
for _, v := range allow {
54+
if _, ok := allowedGitHooks[v]; !ok {
55+
invalidHooks = append(invalidHooks, v)
56+
continue
57+
}
58+
inputHooks[v] = struct{}{}
59+
}
60+
61+
if len(invalidHooks) > 0 {
62+
return nil, fmt.Errorf("invalid hook types: %s", strings.Join(invalidHooks, ", "))
63+
}
64+
65+
return inputHooks, nil
66+
}

‎internal/validate/validate_test.go‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package validate_test
22

33
import (
4+
"fmt"
45
"os"
56
"path/filepath"
67
"testing"
@@ -10,6 +11,13 @@ import (
1011
"github.com/stretchr/testify/require"
1112
)
1213

14+
type hookStructure struct {
15+
inputHooks []string
16+
expectedHooks map[string]struct{}
17+
expectedErr bool
18+
expectedErrMgs error
19+
}
20+
1321
func TestGitDirs(t *testing.T) {
1422
tt := []struct {
1523
name string
@@ -60,3 +68,85 @@ func TestCurrentWD(t *testing.T) {
6068
assert.NoError(t, err)
6169
assert.NotEmpty(t, wd)
6270
}
71+
72+
func TestValidateAllowHooks(t *testing.T) {
73+
testData := map[string]hookStructure{
74+
"no input hooks": {
75+
expectedErr: false,
76+
inputHooks: []string{},
77+
expectedHooks: map[string]struct{}{
78+
"pre-commit": {},
79+
"prepare-commit-msg": {},
80+
"commit-msg": {},
81+
"post-commit": {},
82+
"post-checkout": {},
83+
},
84+
},
85+
"only pre-commit hook": {
86+
expectedErr: false,
87+
inputHooks: []string{"pre-commit"},
88+
expectedHooks: map[string]struct{}{
89+
"pre-commit": {},
90+
},
91+
},
92+
"invalid hook name hook": {
93+
expectedErr: true,
94+
inputHooks: []string{"invalid hook name"},
95+
expectedErrMgs: fmt.Errorf("invalid hook types: invalid hook name"),
96+
expectedHooks: nil,
97+
},
98+
"multiple valid hooks": {
99+
expectedErr: false,
100+
inputHooks: []string{"pre-commit", "commit-msg", "post-checkout"},
101+
expectedHooks: map[string]struct{}{
102+
"pre-commit": {},
103+
"commit-msg": {},
104+
"post-checkout": {},
105+
},
106+
},
107+
"multiple invalid hooks": {
108+
expectedErr: true,
109+
inputHooks: []string{"invalid1", "invalid2"},
110+
expectedErrMgs: fmt.Errorf("invalid hook types: invalid1, invalid2"),
111+
expectedHooks: nil,
112+
},
113+
"mix of valid and invalid hooks": {
114+
expectedErr: true,
115+
inputHooks: []string{"pre-commit", "invalid-hook"},
116+
expectedErrMgs: fmt.Errorf("invalid hook types: invalid-hook"),
117+
expectedHooks: nil,
118+
},
119+
"all valid hooks explicitly provided": {
120+
expectedErr: false,
121+
inputHooks: []string{"pre-commit", "prepare-commit-msg", "commit-msg", "post-commit", "post-checkout"},
122+
expectedHooks: map[string]struct{}{
123+
"pre-commit": {},
124+
"prepare-commit-msg": {},
125+
"commit-msg": {},
126+
"post-commit": {},
127+
"post-checkout": {},
128+
},
129+
},
130+
"duplicate valid hooks": {
131+
expectedErr: false,
132+
inputHooks: []string{"pre-commit", "pre-commit"},
133+
expectedHooks: map[string]struct{}{
134+
"pre-commit": {},
135+
},
136+
},
137+
}
138+
139+
for name, test := range testData {
140+
t.Run(name, func(t *testing.T) {
141+
resp, err := validate.GitHooks(test.inputHooks)
142+
143+
if test.expectedErr {
144+
assert.Error(t, err)
145+
assert.EqualError(t, err, test.expectedErrMgs.Error())
146+
} else {
147+
assert.NoError(t, err)
148+
assert.Equal(t, test.expectedHooks, resp)
149+
}
150+
})
151+
}
152+
}

0 commit comments

Comments
 (0)