feat: port generate-folders function from TS to Go#1216
feat: port generate-folders function from TS to Go#1216danish9039 wants to merge 5 commits intokptdev:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Ports the generate-folders KRM function into the Go functions catalog, implementing ResourceHierarchy→Folder generation (including v3 native refs), wiring up CLI/wasm entrypoints, and adding documentation plus Go-based unit/integration tests.
Changes:
- Added Go transformer implementation for v1/v2/v3 ResourceHierarchy processing and Folder manifest generation.
- Added Go test suite + YAML fixtures for v2/v3 scenarios (namespace, parent kinds, annotation filtering, normalization, deprecation warning).
- Added function packaging/entrypoints (main, run targets) and registered the function in
functions/go/Makefile.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| functions/go/generate-folders/transformer/generate_folders.go | Core Go implementation for parsing ResourceHierarchy and generating Folder resources |
| functions/go/generate-folders/transformer/generate_folders_test.go | Unit/integration tests for normalization, subtree expansion, and v2/v3 end-to-end behavior |
| functions/go/generate-folders/transformer/testdata/simple_v3.yaml | v3 basic hierarchy fixture |
| functions/go/generate-folders/transformer/testdata/v3_with_namespace.yaml | v3 fixture validating namespace + depends-on behavior |
| functions/go/generate-folders/transformer/testdata/v3_folder_parent.yaml | v3 fixture validating Folder parentRef |
| functions/go/generate-folders/transformer/testdata/v2_deprecation.yaml | v2 fixture validating deprecation warning + annotation-based refs |
| functions/go/generate-folders/transformer/testdata/missing_parent_ref.yaml | Fixture validating missing parentRef error result |
| functions/go/generate-folders/transformer/testdata/annotation_inheritance.yaml | Fixture validating annotation filtering/inheritance behavior |
| functions/go/generate-folders/main.go | CLI entrypoint |
| functions/go/generate-folders/run.go | Non-wasm runner wiring the transformer into fn.AsMain |
| functions/go/generate-folders/run_js.go | wasm/js runner wiring (processResourceList / processResourceListErrors) |
| functions/go/generate-folders/README.md | Function documentation and examples |
| functions/go/generate-folders/metadata.yaml | Function metadata (image, tags, sourceURL, license) |
| functions/go/generate-folders/generated/docs.go | Generated placeholder package for docs tooling |
| functions/go/generate-folders/go.mod | New Go module definition for the function |
| functions/go/generate-folders/go.sum | Dependency lockfile for the module |
| functions/go/Makefile | Registers generate-folders in the Go functions build/verify targets |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
functions/go/generate-folders/transformer/generate_folders_test.go
Outdated
Show resolved
Hide resolved
functions/go/generate-folders/go.mod
Outdated
| @@ -0,0 +1,35 @@ | |||
| module github.com/kptdev/krm-functions-catalog/functions/go/generate-folders | |||
|
|
|||
| go 1.25.6 | |||
There was a problem hiding this comment.
You should probably bump the go version to 1.25.7 as I assume we will be updating the other functions shortly as well.
There was a problem hiding this comment.
Bumped the module to Go 1.25.7.
| sourceURL: https://github.com/kptdev/krm-functions-catalog/tree/main/functions/go/generate-folders | ||
|
|
||
| emails: | ||
| - kpt-team@google.com |
There was a problem hiding this comment.
Pretty sure this email is no longer relevant, but I'm not sure we have any relevant mailing lists... (this applies to the rest of the functions too)
I would suggest just removing the emails field altogether for now.
There was a problem hiding this comment.
Removed the emails field for now.
functions/go/generate-folders/run.go
Outdated
| package main | ||
|
|
||
| import ( | ||
| transformer "github.com/kptdev/krm-functions-catalog/functions/go/generate-folders/transformer" |
There was a problem hiding this comment.
Cleaned this up. The explicit alias is gone and the file now uses the package name directly.
| import ( | ||
| "syscall/js" | ||
|
|
||
| transformer "github.com/kptdev/krm-functions-catalog/functions/go/generate-folders/transformer" |
There was a problem hiding this comment.
Same here. Removed the extra alias and used the package name directly.
| ## Function Invocation | ||
|
|
||
| ```shell | ||
| kpt fn eval --image ghcr.io/kptdev/krm-functions-catalog/generate-folders:unstable |
There was a problem hiding this comment.
The tag here should not be "unstable", either "latest" or whatever will be the actual tag once released (v0.1.0?)
There was a problem hiding this comment.
Updated the README example to use latest instead of unstable.
|
|
||
| root := &hierarchyNode{ | ||
| name: orgID, | ||
| kind: "Organization", |
There was a problem hiding this comment.
"Organization" should be a const
There was a problem hiding this comment.
Fixed this one too. Organization is now consistently referenced through a constant.
There was a problem hiding this comment.
Could you split this file up somehow? It is a bit long...
One chunk could definitely be the normalization related things (regexes, the functions themselves).
There was a problem hiding this comment.
I left the file split out of this PR. I wanted to keep the review scope on the behavioral fixes first. I can do the normalization split as a follow-up cleanup.
|
|
||
| // filterNonInheritableAnnotations removes non-inheritable annotations. | ||
| func filterNonInheritableAnnotations(annotations map[string]string) map[string]string { | ||
| if annotations == nil { |
There was a problem hiding this comment.
| if annotations == nil { | |
| if len(annotations) == 0 { |
There was a problem hiding this comment.
Please use testify's assert and require packages instead of t.Errorf and t.Fatalf.
There was a problem hiding this comment.
Updated the tests to use testify assert and require.
| return data | ||
| } | ||
|
|
||
| // Integration tests using full YAML ResourceList inputs |
There was a problem hiding this comment.
Would these bigger tests not be better placed in the E2E tests?
There was a problem hiding this comment.
I moved the larger fixture-driven checks into repo E2E cases under tests/generate-folders and kept the transformer test file focused on unit-level behavior.
❌ Deploy Preview for krm-function-catalog failed. Why did it fail? →
|
b2c4b46 to
a60d169
Compare
|
@mozesl-nokia PTAL |
|
@danish9039 please address the DCO issue: https://github.com/kptdev/krm-functions-catalog/pull/1216/checks?check_run_id=65266203488 |
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
- fix(v1): thread path for proper nested folder naming - fix(compat): remove subtree sorting to match TS implementation - fix(errors): improve error messages for subtree processing - docs: clarify v1 annotation inheritance limitation - test: add v1 integration test with nested hierarchy Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
a60d169 to
01d6282
Compare
|
Rewrote the branch history with sign-offs on all PR commits and force-pushed the branch. The |
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
This PR ports the
generate-foldersKRM function from TypeScript to Go, adding it to thefunctions/godirectory.solves
Changes
transformer/generate_folders.gotransformer/generate_folders_test.gocovering:js,wasmtargetfunctions/go/MakefileVerification
make func-verify(passed)GOOS=js GOARCH=wasm go build(passed)