chore(builder): use env replace creds volume

Author: duanhongyi

Dockerfile

@@ -27,7 +27,7 @@ RUN install-packages git openssh-server coreutils xz-utils tar \
   && install-stack jq $JQ_VERSION \
   && mkdir -p /var/run/sshd \
   && rm -rf /etc/ssh/ssh_host* \
-  && chmod +x /bin/create_bucket /bin/normalize_storage /docker-entrypoint.sh
+  && chmod +x /bin/create_bucket /docker-entrypoint.sh
 
 USER ${DRYCC_UID}
 WORKDIR ${DRYCC_HOME_DIR}

charts/builder/templates/_helpers.tpl

@@ -1,18 +1,11 @@
 {{- define "builder.envs" -}}
 env:
-# NOTE(bacongobbler): use drycc/registry_proxy to work around Docker --insecure-registry requirements
-- name: "DRYCC_REGISTRY_PROXY_HOST"
-  value: "127.0.0.1"
-- name: "DRYCC_REGISTRY_PROXY_PORT"
-  value: "{{ .Values.global.registryProxyPort }}"
 - name: "HEALTH_SERVER_PORT"
   value: "8092"
 - name: "EXTERNAL_PORT"
   value: "2223"
 - name: BUILDER_STORAGE
   value: "{{ .Values.global.storage }}"
-- name: "DRYCC_REGISTRY_LOCATION"
-  value: "{{ .Values.global.registryLocation }}"
 - name: "TTL_SECONDS_AFTER_FINISHED"
   value: "{{ .Values.global.ttlSecondsAfterFinished }}"
 # Set GIT_LOCK_TIMEOUT to number of minutes you want to wait to git push again to the same repository
@@ -34,15 +27,62 @@ env:
     secretKeyRef:
       name: builder-key-auth
       key: builder-key
+- name: "DRYCC_MINIO_LOOKUP"
+  valueFrom:
+    secretKeyRef:
+      name: minio-creds
+      key: lookup
+- name: "DRYCC_MINIO_BUCKET"
+  valueFrom:
+    secretKeyRef:
+      name: minio-creds
+      key: builder-bucket
+- name: "DRYCC_MINIO_ENDPOINT"
+  valueFrom:
+    secretKeyRef:
+      name: minio-creds
+      key: endpoint
+- name: "DRYCC_MINIO_ACCESSKEY"
+  valueFrom:
+    secretKeyRef:
+      name: minio-creds
+      key: accesskey
+- name: "DRYCC_MINIO_SECRETKEY"
+  valueFrom:
+    secretKeyRef:
+      name: minio-creds
+      key: secretkey
+- name: "DRYCC_REGISTRY_LOCATION"
+  value: "{{ .Values.global.registryLocation }}"
+- name: "DRYCC_REGISTRY_HOST"
+  valueFrom:
+    secretKeyRef:
+      name: registry-secret
+      key: host
+{{- if ne .Values.global.registryLocation "on-cluster" }}
+# NOTE(bacongobbler): use drycc/registry_proxy to work around Docker --insecure-registry requirements
+- name: "DRYCC_REGISTRY_PROXY_HOST"
+  value: {{ print "127.0.0.1"  ":" .Values.global.registryProxyPort }}
+{{- else }}
+- name: "DRYCC_REGISTRY_ORGANIZATION"
+  valueFrom:
+    secretKeyRef:
+      name: registry-secret
+      key: organization
+{{- end }}
+- name: "DRYCC_REGISTRY_USERNAME"
+  valueFrom:
+    secretKeyRef:
+      name: registry-secret
+      key: username
+- name: "DRYCC_REGISTRY_PASSWORD"
+  valueFrom:
+    secretKeyRef:
+      name: registry-secret
+      key: password
 {{- if (.Values.builder_pod_node_selector) }}
 - name: BUILDER_POD_NODE_SELECTOR
   value: {{.Values.builder_pod_node_selector}}
-{{- if eq .Values.global.minioLocation "on-cluster" }}
-- name: "DRYCC_MINIO_ENDPOINT"
-  value: http://${DRYCC_MINIO_SERVICE_HOST}:${DRYCC_MINIO_SERVICE_PORT}
-{{- else }}
-- name: "DRYCC_MINIO_ENDPOINT"
-  value: "{{ .Values.minio.endpoint }}"
 {{- end }}
 {{- end }}
 

