Repository: fluxcd/flux (go)

Issue:
Runtime panic while trying to upgrade to 1.20.0
**Describe the bug**

I'm experiencing a panic runtime error in flux while trying to upgrade from flux 1.19.0 to 1.20.0 (logs below). Pod goes into a CrashLoopBackOff. Rolling back the deployment to version 1.19.0 fixes the issue.

**To Reproduce**

Steps to reproduce the behaviour:

1. Update image version to `1.20.0`
``` yaml
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: flux
  namespace: weave
spec:
  replicas: 1
  selector:
    matchLabels:
      name: flux
  strategy:
    type: Recreate
  template:
    metadata:
      annotations:
        prometheus.io/port: "3031"
      labels:
        name: flux
    spec:
      nodeSelector:
        beta.kubernetes.io/os: linux
      serviceAccountName: flux
      volumes:
      - name: git-key
        secret:
          secretName: flux-git-deploy
          defaultMode: 0400
      - name: git-keygen
        emptyDir:
          medium: Memory
      containers:
      - name: flux
        image: docker.io/fluxcd/flux:1.20.0
        imagePullPolicy: Always
        resources:
          requests:
            cpu: 50m
            memory: 64Mi
        ports:
        - containerPort: 3030
        livenessProbe:
          httpGet:
            port: 3030
            path: /api/flux/v6/identity.pub
          initialDelaySeconds: 5
          timeoutSeconds: 5
        readinessProbe:
          httpGet:
            port: 3030
            path: /api/flux/v6/identity.pub
          initialDelaySeconds: 5
          timeoutSeconds: 5
        volumeMounts:
        - name: git-key
          mountPath: /etc/fluxd/ssh
          readOnly: true
        - name: git-keygen
          mountPath: /var/fluxd/keygen
        args:
        - --memcached-service=
        - --ssh-keygen-dir=/var/fluxd/keygen
        - --git-url=git@github.com:foobar/kubernetes
        - --git-path=staging
        - --git-label=flux-staging
        - --git-ci-skip
        - --connect=ws://127.0.0.1:3032
        - --registry-ecr-region=us-west-1,us-west-2
        - --registry-require=ecr
        - --sync-garbage-collection
```

**Expected behavior**

Version 1.20.0 of flux should run normally as expected.

**Logs**

```
ts=2020-07-13T21:10:36.084739153Z caller=main.go:259 version=1.20.0
ts=2020-07-13T21:10:36.084794615Z caller=main.go:412 msg="using kube config: \"/root/.kube/config\" to connect to the cluster"
ts=2020-07-13T21:10:36.113027449Z caller=main.go:492 component=cluster identity=/etc/fluxd/ssh/identity
ts=2020-07-13T21:10:36.113069413Z caller=main.go:493 component=cluster identity.pub="ssh-rsa foobar"
ts=2020-07-13T21:10:36.113140189Z caller=main.go:498 host=https://100.64.0.1:443 version=kubernetes-v1.16.10
ts=2020-07-13T21:10:36.113216873Z caller=main.go:510 kubectl=/usr/local/bin/kubectl
ts=2020-07-13T21:10:36.113854136Z caller=main.go:527 ping=true
ts=2020-07-13T21:10:56.4261769Z caller=aws.go:117 component=aws info="restricting ECR registry scans" regions="[us-west-1 us-west-2]" include-ids=[] exclude-ids="[602401143452 918309763551]"
ts=2020-07-13T21:10:56.427819718Z caller=main.go:666 url=ssh://git@github.com/foobar/kubernetes user="Weave Flux" email=support@weave.works signing-key= verify-signatures-mode=none sync-tag=flux-staging state=git readonly=false registry-disable-scanning=false notes-ref=flux-staging set-author=false git-secret=false sops=false
ts=2020-07-13T21:10:56.427886137Z caller=main.go:751 component=upstream URL=ws://127.0.0.1:3032
ts=2020-07-13T21:10:56.428414956Z caller=main.go:795 addr=:3030
ts=2020-07-13T21:10:56.429732472Z caller=upstream.go:133 component=upstream connecting=true
ts=2020-07-13T21:10:56.440429651Z caller=upstream.go:147 component=upstream connected=true
ts=2020-07-13T21:10:56.440660827Z caller=images.go:17 component=sync-loop msg="polling for new images for automated workloads"
ts=2020-07-13T21:10:56.440691559Z caller=images.go:27 component=sync-loop msg="no automated workloads"
ts=2020-07-13T21:10:56.440744694Z caller=loop.go:108 component=sync-loop err="loading last-synced resources: git repo not ready: git repo has not been cloned yet"
ts=2020-07-13T21:10:56.528869401Z caller=aws.go:212 component=aws info="attempting to refresh auth tokens" region=us-west-1 account-ids=1234
ts=2020-07-13T21:10:56.849358223Z caller=checkpoint.go:24 component=checkpoint msg="up to date" latest=1.20.0
ts=2020-07-13T21:11:07.826854457Z caller=loop.go:134 component=sync-loop event=refreshed url=ssh://git@github.com/foobar/kubernetes branch=master HEAD=b5e0cb1c1df856abcb09c05717221e82c2c24ab0
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x176c94f]

goroutine 104 [running]:
github.com/fluxcd/flux/pkg/cluster/kubernetes/resource.Load.func1(0xc002522030, 0x22, 0x0, 0x0, 0x20f5120, 0xc0024f4f90, 0x0, 0xc0024f4180)
	/home/circleci/go/src/github.com/fluxcd/flux/pkg/cluster/kubernetes/resource/load.go:32 +0x6f
path/filepath.Walk(0xc002522030, 0x22, 0xc0004bf6b0, 0x0, 0x0)
	/usr/local/go/src/path/filepath/path.go:402 +0x6a
github.com/fluxcd/flux/pkg/cluster/kubernetes/resource.Load(0xc0024725c0, 0x1a, 0xc001bb8f60, 0x1, 0x1, 0xc001bb8f00, 0xc0004e7020, 0xc0024725c0, 0xc0004bf790)
	/home/circleci/go/src/github.com/fluxcd/flux/pkg/cluster/kubernetes/resource/load.go:31 +0x19b
github.com/fluxcd/flux/pkg/cluster/kubernetes.(*manifests).LoadManifests(0xc0004e7020, 0xc0024725c0, 0x1a, 0xc001bb8f60, 0x1, 0x1, 0x42e0aa, 0x0, 0xc001bb7d40)
	/home/circleci/go/src/github.com/fluxcd/flux/pkg/cluster/kubernetes/manifests.go:122 +0x67
github.com/fluxcd/flux/pkg/manifests.(*rawFiles).GetAllResourcesByID(0xc001c16840, 0x213dda0, 0xc000042070, 0xc002402f60, 0x28, 0x21257a0)
	/home/circleci/go/src/github.com/fluxcd/flux/pkg/manifests/rawfiles.go:96 +0x60
github.com/fluxcd/flux/pkg/daemon.(*Daemon).getLastResources(0xc00017f680, 0x213dda0, 0xc000042070, 0x21256a0, 0xc000366a80, 0x0, 0x0, 0x0)
	/home/circleci/go/src/github.com/fluxcd/flux/pkg/daemon/sync.go:140 +0x176
github.com/fluxcd/flux/pkg/daemon.(*Daemon).Sync(0xc00017f680, 0x213dda0, 0xc000042070, 0x314a5f4a, 0xed69ec4eb, 0x0, 0xc002402e70, 0x28, 0x21256a0, 0xc000366a80, ...)
	/home/circleci/go/src/github.com/fluxcd/flux/pkg/daemon/sync.go:49 +0x82
github.com/fluxcd/flux/pkg/daemon.(*Daemon).Loop(0xc00017f680, 0xc000084240, 0xc000043460, 0x20f2f20, 0xc000675b00)
	/home/circleci/go/src/github.com/fluxcd/flux/pkg/daemon/loop.go:103 +0x525
created by main.main
	/home/circleci/go/src/github.com/fluxcd/flux/cmd/fluxd/main.go:777 +0x5990
```

