Add go tag filepath that converts marked fields to absolute paths

[gsquared94]

This PR introduces a new tag skaffold:"filepath" for config.go which converts the marked field to an absolute path.

type Artifact struct {
	// Workspace is the directory containing the artifact's sources.
	// Defaults to `.`.
	Workspace string `yaml:"context,omitempty" skaffold:"filepath"`
        ...
}

type KubectlDeploy struct {
	// Manifests lists the Kubernetes yaml or json manifests.
	// Defaults to `["k8s/*.yaml"]`.
	Manifests []string `yaml:"manifests,omitempty" skaffold:"filepath"`
        ...
}

This is useful since the current code assumes all relative paths in the skaffold config are defined from the os.Getwd() directory. With #5160 we'll open up support for multiple skaffold configs. Doing an upfront file path massaging avoids having to maintain the info for base paths for each config individually which can be error prone.

User facing changes:

There are no user facing changes. We retain the current behavior of relative path base being the CWD.

[codecov]

Codecov Report

Merging #5205 (e4c1de4) into master (35214eb) will decrease coverage by 0.00%.
The diff coverage is 67.18%.

@@            Coverage Diff             @@
##           master    #5205      +/-   ##
==========================================
- Coverage   71.82%   71.82%   -0.01%     
==========================================
  Files         387      388       +1     
  Lines       13928    14036     +108     
==========================================
+ Hits        10004    10081      +77     
- Misses       3190     3208      +18     
- Partials      734      747      +13     

Impacted Files	Coverage Δ
pkg/skaffold/schema/latest/config.go	58.82% <ø> (ø)
pkg/skaffold/util/workdir.go	20.00% <ø> (ø)
pkg/skaffold/tags/paths.go	65.00% <65.00%> (ø)
pkg/skaffold/trigger/fsnotify/trigger.go	71.05% <100.00%> (ø)
pkg/skaffold/util/util.go	83.19% <100.00%> (-0.15%)	⬇️
pkg/skaffold/yaml/yaml.go	54.83% <0.00%> (-8.13%)	⬇️
pkg/skaffold/runner/build_deploy.go	68.88% <0.00%> (-1.01%)	⬇️
pkg/skaffold/docker/docker_init.go	100.00% <0.00%> (ø)
pkg/skaffold/initializer/build/util.go	100.00% <0.00%> (ø)
pkg/skaffold/initializer/build/builders.go	70.00% <0.00%> (ø)
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35214eb...e4c1de4. Read the comment docs.

[briandealwis]

integration/TestGeneratePipeline tests are failing as the context directories don't match.

@@ -12,7 +12,7 @@
           build:
             artifacts:
             - image: gcr.io/first-week-244218/getting-started-pipeline
-              context: /skaffold/integration/testdata/generate_pipeline/existing_other
+              context: .
               kaniko: {}
             tagPolicy:
               gitCommit: {}

[briandealwis]

yamltags just doesn't seem to be appropriate. Is there some prior art that we could leverage here, like the Go validator's dir and file types??

[gsquared94]

yamltags just doesn't seem to be appropriate.

We are already using yamltags as the key for custom validation tags. Although I don't mind making this one different.

[gsquared94]

maybe convert: "dir"? I think validate isn't correct either since we aren't validating the actual directory exists.

[gsquared94]

Is there some prior art that we could leverage here, like the Go validator's dir and file types??

I think that the validator package is validating the dir or file exists on the filesystem, which isn't what we're trying to do.

[briandealwis]

Although we are bashing these values to make them reference to files and directories on disk, these values may be relative to places other than the current directory (e.g., within an artifact workspace).

[gsquared94]

After discussing with @briandealwis, removed the invocation of MakeFilePathsAbsolute from the parsed skaffold config. We'll use this function for configs that are resolved as dependency from other places, in subsequent PR.

[MarlonGamez]

I think this looks good. As for Brian's point about using yamltags, I think that if we're already doing this, then we should be consistent here. If we want to change that name in the future, maybe we should have a discuss about it

[briandealwis]

My point is really that these has nothing to do with YAML. Our yamltags describes structural information and constraints specific to YAML. These fields are filepaths, even if we were pulling in a definition encoded in JSON or (gasp) XML.

Why don't we add a skaffold tag. So we'd use skaffold:filepath? That's clearly under our control and independent of any representation formats.

[gsquared94]

Updated. PTAL @briandealwis, @MarlonGamez

[briandealwis]

Should pkg/skaffold/yamltags/paths*.go be moved to pkg/skaffold/config?