Skip to content

Commit 5460be8

Browse files
kmalakmala
committed
fix(slugbuilder): Dont expose the conifg as env vars during build
1 parent 1143f46 commit 5460be8

6 files changed

Lines changed: 151 additions & 71 deletions

File tree

‎pkg/gitreceive/build.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,21 @@ func build(
195195
if !slugBuilderInfo.DisableCaching() {
196196
cacheKey = slugBuilderInfo.CacheKey()
197197
}
198+
envSecretName := fmt.Sprintf("%s-build-env", appName)
199+
err = createAppEnvConfigSecret(kubeClient.Secrets(conf.PodNamespace), envSecretName, appConf.Values)
200+
if err != nil {
201+
return fmt.Errorf("error creating/updating secret %s: (%s)", envSecretName, err)
202+
}
203+
defer func() {
204+
if err := kubeClient.Secrets(conf.PodNamespace).Delete(envSecretName); err != nil {
205+
log.Info("unable to delete secret %s (%s)", envSecretName, err)
206+
}
207+
}()
198208
pod = slugbuilderPod(
199209
conf.Debug,
200210
buildPodName,
201211
conf.PodNamespace,
202-
appConf.Values,
212+
envSecretName,
203213
slugBuilderInfo.TarKey(),
204214
slugBuilderInfo.PushKey(),
205215
cacheKey,

‎pkg/gitreceive/k8s_util.go‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"github.com/deis/builder/pkg/k8s"
88
"github.com/pborman/uuid"
99
"k8s.io/kubernetes/pkg/api"
10+
apierrors "k8s.io/kubernetes/pkg/api/errors"
11+
client "k8s.io/kubernetes/pkg/client/unversioned"
1012
"k8s.io/kubernetes/pkg/labels"
1113
"k8s.io/kubernetes/pkg/util/wait"
1214
)
@@ -25,6 +27,7 @@ const (
2527
dockerSocketPath = "/var/run/docker.sock"
2628
builderStorage = "BUILDER_STORAGE"
2729
objectStorePath = "/var/run/secrets/deis/objectstore/creds"
30+
envRoot = "/tmp/env"
2831
)
2932

3033
func dockerBuilderPodName(appName, shortSha string) string {
@@ -92,7 +95,7 @@ func slugbuilderPod(
9295
debug bool,
9396
name,
9497
namespace string,
95-
env map[string]interface{},
98+
envSecretName string,
9699
tarKey,
97100
putKey,
98101
cacheKey,
@@ -103,7 +106,22 @@ func slugbuilderPod(
103106
pullPolicy api.PullPolicy,
104107
) *api.Pod {
105108

106-
pod := buildPod(debug, name, namespace, pullPolicy, env)
109+
pod := buildPod(debug, name, namespace, pullPolicy, nil)
110+
111+
pod.Spec.Volumes = append(pod.Spec.Volumes, api.Volume{
112+
Name: envSecretName,
113+
VolumeSource: api.VolumeSource{
114+
Secret: &api.SecretVolumeSource{
115+
SecretName: envSecretName,
116+
},
117+
},
118+
})
119+
120+
pod.Spec.Containers[0].VolumeMounts = append(pod.Spec.Containers[0].VolumeMounts, api.VolumeMount{
121+
Name: envSecretName,
122+
MountPath: envRoot,
123+
ReadOnly: true,
124+
})
107125

108126
pod.Spec.Containers[0].Name = slugBuilderName
109127
pod.Spec.Containers[0].Image = image
@@ -267,3 +285,23 @@ func progress(msg string, interval time.Duration) chan bool {
267285
}()
268286
return quit
269287
}
288+
289+
func createAppEnvConfigSecret(secretsClient client.SecretsInterface, secretName string, env map[string]interface{}) error {
290+
newSecret := new(api.Secret)
291+
newSecret.Name = secretName
292+
newSecret.Type = api.SecretTypeOpaque
293+
newSecret.Data = make(map[string][]byte)
294+
for k, v := range env {
295+
newSecret.Data[k] = []byte(fmt.Sprintf("%v", v))
296+
}
297+
if _, err := secretsClient.Create(newSecret); err != nil {
298+
if apierrors.IsAlreadyExists(err) {
299+
if _, err = secretsClient.Update(newSecret); err != nil {
300+
return err
301+
}
302+
return nil
303+
}
304+
return err
305+
}
306+
return nil
307+
}

‎pkg/gitreceive/k8s_util_test.go‎

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
package gitreceive
22

33
import (
4+
"errors"
45
"fmt"
56
"strings"
67
"testing"
78

9+
"github.com/arschles/assert"
10+
"github.com/deis/builder/pkg/k8s"
811
"k8s.io/kubernetes/pkg/api"
12+
apierrors "k8s.io/kubernetes/pkg/api/errors"
913
)
1014

1115
func TestDockerBuilderPodName(t *testing.T) {
@@ -26,7 +30,7 @@ type slugBuildCase struct {
2630
debug bool
2731
name string
2832
namespace string
29-
env map[string]interface{}
33+
envSecretName string
3034
tarKey string
3135
putKey string
3236
cacheKey string
@@ -55,26 +59,26 @@ func TestBuildPod(t *testing.T) {
5559

5660
env := make(map[string]interface{})
5761
env["KEY"] = "VALUE"
58-
62+
envSecretName := "test-build-env"
5963
var pod *api.Pod
6064

6165
slugBuilds := []slugBuildCase{
62-
{true, "test", "default", emptyEnv, "tar", "put-url", "cache-url", "deadbeef", "", "", api.PullAlways, ""},
63-
{true, "test", "default", env, "tar", "put-url", "cache-url", "deadbeef", "", "", api.PullAlways, ""},
64-
{true, "test", "default", emptyEnv, "tar", "put-url", "", "deadbeef", "", "", api.PullAlways, ""},
65-
{true, "test", "default", emptyEnv, "tar", "put-url", "cache-url", "deadbeef", "buildpack", "", api.PullAlways, ""},
66-
{true, "test", "default", env, "tar", "put-url", "cache-url", "deadbeef", "buildpack", "", api.PullAlways, ""},
67-
{true, "test", "default", env, "tar", "put-url", "cache-url", "deadbeef", "buildpack", "customimage", api.PullAlways, ""},
68-
{true, "test", "default", env, "tar", "put-url", "cache-url", "deadbeef", "buildpack", "customimage", api.PullIfNotPresent, ""},
69-
{true, "test", "default", env, "tar", "put-url", "cache-url", "deadbeef", "buildpack", "customimage", api.PullNever, ""},
66+
{true, "test", "default", envSecretName, "tar", "put-url", "cache-url", "deadbeef", "", "", api.PullAlways, ""},
67+
{true, "test", "default", envSecretName, "tar", "put-url", "cache-url", "deadbeef", "", "", api.PullAlways, ""},
68+
{true, "test", "default", envSecretName, "tar", "put-url", "", "deadbeef", "", "", api.PullAlways, ""},
69+
{true, "test", "default", envSecretName, "tar", "put-url", "cache-url", "deadbeef", "buildpack", "", api.PullAlways, ""},
70+
{true, "test", "default", envSecretName, "tar", "put-url", "cache-url", "deadbeef", "buildpack", "", api.PullAlways, ""},
71+
{true, "test", "default", envSecretName, "tar", "put-url", "cache-url", "deadbeef", "buildpack", "customimage", api.PullAlways, ""},
72+
{true, "test", "default", envSecretName, "tar", "put-url", "cache-url", "deadbeef", "buildpack", "customimage", api.PullIfNotPresent, ""},
73+
{true, "test", "default", envSecretName, "tar", "put-url", "cache-url", "deadbeef", "buildpack", "customimage", api.PullNever, ""},
7074
}
7175

7276
for _, build := range slugBuilds {
7377
pod = slugbuilderPod(
7478
build.debug,
7579
build.name,
7680
build.namespace,
77-
build.env,
81+
build.envSecretName,
7882
build.tarKey,
7983
build.putKey,
8084
build.cacheKey,
@@ -193,3 +197,38 @@ func envValueFromKey(pod *api.Pod, key string) (string, error) {
193197

194198
return "", fmt.Errorf("no key with name %v found in pod env", key)
195199
}
200+
201+
func TestCreateAppEnvConfigSecretErr(t *testing.T) {
202+
expectedErr := errors.New("get secret error")
203+
secretsClient := &k8s.FakeSecret{
204+
FnCreate: func(*api.Secret) (*api.Secret, error) {
205+
return &api.Secret{}, expectedErr
206+
},
207+
}
208+
err := createAppEnvConfigSecret(secretsClient, "test", nil)
209+
assert.Err(t, err, expectedErr)
210+
}
211+
212+
func TestCreateAppEnvConfigSecretSuccess(t *testing.T) {
213+
secretsClient := &k8s.FakeSecret{
214+
FnCreate: func(*api.Secret) (*api.Secret, error) {
215+
return &api.Secret{}, nil
216+
},
217+
}
218+
err := createAppEnvConfigSecret(secretsClient, "test", nil)
219+
assert.NoErr(t, err)
220+
}
221+
222+
func TestCreateAppEnvConfigSecretAlreadyExists(t *testing.T) {
223+
alreadyExistErr := apierrors.NewAlreadyExists(api.Resource("tests"), "1")
224+
secretsClient := &k8s.FakeSecret{
225+
FnCreate: func(*api.Secret) (*api.Secret, error) {
226+
return &api.Secret{}, alreadyExistErr
227+
},
228+
FnUpdate: func(*api.Secret) (*api.Secret, error) {
229+
return &api.Secret{}, nil
230+
},
231+
}
232+
err := createAppEnvConfigSecret(secretsClient, "test", nil)
233+
assert.NoErr(t, err)
234+
}

‎pkg/gitreceive/registry.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"errors"
77
"strings"
88

9-
"github.com/deis/builder/pkg/k8s"
109
"github.com/deis/builder/pkg/storage"
1110
"k8s.io/kubernetes/pkg/api"
1211
client "k8s.io/kubernetes/pkg/client/unversioned"
@@ -16,7 +15,7 @@ const (
1615
registrySecret = "registry-secret"
1716
)
1817

19-
func getDetailsFromRegistrySecret(secretGetter k8s.SecretGetter, secret string) (map[string]string, error) {
18+
func getDetailsFromRegistrySecret(secretGetter client.SecretsInterface, secret string) (map[string]string, error) {
2019
regSecret, err := secretGetter.Get(secret)
2120
if err != nil {
2221
return nil, err
@@ -28,7 +27,7 @@ func getDetailsFromRegistrySecret(secretGetter k8s.SecretGetter, secret string)
2827
return regDetails, nil
2928
}
3029

31-
func getDetailsFromDockerConfigSecret(secretGetter k8s.SecretGetter, secret string) (map[string]string, error) {
30+
func getDetailsFromDockerConfigSecret(secretGetter client.SecretsInterface, secret string) (map[string]string, error) {
3231
configSecret, err := secretGetter.Get(secret)
3332
if err != nil {
3433
return nil, err

‎pkg/gitreceive/registry_test.go‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ const (
1818

1919
func TestGetDetailsFromRegistrySecretErr(t *testing.T) {
2020
expectedErr := errors.New("get secret error")
21-
getter := &k8s.FakeSecretGetter{
22-
Fn: func(string) (*api.Secret, error) {
21+
getter := &k8s.FakeSecret{
22+
FnGet: func(string) (*api.Secret, error) {
2323
return &api.Secret{}, expectedErr
2424
},
2525
}
@@ -31,8 +31,8 @@ func TestGetDetailsFromRegistrySecretSuccess(t *testing.T) {
3131
data := map[string][]byte{"test": []byte("test")}
3232
expectedData := map[string]string{"test": "test"}
3333
secret := api.Secret{Data: data}
34-
getter := &k8s.FakeSecretGetter{
35-
Fn: func(string) (*api.Secret, error) {
34+
getter := &k8s.FakeSecret{
35+
FnGet: func(string) (*api.Secret, error) {
3636
return &secret, nil
3737
},
3838
}
@@ -43,8 +43,8 @@ func TestGetDetailsFromRegistrySecretSuccess(t *testing.T) {
4343

4444
func TestGetDetailsFromDockerConfigSecretErr(t *testing.T) {
4545
expectedErr := errors.New("get secret error")
46-
getter := &k8s.FakeSecretGetter{
47-
Fn: func(string) (*api.Secret, error) {
46+
getter := &k8s.FakeSecret{
47+
FnGet: func(string) (*api.Secret, error) {
4848
return &api.Secret{}, expectedErr
4949
},
5050
}
@@ -56,8 +56,8 @@ func TestGetDetailsFromDockerConfigSecretJsonErr(t *testing.T) {
5656
expectedErr := errors.New("invalid character 'o' in literal null (expecting 'u')")
5757
data := map[string][]byte{api.DockerConfigJsonKey: []byte("not a json")}
5858
secret := api.Secret{Data: data}
59-
getter := &k8s.FakeSecretGetter{
60-
Fn: func(string) (*api.Secret, error) {
59+
getter := &k8s.FakeSecret{
60+
FnGet: func(string) (*api.Secret, error) {
6161
return &secret, nil
6262
},
6363
}
@@ -80,8 +80,8 @@ func TestGetDetailsFromDockerConfigSecretTokenerr(t *testing.T) {
8080
data := make(map[string][]byte)
8181
data[api.DockerConfigJsonKey] = auth
8282
secret := api.Secret{Data: data}
83-
getter := &k8s.FakeSecretGetter{
84-
Fn: func(string) (*api.Secret, error) {
83+
getter := &k8s.FakeSecret{
84+
FnGet: func(string) (*api.Secret, error) {
8585
return &secret, nil
8686
},
8787
}
@@ -105,8 +105,8 @@ func TestGetDetailsFromDockerConfigSecretSuccess(t *testing.T) {
105105
data := make(map[string][]byte)
106106
data[api.DockerConfigJsonKey] = auth
107107
secret := api.Secret{Data: data}
108-
getter := &k8s.FakeSecretGetter{
109-
Fn: func(string) (*api.Secret, error) {
108+
getter := &k8s.FakeSecret{
109+
FnGet: func(string) (*api.Secret, error) {
110110
return &secret, nil
111111
},
112112
}
@@ -118,8 +118,8 @@ func TestGetDetailsFromDockerConfigSecretSuccess(t *testing.T) {
118118

119119
func TestGetRegistryDetailsOffclusterErr(t *testing.T) {
120120
expectedErr := errors.New("get secret error")
121-
getter := &k8s.FakeSecretGetter{
122-
Fn: func(string) (*api.Secret, error) {
121+
getter := &k8s.FakeSecret{
122+
FnGet: func(string) (*api.Secret, error) {
123123
return &api.Secret{}, expectedErr
124124
},
125125
}
@@ -139,8 +139,8 @@ func TestGetRegistryDetailsOffclusterSuccess(t *testing.T) {
139139
expectedData := map[string]string{"DEIS_REGISTRY_HOSTNAME": "quay.io", "DEIS_REGISTRY_ORGANIZATION": "kmala"}
140140
expectedImage := "quay.io/kmala/test-image"
141141
secret := api.Secret{Data: data}
142-
getter := &k8s.FakeSecretGetter{
143-
Fn: func(string) (*api.Secret, error) {
142+
getter := &k8s.FakeSecret{
143+
FnGet: func(string) (*api.Secret, error) {
144144
return &secret, nil
145145
},
146146
}
@@ -172,8 +172,8 @@ func TestGetRegistryDetailsGCRSuccess(t *testing.T) {
172172
configData := make(map[string][]byte)
173173
configData[api.DockerConfigJsonKey] = auth
174174
configSecret := api.Secret{Data: configData}
175-
configGetter := &k8s.FakeSecretGetter{
176-
Fn: func(string) (*api.Secret, error) {
175+
configGetter := &k8s.FakeSecret{
176+
FnGet: func(string) (*api.Secret, error) {
177177
return &configSecret, nil
178178
},
179179
}
@@ -185,8 +185,8 @@ func TestGetRegistryDetailsGCRSuccess(t *testing.T) {
185185
`)
186186
data := map[string][]byte{"key.json": srvAccount}
187187
secret := api.Secret{Data: data}
188-
getter := &k8s.FakeSecretGetter{
189-
Fn: func(string) (*api.Secret, error) {
188+
getter := &k8s.FakeSecret{
189+
FnGet: func(string) (*api.Secret, error) {
190190
return &secret, nil
191191
},
192192
}
@@ -213,14 +213,14 @@ func TestGetRegistryDetailsGCRSuccess(t *testing.T) {
213213

214214
func TestGetRegistryDetailsGCRConfigErr(t *testing.T) {
215215
expectedErr := errors.New("get secret error")
216-
configGetter := &k8s.FakeSecretGetter{
217-
Fn: func(string) (*api.Secret, error) {
216+
configGetter := &k8s.FakeSecret{
217+
FnGet: func(string) (*api.Secret, error) {
218218
return &api.Secret{}, expectedErr
219219
},
220220
}
221221

222-
getter := &k8s.FakeSecretGetter{
223-
Fn: func(string) (*api.Secret, error) {
222+
getter := &k8s.FakeSecret{
223+
FnGet: func(string) (*api.Secret, error) {
224224
return &api.Secret{}, nil
225225
},
226226
}
@@ -256,14 +256,14 @@ func TestGetRegistryDetailsGCRSecretErr(t *testing.T) {
256256
configData := make(map[string][]byte)
257257
configData[api.DockerConfigJsonKey] = auth
258258
configSecret := api.Secret{Data: configData}
259-
configGetter := &k8s.FakeSecretGetter{
260-
Fn: func(string) (*api.Secret, error) {
259+
configGetter := &k8s.FakeSecret{
260+
FnGet: func(string) (*api.Secret, error) {
261261
return &configSecret, nil
262262
},
263263
}
264264

265-
getter := &k8s.FakeSecretGetter{
266-
Fn: func(string) (*api.Secret, error) {
265+
getter := &k8s.FakeSecret{
266+
FnGet: func(string) (*api.Secret, error) {
267267
return &api.Secret{}, expectedErr
268268
},
269269
}
@@ -299,16 +299,16 @@ func TestGetRegistryDetailsGCRJsonErr(t *testing.T) {
299299
configData := make(map[string][]byte)
300300
configData[api.DockerConfigJsonKey] = auth
301301
configSecret := api.Secret{Data: configData}
302-
configGetter := &k8s.FakeSecretGetter{
303-
Fn: func(string) (*api.Secret, error) {
302+
configGetter := &k8s.FakeSecret{
303+
FnGet: func(string) (*api.Secret, error) {
304304
return &configSecret, nil
305305
},
306306
}
307307

308308
data := map[string][]byte{"key.json": []byte("test")}
309309
secret := api.Secret{Data: data}
310-
getter := &k8s.FakeSecretGetter{
311-
Fn: func(string) (*api.Secret, error) {
310+
getter := &k8s.FakeSecret{
311+
FnGet: func(string) (*api.Secret, error) {
312312
return &secret, nil
313313
},
314314
}

0 commit comments

Comments
 (0)