(feat) netrc envs are set through k8s secrets (#67)

* netrc envs are set through k8s secrets
* rework of compile unit tests for netrc

engine/compiler/compiler.go

@@ -14,6 +14,7 @@ import (
 	"github.com/drone-runners/drone-runner-kube/engine/resource"
 	"github.com/drone-runners/drone-runner-kube/internal/docker/image"
 
+	"github.com/drone/drone-go/drone"
 	"github.com/drone/runner-go/clone"
 	"github.com/drone/runner-go/container"
 	"github.com/drone/runner-go/environ"
@@ -319,12 +320,6 @@ func (c *Compiler) Compile(ctx context.Context, args runtime.CompilerArgs) runti
 		envs["DRONE_DOCKER_VOLUME_PATH"] = workVolume.HostPath.Path
 	}
 
-	// create the .netrc environment variables if not
-	// explicitly disabled
-	if c.NetrcCloneOnly == false {
-		envs = environ.Combine(envs, environ.Netrc(args.Netrc))
-	}
-
 	// create tmate variables
 	if c.Tmate.Server != "" {
 		envs["DRONE_TMATE_HOST"] = c.Tmate.Server
@@ -366,12 +361,6 @@ func (c *Compiler) Compile(ctx context.Context, args runtime.CompilerArgs) runti
 		step.Volumes = append(step.Volumes, workMount, statusMount)
 		spec.Steps = append(spec.Steps, step)
 
-		// always set the .netrc file for the clone step.
-		// note that environment variables are only set
-		// if the .netrc file is not nil (it will always
-		// be nil for public repositories).
-		step.Envs = environ.Combine(step.Envs, environ.Netrc(args.Netrc))
-
 		// override default clone image.
 		if c.Cloner != "" {
 			step.Image = c.Cloner
@@ -493,7 +482,13 @@ func (c *Compiler) Compile(ctx context.Context, args runtime.CompilerArgs) runti
 		removeCloneDeps(spec)
 	}
 
-	for _, step := range append(spec.Steps, spec.Internal...) {
+	hasNetrc := packNetrcSecrets(spec, args.Netrc)
+
+	for stepIdx, step := range append(spec.Steps, spec.Internal...) {
+		if hasNetrc && (stepIdx == 0 && c.NetrcCloneOnly || !c.NetrcCloneOnly) {
+			setNetrcSecretsToStep(step, spec)
+		}
+
 		for _, s := range step.Secrets {
 			// if the secret was already fetched and stored in the
 			// secret map it can be skipped.
@@ -509,6 +504,14 @@ func (c *Compiler) Compile(ctx context.Context, args runtime.CompilerArgs) runti
 				}
 				spec.Secrets[s.Name] = s
 				step.SpecSecrets = append(step.SpecSecrets, s)
+			} else {
+				s := &engine.Secret{
+					Name: s.Name,
+					Data: "",
+					Mask: false,
+				}
+				spec.Secrets[s.Name] = s
+				step.SpecSecrets = append(step.SpecSecrets, s)
 			}
 		}
 	}
@@ -746,6 +749,45 @@ func (c *Compiler) findSecret(ctx context.Context, args runtime.CompilerArgs, na
 	return found.Data, true
 }
 
+const (
+	envNetrcMachine  = "DRONE_NETRC_MACHINE"
+	envNetrcUsername = "DRONE_NETRC_USERNAME"
+	envNetrcPassword = "DRONE_NETRC_PASSWORD"
+	envNetrcFile     = "DRONE_NETRC_FILE"
+)
+
+// packNetrcSecrets is helper function that packs kubernetes secrets required for netrc to engine.Spec.
+// The function returns true if netrc secrets are set and false if netrc is empty.
+func packNetrcSecrets(spec *engine.Spec, netrc *drone.Netrc) bool {
+	if netrc == nil || netrc.Machine == "" {
+		return false
+	}
+
+	fileData := fmt.Sprintf(
+		"machine %s login %s password %s",
+		netrc.Machine,
+		netrc.Login,
+		netrc.Password)
+
+	spec.Secrets[envNetrcMachine] = &engine.Secret{Name: envNetrcMachine, Data: netrc.Machine}
+	spec.Secrets[envNetrcUsername] = &engine.Secret{Name: envNetrcUsername, Data: netrc.Login, Mask: true}
+	spec.Secrets[envNetrcPassword] = &engine.Secret{Name: envNetrcPassword, Data: netrc.Password, Mask: true}
+	spec.Secrets[envNetrcFile] = &engine.Secret{Name: envNetrcFile, Data: fileData}
+
+	return true
+}
+
+// setNetrcSecretsToStep is a helper function that sets netrc secrets to a engine.Step
+func setNetrcSecretsToStep(step *engine.Step, spec *engine.Spec) {
+	envVars := []string{envNetrcMachine, envNetrcUsername, envNetrcPassword, envNetrcFile}
+	for _, envVar := range envVars {
+		if v, ok := spec.Secrets[envVar]; ok {
+			step.Secrets = append(step.Secrets, &engine.SecretVar{Name: v.Name, Env: v.Name})
+			step.SpecSecrets = append(step.SpecSecrets, v)
+		}
+	}
+}
+
 func divideIntEqually(amount int64, count int, units int64) []int64 {
 	if count <= 0 {
 		return nil

engine/compiler/compiler_test.go

@@ -12,6 +12,7 @@ import (
 	"io/ioutil"
 	"os"
 	"reflect"
+	"sort"
 	"testing"
 
 	"github.com/dchest/uniuri"
@@ -135,24 +136,17 @@ func TestCompile_Secrets(t *testing.T) {
 	// pipeline step.
 	{
 		got := ir.Steps[0].Secrets
+		// sort to avoid changed order
+		sort.Slice(got, func(i, j int) bool {
+			return got[i].Name < got[j].Name
+		})
+
 		want := []*engine.SecretVar{
-			{
-				Name: "my_password",
-				Env:  "PASSWORD",
-				// Data: nil, // secret not found, data nil
-				// Mask: true,
-			},
-			{
-				Name: "my_username",
-				Env:  "USERNAME",
-				// Data: []byte("octocat"), // secret found
-				// Mask: true,
-			},
+			{Name: "my_password", Env: "PASSWORD"},
+			{Name: "my_username", Env: "USERNAME"},
 		}
 		if diff := cmp.Diff(got, want); len(diff) != 0 {
-			// TODO(bradrydzewski) ordering is not guaranteed. this
-			// unit tests needs to be adjusted accordingly.
-			t.Skipf(diff)
+			t.Error(diff)
 		}
 	}
 
@@ -164,7 +158,7 @@ func TestCompile_Secrets(t *testing.T) {
 			"my_password": {
 				Name: "my_password",
 				Data: "", // secret not found, data empty
-				Mask: true,
+				Mask: false,
 			},
 			"my_username": {
 				Name: "my_username",
@@ -173,9 +167,7 @@ func TestCompile_Secrets(t *testing.T) {
 			},
 		}
 		if diff := cmp.Diff(got, want); len(diff) != 0 {
-			// TODO(bradrydzewski) ordering is not guaranteed. this
-			// unit tests needs to be adjusted accordingly.
-			t.Skipf(diff)
+			t.Error(diff)
 		}
 	}
 }
@@ -234,16 +226,87 @@ func testCompile(t *testing.T, source, golden string) *engine.Spec {
 
 	opts := cmp.Options{
 		cmpopts.IgnoreUnexported(engine.Spec{}),
-		cmpopts.IgnoreFields(engine.Step{}, "Envs", "Secrets"),
+		cmpopts.IgnoreFields(engine.Spec{}, "Secrets"),
+		cmpopts.IgnoreFields(engine.Step{}, "Envs", "Secrets", "SpecSecrets"),
 		cmpopts.IgnoreFields(engine.PodSpec{}, "Annotations", "Labels"),
 	}
 	if diff := cmp.Diff(got, want, opts...); len(diff) != 0 {
-		t.Errorf(diff)
+		t.Error(diff)
 	}
 
 	return got.(*engine.Spec)
 }
 
+func TestCompile_Netrc(t *testing.T) {
+	testCompileNetrc(t, "testdata/netrc.yml", "testdata/netrc.json", false)
+}
+
+func TestCompile_NetrcOnly(t *testing.T) {
+	testCompileNetrc(t, "testdata/netrc.yml", "testdata/netrc_only_clone.json", true)
+}
+
+// This test verifies that netrc values are properly defined as kubernetes secrets.
+func testCompileNetrc(t *testing.T, source, golden string, netRcCloneOnly bool) {
+	// replace the default random function with one that
+	// is deterministic, for testing purposes.
+	random = notRandom
+
+	// restore the default random function and the previously
+	// specified temporary directory
+	defer func() {
+		random = uniuri.New
+	}()
+
+	manifest, err := manifest.ParseFile(source)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	compiler := &Compiler{
+		Environ:  provider.Static(nil),
+		Registry: registry.Static(nil),
+		Secret: secret.StaticVars(map[string]string{
+			"token":       "3DA541559918A808C2402BBA5012F6C60B27661C",
+			"password":    "password",
+			"my_username": "octocat",
+		}),
+		NetrcCloneOnly: netRcCloneOnly,
+	}
+	args := runtime.CompilerArgs{
+		Repo:     &drone.Repo{},
+		Build:    &drone.Build{Target: "master"},
+		Stage:    &drone.Stage{},
+		System:   &drone.System{},
+		Netrc:    &drone.Netrc{Machine: "github.com", Login: "octocat", Password: "correct-horse-battery-staple"},
+		Manifest: manifest,
+		Pipeline: manifest.Resources[0].(*resource.Pipeline),
+		Secret:   secret.Static(nil),
+	}
+
+	got := compiler.Compile(nocontext, args)
+
+	raw, err := ioutil.ReadFile(golden)
+	if err != nil {
+		t.Error(err)
+	}
+
+	want := new(engine.Spec)
+	err = json.Unmarshal(raw, want)
+	if err != nil {
+		t.Error(err)
+	}
+
+	opts := cmp.Options{
+		cmpopts.IgnoreUnexported(engine.Spec{}),
+		cmpopts.IgnoreFields(engine.Step{}, "Envs"),
+		cmpopts.IgnoreFields(engine.PodSpec{}, "Annotations", "Labels"),
+	}
+	if diff := cmp.Diff(got, want, opts...); len(diff) != 0 {
+		t.Error(diff)
+	}
+}
+
 func dump(v interface{}) {
 	enc := json.NewEncoder(os.Stdout)
 	enc.SetIndent("", "  ")

engine/compiler/testdata/netrc.json

@@ -0,0 +1,110 @@
+{
+  "platform": {},
+  "pod_spec": {
+    "name": "random",
+    "labels": {},
+    "annotations": {}
+  },
+  "steps": [
+    {
+      "id": "random",
+      "environment": {},
+      "image": "drone/git:latest",
+      "placeholder": "drone/placeholder:1",
+      "labels": {},
+      "name": "clone",
+      "run_policy": "always",
+      "secrets": [
+        { "name": "DRONE_NETRC_MACHINE", "env": "DRONE_NETRC_MACHINE" },
+        { "name": "DRONE_NETRC_USERNAME", "env": "DRONE_NETRC_USERNAME" },
+        { "name": "DRONE_NETRC_PASSWORD", "env": "DRONE_NETRC_PASSWORD" },
+        { "name": "DRONE_NETRC_FILE", "env": "DRONE_NETRC_FILE" }
+      ],
+      "spec_secrets": [
+        { "name": "DRONE_NETRC_MACHINE", "data": "github.com" },
+        { "name": "DRONE_NETRC_USERNAME", "data": "octocat", "mask": true },
+        { "name": "DRONE_NETRC_PASSWORD", "data": "correct-horse-battery-staple", "mask": true },
+        { "name": "DRONE_NETRC_FILE", "data": "machine github.com login octocat password correct-horse-battery-staple" }
+      ],
+      "volumes": [
+        {
+          "name": "_workspace",
+          "path": "/drone/src"
+        },
+        {
+          "name": "_status",
+          "path": "/run/drone"
+        }
+      ],
+      "working_dir": "/drone/src"
+    },
+    {
+      "id": "random",
+      "args": [
+        "echo \"$DRONE_SCRIPT\" | /bin/sh"
+      ],
+      "depends_on": [
+        "clone"
+      ],
+      "entrypoint": [
+        "/bin/sh",
+        "-c"
+      ],
+      "environment": {},
+      "labels": {},
+      "name": "build",
+      "image": "docker.io/library/golang:latest",
+      "placeholder": "drone/placeholder:1",
+      "secrets": [
+        { "name": "DRONE_NETRC_MACHINE", "env": "DRONE_NETRC_MACHINE" },
+        { "name": "DRONE_NETRC_USERNAME", "env": "DRONE_NETRC_USERNAME" },
+        { "name": "DRONE_NETRC_PASSWORD", "env": "DRONE_NETRC_PASSWORD" },
+        { "name": "DRONE_NETRC_FILE", "env": "DRONE_NETRC_FILE" }
+      ],
+      "spec_secrets": [
+        { "name": "DRONE_NETRC_MACHINE", "data": "github.com" },
+        { "name": "DRONE_NETRC_USERNAME", "data": "octocat", "mask": true },
+        { "name": "DRONE_NETRC_PASSWORD", "data": "correct-horse-battery-staple", "mask": true },
+        { "name": "DRONE_NETRC_FILE", "data": "machine github.com login octocat password correct-horse-battery-staple" }
+      ],
+      "volumes": [
+        {
+          "name": "_workspace",
+          "path": "/drone/src"
+        },
+        {
+          "name": "_status",
+          "path": "/run/drone"
+        }
+      ],
+      "working_dir": "/drone/src"
+    }
+  ],
+  "volumes": [
+    {
+      "temp": {
+        "id": "random",
+        "name": "_workspace",
+        "labels": {}
+      }
+    },
+    {
+      "downward_api": {
+        "id": "random",
+        "name": "_status",
+        "items": [
+          {
+            "path": "env",
+            "field_path": "metadata.annotations"
+          }
+        ]
+      }
+    }
+  ],
+  "secrets": {
+    "DRONE_NETRC_MACHINE": { "name": "DRONE_NETRC_MACHINE", "data": "github.com" },
+    "DRONE_NETRC_USERNAME": { "name": "DRONE_NETRC_USERNAME", "data": "octocat", "mask": true },
+    "DRONE_NETRC_PASSWORD": { "name": "DRONE_NETRC_PASSWORD", "data": "correct-horse-battery-staple", "mask": true },
+    "DRONE_NETRC_FILE": { "name": "DRONE_NETRC_FILE", "data": "machine github.com login octocat password correct-horse-battery-staple" }
+  }
+}

engine/compiler/testdata/netrc.yml

@@ -0,0 +1,9 @@
+kind: pipeline
+type: kubernetes
+name: default
+
+steps:
+- name: build
+  image: golang
+  commands:
+  - go build