search

kubernetes-sigs / kustomize

Customization of kubernetes YAML configurations
Apache License 2.0
10.95k stars 2.24k forks source link

embed.FS support for kyaml/filesys #4515

Closed aw185176 closed 2 years ago

aw185176 commented 2 years ago

Is your feature request related to a problem? Please describe. We embed manifests into our Go binaries and then render them in code. Until now, that was all done using raw files, but we would like to use krusty so that code embedded in our binaries are rendered the same way as when someone runs kustomize build ... on those manifests locally.

Describe the solution you'd like

support for embed.FS in filesys

k8s-ci-robot commented 2 years ago

@aw185176: This issue is currently awaiting triage.

SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
KnVerey commented 2 years ago

/triage needs-information

Can you please explain what this would entail and provide an example of your desired usage of the filesys package?

aw185176 commented 2 years ago

I'm not a maintainer of the filesys package so I'm not sure I am the best person suited to outline what the implementation requirements are... but based on reading the package, it looks like the current implementations rely on an internal struct fsNode, so I assume something like fsnode_embed.go would be needed so that embedded file systems could be used in a first class way like in memory and on disk file systems are.

Currently we have a helper function which can convert an embed.FS into a filesys.FileSystem:

// ConvertEmbeddedFSToKustomizeFS performs a walk from the root of the provided
// embed.FS and copies the file tree into an in-memory Kustomize FileSystem,
// allowing the use of Kustomize APIs (build, etc) against embedded manifests.
func ConvertEmbeddedFSToKustomizeFS(efs embed.FS) (filesys.FileSystem, error) {
    fsys := filesys.MakeFsInMemory()
    err := fs.WalkDir(efs, ".", func(path string, d fs.DirEntry, walkErr error) error {
        // this shouldn't happe because we are traversing an embedded file system
        // from the root, but per io/fs we should handle this error explicitly
        if walkErr != nil {
            return fmt.Errorf("encountered a walk error: %w", walkErr)
        }

        if d.IsDir() {
            return fsys.Mkdir(path)
        }

        return writeEmbeddedFileToKustomizeFS(efs, fsys, path)
    })
    if err != nil {
        return nil, fmt.Errorf("failed to convert embedded file system to in-memory kustomize file system: %w", err)
    }

    return fsys, nil
}

func writeEmbeddedFileToKustomizeFS(efs embed.FS, kfs filesys.FileSystem, path string) error {
    data, err := efs.ReadFile(path)
    if err != nil {
        return fmt.Errorf("failed to read path %s from embedded file system: %w", path, err)
    }

    if err := kfs.WriteFile(path, data); err != nil {
        return fmt.Errorf("failed to write path %s to in-memory kustomize file system: %w", path, err)
    }
    return nil
}

Which we can then use to do something like...

fsys, err := util.ConvertEmbeddedFSToKustomizeFS(manifests)
    if err != nil {
        return nil, err
    }

    k := krusty.MakeKustomizer(
        &krusty.Options{LoadRestrictions: ktypes.LoadRestrictionsNone},
    )

    m, err := k.Run(fsys, o.kustomizeBuildTarget)
    if err != nil {
        return nil, fmt.Errorf("failed to execute kustomize build: %w", err)
    }

It would be nice if we could make a call similar to filesys.MakeFSInMemory, eg filesys.MakeFSFromEmbedded?

natasha41575 commented 2 years ago

It would be nice if we could make a call similar to filesys.MakeFSInMemory, eg filesys.MakeFSFromEmbedded?

Are you suggesting that filesys.MakeFSFromEmbedded takes in an embed.FS parameter? What I am imagining is something like

func MakeFSFromEmbedded(efs embed.FS) (FileSystem, error) {
   return convertEmbeddedFSToKustomizeFS(efs)
}

where the implementation of convertEmbeddedFSToKustomizeFS is the same as your ConvertEmbeddedFSToKustomizeFS. Is this what you are looking for?

KnVerey commented 2 years ago

Thanks for the explanation--I see what you mean now. filesys.FileSystem is an interface that currently has three implementations:

My first inclination would be a solution that adds an additional first-class implementation of the interface, e.g. an embedFsNode, instead of making a copy of everything into the in-memory implementation. But embed.FS is read-only, which means an embed.FS-based implementation is impossible. Another way to look at what you want to do is: you want the ability to initialize a filesys.FileSystem from an embed.FS. Looking at your code sample, I'm wondering if there's even anything embed.FS-specific about this feature. Perhaps a generic filesys.Copy(source, target FileSystem) would make sense?

I support this feature, though I don't have time to work on it myself. Would you be interested in contributing?

aw185176 commented 2 years ago

Thanks for the feedback everyone. Great point @KnVerey, I think a generic filesys.Copy(...) function would definitely fit the bill here. I think I am missing something though, how could I use filesys.Copy(...) in this scenario if my source is something like embed.FS instead of a FileSystem? Maybe if source was the FS interface from io/fs? I agree that my snippet should work for anything that implements the FS interface.

If I am on the same page as you all I would be happy to raise a PR adding this

KnVerey commented 2 years ago

I think I am missing something though, how could I use filesys.Copy(...) in this scenario if my source is something like embed.FS instead of a FileSystem? Maybe if source was the FS interface from io/fs?

Yes, totally. Probably fs.ReadFileFS specifically?

If I am on the same page as you all I would be happy to raise a PR adding this

Great, thanks! 🎉

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot commented 2 years ago

@k8s-triage-robot: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/kustomize/issues/4515#issuecomment-1214492737): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues and PRs according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue or PR with `/reopen` >- Mark this issue or PR as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.