charts/builder/templates/builder-deployment.yaml

@@ -63,9 +63,6 @@ spec:
             - name: builder-ssh-private-keys
               mountPath: /var/run/secrets/drycc/builder/ssh
               readOnly: true
-            - name: minio-creds
-              mountPath: /var/run/secrets/drycc/minio/creds
-              readOnly: true
             - name: imagebuilder-config
               mountPath: /etc/imagebuilder
               readOnly: true
@@ -76,9 +73,6 @@ spec:
         - name: builder-ssh-private-keys
           secret:
             secretName: builder-ssh-private-keys
-        - name: minio-creds
-          secret:
-            secretName: minio-creds
         - name: imagebuilder-config
           configMap:
             name: imagebuilder-config

pkg/conf/config.go

@@ -5,15 +5,17 @@ import (
 	"io/ioutil"
 	"net"
 	"net/url"
-	"os"
 	"strings"
 
 	"github.com/drycc/builder/pkg/sys"
 )
 
 const (
-	storageCredLocation = "/var/run/secrets/drycc/minio/creds/"
-	minioEndpointVar    = "DRYCC_MINIO_ENDPOINT"
+	minioLookupEnvVar    = "DRYCC_MINIO_LOOKUP"
+	minioBucketEnvVar    = "DRYCC_MINIO_BUCKET"
+	minioEndpointEnvVar  = "DRYCC_MINIO_ENDPOINT"
+	minioAccesskeyEnvVar = "DRYCC_MINIO_ACCESSKEY"
+	minioSecretkeyEnvVar = "DRYCC_MINIO_SECRETKEY"
 )
 
 // BuilderKeyLocation holds the path of the builder key secret.
