Skip to content

refactor: add GenerateManifest to decouple generating kustomizations from writing#1153

Merged
stefanprodan merged 1 commit intofluxcd:mainfrom
rycli:fix/in-memory-kustomize-vfs
Apr 9, 2026
Merged

refactor: add GenerateManifest to decouple generating kustomizations from writing#1153
stefanprodan merged 1 commit intofluxcd:mainfrom
rycli:fix/in-memory-kustomize-vfs

Conversation

@rycli
Copy link
Copy Markdown
Contributor

@rycli rycli commented Mar 29, 2026

One half of changes necessary to resolve fluxcd/flux2#5781 ; foundation needed for the changes introduced by fluxcd/flux2#5794

These changes to kustomize_generator decouple the file reading/generating logic from the writing and cleanup steps.

The WriteFile function keeps the same behavior, so no change to existing callers; but it now wraps GenerateManifest internally which extracts the logic for generating kustomizations. This allows calling the generating logic directly without writing the files :-)

@rycli rycli force-pushed the fix/in-memory-kustomize-vfs branch 2 times, most recently from 8bc7bfb to 94b031e Compare March 29, 2026 19:21
@stefanprodan stefanprodan added enhancement New feature or request area/kustomize Kustomize related issues and pull requests labels Mar 30, 2026
@rycli rycli force-pushed the fix/in-memory-kustomize-vfs branch 2 times, most recently from 5dc2b1c to d3d47ff Compare April 2, 2026 12:02
@matheuscscp
Copy link
Copy Markdown
Member

Codex found the following:

The new overlay filesystem does not actually preserve "memory layer wins" semantics when a path changes type across layers.

In kustomize/filesys/fs_memory.go, IsDir, ReadDir, and Walk all combine memory and disk results instead of treating the memory node as authoritative once it exists. A concrete failure case is:

  1. memory contains foo as a file
  2. disk contains foo/ as a directory

Then:

  • IsDir("foo") returns true
  • ReadDir("foo") can return the disk directory entries
  • Walk("foo") can still descend into the disk subtree even though the root path was shadowed by the memory file

That breaks the documented overlay contract and can surface resources from disk that should be hidden.

I think this needs dedicated tests for file-over-directory and directory-over-file shadowing before merging.

@rycli
Copy link
Copy Markdown
Contributor Author

rycli commented Apr 9, 2026

Thanks for taking a look @matheuscscp ! If I understand the comment correctly, this is not a bug (per se); mainly comes down to what behavior we'd want to enable. I have to admit that the stated failure case doesn't make a ton of sense to me, in context.

The intention isn't to offer a "free-form" virtual filesystem for general use (that's already provided by kustomize.MakeFsInMemory); it's specifically to isolate writes of [missing] Kustomization manifests, in cases where writing to the actual filesystem isn't desirable. In that case, the behavior of "underlying filesystem reads are always available" is desirable; we should only ever be writing where there is no file. If we end up overwriting existing paths, something is going wrong elsewhere (i.e., how did you end up with the "foo" file in memory, when there was already a same-name directory there?).

I can whip up some additional "edge case" checks and/or tests at the virtual FS layer, but I wonder if that isn't just additional noise at this moment 🤔

@matheuscscp
Copy link
Copy Markdown
Member

Yep, agreed!

Copy link
Copy Markdown
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks @rycli 🏅

PS. Please rebase and force push so we can merge.

@rycli rycli force-pushed the fix/in-memory-kustomize-vfs branch from 5700ab4 to e0f6ecc Compare April 9, 2026 11:04
@stefanprodan
Copy link
Copy Markdown
Member

@rycli did you run the tests locally?

@stefanprodan
Copy link
Copy Markdown
Member

Also please squash all those fixes into the parent commit

@rycli
Copy link
Copy Markdown
Contributor Author

rycli commented Apr 9, 2026

@rycli did you run the tests locally?

My last changes were passing locally, but I didn't manually test after the rebase (done via GitHub). I'll take a look at what's going wrong tonight; sorry about that!

@stefanprodan
Copy link
Copy Markdown
Member

The error from CI is:

# github.com/fluxcd/pkg/kustomize [github.com/fluxcd/pkg/kustomize.test]
Error: ./kustomize_generator.go:317:18: not enough return values
	have (Action, error)
	want ([]byte, string, Action, error)

Signed-off-by: Cyril Mengin <cyril@ryc.li>
@rycli rycli force-pushed the fix/in-memory-kustomize-vfs branch from e0f6ecc to 4bb52f7 Compare April 9, 2026 19:10
@rycli
Copy link
Copy Markdown
Contributor Author

rycli commented Apr 9, 2026

@stefanprodan fixed!

515925b added a reference to the old argument signature, and therefore weren't included in my update. Tests are now passing for me locally :-)

I've also squashed the commits as requested.

@stefanprodan stefanprodan merged commit 63d73fd into fluxcd:main Apr 9, 2026
14 checks passed
@rycli rycli deleted the fix/in-memory-kustomize-vfs branch April 9, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/kustomize Kustomize related issues and pull requests enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

'flux build kustomization' corrupts files when run concurrently

3 participants