Repository: appvia/kev
Pull request #292 (2020-10-05 18:57:39)

Refine skaffold profiles
Addresses #291 

- [x] Skaffold "build" artefacts should also take into account images & build context defined in Docker Compose files in case Skaffold analysis didn't detect any Images (usually due to missing k8s manifests in the project directory)
- [x] Refine Kev environment specific Skaffold profiles to not include kubectl context 
- [x] Revisit helper profiles naming convention


Patch:
diff --git a/cmd/kev/cmd/init.go b/cmd/kev/cmd/init.go
index 6437c50..100c15d 100644
--- a/cmd/kev/cmd/init.go
+++ b/cmd/kev/cmd/init.go
@@ -130,7 +130,12 @@ func runInitCmd(cmd *cobra.Command, _ []string) error {
 		// set skaffold path in kev manifest
 		manifest.Skaffold = kev.SkaffoldFileName
 
-		if err := createOrUpdateSkaffoldManifest(skaffoldManifestPath, envs, &results); err != nil {
+		project, err := manifest.SourcesToComposeProject()
+		if err != nil {
+			return displayError(err)
+		}
+
+		if err := createOrUpdateSkaffoldManifest(skaffoldManifestPath, envs, project, &results); err != nil {
 			return displayError(err)
 		}
 	}
@@ -164,7 +169,7 @@ func writeTo(filePath string, w io.WriterTo) error {
 	return file.Close()
 }
 
-func createOrUpdateSkaffoldManifest(path string, envs []string, results *[]skippableFile) error {
+func createOrUpdateSkaffoldManifest(path string, envs []string, project *kev.ComposeProject, results *[]skippableFile) error {
 	if manifestExistsForPath(path) {
 		// skaffold manifest already present - add additional profiles to it!
 		// Note: kev will skip profiles with names matching those of existing
@@ -185,7 +190,7 @@ func createOrUpdateSkaffoldManifest(path string, envs []string, results *[]skipp
 
 	} else {
 
-		skaffoldManifest, err := kev.PrepareForSkaffold(envs)
+		skaffoldManifest, err := kev.NewSkaffoldManifest(envs, project)
 		if err != nil {
 			return displayError(err)
 		}
diff --git a/pkg/kev/kev.go b/pkg/kev/kev.go
index 316e60d..e73da30 100644
--- a/pkg/kev/kev.go
+++ b/pkg/kev/kev.go
@@ -48,16 +48,6 @@ func Init(composeSources, envs []string, workingDir string) (*Manifest, error) {
 	return m.MintEnvironments(envs), nil
 }
 
-// PrepareForSkaffold initialises a skaffold manifest for kev project.
-func PrepareForSkaffold(envs []string) (*SkaffoldManifest, error) {
-	s, err := NewSkaffoldManifest(envs)
-	if err != nil {
-		return nil, err
-	}
-
-	return s, nil
-}
-
 // Reconcile reconciles changes with docker-compose sources against deployment environments.
 func Reconcile(workingDir string) (*Manifest, error) {
 	m, err := LoadManifest(workingDir)
diff --git a/pkg/kev/manifest.go b/pkg/kev/manifest.go
index 93a6b88..2cc162b 100644
--- a/pkg/kev/manifest.go
+++ b/pkg/kev/manifest.go
@@ -147,7 +147,7 @@ func (m *Manifest) ReconcileConfig() (*Manifest, error) {
 func (m *Manifest) MergeEnvIntoSources(e *Environment) (*ComposeProject, error) {
 	e.prepareForMergeUsing(m.getSourcesOverride())
 
-	p, err := m.sourcesToComposeProject()
+	p, err := m.SourcesToComposeProject()
 	if err != nil {
 		return nil, err
 	}
@@ -216,7 +216,7 @@ func (m *Manifest) getSourcesOverride() *composeOverride {
 	return m.Sources.override
 }
 
-// sourcesToComposeProject returns the manifests compose sources as a ComposeProject.
-func (m *Manifest) sourcesToComposeProject() (*ComposeProject, error) {
+// SourcesToComposeProject returns the manifests compose sources as a ComposeProject.
+func (m *Manifest) SourcesToComposeProject() (*ComposeProject, error) {
 	return m.Sources.toComposeProject()
 }
diff --git a/pkg/kev/skaffold.go b/pkg/kev/skaffold.go
index 5879b0b..2814151 100644
--- a/pkg/kev/skaffold.go
+++ b/pkg/kev/skaffold.go
@@ -24,6 +24,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"sort"
 	"strings"
@@ -52,7 +53,7 @@ const (
 	SkaffoldFileName = "skaffold.yaml"
 
 	// ProfileNamePrefix is a prefix to the added skaffold aprofile
-	ProfileNamePrefix = "zz-"
+	ProfileNamePrefix = "kev-"
 
 	// EnvProfileNameSuffix is a suffix added to environment specific profile name
 	EnvProfileNameSuffix = "-env"
@@ -67,7 +68,7 @@ var (
 )
 
 // NewSkaffoldManifest returns a new SkaffoldManifest struct.
-func NewSkaffoldManifest(envs []string) (*SkaffoldManifest, error) {
+func NewSkaffoldManifest(envs []string, project *ComposeProject) (*SkaffoldManifest, error) {
 
 	analysis, err := analyzeProject()
 	if err != nil {
@@ -76,7 +77,7 @@ func NewSkaffoldManifest(envs []string) (*SkaffoldManifest, error) {
 	}
 
 	manifest := BaseSkaffoldManifest()
-	manifest.SetBuildArtifacts(analysis)
+	manifest.SetBuildArtifacts(analysis, project)
 	manifest.SetProfiles(envs)
 	manifest.AdditionalProfiles()
 
@@ -118,21 +119,25 @@ func UpdateSkaffoldProfiles(path string, envToOutputPath map[string]string) erro
 		return err
 	}
 
-	skaffold.UpdateProfiles(envToOutputPath)
-
-	file, err := os.Create(path)
-	if err != nil {
-		return err
-	}
-	if _, err := skaffold.WriteTo(file); err != nil {
-		return err
+	if changed := skaffold.UpdateProfiles(envToOutputPath); changed {
+		file, err := os.Create(path)
+		if err != nil {
+			return err
+		}
+		if _, err := skaffold.WriteTo(file); err != nil {
+			return err
+		}
+		return file.Close()
 	}
-	return file.Close()
+
+	return nil
 }
 
 // UpdateProfiles updates profile for each environment with its K8s output path
 // Note, currently the only supported format is native kubernetes manifests
-func (s *SkaffoldManifest) UpdateProfiles(envToOutputPath map[string]string) {
+func (s *SkaffoldManifest) UpdateProfiles(envToOutputPath map[string]string) bool {
+	changed := false
+
 	for _, p := range s.Profiles {
 
 		// envToOutputPath is keyed by canonical environment name, however
@@ -148,11 +153,21 @@ func (s *SkaffoldManifest) UpdateProfiles(envToOutputPath map[string]string) {
 				manifestsPath = outputPath
 			}
 
-			p.Deploy.KubectlDeploy.Manifests = []string{
+			manifests := []string{
 				manifestsPath,
 			}
+
+			// only update when necessary
+			if !reflect.DeepEqual(p.Deploy.KubectlDeploy.Manifests, manifests) {
+				p.Deploy.KubectlDeploy.Manifests = []string{
+					manifestsPath,
+				}
+				changed = true
+			}
 		}
 	}
+
+	return changed
 }
 
 // BaseSkaffoldManifest returns base Skaffold manifest
@@ -209,9 +224,6 @@ func (s *SkaffoldManifest) SetProfiles(envs []string) {
 							},
 						},
 					},
-					// @todo define convention on how kubernetes context are named.
-					// for now simply user environment name with `-context` suffix.
-					KubeContext: e + EnvProfileKubeContextSuffix,
 				},
 				Test:        []*latest.TestCase{},
 				PortForward: []*latest.PortForwardResource{},
@@ -232,6 +244,11 @@ func (s *SkaffoldManifest) AdditionalProfiles() {
 					KubeContext: "minikube",
 				},
 			},
+			Pipeline: latest.Pipeline{
+				Deploy: latest.DeployConfig{
+					KubeContext: "minikube",
+				},
+			},
 		})
 	}
 
@@ -244,6 +261,11 @@ func (s *SkaffoldManifest) AdditionalProfiles() {
 					KubeContext: "docker-desktop",
 				},
 			},
+			Pipeline: latest.Pipeline{
+				Deploy: latest.DeployConfig{
+					KubeContext: "docker-desktop",
+				},
+			},
 		})
 	}
 
@@ -285,7 +307,7 @@ func (s *SkaffoldManifest) AdditionalProfiles() {
 }
 
 // SetBuildArtifacts detects build artifacts from the current project and adds `build` section to the manifest
-func (s *SkaffoldManifest) SetBuildArtifacts(analysis *Analysis) error {
+func (s *SkaffoldManifest) SetBuildArtifacts(analysis *Analysis, project *ComposeProject) error {
 
 	// don't set build artefacts if no analysis available
 	if analysis == nil {
@@ -294,7 +316,7 @@ func (s *SkaffoldManifest) SetBuildArtifacts(analysis *Analysis) error {
 
 	artifacts := []*latest.Artifact{}
 
-	for context, image := range collectBuildArtifacts(analysis) {
+	for context, image := range collectBuildArtifacts(analysis, project) {
 		artifacts = append(artifacts, &latest.Artifact{
 			ImageName: image,
 			Workspace: context,
@@ -309,9 +331,23 @@ func (s *SkaffoldManifest) SetBuildArtifacts(analysis *Analysis) error {
 }
 
 // collectBuildArtfacts returns a map of build contexts to corresponding image names
-func collectBuildArtifacts(analysis *Analysis) map[string]string {
+func collectBuildArtifacts(analysis *Analysis, project *ComposeProject) map[string]string {
 	buildArtifacts := map[string]string{}
 
+	if len(analysis.Images) == 0 {
+		// no images detected - usually the case when there are no kubernetes manifests
+		// Extract referenced images and map them to their respective build contexts (if present) from Compose project
+		// Note: It'll miss images without "build" context specified!
+
+		if project.Project != nil && project.Project.Services != nil {
+			for _, s := range project.Project.Services {
+				if s.Build != nil && len(s.Build.Context) > 0 && len(s.Image) > 0 {
+					buildArtifacts[s.Build.Context] = s.Image
+				}
+			}
+		}
+	}
+
 	for _, d := range analysis.Dockerfiles {
 
 		context := strings.ReplaceAll(d, "/Dockerfile", "")


Test patch:
diff --git a/pkg/kev/kev_prepare_for_skaffold_test.go b/pkg/kev/kev_prepare_for_skaffold_test.go
deleted file mode 100644
index 99b5eb1..0000000
--- a/pkg/kev/kev_prepare_for_skaffold_test.go
+++ /dev/null
@@ -1,40 +0,0 @@
-/**
- * Copyright 2020 Appvia Ltd <info@appvia.io>
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package kev_test
-
-import (
-	"github.com/appvia/kev/pkg/kev"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
-)
-
-var _ = Describe("PrepareForSkaffold", func() {
-	var (
-		skaffoldManifest *kev.SkaffoldManifest
-		err              error
-	)
-
-	JustBeforeEach(func() {
-		skaffoldManifest, err = kev.PrepareForSkaffold([]string{})
-	})
-
-	It("generates skaffold config for the project", func() {
-		Expect(skaffoldManifest).ToNot(BeNil())
-		Expect(err).ToNot(HaveOccurred())
-	})
-
-})
diff --git a/pkg/kev/skaffold_test.go b/pkg/kev/skaffold_test.go
index 801f796..3017a82 100644
--- a/pkg/kev/skaffold_test.go
+++ b/pkg/kev/skaffold_test.go
@@ -24,6 +24,7 @@ import (
 	"github.com/appvia/kev/pkg/kev"
 	"github.com/appvia/kev/pkg/kev/converter/kubernetes"
 	"github.com/appvia/kev/pkg/kev/log"
+	composego "github.com/compose-spec/compose-go/types"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/sirupsen/logrus/hooks/test"
@@ -40,6 +41,22 @@ func init() {
 
 var _ = Describe("Skaffold", func() {
 
+	Describe("NewSkaffoldManifest", func() {
+		var (
+			skaffoldManifest *kev.SkaffoldManifest
+			err              error
+		)
+
+		JustBeforeEach(func() {
+			skaffoldManifest, err = kev.NewSkaffoldManifest([]string{}, &kev.ComposeProject{})
+		})
+
+		It("generates skaffold config for the project", func() {
+			Expect(skaffoldManifest).ToNot(BeNil())
+			Expect(err).ToNot(HaveOccurred())
+		})
+	})
+
 	Describe("BaseSkaffoldManifest", func() {
 		It("returns base skaffold", func() {
 			Expect(kev.BaseSkaffoldManifest()).To(Equal(
@@ -77,7 +94,6 @@ var _ = Describe("Skaffold", func() {
 								},
 							},
 						},
-						KubeContext: envs[i] + "-context",
 					}))
 				}
 			})
@@ -154,15 +170,78 @@ var _ = Describe("Skaffold", func() {
 			Expect(manifest.Profiles).To(HaveLen(4))
 		})
 
-		It("adds additional profiles to the skaffold manifest with name containing kev defined prefix", func() {
-			Expect(manifest.Profiles).To(ContainElement(latest.Profile{
-				Name: "zz-minikube",
-				Activation: []latest.Activation{
-					{
-						KubeContext: "minikube",
+		Context("minikube", func() {
+			It("adds additional profiles to the skaffold manifest with name containing kev defined prefix", func() {
+				Expect(manifest.Profiles).To(ContainElement(latest.Profile{
+					Name: "kev-minikube",
+					Activation: []latest.Activation{
+						{
+							KubeContext: "minikube",
+						},
 					},
-				},
-			}))
+					Pipeline: latest.Pipeline{
+						Deploy: latest.DeployConfig{
+							KubeContext: "minikube",
+						},
+					},
+				}))
+			})
+		})
+
+		Context("docker-desktop", func() {
+			It("adds additional profiles to the skaffold manifest with name containing kev defined prefix", func() {
+				Expect(manifest.Profiles).To(ContainElement(latest.Profile{
+					Name: "kev-docker-desktop",
+					Activation: []latest.Activation{
+						{
+							KubeContext: "docker-desktop",
+						},
+					},
+					Pipeline: latest.Pipeline{
+						Deploy: latest.DeployConfig{
+							KubeContext: "docker-desktop",
+						},
+					},
+				}))
+			})
+		})
+
+		Context("ci-build-no-push", func() {
+			enabled := false
+
+			It("adds additional profiles to the skaffold manifest with name containing kev defined prefix", func() {
+				Expect(manifest.Profiles).To(ContainElement(latest.Profile{
+					Name: "kev-ci-build-no-push",
+					Pipeline: latest.Pipeline{
+						Build: latest.BuildConfig{
+							BuildType: latest.BuildType{
+								LocalBuild: &latest.LocalBuild{
+									Push: &enabled,
+								},
+							},
+						},
+					},
+				}))
+			})
+		})
+
+		Context("ci-build-and-push", func() {
+			enabled := true
+
+			It("adds additional profiles to the skaffold manifest with name containing kev defined prefix", func() {
+				Expect(manifest.Profiles).To(ContainElement(latest.Profile{
+					Name: "kev-ci-build-and-push",
+					Pipeline: latest.Pipeline{
+						Build: latest.BuildConfig{
+							BuildType: latest.BuildType{
+								LocalBuild: &latest.LocalBuild{
+									Push: &enabled,
+								},
+							},
+						},
+					},
+				}))
+			})
 		})
 
 		When("profile of the same name already exists in skaffold profiles", func() {
@@ -279,6 +358,7 @@ var _ = Describe("Skaffold", func() {
 
 		var (
 			skaffoldManifest *kev.SkaffoldManifest
+			project          *kev.ComposeProject
 			analysis         *kev.Analysis
 		)
 
@@ -289,7 +369,7 @@ var _ = Describe("Skaffold", func() {
 			})
 
 			JustBeforeEach(func() {
-				skaffoldManifest.SetBuildArtifacts(analysis)
+				skaffoldManifest.SetBuildArtifacts(analysis, project)
 			})
 
 			// Note, service name is derived from the Dockerfile location path
@@ -301,6 +381,7 @@ var _ = Describe("Skaffold", func() {
 						Dockerfiles: []string{"src/myservice/Dockerfile"},
 						Images:      []string{"quay.io/myorg/myservice", "myservice"},
 					}
+					project = &kev.ComposeProject{}
 				})
 
 				It("picks remote registry image path and sets correct Build configuration", func() {
@@ -324,8 +405,90 @@ var _ = Describe("Skaffold", func() {
 					Expect(skaffoldManifest.Build.Artifacts[0].Workspace).To(Equal("src/myservice"))
 				})
 			})
-
 		})
 
+		When("skaffold analysis Images haven't been detected (due to missing k8s manifests)", func() {
+			BeforeEach(func() {
+				skaffoldManifest = &kev.SkaffoldManifest{}
+			})
+
+			JustBeforeEach(func() {
+				skaffoldManifest.SetBuildArtifacts(analysis, project)
+			})
+
+			Context("It falls back to Docker Compose source files for extraction of images and build contexts", func() {
+				BeforeEach(func() {
+					analysis = &kev.Analysis{
+						Images: []string{},
+					}
+				})
+
+				When("Docker Compose project has services referencing images with build contexts", func() {
+					image := "quay.io/org/myimage:latest"
+					context := "my/context"
+
+					BeforeEach(func() {
+						project = &kev.ComposeProject{
+							Project: &composego.Project{
+								Services: composego.Services(
+									[]composego.ServiceConfig{
+										{
+											Name:  "svc1",
+											Image: image,
+											Build: &composego.BuildConfig{
+												Context: context,
+											},
+										},
+									},
+								),
+							},
+						}
+					})
+
+					It("generates skaffold build artefacts with extracted Docker Compose images and their respective contexts", func() {
+						Expect(skaffoldManifest.Build.Artifacts).To(HaveLen(1))
+						Expect(skaffoldManifest.Build.Artifacts[0].ImageName).To(Equal(image))
+						Expect(skaffoldManifest.Build.Artifacts[0].Workspace).To(Equal(context))
+					})
+				})
+
+				When("Docker Compose project doens't have services referencing images with build contexts", func() {
+					image := "quay.io/org/myimage:latest"
+
+					BeforeEach(func() {
+						project = &kev.ComposeProject{
+							Project: &composego.Project{
+								Services: composego.Services(
+									[]composego.ServiceConfig{
+										{
+											Name:  "svc1",
+											Image: image,
+										},
+									},
+								),
+							},
+						}
+					})
+
+					It("skips Docker Compose images without build context defined", func() {
+						Expect(skaffoldManifest.Build.Artifacts).To(HaveLen(0))
+					})
+				})
+
+				When("Docker Compose project doens't have services", func() {
+					BeforeEach(func() {
+						project = &kev.ComposeProject{
+							Project: &composego.Project{
+								Services: composego.Services{},
+							},
+						}
+					})
+
+					It("doesn't add skaffold build artefacts for project without services specified", func() {
+						Expect(skaffoldManifest.Build.Artifacts).To(HaveLen(0))
+					})
+				})
+			})
+		})
 	})
 })