@@ -35,33 +37,23 @@ func GetBuilderKey() (string, error) {
 // GetStorageParams returns the credentials required for connecting to object storage
 func GetStorageParams(env sys.Env) (Parameters, error) {
 	params := make(map[string]interface{})
-	params["builder-bucket"] = "builder" // default
-	files, err := ioutil.ReadDir(storageCredLocation)
-	if err != nil {
-		return nil, err
-	}
-
-	for _, file := range files {
-		if file.IsDir() || file.Name() == "..data" {
-			continue
-		}
-		data, err := ioutil.ReadFile(storageCredLocation + file.Name())
-		if err != nil {
-			return nil, err
-		}
 
-		params[file.Name()] = string(data)
-	}
-	params["bucket"] = params["builder-bucket"]
-	mEndpoint := env.Get(minioEndpointVar)
+	mEndpoint := env.Get(minioEndpointEnvVar)
 	params["regionendpoint"] = mEndpoint
 	region := "us-east-1" //region is required in distribution
 	if endpointURL, err := url.Parse(mEndpoint); err == nil {
 		if endpointURL.Hostname() != "" && net.ParseIP(endpointURL.Hostname()) == nil {
 			region = strings.Split(endpointURL.Hostname(), ".")[0]
 		}
 	}
-	os.Setenv("REGISTRY_STORAGE_S3_REGION", region)
+	params["region"] = region
+
+	params["accesskey"] = env.Get(minioAccesskeyEnvVar)
+	params["secretkey"] = env.Get(minioSecretkeyEnvVar)
+	params["bucket"] = env.Get(minioBucketEnvVar)
+	if env.Get(minioLookupEnvVar) == "path" {
+		params["forcepathstyle"] = "true"
+	}
 
 	return params, nil
 }

pkg/conf/config_test.go

@@ -3,7 +3,6 @@ package conf
 import (
 	"io/ioutil"
 	"os"
-	"os/user"
 	"path/filepath"
 	"testing"
 
@@ -12,73 +11,26 @@ import (
 )
 
 func TestGetStorageParams(t *testing.T) {
-	usr, err := user.Current()
-	if err != nil {
-		t.Logf("could not retrieve current user: %v", err)
-		t.SkipNow()
-	}
-	if usr.Uid != "0" {
-		t.Logf("current user does not have UID of zero (got %s) "+
-			"so cannot create storage cred location, skipping", usr.Uid)
-		t.SkipNow()
-	}
-
-	if err := os.MkdirAll(storageCredLocation, os.ModeDir); err != nil {
-		t.Fatalf("could not create storage cred location: %v", err)
-	}
-
-	// start by writing out a file to storageCredLocation
-	data := []byte("hello world\n")
-	if err := ioutil.WriteFile(storageCredLocation+"foo", data, 0644); err != nil {
-		t.Fatalf("could not write file to storage cred location: %v", err)
-	}
-
-	params, err := GetStorageParams(sys.NewFakeEnv())
-	if err != nil {
-		t.Errorf("received error while retrieving storage params: %v", err)
-	}
-
-	val, ok := params["foo"]
-	if !ok {
-		t.Error("key foo does not exist in storage params")
-	}
-	if val != string(data) {
-		t.Errorf("expected: %s got: %s", string(data), val)
-	}
-
-	// create a directory inside storage cred location, expecting it to pass
-	if err := os.Mkdir(storageCredLocation+"bar", os.ModeDir); err != nil {
-		t.Fatalf("could not create dir %s: %v", storageCredLocation+"bar", err)
-	}
-
-	_, err = GetStorageParams(sys.NewFakeEnv())
-	if err != nil {
-		t.Errorf("received error while retrieving storage params: %v", err)
-	}
-
-	// create the special "..data" directory symlink, expecting it to pass
-	if err := os.Symlink(storageCredLocation+"bar", storageCredLocation+"..data"); err != nil {
-		t.Fatalf("could not create dir symlink ..data -> %s: %v", storageCredLocation+"bar", err)
-	}
-
-	_, err = GetStorageParams(sys.NewFakeEnv())
-	if err != nil {
-		t.Errorf("received error while retrieving storage params: %v", err)
-	}
 
 	env := sys.NewFakeEnv()
 	env.Envs = map[string]string{
-		"BUILDER_STORAGE":      "minio",
-		"DRYCC_MINIO_ENDPOINT": "http://localhost:8088",
-	}
-	params, err = GetStorageParams(env)
+		"BUILDER_STORAGE":       "minio",
+		"DRYCC_MINIO_LOOKUP":    "path",
+		"DRYCC_MINIO_BUCKET":    "builder",
+		"DRYCC_MINIO_ENDPOINT":  "http://localhost:8088",
+		"DRYCC_MINIO_ACCESSKEY": "admin",
+		"DRYCC_MINIO_SECRETKEY": "adminpass",
+	}
+	params, err := GetStorageParams(env)
 	if err != nil {
 		t.Errorf("received error while retrieving storage params: %v", err)
 	}
+	assert.Equal(t, params["forcepathstyle"], "true", "forcepathstyle")
 	assert.Equal(t, params["regionendpoint"], "http://localhost:8088", "region endpoint")
-	assert.Equal(t, params["secure"], false, "secure")
-	assert.Equal(t, params["region"], "us-east-1", "region")
+	assert.Equal(t, params["region"], "localhost", "region")
 	assert.Equal(t, params["bucket"], "builder", "bucket")
+	assert.Equal(t, params["accesskey"], "admin", "accesskey")
+	assert.Equal(t, params["secretkey"], "adminpass", "secretkey")
 }
 
 func TestGetControllerClient(t *testing.T) {

pkg/gitreceive/build.go

@@ -149,16 +149,12 @@ func build(
 
 	imageName := fmt.Sprintf("%s:git-%s", appName, gitSha.Short())
 	buildJobName := imagebuilderJobName(appName, gitSha.Short())
-	registryLocation := conf.RegistryLocation
-	builderImageEnv := make(map[string]string)
-	if registryLocation != "on-cluster" {
-		builderImageEnv, err = getRegistryDetails(kubeClient.CoreV1(), &imageName, registryLocation, conf.PodNamespace)
-		if err != nil {
-			return fmt.Errorf("error getting private registry details %s", err)
-		}
+
+	builderImageEnv, err := getImagebuilderEnv(&imageName, conf, env)
+	if err != nil {
+		return fmt.Errorf("error getting private registry details %s", err)
 	}
 	builderImageEnv["DRYCC_STACK"] = stack["name"]
-	builderImageEnv["DRYCC_REGISTRY_LOCATION"] = registryLocation
 
 	job := createBuilderJob(
 		conf.Debug,
@@ -171,8 +167,6 @@ func build(
 		conf.StorageType,
 		builderName,
 		stack["image"],
-		conf.RegistryHost,
-		conf.RegistryPort,
 		builderImageEnv,
 		imagePullPolicy,
 		securityContext,
@@ -290,7 +284,7 @@ func buildBuilderPodNodeSelector(config string) (map[string]string, error) {
 		for _, line := range strings.Split(config, ",") {
 			param := strings.Split(line, ":")
 			if len(param) != 2 {
-				return nil, fmt.Errorf("Invalid BuilderPodNodeSelector value format: %s", config)
+				return nil, fmt.Errorf("invalid BuilderPodNodeSelector value format: %s", config)
 			}
 			selector[strings.TrimSpace(param[0])] = strings.TrimSpace(param[1])
 		}

pkg/gitreceive/config.go

@@ -14,11 +14,8 @@ const (
 // builder's git-receive hook.
 type Config struct {
 	// k8s service discovery env vars
-	ControllerHost   string `envconfig:"DRYCC_CONTROLLER_SERVICE_HOST" required:"true"`
-	ControllerPort   string `envconfig:"DRYCC_CONTROLLER_SERVICE_PORT" required:"true"`
-	RegistryHost     string `envconfig:"DRYCC_REGISTRY_PROXY_HOST" required:"true"`
-	RegistryPort     string `envconfig:"DRYCC_REGISTRY_PROXY_PORT" required:"true"`
-	RegistryLocation string `envconfig:"DRYCC_REGISTRY_LOCATION" default:"on-cluster"`
+	ControllerHost string `envconfig:"DRYCC_CONTROLLER_SERVICE_HOST" required:"true"`
+	ControllerPort string `envconfig:"DRYCC_CONTROLLER_SERVICE_PORT" required:"true"`
 
 	GitHome                       string `envconfig:"GIT_HOME" required:"true"`
 	SSHConnection                 string `envconfig:"SSH_CONNECTION" required:"true"`

pkg/gitreceive/imagebuilder.go

@@ -0,0 +1,61 @@
+package gitreceive
+
+import (
+	"errors"
+	"fmt"
+
+	"github.com/drycc/builder/pkg/sys"
+)
+
+var (
+	requiredEnvNames = []string{
+		"DRYCC_MINIO_LOOKUP",
+		"DRYCC_MINIO_BUCKET",
+		"DRYCC_MINIO_ENDPOINT",
+		"DRYCC_REGISTRY_HOST",
+	}
+)
+
+func checkImagebuilderRequiredEnv(imagebuilderEnv map[string]string) error {
+	for index := range requiredEnvNames {
+		envName := requiredEnvNames[index]
+		if _, hasKey := imagebuilderEnv[envName]; !hasKey {
+			msg := fmt.Sprintf("the environment variable %s is required", envName)
+			return errors.New(msg)
+		}
+	}
+	if imagebuilderEnv["DRYCC_REGISTRY_LOCATION"] == "off-cluster" {
+		if imagebuilderEnv["DRYCC_REGISTRY_ORGANIZATION"] == "" {
+			return errors.New("the environment variable DRYCC_REGISTRY_ORGANIZATION is required")
+		}
+	} else {
+		if imagebuilderEnv["DRYCC_REGISTRY_PROXY_HOST"] == "" {
+			return errors.New("the environment variable DRYCC_REGISTRY_PROXY_HOST is required")
+		}
+	}
+	return nil
+}
+
+func getImagebuilderEnv(image *string, config *Config, env sys.Env) (map[string]string, error) {
+	imagebuilderEnv := env.Environ([]string{"DRYCC_REGISTRY_", "DRYCC_MINIO_"})
+	if err := checkImagebuilderRequiredEnv(imagebuilderEnv); err != nil {
+		return nil, err
+	}
+	if imagebuilderEnv["DRYCC_REGISTRY_LOCATION"] == "off-cluster" {
+		*image = fmt.Sprintf(
+			"%s/%s/%s",
+			imagebuilderEnv["DRYCC_REGISTRY_HOST"],
+			imagebuilderEnv["DRYCC_REGISTRY_ORGANIZATION"],
+			*image,
+		)
+	} else {
+		imagebuilderEnv["DRYCC_REGISTRY_ORGANIZATION"] = config.App()
+		*image = fmt.Sprintf(
+			"%s/%s/%s",
+			imagebuilderEnv["DRYCC_REGISTRY_PROXY_HOST"],
+			imagebuilderEnv["DRYCC_REGISTRY_ORGANIZATION"],
+			*image,
+		)
+	}
+	return imagebuilderEnv, nil
+}