Fix image controller - properly handle relative paths (#58)

* When matching src we want to be able to pull in files from directories
higher in the tree than the location of the YAMl file
  * Update the matching logic to support that
* Make some changes to CommitAll to allow for reuse of the worktree and
repo to try to speed things up
* Add documentation for the image builder

Dockerfile

@@ -13,6 +13,8 @@ COPY . /workspace
 # but I can't remember precisely why we needed to do it and therefore I don't know if it still applies.
 # If we need to renable CGO we need to stop using chainguard's static image and use one with the appropriate
 # environment.
+#
+# TODO(jeremy): We should be setting version information here
 RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o hydros cmd/main.go
 
 # TODO(jeremy): This won't be able to run Syncer until we update syncer to use GoGit and get rid of shelling

api/v1alpha1/images.go

@@ -30,6 +30,14 @@ type ImageSpec struct {
 
 // Source is a local path to include as an artifact.
 // It is inspired by skaffold; https://skaffold.dev/docs/references/yaml/
+// When building images from a YAML file the src is a relative path to the location of the YAML file.
+// src can start with a parent prefix e.g. ".." to obtain files higher in the directory tree relative to the
+// YAML file. The parent directory wil then be used when computing the destination path.
+// e.g. if we have
+// /p/a/image.yaml
+// /p/b/file.txt
+// And image.yaml contains src "../b/file.txt" then the file will be copied to b/file.txt by default in the
+// tarball
 type Source struct {
 	// Src is a glob pattern to match local paths against. Directories should be delimited by / on all platforms.
 	// e.g. "css/**/*.css"

cmd/main.go

@@ -207,7 +207,7 @@ func newVersionCmd(w io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:     "version",
 		Short:   "Return version",
-		Example: `kap version`,
+		Example: `hydros version`,
 		Run: func(cmd *cobra.Command, args []string) {
 			fmt.Fprintf(w, "hydros %s, commit %s, built at %s by %s", version, commit, date, builtBy)
 		},

docs/image_build.md

@@ -0,0 +1,96 @@
+# Image Building
+
+Hydros provides a controller to ensure images exist for a set of sources. 
+The semantics of the `Image` resource are 
+"Ensure there is an image built from the latest commit of the source repository".
+
+## Defining an image
+
+You can define an image by using the Image resource
+
+Here's an example
+
+```yaml
+kind: Image
+apiVersion: hydros.sailplane.ai/v1alpha1
+metadata:
+  name: kp
+  namespace: kubepilot
+spec:
+  image: us-west1-docker.pkg.dev/dev-sailplane/images/hydros/hydros
+  source:
+    # Leave it at the root of the context because that's what hydros will look for.
+    - src: Dockerfile
+    # Specify individual directories so we don't include hidden directories
+    - src: "go.mod"
+      dest: "kubedr"
+    - src: "go.sum"
+      dest: "kubedr"
+    - src: "api/**/*.go"
+      dest: "kubedr"
+    - src: "cmd/**/*.go"
+      dest: "kubedr"
+    - src: "pkg/**/*"
+      dest: "kubedr"
+    - src: "test/**/*.go"
+      dest: "kubedr"
+    - src: "../vendor/**/*"
+  builder:
+    gcb:
+      project: dev-sailplane
+      bucket : builds-sailplane
+```
+
+Currently only the GCB builder is supported.
+
+### Context
+
+The context for the image is defined by the source field. Each entry in the source field specifies files
+that will be copied into the context. The source field is an array of objects with the following fields:
+
+* src: This is a glob expression matching files to be copied into the context. The glob expression is relative to the
+  directory containing the image definition file.
+  * Double star `**` can be used to match all subdirectories
+  * You can use `..` to go up the directory tree to match files located in parent directories of the `.yaml` file
+* dest: This is the destination directory for the files. 
+* strip: This is a prefix to strip of the matched files when computing the location in the destination directory. 
+
+The location of the files inside the produced context (tarball) is as follows
+
+```
+basePath = dir(imageDefinitionFile)
+rPath = path of matched file relative to basePath
+strippedRPath = strip rPath of prefix 
+destPath = dest + stripPipedRpath
+```
+
+In the case where `src` begins with `..` basePath is adjusted to be the parent directory.
+
+### Dockerfile
+
+Hydros currently requires the Dockerfile to be named `Dockerfile` and located at the root of the context.
+
+### Docker build args
+
+The following build args are passed to kaniko and can be used in your Dockerfile
+
+* `DATE` - A human readable timestap
+* `COMMIT` - The full git commit
+* `Version` - A version string of the form `vv20240126T092312`
+
+### Tags
+
+The build automatically tags the image with the following tags
+
+* `latest` 
+*  The full git commit of the source repository
+
+## Building an image
+
+To build an image you can use the `hydros build` command
+
+```bash
+hydros build ~/git_roboweb/kubedr/image.yaml
+```
+
+If an image already exists in the registry with the same tag as the current commit, the image will not be rebuilt.

pkg/gcp/gcb.go

@@ -49,13 +49,19 @@ func BuildImage(project string, build *cbpb.Build) (*longrunningpb.Operation, er
 // DefaultBuild constructs a default BuildFile
 // image should be the URI of the image with out the tag
 func DefaultBuild() *cbpb.Build {
+	now := time.Now()
+	nowStr := now.Format(time.RFC3339)
 	build := &cbpb.Build{
 		Steps: []*cbpb.BuildStep{
 			{
 				Name: kanikoBuilder,
 				Args: []string{
 					"--dockerfile=Dockerfile",
 					"--cache=true",
+					// Set the date as a build arg
+					// This is so that it can be passed to the builder and used to set the date in the image
+					// of the build
+					"--build-arg=DATE=" + nowStr,
 				},
 			},
 		},
@@ -80,26 +86,59 @@ func AddImages(build *cbpb.Build, images []string) error {
 		return errors.Errorf("Build.Steps[0].Name %s doesn't match expected %s", build.Steps[0].Name, kanikoBuilder)
 	}
 
-	existing := make(map[string]bool)
-
 	destFlag := "--destination="
 
+	args := make([]string, 0, len(images))
+
+	for _, i := range images {
+		args = append(args, destFlag+i)
+	}
+
+	return AddKanikoArgs(build, args)
+	return nil
+}
+
+// AddKanikoArgs adds a build arg to the build
+// null-op if its already added
+func AddKanikoArgs(build *cbpb.Build, buildArgs []string) error {
+	if build.Steps == nil {
+		return errors.New("Build.Steps is nil")
+	}
+
+	if build.Steps[0].Name != kanikoBuilder {
+		return errors.Errorf("Build.Steps[0].Name %s doesn't match expected %s", build.Steps[0].Name, kanikoBuilder)
+	}
+
+	existing := make(map[string]bool)
+
 	for _, arg := range build.Steps[0].Args {
-		if strings.HasPrefix(arg, destFlag) {
-			existing[arg[len(destFlag):]] = true
-		}
+		existing[arg] = true
 	}
 
-	for _, image := range images {
-		if existing[image] {
+	for _, a := range buildArgs {
+		if existing[a] {
 			continue
 		}
 
-		build.Steps[0].Args = append(build.Steps[0].Args, destFlag+image)
+		build.Steps[0].Args = append(build.Steps[0].Args, a)
 	}
 	return nil
 }
 
+// AddBuildTags passes various values as build flags to the build
+func AddBuildTags(build *cbpb.Build, sourceCommit string, version string) error {
+	args := []string{
+		// Pass the values along to Docker
+		"--build-arg=COMMIT=" + sourceCommit,
+		"--build-arg=VERSION=" + version,
+		// Build labels; Does this do anything
+		"--label=COMMIT=" + sourceCommit,
+		"--label=COMMIT=" + version,
+	}
+
+	return AddKanikoArgs(build, args)
+}
+
 // OPNameToBuildID converts an operation name to a build id
 func OPNameToBuildID(name string) (string, error) {
 	// The operation name is of the form projects/<project>/operations/<id>

pkg/gitops/syncer.go

@@ -612,7 +612,16 @@ func (s *Syncer) PushLocal(wDir string, keyFile string) error {
 	}
 
 	message := "hydros automatically committing all files before running a sync."
-	if err := gitutil.CommitAll(r, root, message); err != nil {
+	w, err := r.Worktree()
+	if err != nil {
+		return errors.Wrapf(err, "Error getting worktree")
+	}
+
+	if err := gitutil.AddGitignoreToWorktree(w, root); err != nil {
+		return errors.Wrapf(err, "Failed to add gitignore patterns")
+	}
+
+	if err := gitutil.CommitAll(r, w, message); err != nil {
 		return err
 	}
 

pkg/gitutil/gitutil.go

@@ -50,6 +50,7 @@ func LocateRoot(path string) (string, error) {
 // This is a workaround for https://github.com/go-git/go-git/issues/597. If we don't call this before doing add
 // all then the ignore patterns won't be respected.
 //
+// N.B. It looks like we also need to call this function if we want IsClean to ignore files
 // N.B this doesn't work with nested .gitignore files.
 func AddGitignoreToWorktree(wt *git.Worktree, repositoryPath string) error {
 	log := zapr.NewLogger(zap.L())
@@ -123,53 +124,67 @@ func LoadUser(r *git.Repository) (*User, error) {
 }
 
 // CommitAll is a helper function to commit all changes in the repository.
-// r is the git repository
-// root is the root of the repository
-func CommitAll(r *git.Repository, root string, message string) error {
-	w, err := r.Worktree()
+// null op if its clean.
+// w is the worktree
+// You should call AddGitIgnore on the worktree before calling this function if you want to ensure files are ignored.
+// TODO(jeremy): I'm not sure this is the right API. I did it this way because with kubedr it was really slow
+// to get the worktree and add .gitignore so I didn't want to do that more than once.
+// https://github.com/sailplaneai/code/issues/872 is tracking the slowness.
+func CommitAll(r *git.Repository, w *git.Worktree, message string) error {
+	log := zapr.NewLogger(zap.L())
+	log.Info("Getting git status")
+	status, err := w.Status()
 	if err != nil {
 		return err
 	}
 
-	if err := AddGitignoreToWorktree(w, root); err != nil {
-		return errors.Wrapf(err, "Failed to add gitignore patterns")
+	if status.IsClean() {
+		log.Info("tree is clean; no commit needed")
+		return nil
+	}
+	log.Info("committing all files")
+	if err := w.AddWithOptions(&git.AddOptions{All: true}); err != nil {
+		return err
 	}
 
-	status, err := w.Status()
+	user, err := LoadUser(r)
 	if err != nil {
 		return err
 	}
+	commit, err := w.Commit(message, &git.CommitOptions{
+		Author: &object.Signature{
+			// Use the name and email as specified in the cfg file.
+			Name:  user.Name,
+			Email: user.Email,
+			When:  time.Now(),
+		},
+	})
 
-	log := zapr.NewLogger(zap.L())
-	if !status.IsClean() {
-		log.Info("committing all files")
-		if err := w.AddWithOptions(&git.AddOptions{All: true}); err != nil {
-			return err
-		}
+	if err != nil {
+		return err
+	}
 
-		user, err := LoadUser(r)
-		if err != nil {
-			return err
-		}
-		commit, err := w.Commit(message, &git.CommitOptions{
-			Author: &object.Signature{
-				// Use the name and email as specified in the cfg file.
-				Name:  user.Name,
-				Email: user.Email,
-				When:  time.Now(),
-			},
-		})
-
-		if err != nil {
-			return err
-		}
+	// Prints the current HEAD to verify that all worked well.
+	obj, err := r.CommitObject(commit)
+	if err != nil {
+		return err
+	}
+	log.Info("Commit succeeded", "commit", obj.String())
 
-		// Prints the current HEAD to verify that all worked well.
-		obj, err := r.CommitObject(commit)
-		if err != nil {
-			return err
+	return nil
+}
+
+// TrackedIsClean returns true if the repository is clean except for untracked files.
+// git.IsClean doesn't work because it doesn't ignore untracked files.
+func TrackedIsClean(gitStatus git.Status) bool {
+	for _, s := range gitStatus {
+		if s.Staging == git.Untracked || s.Worktree == git.Untracked {
+			continue
+		}
+		if s.Staging != git.Unmodified || s.Worktree != git.Unmodified {
+			return false
 		}
-		log.Info("Commit succeeded", "commit", obj.String())
 	}
-	return nil
+
+	return true
 }