Skip to content

Commit 72fb7c1

Browse files
adrianrioboclaude
andcommitted
fix(ibmcloud): address CodeRabbit review findings for GitLab runner support
- Validate mutual exclusion of --glrunner-project-id and --glrunner-group-id in params.GitLabRunnerArgs; log error and return nil when both are set - Remove global state mutation (SetAuthToken/GetRunnerArgs) from ApplyT closures in ibm-power and ibm-z; use a local value copy with AuthToken set - Fix error swallowing in ibm-z buildUserDataInput: change signature to (pulumi.StringPtrInput, error) and propagate template errors to callers - Change start_at from beginning to end for filelog/gitlab-runner receiver in both IBM cloud-configs (avoid re-shipping pre-existing log content) - Add apt-get update before apt-get install in IBM Z cloud-config - Add logrotate config for /var/log/gitlab-runner/runner.log in both IBM cloud-configs (daily, 7 rotations, copytruncate) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ecb417d commit 72fb7c1

5 files changed

Lines changed: 77 additions & 46 deletions

File tree

cmd/mapt/cmd/params/params.go

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
cr "github.com/redhat-developer/mapt/pkg/provider/api/compute-request"
88
spotTypes "github.com/redhat-developer/mapt/pkg/provider/api/spot"
99
"github.com/redhat-developer/mapt/pkg/util"
10+
"github.com/redhat-developer/mapt/pkg/util/logging"
1011
"github.com/spf13/pflag"
1112
"github.com/spf13/viper"
1213
)
@@ -285,21 +286,29 @@ func CirrusPersistentWorkerArgs() *cirrus.PersistentWorkerArgs {
285286
return nil
286287
}
287288

