kfctl mirror should recourse over dependencies (#342)

* The current implementation of kfctl mirror was not looking
  at dependent packages that were out of tree. This change
  causes the mirror command to look at the resources directive
  inside the kustomization.yaml and follow those links.

  * This is necessary to support blueprints and some of the refactoring
    of the kustomize manifests intended to better support Day 2 operations.

* Fix #340 kfctl mirror shouldn't hard code the directory to search
  for images but instead allow it to be provided via a flag.

* mirror pipelines should only apply the exclude prefix if it is the
  non empty string.
  * exclude will be empty if we want to match all images.

* On error print stack trace to help locate source of error.

  * If filename is empty throw an error as well.

* kfctl mirror needs to check bases as well.

cmd/kfctl/cmd/mirror_build.go

@@ -15,12 +15,16 @@ import (
 )
 
 var outputFileName string
+var directory string
 var gcb bool
 
 func init() {
 	replicateBuildCmd.Flags().StringVarP(&outputFileName, "output", "o", "",
 		`Name of the output pipeline file
 		kfctl alpha mirror build -o <name>`)
+	replicateBuildCmd.Flags().StringVarP(&directory, "directory", "d", "kustomize",
+		`The directory to search for kustomization files listing images to mirror
+		kfctl alpha mirror build -d <directory>`)
 	replicateBuildCmd.Flags().BoolVar(&gcb, "gcb", false, `Generate cloud build config`)
 	// verbose output
 	replicateBuildCmd.Flags().BoolP(string(kftypes.VERBOSE), "V", false,
@@ -57,6 +61,10 @@ Image replication rules are defined in config file.
 		if isRemoteFile {
 			return fmt.Errorf("config file path should be non-empty local file.")
 		}
+
+		if outputFileName == "" {
+			return fmt.Errorf("You must specify an output file with -o")
+		}
 		if _, err := os.Stat(configFile); err != nil {
 			return err
 		}
@@ -75,6 +83,7 @@ Image replication rules are defined in config file.
 			}
 
 		}
-		return mirror.GenerateMirroringPipeline(replication.Spec, outputFileName, gcb)
+
+		return mirror.GenerateMirroringPipeline(directory, replication.Spec, outputFileName, gcb)
 	},
 }

cmd/kfctl/cmd/root.go

@@ -59,7 +59,7 @@ func Execute(version string) {
 	VERSION = version
 
 	if err := rootCmd.Execute(); err != nil {
-		fmt.Println(err)
+		fmt.Printf("kfctl exited with error: %+v", err)
 		os.Exit(1)
 	}
 }

go.mod

@@ -54,6 +54,7 @@ require (
 	k8s.io/code-generator v0.18.1 // indirect
 	k8s.io/kubernetes v1.16.2
 	sigs.k8s.io/controller-runtime v0.4.0
+	sigs.k8s.io/kustomize/kyaml v0.1.10
 	sigs.k8s.io/kustomize/v3 v3.2.0
 	sigs.k8s.io/yaml v1.1.0
 )

go.sum

@@ -1393,6 +1393,8 @@ sigs.k8s.io/controller-runtime v0.2.0/go.mod h1:ZHqrRDZi3f6BzONcvlUxkqCKgwasGk5F
 sigs.k8s.io/controller-tools v0.2.2/go.mod h1:8SNGuj163x/sMwydREj7ld5mIMJu1cDanIfnx6xsU70=
 sigs.k8s.io/kustomize v2.0.3+incompatible h1:JUufWFNlI44MdtnjUqVnvh29rR37PQFzPbLXqhyOyX0=
 sigs.k8s.io/kustomize v2.0.3+incompatible/go.mod h1:MkjgH3RdOWrievjo6c9T245dYlB5QeXV4WCbnt/PEpU=
+sigs.k8s.io/kustomize/kyaml v0.1.10 h1:ZZfBnA/kYa9ZxUFIgI9oq3pWKzzA1gGOKgU0Hv/NhVg=
+sigs.k8s.io/kustomize/kyaml v0.1.10/go.mod h1:mPmeBSRy0LTMv6fSrYSoi2yIFNZVouGKDsTekE5kdhs=
 sigs.k8s.io/kustomize/kyaml v0.1.11 h1:/VvWxVIgH5gG1K4A7trgbyLgO3tRBiAWNhLFVU1HEmo=
 sigs.k8s.io/kustomize/kyaml v0.1.11/go.mod h1:72/rLkSi+L/pHM1oCjwrf3ClU+tH5kZQvvdLSqIHwWU=
 sigs.k8s.io/kustomize/v3 v3.2.0 h1:EKcEubO29vCbigcMoNynfyZH+ANWkML2UHWibt1Do7o=