**Additional context**

- Flux version: 1.20.0
- Kubernetes version: 1.16.10
- Git provider: Github
- Container registry provider: AWS ECR


Pull request:
Avoid panic when directory does not exist
The PR #1559 rearranged the filesystem walk during Load, so that it
only resulted in an error if there was a problem reading a YAML file
or non-chart directory (which might contain YAML files).

To decide whether a file is of interest, it first checks the stat to
see if it's a directory (in which case, recurse if not a chart ..) --
but if there's an error, that will be nil, and it will panic.

In general, you don't know if the file you can't read is (supposed to
be) a directory or a regular file, so there's no way to treat those
differently. Instead, this commit makes it check before walking that
the path supplied exists, then during the walk, ignore errors unless
it looks like a YAML file.

Fixes #3191.

Patch:
diff --git a/pkg/cluster/kubernetes/resource/load.go b/pkg/cluster/kubernetes/resource/load.go
index 13506b8b..dcfadab4 100644
--- a/pkg/cluster/kubernetes/resource/load.go
+++ b/pkg/cluster/kubernetes/resource/load.go
@@ -28,14 +28,18 @@ func Load(base string, paths []string, sopsEnabled bool) (map[string]KubeManifes
 		return nil, errors.Wrapf(err, "walking %q for chartdirs", base)
 	}
 	for _, root := range paths {
+		// In the walk, we ignore errors (indicating a failure to read
+		// a file) if it's not a file of interest. However, we _are_
+		// interested in the error if an explicitly-mentioned path
+		// does not exist.
+		if _, err := os.Stat(root); err != nil {
+			return nil, errors.Wrapf(err, "unable to read root path %q", root)
+		}
 		err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
-			if info.IsDir() {
+			if err == nil && info.IsDir() {
 				if charts.isDirChart(path) {
 					return filepath.SkipDir
 				}
-				if err != nil {
-					return errors.Wrapf(err, "walking dir %q for yaml files", path)
-				}
 				return nil
 			}
 


Test patch:
diff --git a/pkg/cluster/kubernetes/resource/load_test.go b/pkg/cluster/kubernetes/resource/load_test.go
index 409388bc..5c55be56 100644
--- a/pkg/cluster/kubernetes/resource/load_test.go
+++ b/pkg/cluster/kubernetes/resource/load_test.go
@@ -362,3 +362,15 @@ func TestLoadSomeWithSopsAllEncrypted(t *testing.T) {
 		assert.NotNil(t, objs[expected.String()], "expected to find %s in manifest map after decryption", expected)
 	}
 }
+
+func TestNoPanic(t *testing.T) {
+	dir, cleanup := testfiles.TempDir(t)
+	defer cleanup()
+	if err := testfiles.WriteTestFiles(dir, testfiles.Files); err != nil {
+		t.Fatal(err)
+	}
+	_, err := Load(dir, []string{filepath.Join(dir, "doesnotexist")}, true)
+	if err == nil {
+		t.Error("expected error (but not panic) when loading from directory that doesn't exist")
+	}
+}