Implement defining remote git dependencies in the skaffold configurat…

…ion. (#5361)

cmd/skaffold/app/cmd/flags.go

@@ -120,6 +120,14 @@ var flagRegistry = []Flag{
 		FlagAddMethod: "StringVar",
 		DefinedOn:     []string{"dev", "build", "run", "debug"},
 	},
+	{
+		Name:          "remote-cache-dir",
+		Usage:         "Specify the location of the git repositories cache (default $HOME/.skaffold/repos)",
+		Value:         &opts.RepoCacheDir,
+		DefValue:      "",
+		FlagAddMethod: "StringVar",
+		DefinedOn:     []string{"all"},
+	},
 	{
 		Name:          "insecure-registry",
 		Usage:         "Target registries for built images which are not secure",

cmd/skaffold/app/cmd/parse_config.go

@@ -24,7 +24,10 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/sirupsen/logrus"
+
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/git"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/defaults"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
@@ -45,9 +48,20 @@ type configOpts struct {
 	isDependency bool
 }
 
+// record captures the state of referenced configs.
+type record struct {
+	appliedProfiles  map[string]string      // config -> list of applied profiles
+	configNameToFile map[string]string      // configName -> file path
+	cachedRepos      map[string]interface{} // git repo -> cache path or error
+}
+
+func newRecord() *record {
+	return &record{appliedProfiles: make(map[string]string), configNameToFile: make(map[string]string), cachedRepos: make(map[string]interface{})}
+}
+
 func getAllConfigs(opts config.SkaffoldOptions) ([]*latest.SkaffoldConfig, error) {
 	cOpts := configOpts{file: opts.ConfigurationFile, selection: nil, profiles: opts.Profiles, isRequired: false, isDependency: false}
-	cfgs, err := getConfigs(cOpts, opts, make(map[string]string), make(map[string]string))
+	cfgs, err := getConfigs(cOpts, opts, newRecord())
 	if err != nil {
 		return nil, err
 	}
@@ -61,7 +75,7 @@ func getAllConfigs(opts config.SkaffoldOptions) ([]*latest.SkaffoldConfig, error
 }
 
 // getConfigs recursively parses all configs and their dependencies in the specified `skaffold.yaml`
-func getConfigs(cfgOpts configOpts, opts config.SkaffoldOptions, appliedProfiles map[string]string, configNameToFile map[string]string) ([]*latest.SkaffoldConfig, error) {
+func getConfigs(cfgOpts configOpts, opts config.SkaffoldOptions, r *record) ([]*latest.SkaffoldConfig, error) {
 	parsed, err := schema.ParseConfigAndUpgrade(cfgOpts.file, latest.Version)
 	if err != nil {
 		return nil, err
@@ -80,7 +94,7 @@ func getConfigs(cfgOpts configOpts, opts config.SkaffoldOptions, appliedProfiles
 	var configs []*latest.SkaffoldConfig
 	for i, cfg := range parsed {
 		config := cfg.(*latest.SkaffoldConfig)
-		processed, err := processEachConfig(config, cfgOpts, opts, appliedProfiles, configNameToFile, i)
+		processed, err := processEachConfig(config, cfgOpts, opts, r, i)
 		if err != nil {
 			return nil, err
 		}
@@ -90,14 +104,14 @@ func getConfigs(cfgOpts configOpts, opts config.SkaffoldOptions, appliedProfiles
 }
 
 // processEachConfig processes each parsed config by applying profiles and recursively processing it's dependencies
-func processEachConfig(config *latest.SkaffoldConfig, cfgOpts configOpts, opts config.SkaffoldOptions, appliedProfiles map[string]string, configNameToFile map[string]string, index int) ([]*latest.SkaffoldConfig, error) {
+func processEachConfig(config *latest.SkaffoldConfig, cfgOpts configOpts, opts config.SkaffoldOptions, r *record, index int) ([]*latest.SkaffoldConfig, error) {
 	// check that the same config name isn't repeated in multiple files.
 	if config.Metadata.Name != "" {
-		prevConfig, found := configNameToFile[config.Metadata.Name]
+		prevConfig, found := r.configNameToFile[config.Metadata.Name]
 		if found && prevConfig != cfgOpts.file {
 			return nil, fmt.Errorf("skaffold config named %q found in multiple files: %q and %q", config.Metadata.Name, prevConfig, cfgOpts.file)
 		}
-		configNameToFile[config.Metadata.Name] = cfgOpts.file
+		r.configNameToFile[config.Metadata.Name] = cfgOpts.file
 	}
 
 	// configSelection specifies the exact required configs in this file. Empty configSelection means that all configs are required.
@@ -124,13 +138,13 @@ func processEachConfig(config *latest.SkaffoldConfig, cfgOpts configOpts, opts c
 	}
 
 	sort.Strings(profiles)
-	if revisit, err := checkRevisit(config, profiles, appliedProfiles, cfgOpts.file, required, index); revisit {
+	if revisit, err := checkRevisit(config, profiles, r.appliedProfiles, cfgOpts.file, required, index); revisit {
 		return nil, err
 	}
 
 	var configs []*latest.SkaffoldConfig
 	for _, d := range config.Dependencies {
-		depConfigs, err := processEachDependency(d, cfgOpts.file, required, profiles, opts, appliedProfiles, configNameToFile)
+		depConfigs, err := processEachDependency(d, cfgOpts.file, required, profiles, opts, r)
 		if err != nil {
 			return nil, err
 		}
@@ -144,7 +158,7 @@ func processEachConfig(config *latest.SkaffoldConfig, cfgOpts configOpts, opts c
 }
 
 // processEachDependency parses a config dependency with the calculated set of activated profiles.
-func processEachDependency(d latest.ConfigDependency, fPath string, required bool, profiles []string, opts config.SkaffoldOptions, appliedProfiles, configNameToFile map[string]string) ([]*latest.SkaffoldConfig, error) {
+func processEachDependency(d latest.ConfigDependency, fPath string, required bool, profiles []string, opts config.SkaffoldOptions, r *record) ([]*latest.SkaffoldConfig, error) {
 	var depProfiles []string
 	for _, ap := range d.ActiveProfiles {
 		if len(ap.ActivatedBy) == 0 {
@@ -159,27 +173,60 @@ func processEachDependency(d latest.ConfigDependency, fPath string, required boo
 		}
 	}
 	path := d.Path
+
+	if d.GitRepo != nil {
+		cachePath, err := cacheRepo(*d.GitRepo, opts, r)
+		if err != nil {
+			return nil, fmt.Errorf("caching remote dependency %s: %w", d.GitRepo.Repo, err)
+		}
+		path = cachePath
+	}
+
 	if path == "" {
 		// empty path means configs in the same file
 		path = fPath
 	}
 	fi, err := os.Stat(path)
 	if err != nil {
 		if os.IsNotExist(errors.Unwrap(err)) {
-			return nil, fmt.Errorf("could not find skaffold config %s that is referenced as a dependency in config %s", d.Path, fPath)
+			return nil, fmt.Errorf("could not find skaffold config %s that is referenced as a dependency in config %s", path, fPath)
 		}
 		return nil, fmt.Errorf("parsing dependencies for skaffold config %s: %w", fPath, err)
 	}
 	if fi.IsDir() {
 		path = filepath.Join(path, "skaffold.yaml")
 	}
-	depConfigs, err := getConfigs(configOpts{file: path, selection: d.Names, profiles: depProfiles, isRequired: required, isDependency: path != fPath}, opts, appliedProfiles, configNameToFile)
+	depConfigs, err := getConfigs(configOpts{file: path, selection: d.Names, profiles: depProfiles, isRequired: required, isDependency: path != fPath}, opts, r)
 	if err != nil {
 		return nil, err
 	}
 	return depConfigs, nil
 }
 
+// cacheRepo downloads the referenced git repository to skaffold's cache if required and returns the path to the target configuration file in that repository.
+func cacheRepo(g latest.GitInfo, opts config.SkaffoldOptions, r *record) (string, error) {
+	key := fmt.Sprintf("%s@%s", g.Repo, g.Ref)
+	if p, found := r.cachedRepos[key]; found {
+		switch v := p.(type) {
+		case string:
+			return filepath.Join(v, g.Path), nil
+		case error:
+			return "", v
+		default:
+			logrus.Fatalf("unable to check download status of repo %s at ref %s", g.Repo, g.Ref)
+			return "", nil
+		}
+	} else {
+		p, err := git.SyncRepo(g, opts)
+		if err != nil {
+			r.cachedRepos[key] = err
+			return "", err
+		}
+		r.cachedRepos[key] = p
+		return filepath.Join(p, g.Path), nil
+	}
+}
+
 // checkRevisit ensures that each config is activated with the same set of active profiles
 // It returns true if this config was visited once before. It additionally returns an error if the previous visit was with a different set of active profiles.
 func checkRevisit(config *latest.SkaffoldConfig, profiles []string, appliedProfiles map[string]string, file string, required bool, index int) (bool, error) {

cmd/skaffold/app/cmd/parse_config_test.go

@@ -26,14 +26,15 @@ import (
 	"github.com/google/go-cmp/cmp"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/git"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
 const (
 	template = `
-apiVersion: skaffold/v2beta11
+apiVersion: %s
 kind: Config
 metadata:
   name: %s
@@ -329,6 +330,37 @@ requires:
 			},
 			err: errors.New("did not find any configs matching selection [cfg3]"),
 		},
+		{
+			description: "remote dependencies",
+			documents: []document{
+				{path: "skaffold.yaml", configs: []mockCfg{{name: "cfg00", requiresStanza: `
+requires:
+  - path: doc1
+`}, {name: "cfg01", requiresStanza: ""}}},
+				{path: "doc1/skaffold.yaml", configs: []mockCfg{{name: "cfg10", requiresStanza: `
+requires:
+  - git:
+      repo: doc2
+      path: skaffold.yaml
+      ref: main
+    configs: [cfg21]
+`}, {name: "cfg11", requiresStanza: `
+requires:
+  - git:
+      repo: doc2
+      ref: main
+    configs: [cfg21]
+`}}},
+				{path: "doc2/skaffold.yaml", configs: []mockCfg{{name: "cfg20", requiresStanza: ""}, {name: "cfg21", requiresStanza: ""}}},
+			},
+			expected: []*latest.SkaffoldConfig{
+				createCfg("cfg21", "image21", "doc2", nil),
+				createCfg("cfg10", "image10", "doc1", []latest.ConfigDependency{{GitRepo: &latest.GitInfo{Repo: "doc2", Path: "skaffold.yaml", Ref: "main"}, Names: []string{"cfg21"}}}),
+				createCfg("cfg11", "image11", "doc1", []latest.ConfigDependency{{GitRepo: &latest.GitInfo{Repo: "doc2", Ref: "main"}, Names: []string{"cfg21"}}}),
+				createCfg("cfg00", "image00", ".", []latest.ConfigDependency{{Path: "doc1"}}),
+				createCfg("cfg01", "image01", ".", nil),
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -338,7 +370,7 @@ requires:
 				var cfgs []string
 				for j, c := range d.configs {
 					id := fmt.Sprintf("%d%d", i, j)
-					s := fmt.Sprintf(template, c.name, c.requiresStanza, id, id, id)
+					s := fmt.Sprintf(template, latest.Version, c.name, c.requiresStanza, id, id, id)
 					cfgs = append(cfgs, s)
 				}
 				tmpDir.Write(d.path, strings.Join(cfgs, "\n---\n"))
@@ -355,10 +387,13 @@ requires:
 				wd, _ := util.RealWorkDir()
 				c.Build.Artifacts[0].Workspace = filepath.Join(wd, dir)
 				for i := range c.Dependencies {
+					if c.Dependencies[i].Path == "" {
+						continue
+					}
 					c.Dependencies[i].Path = filepath.Join(wd, dir, c.Dependencies[i].Path)
 				}
 			}
-
+			t.Override(&git.SyncRepo, func(g latest.GitInfo, _ config.SkaffoldOptions) (string, error) { return g.Repo, nil })
 			cfgs, err := getAllConfigs(config.SkaffoldOptions{
 				Command:             "dev",
 				ConfigurationFile:   test.documents[0].path,

docs/content/en/docs/design/config.md

@@ -69,18 +69,18 @@ deploy:
 ```
 
 If the `configs` list isn't defined then it imports all the configs defined in the file pointed by `path`. Additionally, if the `path` to the configuration isn't defined it assumes that all the required configs are defined in the same file as the current config.
-### Remote config dependency [Under Development]
+
+### Remote config dependency
 
 The required skaffold config can live in a remote git repository:
 
 ```yaml
-apiVersion: skaffold/v2beta11
+apiVersion: skaffold/v2beta12
 kind: Config
 requires:
   - configs: ["cfg1", "cfg2"]
     git:
-      provider: github.com
-      repo: GoogleContainerTools/skaffold
+      repo: http://github.com/GoogleContainerTools/skaffold.git
       path: getting-started/skaffold.yaml
       ref: master
 ```
@@ -92,7 +92,7 @@ The remote config gets treated like a local config after substituting the path w
 
 ### Profile Activation in required configs
 
-Additionally the `activeProfiles`  stanza can define the profiles to be activated in the required configs, via:
+Additionally the `activeProfiles` stanza can define the profiles to be activated in the required configs, via:
 
 ```yaml
 apiVersion: skaffold/v2beta11