Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow recursive dry-run over local sources #5219

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

niveau0
Copy link

@niveau0 niveau0 commented Feb 27, 2025

This PR tries to fix the issue using flux build with dry-run and recursion.

We have a setup with nested kustomizations and want to render the whole tree
as yaml files, including postBuild substitutions. Afterwards, this should be
checked via kube-linter.

Currently this seems not possible(?) with the flux-cli together with dry-run,
therefore I made some changes.
Since I don't know all the use cases for flux build, it is hard to tell if this
breaks anything.

Maybe someone with deeper knowledge can review the changes.

The command I use is similar to this:

flux build kustomization flux-system \
--recursive \
--path ./clusters/dev/mykustomizations \
-n flux-system \
--local-sources GitRepository/flux-system/flux-system=./ \
--dry-run \
--kustomization-file ./clusters/dev/flux-system/gotk-sync.yaml

@niveau0 niveau0 force-pushed the allow-recursive-dry-run branch 2 times, most recently from 3d1e964 to 04b5ef1 Compare March 5, 2025 07:57
@bkreitch
Copy link
Contributor

Seems reasonable to me. I would just switch branches of if b.dryRun and else in build() and revert blank lines except at line 370. And add following test to cmd/flux/build_kustomization_test.go in TestBuildLocalKustomization:

{
	name:       "build with recursive in dry-run mode",
	args:       "build kustomization podinfo --kustomization-file " + tmpFile + " --path ./testdata/build-kustomization/podinfo-with-my-app --recursive --local-sources GitRepository/default/podinfo=./testdata/build-kustomization --dry-run",
	resultFile: "./testdata/build-kustomization/podinfo-with-my-app-result.yaml",
	assertFunc: "assertGoldenTemplateFile",
},

@niveau0
Copy link
Author

niveau0 commented Mar 17, 2025

@bkreitch , thanks for the advice, I will add a test. But I'm not sure what you mean with the if and else branches, since the if and else in the PR are not related?

@niveau0 niveau0 force-pushed the allow-recursive-dry-run branch 3 times, most recently from a80f266 to 6d5f7ab Compare March 17, 2025 09:43
@bkreitch
Copy link
Contributor

But I'm not sure what you mean with the if and else branches, since the if and else in the PR are not related?

@niveau0 I mean to reverse if condition so it will be like:

if b.dryRun {
	liveKus = b.kustomization
} else {
	liveKus, err = b.getKustomization(ctx)
	if err != nil {
...

@bkreitch
Copy link
Contributor

@niveau0 and for the test - I think that if you'll put my snippet from above into TestBuildLocalKustomization() instead of TestBuildKustomization() it should be enough and additional files are not needed.

Signed-off-by: niveau0 <plingplong@t-online.de>
@niveau0 niveau0 force-pushed the allow-recursive-dry-run branch from 6d5f7ab to c0e6b98 Compare March 17, 2025 12:33
@niveau0
Copy link
Author

niveau0 commented Mar 17, 2025

@niveau0 and for the test - I think that if you'll put my snippet from above into TestBuildLocalKustomization() instead of TestBuildKustomization() it should be enough and additional files are not needed.

@bkreitch ah ok got it now, I think I failed to see the 'Local' part in TestBuildLocalKustomization and got confused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants