Skip to content

Commit de4eb10

Browse files
author
Romuald Atchadé
committed
Merge branch 'step_284-revert-variables-on-container' into 'main'
Restore environment variables to build container See merge request https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/6333 Merged-by: Romuald Atchadé <ratchade@gitlab.com> Approved-by: Axel von Bertoldi <avonbertoldi@gitlab.com> Approved-by: Cameron Swords <cswords@gitlab.com> Reviewed-by: Cameron Swords <cswords@gitlab.com> Reviewed-by: Romuald Atchadé <ratchade@gitlab.com> Reviewed-by: Axel von Bertoldi <avonbertoldi@gitlab.com>
2 parents 4acf043 + 625a302 commit de4eb10

5 files changed

Lines changed: 166 additions & 13 deletions

File tree

common/spec/variables.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,19 @@ func (b Variables) StringList() (variables []string) {
5050
return variables
5151
}
5252

53+
// GetAllVariableNames returns a semicolon-separated list of all variable names
54+
// that are set in the build. This function is used to pass the list of job variable
55+
// names to the build container via an environment variable (e.g., RUNNER_JOB_VAR_NAMES),
56+
// allowing step-runner to identify and filter out job variables from the OS environment.
57+
func (b Variables) GetAllVariableNames() string {
58+
names := make([]string, 0, len(b))
59+
for _, variable := range b {
60+
names = append(names, variable.Key)
61+
}
62+
63+
return strings.Join(names, ";")
64+
}
65+
5366
// Get returns the value of a variable, or if a file type variable, the
5467
// pathname to the saved file containing the value,
5568
func (b Variables) Get(key string) string {

executors/docker/docker.go

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ package docker
22

33
import (
44
"bytes"
5+
"compress/gzip"
56
"context"
7+
"encoding/base64"
68
"errors"
79
"fmt"
810
"io"
@@ -58,20 +60,22 @@ const (
5860
ExecutorStageCreatingServices common.ExecutorStage = "docker_creating_services"
5961
ExecutorStageCreatingUserVolumes common.ExecutorStage = "docker_creating_user_volumes"
6062
ExecutorStagePullingImage common.ExecutorStage = "docker_pulling_image"
61-
)
6263

63-
const ServiceLogOutputLimit = 64 * 1024
64+
ServiceLogOutputLimit = 64 * 1024
6465

65-
const (
6666
labelServiceType = "service"
6767
labelWaitType = "wait"
68-
)
6968

70-
// internalFakeTunnelHostname is an internal hostname we provide the Docker client
71-
// when we provide a tunnelled dialer implementation. Because we're overriding
72-
// the dialer, this domain should never be used by the client, but we use the
73-
// reserved TLD ".invalid" for safety.
74-
const internalFakeTunnelHostname = "http://internal.tunnel.invalid"
69+
// internalFakeTunnelHostname is an internal hostname we provide the Docker client
70+
// when we provide a tunnelled dialer implementation. Because we're overriding
71+
// the dialer, this domain should never be used by the client, but we use the
72+
// reserved TLD ".invalid" for safety.
73+
internalFakeTunnelHostname = "http://internal.tunnel.invalid"
74+
75+
// runnerJobVarsNames is the name used to identify the all the job variables names.
76+
// It is used to allow step-runner to filter these variables once the gRPC service is started
77+
runnerJobVarsNames = "RUNNER_JOB_VAR_NAMES"
78+
)
7579

7680
var neverRestartPolicy = container.RestartPolicy{Name: "no"}
7781

@@ -871,6 +875,11 @@ func (e *executor) createContainerConfig(
871875
cmd []string,
872876
) (*container.Config, error) {
873877
labels := e.prepareContainerLabels(map[string]string{"type": containerType})
878+
jobVars, err := e.prepareContainerEnvVariables()
879+
if err != nil {
880+
return nil, fmt.Errorf("setting job variables: %w", err)
881+
}
882+
874883
config := &container.Config{
875884
Image: image.ID,
876885
Hostname: hostname,
@@ -883,7 +892,7 @@ func (e *executor) createContainerConfig(
883892
OpenStdin: true,
884893
StdinOnce: true,
885894
Entrypoint: e.overwriteEntrypoint(&imageDefinition),
886-
Env: e.Build.GetAllVariables().StringList(),
895+
Env: jobVars.StringList(),
887896
}
888897

889898
//nolint:nestif
@@ -913,6 +922,53 @@ func (e *executor) createContainerConfig(
913922
return config, nil
914923
}
915924

925+
// prepareContainerEnvVariables prepares the environment variables for the build container.
926+
// When native steps are enabled, it compresses the list of job variable names and adds them
927+
// to the environment as RUNNER_JOB_VAR_NAMES. This allows step-runner to identify and filter
928+
// out job variables from the OS environment, preventing environment variable size limit issues.
929+
//
930+
// The variable names are gzip-compressed to minimize the size of the RUNNER_JOB_VAR_NAMES
931+
// environment variable itself, which is important on systems with strict environment limits
932+
// (particularly Windows).
933+
//
934+
// For non-native step builds, the function returns the variables unchanged since step-runner
935+
// filtering is not needed.
936+
func (e *executor) prepareContainerEnvVariables() (spec.Variables, error) {
937+
vars := e.Build.GetAllVariables()
938+
939+
if !e.Build.UseNativeSteps() {
940+
return vars, nil
941+
}
942+
943+
names := vars.GetAllVariableNames()
944+
compressedVarNames, err := gzipString(names)
945+
if err != nil {
946+
return nil, fmt.Errorf("job variables names compression failed: %w", err)
947+
}
948+
949+
v := append([]spec.Variable{}, vars...)
950+
v = append(v, spec.Variable{
951+
Key: runnerJobVarsNames,
952+
Value: compressedVarNames,
953+
})
954+
955+
return v, nil
956+
}
957+
958+
// gzipString compresses a string and returns the compressed string.
959+
func gzipString(src string) (string, error) {
960+
var b bytes.Buffer
961+
gz := gzip.NewWriter(&b)
962+
if _, err := gz.Write([]byte(src)); err != nil {
963+
return "", fmt.Errorf("writing to gzip writer: %w", err)
964+
}
965+
if err := gz.Close(); err != nil {
966+
return "", fmt.Errorf("closing gzip writer: %w", err)
967+
}
968+
969+
return base64.StdEncoding.EncodeToString(b.Bytes()), nil
970+
}
971+
916972
func (e *executor) getBuildContainerUser(imageDefinition spec.Image) (string, error) {
917973
// runner config takes precedence
918974
user := e.Config.Docker.User

executors/docker/docker_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3174,3 +3174,87 @@ func testDockerServiceContainerCgroup(
31743174
require.NoError(t, err)
31753175
assert.Equal(t, expectedCgroup, hostConfig.CgroupParent, "Service container HostConfig.CgroupParent should be set correctly")
31763176
}
3177+
3178+
func TestPrepareContainerEnvVariables(t *testing.T) {
3179+
test.SkipIfGitLabCIOn(t, test.OSWindows)
3180+
3181+
tests := map[string]struct {
3182+
featureFlagEnabled bool
3183+
jobVariables spec.Variables
3184+
expectedVarNames []string
3185+
shouldHaveRunnerVarNames bool
3186+
}{
3187+
"feature flag disabled returns variables unchanged": {
3188+
featureFlagEnabled: false,
3189+
jobVariables: spec.Variables{
3190+
{Key: "VAR1", Value: "value1"},
3191+
{Key: "VAR2", Value: "value2"},
3192+
},
3193+
shouldHaveRunnerVarNames: false,
3194+
},
3195+
"feature flag enabled compresses variable names": {
3196+
featureFlagEnabled: true,
3197+
jobVariables: spec.Variables{
3198+
{Key: "VAR1", Value: "value1"},
3199+
{Key: "VAR2", Value: "value2"},
3200+
{Key: "VAR3", Value: "value3"},
3201+
},
3202+
expectedVarNames: []string{"VAR1", "VAR2", "VAR3"},
3203+
shouldHaveRunnerVarNames: true,
3204+
},
3205+
"feature flag enabled with empty variables": {
3206+
featureFlagEnabled: true,
3207+
jobVariables: spec.Variables{},
3208+
shouldHaveRunnerVarNames: true,
3209+
},
3210+
"feature flag enabled with many variables": {
3211+
featureFlagEnabled: true,
3212+
jobVariables: spec.Variables{
3213+
{Key: "LONG_VARIABLE_NAME_1", Value: "value1"},
3214+
{Key: "LONG_VARIABLE_NAME_2", Value: "value2"},
3215+
{Key: "LONG_VARIABLE_NAME_3", Value: "value3"},
3216+
},
3217+
expectedVarNames: []string{"LONG_VARIABLE_NAME_1", "LONG_VARIABLE_NAME_2", "LONG_VARIABLE_NAME_3"},
3218+
shouldHaveRunnerVarNames: true,
3219+
},
3220+
}
3221+
3222+
for testName, test := range tests {
3223+
t.Run(testName, func(t *testing.T) {
3224+
e := &executor{
3225+
AbstractExecutor: executors.AbstractExecutor{
3226+
Build: &common.Build{
3227+
Job: spec.Job{
3228+
Variables: test.jobVariables,
3229+
},
3230+
},
3231+
},
3232+
}
3233+
3234+
// Set the feature flag
3235+
if test.featureFlagEnabled {
3236+
e.Build.ExecutorFeatures.NativeStepsIntegration = test.featureFlagEnabled
3237+
e.Build.Variables = append(e.Build.Variables, spec.Variable{
3238+
Key: featureflags.UseScriptToStepMigration,
3239+
Value: "true",
3240+
})
3241+
}
3242+
3243+
result, err := e.prepareContainerEnvVariables()
3244+
3245+
require.NoError(t, err)
3246+
require.NotNil(t, result)
3247+
3248+
require.Equal(t, test.shouldHaveRunnerVarNames, checkVariable(result, runnerJobVarsNames))
3249+
})
3250+
}
3251+
}
3252+
3253+
func checkVariable(vars spec.Variables, key string) bool {
3254+
for i := range vars {
3255+
if vars[i].Key == key {
3256+
return true
3257+
}
3258+
}
3259+
return false
3260+
}

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ require (
8080
gitlab.com/gitlab-org/golang-cli-helpers v0.0.0-20220124161940-198f30295e7e
8181
gitlab.com/gitlab-org/labkit v1.34.0
8282
gitlab.com/gitlab-org/moa v0.0.0-20251209091627-66342f721c88
83-
gitlab.com/gitlab-org/step-runner v0.26.0
83+
gitlab.com/gitlab-org/step-runner v0.27.0
8484
go.mozilla.org/pkcs7 v0.9.0
8585
go.uber.org/automaxprocs v1.6.0
8686
go.yaml.in/yaml/v3 v3.0.4

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -604,8 +604,8 @@ gitlab.com/gitlab-org/labkit v1.34.0 h1:wJrUZdfxbZrA9+qvnpk5gYJtceW+p7ceSJSg1BPZ
604604
gitlab.com/gitlab-org/labkit v1.34.0/go.mod h1:JqQLdgjV/KKAZJ6gvNodaLStmWeTT9mxgJPIEi66VHI=
605605
gitlab.com/gitlab-org/moa v0.0.0-20251209091627-66342f721c88 h1:GVlo8Pr4wfXI/6UF+Rmi0Yv2l7lwVgFiNBeImgWVeko=
606606
gitlab.com/gitlab-org/moa v0.0.0-20251209091627-66342f721c88/go.mod h1:024490ksS75/Bi9UoJTu59qY44JuFBAfi5bzGsLIhtY=
607-
gitlab.com/gitlab-org/step-runner v0.26.0 h1:i9EeEk5HeJFvKB5eaR3XUTSLlaIH4Q17ccsbVQiQ5t0=
608-
gitlab.com/gitlab-org/step-runner v0.26.0/go.mod h1:KZ7aYYlc/DFt3xxhfygrr0TTlXWdnN+LJEsFC9W/1Sw=
607+
gitlab.com/gitlab-org/step-runner v0.27.0 h1:8wrnKWv7x3CcSXhJUDLHYkWimWlCp65T5JDmceTiPaw=
608+
gitlab.com/gitlab-org/step-runner v0.27.0/go.mod h1:KZ7aYYlc/DFt3xxhfygrr0TTlXWdnN+LJEsFC9W/1Sw=
609609
go.etcd.io/bbolt v1.3.5 h1:XAzx9gjCb0Rxj7EoqcClPD1d5ZBxZJk0jbuoPHenBt0=
610610
go.etcd.io/bbolt v1.3.5/go.mod h1:G5EMThwa9y8QZGBClrRx5EY+Yw9kAhnjy3bSjsnlVTQ=
611611
go.mozilla.org/pkcs7 v0.9.0 h1:yM4/HS9dYv7ri2biPtxt8ikvB37a980dg69/pKmS+eI=

0 commit comments

Comments
 (0)