Skip to content

refactor: absorb thirdparty packages into pkg and remove thirdparty folder (#4554) - #4739

Closed
SurbhiAgarwal1 wants to merge 3 commits into
kptdev:mainfrom
SurbhiAgarwal1:feat/4554-refactor-remove-thirdparty
Closed

SurbhiAgarwal1 wants to merge 3 commits into
kptdev:mainfrom
SurbhiAgarwal1:feat/4554-refactor-remove-thirdparty

Conversation

@SurbhiAgarwal1

Copy link
Copy Markdown
Contributor

Description

This pull request refactors the thirdparty/ directory in the repository by absorbing the customized packages into pkg/ and completely removing the thirdparty/ folder:

  • Relocated thirdparty/kyaml/runfn to pkg/fn/runfn.
  • Relocated thirdparty/cmdconfig/commands/* (cmdcat, cmdeval, cmdsink, cmdsource, cmdtree, runner) to pkg/cmdconfig/commands/*.
  • Updated package import paths across commands/fn/fncmd.go, commands/pkg/pkgcmd.go, and internal cross-package dependencies.
  • Updated .golangci.yml, sonar.properties, .github/copilot-code-review.yml, and AGENTS.md to remove thirdparty/ path exclusions.
  • Completely removed the thirdparty/ directory.

All original Apache 2.0 copyright and license headers have been preserved.

Motivation

As outlined in issue #4554 (part of EPIC #4552: Clean up the kpt repo), the code in thirdparty/ is no longer a set of unmodified upstream dependencies. Over time, packages like runfn and cmdconfig/commands were heavily customized to support kpt-specific features (such as kptfile/v1 handling, CEL conditional execution, and custom Cobra command runners).

Absorbing these packages into pkg/ formally establishes kpt's ownership, simplifies the repository layout, and allows linters and static analysis tools (golangci-lint, SonarQube) to cover the entire codebase cleanly.

Issue Fixed

Fixes #4554

Verification

  • go fmt and go vet executed cleanly without errors.
  • go test ./pkg/fn/runfn/... ./pkg/cmdconfig/commands/... passed 100%.
  • go build compiled kpt binary cleanly.

SurbhiAgarwal1 added 2 commits September 1, 2026 16:35
- Consolidate CLI and Porch klog printer logic into an exported Printer interface in pkg/printer.
- Introduce stateful contextual scoping via WithField, WithFields, WithPackage, and WithFunction.
- Provide structured lifecycle event printing (PrintRunning, PrintPass, PrintFail, PrintResult, PrintSummary).
- Remove duplicate packagePrinter implementation in pkg/lib/kptops/render.go.
- Add comprehensive unit tests verifying stateful field scoping and formatting.

Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
…older (kptdev#4554)

Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
@SurbhiAgarwal1
SurbhiAgarwal1 requested review from a team and a balanced review from Copilot September 8, 2026 16:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@netlify

netlify Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for kptdocs ready!

Name Link
🔨 Latest commit c48ada7
🔍 Latest deploy log https://app.netlify.com/projects/kptdocs/deploys/6aa0e458a2cbcd000928161e
😎 Deploy Preview https://deploy-preview-4739--kptdocs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Signed-off-by: Surbhi Agarwal <SurbhiAgarwal1@users.noreply.github.com>
@liamfallon

Copy link
Copy Markdown
Contributor

The third party directory originally came from an external source, we could use the external library and just add the extra fields we need to it.

@mozesl-nokia will comment further.

@SurbhiAgarwal1

Copy link
Copy Markdown
Contributor Author

Hi @liamfallon!

Thanks for the feedback!

Regarding using the external library directly vs. absorbing into pkg/:
Over time, the code in thirdparty/ drifted significantly from upstream kustomize/kyaml to support kpt-specific functionality (like Kptfile/v1 handling, CEL --when conditional execution, and pkg/printer integration). Because of these deep customizations, wrapping upstream directly without breaking kpt features would be very complex.

Absorbing into pkg/ (per issue #4554 under EPIC #4552) formalizes kpt's ownership and brings the code under golangci-lint and test coverage.

Looking forward to hearing thoughts from @mozesl-nokia as well!

@mozesl-nokia

Copy link
Copy Markdown
Contributor

@SurbhiAgarwal1 @liamfallon

Sorry for the late reply here as well;

I was expecting the diff to be too great, I think it is fine as long as we keep that original kubernetes copyrights as well as adding our own line.

However, I think we should be moving these to their logical places, not just dumping them into pkg. For example, eval is kpt fn eval, it should live in commands/fn/eval and tree is kpt pkg tree in commands/pkg/tree, etc. This will greatly help us separating the CLI into it's own module later as well.

@liamfallon

liamfallon commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Superseded by #4811

@liamfallon liamfallon closed this Oct 27, 2026
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.

Need to refactor / remove the "thirdparty" folder - refer to upstream or absorb

4 participants