Skip to content

Commit 766dac9

Browse files
terakoya76jamengual
terakoya76
and
jamengual
authored
fix: ensure cloning workingdir before calling plan api (#3584)
Signed-off-by: Hajime Terasawa <[email protected]> Co-authored-by: PePe Amengual <[email protected]>
1 parent 38ba386 commit 766dac9

File tree

3 files changed

+215
-31
lines changed

3 files changed

+215
-31
lines changed

‎server/controllers/api_controller.go

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ type APIController struct {
3333
RepoAllowlistChecker *events.RepoAllowlistChecker
3434
Scope tally.Scope
3535
VCSClient vcs.Client
36+
WorkingDir events.WorkingDir
37+
WorkingDirLocker events.WorkingDirLocker
3638
CommitStatusUpdater events.CommitStatusUpdater
3739
}
3840

@@ -91,12 +93,18 @@ func (a *APIController) Plan(w http.ResponseWriter, r *http.Request) {
9193
return
9294
}
9395

96+
err = a.apiSetup(ctx)
97+
if err != nil {
98+
a.apiReportError(w, http.StatusInternalServerError, err)
99+
return
100+
}
101+
94102
result, err := a.apiPlan(request, ctx)
95103
if err != nil {
96104
a.apiReportError(w, http.StatusInternalServerError, err)
97105
return
98106
}
99-
defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, 0) // nolint: errcheck
107+
defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, ctx.Pull.Num) // nolint: errcheck
100108
if result.HasErrors() {
101109
code = http.StatusInternalServerError
102110
}
@@ -119,13 +127,19 @@ func (a *APIController) Apply(w http.ResponseWriter, r *http.Request) {
119127
return
120128
}
121129

130+
err = a.apiSetup(ctx)
131+
if err != nil {
132+
a.apiReportError(w, http.StatusInternalServerError, err)
133+
return
134+
}
135+
122136
// We must first make the plan for all projects
123137
_, err = a.apiPlan(request, ctx)
124138
if err != nil {
125139
a.apiReportError(w, http.StatusInternalServerError, err)
126140
return
127141
}
128-
defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, 0) // nolint: errcheck
142+
defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, ctx.Pull.Num) // nolint: errcheck
129143

130144
// We can now prepare and run the apply step
131145
result, err := a.apiApply(request, ctx)
@@ -145,6 +159,27 @@ func (a *APIController) Apply(w http.ResponseWriter, r *http.Request) {
145159
a.respond(w, logging.Warn, code, "%s", string(response))
146160
}
147161