pkg/mirror/mirror_image.go

@@ -5,12 +5,15 @@ import (
 	"github.com/ghodss/yaml"
 	mirrorv1alpha1 "github.com/kubeflow/kfctl/v3/pkg/apis/apps/imagemirror/v1alpha1"
 	"github.com/kubeflow/kfctl/v3/pkg/kfapp/kustomize"
+	"github.com/kubeflow/kfctl/v3/pkg/utils"
+	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
 	pipeline "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	cloudbuild "google.golang.org/api/cloudbuild/v1"
 	"io/ioutil"
 	"k8s.io/apimachinery/pkg/apis/meta/v1"
 	"os"
+	"path"
 	"path/filepath"
 	"regexp"
 	"sort"
@@ -28,14 +31,11 @@ const CLOUD_BUILD_FILE = "cloudbuild.yaml"
 type ReplicateTasks map[string]string
 
 // buildContext: gs://<GCS bucket>/<path to .tar.gz>
-func GenerateMirroringPipeline(spec mirrorv1alpha1.ReplicationSpec, outputFileName string, gcb bool) error {
+func GenerateMirroringPipeline(directory string, spec mirrorv1alpha1.ReplicationSpec, outputFileName string, gcb bool) error {
 	replicateTasks := make(ReplicateTasks)
-	if err := verifyCurrDir(); err != nil {
-		return err
-	}
 
 	for _, pattern := range spec.Patterns {
-		if err := replicateTasks.fillTasks(pattern.Dest, spec.Context, pattern.Src.Include, pattern.Src.Exclude); err != nil {
+		if err := replicateTasks.fillTasks(directory, pattern.Dest, spec.Context, pattern.Src.Include, pattern.Src.Exclude); err != nil {
 			return err
 		}
 	}
@@ -111,7 +111,7 @@ func GenerateMirroringPipeline(spec mirrorv1alpha1.ReplicationSpec, outputFileNa
 	}
 	writeErr := ioutil.WriteFile(outputFileName, buf, 0644)
 	if writeErr != nil {
-		return writeErr
+		return errors.WithStack(writeErr)
 	}
 	if gcb {
 		return generateCloudBuild(replicateTasks)
@@ -154,57 +154,104 @@ func (rt *ReplicateTasks) orderedKeys() []string {
 	return keys
 }
 
-func (rt *ReplicateTasks) fillTasks(registry string, buildContext string, include string, exclude string) error {
-	return filepath.Walk(KUSTOMIZE_FOLDER, func(path string, info os.FileInfo, err error) error {
-		if info.IsDir() {
-			absPath, err := filepath.Abs(path)
+// processKustomizeDir processes the specified kustomize directory
+func (rt *ReplicateTasks) processKustomizeDir(absPath string, registry string, include string, exclude string) error {
+	log.Infof("Processing %v", absPath)
+	kustomizationFilePath := filepath.Join(absPath, "kustomization.yaml")
+	if _, err := os.Stat(kustomizationFilePath); err != nil {
+		log.Infof("Skipping %v; no kustomization.yaml found", absPath)
+		return nil
+	}
+	kustomization := kustomize.GetKustomization(absPath)
+	for _, image := range kustomization.Images {
+		curName := image.Name
+		if image.NewName != "" {
+			curName = image.NewName
+		}
+		if strings.Contains(curName, "$") {
+			log.Infof("Image name %v contains kutomize parameter, skipping\n", curName)
+			continue
+		}
+		// check exclude first
+		if exclude != "" && strings.HasPrefix(curName, exclude) {
+			log.Infof("Image %v matches exclude prefix %v, skipping\n", curName, exclude)
+			continue
+		}
+		// then check include
+		if include != "" && (!strings.HasPrefix(curName, include)) {
+			log.Infof("Image %v doesn't match include prefix %v, skipping\n", curName, include)
+			continue
+		}
+		newName := strings.Join([]string{registry, image.Name}, "/")
+
+		if (image.NewTag == "") == (image.Digest == "") {
+			log.Warnf("One and only one of NewTag or Digest can exist for image %s, skipping\n",
+				image.Name)
+			continue
+		}
+
+		if image.NewTag != "" {
+			(*rt)[strings.Join([]string{newName, image.NewTag}, ":")] =
+				strings.Join([]string{curName, image.NewTag}, ":")
+		}
+		if image.Digest != "" {
+			(*rt)[strings.Join([]string{newName, image.Digest}, "@")] =
+				strings.Join([]string{curName, image.Digest}, "@")
+		}
+		log.Infof("Replacing image name from %s to %s", image.Name, newName)
+		//kustomization.Images[i].NewName = newName
+	}
+
+	// Process any kustomize packages we depend on.
+	for _, r := range kustomization.Resources {
+		if ext := strings.ToLower(filepath.Ext(r)); ext == ".yaml" || ext == ".yml" {
+			continue
+		}
+
+		p := path.Join(absPath, r)
+
+		if b, err := utils.IsRemoteFile(p); b || err != nil {
 			if err != nil {
-				return err
+				log.Infof("Skipping path %v; there was an error determining if it was a local file; error: %v", p, err)
+				continue
 			}
+			log.Infof("Skipping remote file %v", p)
+			continue
+		}
+		if err := rt.processKustomizeDir(p, registry, include, exclude); err != nil {
+			log.Errorf("Error occurred while processing %v; error %v", p, err)
+		}
+	}
 
-			kustomizationFilePath := filepath.Join(absPath, "kustomization.yaml")
-			if _, err := os.Stat(kustomizationFilePath); err == nil {
-				kustomization := kustomize.GetKustomization(absPath)
-				for _, image := range kustomization.Images {
-					curName := image.Name
-					if image.NewName != "" {
-						curName = image.NewName
-					}
-					if strings.Contains(curName, "$") {
-						log.Infof("Image name %v contains kutomize parameter, skipping\n", curName)
-						continue
-					}
-					// check exclude first
-					if strings.HasPrefix(curName, exclude) {
-						log.Infof("Image %v matches exclude prefix %v, skipping\n", curName, exclude)
-						continue
-					}
-					// then check include
-					if include != "" && (!strings.HasPrefix(curName, include)) {
-						log.Infof("Image %v doesn't match include prefix %v, skipping\n", curName, include)
-						continue
-					}
-					newName := strings.Join([]string{registry, image.Name}, "/")
+	// Bases is deprecated but our manifests still use it.
+	for _, r := range kustomization.Bases {
+		p := path.Join(absPath, r)
 
-					if (image.NewTag == "") == (image.Digest == "") {
-						log.Warnf("One and only one of NewTag or Digest can exist for image %s, skipping\n",
-							image.Name)
-						continue
-					}
+		if b, err := utils.IsRemoteFile(p); b || err != nil {
+			if err != nil {
+				log.Infof("Skipping path %v; there was an error determining if it was a local file; error: %v", p, err)
+				continue
+			}
+			log.Infof("Skipping remote file %v", p)
+			continue
+		}
 
-					if image.NewTag != "" {
-						(*rt)[strings.Join([]string{newName, image.NewTag}, ":")] =
-							strings.Join([]string{curName, image.NewTag}, ":")
-					}
-					if image.Digest != "" {
-						(*rt)[strings.Join([]string{newName, image.Digest}, "@")] =
-							strings.Join([]string{curName, image.Digest}, "@")
-					}
-					log.Infof("Replacing image name from %s to %s", image.Name, newName)
-					//kustomization.Images[i].NewName = newName
-				}
+		if err := rt.processKustomizeDir(p, registry, include, exclude); err != nil {
+			log.Errorf("Error occurred while processing %v; error %v", p, err)
+		}
+	}
+	return nil
+}
+
+func (rt *ReplicateTasks) fillTasks(directory string, registry string, buildContext string, include string, exclude string) error {
+	return filepath.Walk(directory, func(path string, info os.FileInfo, err error) error {
+		if info.IsDir() {
+			absPath, err := filepath.Abs(path)
+			if err != nil {
+				return err
 			}
-			return nil
+
+			return rt.processKustomizeDir(absPath, registry, include, exclude)
 		}
 
 		return nil
@@ -214,7 +261,7 @@ func (rt *ReplicateTasks) fillTasks(registry string, buildContext string, includ
 func verifyCurrDir() error {
 	infos, err := ioutil.ReadDir(".")
 	if err != nil {
-		return err
+		return errors.WithStack(err)
 	}
 	foundKus := false
 	for _, info := range infos {
@@ -235,7 +282,7 @@ func UpdateKustomize(inputFile string) error {
 	}
 	buf, err := ioutil.ReadFile(inputFile)
 	if err != nil {
-		return err
+		return errors.WithStack(err)
 	}
 	pipelineRun := pipeline.PipelineRun{}
 	if err := yaml.Unmarshal(buf, &pipelineRun); err != nil {
@@ -302,7 +349,7 @@ func UpdateKustomize(inputFile string) error {
 
 					writeErr := ioutil.WriteFile(kustomizationFilePath, data, 0644)
 					if writeErr != nil {
-						return writeErr
+						return errors.WithStack(writeErr)
 					}
 				}
 			}

pkg/mirror/mirror_image_test.go

@@ -3,9 +3,11 @@ package mirror
 import (
 	"bytes"
 	mirrorv1alpha1 "github.com/kubeflow/kfctl/v3/pkg/apis/apps/imagemirror/v1alpha1"
+	"github.com/kubeflow/kfctl/v3/pkg/utils"
 	log "github.com/sirupsen/logrus"
 	"io/ioutil"
 	"os"
+	"path"
 	"testing"
 )
 
@@ -35,7 +37,12 @@ func TestGenerateMirroringPipeline(t *testing.T) {
 		Context: "gs://kubeflow-examples/image-replicate/replicate-context.tar.gz",
 	}
 
-	if err := GenerateMirroringPipeline(spec, "pipeline.yaml", true); err != nil {
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Could not get working directory; error %v", err)
+	}
+	directory := path.Join(wd, "kustomize")
+	if err := GenerateMirroringPipeline(directory, spec, "pipeline.yaml", true); err != nil {
 		t.Error(err)
 	}
 	defer func(t *testing.T) {
@@ -94,6 +101,7 @@ func compFile(cases []testCase, t *testing.T) {
 			t.Error(err)
 		}
 		if !bytes.Equal(expectedBytes, actualBytes) {
+			utils.PrintDiff(string(actualBytes), string(expectedBytes))
 			t.Errorf("Result not matching; got\n%v\nwant\n%v", string(actualBytes), string(expectedBytes))
 		}
 	}

pkg/mirror/testdata/base-pkg/kustomization.yaml

@@ -0,0 +1,6 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+images:
+- name: docker.io/somerepo/image1
+  newName: docker.io/somerepo/image1
+  newTag: v0.16

pkg/mirror/testdata/expected-cloudbuild.yaml

@@ -9,6 +9,7 @@ images:
 - gcr.io/kubeflow-dev/docker.io/jaegertracing/all-in-one:1.9
 - gcr.io/kubeflow-dev/docker.io/kiali/kiali:v0.16
 - gcr.io/kubeflow-dev/docker.io/prom/prometheus:v2.3.1
+- gcr.io/kubeflow-dev/docker.io/somerepo/image1:v0.16
 - gcr.io/kubeflow-dev/grafana/grafana:6.0.2
 steps:
 - args:
@@ -101,6 +102,15 @@ steps:
   name: gcr.io/cloud-builders/docker
   waitFor:
   - '-'
+- args:
+  - build
+  - -t
+  - gcr.io/kubeflow-dev/docker.io/somerepo/image1:v0.16
+  - --build-arg=INPUT_IMAGE=docker.io/somerepo/image1:v0.16
+  - .
+  name: gcr.io/cloud-builders/docker
+  waitFor:
+  - '-'
 - args:
   - build
   - -t

pkg/mirror/testdata/expected-kustomization.yaml

@@ -37,3 +37,4 @@ kind: Kustomization
 namespace: kubeflow
 resources:
 - istio-noauth.yaml
+- ../base-pkg

pkg/mirror/testdata/expected-pipeline.yaml

@@ -112,7 +112,17 @@ spec:
         value: $(params.context)
       taskRef:
         name: mirror-image
-    - name: 10-grafana-grafana-6-0-2
+    - name: 10-docker-io-somerepo-image1-v0-16
+      params:
+      - name: inputImage
+        value: docker.io/somerepo/image1:v0.16
+      - name: outputImage
+        value: gcr.io/kubeflow-dev/docker.io/somerepo/image1:v0.16
+      - name: context
+        value: $(params.context)
+      taskRef:
+        name: mirror-image
+    - name: 11-grafana-grafana-6-0-2
       params:
       - name: inputImage
         value: grafana/grafana:6.0.2

pkg/mirror/testdata/kustomize/kustomization.yaml

@@ -2,6 +2,7 @@ apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
 resources:
 - istio-noauth.yaml
+- ../base-pkg
 namespace: kubeflow
 images:
 - name: docker.io/istio/kubectl