Promote services by name. Only promote files in services/serviceName/base/config (#15)

Promote services by name. Also, only promote files in services/service-name/base/config

Author: mnuttall

README.md

@@ -23,7 +23,7 @@ $ ./services promote --from https://github.com/organisation/first-environment.gi
 
 If the `commit-name` and `commit-email` are not provided, it will attempt to find them in `~/.gitconfig`, otherwise it will fail.
 
-This will _copy_ a single file `deployment.txt` from `service-a` in `first-environment` to `service-a` in `second-environment`, commit and push, and open a PR for the change.
+This will _copy_ all files under `/services/service-a/base/config/*` in `first-environment` to `second-environment`, commit and push, and open a PR for the change.
 
 ## Testing
 

pkg/avancement/service_manager.go

@@ -51,7 +51,7 @@ const fromBranch = "master"
 //
 // It uses a Git cache to checkout the code to, and will copy the environment
 // configuration for the `fromURL` to the `toURL` in a named branch.
-func (s *ServiceManager) Promote(service, fromURL, toURL, newBranchName string) error {
+func (s *ServiceManager) Promote(serviceName, fromURL, toURL, newBranchName string) error {
 	source, err := s.checkoutSourceRepo(fromURL, fromBranch)
 	if err != nil {
 		return err
@@ -61,7 +61,8 @@ func (s *ServiceManager) Promote(service, fromURL, toURL, newBranchName string)
 		return fmt.Errorf("failed to checkout repo: %w", err)
 	}
 
-	copied, err := git.CopyService(service, source, destination)
+	copied, err := git.CopyService(serviceName, source, destination)
+
 	if err != nil {
 		return fmt.Errorf("failed to copy service: %w", err)
 	}

pkg/avancement/service_manager_test.go

@@ -31,15 +31,15 @@ func TestPromoteWithSuccess(t *testing.T) {
 	sm.repoFactory = func(url, _ string) (git.Repo, error) {
 		return git.Repo(repos[url]), nil
 	}
-	devRepo.AddFiles("/my-service/deploy/myfile.yaml")
+	devRepo.AddFiles("/services/my-service/base/config/myfile.yaml")
 
 	err := sm.Promote("my-service", dev, staging, dstBranch)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	stagingRepo.AssertBranchCreated(t, "master", dstBranch)
-	stagingRepo.AssertFileCopiedInBranch(t, dstBranch, "/dev/my-service/deploy/myfile.yaml", "/staging/my-service/deploy/myfile.yaml")
+	stagingRepo.AssertFileCopiedInBranch(t, dstBranch, "/dev/services/my-service/base/config/myfile.yaml", "/staging/services/my-service/base/config/myfile.yaml")
 	stagingRepo.AssertCommit(t, dstBranch, defaultCommitMsg, author)
 	stagingRepo.AssertPush(t, dstBranch)
 }

pkg/git/copy.go

@@ -1,17 +1,48 @@
 package git
 
-import "path"
+import (
+	"path"
+	"path/filepath"
+	"strings"
+)
 
-// CopyService takes a path within a repository, and a source and destination,
-// and copies the files from within the source, to the destination.
+// CopyService takes the name of a service to copy from a Source to a Destination.
+// Source, Destination implement Walk() and are typically Repository objects.
+//
+// Only files under /services/[serviceName]/base/config/* are copied to the destination
 //
 // Returns the list of files that were copied, and possibly an error.
-func CopyService(filePath string, source Source, dest Destination) ([]string, error) {
+func CopyService(serviceName string, source Source, dest Destination) ([]string, error) {
+
+	// filePath defines the root folder for serviceName's config in the repository
+	filePath := pathForServiceConfig(serviceName)
+
 	copied := []string{}
 	err := source.Walk(filePath, func(prefix, name string) error {
-		err := dest.CopyFile(path.Join(prefix, name), name)
-		copied = append(copied, name)
-		return err
+		sourcePath := path.Join(prefix, name)
+		destPath := pathForServiceConfig(name)
+		if pathValidForPromotion(serviceName, destPath) {
+			err := dest.CopyFile(sourcePath, destPath)
+			if err == nil {
+				copied = append(copied, destPath)
+			}
+			return err
+		}
+		return nil
 	})
 	return copied, err
 }
+
+// pathValidForPromotion()
+//  For a given serviceName, only files in services/serviceName/base/config/* are valid for promotion
+//
+func pathValidForPromotion(serviceName, filePath string) bool {
+	filterPath := filepath.Join(pathForServiceConfig(serviceName), "base", "config")
+	validPath := strings.HasPrefix(filePath, filterPath)
+	return validPath
+}
+
+// pathForServiceConfig defines where in a 'gitops' repository the config for a given service should live.
+func pathForServiceConfig(serviceName string) string {
+	return filepath.Join("services", serviceName)
+}

pkg/git/copy_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"io"
 	"path"
+	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
@@ -13,7 +14,7 @@ import (
 
 func TestCopyService(t *testing.T) {
 	s := &mockSource{localPath: "/tmp/testing"}
-	files := []string{"service-a/my-system/my-file.yaml", "service-a/my-system/this-file.yaml"}
+	files := []string{"services/service-a/base/config/my-file.yaml", "services/service-a/base/config/this-file.yaml"}
 	for _, f := range files {
 		s.addFile(f)
 	}
@@ -29,18 +30,97 @@ func TestCopyService(t *testing.T) {
 	}
 }
 
+func TestPathValidForPromotion(t *testing.T) {
+
+	serviceBeingPromoted := "service-name"
+	servicesNotBeingPromoted := []string{"svc", "base", "serviceName", "services"}
+	promoteTheseWhenServiceNameIsRight := []string{
+		"services/service-name/base/config/kustomization.yaml",
+		"services/service-name/base/config/deployment.yaml",
+		"services/service-name/base/config/dir/below/it/may/contain/important.yaml",
+	}
+	for _, filePath := range promoteTheseWhenServiceNameIsRight {
+		if !pathValidForPromotion(serviceBeingPromoted, filePath) {
+			t.Fatalf("Valid path for promotion for %s incorrectly rejected: %s", serviceBeingPromoted, filePath)
+		}
+		for _, wrongService := range servicesNotBeingPromoted {
+			if pathValidForPromotion(wrongService, filePath) {
+				t.Fatalf("Path for service %s incorrectly accepted for promotion: %s", wrongService, filePath)
+			}
+		}
+	}
+
+	// These paths should never be promoted
+	badServiceNames := []string{"svc", "badService"}
+	neverPromoteThese := []string{
+		"services/svc/overlays/kustomization.yaml",
+		"services/svc/kustomization.yaml",
+		"svc/base/config/deployment.yaml",
+		"services/badService/any/other/path/resource.yaml",
+	}
+	for _, badPath := range neverPromoteThese {
+		for _, badServiceName := range badServiceNames {
+			if pathValidForPromotion(badServiceName, badPath) {
+				t.Fatalf("Invalid path %s for promotion of service %s incorrectly accepted", badPath, badServiceName)
+			}
+		}
+	}
+}
+
+func TestPathForServiceConfig(t *testing.T) {
+	serviceName := "usefulService"
+	correctPath := "services/usefulService"
+	serviceConfigPath := pathForServiceConfig(serviceName)
+	if serviceConfigPath != correctPath {
+		t.Fatalf("Invalid result for pathForServiceConfig(%s): wanted %s got %s", serviceName, correctPath, serviceConfigPath)
+	}
+}
+
+func TestCopyServiceWithFailureCopying(t *testing.T) {
+	testError := errors.New("this is a test error")
+	s := &mockSource{localPath: "/tmp/testing"}
+	files := []string{"services/service-a/base/config/my-file.yaml", "services/service-a/base/config/this-file.yaml"}
+	for _, f := range files {
+		s.addFile(f)
+	}
+	d := &mockDestination{}
+	d.copyError = testError
+
+	copied, err := CopyService("service-a", s, d)
+	if err != testError {
+		t.Fatalf("unexpected error: got %v, wanted %v", err, testError)
+	}
+	d.assertFilesWritten(t, []string{})
+	if !reflect.DeepEqual(copied, []string{}) {
+		t.Fatalf("failed to copy the files, got %#v, want %#v", copied, []string{})
+	}
+}
+
 type mockSource struct {
 	files     []string
 	localPath string
 }
 
-func (s *mockSource) Walk(filePath string, cb func(string, string) error) error {
+// Walk: a mock function to emulate what happens in Repository.Walk()
+// The Mock version is different: it iterates over mockSource.files[] and then drives
+// the visitor callback in CopyService() as usual.
+//
+// To preserve the same behaviour, we see that Repository Walk receives /full/path/to/repo/services/service-name
+// and then calls filePath.Walk() on /full/path/to/repo/services/ .
+// When CopyService() drives Walk(), 'base' is typically services/service-name
+// Thus we take each /full/path/to/file/in/mockSource.files[] and split it at 'services/' as happens in the Walk() method we're mocking.
+func (s *mockSource) Walk(base string, cb func(string, string) error) error {
 	if s.files == nil {
 		return nil
 	}
+
 	for _, f := range s.files {
-		if strings.HasPrefix(f, path.Join(s.localPath, filePath)) {
-			err := cb(f, strings.TrimPrefix(f, s.localPath+"/"))
+		if strings.HasPrefix(f, path.Join(s.localPath, base)) {
+			splitString := filepath.Dir(base) + "/"
+			splitPoint := strings.Index(f, splitString) + len(splitString)
+			prefix := f[:splitPoint]
+			name := f[splitPoint:]
+			err := cb(prefix, name)
 			if err != nil {
 				return err
 			}
@@ -57,13 +137,17 @@ func (s *mockSource) addFile(name string) {
 }
 
 type mockDestination struct {
-	written []string
+	written   []string
+	copyError error
 }
 
 func (d *mockDestination) CopyFile(src, dst string) error {
 	if d.written == nil {
 		d.written = []string{}
 	}
+	if d.copyError != nil {
+		return d.copyError
+	}
 	d.written = append(d.written, dst)
 	return nil
 }

pkg/git/mock/mock.go

@@ -3,6 +3,7 @@ package mock
 import (
 	"io"
 	"path"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -100,15 +101,26 @@ func (m *Repository) WriteFile(src io.Reader, dst string) error {
 	return nil
 }
 
-// Walk fulfils the git.Repo interface.
-func (m *Repository) Walk(filePath string, cb func(string, string) error) error {
+// Walk fulfils the git.Repo interface. It's a mock function to emulate what happens in Repository.Walk()
+// The Mock version is different: it iterates over mockSource.files[] and then drives
+// the visitor callback in CopyService() as usual.
+//
+// To preserve the same behaviour, we see that Repository Walk receives /full/path/to/repo/services/service-name
+// and then calls filePath.Walk() on /full/path/to/repo/services/ .
+// When CopyService() drives Walk(), 'base' is typically services/service-name
+// Thus we take each /full/path/to/file/in/mockSource.files[] and split it at 'services/' as happens in the Walk() method we're mocking.
+func (m *Repository) Walk(base string, cb func(string, string) error) error {
 	if m.files == nil {
 		return nil
 	}
+
 	for _, f := range m.files {
-		prefix := m.localPath + "/"
-		if strings.HasPrefix(f, prefix) {
-			err := cb(prefix, strings.TrimPrefix(f, prefix))
+		if strings.HasPrefix(f, path.Join(m.localPath, base)) {
+			splitString := filepath.Dir(base) + "/"
+			splitPoint := strings.Index(f, splitString) + len(splitString)
+			prefix := f[:splitPoint]
+			name := f[splitPoint:]
+			err := cb(prefix, name)
 			if err != nil {
 				return err
 			}