162+
func (a *APIController) apiSetup(ctx *command.Context) error {
163+
pull := ctx.Pull
164+
baseRepo := ctx.Pull.BaseRepo
165+
headRepo := ctx.HeadRepo
166+
167+
unlockFn, err := a.WorkingDirLocker.TryLock(baseRepo.FullName, pull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)
168+
if err != nil {
169+
return err
170+
}
171+
ctx.Log.Debug("got workspace lock")
172+
defer unlockFn()
173+
174+
// ensure workingDir is present
175+
_, _, err = a.WorkingDir.Clone(ctx.Log, headRepo, pull, events.DefaultWorkspace)
176+
if err != nil {
177+
return err
178+
}
179+
180+
return nil
181+
}
182+
148183
func (a *APIController) apiPlan(request *APIRequest, ctx *command.Context) (*command.Result, error) {
149184
cmds, cc, err := request.getCommands(ctx, a.ProjectCommandBuilder.BuildPlanCommands)
150185
if err != nil {

‎server/controllers/api_controller_test.go

Lines changed: 176 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -25,49 +25,194 @@ const atlantisToken = "token"
2525

2626
func TestAPIController_Plan(t *testing.T) {
2727
ac, projectCommandBuilder, projectCommandRunner := setup(t)
28-
body, _ := json.Marshal(controllers.APIRequest{
29-
Repository: "Repo",
30-
Ref: "main",
31-
Type: "Gitlab",
32-
Projects: []string{"default"},
33-
})
34-
req, _ := http.NewRequest("POST", "", bytes.NewBuffer(body))
35-
req.Header.Set(atlantisTokenHeader, atlantisToken)
36-
w := httptest.NewRecorder()
37-
ac.Plan(w, req)
38-
ResponseContains(t, w, http.StatusOK, "")
39-
projectCommandBuilder.VerifyWasCalledOnce().BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())
40-
projectCommandRunner.VerifyWasCalledOnce().Plan(Any[command.ProjectContext]())
28+
29+
cases := []struct {
30+
repository string
31+
ref string
32+
vcsType string
33+
pr int
34+
projects []string
35+
paths []struct {
36+
Directory string
37+
Workspace string
38+
}
39+
}{
40+
{
41+
repository: "Repo",
42+
ref: "main",
43+
vcsType: "Gitlab",
44+
projects: []string{"default"},
45+
},
46+
{
47+
repository: "Repo",
48+
ref: "main",
49+
vcsType: "Gitlab",
50+
pr: 1,
51+
},
52+
{
53+
repository: "Repo",
54+
ref: "main",
55+
vcsType: "Gitlab",
56+
paths: []struct {
57+
Directory string
58+
Workspace string
59+
}{
60+
{
61+
Directory: ".",
62+
Workspace: "myworkspace",
63+
},
64+
{
65+
Directory: "./myworkspace2",
66+
Workspace: "myworkspace2",
67+
},
68+
},
69+
},
70+
{
71+
repository: "Repo",
72+
ref: "main",
73+
vcsType: "Gitlab",
74+
pr: 1,
75+
projects: []string{"test"},
76+
paths: []struct {
77+
Directory string
78+
Workspace string
79+
}{
80+
{
81+
Directory: ".",
82+
Workspace: "myworkspace",
83+
},
84+
},
85+
},
86+
}
87+
88+
expectedCalls := 0
89+
for _, c := range cases {
90+
body, _ := json.Marshal(controllers.APIRequest{
91+
Repository: c.repository,
92+
Ref: c.ref,
93+
Type: c.vcsType,
94+
PR: c.pr,
95+
Projects: c.projects,
96+
Paths: c.paths,
97+
})
98+
99+
req, _ := http.NewRequest("POST", "", bytes.NewBuffer(body))
100+
req.Header.Set(atlantisTokenHeader, atlantisToken)
101+
w := httptest.NewRecorder()
102+
ac.Plan(w, req)
103+
ResponseContains(t, w, http.StatusOK, "")
104+
105+
expectedCalls += len(c.projects)
106+
expectedCalls += len(c.paths)
107+
}
108+
109+
projectCommandBuilder.VerifyWasCalled(Times(expectedCalls)).BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())
110+
projectCommandRunner.VerifyWasCalled(Times(expectedCalls)).Plan(Any[command.ProjectContext]())
41111
}
42112

