From 57442e8faa9079cba879ac3c9208b93f07a9a372 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 19 Apr 2022 21:22:24 +0200 Subject: [PATCH] kustomize: use FS from `fluxcd/pkg` This switches to a secure FS implementation in most places, except for where we can not make changes at this moment because it would break behavior. Not handled in this commit: - Allowing the root for `manifestgen` packages to be configured. - Allowing the user to define a working root while building locally. - Defaulting to the secure FS implementation in `kustomization.MakeDefaultOptions`. Problem here is that constructing the secure FS could result in an error, which we can not surface without signature changes to the constructor func. Signed-off-by: Hidde Beydals --- go.mod | 2 +- go.sum | 4 +- internal/bootstrap/bootstrap_plain_git.go | 16 ++++++-- internal/build/build.go | 8 +++- pkg/manifestgen/install/manifests.go | 10 +++-- .../kustomization/kustomization.go | 41 +++++++++++++------ pkg/manifestgen/kustomization/options.go | 2 + 7 files changed, 59 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index 1129ed0f..b2e87d20 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/fluxcd/kustomize-controller/api v0.24.4 github.com/fluxcd/notification-controller/api v0.23.4 github.com/fluxcd/pkg/apis/meta v0.12.2 - github.com/fluxcd/pkg/kustomize v0.2.0 + github.com/fluxcd/pkg/kustomize v0.4.0 github.com/fluxcd/pkg/runtime v0.14.1 github.com/fluxcd/pkg/ssa v0.15.2 github.com/fluxcd/pkg/ssh v0.3.2 diff --git a/go.sum b/go.sum index 00541439..67ea07dc 100644 --- a/go.sum +++ b/go.sum @@ -391,8 +391,8 @@ github.com/fluxcd/pkg/apis/kustomize v0.3.3 h1:bPN29SdVzWl0yhgivuf/83IAe2R6vUuDV github.com/fluxcd/pkg/apis/kustomize v0.3.3/go.mod h1:5HTOFZfQFVMMqR2rvuxpbZhpb+sQpcTT6RCQZOhjFzA= github.com/fluxcd/pkg/apis/meta v0.12.2 h1:AiKAZxLyPtV150y63WC+mL1Qm4x5qWQmW6r4mLy1i8c= github.com/fluxcd/pkg/apis/meta v0.12.2/go.mod h1:Z26X5uTU5LxAyWETGueRQY7TvdPaGfKU7Wye9bdUlho= -github.com/fluxcd/pkg/kustomize v0.2.0 h1:twiGAFJctt2tyH8vHxL1uqb6BlU3B9ZqG8uSlluuioM= -github.com/fluxcd/pkg/kustomize v0.2.0/go.mod h1:Qczvl7vNY9NJBpyaFrldsxfGjj6uaMcMmKGsSJ6hcxc= +github.com/fluxcd/pkg/kustomize v0.4.0 h1:ct1YGrO/o++NVecAyeOxd3/YzGNvRdslaE6pmWxBt0M= +github.com/fluxcd/pkg/kustomize v0.4.0/go.mod h1:ZlTFhk6Cxtmf1T7tpYuTACmMfqDdsPcXUGRAq3t9fH8= github.com/fluxcd/pkg/runtime v0.14.1 h1:ZbS3RzR+f+wu1e6Y7GoCxY9PFZkOgX6/gL7Enr75CY0= github.com/fluxcd/pkg/runtime v0.14.1/go.mod h1:eS4378ydLlWPt2fFjcrAAnJegGJNj3Q/iqYZqjBeWlM= github.com/fluxcd/pkg/ssa v0.15.2 h1:hLEIh7Ymlt6ihfQHIEx7DjAa+FCndBpHW6wyELToVsI= diff --git a/internal/bootstrap/bootstrap_plain_git.go b/internal/bootstrap/bootstrap_plain_git.go index ea4895fd..863c4cd7 100644 --- a/internal/bootstrap/bootstrap_plain_git.go +++ b/internal/bootstrap/bootstrap_plain_git.go @@ -31,11 +31,11 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" "sigs.k8s.io/cli-utils/pkg/object" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/yaml" kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1beta2" + "github.com/fluxcd/pkg/kustomize/filesys" runclient "github.com/fluxcd/pkg/runtime/client" "github.com/fluxcd/flux2/internal/bootstrap/git" @@ -105,7 +105,7 @@ func NewPlainGitProvider(git git.Git, kube client.Client, opts ...GitOption) (*P return b, nil } -func (b *PlainGitBootstrapper) ReconcileComponents(ctx context.Context, manifestsBase string, options install.Options, secretOpts sourcesecret.Options) error { +func (b *PlainGitBootstrapper) ReconcileComponents(ctx context.Context, manifestsBase string, options install.Options, _ sourcesecret.Options) error { // Clone if not already if _, err := b.git.Status(); err != nil { if err != git.ErrNoGitRepository { @@ -263,13 +263,21 @@ func (b *PlainGitBootstrapper) ReconcileSyncConfig(ctx context.Context, options if err = b.git.Write(manifests.Path, strings.NewReader(manifests.Content)); err != nil { return fmt.Errorf("failed to write manifest %q: %w", manifests.Path, err) } + + // Create secure Kustomize FS + fs, err := filesys.MakeFsOnDiskSecureBuild(b.git.Path()) + if err != nil { + return fmt.Errorf("failed to initialize Kustomize file system: %w", err) + } + + // Generate Kustomization kusManifests, err := kustomization.Generate(kustomization.Options{ - FileSystem: filesys.MakeFsOnDisk(), + FileSystem: fs, BaseDir: b.git.Path(), TargetPath: filepath.Dir(manifests.Path), }) if err != nil { - return fmt.Errorf("kustomization.yaml generation failed: %w", err) + return fmt.Errorf("%s generation failed: %w", konfig.DefaultKustomizationFileName(), err) } if err = b.git.Write(kusManifests.Path, strings.NewReader(kusManifests.Content)); err != nil { return fmt.Errorf("failed to write manifest %q: %w", kusManifests.Path, err) diff --git a/internal/build/build.go b/internal/build/build.go index df51db5b..74fd34d7 100644 --- a/internal/build/build.go +++ b/internal/build/build.go @@ -37,11 +37,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" - "sigs.k8s.io/kustomize/kyaml/filesys" "sigs.k8s.io/kustomize/kyaml/yaml" kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1beta2" "github.com/fluxcd/pkg/kustomize" + "github.com/fluxcd/pkg/kustomize/filesys" runclient "github.com/fluxcd/pkg/runtime/client" "github.com/fluxcd/flux2/internal/utils" @@ -275,7 +275,11 @@ func (b *Builder) generate(kustomization kustomizev1.Kustomization, dirPath stri } func (b *Builder) do(ctx context.Context, kustomization kustomizev1.Kustomization, dirPath string) (resmap.ResMap, error) { - fs := filesys.MakeFsOnDisk() + // TODO(hidde): provide option to enforce FS boundaries of local build + fs, err := filesys.MakeFsOnDiskSecureBuild("/") + if err != nil { + return nil, fmt.Errorf("kustomization build failed: %w", err) + } // acuire the lock b.mu.Lock() diff --git a/pkg/manifestgen/install/manifests.go b/pkg/manifestgen/install/manifests.go index 3a584ba1..17fc33f1 100644 --- a/pkg/manifestgen/install/manifests.go +++ b/pkg/manifestgen/install/manifests.go @@ -26,8 +26,8 @@ import ( "path/filepath" "strings" + "github.com/fluxcd/pkg/kustomize/filesys" "github.com/fluxcd/pkg/untar" - "sigs.k8s.io/kustomize/api/filesys" "github.com/fluxcd/flux2/pkg/manifestgen/kustomization" ) @@ -126,8 +126,12 @@ func build(base, output string) error { return err } - fs := filesys.MakeFsOnDisk() - if err := fs.WriteFile(output, resources); err != nil { + outputBase := filepath.Dir(strings.TrimSuffix(output, string(filepath.Separator))) + fs, err := filesys.MakeFsOnDiskSecure(outputBase) + if err != nil { + return err + } + if err = fs.WriteFile(output, resources); err != nil { return err } diff --git a/pkg/manifestgen/kustomization/kustomization.go b/pkg/manifestgen/kustomization/kustomization.go index 676f64b7..45a40315 100644 --- a/pkg/manifestgen/kustomization/kustomization.go +++ b/pkg/manifestgen/kustomization/kustomization.go @@ -20,20 +20,23 @@ import ( "fmt" "os" "path/filepath" + "strings" "sync" - "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/krusty" "sigs.k8s.io/kustomize/api/provider" kustypes "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/yaml" + "github.com/fluxcd/pkg/kustomize/filesys" + "github.com/fluxcd/flux2/pkg/manifestgen" ) -// Generate scans the given directory for Kubernetes manifests and creates a kustomization.yaml -// including all discovered manifests as resources. +// Generate scans the given directory for Kubernetes manifests and creates a +// konfig.DefaultKustomizationFileName file, including all discovered manifests +// as resources. func Generate(options Options) (*manifestgen.Manifest, error) { kfile := filepath.Join(options.TargetPath, konfig.DefaultKustomizationFileName()) abskfile := filepath.Join(options.BaseDir, kfile) @@ -50,7 +53,7 @@ func Generate(options Options) (*manifestgen.Manifest, error) { return nil } if info.IsDir() { - // If a sub-directory contains an existing Kustomization file add the + // If a sub-directory contains an existing Kustomization file, add the // directory as a resource and do not decent into it. for _, kfilename := range konfig.RecognizedKustomizationFileNames() { if options.FileSystem.Exists(filepath.Join(path, kfilename)) { @@ -88,7 +91,9 @@ func Generate(options Options) (*manifestgen.Manifest, error) { if err != nil { return nil, err } - f.Close() + if err = f.Close(); err != nil { + return nil, err + } kus := kustypes.Kustomization{ TypeMeta: kustypes.TypeMeta{ @@ -128,20 +133,32 @@ func Generate(options Options) (*manifestgen.Manifest, error) { }, nil } +// kustomizeBuildMutex is a workaround for a concurrent map read and map write bug. +// TODO(stefan): https://github.com/kubernetes-sigs/kustomize/issues/3659 var kustomizeBuildMutex sync.Mutex -// Build takes a Kustomize overlays and returns the resulting manifests as multi-doc YAML. +// Build takes the path to a directory with a konfig.RecognizedKustomizationFileNames, +// builds it, and returns the resulting manifests as multi-doc YAML. func Build(base string) ([]byte, error) { - // TODO(stefan): temporary workaround for concurrent map read and map write bug - // https://github.com/kubernetes-sigs/kustomize/issues/3659 kustomizeBuildMutex.Lock() defer kustomizeBuildMutex.Unlock() - kfile := filepath.Join(base, konfig.DefaultKustomizationFileName()) + // TODO(hidde): make this configurable to a specific root (relative to base) + parent := filepath.Dir(strings.TrimSuffix(base, string(filepath.Separator))) + fs, err := filesys.MakeFsOnDiskSecureBuild(parent) + if err != nil { + return nil, err + } - fs := filesys.MakeFsOnDisk() - if !fs.Exists(kfile) { - return nil, fmt.Errorf("%s not found", kfile) + var kfile string + for _, f := range konfig.RecognizedKustomizationFileNames() { + if kf := filepath.Join(base, f); fs.Exists(kf) { + kfile = kf + break + } + } + if kfile == "" { + return nil, fmt.Errorf("%s not found", konfig.DefaultKustomizationFileName()) } // TODO(hidde): work around for a bug in kustomize causing it to diff --git a/pkg/manifestgen/kustomization/options.go b/pkg/manifestgen/kustomization/options.go index 57ffb9bd..c8cf0955 100644 --- a/pkg/manifestgen/kustomization/options.go +++ b/pkg/manifestgen/kustomization/options.go @@ -25,6 +25,8 @@ type Options struct { } func MakeDefaultOptions() Options { + // TODO(hidde): switch MakeFsOnDisk to MakeFsOnDiskSecureBuild when we + // break API. return Options{ FileSystem: filesys.MakeFsOnDisk(), BaseDir: "",