288-
func GitLabRunnerArgs() *gitlab.GitLabRunnerArgs {
289+
func GitLabRunnerArgs(arch *gitlab.Arch) *gitlab.GitLabRunnerArgs {
289290
if viper.IsSet(glRunnerToken) {
291+
if viper.IsSet(glRunnerProjectID) && viper.IsSet(glRunnerGroupID) {
292+
logging.Error("--glrunner-project-id and --glrunner-group-id are mutually exclusive; ignoring GitLab runner configuration")
293+
return nil
294+
}
290295
return &gitlab.GitLabRunnerArgs{
291296
GitLabToken: viper.GetString(glRunnerToken),
292-
ProjectID: viper.GetString(glRunnerProjectID),
293-
GroupID: viper.GetString(glRunnerGroupID),
294-
URL: viper.GetString(glRunnerURL),
295-
Tags: viper.GetStringSlice(glRunnerTags),
296-
Platform: &gitlab.Linux,
297-
Arch: linuxArchAsGitLabArch(viper.GetString(LinuxArch)),
297+
ProjectID: viper.GetString(glRunnerProjectID),
298+
GroupID: viper.GetString(glRunnerGroupID),
299+
URL: viper.GetString(glRunnerURL),
300+
Tags: viper.GetStringSlice(glRunnerTags),
301+
Platform: &gitlab.Linux,
302+
Arch: arch,
298303
}
299304
}
300305
return nil
301306
}
302307

308+
func LinuxGitLabArch() *gitlab.Arch {
309+
return linuxArchAsGitLabArch(viper.GetString(LinuxArch))
310+
}
311+
303312
func linuxArchAsCirrusArch(arch string) *cirrus.Arch {
304313
switch arch {
305314
case "x86_64":
@@ -340,17 +349,3 @@ func MACArchAsGitLabArch(arch string) *gitlab.Arch {
340349
return &gitlab.Arm64
341350
}
342351

343-
func GitLabRunnerArgsForArch(arch *gitlab.Arch) *gitlab.GitLabRunnerArgs {
344-
if viper.IsSet(glRunnerToken) {
345-
return &gitlab.GitLabRunnerArgs{
346-
GitLabToken: viper.GetString(glRunnerToken),
347-
ProjectID: viper.GetString(glRunnerProjectID),
348-
GroupID: viper.GetString(glRunnerGroupID),
349-
URL: viper.GetString(glRunnerURL),
350-
Tags: viper.GetStringSlice(glRunnerTags),
351-
Platform: &gitlab.Linux,
352-
Arch: arch,
353-
}
354-
}
355-
return nil
356-
}

pkg/provider/ibmcloud/action/ibm-power/cloud-config

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ write_files:
7777
filelog/gitlab-runner:
7878
include:
7979
- /var/log/gitlab-runner/runner.log
80-
start_at: beginning
80+
start_at: end
8181
include_file_path: true
8282
include_file_name: true
8383
operators:
@@ -152,6 +152,18 @@ write_files:
152152
[Service]
153153
StandardOutput=append:/var/log/gitlab-runner/runner.log
154154
StandardError=append:/var/log/gitlab-runner/runner.log
155+
- path: /etc/logrotate.d/gitlab-runner
156+
permissions: '0644'
157+
content: |
158+
/var/log/gitlab-runner/runner.log {
159+
daily
160+
rotate 7
161+
compress
162+
delaycompress
163+
missingok
164+
notifempty
165+
copytruncate
166+
}
155167
- path: /opt/install-glrunner.sh
156168
permissions: '0755'
157169
content: |

pkg/provider/ibmcloud/action/ibm-power/ibm-power.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,10 @@ func (r *pwRequest) deploy(ctx *pulumi.Context) error {
171171
return err
172172
}
173173
gateway := subnetInfo.Gateway
174+
localArgs := *glRunnerArgs
174175
piUserDataInput = authToken.ApplyT(func(token string) (*string, error) {
175-
gitlab.SetAuthToken(token)
176-
defer gitlab.SetAuthToken("")
177-
glSnippet, err := integrations.GetIntegrationSnippetAsCloudInitWritableFile(gitlab.GetRunnerArgs(), defaultUser)
176+
localArgs.AuthToken = token
177+
glSnippet, err := integrations.GetIntegrationSnippetAsCloudInitWritableFile(&localArgs, defaultUser)
178178
if err != nil {
179179
return nil, err
180180
}

pkg/provider/ibmcloud/action/ibm-z/cloud-config

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ write_files:
7777
filelog/gitlab-runner:
7878
include:
7979
- /var/log/gitlab-runner/runner.log
80-
start_at: beginning
80+
start_at: end
8181
include_file_path: true
8282
include_file_name: true
8383
operators:
@@ -152,13 +152,26 @@ write_files:
152152
[Service]
153153
StandardOutput=append:/var/log/gitlab-runner/runner.log
154154
StandardError=append:/var/log/gitlab-runner/runner.log
155+
- path: /etc/logrotate.d/gitlab-runner
156+
permissions: '0644'
157+
content: |
158+
/var/log/gitlab-runner/runner.log {
159+
daily
160+
rotate 7
161+
compress
162+
delaycompress
163+
missingok
164+
notifempty
165+
copytruncate
166+
}
155167
- path: /opt/install-glrunner.sh
156168
permissions: '0755'
157169
content: |
158170
{{.GitLabRunnerScript}}
159171
{{- end}}
160172
{{- end}}
161173
runcmd:
174+
- apt-get update -y
162175
- apt-get install -y git podman
163176
{{- if or (and .AppCode .OtelAuthToken .OtelIndex) .GitLabRunnerScript}}
164177
{{- if and .AppCode .OtelAuthToken .OtelIndex}}

pkg/provider/ibmcloud/action/ibm-z/ibm-z.go

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,17 @@ type ZArgs struct {
6969
}
7070

7171
type zRequest struct {
72-
mCtx *mc.Context
73-
prefix *string
74-
zone *string
75-
subnetID *string
76-
otelAppCode string
77-
otelAuthToken string
78-
otelEndpoint string
79-
otelIndex string
80-
otelExtraAttrs map[string]string
81-
glAuthToken *pulumi.StringOutput
72+
mCtx *mc.Context
73+
prefix *string
74+
zone *string
75+
subnetID *string
76+
otelAppCode string
77+
otelAuthToken string
78+
otelEndpoint string
79+
otelIndex string
80+
otelExtraAttrs map[string]string
81+
glAuthToken *pulumi.StringOutput
82+
glRunnerArgsCopy *gitlab.GitLabRunnerArgs
8283
}
8384

8485
// New provisions an IBM Z (s390x) VPC instance. When SubnetID is set the
@@ -150,6 +151,8 @@ func (r *zRequest) deploy(ctx *pulumi.Context) error {
150151
return err
151152
}
152153
r.glAuthToken = &authToken
154+
argsCopy := *glRunnerArgs
155+
r.glRunnerArgsCopy = &argsCopy
153156
}
154157
if r.subnetID != nil {
155158
return r.deployWithExistingSubnet(ctx)
@@ -202,7 +205,11 @@ func (r *zRequest) deploy(ctx *pulumi.Context) error {
202205
},
203206
},
204207
}
205-
instanceArgs.UserData = r.buildUserDataInput()
208+
userData, err := r.buildUserDataInput()
209+
if err != nil {
210+
return err
211+
}
212+
instanceArgs.UserData = userData
206213
// https://cloud.ibm.com/docs/vpc?topic=vpc-profiles&interface=ui&q=s390x&tags=vpc
207214
i, err := ibmcloud.NewIsInstance(ctx,
208215
resourcesUtil.GetResourceName(*r.prefix, stackIBMS390, "is"),
@@ -284,7 +291,11 @@ func (r *zRequest) deployWithExistingSubnet(ctx *pulumi.Context) error {
284291
SecurityGroups: pulumi.StringArray{sg.ID()},
285292
},
286293
}
287-
existingSubnetInstanceArgs.UserData = r.buildUserDataInput()
294+
existingSubnetUserData, err := r.buildUserDataInput()
295+
if err != nil {
296+
return err
297+
}
298+
existingSubnetInstanceArgs.UserData = existingSubnetUserData
288299
// https://cloud.ibm.com/docs/vpc?topic=vpc-profiles&interface=ui&q=s390x&tags=vpc
289300
i, err := ibmcloud.NewIsInstance(ctx,
290301
resourcesUtil.GetResourceName(*r.prefix, stackIBMS390, "is"),
@@ -330,13 +341,13 @@ func (r *zRequest) deployWithExistingSubnet(ctx *pulumi.Context) error {
330341
// UserData field. If a GitLab runner token output is available it uses ApplyT
331342
// to embed the runner setup script after the token is resolved; otherwise it
332343
// renders the cloud-config synchronously.
333-
func (r *zRequest) buildUserDataInput() pulumi.StringPtrInput {
344+
func (r *zRequest) buildUserDataInput() (pulumi.StringPtrInput, error) {
334345
hasOtel := r.otelAppCode != "" && r.otelAuthToken != ""
335346
if r.glAuthToken != nil {
347+
localArgs := *r.glRunnerArgsCopy
336348
return r.glAuthToken.ApplyT(func(token string) (*string, error) {
337-
gitlab.SetAuthToken(token)
338-
defer gitlab.SetAuthToken("")
339-
glSnippet, err := integrations.GetIntegrationSnippetAsCloudInitWritableFile(gitlab.GetRunnerArgs(), defaultUser)
349+
localArgs.AuthToken = token
350+
glSnippet, err := integrations.GetIntegrationSnippetAsCloudInitWritableFile(&localArgs, defaultUser)
340351
if err != nil {
341352
return nil, err
342353
}
@@ -350,16 +361,16 @@ func (r *zRequest) buildUserDataInput() pulumi.StringPtrInput {
350361
return nil, err
351362
}
352363
return &ud, nil
353-
}).(pulumi.StringPtrOutput)
364+
}).(pulumi.StringPtrOutput), nil
354365
}
355366
if hasOtel {
356367
ud, err := izUserData(r.otelAppCode, r.otelAuthToken, r.otelEndpoint, r.otelIndex, "", r.otelExtraAttrs)
357368
if err != nil {
358-
return nil
369+
return nil, fmt.Errorf("failed to render user data: %w", err)
359370
}
360-
return pulumi.StringPtr(ud)
371+
return pulumi.StringPtr(ud), nil
361372
}
362-
return nil
373+
return nil, nil
363374
}
364375

365376
func izUserData(otelAppCode, otelAuthToken, otelEndpoint, otelIndex, glRunnerScript string, otelExtraAttrs map[string]string) (string, error) {

0 commit comments

Comments
 (0)