43113
func TestAPIController_Apply(t *testing.T) {
44114
ac, projectCommandBuilder, projectCommandRunner := setup(t)
45-
body, _ := json.Marshal(controllers.APIRequest{
46-
Repository: "Repo",
47-
Ref: "main",
48-
Type: "Gitlab",
49-
Projects: []string{"default"},
50-
})
51-
req, _ := http.NewRequest("POST", "", bytes.NewBuffer(body))
52-
req.Header.Set(atlantisTokenHeader, atlantisToken)
53-
w := httptest.NewRecorder()
54-
ac.Apply(w, req)
55-
ResponseContains(t, w, http.StatusOK, "")
56-
projectCommandBuilder.VerifyWasCalledOnce().BuildApplyCommands(Any[*command.Context](), Any[*events.CommentCommand]())
57-
projectCommandRunner.VerifyWasCalledOnce().Plan(Any[command.ProjectContext]())
58-
projectCommandRunner.VerifyWasCalledOnce().Apply(Any[command.ProjectContext]())
115+
116+
cases := []struct {
117+
repository string
118+
ref string
119+
vcsType string
120+
pr int
121+
projects []string
122+
paths []struct {
123+
Directory string
124+
Workspace string
125+
}
126+
}{
127+
{
128+
repository: "Repo",
129+
ref: "main",
130+
vcsType: "Gitlab",
131+
projects: []string{"default"},
132+
},
133+
{
134+
repository: "Repo",
135+
ref: "main",
136+
vcsType: "Gitlab",
137+
pr: 1,
138+
},
139+
{
140+
repository: "Repo",
141+
ref: "main",
142+
vcsType: "Gitlab",
143+
paths: []struct {
144+
Directory string
145+
Workspace string
146+
}{
147+
{
148+
Directory: ".",
149+
Workspace: "myworkspace",
150+
},
151+
{
152+
Directory: "./myworkspace2",
153+
Workspace: "myworkspace2",
154+
},
155+
},
156+
},
157+
{
158+
repository: "Repo",
159+
ref: "main",
160+
vcsType: "Gitlab",
161+
pr: 1,
162+
projects: []string{"test"},
163+
paths: []struct {
164+
Directory string
165+
Workspace string
166+
}{
167+
{
168+
Directory: ".",
169+
Workspace: "myworkspace",
170+
},
171+
},
172+
},
173+
}
174+
175+
expectedCalls := 0
176+
for _, c := range cases {
177+
body, _ := json.Marshal(controllers.APIRequest{
178+
Repository: c.repository,
179+
Ref: c.ref,
180+
Type: c.vcsType,
181+
PR: c.pr,
182+
Projects: c.projects,
183+
Paths: c.paths,
184+
})
185+
186+
req, _ := http.NewRequest("POST", "", bytes.NewBuffer(body))
187+
req.Header.Set(atlantisTokenHeader, atlantisToken)
188+
w := httptest.NewRecorder()
189+
ac.Apply(w, req)
190+
ResponseContains(t, w, http.StatusOK, "")
191+
192+
expectedCalls += len(c.projects)
193+
expectedCalls += len(c.paths)
194+
}
195+
196+
projectCommandBuilder.VerifyWasCalled(Times(expectedCalls)).BuildApplyCommands(Any[*command.Context](), Any[*events.CommentCommand]())
197+
projectCommandRunner.VerifyWasCalled(Times(expectedCalls)).Plan(Any[command.ProjectContext]())
198+
projectCommandRunner.VerifyWasCalled(Times(expectedCalls)).Apply(Any[command.ProjectContext]())
59199
}
60200

61201
func setup(t *testing.T) (controllers.APIController, *MockProjectCommandBuilder, *MockProjectCommandRunner) {
62202
RegisterMockTestingT(t)
63203
locker := NewMockLocker()
64204
logger := logging.NewNoopLogger(t)
65-
scope, _, _ := metrics.NewLoggingScope(logger, "null")
66205
parser := NewMockEventParsing()
67-
vcsClient := NewMockClient()
68206
repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*")
207+
scope, _, _ := metrics.NewLoggingScope(logger, "null")
208+
vcsClient := NewMockClient()
209+
workingDir := NewMockWorkingDir()
69210
Ok(t, err)
70211

212+
workingDirLocker := NewMockWorkingDirLocker()
213+
When(workingDirLocker.TryLock(Any[string](), Any[int](), Eq(events.DefaultWorkspace), Eq(events.DefaultRepoRelDir))).
214+
ThenReturn(func() {}, nil)
215+
71216
projectCommandBuilder := NewMockProjectCommandBuilder()
72217
When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())).
73218
ThenReturn([]command.ProjectContext{{
@@ -111,6 +256,8 @@ func setup(t *testing.T) (controllers.APIController, *MockProjectCommandBuilder,
111256
PostWorkflowHooksCommandRunner: postWorkflowHooksCommandRunner,
112257
VCSClient: vcsClient,
113258
RepoAllowlistChecker: repoAllowlistChecker,
259+
WorkingDir: workingDir,
260+
WorkingDirLocker: workingDirLocker,
114261
CommitStatusUpdater: commitStatusUpdater,
115262
}
116263
return ac, projectCommandBuilder, projectCommandRunner

‎server/server.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -956,6 +956,8 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
956956
RepoAllowlistChecker: repoAllowlist,
957957
Scope: statsScope.SubScope("api"),
958958
VCSClient: vcsClient,
959+
WorkingDir: workingDir,
960+
WorkingDirLocker: workingDirLocker,
959961
}
960962

961963
eventsController := &events_controllers.VCSEventsController{

0 commit comments

Comments
 (0)