Skip to content

Commit

Permalink
Update Kptfiles in Sub Packages
Browse files Browse the repository at this point in the history
  • Loading branch information
phanimarupaka committed Sep 2, 2020
1 parent 17a3796 commit e23c2a5
Show file tree
Hide file tree
Showing 6 changed files with 315 additions and 4 deletions.
4 changes: 4 additions & 0 deletions internal/kptfile/pkgfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ func (updatedKf *KptFile) MergeOpenAPI(localKf, originalKf KptFile) error {
return nil
}
oriDef := original.Field("definitions")
if oriDef == nil {
// no definitions on the destination, fall back to local definitions
oriDef = localDef
}

// merge the definitions
err = mergeDef(updatedDef, localDef, oriDef)
Expand Down
42 changes: 42 additions & 0 deletions internal/kptfile/pkgfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,48 @@ openAPI:
setter:
name: tag
value: 1.8.0
`,
},
{
name: "no defs in origin",
updated: `
openAPI:
definitions:
io.k8s.cli.setters.image:
x-k8s-cli:
setter:
name: "image"
value: "nginx"
io.k8s.cli.setters.tag:
x-k8s-cli:
setter:
name: "tag"
value: "1.8.1"
`,
local: `
openAPI:
definitions:
io.k8s.cli.setters.tag:
x-k8s-cli:
setter:
name: "tag"
value: "1.8.0"
`,
original: `
`,
expected: `
openAPI:
definitions:
io.k8s.cli.setters.image:
x-k8s-cli:
setter:
name: image
value: nginx
io.k8s.cli.setters.tag:
x-k8s-cli:
setter:
name: tag
value: 1.8.1
`,
},
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apiVersion: krm.dev/v1alpha1
kind: Kptfile
metadata:
name: storage
name: nosetters
packageMetadata:
shortDescription: sample description
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apiVersion: krm.dev/v1alpha1
kind: Kptfile
metadata:
name: storage
name: nosetters
packageMetadata:
shortDescription: sample description
99 changes: 97 additions & 2 deletions internal/util/update/resource-merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"sigs.k8s.io/kustomize/kyaml/errors"
"sigs.k8s.io/kustomize/kyaml/kio"
"sigs.k8s.io/kustomize/kyaml/kio/filters"
"sigs.k8s.io/kustomize/kyaml/pathutil"
"sigs.k8s.io/kustomize/kyaml/sets"
"sigs.k8s.io/kustomize/kyaml/setters2/settersutil"
)
Expand Down Expand Up @@ -57,7 +58,6 @@ func (u ResourceMergeUpdater) Update(options UpdateOptions) error {
}
defer os.RemoveAll(updated.AbsPath())

// get the Kptfile to write after the merge
kf, err := u.updatedKptfile(updated.AbsPath(), original.AbsPath(), options)
if err != nil {
return err
Expand All @@ -68,7 +68,7 @@ func (u ResourceMergeUpdater) Update(options UpdateOptions) error {
}

err = settersutil.SetAllSetterDefinitions(
filepath.Join(options.PackagePath, "Kptfile"),
filepath.Join(options.PackagePath, kptfile.KptFileName),
original.AbsPath(),
updated.AbsPath(),
options.PackagePath,
Expand All @@ -77,6 +77,10 @@ func (u ResourceMergeUpdater) Update(options UpdateOptions) error {
return err
}

if err := MergeSubPackages(options.PackagePath, updated.AbsPath(), original.AbsPath()); err != nil {
return err
}

// merge the Resources: original + updated + dest => dest
err = filters.Merge3{
OriginalPath: original.AbsPath(),
Expand Down Expand Up @@ -131,6 +135,87 @@ func (u ResourceMergeUpdater) updatedKptfile(updatedPath, originalPath string, o
return updatedKf, err
}

// MergeSubPackages merges the Kptfiles in the nested subdirectories of the
// root package and also sets the setter definitions in updated to match with
// locally set values so the the resources are correctly identified and merged
func MergeSubPackages(localRoot, updatedRoot, originalRoot string) error {
localPkgPaths, err := pathutil.DirsWithFile(localRoot, kptfile.KptFileName, true)
if err != nil {
return err
}
for _, localPkgPath := range localPkgPaths {
// skip the top level file as it should be merged differently
if filepath.Clean(localPkgPath) == filepath.Clean(localRoot) {
continue
}

cleanLocalPkgPath := filepath.Clean(localPkgPath)
relativePkgPath, err := filepath.Rel(localRoot, cleanLocalPkgPath)
if err != nil {
return err
}

localKf, err := kptfileutil.ReadFile(localPkgPath)
if err != nil {
return err
}

var updatedKf kptfile.KptFile
updatedPkgPath := filepath.Join(updatedRoot, relativePkgPath)
if !fileExists(filepath.Join(updatedPkgPath, kptfile.KptFileName)) {
// if there is no Kptfile in upstream then use the local Kptfile
// to retain it
updatedKf = localKf
} else {
updatedKf, err = kptfileutil.ReadFile(updatedPkgPath)
if err != nil {
return err
}
}

var originalKf kptfile.KptFile
originalPkgPath := filepath.Join(originalRoot, relativePkgPath)
if !fileExists(filepath.Join(originalPkgPath, kptfile.KptFileName)) {
// if there is no Kptfile at origin then use the local Kptfile
// to retain it
originalKf = localKf
} else {
originalKf, err = kptfileutil.ReadFile(originalPkgPath)
if err != nil {
return err
}
}

err = updatedKf.MergeOpenAPI(localKf, originalKf)
if err != nil {
return err
}

if err := kptfileutil.WriteFile(localPkgPath, updatedKf); err != nil {
return err
}

// make sure that the updated and original packages are set with the new values
// to setter parameters in local so that the resources are identified and merged
// correctly in subpackages
dirsForSettersUpdate := []string{localPkgPath}
if fileExists(filepath.Join(updatedPkgPath, kptfile.KptFileName)) {
dirsForSettersUpdate = append(dirsForSettersUpdate, updatedPkgPath)
}
if fileExists(filepath.Join(originalPkgPath, kptfile.KptFileName)) {
dirsForSettersUpdate = append(dirsForSettersUpdate, originalPkgPath)
}
err = settersutil.SetAllSetterDefinitions(
filepath.Join(localPkgPath, kptfile.KptFileName),
dirsForSettersUpdate...,
)
if err != nil {
return err
}
}
return nil
}

// replaceNonKRMFiles replaces the non KRM files in localDir with the corresponding files in updatedDir,
// it also deletes non KRM files and sub dirs which are present in localDir and not in updatedDir
func ReplaceNonKRMFiles(updatedDir, originalDir, localDir string) error {
Expand Down Expand Up @@ -269,3 +354,13 @@ func compareFiles(src, dst string) (bool, error) {
}
return false, nil
}

// fileExists checks if a file exists and is not a directory before we
// try using it to prevent further errors.
func fileExists(filename string) bool {
info, err := os.Stat(filename)
if os.IsNotExist(err) {
return false
}
return !info.IsDir()
}
170 changes: 170 additions & 0 deletions internal/util/update/resource-merge_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
package update

import (
"io/ioutil"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"sigs.k8s.io/kustomize/kyaml/copyutil"
"sigs.k8s.io/kustomize/kyaml/openapi"
)

func TestMergeSubPackages(t *testing.T) {
// this test simulates the end to end scenario of merging subpackages
// original and updated/upstream are same initially and they deviate
// due to upstream changes, localDataSet deviates from original as the
// setters are set with local values
updatedDataSet := "dataset-with-autosetters/mysql"
originalDataSet := "dataset-with-autosetters/mysql"
localDataset := "dataset-with-autosetters-set/mysql"

// reset the openAPI afterward
openapi.ResetOpenAPI()
defer openapi.ResetOpenAPI()
testDataDir := filepath.Join("../../", "testutil", "testdata")
updatedRoot, err := ioutil.TempDir("", "")
if !assert.NoError(t, err) {
t.FailNow()
}
localRoot, err := ioutil.TempDir("", "")
if !assert.NoError(t, err) {
t.FailNow()
}
originalRoot, err := ioutil.TempDir("", "")
if !assert.NoError(t, err) {
t.FailNow()
}
// updated is the upstream dataset with setters not set
err = copyutil.CopyDir(filepath.Join(testDataDir, updatedDataSet), updatedRoot)
if !assert.NoError(t, err) {
t.FailNow()
}
// original is the upstream dataset with setters not set
err = copyutil.CopyDir(filepath.Join(testDataDir, originalDataSet), originalRoot)
if !assert.NoError(t, err) {
t.FailNow()
}
// local root has the setters set to the local values
err = copyutil.CopyDir(filepath.Join(testDataDir, localDataset), localRoot)
if !assert.NoError(t, err) {
t.FailNow()
}

defer os.RemoveAll(updatedRoot)
defer os.RemoveAll(localRoot)
defer os.RemoveAll(originalRoot)

// modify updated/upstream by adding a new setter definition to one of the subpackages Kptfile
nosettersUpdated := `apiVersion: krm.dev/v1alpha1
kind: Kptfile
metadata:
name: nosetters
packageMetadata:
shortDescription: sample description
openAPI:
definitions:
io.k8s.cli.setters.new-setter:
x-k8s-cli:
setter:
name: new-setter
value: some-value
`

err = ioutil.WriteFile(filepath.Join(updatedRoot, "nosetters", "Kptfile"), []byte(nosettersUpdated), 0700)
if !assert.NoError(t, err) {
t.FailNow()
}

// updated has deviated from original with new setter definition added above
// local has deviated from original with auto-setters set to local values
// if update is triggered now, it merges the subpackages using MergeSubPkgsKptfiles
err = MergeSubPackages(localRoot, updatedRoot, originalRoot)
if !assert.NoError(t, err) {
t.FailNow()
}

// the updated subpackage file must have the setters set to the values in local
actualSubPkgFile, err := ioutil.ReadFile(filepath.Join(updatedRoot, "storage", "deployment.yaml"))
if !assert.NoError(t, err) {
t.FailNow()
}
expectedSubPkgFile, err := ioutil.ReadFile(filepath.Join(localRoot, "storage", "deployment.yaml"))
if !assert.NoError(t, err) {
t.FailNow()
}
if !assert.Equal(t, string(expectedSubPkgFile), string(actualSubPkgFile)) {
t.FailNow()
}

// the updated root package file must remain the same as this method should only merge subpackages
actualRootPkgFile, err := ioutil.ReadFile(filepath.Join(updatedRoot, "deployment.yaml"))
if !assert.NoError(t, err) {
t.FailNow()
}
expectedRootPkgFile, err := ioutil.ReadFile(filepath.Join(testDataDir, updatedDataSet, "deployment.yaml"))
if !assert.NoError(t, err) {
t.FailNow()
}
if !assert.Equal(t, string(expectedRootPkgFile), string(actualRootPkgFile)) {
t.FailNow()
}

// make sure that the updated new-setter definition in nosetters subpackage is pulled onto local
actualSubPkgKptfile, err := ioutil.ReadFile(filepath.Join(localRoot, "nosetters", "Kptfile"))
if !assert.NoError(t, err) {
t.FailNow()
}
expectedSubPkgKptfile, err := ioutil.ReadFile(filepath.Join(updatedRoot, "nosetters", "Kptfile"))
if !assert.NoError(t, err) {
t.FailNow()
}
if !assert.Equal(t, string(expectedSubPkgKptfile), string(actualSubPkgKptfile)) {
t.FailNow()
}

// delete the Kptfile in nosetters subpackage in the upstream and make sure it is retained on
// local
err = os.Remove(filepath.Join(updatedRoot, "nosetters", "Kptfile"))
if !assert.NoError(t, err) {
t.FailNow()
}

err = MergeSubPackages(localRoot, updatedRoot, originalRoot)
if !assert.NoError(t, err) {
t.FailNow()
}

actualSubPkgKptfile, err = ioutil.ReadFile(filepath.Join(localRoot, "nosetters", "Kptfile"))
if !assert.NoError(t, err) {
t.FailNow()
}

// make sure that the local Kptfile is retained even if the upstream Kptfile is deleted
if !assert.Equal(t, nosettersUpdated, string(actualSubPkgKptfile)) {
t.FailNow()
}

// delete the Kptfile in nosetters subpackage at the origin and make sure it is retained on
// local
err = os.Remove(filepath.Join(originalRoot, "nosetters", "Kptfile"))
if !assert.NoError(t, err) {
t.FailNow()
}

err = MergeSubPackages(localRoot, updatedRoot, originalRoot)
if !assert.NoError(t, err) {
t.FailNow()
}

actualSubPkgKptfile, err = ioutil.ReadFile(filepath.Join(localRoot, "nosetters", "Kptfile"))
if !assert.NoError(t, err) {
t.FailNow()
}

// make sure that the local Kptfile is retained even if the upstream Kptfile is deleted
if !assert.Equal(t, nosettersUpdated, string(actualSubPkgKptfile)) {
t.FailNow()
}
}

0 comments on commit e23c2a5

Please sign in to comment.